feat: enhance CR status with clearer brief error message

errors/pluginerrors.go

@@ -168,4 +168,11 @@ var (
 		Message:     "Key vault operation failed",
 		Description: "Key vault operation failed. Please validate correct key vault configuration is provided or check the error details for further investigation.",
 	})
+
+	// ErrorCodeKeyManagementProviderFailure is returned when a key management provider operation fails.
+	ErrorCodeKeyManagementProviderFailure = Register("errcode", ErrorDescriptor{
+		Value:       "KEY_MANAGEMENT_PROVIDER_FAILURE",
+		Message:     "Key management provider failure",
+		Description: "Generic failure in key management provider. Please validate correct key management provider configuration is provided or check the error details for further investigation.",
+	})
 )

errors/types.go

@@ -253,6 +253,18 @@ func (e Error) GetRemediation() string {
 	return err.remediation
 }
 
+// GetConciseError returns a formatted error message consisting of the error code and reason.
+// If the generated error message exceeds the specified maxLength, it truncates the message and appends an ellipsis ("...").
+// The function ensures that the returned error message is concise and within the length limit.
+func (e Error) GetConciseError(maxLength int) string {
+	err, _ := e.getRootError()
+	errMsg := fmt.Sprintf("%s: %s", err.ErrorCode().Descriptor().Value, e.GetErrorReason())
+	if len(errMsg) > maxLength {
+		return fmt.Sprintf("%s...", errMsg[:maxLength-3])
+	}
+	return errMsg
+}
+
 func (e Error) getRootError() (err Error, details []string) {
 	err = e
 	for !err.isRootError {

errors/types_test.go

@@ -139,6 +139,18 @@ func TestWithDescription(t *testing.T) {
 	}
 }
 
+func TestGetConciseError(t *testing.T) {
+	err := testEC.WithDetail("long message, long message, long message")
+	if err.GetConciseError(30) != "TEST_ERROR_CODE_1: long mes..." {
+		t.Fatalf("expected: TEST_ERROR_CODE_1: long mes..., got: %s", err.GetConciseError(30))
+	}
+
+	err = testEC.WithDetail("short message")
+	if err.GetConciseError(100) != "TEST_ERROR_CODE_1: short message" {
+		t.Fatalf("expected: TEST_ERROR_CODE_1: short message, got: %s", err.GetConciseError(100))
+	}
+}
+
 func TestIs(t *testing.T) {
 	err := testEC.WithDetail(testDetail1)
 	result := err.Is(err)

internal/constants/constants.go

@@ -19,4 +19,4 @@ package constants
 const RatifyPolicy = "ratify-policy"
 const EmptyNamespace = ""
 const NamespaceSeperator = "/"
-const MaxBriefErrLength = 30
+const MaxBriefErrLength = 150

pkg/controllers/clusterresource/policy_controller.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	configv1beta1 "github.com/ratify-project/ratify/api/v1beta1"
+	re "github.com/ratify-project/ratify/errors"
 	"github.com/ratify-project/ratify/internal/constants"
 	"github.com/ratify-project/ratify/pkg/controllers"
 	"github.com/ratify-project/ratify/pkg/controllers/utils"
@@ -63,19 +64,20 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	}
 
 	if resource != constants.RatifyPolicy {
-		errStr := fmt.Sprintf("metadata.name must be ratify-policy, got %s", resource)
-		policyLogger.Error(errStr)
-		writePolicyStatus(ctx, r, &policy, policyLogger, false, errStr)
+		err := re.ErrorCodeConfigInvalid.WithDetail(fmt.Sprintf("metadata.name must be ratify-policy, got %s", resource))
+		policyLogger.Error(err)
+		writePolicyStatus(ctx, r, &policy, policyLogger, false, &err)
 		return ctrl.Result{}, nil
 	}
 
 	if err := policyAddOrReplace(policy.Spec); err != nil {
-		policyLogger.Error("unable to create policy from policy crd: ", err)
-		writePolicyStatus(ctx, r, &policy, policyLogger, false, err.Error())
-		return ctrl.Result{}, err
+		policyErr := re.ErrorCodePluginInitFailure.WithError(err).WithDetail("Unable to create policy from policy crd.")
+		policyLogger.Error(policyErr)
+		writePolicyStatus(ctx, r, &policy, policyLogger, false, &policyErr)
+		return ctrl.Result{}, policyErr
 	}
 
-	writePolicyStatus(ctx, r, &policy, policyLogger, true, "")
+	writePolicyStatus(ctx, r, &policy, policyLogger, true, nil)
 	return ctrl.Result{}, nil
 }
 
@@ -96,11 +98,11 @@ func policyAddOrReplace(spec configv1beta1.PolicySpec) error {
 	return nil
 }
 
