[ACM-10996] Fixed credential config for auto-import secret (#239)

* fixed credential config for auto-import secret Signed-off-by: Disaiah Bennett <[email protected]> * removed unused discoveryconfig variable Signed-off-by: Disaiah Bennett <[email protected]> --------- Signed-off-by: Disaiah Bennett <[email protected]>
stolostron · Apr 24, 2024 · 5d100eb · 5d100eb
1 parent 5955db4
commit 5d100eb
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 25 deletions.
diff --git a/controllers/discoveredcluster_controller.go b/controllers/discoveredcluster_controller.go
@@ -84,12 +84,6 @@ func (r *DiscoveredClusterReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		dc.Annotations = make(map[string]string)
 	}
 
-	config := &discovery.DiscoveryConfig{}
-	if err := r.Get(ctx, GetDiscoveryConfig(), config); err != nil {
-		logf.Error(err, "failed to get DiscoveryConfig", "Name", GetDiscoveryConfig().Name)
-		return ctrl.Result{RequeueAfter: recon.WarningRefreshInterval}, err
-	}
-
 	/*
 		If the discovered cluster has an Automatic import strategy, we need to ensure that the required resources
 		are available. Otherwise, we will ignore that cluster.
@@ -121,7 +115,14 @@ func (r *DiscoveredClusterReconciler) Reconcile(ctx context.Context, req ctrl.Re
 				}
 			}
 
-			if res, err := r.EnsureAutoImportSecret(ctx, *dc, *config); err != nil {
+			// Ensure that the DiscoveredCluster credentials are available on the cluster.
+			if res, err := r.EnsureDiscoveredClusterCredentialExists(ctx, *dc); err != nil {
+				logf.Error(err, "failed to ensure DiscoveredCluster credential Secret exist", "Name",
+					dc.Spec.DisplayName)
+				return res, err
+			}
+
+			if res, err := r.EnsureAutoImportSecret(ctx, *dc); err != nil {
 				logf.Error(err, "failed to ensure auto import Secret created", "Name", dc.Spec.DisplayName)
 				return res, err
 			}
@@ -252,9 +253,9 @@ namespace. It sets a controller reference to the DiscoveredCluster for ownership
 If creation fails, it logs an error and returns with a requeue signal. If the auto-import secret already exists or if
 an error occurs during retrieval, it logs an error and returns with a requeue signal.
 */
-func (r *DiscoveredClusterReconciler) EnsureAutoImportSecret(ctx context.Context, dc discovery.DiscoveredCluster,
-	config discovery.DiscoveryConfig) (ctrl.Result, error) {
-	nn := types.NamespacedName{Name: config.Spec.Credential, Namespace: config.GetNamespace()}
+func (r *DiscoveredClusterReconciler) EnsureAutoImportSecret(ctx context.Context, dc discovery.DiscoveredCluster) (
+	ctrl.Result, error) {
+	nn := types.NamespacedName{Name: dc.Spec.Credential.Name, Namespace: dc.Spec.Credential.Namespace}
 	existingSecret := corev1.Secret{}
 
 	if err := r.Get(ctx, nn, &existingSecret); apierrors.IsNotFound(err) {
@@ -291,6 +292,24 @@ func (r *DiscoveredClusterReconciler) EnsureAutoImportSecret(ctx context.Context
 	return ctrl.Result{}, nil
 }
 
+// EnsureDiscoveredClusterCredentialExists ...
+func (r *DiscoveredClusterReconciler) EnsureDiscoveredClusterCredentialExists(
+	ctx context.Context, dc discovery.DiscoveredCluster) (ctrl.Result, error) {
+	nn := types.NamespacedName{Name: dc.Spec.Credential.Name, Namespace: dc.Spec.Credential.Namespace}
+	secret := corev1.Secret{}
+
+	if err := r.Get(ctx, nn, &secret); apierrors.IsNotFound(err) {
+		logf.Error(err, "Secret was not found", "Name", nn.Name, "Namespace", nn.Namespace)
+		return ctrl.Result{RequeueAfter: recon.ShortRefreshInterval}, err
+
+	} else if err != nil {
+		logf.Error(err, "failed to get Secret", "Name", nn.Name, "Namespace", nn.Namespace)
+		return ctrl.Result{RequeueAfter: recon.WarningRefreshInterval}, err
+	}
+
+	return ctrl.Result{}, nil
+}
+
 // EnsureKlusterletAddonConfig ensures the existence of a KlusterletAddonConfig resource for the given
 // DiscoveredCluster. It checks if a KlusterletAddonConfig with the specified display name exists.
 // If not found, it creates a new KlusterletAddonConfig with the display name and default configurations.

diff --git a/controllers/discoveredcluster_controller_test.go b/controllers/discoveredcluster_controller_test.go
@@ -75,6 +75,10 @@ func Test_DiscoveredCluster_Reconciler_Reconcile(t *testing.T) {
 					Namespace: "discovery",
 				},
 				Spec: discovery.DiscoveredClusterSpec{
+					Credential: corev1.ObjectReference{
+						Name:      "fake-admin",
+						Namespace: "discovery",
+					},
 					DisplayName:            "fake-cluster",
 					ImportAsManagedCluster: true,
 					RHOCMClusterID:         "349bcdc1dd6a44f3a1a136b2f98a69ca",
@@ -112,10 +116,6 @@ func Test_DiscoveredCluster_Reconciler_Reconcile(t *testing.T) {
 	registerScheme()
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			DiscoveryConfig = types.NamespacedName{
-				Name: tt.config.Name, Namespace: tt.config.Namespace,
-			}
-
 			ns := &corev1.Namespace{}
 			mc := &clusterapiv1.ManagedCluster{}
 			kac := &agentv1.KlusterletAddonConfig{}
@@ -352,7 +352,11 @@ func Test_Reconciler_EnsureAutoImportSecret(t *testing.T) {
 				},
 				Spec: discovery.DiscoveredClusterSpec{
 					DisplayName: "foo",
-					Type:        "ROSA",
+					Credential: corev1.ObjectReference{
+						Name:      "admin",
+						Namespace: "discovery",
+					},
+					Type: "ROSA",
 				},
 			},
 			want: true,
@@ -389,7 +393,7 @@ func Test_Reconciler_EnsureAutoImportSecret(t *testing.T) {
 				t.Errorf("failed to create Secret: %v", err)
 			}
 
-			if _, err := r.EnsureAutoImportSecret(context.TODO(), *tt.dc, *tt.config); err != nil {
+			if _, err := r.EnsureAutoImportSecret(context.TODO(), *tt.dc); err != nil {
 				t.Errorf("failed to ensure auto import Secret created: %v", err)
 			}
 

diff --git a/controllers/discoveryconfig_controller.go b/controllers/discoveryconfig_controller.go
@@ -49,7 +49,6 @@ var (
 	// baseURLAnnotation is the annotation set in a DiscoveryConfig that overrides the URL base used to find clusters
 	baseURLAnnotation     = "ocmBaseURL"
 	baseAuthURLAnnotation = "authBaseURL"
-	DiscoveryConfig       types.NamespacedName
 )
 
 var ErrBadFormat = errors.New("bad format")
@@ -96,10 +95,6 @@ func (r *DiscoveryConfigReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		// If there's an error other than "Not Found", return with the error.
 		return ctrl.Result{}, fmt.Errorf("failed to get DiscoveryConfig %s: %w", req.Name, err)
 	}
-	DiscoveryConfig = types.NamespacedName{
-		Name:      config.GetName(),
-		Namespace: config.GetNamespace(),
-	}
 
 	if err = r.updateDiscoveredClusters(ctx, config); err != nil {
 		logf.Error(err, "Error updating DiscoveredClusters")
@@ -377,7 +372,3 @@ func getAuthURLOverride(config *discovery.DiscoveryConfig) string {
 	}
 	return ""
 }
-
-func GetDiscoveryConfig() types.NamespacedName {
-	return DiscoveryConfig
-}