Add support for arbitrary kickstart file injection into ISOs (HMS-3879) #631

achilleas-k · 2024-04-24T22:45:37Z

Adds a new blueprint customization [installer.kickstart.contents] that will allow users to inject their own kickstart file into an ISO build.

This PR also includes a partial implementation of a recommendation I made in a previous PR: #614 (comment)

A new internal customization struct called kickstart.Options holds all kickstart options that we can set in pipelines and is attached to Anaconda pipelines and image kinds that support them. This is similar to other internal options structs like users.User, users.Group, fsnode.File, etc.
There's a bit more we can do with this (for example a ks.GetPath() method that returns the default when it's not set, even with ks == nil, instead of checking all the time) but I wanted to get this PR open and merged to get this into bootc-image-builder.

Note that our initial idea was to make any other kickstart option illegal when this option is used. Leaving it wide open can cause all sorts of issues for users who don't know what we're adding to the kickstart when they append their own, but from what I understand, it should be safe in the general case where they add some %post bits or enable options we don't support.

I'm fine with restricting it if others disagree.

achilleas-k · 2024-04-25T15:27:01Z

This will need to be rebased once #632 is merged.

thozza

Nice work. Overall this looks good, but one commit needs cleanup.

bd6234b contains a syntax error, which is then fixed by d639950.

pkg/manifest/anaconda_installer_iso_tree_test.go

thozza

👍

mvo5

I did not do a super deep review (too many distractions) but I read through and it looks very nice. I especially like that the kickstart options are now all consolidated, very nice work!

pkg/customizations/kickstart/kickstart.go

bcl · 2024-04-26T22:56:57Z

I wonder if we should run ksvalidator on the user blob at some point, either here or in osbuild? As long as you have a newer version of pykickstart installed it can validate other supported versions by passing it --version RHEL9 for example.

supakeen · 2024-04-29T09:26:44Z

I wonder if we should run ksvalidator on the user blob at some point, either here or in osbuild? As long as you have a newer version of pykickstart installed it can validate other supported versions by passing it --version RHEL9 for example.

That would be a good idea yes; we currently only do so in testcases.

achilleas-k · 2024-04-29T17:57:47Z

I wonder if we should run ksvalidator on the user blob at some point, either here or in osbuild?

Could be a good idea, yeah. Though, it gets tricky with cross-distro builds doesn't it? If you're building a RHEL N image on RHEL N+1, you'd want to run it through the validator for RHEL N, not the newer one. So it should probably happen in the buildroot in osbuild I guess?

bcl · 2024-04-29T23:55:28Z

As long as it knows the release name that pykickstart recoginizes (RHEL9, RHEL10, etc.) and is running on the same or newer release it will work. No need to try and work this into this PR though, it can come later.

pkg/manifest/anaconda_installer_iso_tree.go

achilleas-k · 2024-05-07T10:04:44Z

After discussing with @ondrejbudai and @mvo5, we decided that user kickstarts combined with any other kickstart customization would not be supported and produce errors. So now, if a user adds a kickstart file/contents, we only add the install line for the payload and then have the user's kickstart file include ours.

For that last part, it would be nicer if our kickstart had an %include to the user's, but that requires osbuild stage changes so we'll do it later.

achilleas-k · 2024-05-07T10:11:29Z

The test config file, test/configs/interactive-iso-userkickstart.json, creates the following in the root of the ISO:

› head -n-0 *.ks
==> osbuild-base.ks <==
liveimg --url file:///run/install/repo/liveimg.tar.gz

==> osbuild.ks <==
%include /run/install/repo/osbuild-base.ks
%post

echo "=== Custom user kickstart ==="

echo -e "%sudo\tALL=(ALL)\tNOPASSWD: ALL" > "/etc/sudoers.d/%sudo"
chmod 0440 /etc/sudoers.d/%sudo
echo -e "%wheel\tALL=(ALL)\tNOPASSWD: ALL" > "/etc/sudoers.d/%wheel"
chmod 0440 /etc/sudoers.d/%wheel
restorecon -rvF /etc/sudoers.d
%end

mvo5

Thank you! I really like the new kickstart.Options struct, this makes a lot of things cleaner. I added some ideas inline, I hope they are useful. But no blockers, so could be followups of course.

pkg/distro/fedora/images.go

pkg/distro/fedora/imagetype.go

pkg/distro/rhel/images.go

pkg/distro/rhel/rhel10/options.go

pkg/distro/rhel/rhel8/options.go

pkg/distro/rhel/rhel9/options.go

pkg/manifest/anaconda_installer_iso_tree.go

Do not allow or add any kickstart options to the bootc container installer ISO when a user kickstart is included. The only line that we add is the ostree container installation line in its own kickstart. The user kickstart file is then added separately and it %includes our own.

