Skip to content

Conversation

@achilleas-k
Copy link
Member

@achilleas-k achilleas-k commented Sep 23, 2025

This PR enables the new customizations (see osbuild/blueprint#34) on all RHEL image types where it makes sense. This includes any image type that supports writing files to the OS tree, since the new option creates a file in /etc/{dnf,yum}/vars.

Notes (and request for comments):

  • Most of the PR ended up being changes to the way we handle the dnf config stage generation. I dropped support for creating multiple instances of the org.osbuild.dnf.config stage, because I think it's unnecessary.
  • The merging of the image config with the blueprint customization is a bit awkward: f7b1a75. I considered doing something similar to what we do with the RHSM options, but that also felt strange, because we don't have customizations for dnf (i.e. pkg/customizations/dnf/) and the blueprint only affects one option from the whole stage.
  • Related to the above, I like how DNFStageOptions.UpdateVar() turned out, but the nil case also makes it a bit awkward.

Also related: HMS-9335

@schutzbot

This comment was marked as outdated.

@achilleas-k
Copy link
Member Author

The linter and integration tests aren't very happy about the first commit. So this should be fine once the blueprint PR is merged and released.

@achilleas-k achilleas-k force-pushed the blueprint/dnf-releasever branch 5 times, most recently from 7c02418 to 8cd6465 Compare September 29, 2025 13:25
@achilleas-k achilleas-k marked this pull request as ready for review September 29, 2025 13:26
@achilleas-k achilleas-k requested a review from a team as a code owner September 29, 2025 13:26
@achilleas-k achilleas-k force-pushed the blueprint/dnf-releasever branch from 8cd6465 to a551234 Compare September 29, 2025 13:39
lzap
lzap previously approved these changes Sep 30, 2025
@achilleas-k achilleas-k force-pushed the blueprint/dnf-releasever branch from a551234 to 8178a2f Compare October 1, 2025 08:52
@achilleas-k
Copy link
Member Author

Clicked the rebase button. Cleared that Snyk failure.

Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

I believe there's a bug in the ImageConfig code, and the unit tests could be enhanced to detect the issue.

Other than that, everything looks good. 👍

@achilleas-k achilleas-k force-pushed the blueprint/dnf-releasever branch 2 times, most recently from 70c7ef3 to 7a87380 Compare October 1, 2025 12:39
@achilleas-k achilleas-k requested review from lzap and thozza October 1, 2025 12:39
thozza
thozza previously approved these changes Oct 2, 2025
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fixes!

supakeen
supakeen previously approved these changes Oct 7, 2025
@supakeen supakeen added this pull request to the merge queue Oct 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 7, 2025
@supakeen
Copy link
Member

supakeen commented Oct 7, 2025

Manifest checksums have changed.

@achilleas-k achilleas-k added this pull request to the merge queue Oct 9, 2025
@achilleas-k achilleas-k removed this pull request from the merge queue due to a manual request Oct 9, 2025
@achilleas-k
Copy link
Member Author

Rebasing and fixing up.

@achilleas-k achilleas-k force-pushed the blueprint/dnf-releasever branch from 7a87380 to 19c92c8 Compare October 9, 2025 08:48
supakeen
supakeen previously approved these changes Oct 9, 2025
@achilleas-k achilleas-k requested a review from thozza October 9, 2025 08:48
tag v1.14.0
Tagger: imagebuilder-bot <imagebuilder-bots+imagebuilder-bot@redhat.com>

Changes with 1.14.0

----------------
  - customization: add firstboot (osbuild/blueprint#23)
    - Author: Lukáš Zapletal, Reviewers: Achilleas Koutsou, Katarína Sieklová

— Somewhere on the Internet, 2025-09-29

---

tag v1.15.0
Tagger: imagebuilder-bot <imagebuilder-bots+imagebuilder-bot@redhat.com>

Changes with 1.15.0

----------------
  - blueprint: new customization: dnf (osbuild/blueprint#34)
    - Author: Achilleas Koutsou, Reviewers: Lukáš Zapletal, Michael Vogt, Tomáš Hozza

— Somewhere on the Internet, 2025-09-29

---
The dnf variable name is `releasever` and it is more consistent with
other user-facing use cases where it is treated as a single word.  So
let's treat it internally as a single word as well.

This matches the new blueprint customization.

Similarly, rename the struct field from SetReleaseVerVar to
SetReleaseverVar to match.
Minor fix for the grammar of an internal error.
We currently have no use case for specifying multiple sets of configs
for dnf_config.  More importantly, there's no reason to have multiple
org.osbuild.dnf.config stages in the same pipeline.  The stage can
handle multiple variables and a single set of global dnf config options,
so a single stage should be able to cover all use cases.

Make the dnf_config.options.config option in the image definitions a
single item instead of an array and only support generating a single dnf
config stage.
The dnf config options getter has a fail state when there is a conflict
between different options that shouldn't be used together.  Return an
error from the function instead of panicking.
thozza
thozza previously approved these changes Oct 9, 2025
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

supakeen
supakeen previously approved these changes Oct 9, 2025
@achilleas-k achilleas-k added this pull request to the merge queue Oct 9, 2025
@achilleas-k achilleas-k removed this pull request from the merge queue due to a manual request Oct 9, 2025
Previously this was not supported because the
dnf_config.options.variables can set a releasever which would conflict
with the variable set by enabling dnf_config.set_releasever_var.
However, it's trivial to check for a conflicting variable of the same
name and make that the only error condition.

The tests have been rewritten to test all combinations.
Add a helper function that either updates or appends new variables to
the dnf config stage options.
When the blueprint dnf config customization enables the new
set_releasever option, add it to the dnf.config variables stage option.
Enable the new customizations on all RHEL image types where it makes
sense.  This includes any image type that supports writing files to the
OS tree, since the new option creates a file in /etc/{dnf,yum}/vars.
@achilleas-k achilleas-k dismissed stale reviews from supakeen and thozza via 18d2fdb October 9, 2025 11:20
@achilleas-k achilleas-k force-pushed the blueprint/dnf-releasever branch from db8708c to 18d2fdb Compare October 9, 2025 11:20
@supakeen supakeen added this pull request to the merge queue Oct 9, 2025
Merged via the queue into osbuild:main with commit 24bd8fc Oct 9, 2025
23 checks passed
@achilleas-k achilleas-k deleted the blueprint/dnf-releasever branch October 9, 2025 14:07
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.

5 participants