⚠️ Add default values for the 'cloudName' and 'identityRef' fields (#101

) * Add default values for the 'cloudName' and 'identityRef' fields Signed-off-by: michal.gubricky <michal.gubricky@dnation.cloud> * Catch unknown error from the getCloudNameFromSecret function Signed-off-by: michal.gubricky <michal.gubricky@dnation.cloud> * Change the severity of unknown error to warning Signed-off-by: michal.gubricky <michal.gubricky@dnation.cloud> * Fix yaml lint warnings Signed-off-by: michal.gubricky <michal.gubricky@dnation.cloud> * Update config/cspo/secret.yaml Co-authored-by: Matej Feder <matej.feder@dnation.cloud> Signed-off-by: Michal Gubricky <michal.gubricky@dnation.cloud> * Replace <my-cloud-secret> with openstack in docs Signed-off-by: michal.gubricky <michal.gubricky@dnation.cloud> * Get rid off the spec.CloudName field from the OpenStackNodeImageRelease resource Signed-off-by: michal.gubricky <michal.gubricky@dnation.cloud> * Move defaultCloudName into const Signed-off-by: michal.gubricky <michal.gubricky@dnation.cloud> --------- Signed-off-by: michal.gubricky <michal.gubricky@dnation.cloud> Signed-off-by: Michal Gubricky <michal.gubricky@dnation.cloud> Co-authored-by: Matej Feder <matej.feder@dnation.cloud>
SovereignCloudStack · Feb 27, 2024 · 1d01a75 · 1d01a75
1 parent 731ffe1
commit 1d01a75
Show file tree

Hide file tree

Showing 16 changed files with 86 additions and 61 deletions.
diff --git a/README.md b/README.md
@@ -46,12 +46,15 @@ clusterctl init --infrastructure openstack
 To enable communication between the CSPO and the Cluster API Provider for OpenStack (CAPO) with the OpenStack API, it is necessary to generate a secret containing the access data (clouds.yaml).
 Ensure that this secret is located in the identical namespace as the other Custom Resources.
 
+> [!NOTE]  
+> The default value of `cloudName` is configured as `openstack`. This setting can be overridden by including the `cloudName` key in the secret. Also, be aware that the name of the secret is expected to be `openstack` unless it is not set differently in OpenStackClusterStackReleaseTemplate in `identityRef.name` field.
+
 ```bash
-kubectl create secret generic <my-cloud-secret> --from-file=clouds.yaml=path/to/clouds.yaml
+kubectl create secret generic openstack --from-file=clouds.yaml=path/to/clouds.yaml
 
 # Patch the created secrets so they are automatically moved to the target cluster later.
 
-kubectl patch secret <my-cloud-secret> -p '{"metadata":{"labels":{"clusterctl.cluster.x-k8s.io/move":""}}}'
+kubectl patch secret openstack -p '{"metadata":{"labels":{"clusterctl.cluster.x-k8s.io/move":""}}}'
 ```
 
 ### CSO and CSPO variables preparation

diff --git a/Tiltfile b/Tiltfile
@@ -268,7 +268,7 @@ def deploy_cspo():
 
 def create_secret():
     cmd = "cat .secret.yaml | {} | kubectl apply -f -".format(envsubst_cmd)
-    local_resource('supersecret', cmd, labels=["clouds-yaml-secret"])    
+    local_resource('supersecret', cmd, labels=["clouds-yaml-secret"])
 
 def cspo_template():
     cmd = "cat .cspotemplate.yaml | {}".format(envsubst_cmd)

