HIVE-2859: Add ManifestsSecretRef to ClusterPool for manifest management.

Add support for user-provided manifests in ClusterPool to apply custom
manifests to all clusters created from the pool, similar to ClusterDeployment.

Author: eliorerz

apis/hive/v1/clusterpool_types.go

@@ -109,6 +109,11 @@ type ClusterPoolSpec struct {
 	// ClusterDeployment generated by the ClusterPool.
 	// +optional
 	CustomizationRef *corev1.LocalObjectReference `json:"customizationRef,omitempty"`
+
+	// ManifestsSecretRef is a reference to user-provided manifests to add to or replace manifests
+	// that are generated by the installer for all clusters created by this pool.
+	// +optional
+	ManifestsSecretRef *corev1.LocalObjectReference `json:"manifestsSecretRef,omitempty"`
 }
 
 type HibernationConfig struct {

apis/hive/v1/zz_generated.deepcopy.go

@@ -313,6 +313,40 @@ spec:
                     Labels to be applied to new ClusterDeployments created for the pool. ClusterDeployments that have already been
                     claimed will not be affected when this value is modified.
                   type: object
+                manifestsConfigMapRef:
+                  description: |-
+                    ManifestsConfigMapRef is a reference to user-provided manifests to add to or replace manifests
+                    that are generated by the installer for all clusters created by this pool. It serves the same purpose as, and is mutually exclusive
+                    with, ManifestsSecretRef.
+                  properties:
+                    name:
+                      default: ""
+                      description: |-
+                        Name of the referent.
+                        This field is effectively required, but due to backwards compatibility is
+                        allowed to be empty. Instances of this type with an empty value here are
+                        almost certainly wrong.
+                        More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                      type: string
+                  type: object
+                  x-kubernetes-map-type: atomic
+                manifestsSecretRef:
+                  description: |-
+                    ManifestsSecretRef is a reference to user-provided manifests to add to or replace manifests
+                    that are generated by the installer for all clusters created by this pool. It serves the same purpose as, and is mutually exclusive
+                    with, ManifestsConfigMapRef.
+                  properties:
+                    name:
+                      default: ""
+                      description: |-
+                        Name of the referent.
+                        This field is effectively required, but due to backwards compatibility is
+                        allowed to be empty. Instances of this type with an empty value here are
+                        almost certainly wrong.
+                        More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                      type: string
+                  type: object
+                  x-kubernetes-map-type: atomic
                 maxConcurrent:
                   description: |-
                     MaxConcurrent is the maximum number of clusters that will be provisioned or deprovisioned at an time. This includes the

config/crds/hive.openshift.io_clusterpools.yaml

@@ -695,6 +695,15 @@ func (r *ReconcileClusterPool) addClusters(
 		errs = append(errs, fmt.Errorf("%s: %w", credentialsSecretDependent, err))
 	}
 
+	// Get manifests if specified
+	manifests, err := r.getManifests(clp, logger)
+	if err != nil {
+		errs = append(errs, fmt.Errorf("error getting manifests: %w", err))
+	}
+	if manifests != nil {
+		logger.WithField("manifestCount", len(manifests)).Info("retrieved manifests for cluster pool")
+	}
+
 	if clp.Spec.CustomizationRef != nil && clp.Spec.CustomizationRef.Name != "" {
 		custCDC := clp.Spec.CustomizationRef.Name
 		if cdcs.nonInventory == nil || cdcs.nonInventory.Name != custCDC {
@@ -713,7 +722,7 @@ func (r *ReconcileClusterPool) addClusters(
 	}
 
 	for i := 0; i < newClusterCount; i++ {
-		cd, err := r.createCluster(clp, cloudBuilder, pullSecret, installConfigTemplate, poolVersion, cdcs, logger)
+		cd, err := r.createCluster(clp, cloudBuilder, pullSecret, installConfigTemplate, poolVersion, cdcs, manifests, logger)
 		if err != nil {
 			return err
 		}
@@ -730,6 +739,7 @@ func (r *ReconcileClusterPool) createCluster(
 	installConfigTemplate string,
 	poolVersion string,
 	cdcs *cdcCollection,
+	manifests map[string][]byte,
 	logger log.FieldLogger,
 ) (*hivev1.ClusterDeployment, error) {
 	var err error
@@ -778,6 +788,12 @@ func (r *ReconcileClusterPool) createCluster(
 		builder.HibernateAfter = &clp.Spec.HibernateAfter.Duration
 	}
 
+	// Set manifests if provided
+	if manifests != nil {
+		logger.WithField("manifestCount", len(manifests)).Info("applying manifests to cluster deployment")
+		builder.InstallerManifests = manifests
+	}
+
 	objs, err := builder.Build()
 	if err != nil {
 		return nil, errors.Wrap(err, "error building resources")
@@ -1259,6 +1275,28 @@ func (r *ReconcileClusterPool) getPullSecret(pool *hivev1.ClusterPool, logger lo
 	return string(pullSecret), nil
 }
 
+func (r *ReconcileClusterPool) getManifests(pool *hivev1.ClusterPool, logger log.FieldLogger) (map[string][]byte, error) {
+	if pool.Spec.ManifestsSecretRef == nil {
+		logger.Debug("no manifests secret reference specified")
+		return nil, nil
+	}
+
+	logger.WithField("secretName", pool.Spec.ManifestsSecretRef.Name).Info("retrieving manifests from secret")
+	manifestsSecret := &corev1.Secret{}
+	err := r.Client.Get(
+		context.Background(),
+		types.NamespacedName{Namespace: pool.Namespace, Name: pool.Spec.ManifestsSecretRef.Name},
+		manifestsSecret,
+	)
+	if err != nil {
+		logger.WithError(err).Log(controllerutils.LogLevel(err), "error reading manifests secret")
+		return nil, err
+	}
+
+	logger.WithField("manifestCount", len(manifestsSecret.Data)).Info("successfully retrieved manifests from secret")
+	return manifestsSecret.Data, nil
+}
+
 func (r *ReconcileClusterPool) createCloudBuilder(pool *hivev1.ClusterPool, logger log.FieldLogger) (clusterresource.CloudBuilder, error) {
 	switch platform := pool.Spec.Platform; {
 	case platform.AWS != nil:

pkg/awsclient/mock/client_generated.go

@@ -3263,3 +3263,153 @@ func Test_isBroken(t *testing.T) {
 		})
 	}
 }
+
+func TestGetManifests(t *testing.T) {
+	scheme := scheme.GetScheme()
+	logger := log.NewEntry(log.New())
+
+	tests := []struct {
+		name        string
+		pool        *hivev1.ClusterPool
+		existing    []runtime.Object
+		expected    map[string][]byte
+		expectError bool
+	}{
+		{
+			name: "no manifests specified",
+			pool: testcp.FullBuilder(testNamespace, "test-pool", scheme).
+				Options(
+					testcp.ForAWS(credsSecretName, "us-east-1"),
+					testcp.WithBaseDomain("test-domain"),
+					testcp.WithImageSet(imageSetName),
+				).Build(),
+			existing:    []runtime.Object{},
+			expected:    nil,
+			expectError: false,
+		},
+		{
+			name: "manifests secret exists",
+			pool: testcp.FullBuilder(testNamespace, "test-pool", scheme).
+				Options(
+					testcp.ForAWS(credsSecretName, "us-east-1"),
+					testcp.WithBaseDomain("test-domain"),
+					testcp.WithImageSet(imageSetName),
+					testcp.WithManifestsSecretRef("test-manifests-secret"),
+				).Build(),
+			existing: []runtime.Object{
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-manifests-secret",
+						Namespace: testNamespace,
+					},
+					Data: map[string][]byte{
+						"manifest1.yaml": []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: test"),
+						"manifest2.yaml": []byte("apiVersion: v1\nkind: Secret\nmetadata:\n  name: test"),
+					},
+				},
+			},
+			expected: map[string][]byte{
+				"manifest1.yaml": []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: test"),
+				"manifest2.yaml": []byte("apiVersion: v1\nkind: Secret\nmetadata:\n  name: test"),
+			},
+			expectError: false,
+		},
+		{
+			name: "manifests secret not found",
+			pool: testcp.FullBuilder(testNamespace, "test-pool", scheme).
+				Options(
+					testcp.ForAWS(credsSecretName, "us-east-1"),
+					testcp.WithBaseDomain("test-domain"),
+					testcp.WithImageSet(imageSetName),
+					testcp.WithManifestsSecretRef("missing-secret"),
+				).Build(),
+			existing:    []runtime.Object{},
+			expected:    nil,
+			expectError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := testfake.NewFakeClientBuilder().WithRuntimeObjects(tt.existing...).Build()
+			r := &ReconcileClusterPool{
+				Client: c,
+			}
+
+			result, err := r.getManifests(tt.pool, logger)
+
+			if tt.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+
+func TestClusterPoolWithManifests(t *testing.T) {
+	scheme := scheme.GetScheme()
+	logger := log.NewEntry(log.New())
+
+	poolBuilder := testcp.FullBuilder(testNamespace, testLeasePoolName, scheme).
+		GenericOptions(
+			testgeneric.WithFinalizer(finalizer),
+		).
+		Options(
+			testcp.ForAWS(credsSecretName, "us-east-1"),
+			testcp.WithBaseDomain("test-domain"),
+			testcp.WithImageSet(imageSetName),
+		)
+
+	tests := []struct {
+		name                  string
+		pool                  *hivev1.ClusterPool
+		existing              []runtime.Object
+		expectedTotalClusters int
+		expectedManifests     map[string][]byte
+		expectError           bool
+	}{
+		{
+			name: "clusterpool with manifests secret",
+			pool: poolBuilder.Build(
+				testcp.WithSize(1),
+				testcp.WithManifestsSecretRef("test-manifests-secret"),
+			),
+			existing: []runtime.Object{
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-manifests-secret",
+						Namespace: testNamespace,
+					},
+					Data: map[string][]byte{
+						"manifest1.yaml": []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: test"),
+					},
+				},
+			},
+			expectedManifests: map[string][]byte{
+				"manifest1.yaml": []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: test"),
+			},
+			expectError: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := testfake.NewFakeClientBuilder().WithRuntimeObjects(tt.existing...).Build()
+			r := &ReconcileClusterPool{
+				Client: c,
+			}
+
+			// Test the getManifests function directly
+			result, err := r.getManifests(tt.pool, logger)
+
+			if tt.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expectedManifests, result)
+			}
+		})
+	}
+}

pkg/controller/clusterpool/clusterpool_controller.go

@@ -222,3 +222,11 @@ func WithCustomizationRef(cdcName string) Option {
 		}
 	}
 }