-func writePolicyStatus(ctx context.Context, r client.StatusClient, policy *configv1beta1.Policy, logger *logrus.Entry, isSuccess bool, errString string) {
+func writePolicyStatus(ctx context.Context, r client.StatusClient, policy *configv1beta1.Policy, logger *logrus.Entry, isSuccess bool, err *re.Error) {
 	if isSuccess {
 		updatePolicySuccessStatus(policy)
 	} else {
-		updatePolicyErrorStatus(policy, errString)
+		updatePolicyErrorStatus(policy, err)
 	}
 	if statusErr := r.Status().Update(ctx, policy); statusErr != nil {
 		logger.Error(statusErr, ", unable to update policy error status")
@@ -113,12 +115,8 @@ func updatePolicySuccessStatus(policy *configv1beta1.Policy) {
 	policy.Status.BriefError = ""
 }
 
-func updatePolicyErrorStatus(policy *configv1beta1.Policy, errString string) {
-	briefErr := errString
-	if len(errString) > constants.MaxBriefErrLength {
-		briefErr = fmt.Sprintf("%s...", errString[:constants.MaxBriefErrLength])
-	}
+func updatePolicyErrorStatus(policy *configv1beta1.Policy, err *re.Error) {
 	policy.Status.IsSuccess = false
-	policy.Status.Error = errString
-	policy.Status.BriefError = briefErr
+	policy.Status.Error = err.Error()
+	policy.Status.BriefError = err.GetConciseError(constants.MaxBriefErrLength)
 }

pkg/controllers/clusterresource/policy_controller_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	configv1beta1 "github.com/ratify-project/ratify/api/v1beta1"
+	re "github.com/ratify-project/ratify/errors"
 	"github.com/ratify-project/ratify/internal/constants"
 	"github.com/ratify-project/ratify/pkg/controllers"
 	"github.com/ratify-project/ratify/pkg/customresources/policies"
@@ -110,7 +111,8 @@ func TestWritePolicyStatus(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(_ *testing.T) {
-			writePolicyStatus(context.Background(), tc.reconciler, tc.policy, logger, tc.isSuccess, tc.errString)
+			err := re.ErrorCodeUnknown.WithDetail(tc.errString)
+			writePolicyStatus(context.Background(), tc.reconciler, tc.policy, logger, tc.isSuccess, &err)
 		})
 	}
 }

pkg/controllers/clusterresource/store_controller.go

@@ -25,6 +25,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	configv1beta1 "github.com/ratify-project/ratify/api/v1beta1"
+	re "github.com/ratify-project/ratify/errors"
 	"github.com/ratify-project/ratify/internal/constants"
 	"github.com/ratify-project/ratify/pkg/controllers"
 	"github.com/ratify-project/ratify/pkg/controllers/utils"
@@ -65,12 +66,13 @@ func (r *StoreReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 	}
 
 	if err := storeAddOrReplace(store.Spec, resource); err != nil {
-		storeLogger.Error(err, "unable to create store from store crd")
-		writeStoreStatus(ctx, r, &store, storeLogger, false, err.Error())
-		return ctrl.Result{}, err
+		storeErr := re.ErrorCodeReferrerStoreFailure.WithError(err).WithDetail("Unable to create store from store crd")
+		storeLogger.Error(err)
+		writeStoreStatus(ctx, r, &store, storeLogger, false, &storeErr)
+		return ctrl.Result{}, storeErr
 	}
 
-	writeStoreStatus(ctx, r, &store, storeLogger, true, "")
+	writeStoreStatus(ctx, r, &store, storeLogger, true, nil)
 
 	// returning empty result and no error to indicate we’ve successfully reconciled this object
 	return ctrl.Result{}, nil
@@ -93,17 +95,15 @@ func storeAddOrReplace(spec configv1beta1.StoreSpec, fullname string) error {
 	return utils.UpsertStoreMap(spec.Version, spec.Address, fullname, constants.EmptyNamespace, storeConfig)
 }
 
-func writeStoreStatus(ctx context.Context, r client.StatusClient, store *configv1beta1.Store, logger *logrus.Entry, isSuccess bool, errorString string) {
+func writeStoreStatus(ctx context.Context, r client.StatusClient, store *configv1beta1.Store, logger *logrus.Entry, isSuccess bool, err *re.Error) {
 	if isSuccess {
 		store.Status.IsSuccess = true
 		store.Status.Error = ""
 		store.Status.BriefError = ""
 	} else {
 		store.Status.IsSuccess = false
-		store.Status.Error = errorString
-		if len(errorString) > constants.MaxBriefErrLength {
-			store.Status.BriefError = fmt.Sprintf("%s...", errorString[:constants.MaxBriefErrLength])
-		}
+		store.Status.Error = err.Error()
+		store.Status.BriefError = err.GetConciseError(constants.MaxBriefErrLength)
 	}
 
 	if statusErr := r.Status().Update(ctx, store); statusErr != nil {

pkg/controllers/clusterresource/store_controller_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	configv1beta1 "github.com/ratify-project/ratify/api/v1beta1"
+	re "github.com/ratify-project/ratify/errors"
 	"github.com/ratify-project/ratify/internal/constants"
 	"github.com/ratify-project/ratify/pkg/controllers"
 	rs "github.com/ratify-project/ratify/pkg/customresources/referrerstores"
@@ -120,7 +121,8 @@ func TestWriteStoreStatus(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(_ *testing.T) {
-			writeStoreStatus(context.Background(), tc.reconciler, tc.store, logger, tc.isSuccess, tc.errString)
+			err := re.ErrorCodeUnknown.WithDetail(tc.errString)
+			writeStoreStatus(context.Background(), tc.reconciler, tc.store, logger, tc.isSuccess, &err)
 		})
 	}
 }
@@ -238,6 +240,21 @@ func TestStoreReconcile(t *testing.T) {
 			expectedErr:        true,
 			expectedStoreCount: 0,
 		},
+		{
+			name: "unsupported store",
+			store: &configv1beta1.Store{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: constants.EmptyNamespace,
+					Name:      storeName,
+				},
+				Spec: configv1beta1.StoreSpec{
+					Name:    "unsupported",
+					Address: dirPath,
+				},
+			},
+			expectedErr:        true,
+			expectedStoreCount: 0,
+		},
 	}
 
 	for _, tt := range tests {

pkg/controllers/clusterresource/verifier_controller.go

@@ -17,9 +17,9 @@ package clusterresource
 
 import (
 	"context"
-	"fmt"
 
 	configv1beta1 "github.com/ratify-project/ratify/api/v1beta1"
+	re "github.com/ratify-project/ratify/errors"
 	"github.com/ratify-project/ratify/internal/constants"
 	"github.com/ratify-project/ratify/pkg/controllers"
 
@@ -70,12 +70,13 @@ func (r *VerifierReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	}
 
 	if err := verifierAddOrReplace(verifier.Spec, resource); err != nil {
-		verifierLogger.Error(err, "unable to create verifier from verifier crd")
-		writeVerifierStatus(ctx, r, &verifier, verifierLogger, false, err.Error())
-		return ctrl.Result{}, err
+		verifierErr := re.ErrorCodePluginInitFailure.WithError(err).WithDetail("unable to create verifier from verifier crd")
+		verifierLogger.Error(err)
+		writeVerifierStatus(ctx, r, &verifier, verifierLogger, false, &verifierErr)
+		return ctrl.Result{}, verifierErr
 	}
 
-	writeVerifierStatus(ctx, r, &verifier, verifierLogger, true, "")
+	writeVerifierStatus(ctx, r, &verifier, verifierLogger, true, nil)
 
 	// returning empty result and no error to indicate we’ve successfully reconciled this object
 	return ctrl.Result{}, nil
@@ -85,8 +86,9 @@ func (r *VerifierReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 func verifierAddOrReplace(spec configv1beta1.VerifierSpec, objectName string) error {
 	verifierConfig, err := cutils.SpecToVerifierConfig(spec.Parameters.Raw, objectName, spec.Name, spec.ArtifactTypes, spec.Source)
 	if err != nil {
-		logrus.Error(err, "unable to convert crd specification to verifier config")
-		return fmt.Errorf("unable to convert crd specification to verifier config, err: %w", err)
+		verifierErr := re.ErrorCodePluginInitFailure.WithError(err).WithDetail("Unable to create verifier from verifier crd.")
+		logrus.Error(verifierErr)
+		return verifierErr
 	}
 
 	return cutils.UpsertVerifier(spec.Version, spec.Address, constants.EmptyNamespace, objectName, verifierConfig)
@@ -99,17 +101,15 @@ func (r *VerifierReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func writeVerifierStatus(ctx context.Context, r client.StatusClient, verifier *configv1beta1.Verifier, logger *logrus.Entry, isSuccess bool, errorString string) {
+func writeVerifierStatus(ctx context.Context, r client.StatusClient, verifier *configv1beta1.Verifier, logger *logrus.Entry, isSuccess bool, err *re.Error) {
 	if isSuccess {
 		verifier.Status.IsSuccess = true
 		verifier.Status.Error = ""
 		verifier.Status.BriefError = ""
 	} else {
 		verifier.Status.IsSuccess = false
-		verifier.Status.Error = errorString
-		if len(errorString) > constants.MaxBriefErrLength {
-			verifier.Status.BriefError = fmt.Sprintf("%s...", errorString[:constants.MaxBriefErrLength])
-		}
+		verifier.Status.Error = err.Error()
+		verifier.Status.BriefError = err.GetConciseError(constants.MaxBriefErrLength)
 	}
 
 	if statusErr := r.Status().Update(ctx, verifier); statusErr != nil {

pkg/controllers/clusterresource/verifier_controller_test.go

@@ -23,6 +23,7 @@ import (
 	"testing"
 
 	configv1beta1 "github.com/ratify-project/ratify/api/v1beta1"
+	re "github.com/ratify-project/ratify/errors"
 	"github.com/ratify-project/ratify/internal/constants"
 	"github.com/ratify-project/ratify/pkg/controllers"
 	"github.com/ratify-project/ratify/pkg/customresources/verifiers"
@@ -189,11 +190,12 @@ func TestVerifier_UpdateAndDelete(t *testing.T) {
 func TestWriteVerifierStatus(t *testing.T) {
 	logger := logrus.WithContext(context.Background())
 	testCases := []struct {
-		name       string
-		isSuccess  bool
-		verifier   *configv1beta1.Verifier
-		errString  string
-		reconciler client.StatusClient
+		name              string
+		isSuccess         bool
+		verifier          *configv1beta1.Verifier
+		errString         string
+		expectedErrString string
+		reconciler        client.StatusClient
 	}{
 		{
 			name:       "success status",
@@ -203,11 +205,12 @@ func TestWriteVerifierStatus(t *testing.T) {
 			reconciler: &mockStatusClient{},
 		},
 		{
-			name:       "error status",
-			isSuccess:  false,
-			verifier:   &configv1beta1.Verifier{},
-			errString:  "a long error string that exceeds the max length of 30 characters",
-			reconciler: &mockStatusClient{},
+			name:              "error status",
+			isSuccess:         false,
+			verifier:          &configv1beta1.Verifier{},
+			errString:         "a long error string that exceeds the max length of 30 characters",
+			expectedErrString: "UNKNOWN: a long error string that exceeds the max length of 30 characters",
+			reconciler:        &mockStatusClient{},
 		},
 		{
 			name:      "status update failed",
@@ -221,14 +224,15 @@ func TestWriteVerifierStatus(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			writeVerifierStatus(context.Background(), tc.reconciler, tc.verifier, logger, tc.isSuccess, tc.errString)
+			ratifyErr := re.ErrorCodeUnknown.WithDetail(tc.errString)
+			writeVerifierStatus(context.Background(), tc.reconciler, tc.verifier, logger, tc.isSuccess, &ratifyErr)
 
 			if tc.verifier.Status.IsSuccess != tc.isSuccess {
 				t.Fatalf("Expected isSuccess to be %+v , actual %+v", tc.isSuccess, tc.verifier.Status.IsSuccess)
 			}
 
-			if tc.verifier.Status.Error != tc.errString {
-				t.Fatalf("Expected Error to be %+v , actual %+v", tc.errString, tc.verifier.Status.Error)
+			if tc.verifier.Status.Error != tc.expectedErrString {
+				t.Fatalf("Expected Error to be %+v , actual %+v", tc.expectedErrString, tc.verifier.Status.Error)
 			}
 		})
 	}
@@ -278,6 +282,21 @@ func TestVerifierReconcile(t *testing.T) {
 			expectedErr:           true,
 			expectedVerifierCount: 0,
 		},
+		{
+			name: "unsupported verifier",
+			verifier: &configv1beta1.Verifier{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "",
+					Name:      verifierName,
+				},
+				Spec: configv1beta1.VerifierSpec{
+					Name:    "unsupported",
+					Address: dirPath,
+				},
+			},
+			expectedErr:           true,
+			expectedVerifierCount: 0,
+		},
 		{
 			name: "valid spec",
 			verifier: &configv1beta1.Verifier{

pkg/controllers/namespaceresource/certificatestore_controller_test.go

@@ -17,7 +17,6 @@ package namespaceresource
 
 import (
 	"context"
-	"fmt"
 	"testing"
 
 	configv1beta1 "github.com/ratify-project/ratify/api/v1beta1"
@@ -106,9 +105,8 @@ func TestUpdateErrorStatus(t *testing.T) {
 	if certStore.Status.Error != expectedErr {
 		t.Fatalf("Unexpected error string, expected %+v, got %+v", expectedErr, certStore.Status.Error)
 	}
-	expectedBriedErr := fmt.Sprintf("%s...", expectedErr[:30])
-	if certStore.Status.BriefError != expectedBriedErr {
-		t.Fatalf("Unexpected error string, expected %+v, got %+v", expectedBriedErr, certStore.Status.Error)
+	if certStore.Status.BriefError != expectedErr {
+		t.Fatalf("Unexpected error string, expected %+v, got %+v", expectedErr, certStore.Status.Error)
 	}
 
 	//make sure properties of last cached cert was not overridden