diff --git a/api/v1alpha1/openstackclusterstackrelease_types.go b/api/v1alpha1/openstackclusterstackrelease_types.go
@@ -27,10 +27,9 @@ import (
 
 // OpenStackClusterStackReleaseSpec defines the desired state of OpenStackClusterStackRelease.
 type OpenStackClusterStackReleaseSpec struct {
-	// CloudName is the name of the cloud to use from the cloud's secret.
-	// +kubebuilder:validation:MinLength=1
-	CloudName string `json:"cloudName"`
 	// IdentityRef is a reference to a identity to be used when reconciling this cluster
+	// +optional
+	// +kubebuilder:default:={kind: "Secret", name: "openstack"}
 	IdentityRef *apiv1alpha7.OpenStackIdentityReference `json:"identityRef"`
 }
 

diff --git a/api/v1alpha1/openstacknodeimagerelease_types.go b/api/v1alpha1/openstacknodeimagerelease_types.go
@@ -28,9 +28,6 @@ import (
 
 // OpenStackNodeImageReleaseSpec defines the desired state of OpenStackNodeImageRelease.
 type OpenStackNodeImageReleaseSpec struct {
-	// CloudName is the name of the cloud to use from the cloud's secret.
-	// +kubebuilder:validation:MinLength=1
-	CloudName string `json:"cloudName"`
 	// IdentityRef is a reference to a identity to be used when reconciling this cluster
 	IdentityRef *apiv1alpha7.OpenStackIdentityReference `json:"identityRef"`
 	// Image represents options used to upload an image

diff --git a/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleases.yaml b/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleases.yaml
@@ -52,12 +52,10 @@ spec:
             description: OpenStackClusterStackReleaseSpec defines the desired state
               of OpenStackClusterStackRelease.
             properties:
-              cloudName:
-                description: CloudName is the name of the cloud to use from the cloud's
-                  secret.
-                minLength: 1
-                type: string
               identityRef:
+                default:
+                  kind: Secret
+                  name: openstack
                 description: IdentityRef is a reference to a identity to be used when
                   reconciling this cluster
                 properties:
@@ -75,9 +73,6 @@ spec:
                 - kind
                 - name
                 type: object
-            required:
-            - cloudName
-            - identityRef
             type: object
           status:
             description: OpenStackClusterStackReleaseStatus defines the observed state

diff --git a/...crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleasetemplates.yaml b/...crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleasetemplates.yaml
@@ -47,12 +47,10 @@ spec:
                     description: OpenStackClusterStackReleaseSpec defines the desired
                       state of OpenStackClusterStackRelease.
                     properties:
-                      cloudName:
-                        description: CloudName is the name of the cloud to use from
-                          the cloud's secret.
-                        minLength: 1
-                        type: string
                       identityRef:
+                        default:
+                          kind: Secret
+                          name: openstack
                         description: IdentityRef is a reference to a identity to be
                           used when reconciling this cluster
                         properties:
@@ -72,9 +70,6 @@ spec:
                         - kind
                         - name
                         type: object
-                    required:
-                    - cloudName
-                    - identityRef
                     type: object
                 required:
                 - spec

diff --git a/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstacknodeimagereleases.yaml b/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstacknodeimagereleases.yaml
@@ -52,11 +52,6 @@ spec:
             description: OpenStackNodeImageReleaseSpec defines the desired state of
               OpenStackNodeImageRelease.
             properties:
-              cloudName:
-                description: CloudName is the name of the cloud to use from the cloud's
-                  secret.
-                minLength: 1
-                type: string
               identityRef:
                 description: IdentityRef is a reference to a identity to be used when
                   reconciling this cluster
@@ -129,7 +124,6 @@ spec:
                 - url
                 type: object
             required:
-            - cloudName
             - identityRef
             - image
             type: object

diff --git a/config/cspo/cspotemplate.yaml b/config/cspo/cspotemplate.yaml
@@ -5,8 +5,9 @@ metadata:
   namespace: cluster
 spec:
   template:
-    spec:
-      cloudName: "${CLOUD_NAME}"
-      identityRef:
-        kind: Secret
-        name: "${SECRET_NAME}"
+    spec: {}
+    # Field identityRef is optional and its default values are as follows:
+    # identityRef.kind: "Secret", identityRef.name: "openstack"
+    # identityRef:
+    #   kind: Secret
+    #   name: "<your-secret-name>"
diff --git a/config/cspo/secret.yaml b/config/cspo/secret.yaml
@@ -1,9 +1,14 @@
 apiVersion: v1
 data:
+  # The default value of `cloudName` is configured as `openstack`.
+  # This can be overridden by including the `cloudName` key in this secret.
+  # cloudName: "openstack"
   clouds.yaml: ${ENCODED_CLOUDS_YAML}
 kind: Secret
 metadata:
   labels:
     clusterctl.cluster.x-k8s.io/move: "true"
-  name: "${SECRET_NAME}"
+  # Note: `metadata.name` must be the same as the value of the field
+  # `identityRef.name` in OpenStackClusterStackReleaseTemplate object.
+  name: "openstack"
   namespace: cluster
diff --git a/examples/cspotemplate.yaml b/examples/cspotemplate.yaml
@@ -5,7 +5,8 @@ metadata:
 spec:
   template:
     spec:
-      cloudName: <name of the cloud to use from the clouds secret>
+      # Field identityRef is optional and its default values are as follows:
+      # identityRef.kind: "Secret", identityRef.name: "openstack"
       identityRef:
         kind: Secret
         name: <my-secret>
diff --git a/internal/controller/openstackclusterstackrelease_controller.go b/internal/controller/openstackclusterstackrelease_controller.go
@@ -250,7 +250,6 @@ func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImag
 	}
 	openStackNodeImageRelease.SetOwnerReferences([]metav1.OwnerReference{*ownerRef})
 	openStackNodeImageRelease.Spec.Image = openStackNodeImage
-	openStackNodeImageRelease.Spec.CloudName = openstackclusterstackrelease.Spec.CloudName
 	openStackNodeImageRelease.Spec.IdentityRef = openstackclusterstackrelease.Spec.IdentityRef
 
 	if err := r.Create(ctx, openStackNodeImageRelease); err != nil {

diff --git a/internal/controller/openstackclusterstackrelease_controller_test.go b/internal/controller/openstackclusterstackrelease_controller_test.go
@@ -74,7 +74,6 @@ func TestGenerateOwnerReference(t *testing.T) {
 			UID:       "fb686e33-01a6-42c9-a210-2c26ec8cb331",
 		},
 		Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
-			CloudName: "openstack",
 			IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
 				Kind: "Secret",
 				Name: "supersecret",
@@ -103,7 +102,6 @@ func TestMatchOwnerReference(t *testing.T) {
 			Name: "openstack-ferrol-1-27-v1",
 		},
 		Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
-			CloudName: "openstack",
 			IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
 				Kind: "Secret",
 				Name: "supersecret1",
@@ -122,7 +120,6 @@ func TestMatchOwnerReference(t *testing.T) {
 			Name: "openstack-ferrol-1-27-v2",
 		},
 		Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
-			CloudName: "openstack",
 			IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
 				Kind: "Secret",
 				Name: "supersecret2",
@@ -245,7 +242,6 @@ func TestGetOwnedOpenStackNodeImageReleases(t *testing.T) {
 			Namespace: "test-namespace",
 		},
 		Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
-			CloudName: "test-cloudname",
 			IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
 				Kind: "Secret",
 				Name: "supersecret",
@@ -290,6 +286,8 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) {
 	scheme := runtime.NewScheme()
 	err := apiv1alpha1.AddToScheme(scheme)
 	assert.NoError(t, err)
+	err = corev1.AddToScheme(scheme)
+	assert.NoError(t, err)
 	client := fake.NewClientBuilder().WithScheme(scheme).Build()
 
 	openstackclusterstackrelease := &apiv1alpha1.OpenStackClusterStackRelease{
@@ -302,7 +300,6 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) {
 			Namespace: "test-namespace",
 		},
 		Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
-			CloudName: "test-cloudname",
 			IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
 				Kind: "Secret",
 				Name: "supersecret",
@@ -328,10 +325,24 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) {
 		UID:        openstackclusterstackrelease.UID,
 	}
 
+	secretName := "supersecret"
+	secretNamespace := "test-namespace"
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      secretName,
+			Namespace: secretNamespace,
+		},
+		Type: corev1.SecretTypeOpaque,
+	}
+	err = client.Create(context.TODO(), secret)
+	assert.NoError(t, err)
+
 	r := &OpenStackClusterStackReleaseReconciler{
 		Client: client,
 	}
 
