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

Add bootc tests #278

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Add bootc tests #278

wants to merge 10 commits into from

Conversation

comps
Copy link
Contributor

@comps comps commented Oct 24, 2024

The last commit, adding the actual test(s), is still WIP and will change, but all commits under it should be ready for review.

This was originally done because all VMs were snapshotted, and
the orig_* names pointed to the original disk images on which
snapshots were based.

Since only few of our VMs are snapshotted now, drop the prefix,
snapshots use 'snapshot_path' anyway.

Signed-off-by: Jiri Jaburek <[email protected]>
(Rather than using util.ssh_keygen + reading file contents manually.)

Signed-off-by: Jiri Jaburek <[email protected]>
I originally rejected this because import_image() would be copying
the image file if it wasn't in /var/lib/libvirt/images, and it seemed
better and more explicit to make the user put the image on the right
place (into /var/lib/libvirt/images).

However, an alternate view point on "import" is "just use the disk
image in-place, wherever it is", which is what the function does now
with this change.

In that case, passing arguments makes more sense than setting instance-
wide variables.

Signed-off-by: Jiri Jaburek <[email protected]>
@comps
Copy link
Contributor Author

comps commented Oct 25, 2024

I've redesigned the bootc-image-builder test quite a bit; turns out we don't really need a Blueprint (config.toml) since we can do all the configuration in a Containerfile, incl. ssh keys for testing. And the bootc-image-builder container seems to build qcow2 just fine without config.toml, so it must be just optional.

@comps
Copy link
Contributor Author

comps commented Nov 5, 2024

A note for whoever reviews this in the end:

# prepare a Container file for making a hardened image
cfile = podman.Containerfile()
cfile += util.dedent(fr'''
    FROM {src_image}
    RUN dnf -y install dnf-plugins-core
    RUN dnf -y copr enable packit/OpenSCAP-openscap-2170 centos-stream-9-x86_64
    RUN dnf -y install openscap-utils
    COPY remediation-ds.xml /root/.
    RUN oscap-bootc --profile '{profile}' /root/remediation-ds.xml
''')
cfile.add_ssh_pubkey(guest.ssh_pubkey)
cfile.write_to('Containerfile')

I wanted to de-duplicate this, so it's not copy/pasted across both bootc and anaconda tests, and I implemented a function for it,

def build_hardened_image(src_image, datastream, profile):

but it turned out to be just about as complicated as the copy/pasta, or likely even more - I had to write Containerfile into a tmpfile (since library code cannot rely on CWD being temporary), and pass multiple test-specific details such as datastream xml location (with unselected rules), profile, etc.

So it didn't make things any simpler, and it just obfuscated the logic.

So I left the copy/pasta in there, because - if anything - it's much easier to debug the test(s) by commenting sections of code out, or simply by inserting a sleep + checking the generated Containerfile manually, or modifying it, or otherwise playing with it.
Hiding it in a tmpfile / library code seemed like the worse solution.

The idea is that install_basic() is a very minimal wrapper for
running Anaconda with the provided kickstart, no extra bells.

It is then install() that provides OS customizations for testing
via RpmPack, installs DNF repositories from the host, etc., etc.

I felt this was slightly better than trying to re-introduce the
verbatim_kickstart=False flag for install(), or something similar.
Splitting the functions also seems more readable, as we can now
use 'with' instead of an ExitStack.

The class Kickstart (packages, assembly) changes are also related
here, as this change is really about making install_basic() as minimal
as feasible, without forced %packages.

The intended use case is for 'ostreecontainer', where host repositories
and RpmPack don't make sense, given that the OS has already been
hardened and customized in the podman container.

Signed-off-by: Jiri Jaburek <[email protected]>
This came in handy while waiting for a local podman Registry to start
responding on the given port, but can easily be used for waiting for
ssh.

Signed-off-by: Jiri Jaburek <[email protected]>
This logs the place the backup() / restore() was called from,
rather than 'lib.util.backup' every time.

Signed-off-by: Jiri Jaburek <[email protected]>
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.

1 participant