diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index d6a8bfc60..c4c55f106 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -38,7 +38,7 @@ jobs: with: go-version: "1.22" - name: Initialize CodeQL - uses: github/codeql-action/init@afb54ba388a7dca6ecae48f608c4ff05ff4cc77a # tag=v3.25.15 + uses: github/codeql-action/init@eb055d739abdc2e8de2e5f4ba1a8b246daa779aa # tag=v3.26.0 with: languages: go - name: Run tidy @@ -46,4 +46,4 @@ jobs: - name: Build CLI run: make build - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@afb54ba388a7dca6ecae48f608c4ff05ff4cc77a # tag=v3.25.15 + uses: github/codeql-action/analyze@eb055d739abdc2e8de2e5f4ba1a8b246daa779aa # tag=v3.26.0 diff --git a/.github/workflows/e2e-aks.yml b/.github/workflows/e2e-aks.yml index 17ed2112a..30dced1e8 100644 --- a/.github/workflows/e2e-aks.yml +++ b/.github/workflows/e2e-aks.yml @@ -64,7 +64,7 @@ jobs: make e2e-aks KUBERNETES_VERSION=${{ inputs.k8s_version }} GATEKEEPER_VERSION=${{ inputs.gatekeeper_version }} TENANT_ID=${{ secrets.AZURE_TENANT_ID }} AZURE_SP_OBJECT_ID=${{ secrets.AZURE_SP_OBJECT_ID }} - name: Upload artifacts - uses: actions/upload-artifact@89ef406dd8d7e03cfd12d9e0a4a378f454709029 # v4.3.5 + uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6 if: ${{ always() }} with: name: e2e-logs-aks-${{ inputs.k8s_version }}-${{ inputs.gatekeeper_version }} diff --git a/.github/workflows/e2e-k8s.yml b/.github/workflows/e2e-k8s.yml index bd03c2dd7..06d97d5ef 100644 --- a/.github/workflows/e2e-k8s.yml +++ b/.github/workflows/e2e-k8s.yml @@ -65,7 +65,7 @@ jobs: kubectl logs -n gatekeeper-system -l app=ratify --tail=-1 > logs-ratify-preinstall-${{ matrix.KUBERNETES_VERSION }}-${{ matrix.GATEKEEPER_VERSION }}-rego-policy.json kubectl logs -n gatekeeper-system -l app.kubernetes.io/name=ratify --tail=-1 > logs-ratify-${{ matrix.KUBERNETES_VERSION }}-${{ matrix.GATEKEEPER_VERSION }}-rego-policy.json - name: Upload artifacts - uses: actions/upload-artifact@89ef406dd8d7e03cfd12d9e0a4a378f454709029 # v4.3.5 + uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6 if: ${{ always() }} with: name: e2e-logs-${{ inputs.k8s_version }}-${{ inputs.gatekeeper_version }} diff --git a/.github/workflows/high-availability.yml b/.github/workflows/high-availability.yml index a0cbeaeb1..09d3fe236 100644 --- a/.github/workflows/high-availability.yml +++ b/.github/workflows/high-availability.yml @@ -60,7 +60,7 @@ jobs: kubectl logs -n gatekeeper-system -l app=ratify --tail=-1 > logs-ratify-preinstall-${{ matrix.DAPR_VERSION }}.json kubectl logs -n gatekeeper-system -l app.kubernetes.io/name=ratify --tail=-1 > logs-ratify-${{ matrix.DAPR_VERSION }}.json - name: Upload artifacts - uses: actions/upload-artifact@89ef406dd8d7e03cfd12d9e0a4a378f454709029 # v4.3.5 + uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6 if: ${{ always() }} with: name: e2e-logs-${{ matrix.DAPR_VERSION }} diff --git a/.github/workflows/publish-cosign-sample.yml b/.github/workflows/publish-cosign-sample.yml index 2c5a7d7f6..925284a8c 100644 --- a/.github/workflows/publish-cosign-sample.yml +++ b/.github/workflows/publish-cosign-sample.yml @@ -25,7 +25,7 @@ jobs: egress-policy: audit - name: Install cosign - uses: sigstore/cosign-installer@59acb6260d9c0ba8f4a2f9d9b48431a222b68e20 # v3.5.0 + uses: sigstore/cosign-installer@4959ce089c160fddf62f7b42464195ba1a56d382 # v3.6.0 - name: Get repo run: | diff --git a/.github/workflows/publish-dev-assets.yml b/.github/workflows/publish-dev-assets.yml index b7b294c53..c5dc1ef6a 100644 --- a/.github/workflows/publish-dev-assets.yml +++ b/.github/workflows/publish-dev-assets.yml @@ -25,7 +25,7 @@ jobs: - name: Install Notation uses: notaryproject/notation-action/setup@104aa999103172f827373af8ac14dde7aa6d28f1 # v1.1.0 - name: Install cosign - uses: sigstore/cosign-installer@59acb6260d9c0ba8f4a2f9d9b48431a222b68e20 # v3.5.0 + uses: sigstore/cosign-installer@4959ce089c160fddf62f7b42464195ba1a56d382 # v3.6.0 - name: Az CLI login uses: azure/login@6c251865b4e6290e7b78be643ea2d005bc51f69a # v2.1.1 with: diff --git a/.github/workflows/quick-start.yml b/.github/workflows/quick-start.yml index 2ceda4e8e..ca981e2b5 100644 --- a/.github/workflows/quick-start.yml +++ b/.github/workflows/quick-start.yml @@ -59,7 +59,7 @@ jobs: kubectl logs -n gatekeeper-system -l app=ratify --tail=-1 > logs-ratify-preinstall-${{ matrix.KUBERNETES_VERSION }}-config-policy.json kubectl logs -n gatekeeper-system -l app.kubernetes.io/name=ratify --tail=-1 > logs-ratify-${{ matrix.KUBERNETES_VERSION }}-config-policy.json - name: Upload artifacts - uses: actions/upload-artifact@89ef406dd8d7e03cfd12d9e0a4a378f454709029 # v4.3.5 + uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6 if: ${{ always() }} with: name: e2e-logs-${{ matrix.KUBERNETES_VERSION }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 986eb83c1..28769e410 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -49,7 +49,7 @@ jobs: $RUNNER_TEMP/sbom-tool generate -b . -bc . -pn ratify -pv $GITHUB_REF_NAME -ps Microsoft -nsb https://microsoft.com -V Verbose - name: Upload a Build Artifact - uses: actions/upload-artifact@89ef406dd8d7e03cfd12d9e0a4a378f454709029 # tag=v4.3.5 + uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # tag=v4.3.6 with: name: SBOM SPDX files path: _manifest/spdx_2.2/** diff --git a/.github/workflows/scorecards.yml b/.github/workflows/scorecards.yml index 73c211a6b..838e6d546 100644 --- a/.github/workflows/scorecards.yml +++ b/.github/workflows/scorecards.yml @@ -8,10 +8,12 @@ on: branches: - main - dev + - release-* pull_request: branches: - dev - main + - release-* workflow_dispatch: permissions: read-all @@ -46,13 +48,13 @@ jobs: publish_results: true - name: "Upload artifact" - uses: actions/upload-artifact@89ef406dd8d7e03cfd12d9e0a4a378f454709029 # tag=v4.3.5 + uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # tag=v4.3.6 with: name: SARIF file path: results.sarif retention-days: 5 - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@afb54ba388a7dca6ecae48f608c4ff05ff4cc77a # tag=v3.25.15 + uses: github/codeql-action/upload-sarif@eb055d739abdc2e8de2e5f4ba1a8b246daa779aa # tag=v3.26.0 with: sarif_file: results.sarif diff --git a/cmd/ratify/cmd/cmd_test.go b/cmd/ratify/cmd/cmd_test.go index 49149dcf5..9016198ab 100644 --- a/cmd/ratify/cmd/cmd_test.go +++ b/cmd/ratify/cmd/cmd_test.go @@ -50,8 +50,8 @@ func TestDiscover(t *testing.T) { // TODO: make ratify cli more unit testable // unit test should not need to resolve real image - if !strings.Contains(err.Error(), "referrer store failure") { - t.Errorf("error expected") + if !strings.Contains(err.Error(), "REFERRER_STORE_FAILURE") { + t.Errorf("expected containing: %s, but got: %s", "REFERRER_STORE_FAILURE", err.Error()) } } diff --git a/docs/proposals/Error-Messages-Improvements.md b/docs/proposals/Error-Messages-Improvements.md new file mode 100644 index 000000000..d01b54e06 --- /dev/null +++ b/docs/proposals/Error-Messages-Improvements.md @@ -0,0 +1,135 @@ +# Error messages improvements + +## Problem/Motivation + +Error messages are crucial because they provide specific information about what went wrong, helping users quickly identify and resolve issues. Detailed error messages can pinpoint the exact part of the configuration or code that caused the problem. Error messages can include links or references to documentation, guiding users on how to fix the issue promptly. While testing the Ratify policy for signature verification, we noticed that some error messages were difficult to comprehend from the user perspective. + +Error message example 1: + +```text +time=2024-07-17T16:28:16.939576441Z level=warning msg=Original Error: (Original Error: (HEAD "https:/ +roacr.azurecr.io/v2/net-monitor/manifests/v2": GET "https://roacr.azurecr.io/oauth2/token? +scope=repository%3Anet-monitor%3Apull&service=roacr.azurecr.io": response status code 401: unauthorized: +authentication required, visit https://aka.ms/acr/authorization for more information.), Error: repository +operation failure, Code: REPOSITORY_OPERATION_FAILURE, Plugin Name: oras), Error: get subject descriptor +failure, Code: GET_SUBJECT_DESCRIPTOR_FAILURE, Plugin Name: oras, Component Type: referrerStore, Detail: +failed to resolve the subject descriptor component-type=referrerStore go.version=go1.21.10 namespace= +trace-id=34b27888-5402-443e-9836-77124c840561 +``` + +The above example indicated the an error happened, however, a warning level was set for the log. The error message was set to the field `msg`. It contained nested errors. The first original error message correctly described the source of the problem "401 unauthorized" and pointed to a document for resolution, however, errors following the original error were redundant and not well formatted, thus complicated the overall message. The overall message failed to describe the context of the error, although "401 unauthorized" explained the reason, but did this error happen during signature verification or else? What does the `subject` mean? + +Error message example 2: + +```text +"verifierReports": [ + { + "subject": "docker.io/library/hello-world@sha256:1408fec50309afee38f3535383f5b09419e6dc0925bc69891e79d84cc4cdce6", + "isSuccess": false, + "message": "verification failed: Error: no verifier report, Code: NO_VERIFIER _REPORT, Component Type: + executor, Description: No verifier report was generatec preventing access to the registry, or the absence + of appropriate verifiers corresponding to the referenced image artifacts." + } +] +``` + +When Ratify completes artifact verification, the result is returned to the policy engine in the format of the json object `verifierReports`. The `verifierReports` is also recorded in an INFO log of Ratify. If `isSuccess` field is set to `false`, the `message` field is set to error messages. In above example, the message is not well formatted and lacked clarity on the problem, its cause, and remediation methods. It's hard for users to understand in what context the error happened and what users need to do. For example, is it an error for signature verification? What does "no verifier report" mean? We also observed that the `verifierReports` contains different supported fields when compared with the Ratify Config policy and Ratify Rego policy, which is inconsistent. + +Error message example 3: + +```text +Error from server (Forbidden): admission webhook "validation.gatekeeper.sh" denied the request: +[ratify-constraint] Subject failed verification: huishwabbit1.azurecr.io/ +test8may24@sha256:c780036bc8a6f577910bf01151013aaa18e255057a1653c76d8f3572aa3f6ff6 +``` + +The policy engine, for instance, Gatekeeper, has produced the above error message using a constraint template supplied by Ratify. It is the responsibility of the policy engine to tailor the constraint template for proper error messages to their requirements, however, it requires Ratify to provided useful verification reports as data inputs. In this example, the error message is not clear to users regarding the meaning of the term `Subject`, and fails to specify the context of failure, such as whether it was related to signature verification or SBOM verification or other verifications. Additionally, reasons behind the error were not provided. Furthermore, users may not be able to locate this error in the complete K8s logs to view more logs during error happened, because only artifact digest was shown and it is not enough to pinpoint the exact error in K8s logs. + +Further findings covering a range of cases such as Key Management Provider (KMP), Store, Verifier, Policy configuration, access control, and signature verification issues are recorded at [Ratify Error Handling Scenarios.md](../discussion/Ratify%20Error%20Handling%20Scenarios.md) + +In summary, the areas that need enhancement include: + +- Error messages similar to the first example will appear in Ratify logs. These may be found in the logs of Ratify Pods if Ratify is set up as a Kubernetes service, or they might be output by the Ratify CLI. The primary concerns include excessive nested errors, lacking error context, and not user-friendly error descriptions. +- Error messages contained within `verifierReports` that are sent back to the policy engine, as seen in the second example. These error messages share issues similar to those mentioned for the first example. The policy engine can customize their Rego policies using the messages inside the `verifierReports` to return to users in the logs or UI of the policy engine. +- Ratify failed to provide sufficient information for the policy engine to generate error messages, which is demonstrated in the third example. + +The document aims to provide solutions and guidelines to improve error messages. + +## Scenarios + +### Error messages displayed in the Ratify logs + +Alice works as a DevOps engineer at Contoso. She set up tasks to deploy containerized apps into Kubernetes clusters. The cluster is assigned with the policy to deny the deployment of images that don't pass policy evaluation including verification of signature, SBOM, vulnerability reports and other image metadata. Alice knows that behind the scene, it is the Ratify conduct the verification and returned results as reports to the policy engine. When policy evaluation fails, Alice sees clear and actionable error messages in Ratify logs. The error messages contain concise error descriptions, error reasons and error recommendations, allowing her to act on errors promptly. + +### Error messages displayed in verification reports used by the policy engine + +Bob is a software engineer on Contoso's Policy team, writing policies used during admissions in Kubernetes clusters. These policies evaluate images based on verifier reports generated by Ratify. If policy evaluation fails, Ratify sends back the reports with error messages to the policy engine. The reports, in JSON format, provide structured error messages that Bob utilizes to create clear and actionable error messages for the policy engine. These messages include concise error descriptions, error reasons, and error recommendations, allowing policy users to act on errors promptly. + +### Error messages returned by Ratify CLI commands + +Gina is a software engineer on the CI/CD team at Contoso, where she creates pipeline tasks incorporating Ratify CLI commands to assess artifacts according to policies. Should a policy check not pass, the corresponding artifacts are prevented from progressing in the pipeline. When policy evaluation fails, Gina sees concise, clear and actionable error messages returned by Ratify CLI commands. The error messages contain concise error descriptions, error reasons and error recommendations, allowing her to act on errors promptly. + +## Proposed solutions + +We won’t create new error message guidelines; instead, we’ll refer to the existing ones. [Azure CLI Error Handling Guidelines](https://github.com/Azure/azure-cli/blob/dev/doc/error_handling_guidelines.md#error-message) outlined a general pattern for error messages, consisting of: + +1. __What the error is.__ +2. __Why it happens.__ +3. __What users need to do to fix it.__ + +The proposed improvements for Ratify error messages adhere to this general pattern and the detailed DOs and DON'Ts provided in the guidelines. The error message will also include an error code. Since Ratify already supports a list of error codes, these can be used to search for remediation in the troubleshooting guide. For example, search error code `CERT_INVALID` in [the troubleshooting guide](https://ratify.dev/docs/troubleshoot/key-management-provider/kmp-tsg#cert_invalid). + +The recommended format for an error message in the Ratify log is as following. + +```text +": : : " +``` + +For the error messages displayed in `verifierReports`, it is recommended to add two new optional fields `errorReason` and `remediation`, which will be used when the field `isSuccess` is set to `false`: + +```text +"verifierReports": [ + { + "subject": "" + "isSuccess": false, + "message": "", + "errorReason": "", + "remediation": "" + } +] +``` + +## Examples + +### Error messages displayed in the Ratify logs or returned by Ratify CLI commands + +For the above first example, the error message in the Ratify log can be improved to: + +```text +REPOSITORY_OPERATION_FAILURE: Failed to resolve the artifact descriptor: HEAD "https://roacr.azurecr.io/v2/net-monitor/manifests/v2": GET "https://roacr.azurecr.io/oauth2/token? +scope=repository%3Anet-monitor%3Apull&service=roacr.azurecr.io": response status code 401: unauthorized: +authentication required, visit https://aka.ms/acr/authorization for more information. +``` + +### Error messages displayed in `verifierReports` + +For the second example, the error message can be improved to: + +```text +"verifierReports": [ + { + "subject": "docker.io/library/hello-world@sha256:1408fec50309afee38f3535383f5b09419e6dc0925bc69891e79d84cc4cdce6", + "isSuccess": false, + "message": "NO_VERIFIER_REPORT: Failed to verify artifact docker.io/library/hello-world@sha256:1408fec50309afee38f3535383f5b09419e6dc0925bc69891e79d84cc4cdce6: + "errorReason": "No signature is found or wrong configuration" + "remediation": "Please either sign the artifact or configure verifiers for signature verification. Learn more at https://ratify.dev/docs/plugins/verifier/notation." + } +] +``` + +> This link https://ratify.dev/docs/plugins/verifier/notation is used as an example to illustrate the improvements. The link should vary depending on the particular error encountered. + +## References + +- [Azure CLI Error Handling Guidelines](https://github.com/Azure/azure-cli/blob/dev/doc/error_handling_guidelines.md) +- [ORAS CLI Error Handling and Message Guideline](https://github.com/oras-project/oras/blob/v1.2.0/docs/proposals/error-handling-guideline.md) diff --git a/errors/types.go b/errors/types.go index 3a211c80a..cb5e0311b 100644 --- a/errors/types.go +++ b/errors/types.go @@ -60,15 +60,16 @@ type ErrorCode int // Error provides a wrapper around ErrorCode with extra Details provided. type Error struct { - OriginalError error `json:"originalError,omitempty"` - Code ErrorCode `json:"code"` - Message string `json:"message"` - Detail interface{} `json:"detail,omitempty"` - ComponentType ComponentType `json:"componentType,omitempty"` - PluginName string `json:"pluginName,omitempty"` - LinkToDoc string `json:"linkToDoc,omitempty"` - Stack string `json:"stack,omitempty"` - Description string `json:"description,omitempty"` + originalError error + code ErrorCode + message string + detail interface{} + componentType ComponentType + remediation string + pluginName string + stack string + description string + isRootError bool // isRootError is true if the originalError is either nil or not an Error type } // ErrorDescriptor provides relevant information about a given error code. @@ -150,9 +151,9 @@ func (ec ErrorCode) WithComponentType(componentType ComponentType) Error { return newError(ec, ec.Message()).WithComponentType(componentType) } -// WithLinkToDoc returns a new Error object with attached link to the documentation. -func (ec ErrorCode) WithLinkToDoc(link string) Error { - return newError(ec, ec.Message()).WithLinkToDoc(link) +// WithRemediation returns a new Error object with remediation. +func (ec ErrorCode) WithRemediation(link string) Error { + return newError(ec, ec.Message()).WithRemediation(link) } func (ec ErrorCode) WithDescription() Error { @@ -165,27 +166,29 @@ func (ec ErrorCode) WithPluginName(pluginName string) Error { } // NewError returns a new Error object. -func (ec ErrorCode) NewError(componentType ComponentType, pluginName, link string, err error, detail interface{}, printStackTrace bool) Error { +func (ec ErrorCode) NewError(componentType ComponentType, pluginName, remediation string, err error, detail interface{}, printStackTrace bool) Error { stack := "" if printStackTrace { stack = getStackTrace() } return Error{ - Code: ec, - Message: ec.Message(), - OriginalError: err, - Detail: detail, - ComponentType: componentType, - PluginName: pluginName, - LinkToDoc: link, - Stack: stack, + code: ec, + message: ec.Message(), + originalError: err, + detail: detail, + componentType: componentType, + pluginName: pluginName, + remediation: remediation, + stack: stack, + isRootError: err == nil || !errors.As(err, &Error{}), } } func newError(code ErrorCode, message string) Error { return Error{ - Code: code, - Message: message, + code: code, + message: message, + isRootError: true, } } @@ -193,90 +196,114 @@ func newError(code ErrorCode, message string) Error { func (e Error) Is(target error) bool { t := &Error{} if errors.As(target, t) { - return e.Code.ErrorCode() == t.Code.ErrorCode() + return e.code.ErrorCode() == t.code.ErrorCode() } return false } // ErrorCode returns the ID/Value of this Error func (e Error) ErrorCode() ErrorCode { - return e.Code + return e.code } // Unwrap returns the original error func (e Error) Unwrap() error { - return e.OriginalError + return e.originalError } // Error returns a human readable representation of the error. +// An Error message includes the error code, detail from nested errors, root cause and remediation, all separated by ": ". func (e Error) Error() string { - var errStr string - if e.OriginalError != nil { - errStr += fmt.Sprintf("Original Error: (%s), ", e.OriginalError.Error()) + err, details := e.getRootError() + if err.detail != nil { + details = append(details, fmt.Sprintf("%s", err.detail)) } - - errStr += fmt.Sprintf("Error: %s, Code: %s", e.Message, e.Code.String()) - - if e.PluginName != "" { - errStr += fmt.Sprintf(", Plugin Name: %s", e.PluginName) + if err.originalError != nil { + details = append(details, err.originalError.Error()) } - if e.ComponentType != "" { - errStr += fmt.Sprintf(", Component Type: %s", e.ComponentType) + if err.remediation != "" { + details = append(details, err.remediation) } + return fmt.Sprintf("%s: %s", err.ErrorCode().Descriptor().Value, strings.Join(details, ": ")) +} - if e.LinkToDoc != "" { - errStr += fmt.Sprintf(", Documentation: %s", e.LinkToDoc) +// GetDetail returns details from all nested errors. +func (e Error) GetDetail() string { + err, details := e.getRootError() + if err.originalError != nil && err.detail != nil { + details = append(details, fmt.Sprintf("%s", err.detail)) } - if e.Detail != nil { - errStr += fmt.Sprintf(", Detail: %v", e.Detail) - } + return strings.Join(details, ": ") +} - if e.Description != "" { - errStr += fmt.Sprintf(", Description: %v", e.Description) +// GetErrorReason returns the root cause of the error. +func (e Error) GetErrorReason() string { + err, _ := e.getRootError() + if err.originalError != nil { + return err.originalError.Error() } + return fmt.Sprintf("%s", err.detail) +} - if e.Stack != "" { - errStr += fmt.Sprintf(", Stack trace: %s", e.Stack) - } +// GetRemiation returns the remediation of the root error. +func (e Error) GetRemediation() string { + err, _ := e.getRootError() + return err.remediation +} - return errStr +func (e Error) getRootError() (err Error, details []string) { + err = e + for !err.isRootError { + if err.detail != nil { + details = append(details, fmt.Sprintf("%s", err.detail)) + } + var ratifyError Error + if errors.As(err.originalError, &ratifyError) { + err = ratifyError + } else { + // break is unnecessary, but added for safety + break + } + } + return err, details } // WithDetail will return a new Error, based on the current one, but with // some Detail info added func (e Error) WithDetail(detail interface{}) Error { - e.Detail = detail + e.detail = detail return e } // WithPluginName returns a new Error object with pluginName set. func (e Error) WithPluginName(pluginName string) Error { - e.PluginName = pluginName + e.pluginName = pluginName return e } // WithComponentType returns a new Error object with ComponentType set. func (e Error) WithComponentType(componentType ComponentType) Error { - e.ComponentType = componentType + e.componentType = componentType return e } // WithError returns a new Error object with original error. func (e Error) WithError(err error) Error { - e.OriginalError = err + e.originalError = err + e.isRootError = err == nil || !errors.As(err, &Error{}) return e } -// WithLinkToDoc returns a new Error object attached with link to documentation. -func (e Error) WithLinkToDoc(link string) Error { - e.LinkToDoc = link +// WithRemediation returns a new Error object attached with remediation. +func (e Error) WithRemediation(remediation string) Error { + e.remediation = remediation return e } func (e Error) WithDescription() Error { - e.Description = e.Code.Description() + e.description = e.code.Description() return e } diff --git a/errors/types_test.go b/errors/types_test.go index 5e97400d1..a1f66e388 100644 --- a/errors/types_test.go +++ b/errors/types_test.go @@ -22,26 +22,31 @@ import ( ) const ( - testGroup = "test-group" - testValue = "test-value" - testMessage = "test-message" - testDescription = "test-description" - testDetail = "test-detail" - testComponentType = "test-component-type" - testLink = "test-link" - testPluginName = "test-plugin-name" - testErrorString = "Original Error: (Error: , Code: UNKNOWN), Error: test-message, Code: test-value, Plugin Name: test-plugin-name, Component Type: test-component-type, Documentation: test-link, Detail: test-detail" - nonexistentEC = 2000 + testGroup = "test-group" + testErrCode1 = "TEST_ERROR_CODE_1" + testErrCode2 = "TEST_ERROR_CODE_2" + testMessage = "test-message" + testDescription = "test-description" + testDetail1 = "test-detail-1" + testDetail2 = "test-detail-2" + testComponentType1 = "test-component-type-1" + testComponentType2 = "test-component-type-2" + testLink1 = "test-link-1" + testLink2 = "test-link-2" + testPluginName = "test-plugin-name" + nonexistentEC = 2000 ) var ( testEC = Register(testGroup, ErrorDescriptor{ - Value: testValue, + Value: testErrCode1, Message: testMessage, Description: testDescription, }) - testEC2 = Register(testGroup, ErrorDescriptor{}) + testEC2 = Register(testGroup, ErrorDescriptor{ + Value: testErrCode2, + }) ) func TestErrorCode(t *testing.T) { @@ -52,8 +57,9 @@ func TestErrorCode(t *testing.T) { } func TestError(t *testing.T) { - if testEC.Error() != testValue { - t.Fatalf("expected: %s, got: %s", testValue, testEC.Error()) + expectedStr := "test error code 1" + if testEC.Error() != expectedStr { + t.Fatalf("expected: %s, got: %s", expectedStr, testEC.Error()) } } @@ -66,7 +72,7 @@ func TestDescriptor(t *testing.T) { { name: "existing error code", ec: testEC, - expectedValue: testValue, + expectedValue: testErrCode1, }, { name: "nonexistent error code", @@ -92,9 +98,9 @@ func TestMessage(t *testing.T) { } func TestWithDetail(t *testing.T) { - err := testEC.WithDetail(testDetail) - if err.Detail != testDetail { - t.Fatalf("expected detail: %s, got: %s", testDetail, err.Detail) + err := testEC.WithDetail(testDetail1) + if err.detail != testDetail1 { + t.Fatalf("expected detail: %s, got: %s", testDetail1, err.detail) } } @@ -106,35 +112,35 @@ func TestWithError(t *testing.T) { } func TestWithComponentType(t *testing.T) { - err := testEC.WithComponentType(testComponentType) - if err.ComponentType != testComponentType { - t.Fatalf("expected component type: %s, got: %s", testComponentType, err.ComponentType) + err := testEC.WithComponentType(testComponentType1) + if err.componentType != testComponentType1 { + t.Fatalf("expected component type: %s, got: %s", testComponentType1, err.componentType) } } -func TestWithLinkToDoc(t *testing.T) { - err := testEC.WithLinkToDoc(testLink) - if err.LinkToDoc != testLink { - t.Fatalf("expected link to doc: %s, got: %s", testLink, err.LinkToDoc) +func TestWithRemediation(t *testing.T) { + err := testEC.WithRemediation(testLink1) + if err.remediation != testLink1 { + t.Fatalf("expected remediation: %s, got: %s", testLink1, err.remediation) } } func TestWithPluginName(t *testing.T) { err := testEC.WithPluginName(testPluginName) - if err.PluginName != testPluginName { - t.Fatalf("expected plugin name: %s, got: %s", testPluginName, err.PluginName) + if err.pluginName != testPluginName { + t.Fatalf("expected plugin name: %s, got: %s", testPluginName, err.pluginName) } } func TestWithDescription(t *testing.T) { err := testEC.WithDescription() - if err.Description != testDescription { - t.Fatalf("expected description: %s, got: %s", testDescription, err.Description) + if err.description != testDescription { + t.Fatalf("expected description: %s, got: %s", testDescription, err.description) } } func TestIs(t *testing.T) { - err := testEC.WithDetail(testDetail) + err := testEC.WithDetail(testDetail1) result := err.Is(err) if !result { t.Fatalf("expected true, got: %v", result) @@ -149,7 +155,7 @@ func TestIs(t *testing.T) { func TestError_ErrorCode(t *testing.T) { err := Error{ - Code: 1, + code: 1, } if err.ErrorCode() != 1 { t.Fatalf("expected 1, got: %d", err.ErrorCode()) @@ -168,17 +174,64 @@ func TestIsEmpty(t *testing.T) { } func TestError_Error(t *testing.T) { - 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) + // Nested errors. + rootErr := testEC.NewError(testComponentType1, "", testLink1, errors.New(testMessage), testDetail1, false) + err := testEC2.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr) + + expectedErrStr := strings.Join([]string{testErrCode1, testDetail2, testDetail1, testMessage, testLink1}, ": ") + if err.Error() != expectedErrStr { + t.Fatalf("expected string: %s, but got: %s", expectedErrStr, err.Error()) + } + + // Single error. + err = testEC.WithDetail(testDetail1) + expectedErrStr = "TEST_ERROR_CODE_1: test-detail-1" + if err.Error() != expectedErrStr { + t.Fatalf("expected string: %s, but got: %s", expectedErrStr, err.Error()) + } +} + +func TestError_GetRootCause(t *testing.T) { + // rootErr contains original error. + rootErr := testEC.NewError(testComponentType1, "", testLink1, errors.New(testMessage), testDetail1, false) + err := testEC.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr) + + if err.GetErrorReason() != testMessage { + t.Fatalf("expected root cause: %v, but got: %v", err.GetErrorReason(), testMessage) + } + + // rootErr does not contain original error. + rootErr = testEC.NewError(testComponentType1, "", testLink1, nil, testDetail1, false) + err = testEC.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr) + + if err.GetErrorReason() != testDetail1 { + t.Fatalf("expected root cause: %v, but got: %v", err.GetErrorReason(), testDetail1) + } +} + +func TestError_GetFullDetails(t *testing.T) { + rootErr := testEC.NewError(testComponentType1, "", testLink1, errors.New(testMessage), testDetail1, false) + err := testEC.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr) + + expectedDetails := strings.Join([]string{testDetail2, testDetail1}, ": ") + if err.GetDetail() != expectedDetails { + t.Fatalf("expected full details: %v, but got: %v", expectedDetails, err.GetDetail()) + } +} + +func TestError_GetRootRemediation(t *testing.T) { + rootErr := testEC.NewError(testComponentType1, "", testLink1, errors.New(testMessage), testDetail1, false) + err := testEC.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr) + + if err.GetRemediation() != testLink1 { + t.Fatalf("expected root remediation: %v, but got: %v", err.GetRemediation(), testLink1) } } func TestNewError(t *testing.T) { - err := testEC.NewError(testComponentType, testPluginName, testLink, Error{}, testDetail, false) + err := testEC.NewError(testComponentType1, testPluginName, testLink1, Error{}, testDetail1, false) - if err.ComponentType != testComponentType || err.PluginName != testPluginName || err.LinkToDoc != testLink || err.Detail != testDetail { - t.Fatalf("expected component type: %s, plugin name: %s, link to doc: %s, detail: %s, but got: %s, %s, %s, %s", testComponentType, testPluginName, testLink, testDetail, err.ComponentType, err.PluginName, err.LinkToDoc, err.Detail) + if err.componentType != testComponentType1 || err.pluginName != testPluginName || err.remediation != testLink1 || err.detail != testDetail1 { + t.Fatalf("expected component type: %s, plugin name: %s, link to doc: %s, detail: %s, but got: %s, %s, %s, %s", testComponentType1, testPluginName, testLink1, testDetail1, err.componentType, err.pluginName, err.remediation, err.detail) } } diff --git a/test/bats/plugin-test.bats b/test/bats/plugin-test.bats index 3d37bbd4e..0cc9dea3a 100644 --- a/test/bats/plugin-test.bats +++ b/test/bats/plugin-test.bats @@ -312,7 +312,7 @@ RATIFY_NAMESPACE=gatekeeper-system sed 's/licensechecker/invalidlicensechecker/' ./config/samples/clustered/verifier/config_v1beta1_verifier_complete_licensechecker.yaml >invalidVerifier.yaml run kubectl apply -f invalidVerifier.yaml assert_success - run bash -c "kubectl describe verifiers.config.ratify.deislabs.io/verifier-license-checker -n ${RATIFY_NAMESPACE} | grep 'Brieferror: Original Error:'" + run bash -c "kubectl describe verifiers.config.ratify.deislabs.io/verifier-license-checker -n ${RATIFY_NAMESPACE} | grep 'Brieferror: PLUGIN_NOT_FOUND:'" assert_success # apply a valid verifier, validate status property shows success