+	assert.NoError(t, err)
+
 	err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-osnir", openStackNodeImage, ownerRef)
 
 	assert.NoError(t, err)
@@ -346,7 +357,6 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) {
 			APIVersion: apiv1alpha1.GroupVersion.String(),
 		},
 		Spec: apiv1alpha1.OpenStackNodeImageReleaseSpec{
-			CloudName: "test-cloudname",
 			IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
 				Kind: "Secret",
 				Name: "supersecret",
@@ -388,7 +398,6 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) {
 			Namespace: "test-namespace",
 		},
 		Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
-			CloudName: "test-cloudname",
 			IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
 				Kind: "Secret",
 				Name: "supersecret",
@@ -428,7 +437,6 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) {
 			Namespace: "test-namespace",
 		},
 		Spec: apiv1alpha1.OpenStackNodeImageReleaseSpec{
-			CloudName: "test-cloudname",
 			IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
 				Kind: "Secret",
 				Name: "supersecret",
@@ -510,7 +518,6 @@ var _ = Describe("OpenStackClusterStackRelease controller", func() {
 						Namespace: namespace.Name,
 					},
 					Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{
-						CloudName: "openstack",
 						IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{
 							Kind: "Secret",
 							Name: "supersecret",

diff --git a/internal/controller/openstacknodeimagerelease_controller.go b/internal/controller/openstacknodeimagerelease_controller.go
@@ -50,6 +50,8 @@ type OpenStackNodeImageReleaseReconciler struct {
 }
 
 const (
+	defaultCloudName         = "openstack"
+	cloudNameSecretKey       = "cloudName"
 	cloudsSecretKey          = "clouds.yaml"
 	waitForImageBecomeActive = 30 * time.Second
 )
@@ -95,7 +97,7 @@ func (r *OpenStackNodeImageReleaseReconciler) Reconcile(ctx context.Context, req
 	}()
 
 	// Get OpenStack cloud config from sercet
-	cloud, err := r.getCloudFromSecret(ctx, openstacknodeimagerelease.Namespace, openstacknodeimagerelease.Spec.IdentityRef.Name, openstacknodeimagerelease.Spec.CloudName)
+	cloud, err := r.getCloudFromSecret(ctx, openstacknodeimagerelease.Namespace, openstacknodeimagerelease.Spec.IdentityRef.Name)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			conditions.MarkFalse(openstacknodeimagerelease,
@@ -291,9 +293,10 @@ func (r *OpenStackNodeImageReleaseReconciler) Reconcile(ctx context.Context, req
 	return ctrl.Result{}, nil
 }
 
-func (r *OpenStackNodeImageReleaseReconciler) getCloudFromSecret(ctx context.Context, secretNamespace, secretName, cloudName string) (clientconfig.Cloud, error) {
+func (r *OpenStackNodeImageReleaseReconciler) getCloudFromSecret(ctx context.Context, secretNamespace, secretName string) (clientconfig.Cloud, error) {
 	var clouds clientconfig.Clouds
 	emptyCloud := clientconfig.Cloud{}
+	var cloudName string
 
 	secret := &corev1.Secret{}
 	err := r.Get(ctx, types.NamespacedName{
@@ -303,7 +306,17 @@ func (r *OpenStackNodeImageReleaseReconciler) getCloudFromSecret(ctx context.Con
 	if err != nil {
 		return emptyCloud, fmt.Errorf("failed to get secret %s in namespace %s: %w", secretName, secretNamespace, err)
 	}
-	content, ok := secret.Data[cloudsSecretKey]
+
+	content, ok := secret.Data[cloudNameSecretKey]
+	if !ok {
+		cloudName = defaultCloudName
+	} else {
+		if err := yaml.Unmarshal(content, &cloudName); err != nil {
+			return emptyCloud, fmt.Errorf("failed to unmarshal cloudName stored in secret %s: %w", secretName, err)
+		}
+	}
+
+	content, ok = secret.Data[cloudsSecretKey]
 	if !ok {
 		return emptyCloud, fmt.Errorf("OpenStack credentials secret %s did not contain key %s", secretName, cloudsSecretKey)
 	}

diff --git a/internal/controller/openstacknodeimagerelease_controller_test.go b/internal/controller/openstacknodeimagerelease_controller_test.go
@@ -40,7 +40,6 @@ func TestGetCloudFromSecret(t *testing.T) {
 
 	secretName := "test-secret"
 	secretNamespace := "test-namespace"
-	cloudName := "openstack"
 	cloudsYAML := `
 clouds:
   openstack:
@@ -69,7 +68,7 @@ clouds:
 		Client: client,
 	}
 
-	cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName, cloudName)
+	cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName)
 
 	expectedCloud := clientconfig.Cloud{
 		AuthInfo: &clientconfig.AuthInfo{
@@ -99,9 +98,8 @@ func TestGetCloudFromSecretNotFound(t *testing.T) {
 	secretName := "nonexistent-secret"
 	secretNamespace := "nonexistent-namespace"
 	expectedError := "secrets \"nonexistent-secret\" not found"
-	cloudName := "nonexistent-cloud"
 
-	cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName, cloudName)
+	cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName)
 
 	expectedErrorMessage := fmt.Sprintf("failed to get secret %s in namespace %s: %v", secretName, secretNamespace, expectedError)
 
@@ -120,7 +118,6 @@ func TestGetCloudFromSecretMissingCloudsSecretKey(t *testing.T) {
 
 	secretName := "test-secret"
 	secretNamespace := "test-namespace"
-	cloudName := "openstack"
 
 	// Create a secret with the bad cloudsSecretKey.
 	secret := &corev1.Secret{
@@ -134,7 +131,7 @@ func TestGetCloudFromSecretMissingCloudsSecretKey(t *testing.T) {
 	err := client.Create(context.TODO(), secret)
 	assert.NoError(t, err)
 
-	cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName, cloudName)
+	cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName)
 
 	assert.Error(t, err)
 	assert.EqualError(t, err, fmt.Sprintf("OpenStack credentials secret %s did not contain key %s", secretName, cloudsSecretKey))
@@ -172,13 +169,16 @@ clouds:
 			Name:      secretName,
 			Namespace: secretNamespace,
 		},
-		Data: map[string][]byte{cloudsSecretKey: []byte(cloudsYAML)},
+		Data: map[string][]byte{
+			cloudsSecretKey:    []byte(cloudsYAML),
+			cloudNameSecretKey: []byte(cloudName),
+		},
 		Type: corev1.SecretTypeOpaque,
 	}
 	err := client.Create(context.TODO(), secret)
 	assert.NoError(t, err)
 
-	cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName, cloudName)
+	cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName)
 
 	assert.Error(t, err)
 	assert.EqualError(t, err, fmt.Sprintf("failed to find cloud %s in %s", cloudName, cloudsSecretKey))