Added .spec.caID validation

.gitignore

@@ -15,17 +15,18 @@
 builds/
 bin/
 
-# Intelij setting
-.idea/
+
 
 # Dependency directories
 vendor/
 
 # Go workspace file
 go.work
 
+#VSCode settings folder
+.vscode/
 
-#Intelij setting folder
+#Intelij settings folder
 .idea/
 
 #test reports and coverage

api/v1/issuer_types.go

@@ -181,7 +181,7 @@ type IssuerCondition struct {
 	// Reason is a brief machine-readable explanation for the condition's last
 	// transition.
 	// +optional
-	Reason string `json:"reason,omitempty"`
+	Reason ReasonType `json:"reason,omitempty"`
 
 	// Message is a human-readable description of the details of the last
 	// transition, complementing reason.
@@ -221,6 +221,21 @@ const (
 	ConditionUnknown ConditionStatus = "Unknown"
 )
 
+// ConditionStatus represents a condition's status.
+// +kubebuilder:validation:Enum=SecretNotFound;Verified;Error
+type ReasonType string
+
+const (
+	// ReasonNotFound represents the fact that secrets needed to authenticate to the NCM API do not exist in cluster
+	ReasonNotFound ReasonType = "SecretNotFound"
+
+	// ReasonVerified represents the fact that the NCM Issuer(ClusterIssuer) are configured correctly
+	ReasonVerified ReasonType = "Verified"
+
+	// ReasonError represents the fact that the NCM Issuer(ClusterIssuer) are configured not correctly and require user interaction
+	ReasonError ReasonType = "Error"
+)
+
 func init() {
 	SchemeBuilder.Register(&Issuer{}, &IssuerList{})
 }

config/crd/bases/certmanager.ncm.nokia.com_clusterissuers.yaml

@@ -192,6 +192,10 @@ spec:
                     reason:
                       description: Reason is a brief machine-readable explanation
                         for the condition's last transition.
+                      enum:
+                      - SecretNotFound
+                      - Verified
+                      - Error
                       type: string
                     status:
                       allOf:

config/crd/bases/certmanager.ncm.nokia.com_issuers.yaml

@@ -192,6 +192,10 @@ spec:
                     reason:
                       description: Reason is a brief machine-readable explanation
                         for the condition's last transition.
+                      enum:
+                      - SecretNotFound
+                      - Verified
+                      - Error
                       type: string
                     status:
                       allOf:

pkg/cfg/ncmcfg.go

@@ -17,7 +17,7 @@ limitations under the License.
 package cfg
 
 import (
-	"fmt"
+	"errors"
 	"reflect"
 	"strings"
 	"time"
@@ -213,24 +213,32 @@ func (cfg *NCMConfig) InjectNamespace(namespace string) {
 
 func (cfg *NCMConfig) Validate() error {
 	if cfg.MainAPI == "" {
-		return fmt.Errorf("incorrect NCM API data: missing main API url")
+		return cfg.getError("incorrect NCM API data: missing main API url")
+	}
+
+	if caIDInUnsupportedFormat(cfg.CAID) {
+		return cfg.getError("incorrect caID \"" + cfg.CAID + "\". Please provide only ID of the https://ncm.domain.example/v1/cas/{ID} endpoint")
 	}
 
 	if cfg.Username == "" || cfg.Password == "" {
-		return fmt.Errorf("incorrect authentication data: missing username or usrpassword")
+		return cfg.getError("incorrect authentication data: missing username or usrpassword")
 	}
 
 	if cfg.CAName == "" && cfg.CAID == "" {
-		return fmt.Errorf("incorrect signing CA certificate data: missing CANAME or CAHREF")
+		return cfg.getError("incorrect signing CA certificate data: missing CANAME or CAHREF")
 	}
 
 	if !reflect.DeepEqual(cfg.TLSNamespacedName, types.NamespacedName{}) && cfg.CACert == "" && cfg.Key == "" && cfg.Cert == "" {
-		return fmt.Errorf("incorrect TLS data: missing cacert, key or cert in TLS secret")
+		return cfg.getError("incorrect TLS data: missing cacert, key or cert in TLS secret")
 	}
 
 	return nil
 }
 
+func (cfg *NCMConfig) getError(message string) error {
+	return errors.New("Failed to validate config provided in spec: " + message)
+}
+
 func (cfg *NCMConfig) handleDeprecatedFields(issuerSpec *ncmv1.IssuerSpec) {
 	if cfg.MainAPI == "" {
 		cfg.MainAPI = strings.TrimSuffix(issuerSpec.NCMServer, "/")

pkg/cfg/util.go

@@ -0,0 +1,22 @@
+package cfg
+
+import (
+	"regexp"
+)
+
+var (
+	matchesRestEndpointRegexp = regexp.MustCompile(`^/v\d+/cas/[A-Za-z0-9_-]+$`)
+	containsSlashRegex        = regexp.MustCompile(`/`)
+)
+
+func caIDInUnsupportedFormat(input string) bool {
+	return matchesFullRestCasPattern(input) || containSlash(input)
+}
+
+func matchesFullRestCasPattern(input string) bool {
+	return matchesRestEndpointRegexp.MatchString(input)
+}
+
+func containSlash(input string) bool {
+	return containsSlashRegex.MatchString(input)
+}

pkg/cfg/util_test.go

@@ -0,0 +1,97 @@
+package cfg
+
+import (
+	"testing"
+)
+
+func TestUnsupportedCaIdPatterns(t *testing.T) {
+	tests := []struct {
+		input               string
+		looksLikeMistake    bool
+		testCaseDescription string
+	}{
+		{
+			input:               "/v1/cas/nZ2QfzjGv0U3HmJuVxEsZQ",
+			looksLikeMistake:    true,
+			testCaseDescription: "api-path-copied-from-the-ncm-ca-details-pasted"},
+		{
+			input:               "/v1/cas/nZ2QfzjGv0U3HmJuVxEsZQ/",
+			looksLikeMistake:    true,
+			testCaseDescription: "api-path-copied-from-the-ncm-ca-details-pasted-trailing-slash",
+		},
+		{
+			input:               "v1/cas/nZ2QfzjGv0U3HmJuVxEsZQ",
+			looksLikeMistake:    true,
+			testCaseDescription: "api-path-copied-from-the-ncm-ca-details-pasted-no-initial-slash",
+		},
+		{
+			input:               "v1/cas/nZ2QfzjGv0U3HmJuVxEsZQ/",
+			looksLikeMistake:    true,
+			testCaseDescription: "api-path-copied-from-the-ncm-ca-details-pasted-no-initial-slash-trailing-slash",
+		},
+		{
+			input:               "/v2/cas/abcdef123456",
+			looksLikeMistake:    true,
+			testCaseDescription: "api-path-copied-from-the-ncm-ca-details-pasted-new-api-version-test",
+		},
+		{
+			input:               "/v1/cas/123-456_ABC",
+			looksLikeMistake:    true,
+			testCaseDescription: "api-path-copied-from-the-ncm-ca-details-pasted-different-ID",
+		},
+		{
+			input:               "/v1/cas",
+			looksLikeMistake:    true,
+			testCaseDescription: "api-path-copied-from-the-ncm-ca-details-missing-id",
+		},
+		{
+			input:               "v1/cas/",
+			looksLikeMistake:    true,
+			testCaseDescription: "api-path-copied-from-the-ncm-ca-details-missing-id-no-initial-slash",
+		},
+		{
+			input:               "v1/cas/",
+			looksLikeMistake:    true,
+			testCaseDescription: "api-path-copied-from-the-ncm-ca-details-missing-id-no-initial-slas-trailing-slash",
+		},
+		{
+			input:               "https://ncm.domain.example/v1/cas/123-456_ABC",
+			looksLikeMistake:    true,
+			testCaseDescription: "full-url-provided-https",
+		},
+		{
+			input:               "http://ncm.domain.example/v1/cas/123-456_ABC",
+			looksLikeMistake:    true,
+			testCaseDescription: "full-url-provided-http",
+		},
+		{
+			input:               "ncm.domain.example/v1/cas/123-456_ABC",
+			looksLikeMistake:    true,
+			testCaseDescription: "full-url-provided-no-protocol",
+		},
+		{
+			input:               "https://ncm.ca/cas/123-456_ABC",
+			looksLikeMistake:    true,
+			testCaseDescription: "another-full-url-provided",
+		},
+		{
+			input:               "",
+			looksLikeMistake:    false,
+			testCaseDescription: "empty-should-not-happen-as-this-is-mandatory-non-empty-field-in-spec",
+		},
+		{
+			input:               "nZ2QfzjGv0U3HmJuVxEsZQ",
+			looksLikeMistake:    false,
+			testCaseDescription: "corrent-caID",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.testCaseDescription, func(t *testing.T) {
+			result := caIDInUnsupportedFormat(test.input)
+			if result != test.looksLikeMistake {
+				t.Errorf("Description: %s - matchPattern(%q) = %v; looksLikeMistake %v", test.testCaseDescription, test.input, result, test.looksLikeMistake)
+			}
+		})
+	}
+}

pkg/controllers/issuer_controller.go

@@ -87,7 +87,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		log.Info("Issuer resource not found, ignoring...")
 		return ctrl.Result{}, nil
 	}
-	issuerSpec, _, err := GetSpecAndStatus(issuer)
+	issuerSpec, issuerStatus, err := GetSpecAndStatus(issuer)
 	if err != nil {
 		log.Error(err, "Unexpected error while getting issuer spec and status. Not retrying.")
 		return ctrl.Result{}, nil
@@ -98,9 +98,9 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	authSecret := &core.Secret{}
 	if err = r.Get(ctx, NCMCfg.AuthNamespacedName, authSecret); err != nil {
 		log.Error(err, "Failed to retrieve auth secret from namespace", "namespacedName", NCMCfg.AuthNamespacedName)
-		reason := "Error"
+		reason := ncmv1.ReasonError
 		if apierrors.IsNotFound(err) {
-			reason = "NotFound"
+			reason = ncmv1.ReasonNotFound
 		}
 		_ = r.SetStatus(ctx, issuer, ncmv1.ConditionFalse, reason, "Failed to retrieve auth secret err: %v", err)
 		return ctrl.Result{}, err
@@ -111,35 +111,43 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		tlsSecret := &core.Secret{}
 		if err = r.Get(ctx, NCMCfg.TLSNamespacedName, tlsSecret); err != nil {
 			log.Error(err, "Failed to retrieve TLS secret from namespace", "namespacedName", NCMCfg.TLSNamespacedName)
-			reason := "Error"
+			reason := ncmv1.ReasonError
 			if apierrors.IsNotFound(err) {
-				reason = "NotFound"
+				reason = ncmv1.ReasonNotFound
 			}
 
 			_ = r.SetStatus(ctx, issuer, ncmv1.ConditionFalse, reason, "Failed to retrieve tls secret err: %v", err)
 			return ctrl.Result{}, err
 		}
 		if err = NCMCfg.AddTLSData(tlsSecret); err != nil {
-			_ = r.SetStatus(ctx, issuer, ncmv1.ConditionFalse, "Error", "Failed to add TLS secret data to config err: %v", err)
+			_ = r.SetStatus(ctx, issuer, ncmv1.ConditionFalse, ncmv1.ReasonError, "Failed to add TLS secret data to config err: %v", err)
 			return ctrl.Result{}, err
 		}
 	}
 
-	if err = NCMCfg.Validate(); err != nil {
-		log.Error(err, "Failed to validate config provided in spec")
-		_ = r.SetStatus(ctx, issuer, ncmv1.ConditionFalse, "Error", "Failed to validate config provided in spec: %v", err)
+	if err := NCMCfg.Validate(); err != nil {
+		// if the resource contidion has not changed, stop reconciling until updated
+		if IssuerHasConditionAndReasonAndMessage(*issuerStatus, ncmv1.IssuerCondition{
+			Type:    ncmv1.IssuerConditionReady,
+			Status:  ncmv1.ConditionFalse,
+			Reason:  ncmv1.ReasonError,
+			Message: err.Error(),
+		}) {
+			return ctrl.Result{}, nil
+		}
+		_ = r.SetStatus(ctx, issuer, ncmv1.ConditionFalse, ncmv1.ReasonError, err.Error())
 		return ctrl.Result{}, err
 	}
 
 	p, err := provisioner.NewProvisioner(NCMCfg, log.WithName("provisioner"))
 	if err != nil {
 		log.Error(err, "Failed to create new provisioner")
-		_ = r.SetStatus(ctx, issuer, ncmv1.ConditionFalse, "Error", "Failed to create new provisioner err: %v", err)
+		_ = r.SetStatus(ctx, issuer, ncmv1.ConditionFalse, ncmv1.ReasonError, "Failed to create new provisioner err: %v", err)
 		return ctrl.Result{}, err
 	}
 
 	r.Provisioners.AddOrReplace(req.NamespacedName, p)
-	return ctrl.Result{}, r.SetStatus(ctx, issuer, ncmv1.ConditionTrue, "Verified", "Signing CA verified and ready to sign certificates")
+	return ctrl.Result{}, r.SetStatus(ctx, issuer, ncmv1.ConditionTrue, ncmv1.ReasonVerified, "Signing CA verified and ready to sign certificates")
 }
 
 // SetCondition will set a 'condition' on the given issuer.
@@ -150,7 +158,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 //   - If a condition of the same type and different state already exists, the
 //     condition will be updated and the LastTransitionTime set to the current
 //     time.
-func (r *IssuerReconciler) SetCondition(issuerStatus *ncmv1.IssuerStatus, status ncmv1.ConditionStatus, reason, message string) {
+func (r *IssuerReconciler) SetCondition(issuerStatus *ncmv1.IssuerStatus, status ncmv1.ConditionStatus, reason ncmv1.ReasonType, message string) {
 	newCondition := &ncmv1.IssuerCondition{
 		Type:    ncmv1.IssuerConditionReady,
 		Status:  status,
@@ -185,7 +193,7 @@ func (r *IssuerReconciler) SetCondition(issuerStatus *ncmv1.IssuerStatus, status
 	r.Log.V(2).Info("setting lastTransitionTime for issuer condition", "condition", ncmv1.IssuerConditionReady, "time", nowTime.Time)
 }
 
-func (r *IssuerReconciler) SetStatus(ctx context.Context, issuer client.Object, conditionStatus ncmv1.ConditionStatus, reason, message string, args ...interface{}) error {
+func (r *IssuerReconciler) SetStatus(ctx context.Context, issuer client.Object, conditionStatus ncmv1.ConditionStatus, reason ncmv1.ReasonType, message string, args ...interface{}) error {
 	// Format the message and update the issuer variable with the new Condition
 	var issuerStatus *ncmv1.IssuerStatus
 	switch t := issuer.(type) {
@@ -205,7 +213,7 @@ func (r *IssuerReconciler) SetStatus(ctx context.Context, issuer client.Object,
 	if conditionStatus == ncmv1.ConditionFalse {
 		eventType = core.EventTypeWarning
 	}
-	r.Recorder.Event(issuer, eventType, reason, completeMessage)
+	r.Recorder.Event(issuer, eventType, string(reason), completeMessage)
 
 	// Actually update the issuer resource
 	var err error