Do not allow or add any kickstart options to the tar payload installer (image-installer) or the ostree installer (edge/iot-installer). The only line that we add is the installation line in its own kickstart. The user kickstart file is then added separately and it %includes our own.

1. Kickstart tests now check if the pipeline serialization panic()s when unsupported options are combined. 2. When "extra" kickstart content is defined (user kickstart file), the pre-defined hardcoded kickstart bits aren't added to the checksum calculation.

Unattended + kickstart.contents is no longer allowed, so set unattended to false.

Initialise kickstart customizations for an image with the common options from the blueprint customizations. The kickstart options are initialised with Users, Groups, the Unattended and SudoNopasswd installer options, and the custom kickstart content. Other options (Language, Keyboard, Timezone, and payload options) must be set separately, since they aren't based solely on the blueprint customizations and interact with other configs, like image config or the ostree payload options. Co-authored-by: Michael Vogt <[email protected]>

Co-authored-by: Michael Vogt <[email protected]>

Validate kickstart options in a Validate() function so unify option compatibility handling. The function is called from the kickstart.New() initialiser, but we also call it before stage creation to make sure everything is valid right before stage creation. Co-authored-by: Michael Vogt <[email protected]>

noelmiller · 2024-05-15T18:04:17Z

One major issue I see for this is the User Module is disabled in Anaconda, meaning if I don't specify a user in the kickstart, I cannot create a user in Anaconda:

mvo5

Some quick drive by comments but I like it and nothing blocks it, really just optimizing :)

pkg/customizations/kickstart/kickstart.go

pkg/distro/fedora/imagetype.go

achilleas-k · 2024-05-16T09:41:38Z

