Skip to content

manifest.yaml: Add content_manifest folder to add <nvr>.json for build info #670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gursewak1997 wants to merge 1 commit into from
Closed

manifest.yaml: Add content_manifest folder to add <nvr>.json for build info #670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions manifest.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,29 @@ postprocess:
# NB: we don't use -f here so we break when this is no longer needed
rm -v /etc/iscsi/initiatorname.iscsi

# Add content_manifests dir to have json files for build info
# See https://issues.redhat.com/browse/GRPA-3731
- |
#!/usr/bin/env bash
Comment thread
jlebon marked this conversation as resolved.
set -euo pipefail

REPOS=`jq .repos /usr/share/rpm-ostree/treefile.json`
ARCH=`jq .basearch /usr/share/rpm-ostree/treefile.json | tr -d '"'`
Copy link
Copy Markdown
Member

@jlebon jlebon Nov 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jq -r will print the string without quotes. Or you should be be able to just use $(arch) for this.


. /etc/os-release
mkdir -p /usr/share/buildinfo/content_manifests && cd "$_" && touch $ID-$OSTREE_VERSION.json
# Make an empty json and add all the relevant fields
echo "$(var=$REPOS; jq --argjson repos "$var" '. +=
Copy link
Copy Markdown
Member

@jlebon jlebon Nov 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't entirely follow the jq syntax here, though one thing to note is that the repo names need to be translated to those in https://access.redhat.com/security/data/metrics/repository-to-cpe.json which I think is missing here.

The actual set of repos we use doesn't really change that often, so for this first iteration, we could just hardcode the fact that e.g. rhel-8-baseos maps to rhel-8-for-$ARCH-baseos-rpms, rhel-8-appstream to rhel-8-for-$ARCH-appstream-rpms, etc... and error out if we get a repo name we don't know.

Maybe cleaner eventually is to make that mapping part of the repo files themselves and add a treefile option to teach rpm-ostree to extract it and generate the JSON file and potentially even validate against https://access.redhat.com/security/data/metrics/repository-to-cpe.json.

Copy link
Copy Markdown
Contributor Author

@gursewak1997 gursewak1997 Nov 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mapping the repo names to their pulp repo IDs (ie, "[rhel-8-baseos]" to "[rhel-8-for-x86_64-baseos-eus-rpms__8_DOT_4]") would be a bit more complex task to put in postprocess so we decided to not do that here. I have just added the given repo names for now so that we can access them when mapping them to repository-to-cpe.json
So far, we are just landing the changes to create the relevant DIR/JSON file and symlinking it to /root.

Copy link
Copy Markdown
Member

@jlebon jlebon Nov 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a chat with @gursewak1997 about this. I think in the end doing this in a postprocess script is probably not the right approach. We could hardcode equivalencies here, though the second we change the repo definition files in the internal redhat-coreos repo, it'll become incorrect data. Because of this, I think this data belongs best sitting alongside the repo files.

Apart from the rpm-ostree suggestion in the comment above, another easier option is a YAML file similar to the official content_sets.yml currently in use (internal example):

# content_sets.yaml
repo_mapping:
  rhel-8-baseos:
    name: rhel-8-for-$ARCH-baseos-eus-rpms__8_DOT_4
  rhel-8-appstream:
    name: rhel-8-for-$ARCH-appstream-eus-rpms__8_DOT_4
  ...

(This would live in the redhat-coreos repo so it could be maintained atomically with the repo definitions and would be similarly copied into the workdir before builds.)

And then cosa build would check for the existence of that file and auto-generate the JSON file to inject in the OS.

Copy link
Copy Markdown
Member

@miabbott miabbott Nov 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was that Gursewak was going to work on the tooling to generate the <nvr>.json file and we would handle the problem of the repo names separately. The last working idea was just to rename all the repos to match what is in the repository-to-cpe.json file after the tooling was in place.

The YAML mapping file would also work; either way we are going to incur some amount of manual cost to maintain that the repo names are valid according to repository-to-cpe.json.

Copy link
Copy Markdown
Member

@miabbott miabbott Nov 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spoke with @jlebon today and we discussed various approaches to this, but ultimately settled on the proposal that Jonathan made in his last comment:

  • a YAML doc that lives alongside the repo files which maps repo names to names in repository-to-cpe.json
    • additionally should include the baseurl so we can detect when that changes
  • changes to cosa build that can consume the mapping YAML doc and produce the <nvr>.json file
  • (future project) CI job on openshift/release that enforces changing the mapping YAML doc when changing the repo files

@gursewak1997 if you have any additional questions/concerns, please let us know

{
"metadata": {
"icm_version": 1,
"icm_spec": "https://raw.githubusercontent.com/containerbuildsystem/atomic-reactor/master/atomic_reactor/schemas/content_manifest.json",
"image_layer_index": 1
},
"content_sets": $repos,
"image_contents": []
}' $ID-$OSTREE_VERSION.json)" > $ID-$OSTREE_VERSION.json

etc-group-members:
- wheel
- sudo
Expand Down
10 changes: 10 additions & 0 deletions overlay.d/05rhcos/usr/lib/tmpfiles.d/buildinfo.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# This file is part of systemd.
Copy link
Copy Markdown
Member

@jlebon jlebon Nov 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From #670 (comment) it seems like we don't need this.

Copy link
Copy Markdown
Contributor Author

@gursewak1997 gursewak1997 Nov 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True but @miabbott suggested on going through with the creation of .conf file and symlinking the JSON file to /roothome.

#
# systemd is free software; you can redistribute it and/or modify it
# under the terms of the GNU Lesser General Public License as published by
# the Free Software Foundation; either version 2.1 of the License, or
# (at your option) any later version.

# See tmpfiles.d(5) for details

L /var/roothome/buildinfo - - - - /usr/share/buildinfo