Cleanup and fixup of tests.

Joshua Reed · Joshua Reed · commit b7a45d12bf74 · 2022-07-28T10:00:52.000-07:00
diff --git a/api/v1beta1/cloudstackcluster_types.go b/api/v1beta1/cloudstackcluster_types.go
@@ -25,7 +25,6 @@ import (
 
 const (
 	ClusterFinalizer           = "cloudstackcluster.infrastructure.cluster.x-k8s.io"
-	defaultIdentityRefKind     = "Secret"
 	CloudStackClusterLabelName = "cloudstackcluster.infrastructure.cluster.x-k8s.io/name"
 	NetworkTypeIsolated        = "Isolated"
 	NetworkTypeShared          = "Shared"
diff --git a/api/v1beta2/cloudstackcluster_webhook_test.go b/api/v1beta2/cloudstackcluster_webhook_test.go
@@ -54,11 +54,6 @@ var _ = Describe("CloudStackCluster webhooks", func() {
 			Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(requiredRegex,
 				"each Zone requires a Network specification")))
 		})
-
-		It("Should reject a CloudStackCluster with IdentityRef not of kind 'Secret'", func() {
-			Ω(k8sClient.Create(ctx, dummies.CSCluster)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "must be a Secret")))
-		})
 	})
 
 	Context("When updating a CloudStackCluster", func() {
@@ -85,13 +80,5 @@ var _ = Describe("CloudStackCluster webhooks", func() {
 			Ω(k8sClient.Update(ctx, dummies.CSCluster)).
 				Should(MatchError(MatchRegexp(forbiddenRegex, "controlplaneendpoint\\.port")))
 		})
-		It("Should reject updates to the CloudStackCluster identity reference kind", func() {
-			Ω(k8sClient.Update(ctx, dummies.CSCluster)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "identityref\\.kind")))
-		})
-		It("Should reject updates to the CloudStackCluster identity reference name", func() {
-			Ω(k8sClient.Update(ctx, dummies.CSCluster)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "identityref\\.name")))
-		})
 	})
 })