+
+func WithManifestsSecretRef(secretName string) Option {
+	return func(clusterPool *hivev1.ClusterPool) {
+		clusterPool.Spec.ManifestsSecretRef = &corev1.LocalObjectReference{
+			Name: secretName,
+		}
+	}
+}

pkg/controller/clusterpool/clusterpool_controller_test.go

@@ -160,6 +160,11 @@ func (a *ClusterPoolValidatingAdmissionHook) validateCreate(admissionSpec *admis
 	// Add the new data to the contextLogger
 	contextLogger.Data["object.Name"] = newObject.Name
 
+	// Log if manifests are being validated
+	if newObject.Spec.ManifestsSecretRef != nil {
+		contextLogger.WithField("manifestsSecretRef", newObject.Spec.ManifestsSecretRef.Name).Info("validating cluster pool with manifests secret reference")
+	}
+
 	// TODO: Put Create Validation Here (or in openAPIV3Schema validation section of crd)
 
 	if len(newObject.Name) > validation.DNS1123LabelMaxLength {
@@ -234,6 +239,11 @@ func (a *ClusterPoolValidatingAdmissionHook) validateUpdate(admissionSpec *admis
 	// Add the new data to the contextLogger
 	contextLogger.Data["oldObject.Name"] = oldObject.Name
 
+	// Log if manifests are being validated
+	if newObject.Spec.ManifestsSecretRef != nil {
+		contextLogger.WithField("manifestsSecretRef", newObject.Spec.ManifestsSecretRef.Name).Info("validating cluster pool update with manifests secret reference")
+	}
+
 	allErrs := field.ErrorList{}
 	specPath := field.NewPath("spec")