MCO-2146: do not use OSImageStream in Hypershift

pkg/controller/bootstrap/bootstrap.go

@@ -51,6 +51,8 @@ type Bootstrap struct {
 	manifestDir string
 	// pull secret file
 	pullSecretFile string
+	// OSImageStreams factory. Used for testing purposes.
+	imageStreamFactory osimagestream.ImageStreamFactory
 }
 
 // New returns controller for bootstrap
@@ -225,8 +227,8 @@ func (b *Bootstrap) Run(destDir string) error {
 	}
 
 	var osImageStream *mcfgv1alpha1.OSImageStream
-	// Enable OSImageStreams if the FeatureGate is active and the deployment is not OKD
-	if osimagestream.IsFeatureEnabled(fgHandler) {
+	// Enable OSImageStreams if the FeatureGate is active and the control plane topology is not external (e.g., Hypershift)
+	if osimagestream.IsFeatureEnabled(fgHandler) && cconfig.Spec.Infra.Status.ControlPlaneTopology != apicfgv1.ExternalTopologyMode {
 		osImageStream, err = b.fetchOSImageStream(imageStream, cconfig, icspRules, idmsRules, itmsRules, imgCfg, pullSecret)
 		if err != nil {
 			return err
@@ -486,14 +488,23 @@ func (b *Bootstrap) fetchOSImageStream(
 			OSImage:           cconfig.Spec.BaseOSContainerImage,
 			OSExtensionsImage: cconfig.Spec.BaseOSExtensionsContainerImage,
 		},
-		osimagestream.NewDefaultStreamSourceFactory(nil, &osimagestream.DefaultImagesInspectorFactory{}),
+		b.getImageStreamFactory(),
 	)
 	if err != nil {
 		return nil, fmt.Errorf("error inspecting available OSImageStreams: %w", err)
 	}
 	return osImageStream, nil
 }
 
+// Returns the embedded ImageStreamFactory or constructs a new default one. Used primarily for testing.
+func (b *Bootstrap) getImageStreamFactory() osimagestream.ImageStreamFactory {
+	if b.imageStreamFactory != nil {
+		return b.imageStreamFactory
+	}
+
+	return osimagestream.NewDefaultStreamSourceFactory(nil, &osimagestream.DefaultImagesInspectorFactory{})
+}
+
 func getValidPullSecretFromBytes(sData []byte) (*corev1.Secret, error) {
 	obji, err := runtime.Decode(kscheme.Codecs.UniversalDecoder(corev1.SchemeGroupVersion), sData)
 	if err != nil {

pkg/controller/bootstrap/bootstrap_test.go

@@ -1,20 +1,26 @@
 package bootstrap
 
 import (
+	"context"
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
 
+	"github.com/containers/image/v5/types"
 	ign3types "github.com/coreos/ignition/v2/config/v3_5/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/util/diff"
 
+	imagev1 "github.com/openshift/api/image/v1"
+	"github.com/openshift/api/machineconfiguration/v1alpha1"
 	mcoResourceRead "github.com/openshift/machine-config-operator/lib/resourceread"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
+	"github.com/openshift/machine-config-operator/pkg/osimagestream"
 )
 
 func TestParseManifests(t *testing.T) {
@@ -157,13 +163,85 @@ spec:
 	}
 }
 
-func TestBootstrapRun(t *testing.T) {
-	destDir, err := os.MkdirTemp("", "controller-bootstrap")
+// Implements a fake ImageStreamFactory.
+type fakeImageStreamFactory struct {
+	// The OSImageStream to return.
+	stream *v1alpha1.OSImageStream
+	// Whether the CreateRuntimeSources method was called.
+	createRuntimeSourcesCalled bool
+	// Whether the CreateBootsrapSources method was called.
+	createBootstrapSourcesCalled bool
+}
+
+func (f *fakeImageStreamFactory) CreateRuntimeSources(_ context.Context, _ string, _ *types.SystemContext) (*v1alpha1.OSImageStream, error) {
+	f.createRuntimeSourcesCalled = true
+	return f.stream, nil
+}
+
+func (f *fakeImageStreamFactory) CreateBootstrapSources(_ context.Context, _ *imagev1.ImageStream, _ *osimagestream.OSImageTuple, _ *types.SystemContext) (*v1alpha1.OSImageStream, error) {
+	f.createBootstrapSourcesCalled = true
+	return f.stream, nil
+}
+
+// Instantiates a new instance of the Bootstrap struct for testing. This also
+// does the following:
+// 1. Copies the data from testdata/bootstrap into a temp directory so that it
+// may be safely overwritten to test specific scenarios.
+// 2. Creates a fake ImageStreamFactory instance and wires it up to return an
+// OSImageStream.
+func setupForBootstrapTest(t *testing.T) (*Bootstrap, *fakeImageStreamFactory, string, string) {
+	t.Helper()
+
+	srcDir := t.TempDir()
+	destDir := t.TempDir()
+
+	require.NoError(t, exec.Command("cp", "-r", "testdata/bootstrap/.", srcDir).Run())
+
+	bootstrap := New("../../../templates", srcDir, filepath.Join(srcDir, "machineconfigcontroller-pull-secret"))
+
+	fakeFactory := &fakeImageStreamFactory{
+		stream: &v1alpha1.OSImageStream{
+			Status: v1alpha1.OSImageStreamStatus{
+				AvailableStreams: []v1alpha1.OSImageStreamSet{
+					{
+						Name:              "stream-1",
+						OSImage:           v1alpha1.ImageDigestFormat("registry.host.com/os:latest"),
+						OSExtensionsImage: v1alpha1.ImageDigestFormat("registry.host.com/extensions:latest"),
+					},
+				},
+				DefaultStream: "stream-1",
+			},
+		},
+	}
+
+	bootstrap.imageStreamFactory = fakeFactory
+
+	return bootstrap, fakeFactory, srcDir, destDir
+}
+
+// Ensures that when Hypershift is enabled that the OSImageStream value is not consumed.
+func TestBootstrapRunHypershift(t *testing.T) {
+	bootstrap, fakeFactory, srcDir, destDir := setupForBootstrapTest(t)
+
+	// Overwrite the default ControllerConfig with one that specifies an
+	// external control plane value e.g., Hypershift.
+	require.NoError(t, exec.Command("cp", "testdata/bootstrap-hypershift/machineconfigcontroller-controllerconfig.yaml", srcDir).Run())
+
+	err := bootstrap.Run(destDir)
 	require.NoError(t, err)
-	defer os.RemoveAll(destDir)
 
-	bootstrap := New("../../../templates", "testdata/bootstrap", "testdata/bootstrap/machineconfigcontroller-pull-secret")
-	err = bootstrap.Run(destDir)
+	// Ensure that the values from the OSImageStream are *not* populated into the ControllerConfig.
+	assert.False(t, fakeFactory.createBootstrapSourcesCalled)
+	cconfigBytes, err := os.ReadFile(filepath.Join(destDir, "controller-config", "machine-config-controller.yaml"))
+	require.NoError(t, err)
+	assert.NotContains(t, string(cconfigBytes), "baseOSContainerImage: registry.host.com/os:latest")
+	assert.NotContains(t, string(cconfigBytes), "baseOSExtensionsContainerImage: registry.host.com/extensions:latest")
+}
+
+func TestBootstrapRun(t *testing.T) {
+	bootstrap, fakeFactory, _, destDir := setupForBootstrapTest(t)
+
+	err := bootstrap.Run(destDir)
 	require.NoError(t, err)
 
 	for _, poolName := range []string{"master", "worker"} {
@@ -188,15 +266,22 @@ func TestBootstrapRun(t *testing.T) {
 				require.False(t, f.Path == "/etc/kubernetes/kubelet-ca.crt")
 			}
 			require.NotNil(t, registriesConfig)
-			contents, err := ctrlcommon.DecodeIgnitionFileContents(registriesConfig.Contents.Source, registriesConfig.Contents.Compression)
+			ignContents, err := ctrlcommon.DecodeIgnitionFileContents(registriesConfig.Contents.Source, registriesConfig.Contents.Compression)
 			require.NoError(t, err)
 			// Only a minimal presence check; more comprehensive tests that the contents correspond to the ICSP semantics are
 			// maintained in pkg/controller/container-runtime-config.
-			assert.Contains(t, string(contents), "registry.mirror.example.com/ocp")
-			assert.Contains(t, string(contents), "insecure-reg-1.io")
-			assert.Contains(t, string(contents), "insecure-reg-2.io")
-			assert.Contains(t, string(contents), "blocked-reg.io")
-			assert.NotContains(t, string(contents), "release-registry.product.example.org")
+			assert.Contains(t, string(ignContents), "registry.mirror.example.com/ocp")
+			assert.Contains(t, string(ignContents), "insecure-reg-1.io")
+			assert.Contains(t, string(ignContents), "insecure-reg-2.io")
+			assert.Contains(t, string(ignContents), "blocked-reg.io")
+			assert.NotContains(t, string(ignContents), "release-registry.product.example.org")
+
+			// Ensure that the values from the OSImageStream are populated into the ControllerConfig.
+			assert.True(t, fakeFactory.createBootstrapSourcesCalled)
+			cconfigBytes, err := os.ReadFile(filepath.Join(destDir, "controller-config", "machine-config-controller.yaml"))
+			require.NoError(t, err)
+			assert.Contains(t, string(cconfigBytes), "baseOSContainerImage: registry.host.com/os:latest")
+			assert.Contains(t, string(cconfigBytes), "baseOSExtensionsContainerImage: registry.host.com/extensions:latest")
 		})
 	}
 }

pkg/controller/bootstrap/testdata/bootstrap-hypershift/machineconfigcontroller-controllerconfig.yaml

@@ -0,0 +1,39 @@
+apiVersion: machineconfiguration.openshift.io/v1
+kind: ControllerConfig
+metadata:
+  name: machine-config-controller
+  annotations:
+    machineconfiguration.openshift.io/generated-by-version: "v0.0.0-was-not-built-properly"
+spec:
+  additionalTrustBundle: null
+  cloudProviderConfig: ""
+  clusterDNSIP: 172.30.0.10
+  images:
+    baremetalRuntimeCfgImage: ""
+    corednsImage: ""
+    haproxyImage: ""
+    infraImageKey: registry.product.example.org/ocp/4.2-DATE-VERSION@sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+    keepalivedImage: ""
+    kubeClientAgentImageKey: registry.product.example.org/ocp/4.2-DATE-VERSION@sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc
+  infra:
+    apiVersion: config.openshift.io/v1
+    kind: Infrastructure
+    metadata:
+      creationTimestamp: null
+      name: cluster
+    spec:
+      cloudConfig:
+        name: ""
+    status:
+      apiServerInternalURI: https://api-int.domain.example.com:6443
+      apiServerURL: https://api.domain.example.com:6443
+      infrastructureName: lab-0aaaa
+      infrastructureTopology: External
+      controlPlaneTopology: External
+      platformStatus:
+        type: None
+  kubeAPIServerServingCAData: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCktVQkUgQVBJIFNFUlZFUiBTRVJWSU5HIENBIERBVEEKLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=
+  osImageURL: registry.product.example.org/ocp/4.2-DATE-VERSION@sha256:eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+  releaseImage: release-registry.product.example.org/ocp/4.2-date-version@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
+  proxy: null
+  rootCAData: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tClJPT1QgQ0EgREFUQQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg==

pkg/controller/bootstrap/testdata/bootstrap/featuregate.yaml

@@ -7,5 +7,6 @@ status:
   - version: 0.0.1-snapshot
     enabled:
     - name: OpenShiftPodSecurityAdmission
+    - name: OSStreams
     disabled:
     - name: SigstoreImageVerification