Merge pull request #4705 from umohnani8/extensions

MCO-1331: Install extensions via Containerfile for OCL

Author: openshift-merge-bot[bot]

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 .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 \
+    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 := .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
+{{end}}
 
 COPY ./openshift-config-user-ca-bundle.crt /etc/pki/ca-trust/source/anchors/openshift-config-user-ca-bundle.crt
 RUN update-ca-trust

pkg/controller/build/buildrequest/buildrequest.go

@@ -214,26 +214,33 @@ 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
 	// lowercase fields. Additionally, since there are a few fields where we
 	// 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
+		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(),
+		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 {

pkg/controller/build/buildrequest/buildrequest_test.go

@@ -21,6 +21,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()
@@ -42,21 +60,40 @@ 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("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSExtensionsContainerImage),
+				`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("FROM %s AS extensions", osImageURLConfig.BaseOSExtensionsContainerImage),
+				fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSExtensionsContainerImage),
+				`extensions="krb5-workstation libkadm5 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\"",
 			},
 		},
 		{
@@ -100,12 +137,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(),

pkg/controller/build/buildrequest/buildrequestopts.go

@@ -74,6 +74,15 @@ func (b BuildRequestOpts) getReleaseVersion() string {
 	return b.OSImageURLConfig.ReleaseVersion
 }
 
+// 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 ctrlcommon.GetPackagesForSupportedExtensions(b.MachineConfig.Spec.Extensions)
+}
+
 // 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{

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) {

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)
+		})
+	}
+}

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
 		}