Repository: deislabs/ratify
Pull request #1408 (2024-04-18 13:38:34)

fix: update ReferrerNotFound error to be more accurate
# Description

## What this PR does / why we need it:
1. Modify ErrorCodeReferrersNotFound to ErrorCodeEmptyVerifierReport as it aligns better with the actual issue.
2. Revise the description of the error to list potential underlying causes.
3. Add `WithDescription` to custom error for improved printout capability.

![image](https://github.com/deislabs/ratify/assets/6919333/a10cdabb-4d55-4cfd-aeca-de15740e8970)


## Which issue(s) this PR fixes *(optional, using `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when the PR gets merged)*:
Fixing one error in: https://github.com/deislabs/ratify/issues/1321

## Type of change

Please delete options that are not relevant.

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Helm Chart Change (any edit/addition/update that is necessary for changes merged to the `main` branch)
- [ ] This change requires a documentation update

# How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

- [ ] Test A
- [ ] Test B

# Checklist:

- [ ] Does the affected code have corresponding tests?
- [ ] Are the changes documented, not just with inline documentation, but also with conceptual documentation such as an overview of a new feature, or task-based documentation like a tutorial? Consider if this change should be announced on your project blog.
- [ ] Does this introduce breaking changes that would require an announcement or bumping the major version?
- [ ] Do all new files have appropriate license header?

# Post Merge Requirements
- [ ] MAINTAINERS: manually trigger the "Publish Package" workflow after merging any PR that indicates `Helm Chart Change`


binbin-li: @yizha1 do you think if the new description makes sense to customers? Any suggestions for improvement?
binbin-li: synced with Yi offline, rename the error code to `ErrorCodeNoVerifierReport`
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/deislabs/ratify/pull/1408?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deislabs) Report
All modified and coverable lines are covered by tests :white_check_mark:
> :exclamation: No coverage uploaded for pull request base (`dev@cc7d780`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deislabs#section-missing-base-commit).


<details><summary>Additional details and impacted files</summary>


```diff
@@          Coverage Diff           @@
##             dev    #1408   +/-   ##
======================================
  Coverage       ?   65.39%           
======================================
  Files          ?       92           
  Lines          ?     4869           
  Branches       ?        0           
======================================
  Hits           ?     3184           
  Misses         ?     1399           
  Partials       ?      286           
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/deislabs/ratify/pull/1408?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deislabs).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deislabs).


Patch:
diff --git a/errors/pluginerrors.go b/errors/pluginerrors.go
index cc851e1..0ec7f4c 100644
--- a/errors/pluginerrors.go
+++ b/errors/pluginerrors.go
@@ -105,11 +105,11 @@ var (
 		Description: `The manifest is invalid. Please validate the manifest is correctly formatted.`,
 	})
 
-	// ErrorCodeReferrersNotFound is returned if there is no ReferrerStore set.
-	ErrorCodeReferrersNotFound = Register("errcode", ErrorDescriptor{
-		Value:       "REFERRERS_NOT_FOUND",
-		Message:     "referrers not found",
-		Description: "No referrers are found. Please verify the subject has attached expected artifacts and refer to https://ratify.dev/docs/reference/store/ to investigate Referrer Store configuration.",
+	// ErrorCodeNoVerifierReport is returned if there is no ReferrerStore set.
+	ErrorCodeNoVerifierReport = Register("errcode", ErrorDescriptor{
+		Value:       "NO_VERIFIER_REPORT",
+		Message:     "no verifier report",
+		Description: "No verifier report was generated. This might be due to various factors, such as lack of artifacts attached to the image, a misconfiguration in the Referrer Store preventing access to the registry, or the absence of appropriate verifiers corresponding to the referenced image artifacts.",
 	})
 
 	// Generic errors happen in plugins
diff --git a/errors/types.go b/errors/types.go
index 8b4529e..3a211c8 100644
--- a/errors/types.go
+++ b/errors/types.go
@@ -68,6 +68,7 @@ type Error struct {
 	PluginName    string        `json:"pluginName,omitempty"`
 	LinkToDoc     string        `json:"linkToDoc,omitempty"`
 	Stack         string        `json:"stack,omitempty"`
+	Description   string        `json:"description,omitempty"`
 }
 
 // ErrorDescriptor provides relevant information about a given error code.
@@ -124,6 +125,11 @@ func (ec ErrorCode) Message() string {
 	return ec.Descriptor().Message
 }
 
+// Description returned the description of this error code.
+func (ec ErrorCode) Description() string {
+	return ec.Descriptor().Description
+}
+
 // String returns the canonical identifier for this error code.
 func (ec ErrorCode) String() string {
 	return ec.Descriptor().Value
@@ -149,6 +155,10 @@ func (ec ErrorCode) WithLinkToDoc(link string) Error {
 	return newError(ec, ec.Message()).WithLinkToDoc(link)
 }
 
+func (ec ErrorCode) WithDescription() Error {
+	return newError(ec, ec.Message()).WithDescription()
+}
+
 // WithPluginName returns a new Error object with pluginName set.
 func (ec ErrorCode) WithPluginName(pluginName string) Error {
 	return newError(ec, ec.Message()).WithPluginName(pluginName)
@@ -223,6 +233,10 @@ func (e Error) Error() string {
 		errStr += fmt.Sprintf(", Detail: %v", e.Detail)
 	}
 
+	if e.Description != "" {
+		errStr += fmt.Sprintf(", Description: %v", e.Description)
+	}
+
 	if e.Stack != "" {
 		errStr += fmt.Sprintf(", Stack trace: %s", e.Stack)
 	}
@@ -261,6 +275,11 @@ func (e Error) WithLinkToDoc(link string) Error {
 	return e
 }
 
+func (e Error) WithDescription() Error {
+	e.Description = e.Code.Description()
+	return e
+}
+
 // IsEmpty returns true if the error is empty.
 func (e Error) IsEmpty() bool {
 	return e == emptyError
diff --git a/pkg/executor/core/executor.go b/pkg/executor/core/executor.go
index 08e89be..f1cec9d 100644
--- a/pkg/executor/core/executor.go
+++ b/pkg/executor/core/executor.go
@@ -83,7 +83,7 @@ func (executor Executor) verifySubjectInternal(ctx context.Context, verifyParame
 	}
 	if executor.PolicyEnforcer.GetPolicyType(ctx) == pt.ConfigPolicy {
 		if len(verifierReports) == 0 {
-			return types.VerifyResult{}, errors.ErrorCodeReferrersNotFound.WithComponentType(errors.Executor)
+			return types.VerifyResult{}, errors.ErrorCodeNoVerifierReport.WithComponentType(errors.Executor).WithDescription()
 		}
 	}
 	// If it requires embedded Rego Policy Engine make the decision, execute


Test patch:
diff --git a/errors/types_test.go b/errors/types_test.go
index 4dc5dd6..5e97400 100644
--- a/errors/types_test.go
+++ b/errors/types_test.go
@@ -126,6 +126,13 @@ func TestWithPluginName(t *testing.T) {
 	}
 }
 
+func TestWithDescription(t *testing.T) {
+	err := testEC.WithDescription()
+	if err.Description != testDescription {
+		t.Fatalf("expected description: %s, got: %s", testDescription, err.Description)
+	}
+}
+
 func TestIs(t *testing.T) {
 	err := testEC.WithDetail(testDetail)
 	result := err.Is(err)
@@ -161,7 +168,7 @@ func TestIsEmpty(t *testing.T) {
 }
 
 func TestError_Error(t *testing.T) {
-	err := testEC.WithPluginName(testPluginName).WithComponentType(testComponentType).WithLinkToDoc(testLink).WithDetail(testDetail).WithError(Error{})
+	err := testEC.WithPluginName(testPluginName).WithComponentType(testComponentType).WithLinkToDoc(testLink).WithDetail(testDetail).WithError(Error{}).WithDescription()
 	result := err.Error()
 	if !strings.HasPrefix(result, testErrorString) {
 		t.Fatalf("expected string starts with: %s, but got: %s", testErrorString, result)
diff --git a/pkg/executor/core/executor_test.go b/pkg/executor/core/executor_test.go
index 36edaff..a4fbb15 100644
--- a/pkg/executor/core/executor_test.go
+++ b/pkg/executor/core/executor_test.go
@@ -188,7 +188,7 @@ func TestVerifySubjectInternal_ResolveSubjectDescriptor_Success(t *testing.T) {
 		Subject: "localhost:5000/net-monitor:v1",
 	}
 
-	if _, err := executor.verifySubjectInternal(context.Background(), verifyParameters); !errors.Is(err, ratifyerrors.ErrorCodeReferrersNotFound.WithDetail("")) {
+	if _, err := executor.verifySubjectInternal(context.Background(), verifyParameters); !errors.Is(err, ratifyerrors.ErrorCodeNoVerifierReport.WithDetail("")) {
 		t.Fatalf("expected ErrReferrersNotFound actual %v", err)
 	}
 }
@@ -214,7 +214,7 @@ func TestVerifySubjectInternal_Verify_NoReferrers(t *testing.T) {
 		Subject: "localhost:5000/net-monitor:v1",
 	}
 
-	if _, err := ex.verifySubjectInternal(context.Background(), verifyParameters); !errors.Is(err, ratifyerrors.ErrorCodeReferrersNotFound.WithDetail("")) {
+	if _, err := ex.verifySubjectInternal(context.Background(), verifyParameters); !errors.Is(err, ratifyerrors.ErrorCodeNoVerifierReport.WithDetail("")) {
 		t.Fatalf("expected ErrReferrersNotFound actual %v", err)
 	}
 }