Skip to content
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.

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

dnfjson: support new osbuild-depsolve-dnf options for loading repositories from a root dir #537

Merged
merged 18 commits into from
Apr 11, 2024

Conversation

achilleas-k
Copy link
Member

@achilleas-k achilleas-k commented Mar 20, 2024

Support setting a repos directory on the Solver and add it to the depsolve request when set.

Depends on osbuild/osbuild#1674

@bcl
Copy link
Contributor

bcl commented Mar 25, 2024

Looks ok

@achilleas-k achilleas-k force-pushed the dnfjson/repos-from-path branch 2 times, most recently from fb2486d to c5fdf18 Compare April 3, 2024 20:54
@achilleas-k achilleas-k changed the title dnfjson: SetReposDir() on Solver and request dnfjson: support new osbuild-depsolve-dnf options for loading repositories from a root dir Apr 3, 2024
@achilleas-k
Copy link
Member Author

This PR is basically ready now. It just needs the osbuild PR to be merged and then we can update the osbuild dependency to test against it.

ondrejbudai added a commit to ondrejbudai/osbuild-deploy-container that referenced this pull request Apr 9, 2024
The latest goodies from
- osbuild/images#537
- osbuild/osbuild#1674

This is a horrible hack, let's remove these when they land
@achilleas-k achilleas-k force-pushed the dnfjson/repos-from-path branch 3 times, most recently from f9670b4 to 24897f0 Compare April 11, 2024 12:51
@achilleas-k achilleas-k marked this pull request as ready for review April 11, 2024 12:52
@achilleas-k
Copy link
Member Author

This is ready now. There's a couple of things I'd like to do for cleanup but I'd rather we hold them for a follow-up to unblock other work that depends on this.

  • Deduplicate functions in cmd/ (there's an old issue for this: Minimise code duplication in cmd/ utilities #88)
  • Squash the serialize() arguments into a struct for resolved content.
  • Deduplicate rpm repos in pipelines, or only set repos after depsolving (during serialize) and not during instantiation.

@achilleas-k
Copy link
Member Author

achilleas-k commented Apr 11, 2024

GitHub unit tests don't install the osbuild from Schutzfile. :/

@achilleas-k achilleas-k force-pushed the dnfjson/repos-from-path branch 2 times, most recently from c3acf23 to 79928e5 Compare April 11, 2024 13:37
croissanne
croissanne previously approved these changes Apr 11, 2024
@croissanne
Copy link
Member

thank you ever so much <3

@ondrejbudai
Copy link
Member

Gimme a sec, I want to test this E2E in bootc-image-builder. :)

ondrejbudai
ondrejbudai previously approved these changes Apr 11, 2024
cmd/build/main.go Show resolved Hide resolved
cmd/gen-manifests/main.go Show resolved Hide resolved
cmd/gen-manifests/main.go Show resolved Hide resolved
Some repo properties were renamed in osbuild-depsolve-dnf to match the
equivalent ones in dnf (libdnf).

Of note is the SSLVerify property which was previously called IgnoreSSL
and had the opposite effect.  The nil value for SSLVerify falls back to
'true' in osbuild-depsolve-dnf which maintains the same behaviour.  We
make the new SSLVerify property a pointer so that we can express the
unset/undefined/nil value properly and let osbuild-depsolve-dnf define
the fallback value.

This change breaks backwards compatibility.
A new version of osbuild-depsolve-dnf returns, for the depsolve command,
an object with two keys:
- packages: a list of resolved packages which is the same as the old
  response format.
- repos: a map (id -> config) of repositories used to resolve the
  packages.  This map contains both repositories defined in the requests
  and repositories loaded from a directory, which is a new feature that
  osbuild-depsolve-dnf supports.

Currently, the response handler ignores the repos part of the response
and only returns the package list, so this has no functional effect
outside the dnfjson package.

See osbuild/osbuild#1674
Add the new root_dir argument and releaever value to the depsolve
request.  The releasever is (currently) only required when root_dir is
set, but it will eventually be required for all requests so let's set it
always.
The docstring was describing an older implementation of the function.
Now that the response contains packages and repository configurations we
can do the conversion to rpmmd.PackageSet on the request itself directly
without needing extra repo configs.

