AGENT-1454: Enforce APIServer TLS profile on IRI registry

cmd/machine-config-controller/start.go

@@ -138,6 +138,7 @@ func runStartCmd(_ *cobra.Command, _ []string) {
 				ctrlctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
 				ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(),
 				ctrlctx.ConfigInformerFactory.Config().V1().ClusterVersions(),
+				ctrlctx.ConfigInformerFactory.Config().V1().APIServers(),
 				ctrlctx.KubeInformerFactory.Core().V1().Secrets(),
 				ctrlctx.ClientBuilder.KubeClientOrDie("internalreleaseimage-controller"),
 				ctrlctx.ClientBuilder.MachineConfigClientOrDie("internalreleaseimage-controller"))

pkg/apihelpers/apihelpers.go

@@ -109,6 +109,22 @@ var (
 				},
 			},
 		},
+		Units: []opv1.NodeDisruptionPolicyStatusUnit{
+			{
+				Name: opv1.NodeDisruptionPolicyServiceName(constants.IRIRegistryServiceName),
+				Actions: []opv1.NodeDisruptionPolicyStatusAction{
+					{
+						Type: opv1.DaemonReloadStatusAction,
+					},
+					{
+						Type: opv1.RestartStatusAction,
+						Restart: &opv1.RestartService{
+							ServiceName: constants.IRIRegistryServiceName,
+						},
+					},
+				},
+			},
+		},
 		SSHKey: opv1.NodeDisruptionPolicyStatusSSHKey{
 			Actions: []opv1.NodeDisruptionPolicyStatusAction{
 				{

pkg/controller/bootstrap/bootstrap.go

@@ -305,7 +305,11 @@ func (b *Bootstrap) Run(destDir string) error {
 
 	if fgHandler != nil && fgHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) {
 		if iri != nil {
-			iriConfigs, err := internalreleaseimage.RunInternalReleaseImageBootstrap(iri, iriTLSCert, cconfig)
+			var tlsProfile *apicfgv1.TLSSecurityProfile
+			if apiServer != nil {
+				tlsProfile = apiServer.Spec.TLSSecurityProfile
+			}
+			iriConfigs, err := internalreleaseimage.RunInternalReleaseImageBootstrap(iri, iriTLSCert, cconfig, tlsProfile)
 			if err != nil {
 				return err
 			}

pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go

@@ -3,16 +3,17 @@ package internalreleaseimage
 import (
 	corev1 "k8s.io/api/core/v1"
 
+	configv1 "github.com/openshift/api/config/v1"
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	mcfgv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1"
 )
 
 // RunInternalReleaseImageBootstrap generates the MachineConfig objects for InternalReleaseImage that would have been generated by syncInternalReleaseImage.
-func RunInternalReleaseImageBootstrap(iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) ([]*mcfgv1.MachineConfig, error) {
+func RunInternalReleaseImageBootstrap(iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig, tlsProfile *configv1.TLSSecurityProfile) ([]*mcfgv1.MachineConfig, error) {
 	configs := []*mcfgv1.MachineConfig{}
 
 	for _, role := range SupportedRoles {
-		r := NewRendererByRole(role, iri, iriSecret, cconfig)
+		r := NewRendererByRole(role, iri, iriSecret, cconfig, tlsProfile)
 		mc, err := r.CreateEmptyMachineConfig()
 		if err != nil {
 			return nil, err

pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go

@@ -8,7 +8,7 @@ import (
 )
 
 func TestRunInternalReleaseImageBootstrap(t *testing.T) {
-	configs, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, iriCertSecret().obj, cconfig().obj)
+	configs, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, iriCertSecret().obj, cconfig().obj, nil)
 	assert.NoError(t, err)
 	verifyAllInternalReleaseImageMachineConfigs(t, configs)
 }

pkg/controller/internalreleaseimage/internalreleaseimage_controller.go

@@ -20,6 +20,7 @@ import (
 
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	mcfgv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1"
+	configv1 "github.com/openshift/api/config/v1"
 	configinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1"
 	configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
 	mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned"
@@ -71,10 +72,15 @@ type Controller struct {
 	clusterVersionLister       configlistersv1.ClusterVersionLister
 	clusterVersionListerSynced cache.InformerSynced
 
+	apiServerLister       configlistersv1.APIServerLister
+	apiServerListerSynced cache.InformerSynced
+
 	secretLister       corelistersv1.SecretLister
 	secretListerSynced cache.InformerSynced
 
 	queue workqueue.TypedRateLimitingInterface[string]
+
+	lastLoggedTLSProfile string
 }
 
 // New returns a new InternalReleaseImage controller.
@@ -83,6 +89,7 @@ func New(
 	ccInformer mcfginformersv1.ControllerConfigInformer,
 	mcInformer mcfginformersv1.MachineConfigInformer,
 	clusterVersionInformer configinformersv1.ClusterVersionInformer,
+	apiServerInformer configinformersv1.APIServerInformer,
 	secretInformer coreinformersv1.SecretInformer,
 	kubeClient clientset.Interface,
 	mcfgClient mcfgclientset.Interface,
@@ -121,6 +128,11 @@ func New(
 		UpdateFunc: ctrl.updateSecret,
 	})
 
+	apiServerInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc:    ctrl.addAPIServer,
+		UpdateFunc: ctrl.updateAPIServer,
+	})
+
 	ctrl.iriLister = iriInformer.Lister()
 	ctrl.iriListerSynced = iriInformer.Informer().HasSynced
 
@@ -133,6 +145,9 @@ func New(
 	ctrl.clusterVersionLister = clusterVersionInformer.Lister()
 	ctrl.clusterVersionListerSynced = clusterVersionInformer.Informer().HasSynced
 
+	ctrl.apiServerLister = apiServerInformer.Lister()
+	ctrl.apiServerListerSynced = apiServerInformer.Informer().HasSynced
+
 	ctrl.secretLister = secretInformer.Lister()
 	ctrl.secretListerSynced = secretInformer.Informer().HasSynced
 
@@ -144,7 +159,7 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) {
 	defer utilruntime.HandleCrash()
 	defer ctrl.queue.ShutDown()
 
-	if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced, ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.clusterVersionListerSynced, ctrl.secretListerSynced) {
+	if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced, ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.clusterVersionListerSynced, ctrl.apiServerListerSynced, ctrl.secretListerSynced) {
 		return
 	}
 
@@ -286,6 +301,22 @@ func (ctrl *Controller) updateSecret(obj, _ interface{}) {
 	ctrl.queue.Add(ctrlcommon.InternalReleaseImageInstanceName)
 }
 
+func (ctrl *Controller) addAPIServer(obj interface{}) {
+	apiServer := obj.(*configv1.APIServer)
+	klog.V(4).Infof("APIServer %s added", apiServer.Name)
+	ctrl.queue.Add(ctrlcommon.InternalReleaseImageInstanceName)
+}
+
+func (ctrl *Controller) updateAPIServer(old, cur interface{}) {
+	oldAPIServer := old.(*configv1.APIServer)
+	newAPIServer := cur.(*configv1.APIServer)
+	if reflect.DeepEqual(oldAPIServer.Spec.TLSSecurityProfile, newAPIServer.Spec.TLSSecurityProfile) {
+		return
+	}
+	klog.V(4).Infof("APIServer %s TLS profile updated", newAPIServer.Name)
+	ctrl.queue.Add(ctrlcommon.InternalReleaseImageInstanceName)
+}
+
 func (ctrl *Controller) enqueue(iri *mcfgv1alpha1.InternalReleaseImage) {
 	key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(iri)
 	if err != nil {
@@ -341,8 +372,29 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error {
 		return fmt.Errorf("could not get Secret %s: %w", ctrlcommon.InternalReleaseImageTLSSecretName, err)
 	}
 
+	// Fetch the APIServer TLS profile. If not found, use nil (defaults to Intermediate).
+	apiServer, err := ctrl.apiServerLister.Get(ctrlcommon.APIServerInstanceName)
+	if err != nil && !errors.IsNotFound(err) {
+		return fmt.Errorf("could not get APIServer: %w", err)
+	}
+	var tlsProfile *configv1.TLSSecurityProfile
+	if apiServer != nil {
+		tlsProfile = apiServer.Spec.TLSSecurityProfile
+	}
+
+	profileType := "nil (defaulting to Intermediate)"
+	if tlsProfile != nil {
+		profileType = string(tlsProfile.Type)
+	}
+	tlsMinVersion, tlsCipherSuites := registryTLSFromProfile(tlsProfile)
+	tlsLogMsg := fmt.Sprintf("APIServer TLS profile: %s, IRI registry TLS minimum version: %s, cipher suites: %q", profileType, tlsMinVersion, tlsCipherSuites)
+	if tlsLogMsg != ctrl.lastLoggedTLSProfile {
+		klog.Infof("%s", tlsLogMsg)
+		ctrl.lastLoggedTLSProfile = tlsLogMsg
+	}
+
 	for _, role := range SupportedRoles {
-		r := NewRendererByRole(role, iri, iriSecret, cconfig)
+		r := NewRendererByRole(role, iri, iriSecret, cconfig, tlsProfile)
 
 		mc, err := ctrl.mcLister.Get(r.GetMachineConfigName())
 		isNotFound := errors.IsNotFound(err)

pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go

@@ -95,6 +95,36 @@ func TestInternalReleaseImageCreate(t *testing.T) {
 				verifyInternalReleaseWorkerMachineConfig(t, actualWorkerMC)
 			},
 		},
+		{
+			name: "apply Modern TLS profile from APIServer",
+			initialObjects: objs(
+				iri(), clusterVersion(), cconfig(), iriCertSecret(),
+				apiServer().tlsProfile(configv1.TLSProfileModernType)),
+			verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
+				ignCfg, err := ctrlcommon.ParseAndConvertConfig(actualMasterMC.Spec.Config.Raw)
+				assert.NoError(t, err)
+				assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "REGISTRY_HTTP_TLS_MINIMUMTLS=tls1.3")
+				assert.NotContains(t, *ignCfg.Systemd.Units[0].Contents, "REGISTRY_HTTP_TLS_CIPHERSUITES")
+			},
+		},
+		{
+			name: "cipher suites with spaces are quoted in systemd unit to prevent word splitting",
+			initialObjects: objs(
+				iri(), clusterVersion(), cconfig(), iriCertSecret(),
+				apiServer().tlsProfile(configv1.TLSProfileIntermediateType)),
+			verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
+				ignCfg, err := ctrlcommon.ParseAndConvertConfig(actualMasterMC.Spec.Config.Raw)
+				assert.NoError(t, err)
+				unitContents := *ignCfg.Systemd.Units[0].Contents
+				assert.Contains(t, unitContents, "REGISTRY_HTTP_TLS_MINIMUMTLS=tls1.2")
+				assert.Contains(t, unitContents, "REGISTRY_HTTP_TLS_CIPHERSUITES=")
+				// The cipher suites value (a YAML array with spaces) must be quoted
+				// in the systemd unit to prevent word splitting. Without quotes,
+				// systemd splits on spaces and podman interprets cipher names as
+				// container image references.
+				assert.Contains(t, unitContents, `-e "REGISTRY_HTTP_TLS_CIPHERSUITES=[`)
+			},
+		},
 		{
 			name: "machine-config cascade delete on iri removal - removes the first machineconfig",
 			initialObjects: objs(
@@ -173,6 +203,7 @@ type fixture struct {
 	mcLister             []*mcfgv1.MachineConfig
 	secretLister         []*corev1.Secret
 	clusterVersionLister []*configv1.ClusterVersion
+	apiServerLister      []*configv1.APIServer
 
 	controller    *Controller
 	objects       []runtime.Object
@@ -198,6 +229,9 @@ func (f *fixture) setupObjects(objs []runtime.Object) {
 			}
 		case *configv1.ClusterVersion:
 			f.configObjects = append(f.configObjects, obj)
+		case *configv1.APIServer:
+			f.configObjects = append(f.configObjects, obj)
+			f.apiServerLister = append(f.apiServerLister, obj.(*configv1.APIServer))
 		default:
 			f.objects = append(f.objects, obj)
 			switch o := obj.(type) {
@@ -226,6 +260,7 @@ func (f *fixture) newController() *Controller {
 		i.Machineconfiguration().V1().ControllerConfigs(),
 		i.Machineconfiguration().V1().MachineConfigs(),
 		ci.Config().V1().ClusterVersions(),
+		ci.Config().V1().APIServers(),
 		k.Core().V1().Secrets(),
 		f.k8sClient,
 		f.client,
@@ -236,6 +271,7 @@ func (f *fixture) newController() *Controller {
 	c.ccListerSynced = alwaysReady
 	c.mcListerSynced = alwaysReady
 	c.clusterVersionListerSynced = alwaysReady
+	c.apiServerListerSynced = alwaysReady
 	c.secretListerSynced = alwaysReady
 	c.eventRecorder = &record.FakeRecorder{}
 
@@ -264,6 +300,9 @@ func (f *fixture) newController() *Controller {
 	for _, c := range f.clusterVersionLister {
 		ci.Config().V1().ClusterVersions().Informer().GetIndexer().Add(c)
 	}
+	for _, c := range f.apiServerLister {
+		ci.Config().V1().APIServers().Informer().GetIndexer().Add(c)
+	}
 
 	return c
 }

pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go

@@ -34,6 +34,7 @@ func verifyInternalReleaseMasterMachineConfig(t *testing.T, mc *mcfgv1.MachineCo
 
 	assert.Len(t, ignCfg.Systemd.Units, 1)
 	assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "docker-registry-image-pullspec")
+	assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "REGISTRY_HTTP_TLS_MINIMUMTLS=tls1.2")
 
 	assert.Len(t, ignCfg.Storage.Files, 4, "Found an unexpected file")
 	verifyIgnitionFile(t, &ignCfg, "/etc/pki/ca-trust/source/anchors/iri-root-ca.crt", "iri-root-ca-data")
@@ -247,3 +248,29 @@ func clusterVersion() *clusterVersionBuilder {
 func (cvb *clusterVersionBuilder) build() runtime.Object {
 	return cvb.obj
 }
+
+// apiServerBuilder simplifies the creation of an APIServer resource in the test.
+type apiServerBuilder struct {
+	obj *configv1.APIServer
+}
+
+func apiServer() *apiServerBuilder {
+	return &apiServerBuilder{
+		obj: &configv1.APIServer{
+			ObjectMeta: v1.ObjectMeta{
+				Name: ctrlcommon.APIServerInstanceName,
+			},
+		},
+	}
+}
+
+func (asb *apiServerBuilder) tlsProfile(profileType configv1.TLSProfileType) *apiServerBuilder {
+	asb.obj.Spec.TLSSecurityProfile = &configv1.TLSSecurityProfile{
+		Type: profileType,
+	}
+	return asb
+}
+
+func (asb *apiServerBuilder) build() runtime.Object {
+	return asb.obj
+}