diff --git a/api/v1beta2/cloudstackfailuredomain_types.go b/api/v1beta2/cloudstackfailuredomain_types.go
@@ -30,11 +30,11 @@ const (
 type Network struct {
 	// Cloudstack Network ID the cluster is built in.
 	// +optional
-	ID string `json:"id"`
+	ID string `json:"id,omitempty"`
 
 	// Cloudstack Network Type the cluster is built in.
 	// + optional
-	Type string `json:"type"`
+	Type string `json:"type,omitempty"`
 
 	// Cloudstack Network Name the cluster is built in.
 	Name string `json:"name"`
@@ -44,11 +44,11 @@ type Network struct {
 type CloudStackZoneSpec struct {
 	// Name.
 	//+optional
-	Name string `json:"name"`
+	Name string `json:"name,omitempty"`
 
 	// ID.
 	//+optional
-	ID string `json:"id"`
+	ID string `json:"id,omitempty"`
 
 	// The network within the Zone to use.
 	Network Network `json:"network"`
@@ -64,11 +64,11 @@ type CloudStackFailureDomainSpec struct {
 
 	// CloudStack account.
 	// +optional
-	Account string `json:"account"`
+	Account string `json:"account,omitempty"`
 
 	// CloudStack domain.
 	// +optional
-	Domain string `json:"domain"`
+	Domain string `json:"domain,omitempty"`
 
 	// Apache CloudStack Endpoint secret reference.
 	ACSEndpoint corev1.SecretReference `json:"acsendpoint"`
@@ -90,7 +90,7 @@ type CloudStackFailureDomain struct {
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
 	Spec   CloudStackFailureDomainSpec   `json:"spec"`
-	Status CloudStackFailureDomainStatus `json:"status"`
+	Status CloudStackFailureDomainStatus `json:"status,omitempty"`
 }
 
 //+kubebuilder:object:root=true
diff --git a/api/v1beta2/cloudstackmachine_types.go b/api/v1beta2/cloudstackmachine_types.go
@@ -112,11 +112,11 @@ type CloudStackResourceDiskOffering struct {
 // Type pulled mostly from the CloudStack API.
 type CloudStackMachineStatus struct {
 	// Addresses contains a CloudStack VM instance's IP addresses.
-	Addresses []corev1.NodeAddress `json:"addresses"`
+	Addresses []corev1.NodeAddress `json:"addresses,omitempty"`
 
 	// InstanceState is the state of the CloudStack instance for this machine.
 	// +optional
-	InstanceState string `json:"instanceState"`
+	InstanceState string `json:"instanceState,omitempty"`
 
 	// InstanceStateLastUpdated is the time the instance state was last updated.
 	// +optional
diff --git a/api/v1beta2/cloudstackmachine_webhook_test.go b/api/v1beta2/cloudstackmachine_webhook_test.go
@@ -70,11 +70,6 @@ var _ = Describe("CloudStackMachine webhook", func() {
 			Expect(k8sClient.Create(ctx, dummies.CSMachine1)).
 				Should(MatchError(MatchRegexp(requiredRegex, "Template")))
 		})
-
-		It("should reject a CloudStackMachine with IdentityRef not of kind 'Secret'", func() {
-			Expect(k8sClient.Create(ctx, dummies.CSMachine1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "must be a Secret")))
-		})
 	})
 
 	Context("When updating a CloudStackMachine", func() {
@@ -106,16 +101,6 @@ var _ = Describe("CloudStackMachine webhook", func() {
 				Should(MatchError(MatchRegexp(forbiddenRegex, "details")))
 		})
 
-		It("should reject identity reference kind updates to the CloudStackMachine", func() {
-			Ω(k8sClient.Update(ctx, dummies.CSMachine1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "identityRef\\.Kind")))
-		})
-
-		It("should reject identity reference name updates to the CloudStackMachine", func() {
-			Ω(k8sClient.Update(ctx, dummies.CSMachine1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "identityRef\\.Name")))
-		})
-
 		It("should reject updates to the list of affinty groups of the CloudStackMachine", func() {
 			dummies.CSMachine1.Spec.AffinityGroupIDs = []string{"28b907b8-75a7-4214-bd3d-6c61961fc2af"}
 			Ω(k8sClient.Update(ctx, dummies.CSMachine1)).
diff --git a/api/v1beta2/cloudstackmachinetemplate_webhook_test.go b/api/v1beta2/cloudstackmachinetemplate_webhook_test.go
@@ -60,11 +60,6 @@ var _ = Describe("CloudStackMachineTemplate webhook", func() {
 			Expect(k8sClient.Create(ctx, dummies.CSMachineTemplate1)).
 				Should(MatchError(MatchRegexp(requiredRegex, "Template")))
 		})
-
-		It("should reject a CloudStackMachineTemplate with IdentityRef not of kind 'Secret'", func() {
-			Expect(k8sClient.Create(ctx, dummies.CSMachine1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "must be a Secret")))
-		})
 	})
 
 	Context("When updating a CloudStackMachineTemplate", func() {
@@ -97,16 +92,6 @@ var _ = Describe("CloudStackMachineTemplate webhook", func() {
 				Should(MatchError(MatchRegexp(forbiddenRegex, "details")))
 		})
 
-		It("should reject identity reference kind updates to the CloudStackMachineTemplate", func() {
-			Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "identityRef\\.Kind")))
-		})
-
-		It("should reject identity reference name updates to the CloudStackMachineTemplate", func() {
-			Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "identityRef\\.Name")))
-		})
-
 		It("should reject updates to the list of AffinityGroupIDs of the CloudStackMachineTemplate", func() {
 			dummies.CSMachineTemplate1.Spec.Spec.Spec.AffinityGroupIDs = []string{"28b907b8-75a7-4214-bd3d-6c61961fc2ag"}
 			Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).ShouldNot(Succeed())
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackclusters.yaml
@@ -273,7 +273,6 @@ spec:
                               type: string
                           required:
                           - name
-                          - type
                           type: object
                       required:
                       - network
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackfailuredomains.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackfailuredomains.yaml
@@ -84,7 +84,6 @@ spec:
                         type: string
                     required:
                     - name
-                    - type
                     type: object
                 required:
                 - network
@@ -106,7 +105,6 @@ spec:
             type: object
         required:
         - spec
-        - status
         type: object
     served: true
     storage: true
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackisolatednetworks.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackisolatednetworks.yaml
@@ -169,7 +169,6 @@ spec:
                             type: string
                         required:
                         - name
-                        - type
                         type: object
                     required:
                     - network
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackmachines.yaml
@@ -461,7 +461,6 @@ spec:
                 description: Ready indicates the readiness of the provider resource.
                 type: boolean
             required:
-            - addresses
             - ready
             type: object
         type: object
diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml
@@ -15,8 +15,8 @@ spec:
       labels:
         control-plane: capc-controller-manager
     spec:
-      securityContext:
-        runAsNonRoot: true
+      # securityContext:
+      #   runAsNonRoot: true
       containers:
       - command:
         - /manager
diff --git a/controllers/cloudstackmachine_controller_test.go b/controllers/cloudstackmachine_controller_test.go
@@ -50,7 +50,7 @@ var _ = Describe("CloudStackMachineReconciler", func() {
 			setClusterReady()
 		})
 
-		It("Should call GetOrCreateVMInstance and set Status.Ready to true", func() {
+		It("Shoueld call GetOrCreateVMInstance and set Status.Ready to true", func() {
 			// Mock a call to GetOrCreateVMInstance and set the machine to running.
 			mockCloudClient.EXPECT().GetOrCreateVMInstance(
 				gomock.Any(), gomock.Any(), gomock.Any(),
diff --git a/controllers/controllers_suite_test.go b/controllers/controllers_suite_test.go
@@ -159,16 +159,16 @@ type MockCtrlrCloudClientImplementation struct {
 
 // AsFailureDomainUser is a method used in the reconciliation runner to set up the CloudStack client. Using this here
 // just sets the CSClient to a mock client.
-func (r *MockCtrlrCloudClientImplementation) AsFailureDomainUser(
+func (m *MockCtrlrCloudClientImplementation) AsFailureDomainUser(
 	*infrav1.CloudStackFailureDomainSpec) csCtrlrUtils.CloudStackReconcilerMethod {
 	return func() (ctrl.Result, error) {
-		r.CSUser = mockCloudClient
+		m.CSUser = mockCloudClient
 
 		return ctrl.Result{}, nil
 	}
 }
 
-func (M *MockCtrlrCloudClientImplementation) RegisterExtension(r *csCtrlrUtils.ReconciliationRunner) csCtrlrUtils.CloudClientExtension {
+func (m *MockCtrlrCloudClientImplementation) RegisterExtension(r *csCtrlrUtils.ReconciliationRunner) csCtrlrUtils.CloudClientExtension {
 	return &MockCtrlrCloudClientImplementation{ReconciliationRunner: r}
 }
 
diff --git a/controllers/utils/failuredomains.go b/controllers/utils/failuredomains.go
@@ -94,44 +94,44 @@ type CloudClientImplementation struct {
 	*ReconciliationRunner
 }
 
-func (C *CloudClientImplementation) RegisterExtension(r *ReconciliationRunner) CloudClientExtension {
-	C.ReconciliationRunner = r
-	return C
+func (c *CloudClientImplementation) RegisterExtension(r *ReconciliationRunner) CloudClientExtension {
+	c.ReconciliationRunner = r
+	return c
 }
 
 // AsFailureDomainUser uses the credentials specified in the failure domain to set the ReconciliationSubject's CSUser client.
-func (r *CloudClientImplementation) AsFailureDomainUser(fdSpec *infrav1.CloudStackFailureDomainSpec) CloudStackReconcilerMethod {
+func (c *CloudClientImplementation) AsFailureDomainUser(fdSpec *infrav1.CloudStackFailureDomainSpec) CloudStackReconcilerMethod {
 	return func() (ctrl.Result, error) {
-		if !strings.HasSuffix(fdSpec.Name, "-"+r.CAPICluster.Name) { // Add cluster name suffix if missing.
-			fdSpec.Name = fdSpec.Name + "-" + r.CAPICluster.Name
+		if !strings.HasSuffix(fdSpec.Name, "-"+c.CAPICluster.Name) { // Add cluster name suffix if missing.
+			fdSpec.Name = fdSpec.Name + "-" + c.CAPICluster.Name
 		}
 		endpointCredentials := &corev1.Secret{}
 		key := client.ObjectKey{Name: fdSpec.ACSEndpoint.Name, Namespace: fdSpec.ACSEndpoint.Namespace}
-		if err := r.K8sClient.Get(r.RequestCtx, key, endpointCredentials); err != nil {
+		if err := c.K8sClient.Get(c.RequestCtx, key, endpointCredentials); err != nil {
 			return ctrl.Result{}, errors.Wrapf(err, "getting ACSEndpoint secret with ref: %v", fdSpec.ACSEndpoint)
 		}
 
 		var err error
-		if r.CSClient, err = cloud.NewClientFromK8sSecret(endpointCredentials); err != nil {
+		if c.CSClient, err = cloud.NewClientFromK8sSecret(endpointCredentials); err != nil {
 			return ctrl.Result{}, errors.Wrapf(err, "parsing ACSEndpoint secret with ref: %v", fdSpec.ACSEndpoint)
 		}
 
 		// Transfer Cluster Domain & Account to FailureDomain as needed.
 		if fdSpec.Account == "" {
-			if r.CSCluster.Spec.Account != "" {
-				fdSpec.Account = r.CSCluster.Spec.Account
-				fdSpec.Domain = r.CSCluster.Spec.Domain
+			if c.CSCluster.Spec.Account != "" {
+				fdSpec.Account = c.CSCluster.Spec.Account
+				fdSpec.Domain = c.CSCluster.Spec.Domain
 			}
 		}
 
-		if r.CSCluster.Spec.Account != "" { // Set r.CSUser CloudStack Client per Account and Domain.
-			client, err := r.CSClient.NewClientInDomainAndAccount(fdSpec.Domain, fdSpec.Account)
+		if c.CSCluster.Spec.Account != "" { // Set r.CSUser CloudStack Client per Account and Domain.
+			client, err := c.CSClient.NewClientInDomainAndAccount(fdSpec.Domain, fdSpec.Account)
 			if err != nil {
 				return ctrl.Result{}, err
 			}
-			r.CSUser = client
+			c.CSUser = client
 		} else { // Set r.CSUser CloudStack Client to r.CSClient since Account & Domain weren't provided.
-			r.CSUser = r.CSClient
+			c.CSUser = c.CSClient
 		}
 
 		return ctrl.Result{}, nil
diff --git a/pkg/cloud/client.go b/pkg/cloud/client.go
@@ -59,7 +59,7 @@ type client struct {
 }
 
 type SecretConfig struct {
-	ApiVersion string            `yaml:"apiVersion"`
+	APIVersion string            `yaml:"apiVersion"`
 	Kind       string            `yaml:"kind"`
 	Type       string            `yaml:"type"`
 	Metadata   map[string]string `yaml:"metadata"`
