Merge pull request #7 from scality/improvement/remove-validate-from-reconcile

chore: Remove validation from reconcile loop

Author: TeddyAndrieux

api/v1alpha1/managedcrl_types.go

@@ -95,7 +95,7 @@ type CRLExposeSpec struct {
 
 	// Image specifies the container image to use for exposing the CRL.
 	// +optional
-	Image *ImageSpec `json:"image"`
+	Image ImageSpec `json:"image,omitempty"`
 	// Node Selector to deploy the CRL server
 	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
@@ -128,8 +128,7 @@ type RevocationSpec struct {
 	RevocationTime *metav1.Time `json:"revocationTime,omitempty"`
 
 	// Reason is the reason for revocation (refer to RFC 5280 Section 5.3.1.).
-	// +optional
-	ReasonCode *int `json:"reasonCode,omitempty"`
+	ReasonCode int `json:"reasonCode,omitempty"`
 }
 
 // ManagedCRLSpec defines the desired state of ManagedCRL.
@@ -300,15 +299,9 @@ func (rs *RevocationSpec) withDefaults() {
 	if rs.RevocationTime == nil {
 		rs.RevocationTime = &metav1.Time{Time: metav1.Now().Time}
 	}
-	if rs.ReasonCode == nil {
-		rs.ReasonCode = ptr.To(0) // Unspecified
-	}
 }
 
 func (ces *CRLExposeSpec) withDefaults() {
-	if ces.Image == nil {
-		ces.Image = &ImageSpec{}
-	}
 	ces.Image.withDefaults()
 
 	if ces.Ingress != nil {
@@ -340,6 +333,7 @@ func (is *IngressSpec) withDefaults() {
 
 // Validate validates the ManagedCRL resource.
 func (mcrl *ManagedCRL) Validate() error {
+	mcrl.WithDefaults()
 	err := mcrl.Spec.validate()
 	if err != nil {
 		return fmt.Errorf("spec validation failed: %w", err)
@@ -439,12 +433,14 @@ func (rs RevocationSpec) ToRevocationListEntry() (x509.RevocationListEntry, erro
 	return x509.RevocationListEntry{
 		SerialNumber:   serial,
 		RevocationTime: rs.RevocationTime.Time,
-		ReasonCode:     *rs.ReasonCode,
+		ReasonCode:     rs.ReasonCode,
 	}, nil
 }
 
 // GetRevokedListEntries converts the Revocations in ManagedCRLSpec to a slice of x509.RevocationListEntry.
 func (mcrls *ManagedCRLSpec) GetRevokedListEntries() ([]x509.RevocationListEntry, error) {
+	mcrls.withDefaults()
+
 	if mcrls.Revocations == nil {
 		return []x509.RevocationListEntry{}, nil
 	}
@@ -462,6 +458,8 @@ func (mcrls *ManagedCRLSpec) GetRevokedListEntries() ([]x509.RevocationListEntry
 
 // GetImage returns the full image string in the format "repository/name:tag".
 func (is *ImageSpec) GetImage() string {
+	is.withDefaults()
+
 	image := fmt.Sprintf("%s:%s", *is.Name, *is.Tag)
 	if is.Repository != nil {
 		image = fmt.Sprintf("%s/%s", *is.Repository, image)
@@ -471,6 +469,8 @@ func (is *ImageSpec) GetImage() string {
 
 // GetCRLDistributionPoint returns the CRL distribution point URL based on the Ingress configuration.
 func (mcrl *ManagedCRL) GetCRLDistributionPoint() []string {
+	mcrl.WithDefaults()
+
 	var urls []string
 
 	// Add Ingress URLs if enabled

api/v1alpha1/zz_generated.deepcopy.go

@@ -152,9 +152,6 @@ func (r *ManagedCRLReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 	// Apply defaults
 	instance.WithDefaults()
-	if err := instance.Validate(); err != nil {
-		return ctrl.Result{}, fmt.Errorf("validation failed: %w", err)
-	}
 
 	needRenewal := false
 	original := instance.DeepCopy()

internal/controller/managedcrl_controller.go

@@ -103,7 +103,6 @@ func (v *ManagedCRLCustomValidator) ValidateDelete(ctx context.Context, obj runt
 
 // validationManagedCRL validates the ManagedCRL fields.
 func validationManagedCRL(logger logr.Logger, ctx context.Context, c client.Client, managedcrl *crloperatorv1alpha1.ManagedCRL) error {
-	managedcrl.WithDefaults()
 	if err := managedcrl.Validate(); err != nil {
 		logger.Error(err, "Validation failed")
 		return err

internal/webhook/v1alpha1/managedcrl_webhook.go

@@ -69,7 +69,7 @@ var (
 			spec: crloperatorv1alpha1.ManagedCRLSpec{
 				Expose: &crloperatorv1alpha1.CRLExposeSpec{
 					Enabled:  true,
-					Image:    &crloperatorv1alpha1.ImageSpec{Repository: ptr.To("custom/repo"), Tag: ptr.To("v1.2.3")},
+					Image:    crloperatorv1alpha1.ImageSpec{Repository: ptr.To("custom/repo"), Tag: ptr.To("v1.2.3")},
 					Internal: ptr.To(false),
 				},
 			},
@@ -401,7 +401,7 @@ var _ = Describe("ManagedCRL Controller", func() {
 				retrieved.Spec.Revocations = []crloperatorv1alpha1.RevocationSpec{
 					{
 						SerialNumber: "123456789",
-						ReasonCode:   ptr.To(2),
+						ReasonCode:   2,
 					},
 				}
 				Expect(k8sClient.Update(ctx, retrieved)).To(Succeed())
@@ -412,7 +412,7 @@ var _ = Describe("ManagedCRL Controller", func() {
 				retrieved.Spec.Revocations = []crloperatorv1alpha1.RevocationSpec{
 					{
 						SerialNumber: "123456789",
-						ReasonCode:   ptr.To(1),
+						ReasonCode:   1,
 					},
 				}
 				Expect(k8sClient.Update(ctx, retrieved)).To(Succeed())
@@ -510,7 +510,6 @@ func checkSecret(mcrlRef types.NamespacedName) {
 		}
 		return false
 	}, 10*time.Second, time.Second).Should(BeTrue())
-	retrieved.WithDefaults()
 
 	Expect(retrieved.ObjectMeta.Finalizers).To(ContainElement("crl-operator.scality.com/finalizer"))
 
@@ -576,7 +575,6 @@ func checkExposePod(mcrlRef types.NamespacedName, shouldRestart bool) {
 			}
 			return false
 		}, 10*time.Second, time.Second).Should(BeTrue())
-		retrieved.WithDefaults()
 		Expect(retrieved.Status.PodExposed).To(PointTo(BeFalse()))
 
 		// Check the deployment
@@ -615,7 +613,6 @@ func checkExposePod(mcrlRef types.NamespacedName, shouldRestart bool) {
 		}
 		return false
 	}, 10*time.Second, time.Second).Should(BeTrue())
-	retrieved.WithDefaults()
 
 	Expect(retrieved.Status.PodExposed).To(PointTo(BeTrue()))
 
@@ -663,7 +660,6 @@ func checkIngress(mcrlRef types.NamespacedName) {
 		}
 		return false
 	}, 10*time.Second, time.Second).Should(BeTrue())
-	retrieved.WithDefaults()
 
 	Expect(retrieved.Status.IngressExposed).To(PointTo(BeTrue()))
 
@@ -707,7 +703,6 @@ func checkIssuerConfigured(mcrlRef types.NamespacedName) {
 		}
 		return false
 	}, 10*time.Second, time.Second).Should(BeTrue())
-	retrieved.WithDefaults()
 
 	Expect(retrieved.Status.IssuerConfigured).To(PointTo(BeTrue()))