MCO-1972: Removes OSImageURLConfig from the build controller

pkg/controller/build/buildrequest/buildrequest.go

@@ -335,8 +335,8 @@ func (br buildRequestImpl) renderContainerfile() (string, error) {
 		MachineOSBuild:     br.opts.MachineOSBuild,
 		MachineOSConfig:    br.opts.MachineOSConfig,
 		UserContainerfile:  br.userContainerfile,
-		BaseOSImage:        br.opts.OSImageURLConfig.BaseOSContainerImage,
-		ExtensionsImage:    br.opts.OSImageURLConfig.BaseOSExtensionsContainerImage,
+		BaseOSImage:        br.opts.MachineConfig.Spec.OSImageURL,
+		ExtensionsImage:    br.opts.MachineConfig.Spec.BaseOSExtensionsContainerImage,
 		ExtensionsPackages: extPkgs,
 		KernelType:         kernelType,
 		KernelPackages:     kernelPackages,
@@ -671,7 +671,7 @@ func (br buildRequestImpl) toBuildahPod() *corev1.Pod {
 					// us to avoid parsing log files.
 					Name:            "create-digest-configmap",
 					Command:         append(command, digestCMScript),
-					Image:           br.opts.OSImageURLConfig.BaseOSContainerImage,
+					Image:           br.opts.MachineConfig.Spec.OSImageURL,
 					Env:             env,
 					ImagePullPolicy: corev1.PullAlways,
 					SecurityContext: securityContext,

pkg/controller/build/buildrequest/buildrequest_test.go

@@ -42,13 +42,11 @@ func TestBuildRequestInvalidExtensions(t *testing.T) {
 func TestBuildRequest(t *testing.T) {
 	t.Parallel()
 
-	osImageURLConfig := fixtures.OSImageURLConfig()
-
 	expectedContents := func() []string {
 		return []string{
-			fmt.Sprintf("FROM %s AS extract", osImageURLConfig.BaseOSContainerImage),
-			fmt.Sprintf("FROM %s AS configs", osImageURLConfig.BaseOSContainerImage),
-			fmt.Sprintf("LABEL baseOSContainerImage=%s", osImageURLConfig.BaseOSContainerImage),
+			fmt.Sprintf("FROM %s AS extract", fixtures.BaseOSContainerImage),
+			fmt.Sprintf("FROM %s AS configs", fixtures.BaseOSContainerImage),
+			fmt.Sprintf("LABEL baseOSContainerImage=%s", fixtures.BaseOSContainerImage),
 		}
 	}
 
@@ -66,7 +64,7 @@ func TestBuildRequest(t *testing.T) {
 				return opts
 			},
 			expectedContainerfileContents: append(expectedContents(), []string{
-				fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSExtensionsContainerImage),
+				fmt.Sprintf("RUN --mount=type=bind,from=%s", fixtures.BaseOSExtensionsContainerImage),
 				`extensions="usbguard"`,
 			}...),
 		},
@@ -78,20 +76,20 @@ func TestBuildRequest(t *testing.T) {
 				return opts
 			},
 			expectedContainerfileContents: append(expectedContents(), []string{
-				fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSExtensionsContainerImage),
+				fmt.Sprintf("RUN --mount=type=bind,from=%s", fixtures.BaseOSExtensionsContainerImage),
 				`extensions="krb5-workstation libkadm5 usbguard"`,
 			}...),
 		},
 		{
 			name: "Missing extensions image and extensions",
 			optsFunc: func() BuildRequestOpts {
 				opts := getBuildRequestOpts()
-				opts.OSImageURLConfig.BaseOSExtensionsContainerImage = ""
+				opts.MachineConfig.Spec.BaseOSExtensionsContainerImage = ""
 				opts.MachineConfig.Spec.Extensions = []string{"usbguard"}
 				return opts
 			},
 			unexpectedContainerfileContents: []string{
-				fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSContainerImage),
+				fmt.Sprintf("RUN --mount=type=bind,from=%s", fixtures.BaseOSContainerImage),
 				"extensions=\"usbguard\"",
 			},
 		},