This commit doesn't complete the move but simply moves the method to the
depsolve response.
Use the repository configurations that osbuild-depsolve-dnf returns to
populate extra repo-related metadata on each package (like CheckGPG).
This will now also include repositories loaded from the RootDir, so
packages are properly configured even when they're retrieved from
repositories not configured in the request.

RHSM is treated as a special property now, outside of other repository
properties and options.  RHSM can only be set on repos defined in the
request, not from repositories in the RootDir, so we keep a set of
repository IDs that specify RHSM in order to set them on each package
that requires it.
The WriteConfig() function wasn't being used.  Repurpose it to write a
.repo format config that we can use to test the new osbuild-depsolve-dnf
feature.
Add more test cases that use the test repo metadata but only define the
repo through the RootDir.
This test requires the new osbuild-depsolve-dnf that supports reading
repositories from a directory.
Convert the repos returned from osbuild-depsolve-dnf to rpmmd.RepoConfig
and return them alongside the packages.  This is needed to get access to
GPG keys that were loaded from repositories read from a directory.
Return the repository configurations from the depsolve() functions used
in the gen-manifests and build commands.  These repository
configurations are a map keyed by the pipeline name they belong to, just
like package sets.

We don't do anything with these yet, so assign them to _ to work around
compile errors.
Pass down the repository configurations we get from osbuild-depsolve-dnf
into the manifest serialization functions.  These repositories are used
to add gpg keys to import for each rpm stage in a pipeline.  The
repository configurations are appended to the ones already defined in
each pipeline before serialization.

This will duplicate repositories in each pipeline, effectively
duplicating gpg key importing.  This wont have any effect on the build
or the final image.
Run the setup-osbuild-repo script before running unit tests in github
actions to get the pinned osbuild-depsolve-dnf rpm.

Some steps had to be reordered for this.  The repo setup requires the
repository to be checked out and the dependency installation should
happen after the osbuild repo setup.
Needed for unit tests in github actions.
We run setup-osbuild-repo on CentOS Stream 8 now which imports the
library but doesn't have a new enough python version to include the
'match' statement.
@achilleas-k
Copy link
Member Author

Rebased to fix conflict

Since we now inspect the generated manifests in he generate-manifests
jobs [1], we need the pinned version of osbuild.

[1] cdbf17d
@ondrejbudai ondrejbudai added this pull request to the merge queue Apr 11, 2024
Merged via the queue into osbuild:main with commit 9b91fba Apr 11, 2024
14 of 16 checks passed
@achilleas-k achilleas-k deleted the dnfjson/repos-from-path branch April 12, 2024 09:23
achilleas-k added a commit to achilleas-k/images that referenced this pull request Apr 12, 2024
The addition of repo configs from the depsolve result added in PR osbuild#537
[1] added a bit of non-determinism to the rpm stage option generation.
The list of gpg keys to import always had duplicates but it was at least
stable, based on the repository configurations for each build.  Now,
the repository configurations that we get from the depsolve aren't in
stable order so the key order can change.  This had no functional effect
on the image build process, but it does mean that manifests generated
with the same inputs have different IDs.

Sort and deduplicate keys in the rpm stage option generation to make
manifests stable.  Deduplicating the keys also makes the manifests a bit
"cleaner".

[1] osbuild#537

Signed-off-by: Achilleas Koutsou <achilleas@koutsou.net>
github-merge-queue bot pushed a commit that referenced this pull request Apr 12, 2024
The addition of repo configs from the depsolve result added in PR #537
[1] added a bit of non-determinism to the rpm stage option generation.
The list of gpg keys to import always had duplicates but it was at least
stable, based on the repository configurations for each build.  Now,
the repository configurations that we get from the depsolve aren't in
stable order so the key order can change.  This had no functional effect
on the image build process, but it does mean that manifests generated
with the same inputs have different IDs.

Sort and deduplicate keys in the rpm stage option generation to make
manifests stable.  Deduplicating the keys also makes the manifests a bit
"cleaner".

[1] #537

Signed-off-by: Achilleas Koutsou <achilleas@koutsou.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants