From 0ce0cbc1a51c1bf217ffe47b182808b42f9c6763 Mon Sep 17 00:00:00 2001 From: Urvashi Date: Mon, 11 Nov 2024 20:25:18 -0500 Subject: [PATCH 1/3] Install extensions via Containerfile for OCL Add logic to the Containerfile used to build the new OS image to be able to install extensions when using OCL. Extensions are installed via rpm-ostree and commited to the container image. Signed-off-by: Urvashi --- .../Containerfile.on-cluster-build-template | 28 ++++++---- .../build/buildrequest/buildrequest.go | 2 + .../build/buildrequest/buildrequest_test.go | 21 +++++--- .../build/buildrequest/buildrequestopts.go | 8 +++ .../e2e-techpreview/onclusterlayering_test.go | 52 ++++++++++++++++++- 5 files changed, 94 insertions(+), 17 deletions(-) diff --git a/pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template b/pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template index 5cdbb6f468..56961f770e 100644 --- a/pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template +++ b/pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template @@ -10,16 +10,6 @@ COPY ./machineconfig/machineconfig.json.gz /tmp/machineconfig.json.gz RUN mkdir -p /etc/machine-config-daemon && \ cat /tmp/machineconfig.json.gz | base64 -d | gunzip - > /etc/machine-config-daemon/currentconfig -{{if .ExtensionsImage}} -# Pull our extensions image. Not sure yet what / how this should be wired up -# though. Ideally, I'd like to use some Buildah tricks to have the extensions -# directory mounted into the container at build-time so that I don't have to -# copy the RPMs into the container, configure the repo, and do the -# installation. Alternatively, I'd have to start a pod with an HTTP server. -FROM {{.ExtensionsImage}} AS extensions -{{end}} - - FROM {{.BaseOSImage}} AS configs # Copy the extracted MachineConfig into the expected place in the image. COPY --from=extract /etc/machine-config-daemon/currentconfig /etc/machine-config-daemon/currentconfig @@ -28,6 +18,24 @@ COPY --from=extract /etc/machine-config-daemon/currentconfig /etc/machine-config # since it should be set by the container runtime / builder. RUN container="oci" exec -a ignition-apply /usr/lib/dracut/modules.d/30ignition/ignition --ignore-unsupported <(cat /etc/machine-config-daemon/currentconfig | jq '.spec.config') && \ ostree container commit +# Install any extensions specified +{{if and .ExtensionsImage .Extensions}} +# Mount the extensions image to use the content from it +# and add the extensions repo to /etc/yum.repos.d/coreos-extensions.repo +RUN --mount=type=bind,from={{.ExtensionsImage}},source=/,target=/tmp/mco-extensions/os-extensions-content,bind-propagation=rshared,rw,z \ + echo -e "[coreos-extensions]\n\ +enabled=1\n\ +metadata_expire=1m\n\ +baseurl=/tmp/mco-extensions/os-extensions-content/usr/share/rpm-ostree/extensions/\n\ +gpgcheck=0\n\ +skip_if_unavailable=False" > /etc/yum.repos.d/coreos-extensions.repo && \ + chmod 644 /etc/yum.repos.d/coreos-extensions.repo && \ + extensions="{{- range $index, $item := .Extensions }}{{- if $index }} {{ end }}{{$item}}{{- end }}" && \ + echo "Installing packages: $extensions" && \ + rpm-ostree install $extensions && \ + rm /etc/yum.repos.d/coreos-extensions.repo +RUN ostree container commit +{{end}} LABEL machineconfig={{.MachineOSBuild.Spec.DesiredConfig.Name}} LABEL machineconfigpool={{.MachineOSConfig.Spec.MachineConfigPool.Name}} diff --git a/pkg/controller/build/buildrequest/buildrequest.go b/pkg/controller/build/buildrequest/buildrequest.go index e682dfef12..9dc4aa0fe7 100644 --- a/pkg/controller/build/buildrequest/buildrequest.go +++ b/pkg/controller/build/buildrequest/buildrequest.go @@ -211,6 +211,7 @@ func (br buildRequestImpl) renderContainerfile() (string, error) { ReleaseVersion string BaseOSImage string ExtensionsImage string + Extensions []string }{ MachineOSBuild: br.opts.MachineOSBuild, MachineOSConfig: br.opts.MachineOSConfig, @@ -218,6 +219,7 @@ func (br buildRequestImpl) renderContainerfile() (string, error) { ReleaseVersion: br.opts.getReleaseVersion(), BaseOSImage: br.opts.getBaseOSImagePullspec(), ExtensionsImage: br.opts.getExtensionsImagePullspec(), + Extensions: br.opts.getExtensions(), } if err := tmpl.Execute(out, items); err != nil { diff --git a/pkg/controller/build/buildrequest/buildrequest_test.go b/pkg/controller/build/buildrequest/buildrequest_test.go index 557da56ec1..6b9c543d99 100644 --- a/pkg/controller/build/buildrequest/buildrequest_test.go +++ b/pkg/controller/build/buildrequest/buildrequest_test.go @@ -40,21 +40,28 @@ func TestBuildRequest(t *testing.T) { unexpectedContainerfileContents []string }{ { - name: "With extensions image", - optsFunc: getBuildRequestOpts, + name: "With extensions image and extensions", + optsFunc: func() BuildRequestOpts { + opts := getBuildRequestOpts() + opts.MachineConfig.Spec.Extensions = []string{"usbguard"} + return opts + }, expectedContainerfileContents: append(expectedContents(), []string{ - fmt.Sprintf("FROM %s AS extensions", osImageURLConfig.BaseOSExtensionsContainerImage), + fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSExtensionsContainerImage), + "extensions=\"usbguard\"", }...), }, { - name: "Missing extensions image", + name: "Missing extensions image and extensions", optsFunc: func() BuildRequestOpts { opts := getBuildRequestOpts() opts.OSImageURLConfig.BaseOSExtensionsContainerImage = "" + opts.MachineConfig.Spec.Extensions = []string{"usbguard"} return opts }, unexpectedContainerfileContents: []string{ - fmt.Sprintf("FROM %s AS extensions", osImageURLConfig.BaseOSContainerImage), + fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSContainerImage), + "extensions=\"usbguard\"", }, }, { @@ -98,12 +105,14 @@ func TestBuildRequest(t *testing.T) { opts.MachineOSConfig.Spec.BuildInputs.BaseOSImagePullspec = "base-os-image-from-machineosconfig" opts.MachineOSConfig.Spec.BuildInputs.BaseOSExtensionsImagePullspec = "base-ext-image-from-machineosconfig" opts.MachineOSConfig.Spec.BuildInputs.ReleaseVersion = "release-version-from-machineosconfig" + opts.MachineConfig.Spec.Extensions = []string{"usbguard"} return opts }, expectedContainerfileContents: []string{ "FROM base-os-image-from-machineosconfig AS extract", "FROM base-os-image-from-machineosconfig AS configs", - "FROM base-ext-image-from-machineosconfig AS extensions", + "RUN --mount=type=bind,from=base-ext-image-from-machineosconfig", + "extensions=\"usbguard\"", "LABEL releaseversion=release-version-from-machineosconfig", }, unexpectedContainerfileContents: expectedContents(), diff --git a/pkg/controller/build/buildrequest/buildrequestopts.go b/pkg/controller/build/buildrequest/buildrequestopts.go index c05d8af47c..2e86d6a723 100644 --- a/pkg/controller/build/buildrequest/buildrequestopts.go +++ b/pkg/controller/build/buildrequest/buildrequestopts.go @@ -68,6 +68,14 @@ func (b BuildRequestOpts) getReleaseVersion() string { return b.OSImageURLConfig.ReleaseVersion } +// Gets the extensions from the MachineConfig if available. +func (b BuildRequestOpts) getExtensions() []string { + if len(b.MachineConfig.Spec.Extensions) > 0 { + return b.MachineConfig.Spec.Extensions + } + return []string{} +} + // Gets all of the image build request opts from the Kube API server. func newBuildRequestOptsFromAPI(ctx context.Context, kubeclient clientset.Interface, mcfgclient mcfgclientset.Interface, mosb *mcfgv1alpha1.MachineOSBuild, mosc *mcfgv1alpha1.MachineOSConfig) (*BuildRequestOpts, error) { og := optsGetter{ diff --git a/test/e2e-techpreview/onclusterlayering_test.go b/test/e2e-techpreview/onclusterlayering_test.go index f8a781b9e9..bf196c4cf8 100644 --- a/test/e2e-techpreview/onclusterlayering_test.go +++ b/test/e2e-techpreview/onclusterlayering_test.go @@ -88,6 +88,9 @@ type onClusterLayeringTestOpts struct { // Inject YUM repo information from a Centos 9 stream container useYumRepos bool + + // Add Extensions for testing + useExtensions bool } func TestOnClusterBuildsOnOKD(t *testing.T) { @@ -113,12 +116,13 @@ func TestOnClusterBuildsCustomPodBuilder(t *testing.T) { // Tests that an on-cluster build can be performed and that the resulting image // is rolled out to an opted-in node. -func TestOnClusterBuildRollsOutImage(t *testing.T) { +func TestOnClusterBuildRollsOutImageWithExtensionsInstalled(t *testing.T) { imagePullspec := runOnClusterLayeringTest(t, onClusterLayeringTestOpts{ poolName: layeredMCPName, customDockerfiles: map[string]string{ layeredMCPName: cowsayDockerfile, }, + useExtensions: true, }) cs := framework.NewClientSet("") @@ -129,12 +133,14 @@ func TestOnClusterBuildRollsOutImage(t *testing.T) { helpers.AssertNodeBootedIntoImage(t, cs, node, imagePullspec) t.Logf("Node %s is booted into image %q", node.Name, imagePullspec) + assertExtensionInstalledOnNode(t, cs, node, true) t.Log(helpers.ExecCmdOnNode(t, cs, node, "chroot", "/rootfs", "cowsay", "Moo!")) unlabelFunc() assertNodeRevertsToNonLayered(t, cs, node) + assertExtensionInstalledOnNode(t, cs, node, false) } func assertNodeRevertsToNonLayered(t *testing.T, cs *framework.ClientSet, node corev1.Node) { @@ -151,6 +157,22 @@ func assertNodeRevertsToNonLayered(t *testing.T, cs *framework.ClientSet, node c helpers.AssertFileNotOnNode(t, cs, node, runtimeassets.RevertServiceMachineConfigFile) } +func assertExtensionInstalledOnNode(t *testing.T, cs *framework.ClientSet, node corev1.Node, shouldExist bool) { + foundPkg, err := helpers.ExecCmdOnNodeWithError(cs, node, "chroot", "/rootfs", "rpm", "-q", "usbguard") + if shouldExist { + require.NoError(t, err, "usbguard extension not found") + if strings.Contains(foundPkg, "package usbguard is not installed") { + t.Fatalf("usbguard package not installed on node %s, got %s", node.Name, foundPkg) + } + t.Logf("usbguard extension installed, got %s", foundPkg) + } else { + if !strings.Contains(foundPkg, "package usbguard is not installed") { + t.Fatalf("usbguard package is installed on node %s, got %s", node.Name, foundPkg) + } + t.Logf("usbguard extension not installed as expected, got %s", foundPkg) + } +} + // This test extracts the /etc/yum.repos.d and /etc/pki/rpm-gpg content from a // Centos Stream 9 image and injects them into the MCO namespace. It then // performs a build with the expectation that these artifacts will be used, @@ -923,6 +945,34 @@ func prepareForOnClusterLayeringTest(t *testing.T, cs *framework.ClientSet, test t.Cleanup(makeIdempotentAndRegister(t, helpers.CreateMCP(t, cs, testOpts.poolName))) } + if testOpts.useExtensions { + extensionsMC := &mcfgv1.MachineConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "99-extensions", + Labels: helpers.MCLabelForRole(testOpts.poolName), + }, + Spec: mcfgv1.MachineConfigSpec{ + Config: runtime.RawExtension{ + Raw: helpers.MarshalOrDie(ctrlcommon.NewIgnConfig()), + }, + Extensions: []string{"usbguard"}, + }, + } + + helpers.SetMetadataOnObject(t, extensionsMC) + // Apply the extensions MC + mcCleanupFunc := helpers.ApplyMC(t, cs, extensionsMC) + t.Cleanup(func() { + mcCleanupFunc() + t.Logf("Deleted MachineConfig %s", extensionsMC.Name) + }) + t.Logf("Created new MachineConfig %q", extensionsMC.Name) + // Wait for rendered config to finish creating + renderedConfig, err := helpers.WaitForRenderedConfig(t, cs, testOpts.poolName, extensionsMC.Name) + require.NoError(t, err) + t.Logf("Finished rendering config %s", renderedConfig) + } + _, err := helpers.WaitForRenderedConfig(t, cs, testOpts.poolName, "00-worker") require.NoError(t, err) From 69e89548cd5fb4ed45e3c823ea8a1021b6743ea2 Mon Sep 17 00:00:00 2001 From: Zack Zlotnik Date: Thu, 21 Nov 2024 12:02:30 -0500 Subject: [PATCH 2/3] moves extensions validation into ctrlcommon --- pkg/controller/common/helpers.go | 57 +++++++++++++++++ pkg/controller/common/helpers_test.go | 89 +++++++++++++++++++++++++++ pkg/daemon/daemon.go | 2 +- pkg/daemon/update.go | 37 +---------- 4 files changed, 149 insertions(+), 36 deletions(-) diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index e208a99518..f435f01cc1 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -612,6 +612,63 @@ func ValidateMachineConfig(cfg mcfgv1.MachineConfigSpec) error { return nil } +// Validates that a given MachineConfig's extensions are supported. +func ValidateMachineConfigExtensions(cfg mcfgv1.MachineConfigSpec) error { + return validateExtensions(cfg.Extensions) +} + +func validateExtensions(exts []string) error { + supportedExtensions := SupportedExtensions() + invalidExts := []string{} + for _, ext := range exts { + if _, ok := supportedExtensions[ext]; !ok { + invalidExts = append(invalidExts, ext) + } + } + if len(invalidExts) != 0 { + return fmt.Errorf("invalid extensions found: %v", invalidExts) + } + return nil +} + +// Resolves a list of supported extensions to the individual packages required +// for each of those extensions. Returns an error is any of the supplied +// extensions is invalid. +func GetPackagesForSupportedExtensions(exts []string) ([]string, error) { + if err := validateExtensions(exts); err != nil { + return nil, err + } + + pkgs := []string{} + + supported := SupportedExtensions() + for _, ext := range exts { + for _, pkg := range supported[ext] { + pkgs = append(pkgs, pkg) + } + } + + return pkgs, nil +} + +// Returns list of extensions possible to install on a CoreOS based system. +func SupportedExtensions() map[string][]string { + // In future when list of extensions grow, it will make + // more sense to populate it in a dynamic way. + + // These are RHCOS supported extensions. + // Each extension keeps a list of packages required to get enabled on host. + return map[string][]string{ + "wasm": {"crun-wasm"}, + "ipsec": {"NetworkManager-libreswan", "libreswan"}, + "usbguard": {"usbguard"}, + "kerberos": {"krb5-workstation", "libkadm5"}, + "kernel-devel": {"kernel-devel", "kernel-headers"}, + "sandboxed-containers": {"kata-containers"}, + "sysstat": {"sysstat"}, + } +} + // IgnParseWrapper parses rawIgn for both V2 and V3 ignition configs and returns // a V2 or V3 Config or an error. This wrapper is necessary since V2 and V3 use different parsers. func IgnParseWrapper(rawIgn []byte) (interface{}, error) { diff --git a/pkg/controller/common/helpers_test.go b/pkg/controller/common/helpers_test.go index 005df72e9f..3c9ee821d2 100644 --- a/pkg/controller/common/helpers_test.go +++ b/pkg/controller/common/helpers_test.go @@ -793,3 +793,92 @@ func TestParseAndConvertGzippedConfig(t *testing.T) { }) } } + +func TestValidateMachineConfigExtensions(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + extensions []string + errExpected bool + }{ + { + name: "Supported", + extensions: []string{"sysstat", "sandboxed-containers"}, + }, + { + name: "Unsupported", + extensions: []string{"unsupported1", "unsupported2"}, + errExpected: true, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + mcfgSpec := mcfgv1.MachineConfigSpec{ + Extensions: testCase.extensions, + } + + err := ValidateMachineConfigExtensions(mcfgSpec) + + if testCase.errExpected { + assert.Error(t, err) + for _, ext := range testCase.extensions { + assert.Contains(t, err.Error(), ext) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestGetPackagesForSupportedExtensions(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + extensions []string + expectedPackages []string + errExpected bool + }{ + { + name: "Unsupported extension", + extensions: []string{"unsupported"}, + errExpected: true, + }, + { + name: "Supported single package extension", + extensions: []string{"wasm"}, + expectedPackages: []string{"crun-wasm"}, + }, + { + name: "Supported single multi-package extension", + extensions: []string{"kerberos"}, + expectedPackages: []string{"krb5-workstation", "libkadm5"}, + }, + { + name: "Supported multiple multi-package extensions", + extensions: []string{"ipsec", "kerberos"}, + expectedPackages: []string{"NetworkManager-libreswan", "libreswan", "krb5-workstation", "libkadm5"}, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + pkgs, err := GetPackagesForSupportedExtensions(testCase.extensions) + if testCase.errExpected { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, testCase.expectedPackages, pkgs) + }) + } +} diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index fd1778a975..64ba4642c4 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -2862,7 +2862,7 @@ func (dn *Daemon) getUnsupportedPackages() { } supportedPackages := make(map[string]bool) - for _, packages := range getSupportedExtensions() { + for _, packages := range ctrlcommon.SupportedExtensions() { for _, pkg := range packages { supportedPackages[pkg] = true } diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 45ca8e2748..ee8c84afbb 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -1731,7 +1731,7 @@ func (dn *Daemon) generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineCon extArgs := []string{"update"} if dn.os.IsEL() { - extensions := getSupportedExtensions() + extensions := ctrlcommon.SupportedExtensions() for _, ext := range added { for _, pkg := range extensions[ext] { extArgs = append(extArgs, "--install", pkg) @@ -1761,39 +1761,6 @@ func (dn *Daemon) generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineCon return extArgs } -// Returns list of extensions possible to install on a CoreOS based system. -func getSupportedExtensions() map[string][]string { - // In future when list of extensions grow, it will make - // more sense to populate it in a dynamic way. - - // These are RHCOS supported extensions. - // Each extension keeps a list of packages required to get enabled on host. - return map[string][]string{ - "wasm": {"crun-wasm"}, - "ipsec": {"NetworkManager-libreswan", "libreswan"}, - "usbguard": {"usbguard"}, - "kerberos": {"krb5-workstation", "libkadm5"}, - "kernel-devel": {"kernel-devel", "kernel-headers"}, - "sandboxed-containers": {"kata-containers"}, - "sysstat": {"sysstat"}, - } -} - -func validateExtensions(exts []string) error { - supportedExtensions := getSupportedExtensions() - invalidExts := []string{} - for _, ext := range exts { - if _, ok := supportedExtensions[ext]; !ok { - invalidExts = append(invalidExts, ext) - } - } - if len(invalidExts) != 0 { - return fmt.Errorf("invalid extensions found: %v", invalidExts) - } - return nil - -} - func (dn *CoreOSDaemon) applyExtensions(oldConfig, newConfig *mcfgv1.MachineConfig) error { extensionsEmpty := len(oldConfig.Spec.Extensions) == 0 && len(newConfig.Spec.Extensions) == 0 if (extensionsEmpty) || @@ -1802,7 +1769,7 @@ func (dn *CoreOSDaemon) applyExtensions(oldConfig, newConfig *mcfgv1.MachineConf } // Validate extensions allowlist on RHCOS nodes - if err := validateExtensions(newConfig.Spec.Extensions); err != nil && dn.os.IsEL() { + if err := ctrlcommon.ValidateMachineConfigExtensions(newConfig.Spec); err != nil && dn.os.IsEL() { return err } From fee1b99d795e6bbe859ca057b6e3525a2fd1cd4e Mon Sep 17 00:00:00 2001 From: Zack Zlotnik Date: Thu, 21 Nov 2024 15:40:38 -0500 Subject: [PATCH 3/3] validate extensions and packages before OCL build --- .../Containerfile.on-cluster-build-template | 6 ++-- .../build/buildrequest/buildrequest.go | 33 +++++++++++-------- .../build/buildrequest/buildrequest_test.go | 32 +++++++++++++++++- .../build/buildrequest/buildrequestopts.go | 11 ++++--- 4 files changed, 59 insertions(+), 23 deletions(-) diff --git a/pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template b/pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template index 56961f770e..891275d6ea 100644 --- a/pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template +++ b/pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template @@ -19,7 +19,7 @@ COPY --from=extract /etc/machine-config-daemon/currentconfig /etc/machine-config RUN container="oci" exec -a ignition-apply /usr/lib/dracut/modules.d/30ignition/ignition --ignore-unsupported <(cat /etc/machine-config-daemon/currentconfig | jq '.spec.config') && \ ostree container commit # Install any extensions specified -{{if and .ExtensionsImage .Extensions}} +{{if and .ExtensionsImage .ExtensionsPackages}} # Mount the extensions image to use the content from it # and add the extensions repo to /etc/yum.repos.d/coreos-extensions.repo RUN --mount=type=bind,from={{.ExtensionsImage}},source=/,target=/tmp/mco-extensions/os-extensions-content,bind-propagation=rshared,rw,z \ @@ -30,8 +30,8 @@ baseurl=/tmp/mco-extensions/os-extensions-content/usr/share/rpm-ostree/extension gpgcheck=0\n\ skip_if_unavailable=False" > /etc/yum.repos.d/coreos-extensions.repo && \ chmod 644 /etc/yum.repos.d/coreos-extensions.repo && \ - extensions="{{- range $index, $item := .Extensions }}{{- if $index }} {{ end }}{{$item}}{{- end }}" && \ - echo "Installing packages: $extensions" && \ + extensions="{{- range $index, $item := .ExtensionsPackages }}{{- if $index }} {{ end }}{{$item}}{{- end }}" && \ + echo "Installing extension packages: $extensions" && \ rpm-ostree install $extensions && \ rm /etc/yum.repos.d/coreos-extensions.repo RUN ostree container commit diff --git a/pkg/controller/build/buildrequest/buildrequest.go b/pkg/controller/build/buildrequest/buildrequest.go index 9dc4aa0fe7..dd3e2c7b87 100644 --- a/pkg/controller/build/buildrequest/buildrequest.go +++ b/pkg/controller/build/buildrequest/buildrequest.go @@ -198,6 +198,11 @@ func (br buildRequestImpl) renderContainerfile() (string, error) { return "", fmt.Errorf("could not parse containerfile template: %w", err) } + extPkgs, err := br.opts.getExtensionsPackages() + if err != nil { + return "", err + } + out := &strings.Builder{} // This anonymous struct is necessary because templates cannot access @@ -205,21 +210,21 @@ func (br buildRequestImpl) renderContainerfile() (string, error) { // default to a value from a different location, it makes more sense for us // to implement that logic in Go as opposed to the Go template language. items := struct { - MachineOSBuild *mcfgv1alpha1.MachineOSBuild - MachineOSConfig *mcfgv1alpha1.MachineOSConfig - UserContainerfile string - ReleaseVersion string - BaseOSImage string - ExtensionsImage string - Extensions []string + MachineOSBuild *mcfgv1alpha1.MachineOSBuild + MachineOSConfig *mcfgv1alpha1.MachineOSConfig + UserContainerfile string + ReleaseVersion string + BaseOSImage string + ExtensionsImage string + ExtensionsPackages []string }{ - MachineOSBuild: br.opts.MachineOSBuild, - MachineOSConfig: br.opts.MachineOSConfig, - UserContainerfile: br.userContainerfile, - ReleaseVersion: br.opts.getReleaseVersion(), - BaseOSImage: br.opts.getBaseOSImagePullspec(), - ExtensionsImage: br.opts.getExtensionsImagePullspec(), - Extensions: br.opts.getExtensions(), + MachineOSBuild: br.opts.MachineOSBuild, + MachineOSConfig: br.opts.MachineOSConfig, + UserContainerfile: br.userContainerfile, + ReleaseVersion: br.opts.getReleaseVersion(), + BaseOSImage: br.opts.getBaseOSImagePullspec(), + ExtensionsImage: br.opts.getExtensionsImagePullspec(), + ExtensionsPackages: extPkgs, } if err := tmpl.Execute(out, items); err != nil { diff --git a/pkg/controller/build/buildrequest/buildrequest_test.go b/pkg/controller/build/buildrequest/buildrequest_test.go index 6b9c543d99..ce65215551 100644 --- a/pkg/controller/build/buildrequest/buildrequest_test.go +++ b/pkg/controller/build/buildrequest/buildrequest_test.go @@ -19,6 +19,24 @@ const ( mcoImagePullspec = "registry.hostname.com/org/repo@sha256:87980e0edfc86d01182f70c53527f74b5b01df00fe6d47668763d228d4de43a9" ) +// Validates that if an invalid extension is provided that the ConfigMap +// generation fails and the error contains the names of the invalid extensions. +func TestBuildRequestInvalidExtensions(t *testing.T) { + t.Parallel() + + opts := getBuildRequestOpts() + opts.MachineConfig.Spec.Extensions = []string{"invalid-ext1", "invalid-ext2"} + + br := newBuildRequest(opts) + + _, err := br.ConfigMaps() + assert.Error(t, err) + + for _, ext := range opts.MachineConfig.Spec.Extensions { + assert.Contains(t, err.Error(), ext) + } +} + // Tests that the BuildRequest is constructed as expected. func TestBuildRequest(t *testing.T) { t.Parallel() @@ -48,7 +66,19 @@ func TestBuildRequest(t *testing.T) { }, expectedContainerfileContents: append(expectedContents(), []string{ fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSExtensionsContainerImage), - "extensions=\"usbguard\"", + `extensions="usbguard"`, + }...), + }, + { + name: "With extensions image and resolved extensions packages", + optsFunc: func() BuildRequestOpts { + opts := getBuildRequestOpts() + opts.MachineConfig.Spec.Extensions = []string{"kerberos", "usbguard"} + return opts + }, + expectedContainerfileContents: append(expectedContents(), []string{ + fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSExtensionsContainerImage), + `extensions="krb5-workstation libkadm5 usbguard"`, }...), }, { diff --git a/pkg/controller/build/buildrequest/buildrequestopts.go b/pkg/controller/build/buildrequest/buildrequestopts.go index 2e86d6a723..b792dfbb87 100644 --- a/pkg/controller/build/buildrequest/buildrequestopts.go +++ b/pkg/controller/build/buildrequest/buildrequestopts.go @@ -68,12 +68,13 @@ func (b BuildRequestOpts) getReleaseVersion() string { return b.OSImageURLConfig.ReleaseVersion } -// Gets the extensions from the MachineConfig if available. -func (b BuildRequestOpts) getExtensions() []string { - if len(b.MachineConfig.Spec.Extensions) > 0 { - return b.MachineConfig.Spec.Extensions +// Gets the packages for the extensions from the MachineConfig, if available. +func (b BuildRequestOpts) getExtensionsPackages() ([]string, error) { + if len(b.MachineConfig.Spec.Extensions) == 0 { + return nil, nil } - return []string{} + + return ctrlcommon.GetPackagesForSupportedExtensions(b.MachineConfig.Spec.Extensions) } // Gets all of the image build request opts from the Kube API server.