@@ -235,7 +233,7 @@ func assertBuildJobIsCorrect(t *testing.T, buildJob *batchv1.Job, opts BuildRequ
 	assert.Equal(t, buildJob.Spec.Template.Spec.InitContainers[0].Image, mcoImagePullspec)
 	expectedPullspecs := []string{
 		"base-os-image-from-machineosconfig",
-		fixtures.OSImageURLConfig().BaseOSContainerImage,
+		fixtures.BaseOSContainerImage,
 	}
 
 	assert.Contains(t, expectedPullspecs, buildJob.Spec.Template.Spec.Containers[0].Image)
@@ -312,15 +310,14 @@ RUN rpm-ostree install && \
 	newSecret := `{"auths":` + legacySecret + `}`
 
 	return BuildRequestOpts{
-		MachineConfig:   &mcfgv1.MachineConfig{},
+		MachineConfig:   fixtures.NewObjectsForTest("worker").RenderedMachineConfig,
 		MachineOSConfig: layeredObjects.MachineOSConfigBuilder.MachineOSConfig(),
 		MachineOSBuild:  layeredObjects.MachineOSBuildBuilder.MachineOSBuild(),
 		Images: &ctrlcommon.Images{
 			RenderConfigImages: ctrlcommon.RenderConfigImages{
 				MachineConfigOperator: mcoImagePullspec,
 			},
 		},
-		OSImageURLConfig: fixtures.OSImageURLConfig(),
 		BaseImagePullSecret: &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "base-image-pull-secret",

pkg/controller/build/buildrequest/buildrequestopts.go

@@ -23,11 +23,10 @@ import (
 
 // Holds all of the options used to produce a BuildRequest.
 type BuildRequestOpts struct { //nolint:revive // This name is fine.
-	MachineOSConfig  *mcfgv1.MachineOSConfig
-	MachineOSBuild   *mcfgv1.MachineOSBuild
-	MachineConfig    *mcfgv1.MachineConfig
-	Images           *ctrlcommon.Images
-	OSImageURLConfig *ctrlcommon.OSImageURLConfig
+	MachineOSConfig *mcfgv1.MachineOSConfig
+	MachineOSBuild  *mcfgv1.MachineOSBuild
+	MachineConfig   *mcfgv1.MachineConfig
+	Images          *ctrlcommon.Images
 
 	BaseImagePullSecret  *corev1.Secret
 	FinalImagePushSecret *corev1.Secret
@@ -100,10 +99,6 @@ func newBuildRequestOptsFromAPI(ctx context.Context, kubeclient clientset.Interf
 		return nil, fmt.Errorf("expected images to not be nil")
 	}
 
-	if opts.OSImageURLConfig == nil {
-		return nil, fmt.Errorf("expected osimageurlconfig to not be nil")
-	}
-
 	if opts.BaseImagePullSecret == nil {
 		return nil, fmt.Errorf("expected base image pull secret to not be nil")
 	}
@@ -176,11 +171,6 @@ func (o *optsGetter) getOpts(ctx context.Context, mosb *mcfgv1.MachineOSBuild, m
 		return nil, fmt.Errorf("could not get images.json config: %w", err)
 	}
 
-	osImageURLConfig, err := ctrlcommon.GetOSImageURLConfig(ctx, o.kubeclient)
-	if err != nil {
-		return nil, fmt.Errorf("could not get osImageURL config: %w", err)
-	}
-
 	var baseImagePullSecretName string
 	// Check if a base image pull secret was provided
 	opts.hasUserDefinedBaseImagePullSecret = mosc.Spec.BaseImagePullSecret != nil
@@ -214,7 +204,6 @@ func (o *optsGetter) getOpts(ctx context.Context, mosb *mcfgv1.MachineOSBuild, m
 
 	opts.Images = imagesConfig
 	opts.MachineConfig = mc
-	opts.OSImageURLConfig = osImageURLConfig
 	opts.BaseImagePullSecret = baseImagePullSecret
 	opts.FinalImagePushSecret = finalImagePushSecret
 	opts.MachineOSConfig = mosc.DeepCopy()

pkg/controller/build/buildrequest/buildrequestopts_test.go

@@ -148,7 +148,6 @@ func TestBuildRequestOpts(t *testing.T) {
 			assert.NotNil(t, brOpts.MachineOSConfig)
 			assert.NotNil(t, brOpts.MachineOSBuild)
 			assert.NotNil(t, brOpts.Images)
-			assert.NotNil(t, brOpts.OSImageURLConfig)
 			assert.NotNil(t, brOpts.BaseImagePullSecret)
 			assert.NotNil(t, brOpts.FinalImagePushSecret)
 		})

pkg/controller/build/buildrequest/machineosbuild.go

@@ -1,10 +1,11 @@
 package buildrequest
 
 import (
-	"context"
+
 	//nolint:gosec
 	"crypto/md5"
 	"fmt"
+	"strings"
 
 	"github.com/distribution/reference"
 	"github.com/ghodss/yaml"
@@ -13,7 +14,6 @@ import (
 	"github.com/openshift/machine-config-operator/pkg/controller/build/utils"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	clientset "k8s.io/client-go/kubernetes"
 )
 
 // This is the same salt / pattern from pkg/controller/render/hash.go
@@ -32,13 +32,17 @@ var (
 // Holds the objects that are used to construct a MachineOSBuild with a hashed
 // name.
 type MachineOSBuildOpts struct {
+	MachineConfig     *mcfgv1.MachineConfig
 	MachineOSConfig   *mcfgv1.MachineOSConfig
 	MachineConfigPool *mcfgv1.MachineConfigPool
-	OSImageURLConfig  *ctrlcommon.OSImageURLConfig
 }
 
 // Validates that the required options are provided.
 func (m *MachineOSBuildOpts) validateForHash() error {
+	if err := m.validateMachineConfig(); err != nil {
+		return fmt.Errorf("machineconfig failed validation: %w", err)
+	}
+
 	if m.MachineOSConfig == nil {
 		return fmt.Errorf("missing required MachineOSConfig")
 	}
@@ -51,15 +55,55 @@ func (m *MachineOSBuildOpts) validateForHash() error {
 		return fmt.Errorf("name mismatch, MachineConfigPool has %q, MachineOSConfig has %q", m.MachineConfigPool.Name, m.MachineOSConfig.Spec.MachineConfigPool.Name)
 	}
 
-	if m.OSImageURLConfig == nil {
-		return fmt.Errorf("misssing OSImageURLConfig")
+	return nil
+}
+
+// Validates that a MachineConfig has the necessary metadata for generating a
+// MachineOSBuild.
+func (m *MachineOSBuildOpts) validateMachineConfig() error {
+	if m.MachineConfig == nil {
+		return fmt.Errorf("missing required MachineConfig")
+	}
+
+	if !strings.HasPrefix(m.MachineConfig.Name, "rendered-") {
+		return fmt.Errorf("machineconfig %q is not a rendered MachineConfig", m.MachineConfig.Name)
+	}
+
+	requiredAnnos := []string{ctrlcommon.ReleaseImageVersionAnnotationKey, ctrlcommon.GeneratedByControllerVersionAnnotationKey}
+	for _, anno := range requiredAnnos {
+		val, ok := m.MachineConfig.Annotations[anno]
+		if !ok {
+			return fmt.Errorf("missing annotation %q on MachineConfig %q", anno, m.MachineConfig.Name)
+		}
+
+		if val == "" {
+			return fmt.Errorf("empty annotation %q value on MachineConfig %q", anno, m.MachineConfig.Name)
+		}
 	}
 
 	return nil
 }
 
 // Creates a list of objects that are consumed by the SHA256 hash.
 func (m *MachineOSBuildOpts) objectsForHash() []interface{} {
+	// Represents a private version of the OSImageURLConfig struct to keep the
+	// hashed name generation stable regardless of the input source. This means
+	// that we can eventually remove the OSImageURLConfig struct.
+	type osImageURLConfig struct {
+		BaseOSContainerImage           string
+		BaseOSExtensionsContainerImage string
+		OSImageURL                     string
+		ReleaseVersion                 string
+	}
+
+	cfg := osImageURLConfig{
+		BaseOSContainerImage:           m.MachineConfig.Spec.OSImageURL,
+		BaseOSExtensionsContainerImage: m.MachineConfig.Spec.BaseOSExtensionsContainerImage,
+		// This value is purposely left empty because the ConfigMap does not actually
+		// populate this value. However, we want the hashing to be stable.
+		OSImageURL:     "",
+		ReleaseVersion: m.MachineConfig.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey],
+	}
 
 	// The objects considered for hashing described inline:
 	out := []interface{}{
@@ -70,8 +114,8 @@ func (m *MachineOSBuildOpts) objectsForHash() []interface{} {
 		m.MachineConfigPool.Spec.Configuration,
 		// The MachineOSConfig Spec field.
 		m.MachineOSConfig.Spec,
-		// The complete OSImageURLConfig object.
-		m.OSImageURLConfig,
+		// The complete osImageURLConfig object.
+		cfg,
 	}
 
 	return out
@@ -118,23 +162,6 @@ func (m *MachineOSBuildOpts) getHashedName() (string, error) {
 	return fmt.Sprintf("%x", hasher.Sum(nil)), nil
 }
 
-// Constructs the MachineOSBuildOpts by retrieving the OSImageURLConfig from
-// the API server.
-func NewMachineOSBuildOpts(ctx context.Context, kubeclient clientset.Interface, mosc *mcfgv1.MachineOSConfig, mcp *mcfgv1.MachineConfigPool) (MachineOSBuildOpts, error) {
-	// TODO: Consider an implementation that uses listers instead of API clients
-	// just to cut down on API server traffic.
-	osImageURLs, err := ctrlcommon.GetOSImageURLConfig(ctx, kubeclient)
-	if err != nil {
-		return MachineOSBuildOpts{}, fmt.Errorf("could not get OSImageURLConfig: %w", err)
-	}
-
-	return MachineOSBuildOpts{
-		MachineOSConfig:   mosc,
-		MachineConfigPool: mcp,
-		OSImageURLConfig:  osImageURLs,
-	}, nil
-}
-
 // Constructs a new MachineOSBuild object or panics trying. Useful for testing
 // scenarios.
 func NewMachineOSBuildOrDie(opts MachineOSBuildOpts) *mcfgv1.MachineOSBuild {
@@ -147,30 +174,6 @@ func NewMachineOSBuildOrDie(opts MachineOSBuildOpts) *mcfgv1.MachineOSBuild {
 	return mosb
 }
 
-// Retrieves the MachineOSBuildOpts from the API and constructs a new
-// MachineOSBuild object or panics trying. Useful for testing scenarios.
-func NewMachineOSBuildFromAPIOrDie(ctx context.Context, kubeclient clientset.Interface, mosc *mcfgv1.MachineOSConfig, mcp *mcfgv1.MachineConfigPool) *mcfgv1.MachineOSBuild {
-	mosb, err := NewMachineOSBuildFromAPI(ctx, kubeclient, mosc, mcp)
-
-	if err != nil {
-		panic(err)
-	}
-
-	return mosb
-}
-
-// Retrieves the MachineOSBuildOpts from the API and constructs a new
-// MachineOSBuild object.
-func NewMachineOSBuildFromAPI(ctx context.Context, kubeclient clientset.Interface, mosc *mcfgv1.MachineOSConfig, mcp *mcfgv1.MachineConfigPool) (*mcfgv1.MachineOSBuild, error) {
-	opts, err := NewMachineOSBuildOpts(ctx, kubeclient, mosc, mcp)
-
-	if err != nil {
-		return nil, fmt.Errorf("could not get MachineOSBuildOpts: %w", err)
-	}
-
-	return NewMachineOSBuild(opts)
-}
-
 // Constructs a new MachineOSBuild object with all of the labels, the tagged
 // image pushpsec, and a hashed name.
 func NewMachineOSBuild(opts MachineOSBuildOpts) (*mcfgv1.MachineOSBuild, error) {