[release-4.21] OCPBUGS-68364: InternalReleaseImage should use TLS certs

pkg/controller/bootstrap/bootstrap.go

@@ -102,6 +102,7 @@ func (b *Bootstrap) Run(destDir string) error {
 		imgCfg               *apicfgv1.Image
 		apiServer            *apicfgv1.APIServer
 		iri                  *mcfgv1alpha1.InternalReleaseImage
+		iriTLSCert           *corev1.Secret
 	)
 	for _, info := range infos {
 		if info.IsDir() {
@@ -171,6 +172,10 @@ func (b *Bootstrap) Run(destDir string) error {
 				if obj.GetName() == ctrlcommon.InternalReleaseImageInstanceName {
 					iri = obj
 				}
+			case *corev1.Secret:
+				if obj.GetName() == ctrlcommon.InternalReleaseImageTLSSecretName {
+					iriTLSCert = obj
+				}
 			default:
 				klog.Infof("skipping %q [%d] manifest because of unhandled %T", file.Name(), idx+1, obji)
 			}
@@ -253,11 +258,11 @@ func (b *Bootstrap) Run(destDir string) error {
 
 	if fgHandler != nil && fgHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) {
 		if iri != nil {
-			iriConfig, err := internalreleaseimage.RunInternalReleaseImageBootstrap(iri, cconfig)
+			iriConfigs, err := internalreleaseimage.RunInternalReleaseImageBootstrap(iri, iriTLSCert, cconfig)
 			if err != nil {
 				return err
 			}
-			configs = append(configs, iriConfig)
+			configs = append(configs, iriConfigs...)
 			klog.Infof("Successfully generated MachineConfig from InternalReleaseImage.")
 		}
 	}

pkg/controller/common/constants.go

@@ -66,6 +66,9 @@ const (
 	// InternalReleaseImageInstanceName is a singleton name for InternalReleaseImage
 	InternalReleaseImageInstanceName = "cluster"
 
+	// InternalReleaseImageTLSSecretName is the name of the secret manifest containing the InternalReleaseImage TLS certificate.
+	InternalReleaseImageTLSSecretName = "internal-release-image-tls"
+
 	// APIServerInstanceName is a singleton name for APIServer configuration
 	APIServerBootstrapFileLocation = "/etc/mcs/bootstrap/api-server/api-server.yaml"
 

pkg/controller/internalreleaseimage/OWNERS

@@ -0,0 +1,14 @@
+# See the OWNERS docs: https://git.k8s.io/community/contributors/guide/owners.md
+
+approvers:
+  - andfasano
+  - bfournie
+  - pawanpinjarkar
+  - rwsu
+  - zaneb
+reviewers:
+  - andfasano
+  - bfournie
+  - pawanpinjarkar
+  - rwsu
+  - zaneb

pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go

@@ -1,11 +1,28 @@
 package internalreleaseimage
 
 import (
+	corev1 "k8s.io/api/core/v1"
+
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	mcfgv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1"
 )
 
-// RunInternalReleaseImageBootstrap generates a MachineConfig object for InternalReleaseImage that would have been generated by syncInternalReleaseImage
-func RunInternalReleaseImageBootstrap(iri *mcfgv1alpha1.InternalReleaseImage, controllerConfig *mcfgv1.ControllerConfig) (*mcfgv1.MachineConfig, error) {
-	return generateInternalReleaseImageMachineConfig(iri, controllerConfig)
+// 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) {
+	configs := []*mcfgv1.MachineConfig{}
+
+	for _, role := range SupportedRoles {
+		r := NewRendererByRole(role, iri, iriSecret, cconfig)
+		mc, err := r.CreateEmptyMachineConfig()
+		if err != nil {
+			return nil, err
+		}
+		err = r.RenderAndSetIgnition(mc)
+		if err != nil {
+			return nil, err
+		}
+		configs = append(configs, mc)
+	}
+
+	return configs, nil
 }

pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go

@@ -3,25 +3,12 @@ package internalreleaseimage
 import (
 	"testing"
 
-	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	mcfgv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1"
-	templatectrl "github.com/openshift/machine-config-operator/pkg/controller/template"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestRunInternalReleaseImageBootstrap(t *testing.T) {
-	cc := &mcfgv1.ControllerConfig{
-		Spec: mcfgv1.ControllerConfigSpec{
-			Images: map[string]string{
-				templatectrl.DockerRegistryKey: "docker-registry-image-pullspec",
-			},
-		},
-	}
-
-	mc, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, cc)
+	configs, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, iriCertSecret().obj, cconfig().obj)
 	assert.NoError(t, err)
-	assert.Equal(t, mc.Name, iriMachineConfigName)
-	assert.Equal(t, mc.Labels[mcfgv1.MachineConfigRoleLabelKey], "master")
-	assert.Equal(t, mc.OwnerReferences[0].Kind, "InternalReleaseImage")
-	assert.Contains(t, string(mc.Spec.Config.Raw), "docker-registry-image-pullspec")
+	verifyAllInternalReleaseImageMachineConfigs(t, configs)
 }

pkg/controller/internalreleaseimage/internalreleaseimage_controller.go

@@ -1,16 +1,11 @@
 package internalreleaseimage
 
 import (
-	"bytes"
 	"context"
-	_ "embed"
 	"fmt"
 	"reflect"
-	"text/template"
 	"time"
 
-	"github.com/clarketm/json"
-	ign3types "github.com/coreos/ignition/v2/config/v3_5/types"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -31,17 +26,13 @@ import (
 	mcfginformersv1alpha1 "github.com/openshift/client-go/machineconfiguration/informers/externalversions/machineconfiguration/v1alpha1"
 	mcfglistersv1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1"
 	mcfglistersv1alpha1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1alpha1"
-	"github.com/openshift/machine-config-operator/pkg/controller/common"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	templatectrl "github.com/openshift/machine-config-operator/pkg/controller/template"
-	"github.com/openshift/machine-config-operator/pkg/version"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 const (
 	maxRetries = 15
-
-	iriMachineConfigName = "02-master-internalreleaseimage"
 )
 
 var (
@@ -53,14 +44,12 @@ var (
 		Duration: 100 * time.Millisecond,
 		Jitter:   1.0,
 	}
-
-	//go:embed templates/iri-registry.service.yaml
-	iriRegistryServiceTemplate string
 )
 
 // Controller defines the InternalReleaseImage controller.
 type Controller struct {
 	client        mcfgclientset.Interface
+	kubeClient    clientset.Interface
 	eventRecorder record.EventRecorder
 
 	syncHandler                 func(mcp string) error
@@ -91,8 +80,8 @@ func New(
 	eventBroadcaster.StartRecordingToSink(&corev1client.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")})
 
 	ctrl := &Controller{
-		client: mcfgClient,
-
+		client:        mcfgClient,
+		kubeClient:    kubeClient,
 		eventRecorder: ctrlcommon.NamespacedEventRecorder(eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "machineconfigcontroller-internalreleaseimagecontroller"})),
 		queue: workqueue.NewTypedRateLimitingQueueWithConfig(
 			workqueue.DefaultTypedControllerRateLimiter[string](),
@@ -255,7 +244,8 @@ func (ctrl *Controller) deleteMachineConfig(obj interface{}) {
 func (ctrl *Controller) processMachineConfigEvent(obj interface{}, logMsg string) {
 	mc := obj.(*mcfgv1.MachineConfig)
 
-	if mc.Name != iriMachineConfigName {
+	// Skip any event not related to the InternalReleaseImage machine configs
+	if len(mc.OwnerReferences) == 0 || mc.OwnerReferences[0].Kind != controllerKind.Kind {
 		return
 	}
 
@@ -300,137 +290,85 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error {
 	// Deep-copy otherwise we are mutating our cache.
 	iri = iri.DeepCopy()
 
-	// Check for Deleted InternalReleaseImage and optionally delete finalizers
+	// Check for Deleted InternalReleaseImage and optionally delete finalizers.
 	if !iri.DeletionTimestamp.IsZero() {
 		if len(iri.GetFinalizers()) > 0 {
 			return ctrl.cascadeDelete(iri)
 		}
 		return nil
 	}
 
-	// Create or update InternalReleaseImage MachineConfig
-	mc, err := ctrl.client.MachineconfigurationV1().MachineConfigs().Get(context.TODO(), iriMachineConfigName, metav1.GetOptions{})
-	isNotFound := errors.IsNotFound(err)
-	if err != nil && !isNotFound {
-		return err // syncStatus, could not find MachineConfig
-	}
-
 	cconfig, err := ctrl.client.MachineconfigurationV1().ControllerConfigs().Get(context.TODO(), ctrlcommon.ControllerConfigName, metav1.GetOptions{})
 	if err != nil {
 		return fmt.Errorf("could not get ControllerConfig %w", err)
 	}
 
-	if isNotFound {
-		mc, err = generateInternalReleaseImageMachineConfig(iri, cconfig)
-	} else {
-		err = updateInternalReleaseImageMachineConfig(mc, cconfig)
-	}
+	iriSecret, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{})
 	if err != nil {
-		return err // syncStatus, could not create/update MachineConfig
+		return fmt.Errorf("could not get Secret %s: %w", ctrlcommon.InternalReleaseImageTLSSecretName, err)
 	}
 
-	if err := retry.RetryOnConflict(updateBackoff, func() error {
-		var err error
+	for _, role := range SupportedRoles {
+		r := NewRendererByRole(role, iri, iriSecret, cconfig)
+
+		mc, err := ctrl.client.MachineconfigurationV1().MachineConfigs().Get(context.TODO(), r.GetMachineConfigName(), metav1.GetOptions{})
+		isNotFound := errors.IsNotFound(err)
+		if err != nil && !isNotFound {
+			return err // syncStatus, could not find MachineConfig
+		}
 		if isNotFound {
-			_, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Create(context.TODO(), mc, metav1.CreateOptions{})
-		} else {
-			_, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Update(context.TODO(), mc, metav1.UpdateOptions{})
+			mc, err = r.CreateEmptyMachineConfig()
+			if err != nil {
+				return err // syncStatusOnly, could not create MachineConfig
+			}
 		}
-		return err
-	}); err != nil {
-		return err // syncStatus, could not Create/Update MachineConfig
-	}
 
-	// Add finalizer to the InternalReleaseImage
-	if err := ctrl.addFinalizerToInternalReleaseImage(iri, mc); err != nil {
-		return err // syncStatus , could not add finalizers
+		err = r.RenderAndSetIgnition(mc)
+		if err != nil {
+			return err // syncStatus, could not generate IRI configs
+		}
+		err = ctrl.createOrUpdateMachineConfig(isNotFound, mc)
+		if err != nil {
+			return err // syncStatus, could not Create/Update MachineConfig
+		}
+		if err := ctrl.addFinalizerToInternalReleaseImage(iri, mc); err != nil {
+			return err // syncStatus , could not add finalizers
+		}
 	}
 
 	return nil
 }
 
-func updateInternalReleaseImageMachineConfig(mc *mcfgv1.MachineConfig, controllerConfig *mcfgv1.ControllerConfig) error {
-	ignCfg, err := generateIgnitionFromTemplate(controllerConfig)
-	if err != nil {
-		return err
-	}
-
-	rawIgn, err := json.Marshal(ignCfg)
-	if err != nil {
+func (ctrl *Controller) createOrUpdateMachineConfig(isNotFound bool, mc *mcfgv1.MachineConfig) error {
+	return retry.RetryOnConflict(updateBackoff, func() error {
+		var err error
+		if isNotFound {
+			_, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Create(context.TODO(), mc, metav1.CreateOptions{})
+		} else {
+			_, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Update(context.TODO(), mc, metav1.UpdateOptions{})
+		}
 		return err
-	}
-
-	mc.Spec.Config.Raw = rawIgn
-	return nil
+	})
 }
 
 func (ctrl *Controller) addFinalizerToInternalReleaseImage(iri *mcfgv1alpha1.InternalReleaseImage, mc *mcfgv1.MachineConfig) error {
-	if len(iri.GetFinalizers()) > 0 {
+	if ctrlcommon.InSlice(mc.Name, iri.Finalizers) {
 		return nil
 	}
 
-	return ctrl.updateInternalReleaseImageFinalizers(iri, []string{mc.Name})
+	iri.Finalizers = append(iri.Finalizers, mc.Name)
+	_, err := ctrl.client.MachineconfigurationV1alpha1().InternalReleaseImages().Update(context.TODO(), iri, metav1.UpdateOptions{})
+	return err
 }
 
 func (ctrl *Controller) cascadeDelete(iri *mcfgv1alpha1.InternalReleaseImage) error {
-	// Delete the InternalReleaseImage machine config
-	err := ctrl.client.MachineconfigurationV1().MachineConfigs().Delete(context.TODO(), iriMachineConfigName, metav1.DeleteOptions{})
+	mcName := iri.GetFinalizers()[0]
+	err := ctrl.client.MachineconfigurationV1().MachineConfigs().Delete(context.TODO(), mcName, metav1.DeleteOptions{})
 	if err != nil && !errors.IsNotFound(err) {
 		return err
 	}
 
-	// Remove the InternalRelaseImage finalizer
-	return ctrl.updateInternalReleaseImageFinalizers(iri, []string{})
-}
-
-func (ctrl *Controller) updateInternalReleaseImageFinalizers(iri *mcfgv1alpha1.InternalReleaseImage, finalizers []string) error {
-	iri.SetFinalizers(finalizers)
-	_, err := ctrl.client.MachineconfigurationV1alpha1().InternalReleaseImages().Update(context.TODO(), iri, metav1.UpdateOptions{})
+	iri.Finalizers = append([]string{}, iri.Finalizers[1:]...)
+	_, err = ctrl.client.MachineconfigurationV1alpha1().InternalReleaseImages().Update(context.TODO(), iri, metav1.UpdateOptions{})
 	return err
 }
-
-func generateInternalReleaseImageMachineConfig(iri *mcfgv1alpha1.InternalReleaseImage, controllerConfig *mcfgv1.ControllerConfig) (*mcfgv1.MachineConfig, error) {
-	ignCfg, err := generateIgnitionFromTemplate(controllerConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	mcfg, err := ctrlcommon.MachineConfigFromIgnConfig(common.MachineConfigPoolMaster, iriMachineConfigName, ignCfg)
-	if err != nil {
-		return nil, fmt.Errorf("error creating MachineConfig from Ignition config: %w", err)
-	}
-
-	cref := metav1.NewControllerRef(iri, controllerKind)
-	mcfg.SetOwnerReferences([]metav1.OwnerReference{*cref})
-	mcfg.SetAnnotations(map[string]string{
-		ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash,
-	})
-
-	return mcfg, nil
-}
-
-func generateIgnitionFromTemplate(controllerConfig *mcfgv1.ControllerConfig) (*ign3types.Config, error) {
-	// Parse the iri template
-	tmpl, err := template.New("iri-template").Parse(iriRegistryServiceTemplate)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse iri-template : %w", err)
-	}
-
-	type iriRenderConfig struct {
-		DockerRegistryImage string
-	}
-
-	buf := new(bytes.Buffer)
-	if err := tmpl.Execute(buf, iriRenderConfig{
-		DockerRegistryImage: controllerConfig.Spec.Images[templatectrl.DockerRegistryKey],
-	}); err != nil {
-		return nil, fmt.Errorf("failed to execute template: %w", err)
-	}
-
-	// Generate the iri ignition
-	ignCfg, err := ctrlcommon.TranspileCoreOSConfigToIgn(nil, []string{buf.String()})
-	if err != nil {
-		return nil, fmt.Errorf("error transpiling CoreOS config to Ignition config: %w", err)
-	}
-	return ignCfg, nil
-}