@mvo5 @noelmiller I created issues with your comments to look into later (#691, #692). The PR is already long enough and does the basics. We can polish the code in separate PRs (for Michael's comments). We need to have a discussion about the mechanics of Anaconda Kickstart modules (for Noel's comments) and I think that might block this for too long and make the PR even harder to review.

I propose we merge this as is unless there are other blockers.

mvo5

Thank you! This looks great, I had ideas in the previous comment but they should not be blockers, could be a followup as well (or ignored if I missed something there of course :)

pkg/manifest/anaconda_installer_iso_tree.go

mvo5

Still looking good! Sorry, I accidentally pushed to this branch but fixed it right away.

Update to accomodate for the changes in the API that got introduced in osbuild/images#631 (ideally that PR would have been split into a part that changes the API and one that adds the new feature. This would have made it slightly easier to see the API changes in isolation).

I also had to adjust our use of AnacondaContainerInstaller because it was changed in osbuild/images#631.

achilleas-k requested review from ondrejbudai, mvo5 and thozza April 24, 2024 22:45

achilleas-k force-pushed the user-kickstarts branch from a94dd59 to 04dae33 Compare April 25, 2024 13:13

achilleas-k mentioned this pull request Apr 25, 2024

bib: allow blueprints in toml as well osbuild/bootc-image-builder#392

Merged

achilleas-k force-pushed the user-kickstarts branch from 04dae33 to 31cf582 Compare April 25, 2024 17:05

bcl self-requested a review April 25, 2024 18:54

thozza force-pushed the user-kickstarts branch from 31cf582 to 1acd9bd Compare April 26, 2024 07:47

thozza reviewed Apr 26, 2024

View reviewed changes

pkg/manifest/anaconda_installer_iso_tree_test.go Outdated Show resolved Hide resolved

achilleas-k force-pushed the user-kickstarts branch from 1acd9bd to 1684b8f Compare April 26, 2024 13:37

thozza previously approved these changes Apr 26, 2024

View reviewed changes

mvo5 reviewed Apr 26, 2024

View reviewed changes

bcl reviewed Apr 26, 2024

View reviewed changes

pkg/customizations/kickstart/kickstart.go Outdated Show resolved Hide resolved

achilleas-k force-pushed the user-kickstarts branch from 1684b8f to d30ec72 Compare April 29, 2024 17:11

achilleas-k dismissed thozza’s stale review via 49371fb April 29, 2024 17:36

achilleas-k force-pushed the user-kickstarts branch from d30ec72 to 49371fb Compare April 29, 2024 17:36

achilleas-k force-pushed the user-kickstarts branch from 49371fb to c44037b Compare April 29, 2024 18:08

ondrejbudai reviewed Apr 30, 2024

View reviewed changes

pkg/manifest/anaconda_installer_iso_tree.go Outdated Show resolved Hide resolved

pkg/manifest/anaconda_installer_iso_tree.go Outdated Show resolved Hide resolved

achilleas-k force-pushed the user-kickstarts branch 3 times, most recently from c0578d6 to b4a7ad7 Compare May 7, 2024 10:03

achilleas-k mentioned this pull request May 7, 2024

Add support for arbitrary kickstart file injection into ISOs (HMS-3879) osbuild/osbuild-composer#4135

Merged

mvo5 approved these changes May 8, 2024

View reviewed changes

achilleas-k added 5 commits May 14, 2024 01:42

osbuild: add note about future include plans in kickstart stage

1df77c8

test: update config to match new rules

c10a539

Unattended + kickstart.contents is no longer allowed, so set unattended to false.

achilleas-k force-pushed the user-kickstarts branch from b4a7ad7 to 8e9efc4 Compare May 14, 2024 00:07

achilleas-k and others added 3 commits May 14, 2024 02:07

distro: flip the installer customization check order

3beb9c9

Co-authored-by: Michael Vogt <[email protected]>

achilleas-k force-pushed the user-kickstarts branch from 8e9efc4 to df3bdb4 Compare May 14, 2024 00:09

achilleas-k requested a review from mvo5 May 14, 2024 17:14

mvo5 reviewed May 16, 2024

View reviewed changes

pkg/customizations/kickstart/kickstart.go Show resolved Hide resolved

pkg/customizations/kickstart/kickstart.go Show resolved Hide resolved

pkg/distro/fedora/imagetype.go Show resolved Hide resolved

achilleas-k mentioned this pull request May 16, 2024

Installer customizations validation improvements #691

Open

achilleas-k mentioned this pull request May 16, 2024

Determine which modules should be enabled with user-provided kickstart blobs #692

Open

mvo5 previously approved these changes May 16, 2024

View reviewed changes

pkg/manifest/anaconda_installer_iso_tree.go Show resolved Hide resolved

achilleas-k mentioned this pull request May 16, 2024

Add support for arbitrary kickstart file injection into ISOs (HMS-3879) osbuild/bootc-image-builder#438

Merged

mvo5 dismissed their stale review via cd30cc8 May 16, 2024 15:27

mvo5 force-pushed the user-kickstarts branch from cd30cc8 to df3bdb4 Compare May 16, 2024 15:30

mvo5 approved these changes May 16, 2024

View reviewed changes

mvo5 added this pull request to the merge queue May 16, 2024

Merged via the queue into osbuild:main with commit d56a4e6 May 16, 2024
29 of 33 checks passed

achilleas-k deleted the user-kickstarts branch May 16, 2024 18:16

ochosi changed the title ~~Add support for arbitrary kickstart file injection into ISOs~~ Add support for arbitrary kickstart file injection into ISOs (HMS-3879) May 16, 2024

ondrejbudai added a commit to ondrejbudai/osbuild-deploy-container that referenced this pull request Jun 21, 2024

go.mod: bump images

90d794c

I also had to adjust our use of AnacondaContainerInstaller because it was changed in osbuild/images#631.

github-merge-queue bot pushed a commit to osbuild/bootc-image-builder that referenced this pull request Jun 25, 2024

go.mod: bump images

1d04de0

I also had to adjust our use of AnacondaContainerInstaller because it was changed in osbuild/images#631.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for arbitrary kickstart file injection into ISOs (HMS-3879) #631

Add support for arbitrary kickstart file injection into ISOs (HMS-3879) #631

achilleas-k commented Apr 24, 2024 •

edited

Loading

achilleas-k commented Apr 25, 2024

thozza left a comment

thozza left a comment

mvo5 left a comment

bcl commented Apr 26, 2024

supakeen commented Apr 29, 2024

achilleas-k commented Apr 29, 2024

bcl commented Apr 29, 2024

achilleas-k commented May 7, 2024

achilleas-k commented May 7, 2024 •

edited

Loading

mvo5 left a comment

noelmiller commented May 15, 2024

mvo5 left a comment

achilleas-k commented May 16, 2024 •

edited

Loading

mvo5 left a comment

mvo5 left a comment

Add support for arbitrary kickstart file injection into ISOs (HMS-3879) #631

Add support for arbitrary kickstart file injection into ISOs (HMS-3879) #631

Conversation

achilleas-k commented Apr 24, 2024 • edited Loading

achilleas-k commented Apr 25, 2024

thozza left a comment

Choose a reason for hiding this comment

thozza left a comment

Choose a reason for hiding this comment

mvo5 left a comment

Choose a reason for hiding this comment

bcl commented Apr 26, 2024

supakeen commented Apr 29, 2024

achilleas-k commented Apr 29, 2024

bcl commented Apr 29, 2024

achilleas-k commented May 7, 2024

achilleas-k commented May 7, 2024 • edited Loading

mvo5 left a comment

Choose a reason for hiding this comment

noelmiller commented May 15, 2024

mvo5 left a comment

Choose a reason for hiding this comment

achilleas-k commented May 16, 2024 • edited Loading

mvo5 left a comment

Choose a reason for hiding this comment

mvo5 left a comment

Choose a reason for hiding this comment

achilleas-k commented Apr 24, 2024 •

edited

Loading

achilleas-k commented May 7, 2024 •

edited

Loading

achilleas-k commented May 16, 2024 •

edited

Loading