diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index 504ce97bb0..e42eb38f59 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/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,7 +488,7 @@ 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) @@ -494,6 +496,15 @@ func (b *Bootstrap) fetchOSImageStream( 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 { diff --git a/pkg/controller/bootstrap/bootstrap_test.go b/pkg/controller/bootstrap/bootstrap_test.go index 38bd28ee1e..3bb28e3d99 100644 --- a/pkg/controller/bootstrap/bootstrap_test.go +++ b/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") }) } } diff --git a/pkg/controller/bootstrap/testdata/bootstrap-hypershift/machineconfigcontroller-controllerconfig.yaml b/pkg/controller/bootstrap/testdata/bootstrap-hypershift/machineconfigcontroller-controllerconfig.yaml new file mode 100644 index 0000000000..bfd934f0dd --- /dev/null +++ b/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== diff --git a/pkg/controller/bootstrap/testdata/bootstrap/featuregate.yaml b/pkg/controller/bootstrap/testdata/bootstrap/featuregate.yaml index eaeda285cd..2a6f03d7d8 100644 --- a/pkg/controller/bootstrap/testdata/bootstrap/featuregate.yaml +++ b/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