diff --git a/TODO.md b/TODO.md index c4cdd0cf8..002415ff1 100644 --- a/TODO.md +++ b/TODO.md @@ -3,6 +3,13 @@ +## Structured exit codes (issue #4988) + +- [x] Slice 1: Typed errors + `os.Exit` dispatch in `main.go` + compliance exit code 1 +- [x] Slice 2: HTTP error classification in `requests.go` (5xx→2, 401/403→3, network→2) +- [x] Slice 3: Usage errors (unknown flag, missing required flag → exit 4) +- [x] Slice 4: Remaining assert commands + docs + ## kosli evaluate trail - [x] Slice 1: Skeleton `evaluate` parent + `evaluate trail` fetches trail JSON diff --git a/cmd/kosli/assertApproval.go b/cmd/kosli/assertApproval.go index 1a7ea120d..38ca93e28 100644 --- a/cmd/kosli/assertApproval.go +++ b/cmd/kosli/assertApproval.go @@ -7,6 +7,7 @@ import ( "net/http" "net/url" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" ) @@ -116,7 +117,7 @@ func (o *assertApprovalOptions) run(args []string) error { return err } if len(approvals) == 0 { - return fmt.Errorf("artifact with fingerprint %s has no approvals created", o.fingerprint) + return kosliErrors.NewErrCompliance(fmt.Sprintf("artifact with fingerprint %s has no approvals created", o.fingerprint)) } state, ok := approvals[len(approvals)-1]["state"].(string) @@ -125,6 +126,6 @@ func (o *assertApprovalOptions) run(args []string) error { logger.Info("artifact with fingerprint %s is approved (approval no. [%v])", o.fingerprint, approvalNumber) return nil } else { - return fmt.Errorf("artifact with fingerprint %s is not approved", o.fingerprint) + return kosliErrors.NewErrCompliance(fmt.Sprintf("artifact with fingerprint %s is not approved", o.fingerprint)) } } diff --git a/cmd/kosli/assertApproval_test.go b/cmd/kosli/assertApproval_test.go index 0835a7f79..ae1865718 100644 --- a/cmd/kosli/assertApproval_test.go +++ b/cmd/kosli/assertApproval_test.go @@ -49,10 +49,11 @@ func (suite *AssertApprovalCommandTestSuite) SetupTest() { func (suite *AssertApprovalCommandTestSuite) TestAssertApprovalCmd() { tests := []cmdTestCase{ { - wantError: true, - name: "1 missing --org fails", - cmd: fmt.Sprintf(`assert approval --fingerprint 8e568bd886069f1290def0caabc1e97ce0e7b80c105e611258b57d76fcef234c --flow %s --api-token secret`, suite.flowName), - golden: "Error: --org is not set\nUsage: kosli assert approval [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n", + wantError: true, + wantExitCode: 4, + name: "1 missing --org fails", + cmd: fmt.Sprintf(`assert approval --fingerprint 8e568bd886069f1290def0caabc1e97ce0e7b80c105e611258b57d76fcef234c --flow %s --api-token secret`, suite.flowName), + golden: "Error: --org is not set\nUsage: kosli assert approval [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n", }, { wantError: true, @@ -61,21 +62,24 @@ func (suite *AssertApprovalCommandTestSuite) TestAssertApprovalCmd() { golden: "Error: Artifact with fingerprint '8e568bd886069f1290def0caabc1e97ce0e7b80c105e611258b57d76fcef234c' does not exist in flow 'assert-approval' belonging to organization 'docs-cmd-test-user'\n", }, { - wantError: true, - name: "3 asserting an existing artifact that does not have an approval (using --fingerprint) works and exits with non-zero code", - cmd: fmt.Sprintf(`assert approval --fingerprint %s --flow %s %s`, suite.fingerprint, suite.flowName, suite.defaultKosliArguments), - golden: "Error: artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 has no approvals created\n", + wantError: true, + wantExitCode: 1, + name: "3 asserting an existing artifact that does not have an approval (using --fingerprint) works and exits with non-zero code", + cmd: fmt.Sprintf(`assert approval --fingerprint %s --flow %s %s`, suite.fingerprint, suite.flowName, suite.defaultKosliArguments), + golden: "Error: artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 has no approvals created\n", }, { - wantError: true, - name: "4 asserting approval of an existing artifact that does not have an approval (using --artifact-type) works and exits with non-zero code", - cmd: fmt.Sprintf(`assert approval %s --artifact-type file --flow %s %s`, suite.artifactPath, suite.flowName, suite.defaultKosliArguments), - golden: "Error: artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 has no approvals created\n", + wantError: true, + wantExitCode: 1, + name: "4 asserting approval of an existing artifact that does not have an approval (using --artifact-type) works and exits with non-zero code", + cmd: fmt.Sprintf(`assert approval %s --artifact-type file --flow %s %s`, suite.artifactPath, suite.flowName, suite.defaultKosliArguments), + golden: "Error: artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 has no approvals created\n", }, { - name: "5 asserting approval of an existing artifact that has an approval (using --artifact-type) works and exits with zero code", - cmd: fmt.Sprintf(`assert approval %s --artifact-type file --flow %s %s`, suite.artifactPath, suite.flowName, suite.defaultKosliArguments), - golden: "artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 is approved (approval no. [1])\n", + name: "5 asserting approval of an existing artifact that has an approval (using --artifact-type) works and exits with zero code", + wantExitCode: 0, + cmd: fmt.Sprintf(`assert approval %s --artifact-type file --flow %s %s`, suite.artifactPath, suite.flowName, suite.defaultKosliArguments), + golden: "artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 is approved (approval no. [1])\n", additionalConfig: assertApprovalTestConfig{ createApproval: true, isRequest: false, @@ -88,28 +92,32 @@ func (suite *AssertApprovalCommandTestSuite) TestAssertApprovalCmd() { golden: "artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 is approved (approval no. [1])\n", }, { - wantError: true, - name: "7 not providing --fingerprint nor --artifact-type fails", - cmd: fmt.Sprintf(`assert approval --flow %s %s`, suite.flowName, suite.defaultKosliArguments), - golden: "Error: docker image name or file/dir path is required when --fingerprint is not provided\nUsage: kosli assert approval [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n", + wantError: true, + wantExitCode: 4, + name: "7 not providing --fingerprint nor --artifact-type fails", + cmd: fmt.Sprintf(`assert approval --flow %s %s`, suite.flowName, suite.defaultKosliArguments), + golden: "Error: docker image name or file/dir path is required when --fingerprint is not provided\nUsage: kosli assert approval [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n", }, { - wantError: true, - name: "8 providing both --fingerprint and --artifact-type fails", - cmd: fmt.Sprintf(`assert approval --artifact-type file --fingerprint %s --flow %s %s`, suite.fingerprint, suite.flowName, suite.defaultKosliArguments), - golden: "Error: only one of --fingerprint, --artifact-type is allowed\n", + wantError: true, + wantExitCode: 1, + name: "8 providing both --fingerprint and --artifact-type fails", + cmd: fmt.Sprintf(`assert approval --artifact-type file --fingerprint %s --flow %s %s`, suite.fingerprint, suite.flowName, suite.defaultKosliArguments), + golden: "Error: only one of --fingerprint, --artifact-type is allowed\n", }, { - wantError: true, - name: "9 missing --flow fails", - cmd: fmt.Sprintf(`assert approval --fingerprint %s %s`, suite.fingerprint, suite.defaultKosliArguments), - golden: "Error: required flag(s) \"flow\" not set\n", + wantError: true, + wantExitCode: 4, + name: "9 missing --flow fails", + cmd: fmt.Sprintf(`assert approval --fingerprint %s %s`, suite.fingerprint, suite.defaultKosliArguments), + golden: "Error: required flag(s) \"flow\" not set\n", }, { - wantError: true, - name: "10 asserting approval of an unapproved existing artifact (using --artifact-type) works and exits with non-zero code", - cmd: fmt.Sprintf(`assert approval %s --artifact-type file --flow %s %s`, suite.artifactPath, suite.flowName, suite.defaultKosliArguments), - golden: "Error: artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 is not approved\n", + wantError: true, + wantExitCode: 1, + name: "10 asserting approval of an unapproved existing artifact (using --artifact-type) works and exits with non-zero code", + cmd: fmt.Sprintf(`assert approval %s --artifact-type file --flow %s %s`, suite.artifactPath, suite.flowName, suite.defaultKosliArguments), + golden: "Error: artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 is not approved\n", additionalConfig: assertApprovalTestConfig{ createApproval: true, isRequest: true, @@ -117,10 +125,11 @@ func (suite *AssertApprovalCommandTestSuite) TestAssertApprovalCmd() { }, // The approval request created in test 9 is valid here too { - wantError: true, - name: "11 asserting approval of an unapproved existing artifact (using --fingerprint) works and exits with non-zero code", - cmd: fmt.Sprintf(`assert approval --fingerprint %s --flow %s %s`, suite.fingerprint, suite.flowName, suite.defaultKosliArguments), - golden: "Error: artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 is not approved\n", + wantError: true, + wantExitCode: 1, + name: "11 asserting approval of an unapproved existing artifact (using --fingerprint) works and exits with non-zero code", + cmd: fmt.Sprintf(`assert approval --fingerprint %s --flow %s %s`, suite.fingerprint, suite.flowName, suite.defaultKosliArguments), + golden: "Error: artifact with fingerprint fcf33337634c2577a5d86fd7ecb0a25a7c1bb5d89c14fd236f546a5759252c02 is not approved\n", }, } diff --git a/cmd/kosli/assertArtifact.go b/cmd/kosli/assertArtifact.go index bc6b19d4d..0fd870690 100644 --- a/cmd/kosli/assertArtifact.go +++ b/cmd/kosli/assertArtifact.go @@ -7,6 +7,7 @@ import ( "net/http" "net/url" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/output" "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" @@ -169,7 +170,7 @@ func (o *assertArtifactOptions) run(out io.Writer, args []string) error { isCompliant := evaluationResult["compliant"].(bool) if !isCompliant { - return fmt.Errorf("Artifact is not compliant") + return kosliErrors.NewErrCompliance("Artifact is not compliant") } return nil } diff --git a/cmd/kosli/assertArtifact_test.go b/cmd/kosli/assertArtifact_test.go index 333821978..c6d37f988 100644 --- a/cmd/kosli/assertArtifact_test.go +++ b/cmd/kosli/assertArtifact_test.go @@ -83,10 +83,11 @@ func (suite *AssertArtifactCommandTestSuite) SetupTest() { func (suite *AssertArtifactCommandTestSuite) TestAssertArtifactCmd() { tests := []cmdTestCase{ { - wantError: true, - name: "01 missing --org fails", - cmd: fmt.Sprintf(`assert artifact --fingerprint 8e568bd886069f1290def0caabc1e97ce0e7b80c105e611258b57d76fcef234c --flow %s --api-token secret`, suite.flowName1), - golden: "Error: --org is not set\nUsage: kosli assert artifact [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n", + wantError: true, + wantExitCode: 4, + name: "01 missing --org fails", + cmd: fmt.Sprintf(`assert artifact --fingerprint 8e568bd886069f1290def0caabc1e97ce0e7b80c105e611258b57d76fcef234c --flow %s --api-token secret`, suite.flowName1), + golden: "Error: --org is not set\nUsage: kosli assert artifact [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n", }, { wantError: true, @@ -95,9 +96,10 @@ func (suite *AssertArtifactCommandTestSuite) TestAssertArtifactCmd() { golden: "Error: Artifact with fingerprint '8e568bd886069f1290def0caabc1e97ce0e7b80c105e611258b57d76fcef234c' does not exist in flow 'assert-artifact-one' belonging to organization 'docs-cmd-test-user'\n", }, { - name: "03 asserting a single existing compliant artifact (using --fingerprint) results in OK and zero exit", - cmd: fmt.Sprintf(`assert artifact --fingerprint %s %s`, suite.fingerprint1, suite.defaultKosliArguments), - goldenRegex: "(?s)^COMPLIANT\n.*Attestation-name.*See more details at http://localhost(:8001)?/docs-cmd-test-user/flows/assert-artifact-one/artifacts/0089a849fce9c7c9128cd13a2e8b1c0757bdb6a7bad0fdf2800e38c19055b7fc(?:\\?artifact_id=[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{8})?\n", + name: "03 asserting a single existing compliant artifact (using --fingerprint) results in OK and zero exit", + wantExitCode: 0, + cmd: fmt.Sprintf(`assert artifact --fingerprint %s %s`, suite.fingerprint1, suite.defaultKosliArguments), + goldenRegex: "(?s)^COMPLIANT\n.*Attestation-name.*See more details at http://localhost(:8001)?/docs-cmd-test-user/flows/assert-artifact-one/artifacts/0089a849fce9c7c9128cd13a2e8b1c0757bdb6a7bad0fdf2800e38c19055b7fc(?:\\?artifact_id=[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{8})?\n", }, { name: "04 json output of asserting a single existing compliant artifact (using --fingerprint) results in OK and zero exit", @@ -176,28 +178,32 @@ func (suite *AssertArtifactCommandTestSuite) TestAssertArtifactCmd() { }, }, { - wantError: true, - name: "14 not providing --fingerprint nor --artifact-type fails", - cmd: fmt.Sprintf(`assert artifact --flow %s %s`, suite.flowName1, suite.defaultKosliArguments), - golden: "Error: docker image name or file/dir path is required when --fingerprint is not provided\nUsage: kosli assert artifact [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n", + wantError: true, + wantExitCode: 4, + name: "14 not providing --fingerprint nor --artifact-type fails", + cmd: fmt.Sprintf(`assert artifact --flow %s %s`, suite.flowName1, suite.defaultKosliArguments), + golden: "Error: docker image name or file/dir path is required when --fingerprint is not provided\nUsage: kosli assert artifact [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n", }, { - wantError: true, - name: "15 providing both --environment and --polices fails", - cmd: fmt.Sprintf(`assert artifact --fingerprint %s --environment %s --policy %s %s`, suite.fingerprint1, suite.envName, suite.policyName1, suite.defaultKosliArguments), - golden: "Error: Cannot specify both 'environment_name' and 'policy_name' at the same time\n", + wantError: true, + wantExitCode: 1, + name: "15 providing both --environment and --polices fails", + cmd: fmt.Sprintf(`assert artifact --fingerprint %s --environment %s --policy %s %s`, suite.fingerprint1, suite.envName, suite.policyName1, suite.defaultKosliArguments), + golden: "Error: Cannot specify both 'environment_name' and 'policy_name' at the same time\n", }, { - wantError: true, - name: "16 asserting a single existing non-compliant artifact (using --fingerprint) results in non-zero exit", - cmd: fmt.Sprintf(`assert artifact --fingerprint %s %s`, suite.fingerprint3, suite.defaultKosliArguments), - goldenRegex: "^Error: NON-COMPLIANT\n", + wantError: true, + wantExitCode: 1, + name: "16 asserting a single existing non-compliant artifact (using --fingerprint) results in non-zero exit", + cmd: fmt.Sprintf(`assert artifact --fingerprint %s %s`, suite.fingerprint3, suite.defaultKosliArguments), + goldenRegex: "^Error: NON-COMPLIANT\n", }, { - wantError: true, - name: "17 asserting a single existing non-compliant artifact (using --artifact-type) results in non-zero exit", - cmd: fmt.Sprintf(`assert artifact %s --artifact-type file %s`, suite.artifact3Path, suite.defaultKosliArguments), - goldenRegex: "^Error: NON-COMPLIANT\n", + wantError: true, + wantExitCode: 1, + name: "17 asserting a single existing non-compliant artifact (using --artifact-type) results in non-zero exit", + cmd: fmt.Sprintf(`assert artifact %s --artifact-type file %s`, suite.artifact3Path, suite.defaultKosliArguments), + goldenRegex: "^Error: NON-COMPLIANT\n", }, } diff --git a/cmd/kosli/assertPRAzure.go b/cmd/kosli/assertPRAzure.go index 30e452a62..70c21cafe 100644 --- a/cmd/kosli/assertPRAzure.go +++ b/cmd/kosli/assertPRAzure.go @@ -5,6 +5,7 @@ import ( "io" azUtils "github.com/kosli-dev/cli/internal/azure" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/spf13/cobra" ) @@ -64,7 +65,7 @@ func (o *assertPullRequestAzureOptions) run(args []string) error { return err } if len(pullRequestsEvidence) == 0 { - return fmt.Errorf("assert failed: found no pull request(s) in Azure DevOps for commit: %s", o.commit) + return kosliErrors.NewErrCompliance(fmt.Sprintf("assert failed: found no pull request(s) in Azure DevOps for commit: %s", o.commit)) } logger.Info("found [%d] pull request(s) in Azure DevOps for commit: %s", len(pullRequestsEvidence), o.commit) return nil diff --git a/cmd/kosli/assertPRAzure_test.go b/cmd/kosli/assertPRAzure_test.go index 375552edf..ca265a244 100644 --- a/cmd/kosli/assertPRAzure_test.go +++ b/cmd/kosli/assertPRAzure_test.go @@ -30,22 +30,25 @@ func (suite *AssertPRAzureCommandTestSuite) SetupTest() { func (suite *AssertPRAzureCommandTestSuite) TestAssertPRAzureCmd() { tests := []cmdTestCase{ { - name: "assert Azure PR evidence passes when commit has a PR in Azure", - cmd: `assert pullrequest azure --azure-org-url https://dev.azure.com/kosli --project kosli-azure --repository cli + name: "assert Azure PR evidence passes when commit has a PR in Azure", + wantExitCode: 0, + cmd: `assert pullrequest azure --azure-org-url https://dev.azure.com/kosli --project kosli-azure --repository cli --commit e6b38318747f1c225e6d2cdba1e88aa00fbcae29` + suite.defaultKosliArguments, golden: "found [1] pull request(s) in Azure DevOps for commit: e6b38318747f1c225e6d2cdba1e88aa00fbcae29\n", }, { - wantError: true, - name: "assert Azure PR evidence fails when commit has no PRs in Azure", - cmd: `assert pullrequest azure --azure-org-url https://dev.azure.com/kosli --project kosli-azure --repository cli + wantError: true, + wantExitCode: 1, + name: "assert Azure PR evidence fails when commit has no PRs in Azure", + cmd: `assert pullrequest azure --azure-org-url https://dev.azure.com/kosli --project kosli-azure --repository cli --commit 58d8aad96e0dcd11ada3dc6650d23909eed336ed` + suite.defaultKosliArguments, golden: "Error: assert failed: found no pull request(s) in Azure DevOps for commit: 58d8aad96e0dcd11ada3dc6650d23909eed336ed\n", }, { - wantError: true, - name: "assert Azure PR evidence fails when commit does not exist", - cmd: `assert pullrequest azure --azure-org-url https://dev.azure.com/kosli --project kosli-azure --repository cli + wantError: true, + wantExitCode: 1, + name: "assert Azure PR evidence fails when commit does not exist", + cmd: `assert pullrequest azure --azure-org-url https://dev.azure.com/kosli --project kosli-azure --repository cli --commit c4fa4c2ce6bef984abc93be9258a85f9137ff1c9` + suite.defaultKosliArguments, golden: "Error: assert failed: found no pull request(s) in Azure DevOps for commit: c4fa4c2ce6bef984abc93be9258a85f9137ff1c9\n", }, diff --git a/cmd/kosli/assertPRBitbucket.go b/cmd/kosli/assertPRBitbucket.go index 6bf80cd47..f95e48ff6 100644 --- a/cmd/kosli/assertPRBitbucket.go +++ b/cmd/kosli/assertPRBitbucket.go @@ -5,6 +5,7 @@ import ( "io" bbUtils "github.com/kosli-dev/cli/internal/bitbucket" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/spf13/cobra" ) @@ -86,7 +87,7 @@ func (o *assertPullRequestBitbucketOptions) run(args []string) error { return err } if len(pullRequestsEvidence) == 0 { - return fmt.Errorf("assert failed: found no pull request(s) in Bitbucket for commit: %s", o.commit) + return kosliErrors.NewErrCompliance(fmt.Sprintf("assert failed: found no pull request(s) in Bitbucket for commit: %s", o.commit)) } logger.Info("found [%d] pull request(s) in Bitbucket for commit: %s", len(pullRequestsEvidence), o.commit) return nil diff --git a/cmd/kosli/assertPRBitbucket_test.go b/cmd/kosli/assertPRBitbucket_test.go index fcc0e044f..19ff643b4 100644 --- a/cmd/kosli/assertPRBitbucket_test.go +++ b/cmd/kosli/assertPRBitbucket_test.go @@ -30,22 +30,25 @@ func (suite *AssertPRBitbucketCommandTestSuite) SetupTest() { func (suite *AssertPRBitbucketCommandTestSuite) TestAssertPRBitbucketCmd() { tests := []cmdTestCase{ { - name: "assert Bitbucket PR evidence passes when commit has a PR in bitbucket", - cmd: `assert pullrequest bitbucket --bitbucket-workspace kosli-dev --repository cli-test + name: "assert Bitbucket PR evidence passes when commit has a PR in bitbucket", + wantExitCode: 0, + cmd: `assert pullrequest bitbucket --bitbucket-workspace kosli-dev --repository cli-test --commit fd54040fc90e7e83f7b152619bfa18917b72c34f` + suite.defaultKosliArguments, golden: "found [1] pull request(s) in Bitbucket for commit: fd54040fc90e7e83f7b152619bfa18917b72c34f\n", }, { - wantError: true, - name: "assert Bitbucket PR evidence fails when both password and access token are provided", - cmd: `assert pullrequest bitbucket --bitbucket-workspace kosli-dev --repository cli-test + wantError: true, + wantExitCode: 1, + name: "assert Bitbucket PR evidence fails when both password and access token are provided", + cmd: `assert pullrequest bitbucket --bitbucket-workspace kosli-dev --repository cli-test --commit fd54040fc90e7e83f7b152619bfa18917b72c34f --bitbucket-password xxxx` + suite.defaultKosliArguments, golden: "Error: only one of --bitbucket-password, --bitbucket-access-token is allowed\n", }, { - wantError: true, - name: "assert Bitbucket PR evidence fails when commit has no PRs in bitbucket", - cmd: `assert pullrequest bitbucket --bitbucket-workspace kosli-dev --repository cli-test + wantError: true, + wantExitCode: 1, + name: "assert Bitbucket PR evidence fails when commit has no PRs in bitbucket", + cmd: `assert pullrequest bitbucket --bitbucket-workspace kosli-dev --repository cli-test --commit 3dce097040987c4693d2e4be817474d9d0063c93` + suite.defaultKosliArguments, golden: "Error: assert failed: found no pull request(s) in Bitbucket for commit: 3dce097040987c4693d2e4be817474d9d0063c93\n", }, diff --git a/cmd/kosli/assertPRGithub.go b/cmd/kosli/assertPRGithub.go index a74243128..1ecedf303 100644 --- a/cmd/kosli/assertPRGithub.go +++ b/cmd/kosli/assertPRGithub.go @@ -4,6 +4,7 @@ import ( "fmt" "io" + kosliErrors "github.com/kosli-dev/cli/internal/errors" ghUtils "github.com/kosli-dev/cli/internal/github" "github.com/spf13/cobra" ) @@ -63,7 +64,7 @@ func (o *assertPullRequestGithubOptions) run(args []string) error { return err } if len(pullRequestsEvidence) == 0 { - return fmt.Errorf("assert failed: found no pull request(s) in Github for commit: %s", o.commit) + return kosliErrors.NewErrCompliance(fmt.Sprintf("assert failed: found no pull request(s) in Github for commit: %s", o.commit)) } logger.Info("found [%d] pull request(s) in Github for commit: %s", len(pullRequestsEvidence), o.commit) return nil diff --git a/cmd/kosli/assertPRGithub_test.go b/cmd/kosli/assertPRGithub_test.go index ce2919c24..6e755fde4 100644 --- a/cmd/kosli/assertPRGithub_test.go +++ b/cmd/kosli/assertPRGithub_test.go @@ -30,22 +30,25 @@ func (suite *AssertPRGithubCommandTestSuite) SetupTest() { func (suite *AssertPRGithubCommandTestSuite) TestAssertPRGithubCmd() { tests := []cmdTestCase{ { - name: "assert Github PR evidence passes when commit has a PR in github", + name: "assert Github PR evidence passes when commit has a PR in github", + wantExitCode: 0, cmd: `assert pullrequest github --github-org kosli-dev --repository cli --commit ` + testHelpers.GithubCommitWithPR() + suite.defaultKosliArguments, golden: fmt.Sprintf("found [1] pull request(s) in Github for commit: %s\n", testHelpers.GithubCommitWithPR()), }, { - wantError: true, - name: "assert Github PR evidence fails when commit has no PRs in github", - cmd: `assert pullrequest github --github-org kosli-dev --repository cli + wantError: true, + wantExitCode: 1, + name: "assert Github PR evidence fails when commit has no PRs in github", + cmd: `assert pullrequest github --github-org kosli-dev --repository cli --commit 19aab7f063147614451c88969602a10afbabb43d` + suite.defaultKosliArguments, golden: "Error: assert failed: found no pull request(s) in Github for commit: 19aab7f063147614451c88969602a10afbabb43d\n", }, { - wantError: true, - name: "assert Github PR evidence fails when commit does not exist", - cmd: `assert pullrequest github --github-org kosli-dev --repository cli + wantError: true, + wantExitCode: 1, + name: "assert Github PR evidence fails when commit does not exist", + cmd: `assert pullrequest github --github-org kosli-dev --repository cli --commit 19aab7f063147614451c88969602a10afba123ab` + suite.defaultKosliArguments, golden: "Error: assert failed: found no pull request(s) in Github for commit: 19aab7f063147614451c88969602a10afba123ab\n", }, diff --git a/cmd/kosli/assertPRGitlab.go b/cmd/kosli/assertPRGitlab.go index 019f5912e..a9f6a175e 100644 --- a/cmd/kosli/assertPRGitlab.go +++ b/cmd/kosli/assertPRGitlab.go @@ -4,6 +4,7 @@ import ( "fmt" "io" + kosliErrors "github.com/kosli-dev/cli/internal/errors" gitlabUtils "github.com/kosli-dev/cli/internal/gitlab" "github.com/spf13/cobra" ) @@ -63,7 +64,7 @@ func (o *assertPullRequestGitlabOptions) run(args []string) error { return err } if len(pullRequestsEvidence) == 0 { - return fmt.Errorf("assert failed: found no merge request(s) in Gitlab for commit: %s", o.commit) + return kosliErrors.NewErrCompliance(fmt.Sprintf("assert failed: found no merge request(s) in Gitlab for commit: %s", o.commit)) } logger.Info("found [%d] merge request(s) in Gitlab for commit: %s", len(pullRequestsEvidence), o.commit) return nil diff --git a/cmd/kosli/assertPRGitlab_test.go b/cmd/kosli/assertPRGitlab_test.go index f25b23504..f97c3a789 100644 --- a/cmd/kosli/assertPRGitlab_test.go +++ b/cmd/kosli/assertPRGitlab_test.go @@ -30,7 +30,8 @@ func (suite *AssertPRGitlabCommandTestSuite) SetupTest() { func (suite *AssertPRGitlabCommandTestSuite) TestAssertPRGitlabCmd() { tests := []cmdTestCase{ { - name: "assert Gitlab PR evidence passes when commit has a PR in gitlab", + name: "assert Gitlab PR evidence passes when commit has a PR in gitlab", + wantExitCode: 0, cmd: `assert mergerequest gitlab --gitlab-org kosli-dev --repository merkely-gitlab-demo --commit f6d2c1a288f2c400c04e8451f4fdddb1f3b4ce01` + suite.defaultKosliArguments, golden: "found [1] merge request(s) in Gitlab for commit: f6d2c1a288f2c400c04e8451f4fdddb1f3b4ce01\n", @@ -48,8 +49,9 @@ func (suite *AssertPRGitlabCommandTestSuite) TestAssertPRGitlabCmd() { golden: "found [1] merge request(s) in Gitlab for commit: f6d2c1a288f2c400c04e8451f4fdddb1f3b4ce01\n", }, { - wantError: true, - name: "assert Gitlab PR evidence fails when commit has no PRs in gitlab", + wantError: true, + wantExitCode: 1, + name: "assert Gitlab PR evidence fails when commit has no PRs in gitlab", cmd: `assert mergerequest gitlab --gitlab-org kosli-dev --repository merkely-gitlab-demo --commit 2ec23dda01fc85e3f94a2b5ea8cb8cf7e79c4ed6` + suite.defaultKosliArguments, golden: "Error: assert failed: found no merge request(s) in Gitlab for commit: 2ec23dda01fc85e3f94a2b5ea8cb8cf7e79c4ed6\n", diff --git a/cmd/kosli/assertSnapshot.go b/cmd/kosli/assertSnapshot.go index dac6d117f..acc80cd97 100644 --- a/cmd/kosli/assertSnapshot.go +++ b/cmd/kosli/assertSnapshot.go @@ -2,12 +2,12 @@ package main import ( "encoding/json" - "fmt" "io" "net/http" "net/url" "strconv" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" ) @@ -87,7 +87,7 @@ func run(out io.Writer, args []string) error { if environmentData["compliant"].(bool) { logger.Info("COMPLIANT") } else { - return fmt.Errorf("INCOMPLIANT") + return kosliErrors.NewErrCompliance("INCOMPLIANT") } return nil diff --git a/cmd/kosli/assertSnapshot_test.go b/cmd/kosli/assertSnapshot_test.go index 8e1db7705..3ad496754 100644 --- a/cmd/kosli/assertSnapshot_test.go +++ b/cmd/kosli/assertSnapshot_test.go @@ -64,10 +64,11 @@ func (suite *AssertSnapshotCommandTestSuite) SetupTest() { func (suite *AssertSnapshotCommandTestSuite) TestAssertSnapshotCmd() { tests := []cmdTestCase{ { - wantError: true, - name: "01 missing --org fails", - cmd: fmt.Sprintf(`assert snapshot %s --api-token secret`, suite.nonCompliantEnvName), - golden: "Error: --org is not set\nUsage: kosli assert snapshot ENVIRONMENT-NAME-OR-EXPRESSION [flags]\n", + wantError: true, + wantExitCode: 4, + name: "01 missing --org fails", + cmd: fmt.Sprintf(`assert snapshot %s --api-token secret`, suite.nonCompliantEnvName), + golden: "Error: --org is not set\nUsage: kosli assert snapshot ENVIRONMENT-NAME-OR-EXPRESSION [flags]\n", }, { wantError: true, @@ -82,9 +83,10 @@ func (suite *AssertSnapshotCommandTestSuite) TestAssertSnapshotCmd() { golden: "Error: Environment named 'non-existing' does not exist for organization 'docs-cmd-test-user'\n", }, { - wantError: true, - name: "04 asserting a non compliant env results in INCOMPLIANT and non-zero exit", - cmd: fmt.Sprintf(`assert snapshot %s %s`, suite.nonCompliantEnvName, suite.defaultKosliArguments), + wantError: true, + wantExitCode: 1, + name: "04 asserting a non compliant env results in INCOMPLIANT and non-zero exit", + cmd: fmt.Sprintf(`assert snapshot %s %s`, suite.nonCompliantEnvName, suite.defaultKosliArguments), additionalConfig: assertSnapshotTestConfig{ reportToEnv: true, envName: suite.nonCompliantEnvName, diff --git a/cmd/kosli/attestJira.go b/cmd/kosli/attestJira.go index c90df0d5e..66345c42d 100644 --- a/cmd/kosli/attestJira.go +++ b/cmd/kosli/attestJira.go @@ -9,6 +9,7 @@ import ( "regexp" "strings" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/gitview" "github.com/kosli-dev/cli/internal/jira" "github.com/kosli-dev/cli/internal/requests" @@ -332,7 +333,7 @@ func (o *attestJiraOptions) run(args []string) error { if err != nil { errString = fmt.Sprintf("%s\nError: ", err.Error()) } - err = fmt.Errorf("%sno Jira references are found in commit message or branch name", errString) + err = kosliErrors.NewErrCompliance(fmt.Sprintf("%sno Jira references are found in commit message or branch name", errString)) } if issueFoundCount != len(issueIDs) && o.assert && !global.DryRun { @@ -340,7 +341,7 @@ func (o *attestJiraOptions) run(args []string) error { if err != nil { errString = fmt.Sprintf("%s\nError: ", err.Error()) } - err = fmt.Errorf("%smissing Jira issues from references found in commit message or branch name%s", errString, issueLog) + err = kosliErrors.NewErrCompliance(fmt.Sprintf("%smissing Jira issues from references found in commit message or branch name%s", errString, issueLog)) } return wrapAttestationError(err) } diff --git a/cmd/kosli/attestJira_test.go b/cmd/kosli/attestJira_test.go index fd301bb81..fd0a9e2e9 100644 --- a/cmd/kosli/attestJira_test.go +++ b/cmd/kosli/attestJira_test.go @@ -108,8 +108,9 @@ func (suite *AttestJiraCommandTestSuite) TestAttestJiraCmd() { }, }, { - wantError: true, - name: "07 assert for non-existing Jira issue gives an error", + wantError: true, + wantExitCode: 1, + name: "07 assert for non-existing Jira issue gives an error", cmd: fmt.Sprintf(`attest jira --name jira-validation --jira-base-url https://kosli-test.atlassian.net --repo-root %s @@ -273,8 +274,9 @@ func (suite *AttestJiraCommandTestSuite) TestAttestJiraCmd() { }, }, { - wantError: true, - name: "22 if no matching issue exists, assert fails with a non-zero exit code", + wantError: true, + wantExitCode: 1, + name: "22 if no matching issue exists, assert fails with a non-zero exit code", cmd: fmt.Sprintf(`attest jira --name bar --jira-base-url https://kosli-test.atlassian.net --jira-project-key ABC @@ -297,8 +299,9 @@ func (suite *AttestJiraCommandTestSuite) TestAttestJiraCmd() { }, }, { - wantError: true, - name: "24 if there is a server error, this is output even when assert fails due to no matching issue", + wantError: true, + wantExitCode: 1, + name: "24 if there is a server error, this is output even when assert fails due to no matching issue", cmd: fmt.Sprintf(`attest jira --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --repo-root %s @@ -312,8 +315,9 @@ func (suite *AttestJiraCommandTestSuite) TestAttestJiraCmd() { }, }, { - wantError: true, - name: "25 if there is a server error, this is output even when assert fails due to non-existing issue", + wantError: true, + wantExitCode: 1, + name: "25 if there is a server error, this is output even when assert fails due to non-existing issue", cmd: fmt.Sprintf(`attest jira --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --repo-root %s diff --git a/cmd/kosli/attestPRAzure_test.go b/cmd/kosli/attestPRAzure_test.go index 4582b580d..fa09c55c1 100644 --- a/cmd/kosli/attestPRAzure_test.go +++ b/cmd/kosli/attestPRAzure_test.go @@ -106,15 +106,17 @@ func (suite *AttestAzurePRCommandTestSuite) TestAttestAzurePRCmd() { goldenRegex: "found 0 pull request\\(s\\) for commit: .*\nazure pull request attestation 'foo' is reported to trail: test-123\n", }, { - wantError: true, - name: "12 assert fails with non-zero exit code when commit has no PRs", + wantError: true, + wantExitCode: 1, + name: "12 assert fails with non-zero exit code when commit has no PRs", cmd: fmt.Sprintf(`attest pullrequest azure --fingerprint 7509e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --azure-org-url https://dev.azure.com/kosli --project kosli-azure --repository cli --commit HEAD --assert %s`, suite.defaultKosliArguments), goldenRegex: "found 0 pull request\\(s\\) for commit: .*\nazure pull request attestation 'foo' is reported to trail: test-123\nError: assert failed: no pull request found for the given commit: .*\n", }, { - wantError: true, - name: "13 if there is a server error, this is output even when assert fails", + wantError: true, + wantExitCode: 1, + name: "13 if there is a server error, this is output even when assert fails", cmd: fmt.Sprintf(`attest pullrequest azure --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --azure-org-url https://dev.azure.com/kosli --project kosli-azure --repository cli --commit HEAD --assert %s`, suite.defaultKosliArguments), goldenRegex: "found 0 pull request\\(s\\) for commit: .*\nError: Artifact with fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 does not exist in trail \"test-123\" of flow \"attest-azure-pr\" belonging to organization \"docs-cmd-test-user\"\nError: assert failed: no pull request found for the given commit: .*\n", diff --git a/cmd/kosli/attestPRBitbucket_test.go b/cmd/kosli/attestPRBitbucket_test.go index 176d9bd73..4d768aaa4 100644 --- a/cmd/kosli/attestPRBitbucket_test.go +++ b/cmd/kosli/attestPRBitbucket_test.go @@ -133,8 +133,9 @@ func (suite *AttestBitbucketPRCommandTestSuite) TestAttestBitbucketPRCmd() { goldenRegex: "found 1 pull request\\(s\\) for commit: .*\nbitbucket pull request attestation 'foo' is reported to trail: test-123\n", }, { - wantError: true, - name: "13 assert fails with non-zero exit code when commit has no PRs", + wantError: true, + wantExitCode: 1, + name: "13 assert fails with non-zero exit code when commit has no PRs", cmd: fmt.Sprintf(`attest pullrequest bitbucket --name cli.foo --bitbucket-workspace kosli-dev --repository cli-test --assert --commit %s %s`, suite.commitWithNoPR, suite.defaultKosliArguments), goldenRegex: "found 0 pull request\\(s\\) for commit: .*\nbitbucket pull request attestation 'foo' is reported to trail: test-123\nError: assert failed: no pull request found for the given commit: .*\n", @@ -146,8 +147,9 @@ func (suite *AttestBitbucketPRCommandTestSuite) TestAttestBitbucketPRCmd() { goldenRegex: "found 1 pull request\\(s\\) for commit: .*\nbitbucket pull request attestation 'foo' is reported to trail: test-123\n", }, { - wantError: true, - name: "15 if there is a server error, this is output even when assert fails", + wantError: true, + wantExitCode: 1, + name: "15 if there is a server error, this is output even when assert fails", cmd: fmt.Sprintf(`attest pullrequest bitbucket --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --bitbucket-workspace kosli-dev --repository cli-test --commit %s --assert %s`, suite.commitWithNoPR, suite.defaultKosliArguments), goldenRegex: "found 0 pull request\\(s\\) for commit: .*\nError: Artifact with fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 does not exist in trail \"test-123\" of flow \"attest-bitbucket-pr\" belonging to organization \"docs-cmd-test-user\"\nError: assert failed: no pull request found for the given commit: .*\n", diff --git a/cmd/kosli/attestPRGithub_test.go b/cmd/kosli/attestPRGithub_test.go index 2cfc18ec4..a55d4e1ff 100644 --- a/cmd/kosli/attestPRGithub_test.go +++ b/cmd/kosli/attestPRGithub_test.go @@ -124,8 +124,9 @@ func (suite *AttestGithubPRCommandTestSuite) TestAttestGithubPRCmd() { goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'bar' is reported to trail: test-123\n", }, { - wantError: true, - name: "14 assert fails with non-zero exit code when commit has no PRs", + wantError: true, + wantExitCode: 1, + name: "14 assert fails with non-zero exit code when commit has no PRs", cmd: fmt.Sprintf(`attest pullrequest github --name bar --github-org kosli-dev --repository cli --commit %s --assert %s`, suite.commitWithNoPR, suite.defaultKosliArguments), goldenRegex: "found 0 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'bar' is reported to trail: test-123\nError: assert failed: no pull request found for the given commit: .*\n", @@ -137,8 +138,9 @@ func (suite *AttestGithubPRCommandTestSuite) TestAttestGithubPRCmd() { goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'bar' is reported to trail: test-123\n", }, { - wantError: true, - name: "16 if there is a server error, this is output even when assert fails", + wantError: true, + wantExitCode: 1, + name: "16 if there is a server error, this is output even when assert fails", cmd: fmt.Sprintf(`attest pullrequest github --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --github-org kosli-dev --repository cli --commit %s --assert %s`, suite.commitWithNoPR, suite.defaultKosliArguments), goldenRegex: "found 0 pull request\\(s\\) for commit: .*\nError: Artifact with fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 does not exist in trail \"test-123\" of flow \"attest-github-pr\" belonging to organization \"docs-cmd-test-user\"\nError: assert failed: no pull request found for the given commit: .*\n", diff --git a/cmd/kosli/attestPRGitlab_test.go b/cmd/kosli/attestPRGitlab_test.go index 129112053..410a59de1 100644 --- a/cmd/kosli/attestPRGitlab_test.go +++ b/cmd/kosli/attestPRGitlab_test.go @@ -126,8 +126,9 @@ func (suite *AttestGitlabPRCommandTestSuite) TestAttestGitlabPRCmd() { goldenRegex: "found 1 merge request\\(s\\) for commit: .*\ngitlab merge request attestation 'foo' is reported to trail: test-123\n", }, { - wantError: true, - name: "12 assert fails with non-zero exit code when commit has no merge requests", + wantError: true, + wantExitCode: 1, + name: "12 assert fails with non-zero exit code when commit has no merge requests", cmd: fmt.Sprintf(`attest pullrequest gitlab --name bar --gitlab-org kosli-dev --repository merkely-gitlab-demo --commit %s --assert %s`, suite.commitWithNoPR, suite.defaultKosliArguments), goldenRegex: "found 0 merge request\\(s\\) for commit: .*\ngitlab merge request attestation 'bar' is reported to trail: test-123\nError: assert failed: no merge request found for the given commit: .*\n", @@ -139,8 +140,9 @@ func (suite *AttestGitlabPRCommandTestSuite) TestAttestGitlabPRCmd() { goldenRegex: "found 1 merge request\\(s\\) for commit: .*\ngitlab merge request attestation 'bar' is reported to trail: test-123\n", }, { - wantError: true, - name: "14 if there is a server error, this is output even when assert fails", + wantError: true, + wantExitCode: 1, + name: "14 if there is a server error, this is output even when assert fails", cmd: fmt.Sprintf(`attest pullrequest gitlab --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --gitlab-org kosli-dev --repository merkely-gitlab-demo --commit %s --assert %s`, suite.commitWithNoPR, suite.defaultKosliArguments), goldenRegex: "found 0 merge request\\(s\\) for commit: .*\nError: Artifact with fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 does not exist in trail \"test-123\" of flow \"attest-gitlab-pr\" belonging to organization \"docs-cmd-test-user\"\nError: assert failed: no merge request found for the given commit: .*\n", diff --git a/cmd/kosli/attestation.go b/cmd/kosli/attestation.go index 48b880bd5..cf5e2448a 100644 --- a/cmd/kosli/attestation.go +++ b/cmd/kosli/attestation.go @@ -232,8 +232,12 @@ func newAttestationForm(payload interface{}, attachments []string) ( func wrapAttestationError(err error) error { if err != nil { - return fmt.Errorf("%s", strings.Replace(err.Error(), "requires at least one of: artifact_fingerprint or git_commit_info.", - "requires at least one of: specifying the fingerprint (either by calculating it using the artifact name/path and --artifact-type, or by providing it using --fingerprint) or providing --commit (requires an available git repo to access commit details)", 1)) + msg := strings.Replace(err.Error(), "requires at least one of: artifact_fingerprint or git_commit_info.", + "requires at least one of: specifying the fingerprint (either by calculating it using the artifact name/path and --artifact-type, or by providing it using --fingerprint) or providing --commit (requires an available git repo to access commit details)", 1) + if msg == err.Error() { + return err // no replacement made — return original to preserve error type + } + return fmt.Errorf("%s", msg) } return err } diff --git a/cmd/kosli/attestation_test.go b/cmd/kosli/attestation_test.go index ebf6b5bf8..98525f44d 100644 --- a/cmd/kosli/attestation_test.go +++ b/cmd/kosli/attestation_test.go @@ -1,8 +1,11 @@ package main import ( + "errors" + "fmt" "testing" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/gitview" "github.com/stretchr/testify/assert" ) @@ -116,3 +119,30 @@ func TestMergeGitRepoInfo(t *testing.T) { }) } } + +func TestWrapAttestationError(t *testing.T) { + t.Run("nil returns nil", func(t *testing.T) { + assert.NoError(t, wrapAttestationError(nil)) + }) + + t.Run("ErrCompliance is preserved through wrapAttestationError", func(t *testing.T) { + inner := kosliErrors.NewErrCompliance("assert failed: no pull request found") + err := wrapAttestationError(inner) + var ec *kosliErrors.ErrCompliance + assert.True(t, errors.As(err, &ec), "expected ErrCompliance to be preserved, got %T: %v", err, err) + }) + + t.Run("ErrServer is preserved through wrapAttestationError", func(t *testing.T) { + inner := kosliErrors.NewErrServer("server error") + err := wrapAttestationError(inner) + var es *kosliErrors.ErrServer + assert.True(t, errors.As(err, &es), "expected ErrServer to be preserved, got %T: %v", err, err) + }) + + t.Run("API message requiring rewrite loses type but message is improved", func(t *testing.T) { + raw := fmt.Errorf("requires at least one of: artifact_fingerprint or git_commit_info. some detail") + err := wrapAttestationError(raw) + assert.Error(t, err) + assert.Contains(t, err.Error(), "requires at least one of: specifying the fingerprint") + }) +} diff --git a/cmd/kosli/cli_utils.go b/cmd/kosli/cli_utils.go index 506958699..e68510211 100644 --- a/cmd/kosli/cli_utils.go +++ b/cmd/kosli/cli_utils.go @@ -16,6 +16,7 @@ import ( "unicode" "github.com/kosli-dev/cli/internal/digest" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/gitview" log "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/utils" @@ -452,11 +453,11 @@ func ValidateSliceValues(values []string, allowedValues map[string]struct{}) err // ErrorBeforePrintingUsage func ErrorBeforePrintingUsage(cmd *cobra.Command, errMsg string) error { - return fmt.Errorf( + return kosliErrors.NewErrUsage(fmt.Sprintf( "%s\nUsage: %s", errMsg, cmd.UseLine(), - ) + )) } // tabFormattedPrint prints data in a tabular format. Takes header titles in a string slice diff --git a/cmd/kosli/cli_utils_test.go b/cmd/kosli/cli_utils_test.go index edb7ff145..8b5923a6d 100644 --- a/cmd/kosli/cli_utils_test.go +++ b/cmd/kosli/cli_utils_test.go @@ -1,11 +1,13 @@ package main import ( + "errors" "fmt" "os" "path/filepath" "testing" + kosliErrors "github.com/kosli-dev/cli/internal/errors" log "github.com/kosli-dev/cli/internal/logger" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -1093,3 +1095,20 @@ func TestValidateSliceValues(t *testing.T) { }) } } + +func (suite *CliUtilsTestSuite) TestErrorBeforePrintingUsageReturnsErrUsage() { + cmd := &cobra.Command{Use: "test"} + err := ErrorBeforePrintingUsage(cmd, "missing required flag") + require.Error(suite.T(), err) + var eu *kosliErrors.ErrUsage + require.True(suite.T(), errors.As(err, &eu), "expected ErrUsage, got %T: %v", err, err) +} + +func (suite *CliUtilsTestSuite) TestMissingRequiredFlagIsErrUsage() { + // kosli assert artifact requires --org and --api-token; omitting them triggers PreRunE + // which calls RequireGlobalFlags → ErrorBeforePrintingUsage + _, _, err := executeCommandC(`assert artifact --fingerprint abc123 --org myorg`) + require.Error(suite.T(), err) + var eu *kosliErrors.ErrUsage + require.True(suite.T(), errors.As(err, &eu), "expected ErrUsage, got %T: %v", err, err) +} diff --git a/cmd/kosli/docs.go b/cmd/kosli/docs.go index 113fe06cb..24b718fa8 100644 --- a/cmd/kosli/docs.go +++ b/cmd/kosli/docs.go @@ -12,6 +12,84 @@ import ( "github.com/spf13/cobra/doc" ) +// Exit code sets used across command doc generation. +var ( + // exitCodesDefault applies to all commands that call the Kosli API. + exitCodesDefault = []docgen.ExitCodeEntry{ + {Code: 0, Meaning: "No error."}, + {Code: 1, Meaning: "Unexpected error."}, + {Code: 2, Meaning: "Kosli server is unreachable or returned a server error."}, + {Code: 3, Meaning: "Invalid API token or unauthorized access."}, + {Code: 4, Meaning: "CLI usage error (e.g. missing or invalid flags)."}, + } + + // exitCodesAssert applies to assert commands that always signal compliance violations on failure. + exitCodesAssert = []docgen.ExitCodeEntry{ + {Code: 0, Meaning: "No error."}, + {Code: 1, Meaning: "Assertion/compliance violation."}, + {Code: 2, Meaning: "Kosli server is unreachable or returned a server error."}, + {Code: 3, Meaning: "Invalid API token or unauthorized access."}, + {Code: 4, Meaning: "CLI usage error (e.g. missing or invalid flags)."}, + } + + // exitCodesAttest applies to attest commands that support --assert flag. + exitCodesAttest = []docgen.ExitCodeEntry{ + {Code: 0, Meaning: "No error."}, + {Code: 1, Meaning: "Assertion/compliance violation (only when --assert is used)."}, + {Code: 2, Meaning: "Kosli server is unreachable or returned a server error."}, + {Code: 3, Meaning: "Invalid API token or unauthorized access."}, + {Code: 4, Meaning: "CLI usage error (e.g. missing or invalid flags)."}, + } + + // exitCodesAssertStatus applies to `kosli assert status` which only checks reachability. + exitCodesAssertStatus = []docgen.ExitCodeEntry{ + {Code: 0, Meaning: "Kosli server is responsive."}, + {Code: 2, Meaning: "Kosli server is unreachable or down."}, + } + + // exitCodesEvaluate applies to evaluate commands that run a policy and can signal policy denial. + exitCodesEvaluate = []docgen.ExitCodeEntry{ + {Code: 0, Meaning: "No error (policy allowed)."}, + {Code: 1, Meaning: "Policy denied."}, + {Code: 2, Meaning: "Kosli server is unreachable or returned a server error."}, + {Code: 3, Meaning: "Invalid API token or unauthorized access."}, + {Code: 4, Meaning: "CLI usage error (e.g. missing or invalid flags)."}, + } + + // exitCodesNoAPI applies to commands that do not call the Kosli API (version, completion, docs). + exitCodesNoAPI = []docgen.ExitCodeEntry{ + {Code: 0, Meaning: "No error."}, + {Code: 4, Meaning: "CLI usage error."}, + } +) + +// commandExitCodes maps command paths to their exit code sets. +// Commands not in this map receive exitCodesDefault. +var commandExitCodes = map[string][]docgen.ExitCodeEntry{ + // assert commands — can signal compliance violations + "kosli assert artifact": exitCodesAssert, + "kosli assert approval": exitCodesAssert, + "kosli assert snapshot": exitCodesAssert, + "kosli assert pullrequest github": exitCodesAssert, + "kosli assert pullrequest gitlab": exitCodesAssert, + "kosli assert pullrequest azure": exitCodesAssert, + "kosli assert pullrequest bitbucket": exitCodesAssert, + "kosli assert status": exitCodesAssertStatus, + // attest commands with --assert flag — can signal compliance violations when --assert is used + "kosli attest pullrequest github": exitCodesAttest, + "kosli attest pullrequest gitlab": exitCodesAttest, + "kosli attest pullrequest azure": exitCodesAttest, + "kosli attest pullrequest bitbucket": exitCodesAttest, + "kosli attest jira": exitCodesAttest, + // evaluate commands — policy denial exits with code 1 + "kosli evaluate trail": exitCodesEvaluate, + "kosli evaluate trails": exitCodesEvaluate, + // non-API commands + "kosli version": exitCodesNoAPI, + "kosli completion": exitCodesNoAPI, + "kosli docs": exitCodesNoAPI, +} + const docsShortDesc = `Generate documentation files for Kosli CLI. ` const docsLongDesc = docsShortDesc + ` @@ -58,8 +136,13 @@ func (o *docsOptions) run() error { } metaFn := func(cmd *cobra.Command) docgen.CommandMeta { + path := cmd.CommandPath() + exitCodes, ok := commandExitCodes[path] + if !ok && cmd.Runnable() { + exitCodes = exitCodesDefault + } return docgen.CommandMeta{ - Name: cmd.CommandPath(), + Name: path, Beta: isBeta(cmd), Deprecated: isDeprecated(cmd), DeprecMsg: cmd.Deprecated, @@ -68,6 +151,7 @@ func (o *docsOptions) run() error { UseLine: cmd.UseLine(), Runnable: cmd.Runnable(), Example: cmd.Example, + ExitCodes: exitCodes, } } diff --git a/cmd/kosli/evaluateHelpers.go b/cmd/kosli/evaluateHelpers.go index fd4c2aa9c..3eb572ba0 100644 --- a/cmd/kosli/evaluateHelpers.go +++ b/cmd/kosli/evaluateHelpers.go @@ -8,6 +8,7 @@ import ( "net/url" "os" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/evaluate" "github.com/kosli-dev/cli/internal/output" "github.com/kosli-dev/cli/internal/requests" @@ -136,7 +137,7 @@ func printEvaluateResultAsJson(raw string, out io.Writer, _ int) error { return err } if allow, ok := result["allow"].(bool); ok && !allow { - return fmt.Errorf("policy denied") + return kosliErrors.NewErrCompliance("policy denied") } return nil } @@ -167,8 +168,8 @@ func printEvaluateResultAsTable(raw string, out io.Writer, _ int) error { } } tabFormattedPrint(out, []string{}, rows) - return fmt.Errorf("policy denied: %v", violations) + return kosliErrors.NewErrCompliance(fmt.Sprintf("policy denied: %v", violations)) } tabFormattedPrint(out, []string{}, rows) - return fmt.Errorf("policy denied") + return kosliErrors.NewErrCompliance("policy denied") } diff --git a/cmd/kosli/evaluateHelpers_test.go b/cmd/kosli/evaluateHelpers_test.go new file mode 100644 index 000000000..79802fa2d --- /dev/null +++ b/cmd/kosli/evaluateHelpers_test.go @@ -0,0 +1,36 @@ +package main + +import ( + "bytes" + "errors" + "testing" + + kosliErrors "github.com/kosli-dev/cli/internal/errors" + "github.com/stretchr/testify/assert" +) + +func TestPolicyDenialIsErrCompliance(t *testing.T) { + deniedJSON := `{"allow":false,"violations":["missing approval"]}` + + t.Run("printEvaluateResultAsJson returns ErrCompliance on policy denial", func(t *testing.T) { + err := printEvaluateResultAsJson(deniedJSON, &bytes.Buffer{}, 0) + var ec *kosliErrors.ErrCompliance + assert.True(t, errors.As(err, &ec), "expected ErrCompliance, got %T: %v", err, err) + }) + + t.Run("printEvaluateResultAsTable returns ErrCompliance on policy denial", func(t *testing.T) { + err := printEvaluateResultAsTable(deniedJSON, &bytes.Buffer{}, 0) + var ec *kosliErrors.ErrCompliance + assert.True(t, errors.As(err, &ec), "expected ErrCompliance, got %T: %v", err, err) + }) + + t.Run("printEvaluateResultAsJson returns nil on policy allow", func(t *testing.T) { + err := printEvaluateResultAsJson(`{"allow":true,"violations":[]}`, &bytes.Buffer{}, 0) + assert.NoError(t, err) + }) + + t.Run("printEvaluateResultAsTable returns nil on policy allow", func(t *testing.T) { + err := printEvaluateResultAsTable(`{"allow":true,"violations":[]}`, &bytes.Buffer{}, 0) + assert.NoError(t, err) + }) +} diff --git a/cmd/kosli/evaluateTrail_test.go b/cmd/kosli/evaluateTrail_test.go index 38601513d..913940f9e 100644 --- a/cmd/kosli/evaluateTrail_test.go +++ b/cmd/kosli/evaluateTrail_test.go @@ -37,33 +37,38 @@ func (suite *EvaluateTrailCommandTestSuite) SetupTest() { func (suite *EvaluateTrailCommandTestSuite) TestEvaluateTrailCmd() { tests := []cmdTestCase{ { - wantError: true, - name: "missing trail name argument fails", - cmd: fmt.Sprintf(`evaluate trail --flow %s %s`, suite.flowName, suite.defaultKosliArguments), - golden: "Error: accepts 1 arg(s), received 0\n", + wantError: true, + wantExitCode: 4, + name: "missing trail name argument fails", + cmd: fmt.Sprintf(`evaluate trail --flow %s %s`, suite.flowName, suite.defaultKosliArguments), + golden: "Error: accepts 1 arg(s), received 0\n", }, { - wantError: true, - name: "providing more than one argument fails", - cmd: fmt.Sprintf(`evaluate trail %s xxx --flow %s %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), - golden: "Error: accepts 1 arg(s), received 2\n", + wantError: true, + wantExitCode: 4, + name: "providing more than one argument fails", + cmd: fmt.Sprintf(`evaluate trail %s xxx --flow %s %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), + golden: "Error: accepts 1 arg(s), received 2\n", }, { - wantError: true, - name: "missing --flow flag fails", - cmd: fmt.Sprintf(`evaluate trail %s %s`, suite.trailName, suite.defaultKosliArguments), - golden: "Error: required flag(s) \"flow\", \"policy\" not set\n", + wantError: true, + wantExitCode: 4, + name: "missing --flow flag fails", + cmd: fmt.Sprintf(`evaluate trail %s %s`, suite.trailName, suite.defaultKosliArguments), + golden: "Error: required flag(s) \"flow\", \"policy\" not set\n", }, { - wantError: true, - name: "missing --policy flag fails", - cmd: fmt.Sprintf(`evaluate trail %s --flow %s %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), - golden: "Error: required flag(s) \"policy\" not set\n", + wantError: true, + wantExitCode: 4, + name: "missing --policy flag fails", + cmd: fmt.Sprintf(`evaluate trail %s --flow %s %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), + golden: "Error: required flag(s) \"policy\" not set\n", }, { - wantError: true, - name: "missing --api-token fails", - cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/allow-all.rego --org orgX`, suite.trailName, suite.flowName), + wantError: true, + wantExitCode: 4, + name: "missing --api-token fails", + cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/allow-all.rego --org orgX`, suite.trailName, suite.flowName), }, { wantError: true, @@ -76,9 +81,10 @@ func (suite *EvaluateTrailCommandTestSuite) TestEvaluateTrailCmd() { cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/allow-all.rego %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), }, { - wantError: true, - name: "with --policy deny-all exits 1", - cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/deny-all.rego %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), + wantError: true, + wantExitCode: 1, + name: "with --policy deny-all exits 1", + cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/deny-all.rego %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), }, { wantError: true, @@ -96,10 +102,11 @@ func (suite *EvaluateTrailCommandTestSuite) TestEvaluateTrailCmd() { goldenJson: []jsonCheck{{"allow", true}}, }, { - wantError: true, - name: "with --policy deny-all --output json prints JSON with allow false and violations", - cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/deny-all.rego --output json %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), - goldenRegex: `(?s)"allow":\s*false.*"violations":\s*\[.*"always denied"`, + wantError: true, + wantExitCode: 1, + name: "with --policy deny-all --output json prints JSON with allow false and violations", + cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/deny-all.rego --output json %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), + goldenRegex: `(?s)"allow":\s*false.*"violations":\s*\[.*"always denied"`, }, { name: "with --policy allow-all --output table prints allowed text", @@ -107,10 +114,11 @@ func (suite *EvaluateTrailCommandTestSuite) TestEvaluateTrailCmd() { goldenRegex: `RESULT:\s+ALLOWED`, }, { - wantError: true, - name: "with --policy deny-all --output table prints denied text with violations", - cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/deny-all.rego --output table %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), - goldenRegex: `RESULT:\s+DENIED\nVIOLATIONS:\s+always denied`, + wantError: true, + wantExitCode: 1, + name: "with --policy deny-all --output table prints denied text with violations", + cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/deny-all.rego --output table %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), + goldenRegex: `RESULT:\s+DENIED\nVIOLATIONS:\s+always denied`, }, { name: "with --policy allow-all and no --output defaults to table output", @@ -129,10 +137,11 @@ func (suite *EvaluateTrailCommandTestSuite) TestEvaluateTrailCmd() { goldenJson: []jsonCheck{{"allow", true}, {"input.trail.name", suite.trailName}}, }, { - wantError: true, - name: "with --policy deny-all --output json --show-input includes input alongside allow and violations", - cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/deny-all.rego --output json --show-input %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), - goldenRegex: `(?s)"allow":\s*false.*"input":\s*\{.*"trail".*"violations":\s*\[.*"always denied"`, + wantError: true, + wantExitCode: 1, + name: "with --policy deny-all --output json --show-input includes input alongside allow and violations", + cmd: fmt.Sprintf(`evaluate trail %s --flow %s --policy testdata/policies/deny-all.rego --output json --show-input %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), + goldenRegex: `(?s)"allow":\s*false.*"input":\s*\{.*"trail".*"violations":\s*\[.*"always denied"`, }, { name: "with --policy allow-all --output table --show-input ignores show-input", diff --git a/cmd/kosli/evaluateTrails_test.go b/cmd/kosli/evaluateTrails_test.go index 68f33706a..90db4ee86 100644 --- a/cmd/kosli/evaluateTrails_test.go +++ b/cmd/kosli/evaluateTrails_test.go @@ -34,27 +34,31 @@ func (suite *EvaluateTrailsCommandTestSuite) SetupTest() { func (suite *EvaluateTrailsCommandTestSuite) TestEvaluateTrailsCmd() { tests := []cmdTestCase{ { - wantError: true, - name: "missing trail name argument fails", - cmd: fmt.Sprintf(`evaluate trails --flow %s %s`, suite.flowName, suite.defaultKosliArguments), - golden: "Error: requires at least 1 arg(s), only received 0\n", + wantError: true, + wantExitCode: 4, + name: "missing trail name argument fails", + cmd: fmt.Sprintf(`evaluate trails --flow %s %s`, suite.flowName, suite.defaultKosliArguments), + golden: "Error: requires at least 1 arg(s), only received 0\n", }, { - wantError: true, - name: "missing --flow flag fails", - cmd: fmt.Sprintf(`evaluate trails %s %s`, suite.trailName, suite.defaultKosliArguments), - golden: "Error: required flag(s) \"flow\", \"policy\" not set\n", + wantError: true, + wantExitCode: 4, + name: "missing --flow flag fails", + cmd: fmt.Sprintf(`evaluate trails %s %s`, suite.trailName, suite.defaultKosliArguments), + golden: "Error: required flag(s) \"flow\", \"policy\" not set\n", }, { - wantError: true, - name: "missing --policy flag fails", - cmd: fmt.Sprintf(`evaluate trails %s --flow %s %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), - golden: "Error: required flag(s) \"policy\" not set\n", + wantError: true, + wantExitCode: 4, + name: "missing --policy flag fails", + cmd: fmt.Sprintf(`evaluate trails %s --flow %s %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), + golden: "Error: required flag(s) \"policy\" not set\n", }, { - wantError: true, - name: "missing --api-token fails", - cmd: fmt.Sprintf(`evaluate trails %s --flow %s --policy testdata/policies/allow-all.rego --org orgX`, suite.trailName, suite.flowName), + wantError: true, + wantExitCode: 4, + name: "missing --api-token fails", + cmd: fmt.Sprintf(`evaluate trails %s --flow %s --policy testdata/policies/allow-all.rego --org orgX`, suite.trailName, suite.flowName), }, { wantError: true, @@ -67,9 +71,10 @@ func (suite *EvaluateTrailsCommandTestSuite) TestEvaluateTrailsCmd() { cmd: fmt.Sprintf(`evaluate trails %s --flow %s --policy testdata/policies/allow-all.rego %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), }, { - wantError: true, - name: "with --policy deny-all exits 1", - cmd: fmt.Sprintf(`evaluate trails %s --flow %s --policy testdata/policies/deny-all.rego %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), + wantError: true, + wantExitCode: 1, + name: "with --policy deny-all exits 1", + cmd: fmt.Sprintf(`evaluate trails %s --flow %s --policy testdata/policies/deny-all.rego %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), }, { name: "with --output json and --policy prints allow/violations JSON", @@ -82,10 +87,11 @@ func (suite *EvaluateTrailsCommandTestSuite) TestEvaluateTrailsCmd() { goldenRegex: `RESULT:\s+ALLOWED`, }, { - wantError: true, - name: "with --output table and deny --policy prints DENIED text with violations", - cmd: fmt.Sprintf(`evaluate trails %s --flow %s --policy testdata/policies/deny-all.rego --output table %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), - goldenRegex: `RESULT:\s+DENIED\nVIOLATIONS:\s+always denied`, + wantError: true, + wantExitCode: 1, + name: "with --output table and deny --policy prints DENIED text with violations", + cmd: fmt.Sprintf(`evaluate trails %s --flow %s --policy testdata/policies/deny-all.rego --output table %s`, suite.trailName, suite.flowName, suite.defaultKosliArguments), + goldenRegex: `RESULT:\s+DENIED\nVIOLATIONS:\s+always denied`, }, { wantError: true, diff --git a/cmd/kosli/exitcodes_test.go b/cmd/kosli/exitcodes_test.go new file mode 100644 index 000000000..596f56a30 --- /dev/null +++ b/cmd/kosli/exitcodes_test.go @@ -0,0 +1,119 @@ +package main + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + kosliErrors "github.com/kosli-dev/cli/internal/errors" + "github.com/stretchr/testify/assert" +) + +// TestExitCodeScenarios verifies that each documented exit code is produced +// by the right class of error. Uses executeCommandC so no binary build is needed. +// +// Codes covered: +// +// 0 — success +// 1 — compliance/policy violation +// 2 — server unreachable or 5xx +// 3 — invalid API token (401/403) +// 4 — CLI usage error (missing flags, unknown flag, wrong arg count) +func TestExitCodeScenarios(t *testing.T) { + assertCode := func(t *testing.T, cmd string, want int) { + t.Helper() + _, _, err := executeCommandC(cmd) + got := kosliErrors.ExitCodeFor(err) + assert.Equal(t, want, got, "command: %s\nerror: %v", cmd, err) + } + + // ── exit 0: success ─────────────────────────────────────────────────────── + + t.Run("exit 0: kosli version succeeds", func(t *testing.T) { + assertCode(t, "version", kosliErrors.ExitOK) + }) + + // ── exit 1: compliance / policy violation ───────────────────────────────── + + t.Run("exit 1: evaluate trail with deny-all policy", func(t *testing.T) { + srv := newStaticJSONServer(t, http.StatusOK, `{"name":"my-trail","compliance_status":{}}`) + cmd := fmt.Sprintf( + "evaluate trail my-trail --flow my-flow --policy testdata/policies/deny-all.rego --host %s --org test-org --api-token secret", + srv.URL, + ) + assertCode(t, cmd, kosliErrors.ExitCompliance) + }) + + // ── exit 2: server unreachable / 5xx ────────────────────────────────────── + + t.Run("exit 2: server returns 500", func(t *testing.T) { + srv := newStaticJSONServer(t, http.StatusInternalServerError, `{"message":"internal error"}`) + cmd := fmt.Sprintf( + "list environments --host %s --org test-org --api-token secret --max-api-retries 0", + srv.URL, + ) + assertCode(t, cmd, kosliErrors.ExitServer) + }) + + t.Run("exit 2: server unreachable (bad host)", func(t *testing.T) { + assertCode(t, + "list environments --host http://localhost:19999 --org test-org --api-token secret --max-api-retries 0", + kosliErrors.ExitServer, + ) + }) + + // ── exit 3: invalid API token ───────────────────────────────────────────── + + t.Run("exit 3: server returns 401", func(t *testing.T) { + srv := newStaticJSONServer(t, http.StatusUnauthorized, `{"message":"unauthorized"}`) + cmd := fmt.Sprintf( + "list environments --host %s --org test-org --api-token bad-token", + srv.URL, + ) + assertCode(t, cmd, kosliErrors.ExitConfig) + }) + + t.Run("exit 3: server returns 403", func(t *testing.T) { + srv := newStaticJSONServer(t, http.StatusForbidden, `{"message":"forbidden"}`) + cmd := fmt.Sprintf( + "list environments --host %s --org test-org --api-token bad-token", + srv.URL, + ) + assertCode(t, cmd, kosliErrors.ExitConfig) + }) + + // ── exit 4: CLI usage errors ────────────────────────────────────────────── + + t.Run("exit 4: unknown flag", func(t *testing.T) { + assertCode(t, "version --no-such-flag", kosliErrors.ExitUsage) + }) + + t.Run("exit 4: cobra required flag not set (--flow, --policy)", func(t *testing.T) { + assertCode(t, + "evaluate trail my-trail --host http://localhost:8001 --org test-org --api-token secret", + kosliErrors.ExitUsage, + ) + }) + + t.Run("exit 4: wrong number of arguments", func(t *testing.T) { + // evaluate trail requires exactly 1 positional argument + assertCode(t, + "evaluate trail --host http://localhost:8001 --org test-org --api-token secret", + kosliErrors.ExitUsage, + ) + }) +} + +// newStaticJSONServer starts a test HTTP server that always responds with the +// given status code and JSON body, and closes itself when the test ends. +func newStaticJSONServer(t *testing.T, status int, body string) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = fmt.Fprint(w, body) + })) + t.Cleanup(srv.Close) + return srv +} diff --git a/cmd/kosli/main.go b/cmd/kosli/main.go index ef60bd0cc..5eb97de93 100644 --- a/cmd/kosli/main.go +++ b/cmd/kosli/main.go @@ -5,6 +5,7 @@ import ( "os" "strings" + kosliErrors "github.com/kosli-dev/cli/internal/errors" log "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" @@ -36,7 +37,8 @@ func main() { } } if err != nil { - logger.Error(err.Error()) + fmt.Fprintf(os.Stderr, "Error: %s\n", err.Error()) + os.Exit(kosliErrors.ExitCodeFor(err)) } } @@ -49,9 +51,9 @@ func innerMain(cmd *cobra.Command, args []string) error { // cobra does not capture unknown/missing commands, see https://github.com/spf13/cobra/issues/706 // so we handle this here until it is fixed in cobra if strings.Contains(err.Error(), "unknown flag:") { - c, flags, err := cmd.Traverse(args[1:]) - if err != nil { - return err + c, flags, traverseErr := cmd.Traverse(args[1:]) + if traverseErr != nil { + return kosliErrors.NewErrUsage(traverseErr.Error()) } if c.HasSubCommands() { errMessage := "" @@ -66,8 +68,9 @@ func innerMain(cmd *cobra.Command, args []string) error { availableSubcommands = append(availableSubcommands, strings.Split(sc.Use, " ")[0]) } } - logger.Error("%s\navailable subcommands are: %s", errMessage, strings.Join(availableSubcommands, " | ")) + fmt.Fprintf(os.Stderr, "Error: %s\navailable subcommands are: %s\n", errMessage, strings.Join(availableSubcommands, " | ")) } + return kosliErrors.NewErrUsage(err.Error()) } if global.DryRun { logger.Info("Error: %s", err.Error()) diff --git a/cmd/kosli/pullrequest.go b/cmd/kosli/pullrequest.go index 72cae3b2c..b6d861ffe 100644 --- a/cmd/kosli/pullrequest.go +++ b/cmd/kosli/pullrequest.go @@ -9,6 +9,7 @@ import ( azUtils "github.com/kosli-dev/cli/internal/azure" bbUtils "github.com/kosli-dev/cli/internal/bitbucket" + kosliErrors "github.com/kosli-dev/cli/internal/errors" ghUtils "github.com/kosli-dev/cli/internal/github" gitlabUtils "github.com/kosli-dev/cli/internal/gitlab" "github.com/kosli-dev/cli/internal/requests" @@ -85,7 +86,7 @@ func (o *attestPROptions) run(args []string) error { if err != nil { errString = fmt.Sprintf("%s\nError: ", err.Error()) } - err = fmt.Errorf("%sassert failed: no %s found for the given commit: %s", errString, label, o.payload.Commit.Sha1) + err = kosliErrors.NewErrCompliance(fmt.Sprintf("%sassert failed: no %s found for the given commit: %s", errString, label, o.payload.Commit.Sha1)) } return wrapAttestationError(err) diff --git a/cmd/kosli/status.go b/cmd/kosli/status.go index 1e50dcf61..37aab4e65 100644 --- a/cmd/kosli/status.go +++ b/cmd/kosli/status.go @@ -6,6 +6,7 @@ import ( "net/http" "net/url" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" ) @@ -52,7 +53,7 @@ func (o *statusOptions) run(out io.Writer) error { if err != nil { logger.Debug("failed to check Kosli's readiness: %s", err.Error()) if o.assert { - return fmt.Errorf("kosli server %s is unresponsive", global.Host) + return kosliErrors.NewErrServer(fmt.Sprintf("kosli server %s is unresponsive", global.Host)) } logger.Info("Kosli is Down") } else { diff --git a/cmd/kosli/testHelpers.go b/cmd/kosli/testHelpers.go index 4279c9e7f..27e5bb66e 100644 --- a/cmd/kosli/testHelpers.go +++ b/cmd/kosli/testHelpers.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/gitview" shellwords "github.com/mattn/go-shellwords" "github.com/pkg/errors" @@ -32,6 +33,7 @@ type cmdTestCase struct { goldenRegex string goldenJson []jsonCheck // Use like this for array {"[0].compliant", false} wantError bool + wantExitCode int // when non-zero, asserts ExitCodeFor(err) == wantExitCode additionalConfig interface{} } @@ -80,6 +82,11 @@ func runTestCmd(t *testing.T, tests []cmdTestCase) { if (err != nil) != tt.wantError { t.Errorf("error expectation not matched\n\n WANT error is: %t\n\n but GOT: '%v'", tt.wantError, err) } + if tt.wantExitCode != 0 { + if got := kosliErrors.ExitCodeFor(err); got != tt.wantExitCode { + t.Errorf("exit code mismatch: want %d, got %d (error: %v)", tt.wantExitCode, got, err) + } + } if tt.golden != "" { if !bytes.Equal([]byte(tt.golden), []byte(out)) { t.Errorf("does not match golden\n\nWANT:\n'%s'\n\nGOT:\n'%s'\n", tt.golden, out) diff --git a/cmd/kosli/testdata/output/docs/hugo/artifact.md b/cmd/kosli/testdata/output/docs/hugo/artifact.md index 57f1a4a9a..37e6065de 100644 --- a/cmd/kosli/testdata/output/docs/hugo/artifact.md +++ b/cmd/kosli/testdata/output/docs/hugo/artifact.md @@ -35,6 +35,16 @@ artifact {IMAGE-NAME | FILE-PATH | DIR-PATH} [flags] | --repo-root string | [defaulted] The directory where the source git repository is available. (default ".") | +## Exit Codes +| Code | Meaning | +| :---: | :--- | +| 0 | No error. | +| 1 | Unexpected error. | +| 2 | Kosli server is unreachable or returned a server error. | +| 3 | Invalid API token or unauthorized access. | +| 4 | CLI usage error (e.g. missing or invalid flags). | + + ## Examples ```shell diff --git a/cmd/kosli/testdata/output/docs/hugo/snyk.md b/cmd/kosli/testdata/output/docs/hugo/snyk.md index 9c376ab4a..0b4829d49 100644 --- a/cmd/kosli/testdata/output/docs/hugo/snyk.md +++ b/cmd/kosli/testdata/output/docs/hugo/snyk.md @@ -64,6 +64,16 @@ binding the attestation to the right artifact. | -u, --user-data string | [optional] The path to a JSON file containing additional data you would like to attach to the attestation. | +## Exit Codes +| Code | Meaning | +| :---: | :--- | +| 0 | No error. | +| 1 | Unexpected error. | +| 2 | Kosli server is unreachable or returned a server error. | +| 3 | Invalid API token or unauthorized access. | +| 4 | CLI usage error (e.g. missing or invalid flags). | + + ## Examples Use Cases These examples all assume that the flags `--api-token`, `--org`, `--host`, (and `--flow`, `--trail` when required), are [set/provided](https://docs.kosli.com/getting_started/install/#assigning-flags-via-environment-variables). diff --git a/cmd/kosli/testdata/output/docs/mintlify/artifact.md b/cmd/kosli/testdata/output/docs/mintlify/artifact.md index 40d6941c5..dc34e9a6f 100644 --- a/cmd/kosli/testdata/output/docs/mintlify/artifact.md +++ b/cmd/kosli/testdata/output/docs/mintlify/artifact.md @@ -42,6 +42,16 @@ images in registries or "docker" for local docker images. | --repo-root string | [defaulted] The directory where the source git repository is available. (default ".") | +## Exit Codes +| Code | Meaning | +| :---: | :--- | +| 0 | No error. | +| 1 | Unexpected error. | +| 2 | Kosli server is unreachable or returned a server error. | +| 3 | Invalid API token or unauthorized access. | +| 4 | CLI usage error (e.g. missing or invalid flags). | + + ## Examples Use Cases These examples all assume that the flags `--api-token`, `--org`, `--host`, (and `--flow`, `--trail` when required), are [set/provided](/getting_started/install/#assigning-flags-via-environment-variables). diff --git a/cmd/kosli/testdata/output/docs/mintlify/snyk.md b/cmd/kosli/testdata/output/docs/mintlify/snyk.md index b91fb9ee3..9a11ab4e8 100644 --- a/cmd/kosli/testdata/output/docs/mintlify/snyk.md +++ b/cmd/kosli/testdata/output/docs/mintlify/snyk.md @@ -62,6 +62,16 @@ binding the attestation to the right artifact. | -u, --user-data string | [optional] The path to a JSON file containing additional data you would like to attach to the attestation. | +## Exit Codes +| Code | Meaning | +| :---: | :--- | +| 0 | No error. | +| 1 | Unexpected error. | +| 2 | Kosli server is unreachable or returned a server error. | +| 3 | Invalid API token or unauthorized access. | +| 4 | CLI usage error (e.g. missing or invalid flags). | + + ## Examples Use Cases These examples all assume that the flags `--api-token`, `--org`, `--host`, (and `--flow`, `--trail` when required), are [set/provided](/getting_started/install/#assigning-flags-via-environment-variables). diff --git a/docs/adr/20260319-differentiated-exit-codes.md b/docs/adr/20260319-differentiated-exit-codes.md new file mode 100644 index 000000000..eb6aa509c --- /dev/null +++ b/docs/adr/20260319-differentiated-exit-codes.md @@ -0,0 +1,157 @@ +--- +title: "20260319 - Differentiated exit codes for the Kosli CLI" +description: "Differentiate the existing uniform exit-1-on-all-errors into structured exit codes that distinguish compliance failures, server errors, config errors, and usage errors" +status: "Proposed" +date: "2026-03-19" +--- + +# 20260319 - Differentiated exit codes for the Kosli CLI + +## Overview + +Introduce structured, semantic exit codes to the Kosli CLI so that callers (CI/CD pipelines, shell scripts) can distinguish between a compliance/policy violation and other failure modes without parsing stderr output. + +## Context + +Prior to this change, the Kosli CLI exits with code `1` for **all** errors. This is because `logger.Error()` in `internal/logger/logger.go` calls `log.Fatalf()`, which unconditionally calls `os.Exit(1)`: + +```go +// internal/logger/logger.go — current behaviour +func (l *Logger) Error(format string, v ...interface{}) { + format = fmt.Sprintf("Error: %s\n", format) + l.errLog.Fatalf(format, v...) // always exits with code 1 +} +``` + +In `cmd/kosli/main.go`, every error flows through this path: + +```go +if err != nil { + logger.Error(err.Error()) // exits 1 via Fatalf — always +} +``` + +This means: +- A script running `kosli assert artifact` gets exit 1 whether the artifact is non-compliant, the server is down, or the API token is wrong. +- A CI pipeline cannot distinguish a network outage (retry-able) from a policy denial (actionable) from a bad token (fix config). +- The only way to differentiate failures is to parse stderr output. + +## Decision Drivers + +- CI/CD pipelines need to fail fast on compliance violations without parsing stderr +- Operators need to distinguish transient server failures (retry) from auth failures (fix token) from compliance violations (fix the artifact/policy) +- Shell scripts using `$?` should be able to act on the specific class of failure +- The change should preserve the existing exit-1 behaviour for compliance failures (the most common case) to minimise disruption + +## Options Considered + +### Option A: Keep uniform exit 1 for all errors (status quo) +- **Pro**: No change; no migration needed +- **Con**: Cannot distinguish failure types without parsing stderr; limits CI/CD automation + +### Option B: Structured exit codes — 0/1/2/3/4 with semantic meaning (chosen) +- **Pro**: Rich signalling; enables CI/CD to route failures to the right handler; aligns with how tools like `git`, `curl`, and policy engines behave +- **Con**: Scripts checking `[ $? -eq 1 ]` as a proxy for "any error" will miss exit codes 2, 3, 4 +- **Mitigation**: Exit 1 remains the most common failure code (compliance violations AND the default for unclassified errors), so most existing `[ $? -eq 1 ]` checks will continue to work for the dominant use case + +## Decision + +**Option B — structured exit codes.** + +| Code | Meaning | Error type | +|------|---------|------------| +| 0 | Success | — | +| 1 | Compliance/policy violation (also: unclassified errors) | `ErrCompliance` | +| 2 | Server unreachable or 5xx | `ErrServer` | +| 3 | Auth/config error (401/403) | `ErrConfig` | +| 4 | CLI usage error (unknown flag, missing required flag, wrong arg count) | `ErrUsage` | + +Unknown/unclassified errors fall back to exit 1 (same as before). + +Commands affected: +- **Assert commands** (8): `assert artifact`, `assert approval`, `assert snapshot`, `assert pullrequest *` — exit 1 on compliance failure (unchanged) +- **Attest commands with `--assert`** (5): `attest pullrequest *`, `attest jira` — exit 1 when assertion fails (unchanged) +- **Evaluate commands** (2): `evaluate trail`, `evaluate trails` — exit 1 when policy denies (unchanged) +- **All commands**: exit 2 on server/network failure, exit 3 on 401/403, exit 4 on usage errors (NEW — previously these all exited 1) +- Special case: `assert status` exits 0 (responsive) or 2 (unreachable) — never 1 + +## Consequences + +### Positive +- Compliance assertions remain usable as hard gates (exit 1, same as before) +- Operators can write scripts that retry on exit 2, fix config on exit 3, and triage policy violations on exit 1 +- Exit code meaning is documented per-command in generated CLI docs (via `commandExitCodes` in `cmd/kosli/docs.go`) +- Dry-run mode is preserved: errors in `--dry-run` still exit 0 + +### Negative +- Scripts checking `[ $? -eq 1 ]` as a proxy for "any error" will stop catching server errors (now exit 2), auth errors (now exit 3), and usage errors (now exit 4). Scripts using `[ $? -ne 0 ]` are unaffected. +- Whether this warrants a major version bump is debatable — exit code 1 was never documented as meaning "any error", but it was the de facto implicit contract via `Fatalf`. + +### Neutral +- Cobra built-in usage errors (unknown flags, missing required flags, wrong argument count) are auto-detected via string pattern matching and classified as `ErrUsage` without requiring explicit wrapping in each command. +- HTTP 4xx errors other than 401/403 (e.g. 400, 404) return plain `fmt.Errorf` and fall through to exit 1 — consistent with treating them as data/compliance failures. + +## Implementation + +Error types are thin wrappers in `internal/errors/errors.go`: + +```go +type ErrCompliance struct{ msg string } +type ErrServer struct{ msg string } +type ErrConfig struct{ msg string } +type ErrUsage struct{ msg string } + +func ExitCodeFor(err error) int { + if err == nil { return 0 } + var e1 ErrCompliance; if errors.As(err, &e1) { return 1 } + var e2 ErrServer; if errors.As(err, &e2) { return 2 } + var e3 ErrConfig; if errors.As(err, &e3) { return 3 } + var e4 ErrUsage; if errors.As(err, &e4) { return 4 } + if isCobraUsageError(err) { return 4 } + return 1 // safe default +} +``` + +`errors.As` correctly unwraps chained errors (`fmt.Errorf("...: %w", ErrCompliance{...})`). + +The exit code dispatch in `cmd/kosli/main.go` bypasses `logger.Error()` (which calls `log.Fatalf` → `os.Exit(1)`) and writes to stderr directly: + +```go +if err != nil { + fmt.Fprintf(os.Stderr, "Error: %s\n", err.Error()) + os.Exit(kosliErrors.ExitCodeFor(err)) +} +``` + +This is necessary because ~40 command factory functions call `logger.Error()` for fatal setup errors and rely on `Fatalf` to halt — changing `logger.Error` itself would require auditing all those call sites. + +## Review Process + +This branch was reviewed by a five-agent code review swarm (Opus 4.6) before the fixes were applied. + +### Bugs found and fixed + +1. **Exit code dispatch was dead code** — `logger.Error()` in `main.go` called `log.Fatalf` → `os.Exit(1)` before `ExitCodeFor` could run. All errors exited 1 regardless of type. Tests didn't catch this because they call `ExitCodeFor(err)` on the returned error directly, never going through `main()`. Fixed by replacing `logger.Error` with `fmt.Fprintf` in `main.go` only. + +2. **Same issue in `innerMain()`** — `logger.Error` at the unknown-subcommand path exited before `return ErrUsage` could execute. Fixed the same way. + +3. **HTTP 5xx not classified as `ErrServer`** — In `internal/requests/requests.go`, 5xx responses (when retries are exhausted) were returned as plain `fmt.Errorf`, falling through to exit 1 instead of exit 2. Fixed by adding a `resp.StatusCode >= 500` check. + +### Verified after fixes + +``` +$ ./kosli version # exit 0 +$ KOSLI_API_TOKEN=bad ./kosli list environments --org x # exit 3 +$ ./kosli list environments --host http://localhost:1 ... # exit 2 +$ ./kosli list environments --bogus-flag # exit 4 +``` + +### Remaining work — test coverage + +55 `wantExitCode` annotations now exist across 14 test files, covering all assert, attest `--assert`, and evaluate command families with compliance failure (exit 1), usage error (exit 4), and success regression guard (exit 0) cases. + +No end-to-end test verifies the actual compiled binary's process exit code. The current tests validate error classification logic in isolation. + +## Related Decisions + +- [20260302 - Client-side policy evaluation](./20260302-client-side-policy-evaluation.md) — the evaluate command whose exit behaviour this ADR formalises diff --git a/docs/adr/20260320-error-reporting-strategy.md b/docs/adr/20260320-error-reporting-strategy.md new file mode 100644 index 000000000..057217ea7 --- /dev/null +++ b/docs/adr/20260320-error-reporting-strategy.md @@ -0,0 +1,359 @@ +--- +title: "20260320 - Error reporting strategy: exit codes vs structured JSON envelope" +description: "Evaluate whether the CLI should report errors via numeric exit codes, structured JSON envelopes, or a hybrid — informed by codebase analysis and industry research" +status: "Proposed" +date: "2026-03-20" +--- + +# 20260320 - Error reporting strategy: exit codes vs structured JSON envelope + +## Context + +[ADR 20260319](./20260319-differentiated-exit-codes.md) introduced structured exit codes (0/1/2/3/4) to replace the uniform exit-1-on-all-errors behaviour. Before merging that work, we want to evaluate whether a fundamentally different approach — structured JSON error output — would serve consumers better, and choose one path before the exit code contract becomes load-bearing. + +### What the swarm investigation found + +**Error surface analysis** — The CLI has ~80 commands across 6 categories. Attestation commands have the highest error diversity: a single `attest pullrequest` invocation can fail in 5+ distinct ways (flag validation, fingerprint calculation, git access, VCS provider API, Kosli server, compliance assertion). Several commands already emit **multiple errors per invocation** (e.g. a server error followed by an assertion failure). A numeric exit code can only represent one of these. + +**Consumption pattern analysis** — Today, all consumers rely on `$?` and plain-text stderr: +- GitHub Actions workflows use `kosli-dev/setup-cli-action@v2` and rely on step-level failure (non-zero = step fails) +- Shell scripts check `$?` (e.g. `bin/test_install_script_over_homebrew.sh`) +- No SDK wrappers or programmatic JSON parsers exist +- `--output json` only affects success output; errors are always plain text on stderr +- The server returns structured JSON errors (`{"message": "...", "errors": [...]}`) but the CLI flattens them to plain strings before printing + +**Industry research** — How comparable tools handle it: + +| Tool | Exit codes | Structured errors | Notes | +|------|-----------|-------------------|-------| +| curl | ~100 codes | No | Most granular; CI uses `case $?` | +| git | 0, 1, 128, 129 | No | Exit 1 overloaded (error vs "differences found") | +| kubectl | 0, 1 | Partial (K8s Status object on stderr) | Programmatic users prefer client-go | +| gh | 0, 1, 2, 4 | Partial (some JSON on stderr) | Inconsistent — not all errors are JSON | +| AWS CLI | 0, 1, 2, 130, 252-255 | Yes (JSON envelope with `Error.Code`) | Gold standard for programmatic use | +| gcloud | 0, 1, 2 | No | Widely seen as a weakness | +| Terraform | 0, 1, 2 | Yes (`-json` flag covers everything) | Best hybrid: exit code + NDJSON stream | + +No RFC or standard exists for CLI error output. The closest references are BSD `sysexits.h` (largely ignored by modern tools) and RFC 9457 (Problem Details for HTTP APIs, adaptable to CLI). + +## Decision Drivers + +1. **Who consumes errors programmatically today?** Shell scripts and CI pipelines — both understand `$?` natively. +2. **Who might consume errors programmatically in the future?** SDK wrappers, GitHub Actions composites, Terraform providers, or orchestration tools calling `kosli` as a subprocess. +3. **Can a single exit code represent the error surface?** For most commands, yes. For commands with multi-error paths (attest with `--assert`, evaluate), a single code loses information. +4. **What is the migration cost?** Exit codes are already partially shipped (ADR 20260319). JSON envelopes would require changes to every consumer. +5. **What does the Kosli server already return?** Structured JSON errors with `message` and `errors` fields — the CLI currently discards this structure. + +## Open Design Questions + +These questions apply to any option that uses exit codes (A, C, and the ranged variant A2). They were raised during [PR #714 review](https://github.com/kosli-dev/cli/pull/714#issuecomment-4097867565). + +### Should exit codes use spaced ranges for future subdivision? + +The current scheme packs codes tightly (1, 2, 3, 4). An alternative uses ranges with gaps: + +``` +┌───────┬───────────────────┐ +│ Range │ Domain │ +├───────┼───────────────────┤ +│ 51-59 │ Compliance/policy │ +├───────┼───────────────────┤ +│ 61-69 │ Server/infra │ +├───────┼───────────────────┤ +│ 71-79 │ Auth/config │ +├───────┼───────────────────┤ +│ 81-89 │ Usage │ +└───────┴───────────────────┘ +``` + +This would allow later subdivision (e.g. 51 = assertion failed, 52 = approval missing, 53 = artifact not found) without renumbering. + +**Trade-off**: More future-proof, but harder to remember and document. CI scripts with `case $?` become range checks (`if [ $? -ge 51 ] && [ $? -le 59 ]`). The compact 1-4 scheme is easier to use today but paints us into a corner if we ever need finer granularity within a category. + +**Note**: POSIX reserves exit codes 126-128 for shell use, and 128+N for signal termination. Any scheme must stay below 126. + +### Where does HTTP 404 belong? + +Currently, 404 responses return a plain `fmt.Errorf` and fall through to exit 1. But "environment not found" and "server broken" are very different for scripts. + +Options: +- **Stay in exit 1** (current): 404 is a data/application error — the server responded correctly, the thing just doesn't exist. This is analogous to `git diff` returning 1 for "differences found." +- **Own category**: A "not found" exit code (e.g. 5, or 55 in the ranged scheme) for resources that should exist but don't. +- **Context-dependent**: 404 during `assert artifact` is a compliance signal (exit 1); 404 during `get environment` is a usage/config issue (exit 4). + +### Should 429 (rate limiting) be separate from 5xx? + +Currently, `retryablehttp` retries both 429 and 5xx, and both ultimately exit 2 (server). But they have different causes: +- 5xx = server broken → escalate to ops +- 429 = client sending too fast → back off and retry + +A separate exit code for rate limiting would let CI pipelines add longer backoff instead of escalating. + +### Timeouts vs connection refused + +Both are currently exit 2 (server). But: +- Connection refused = server is down → check infrastructure +- Timeout = server is slow or network is lossy → retry with longer timeout + +Separating these helps operators triage, but adds code proliferation. + +### Partial failures in multi-host mode + +`snapshot k8s` can operate across multiple environments. If 2 of 3 succeed, what exit code? +- Exit 0 (partial success) loses the failure signal +- Exit 2 (server error) overstates the problem +- A dedicated "partial failure" code could work but adds complexity + +This is the strongest argument for JSON envelopes: partial failures need per-item status. + +### Dry-run: should it return "what would have been" the exit code? + +Currently `--dry-run` always exits 0. An alternative: dry-run exits with the code that *would have* resulted, allowing CI to validate pipeline logic without side effects. This would break the current contract where `--dry-run` is guaranteed safe (exit 0). + +### Signal handling (SIGTERM, SIGINT) + +If the CLI receives SIGTERM mid-request, should it: +- Exit immediately with 128+signal (standard Unix convention)? +- Attempt graceful shutdown and exit with the original error category? +- This is currently unhandled — the process just dies. + +### Exit code stability contract + +**This is the most important question.** Should exit codes be stable across minor versions? + +If yes: CI pipelines can hardcode `case $?` and rely on semver. Any new code or renumbering is a major version bump. + +If no: exit codes are advisory and can change. But then CI pipelines can't depend on them, which undermines the whole point. + +**Recommendation**: Treat exit codes as a stable contract from the moment they ship. Document this explicitly. New codes can be added (additive change = minor version), but existing codes must not change meaning (= major version if they do). + +## Options Considered + +### Option A: Compact exit codes (current implementation from ADR 20260319) + +Keep the 0/1/2/3/4 scheme. Errors are plain text on stderr. No structured error output. + +**Pros:** +- Already implemented (55 test annotations, 14 files, docs generated) +- Universal — every shell, CI system, and language can check `$?` +- Simple to document: 5 codes, one table per command +- No parsing required for the dominant use case (CI gate: zero vs non-zero) +- `set -e` and GitHub Actions step failure work out of the box + +**Cons:** +- Loses context: exit 1 means "compliance failure" but not *which* assertion, *which* artifact, or *what* the policy said +- Multi-error commands (e.g. server error + assertion failure) can only report one exit code +- Programmatic consumers must parse stderr text to get details +- No versioning story — if we add exit code 5 later, existing `case` statements silently miss it +- Tightly packed: no room to subdivide categories without renumbering (breaking change) + +### Option A2: Ranged exit codes (spaced scheme) + +Same as Option A but with spaced ranges (51-59, 61-69, 71-79, 81-89) to allow future subdivision within each category. + +**Pros:** +- All the pros of Option A +- Future-proof: can add 52 = "approval missing", 53 = "artifact not found" without renumbering +- Each range can be checked with a simple bash range test +- Room for the edge cases above (429, timeout, partial failure) in their respective ranges + +**Cons:** +- Harder to remember: "what does exit 63 mean?" vs "what does exit 2 mean?" +- More complex `case $?` in shell scripts (range checks vs exact matches) +- YAGNI risk: we may never need the granularity, and the gaps become wasted design space +- No industry precedent — curl uses sequential codes, not ranges +- Still can't represent multi-error scenarios + +### Option B: Structured JSON envelope (replacing exit codes) + +All commands write a JSON envelope to stdout on both success and failure. Exit code is always 0 (success) or 1 (failure). The envelope carries the error details: + +```json +{ + "version": "1", + "status": "error", + "exit_code": 3, + "category": "auth", + "message": "Invalid API token or unauthorized access", + "errors": [ + { + "code": "AUTH_FAILED", + "message": "401 Unauthorized", + "detail": "The provided API token is not valid for org 'acme'" + } + ] +} +``` + +**Pros:** +- Rich, machine-readable context: category, code, message, detail, multiple errors +- Versioned schema (`"version": "1"`) — clients can adapt without breaking +- Can represent multi-error scenarios (server error + compliance failure) +- Forward-compatible: adding new fields or error codes doesn't break consumers +- Preserves structured data the server already returns (instead of flattening it) + +**Cons:** +- **Breaking change for every consumer**: `$?` stops being meaningful; all scripts must parse JSON +- **`set -e` and CI step failure stop working** if exit is always 0/1 — or the envelope duplicates what the exit code already says +- **Requires a JSON parser** in every consumer context (not always available in minimal CI environments) +- Every command's output contract changes, including success output for commands that currently print plain text +- Human readability suffers — `Error: bad token` becomes a JSON blob unless you add a `--human` flag +- Significantly more implementation work than what's already done + +### Option C: Hybrid — exit codes + optional structured JSON errors (recommended evaluation) + +Keep exit codes as the primary error signal. Add an opt-in `--output json` mode that wraps **both success and error output** in a structured envelope. When `--output json` is not set, behaviour is identical to Option A. + +``` +# Default: human-readable, exit code carries the signal +$ kosli assert artifact --fingerprint abc123 ... +Error: Artifact is not compliant +$ echo $? +1 + +# JSON mode: same exit code, but stderr is structured +$ kosli assert artifact --fingerprint abc123 ... --output json 2>&1 +{ + "version": "1", + "status": "error", + "exit_code": 1, + "category": "compliance", + "message": "Artifact is not compliant", + "errors": [ + {"code": "COMPLIANCE_VIOLATION", "message": "Artifact is not compliant"} + ] +} +$ echo $? +1 +``` + +**Pros:** +- Exit codes work today for all existing consumers (no migration) +- Programmatic consumers get structured errors when they opt in +- Versioned schema for the JSON envelope +- Multi-error representation in JSON mode +- Can be added incrementally (start with `list`/`get` commands that already support `--output json`, extend to all commands later) + +**Cons:** +- Two error output paths to maintain and test +- Risk of the two paths diverging (JSON says one thing, text says another) +- More implementation surface than Option A alone +- Commands that don't currently support `--output json` need it added + +## Analysis + +### Mapping the decision to actual consumers + +| Consumer | Needs exit codes? | Needs JSON errors? | Notes | +|----------|:-:|:-:|-------| +| GitHub Actions step failure | Yes | No | Non-zero = red step | +| Shell script with `case $?` | Yes | No | Routes to retry/alert/fail | +| `kosli-dev/setup-cli-action` | Yes | No | Wraps CLI in a GH Action | +| Terraform provider | No | No | Already exists; calls the Kosli API directly, not the CLI | +| MCP server wrapping the CLI | Yes | **Yes** | See below | +| Future SDK wrapper | No | Yes | Would want structured errors to map to exceptions | +| Human at terminal | Yes | No | Reads plain text | +| Log aggregation (Datadog, etc.) | No | Yes | Structured fields for indexing | + +Exit codes serve **all current consumers**. JSON envelopes serve **near-term future consumers**, most notably an MCP server. + +### MCP server as a consumer + +An MCP (Model Context Protocol) server wrapping the Kosli CLI would invoke commands as subprocesses and translate results into MCP tool responses. This consumer has specific needs: + +- **Needs to distinguish error categories** to return appropriate MCP error types (e.g. retryable vs permanent failure) +- **Needs structured error detail** to include in the MCP response content — an LLM calling the tool needs to understand *why* something failed, not just *that* it failed +- **Cannot reliably parse stderr text** — error message formats may change between CLI versions; an MCP server needs a stable contract +- **Multi-error context is valuable** — if an attestation partially succeeds (reported to server) but assertion fails (compliance), the MCP server should convey both facts to the calling model +- **Exit codes alone are insufficient** — exit 1 tells the MCP server "compliance failure" but not which policy, which artifact, or what the remediation is. The server would need to forward raw stderr as unstructured text, losing the ability to provide structured tool error responses. + +This makes the MCP server the most concrete near-term consumer that would benefit from Option C (hybrid). An MCP server could work with exit codes + stderr text today, but structured JSON would make it significantly more reliable and version-resilient. + +### Multi-error problem + +The strongest argument for JSON envelopes is multi-error commands. Today, `attest pullrequest --assert` can produce: +1. A compliance error (no PRs found) — exit 1 +2. A server error (artifact doesn't exist) — exit 2 + +The CLI currently prints both to stderr but can only exit with one code. With exit codes, the last/most-severe error wins. With JSON, both are captured. However, this affects only ~7 commands (attest with `--assert` + evaluate), not the full ~80. + +### Implementation cost + +| Option | New code | Test changes | Consumer migration | +|--------|----------|-------------|-------------------| +| A (compact exit codes) | Done | Done (55 annotations) | None | +| A2 (ranged exit codes) | Moderate (renumber + add codes) | All `wantExitCode` annotations change | Scripts using exact code checks break | +| B (JSON envelope only) | Major rewrite | All tests change | All consumers must update | +| C (hybrid) | Moderate (add JSON error path) | Add JSON error tests | None (opt-in) | + +### How each option handles the edge cases + +| Edge case | A (compact) | A2 (ranged) | B (JSON) | C (hybrid) | +|-----------|:-:|:-:|:-:|:-:| +| 404 not found | Falls to exit 1 | Could get own code (e.g. 54) | Distinct error code in envelope | Both | +| 429 rate limit | Exit 2 (lumped with 5xx) | Could get own code (e.g. 62) | Distinct error code | Both | +| Timeout vs refused | Both exit 2 | Could split (61 vs 62) | Distinct per-error | Both | +| Partial multi-host failure | Last error wins | Last error wins | Per-item status array | Per-item in JSON mode | +| Multi-error (assert + server) | Last error wins | Last error wins | All errors in array | All errors in JSON mode | +| Dry-run exit code | Always 0 | Always 0 | Could include "would_be" field | Could include "would_be" field | + +## Decision + +**Deferred — this ADR is open for discussion.** + +The swarm analysis and [PR #714 review feedback](https://github.com/kosli-dev/cli/pull/714#issuecomment-4097867565) suggest: + +1. **Exit codes (Option A) are sufficient for all current consumers.** No consumer today parses error output. The 0/1/2/3/4 scheme gives CI pipelines exactly the routing signal they need. + +2. **Ranged exit codes (Option A2) are future-proof but speculative.** The gaps allow subdivision, but no consumer today needs "approval missing" vs "artifact not found" as distinct exit codes. The renumbering cost is non-trivial (all tests, all docs, all CI scripts), and there's no industry precedent for ranged CLI exit codes. + +3. **JSON envelopes (Option B) as a full replacement are premature.** There are no programmatic consumers that would benefit, and the migration cost is high. The gh CLI and gcloud both attempted partial JSON errors and ended up with inconsistency — a cautionary tale. + +4. **The hybrid (Option C) is the natural evolution path** — and is the only option that cleanly handles partial failures, multi-error commands, and the 404/429/timeout distinctions without proliferating exit codes. But it should be driven by demand, not speculation. + +5. **Exit code stability must be an explicit contract.** Whichever option ships first, document that exit codes are stable across minor versions. New codes = minor version bump. Changed codes = major version bump. + +**Recommendation**: Merge the compact exit codes (Option A / ADR 20260319) as the foundation. Explicitly document the stability contract. Design the JSON error envelope schema now (so it's ready when needed), but don't implement it until a real consumer demands it. If subdivision within categories becomes necessary, prefer adding JSON output (Option C) over renumbering to ranges (Option A2), since JSON handles the edge cases that drive the subdivision need in the first place. + +## Appendix: Draft JSON error envelope schema + +For future reference, if/when Option C is implemented: + +```json +{ + "version": "1", + "status": "error | success", + "exit_code": 1, + "category": "compliance | server | auth | usage | unknown", + "message": "Human-readable summary", + "errors": [ + { + "code": "COMPLIANCE_VIOLATION | SERVER_ERROR | AUTH_FAILED | USAGE_ERROR", + "message": "Short description", + "detail": "Extended context (optional)", + "source": { + "flag": "--fingerprint", + "command": "assert artifact" + } + } + ], + "metadata": { + "command": "kosli assert artifact", + "cli_version": "2.12.0", + "request_id": "optional-server-request-id" + } +} +``` + +Design notes: +- `version` field enables schema evolution without breaking parsers +- `errors` is an array to support multi-error commands +- `exit_code` is included so JSON parsers don't need to also check `$?` +- `source.flag` helps users fix usage errors programmatically +- `metadata.request_id` preserves server-side correlation (currently discarded) +- Inspired by RFC 9457 (Problem Details) and AWS CLI's `Error.Code` pattern + +## Related Decisions + +- [20260319 - Differentiated exit codes](./20260319-differentiated-exit-codes.md) — the exit code scheme this ADR evaluates +- [20260302 - Client-side policy evaluation](./20260302-client-side-policy-evaluation.md) — the evaluate command, one of the multi-error commands that would benefit most from structured output diff --git a/internal/docgen/formatter.go b/internal/docgen/formatter.go index 24039b1ff..48e84858c 100644 --- a/internal/docgen/formatter.go +++ b/internal/docgen/formatter.go @@ -2,6 +2,12 @@ package docgen import "github.com/spf13/cobra" +// ExitCodeEntry describes one exit code a command can produce. +type ExitCodeEntry struct { + Code int + Meaning string +} + // CommandMeta holds metadata about a cobra command for doc generation. type CommandMeta struct { Name string @@ -13,6 +19,7 @@ type CommandMeta struct { UseLine string Runnable bool Example string + ExitCodes []ExitCodeEntry } // CIExample holds data for a single CI system's live example. @@ -38,6 +45,7 @@ type Formatter interface { DeprecatedWarning(name, message string) string Synopsis(meta CommandMeta) string FlagsSection(flags, inherited string) string + ExitCodesSection(codes []ExitCodeEntry) string LiveCIExamples(examples []CIExample, commandName string) string LiveCLIExample(commandName, fullCommand, url string) string ExampleUseCases(commandName, example string) string diff --git a/internal/docgen/generate.go b/internal/docgen/generate.go index 8d2cb05c4..7c6772e4e 100644 --- a/internal/docgen/generate.go +++ b/internal/docgen/generate.go @@ -78,6 +78,9 @@ func genMarkdownCustom(cmd *cobra.Command, w io.Writer, formatter Formatter, met flags, inherited := RenderFlagsTables(cmd) buf.WriteString(formatter.FlagsSection(flags, inherited)) + // Exit codes + buf.WriteString(formatter.ExitCodesSection(meta.ExitCodes)) + // Live CI examples urlSafeName := url.QueryEscape(name) var ciExamples []CIExample diff --git a/internal/docgen/generate_test.go b/internal/docgen/generate_test.go index cc0d5cb5e..fbe8488b0 100644 --- a/internal/docgen/generate_test.go +++ b/internal/docgen/generate_test.go @@ -55,6 +55,53 @@ func TestGenMarkdownTreeCreatesFiles(t *testing.T) { } } +func TestGenMarkdownTreeIncludesExitCodes(t *testing.T) { + dir := t.TempDir() + + root := &cobra.Command{Use: "root"} + child := &cobra.Command{ + Use: "child", + Short: "A child command", + Long: "A child command with a longer description.", + RunE: func(cmd *cobra.Command, args []string) error { return nil }, + } + root.AddCommand(child) + + metaFn := func(cmd *cobra.Command) CommandMeta { + return CommandMeta{ + Name: cmd.CommandPath(), + Summary: cmd.Short, + Long: cmd.Long, + UseLine: cmd.UseLine(), + Runnable: cmd.Runnable(), + ExitCodes: []ExitCodeEntry{ + {Code: 0, Meaning: "No error."}, + {Code: 1, Meaning: "Violation."}, + }, + } + } + + err := GenMarkdownTree(root, dir, HugoFormatter{}, metaFn, NullLiveDocProvider{}) + if err != nil { + t.Fatalf("GenMarkdownTree error: %v", err) + } + + content, err := os.ReadFile(filepath.Join(dir, "root_child.md")) + if err != nil { + t.Fatalf("failed to read file: %v", err) + } + s := string(content) + if !strings.Contains(s, "## Exit Codes") { + t.Error("expected Exit Codes section in generated doc") + } + if !strings.Contains(s, "| 0 | No error. |") { + t.Error("expected exit code 0 row in generated doc") + } + if !strings.Contains(s, "| 1 | Violation. |") { + t.Error("expected exit code 1 row in generated doc") + } +} + func TestGenMarkdownTreeSkipsHiddenCommands(t *testing.T) { dir := t.TempDir() diff --git a/internal/docgen/hugo.go b/internal/docgen/hugo.go index 4fe1af932..ba0a00b20 100644 --- a/internal/docgen/hugo.go +++ b/internal/docgen/hugo.go @@ -70,6 +70,21 @@ func (HugoFormatter) FlagsSection(flags, inherited string) string { return b.String() } +func (HugoFormatter) ExitCodesSection(codes []ExitCodeEntry) string { + if len(codes) == 0 { + return "" + } + var b strings.Builder + b.WriteString("## Exit Codes\n") + b.WriteString("| Code | Meaning |\n") + b.WriteString("| :---: | :--- |\n") + for _, e := range codes { + fmt.Fprintf(&b, "| %d | %s |\n", e.Code, e.Meaning) + } + b.WriteString("\n\n") + return b.String() +} + func (HugoFormatter) LiveCIExamples(examples []CIExample, commandName string) string { if len(examples) == 0 { return "" diff --git a/internal/docgen/hugo_test.go b/internal/docgen/hugo_test.go index ef8184a23..3b9ceaac7 100644 --- a/internal/docgen/hugo_test.go +++ b/internal/docgen/hugo_test.go @@ -5,6 +5,33 @@ import ( "testing" ) +func TestHugoExitCodesSection(t *testing.T) { + f := HugoFormatter{} + codes := []ExitCodeEntry{ + {Code: 0, Meaning: "No error."}, + {Code: 1, Meaning: "Assertion/compliance violation."}, + {Code: 2, Meaning: "Server unreachable."}, + } + got := f.ExitCodesSection(codes) + if !strings.Contains(got, "## Exit Codes") { + t.Errorf("expected '## Exit Codes' heading, got:\n%s", got) + } + if !strings.Contains(got, "| 0 | No error. |") { + t.Errorf("expected exit code 0 row, got:\n%s", got) + } + if !strings.Contains(got, "| 1 | Assertion/compliance violation. |") { + t.Errorf("expected exit code 1 row, got:\n%s", got) + } +} + +func TestHugoExitCodesSectionEmpty(t *testing.T) { + f := HugoFormatter{} + got := f.ExitCodesSection(nil) + if got != "" { + t.Errorf("expected empty string for nil codes, got:\n%s", got) + } +} + func TestHugoFrontMatter(t *testing.T) { f := HugoFormatter{} meta := CommandMeta{Name: "kosli attest snyk", Summary: "Report a snyk attestation"} diff --git a/internal/docgen/mintlify.go b/internal/docgen/mintlify.go index 2c87e9593..df662af1f 100644 --- a/internal/docgen/mintlify.go +++ b/internal/docgen/mintlify.go @@ -78,6 +78,21 @@ func (MintlifyFormatter) FlagsSection(flags, inherited string) string { return b.String() } +func (MintlifyFormatter) ExitCodesSection(codes []ExitCodeEntry) string { + if len(codes) == 0 { + return "" + } + var b strings.Builder + b.WriteString("## Exit Codes\n") + b.WriteString("| Code | Meaning |\n") + b.WriteString("| :---: | :--- |\n") + for _, e := range codes { + fmt.Fprintf(&b, "| %d | %s |\n", e.Code, e.Meaning) + } + b.WriteString("\n\n") + return b.String() +} + func (MintlifyFormatter) LiveCIExamples(examples []CIExample, commandName string) string { if len(examples) == 0 { return "" diff --git a/internal/docgen/mintlify_test.go b/internal/docgen/mintlify_test.go index d2b31a43e..801fe945a 100644 --- a/internal/docgen/mintlify_test.go +++ b/internal/docgen/mintlify_test.go @@ -5,6 +5,32 @@ import ( "testing" ) +func TestMintlifyExitCodesSection(t *testing.T) { + f := MintlifyFormatter{} + codes := []ExitCodeEntry{ + {Code: 0, Meaning: "No error."}, + {Code: 2, Meaning: "Server unreachable."}, + } + got := f.ExitCodesSection(codes) + if !strings.Contains(got, "## Exit Codes") { + t.Errorf("expected '## Exit Codes' heading, got:\n%s", got) + } + if !strings.Contains(got, "| 0 | No error. |") { + t.Errorf("expected exit code 0 row, got:\n%s", got) + } + if !strings.Contains(got, "| 2 | Server unreachable. |") { + t.Errorf("expected exit code 2 row, got:\n%s", got) + } +} + +func TestMintlifyExitCodesSectionEmpty(t *testing.T) { + f := MintlifyFormatter{} + got := f.ExitCodesSection(nil) + if got != "" { + t.Errorf("expected empty string for nil codes, got:\n%s", got) + } +} + func TestMintlifyFrontMatter(t *testing.T) { f := MintlifyFormatter{} meta := CommandMeta{Name: "kosli attest snyk", Summary: "Report a snyk attestation"} diff --git a/internal/errors/errors.go b/internal/errors/errors.go new file mode 100644 index 000000000..88ff439d5 --- /dev/null +++ b/internal/errors/errors.go @@ -0,0 +1,86 @@ +package errors + +import ( + "errors" + "strings" +) + +// Exit codes +const ( + ExitOK = 0 + ExitCompliance = 1 + ExitServer = 2 + ExitConfig = 3 + ExitUsage = 4 +) + +// ErrCompliance indicates a compliance or assertion violation (exit 1). +type ErrCompliance struct{ msg string } + +func (e *ErrCompliance) Error() string { return e.msg } + +func NewErrCompliance(msg string) error { return &ErrCompliance{msg: msg} } + +// ErrServer indicates the Kosli server is unreachable or returned a 5xx (exit 2). +type ErrServer struct{ msg string } + +func (e *ErrServer) Error() string { return e.msg } + +func NewErrServer(msg string) error { return &ErrServer{msg: msg} } + +// ErrConfig indicates a configuration or authentication error (exit 3). +type ErrConfig struct{ msg string } + +func (e *ErrConfig) Error() string { return e.msg } + +func NewErrConfig(msg string) error { return &ErrConfig{msg: msg} } + +// ErrUsage indicates a CLI usage error such as a missing flag or unknown command (exit 4). +type ErrUsage struct{ msg string } + +func (e *ErrUsage) Error() string { return e.msg } + +func NewErrUsage(msg string) error { return &ErrUsage{msg: msg} } + +// ExitCodeFor returns the appropriate process exit code for err. +// Returns 0 for nil, and 1 for any unrecognised error. +func ExitCodeFor(err error) int { + if err == nil { + return ExitOK + } + var ec *ErrCompliance + if errors.As(err, &ec) { + return ExitCompliance + } + var es *ErrServer + if errors.As(err, &es) { + return ExitServer + } + var cfg *ErrConfig + if errors.As(err, &cfg) { + return ExitConfig + } + var eu *ErrUsage + if errors.As(err, &eu) { + return ExitUsage + } + if isCobraUsageError(err) { + return ExitUsage + } + return ExitCompliance // default non-zero; use exit 1 for unclassified errors +} + +// isCobraUsageError returns true for plain errors produced by Cobra's built-in +// flag and argument validation that were not already wrapped as ErrUsage. +// This covers cases where innerMain's wrapping is bypassed (e.g. in tests). +func isCobraUsageError(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "unknown flag:") || + strings.Contains(msg, "unknown shorthand flag:") || + strings.Contains(msg, "required flag(s)") || + strings.Contains(msg, "accepts") && strings.Contains(msg, "arg(s)") || + strings.Contains(msg, "requires at least") && strings.Contains(msg, "arg(s)") +} diff --git a/internal/errors/errors_test.go b/internal/errors/errors_test.go new file mode 100644 index 000000000..7e1108605 --- /dev/null +++ b/internal/errors/errors_test.go @@ -0,0 +1,85 @@ +package errors_test + +import ( + "fmt" + "testing" + + kosliErrors "github.com/kosli-dev/cli/internal/errors" + "github.com/stretchr/testify/assert" +) + +func TestExitCodeFor(t *testing.T) { + t.Run("nil returns 0", func(t *testing.T) { + assert.Equal(t, 0, kosliErrors.ExitCodeFor(nil)) + }) + + t.Run("ErrCompliance returns 1", func(t *testing.T) { + err := kosliErrors.NewErrCompliance("artifact is not compliant") + assert.Equal(t, 1, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("wrapped ErrCompliance returns 1", func(t *testing.T) { + inner := kosliErrors.NewErrCompliance("artifact is not compliant") + err := fmt.Errorf("outer: %w", inner) + assert.Equal(t, 1, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("ErrServer returns 2", func(t *testing.T) { + err := kosliErrors.NewErrServer("server unreachable") + assert.Equal(t, 2, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("wrapped ErrServer returns 2", func(t *testing.T) { + inner := kosliErrors.NewErrServer("server unreachable") + err := fmt.Errorf("outer: %w", inner) + assert.Equal(t, 2, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("ErrConfig returns 3", func(t *testing.T) { + err := kosliErrors.NewErrConfig("bad api token") + assert.Equal(t, 3, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("wrapped ErrConfig returns 3", func(t *testing.T) { + inner := kosliErrors.NewErrConfig("bad api token") + err := fmt.Errorf("outer: %w", inner) + assert.Equal(t, 3, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("ErrUsage returns 4", func(t *testing.T) { + err := kosliErrors.NewErrUsage("missing required flag") + assert.Equal(t, 4, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("wrapped ErrUsage returns 4", func(t *testing.T) { + inner := kosliErrors.NewErrUsage("missing required flag") + err := fmt.Errorf("outer: %w", inner) + assert.Equal(t, 4, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("unknown error returns 1", func(t *testing.T) { + err := fmt.Errorf("some unexpected error") + assert.Equal(t, 1, kosliErrors.ExitCodeFor(err)) + }) + + // Cobra built-in errors are classified as exit 4 without explicit wrapping. + t.Run("cobra unknown flag error returns 4", func(t *testing.T) { + err := fmt.Errorf("unknown flag: --foo") + assert.Equal(t, 4, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("cobra required flag not set returns 4", func(t *testing.T) { + err := fmt.Errorf(`required flag(s) "flow", "policy" not set`) + assert.Equal(t, 4, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("cobra ExactArgs error returns 4", func(t *testing.T) { + err := fmt.Errorf("accepts 1 arg(s), received 0") + assert.Equal(t, 4, kosliErrors.ExitCodeFor(err)) + }) + + t.Run("cobra MinimumNArgs error returns 4", func(t *testing.T) { + err := fmt.Errorf("requires at least 1 arg(s), only received 0") + assert.Equal(t, 4, kosliErrors.ExitCodeFor(err)) + }) +} diff --git a/internal/requests/requests.go b/internal/requests/requests.go index c444afb9a..d19c0897a 100644 --- a/internal/requests/requests.go +++ b/internal/requests/requests.go @@ -16,6 +16,7 @@ import ( "strings" retryablehttp "github.com/hashicorp/go-retryablehttp" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/version" ) @@ -247,8 +248,8 @@ func (c *Client) Do(p *RequestParams) (*HTTPResponse, error) { } resp, err := c.HttpClient.Do(req) if err != nil { - // err from retryable client is detailed enough - return nil, fmt.Errorf("%v", err) + // err from retryable client is detailed enough; covers 5xx after retries and network failures + return nil, kosliErrors.NewErrServer(fmt.Sprintf("%v", err)) } defer func() { @@ -278,10 +279,10 @@ func (c *Client) Do(p *RequestParams) (*HTTPResponse, error) { respBodyMap := respBody.(map[string]any) message, ok := respBodyMap["message"] if ok { - errors, ok := respBodyMap["errors"] + errs, ok := respBodyMap["errors"] if ok { cleanedErrorMessage = strings.Split(message.(string), "You have requested")[0] + - ": " + fmt.Sprintf("%v", errors) + ": " + fmt.Sprintf("%v", errs) } else { cleanedErrorMessage = strings.Split(message.(string), "You have requested")[0] } @@ -289,6 +290,12 @@ func (c *Client) Do(p *RequestParams) (*HTTPResponse, error) { cleanedErrorMessage = fmt.Sprintf("%s", respBodyMap) } } + if resp.StatusCode >= 500 { + return nil, kosliErrors.NewErrServer(cleanedErrorMessage) + } + if resp.StatusCode == 401 || resp.StatusCode == 403 { + return nil, kosliErrors.NewErrConfig(cleanedErrorMessage) + } return nil, fmt.Errorf("%s", cleanedErrorMessage) } return &HTTPResponse{string(body), resp}, nil diff --git a/internal/requests/requests_test.go b/internal/requests/requests_test.go index ead8c64e8..e04dbd686 100644 --- a/internal/requests/requests_test.go +++ b/internal/requests/requests_test.go @@ -2,11 +2,13 @@ package requests import ( "bytes" + "errors" "fmt" "net/http" "strings" "testing" + kosliErrors "github.com/kosli-dev/cli/internal/errors" "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/version" "github.com/maxcnunes/httpfake" @@ -59,6 +61,10 @@ func (suite *RequestsTestSuite) SetupSuite() { Get("/fail/"). Reply(500). BodyString("server broken") + suite.fakeService.NewHandler(). + Get("/unauthorized/"). + Reply(401). + BodyString(`"Unauthorized"`) } // shutdown the fake service after the suite execution @@ -522,6 +528,60 @@ func (suite *RequestsTestSuite) TestCreateMultipartRequestBody() { } } +func (suite *RequestsTestSuite) TestDoErrorTypes() { + for _, t := range []struct { + name string + url string + wantErrServer bool + wantErrConfig bool + wantPlainErr bool + }{ + { + name: "500 returns ErrServer", + url: suite.fakeService.ResolveURL("/fail/"), + wantErrServer: true, + }, + { + name: "403 returns ErrConfig", + url: suite.fakeService.ResolveURL("/denied/"), + wantErrConfig: true, + }, + { + name: "401 returns ErrConfig", + url: suite.fakeService.ResolveURL("/unauthorized/"), + wantErrConfig: true, + }, + { + name: "404 returns plain error (not ErrServer or ErrConfig)", + url: suite.fakeService.ResolveURL("/no-go/"), + wantPlainErr: true, + }, + } { + suite.Run(t.name, func() { + client, err := NewKosliClient("", 1, false, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + _, err = client.Do(&RequestParams{ + Method: http.MethodGet, + URL: t.url, + }) + require.Error(suite.T(), err) + + var errServer *kosliErrors.ErrServer + var errConfig *kosliErrors.ErrConfig + if t.wantErrServer { + require.True(suite.T(), errors.As(err, &errServer), "expected ErrServer, got %T: %v", err, err) + } + if t.wantErrConfig { + require.True(suite.T(), errors.As(err, &errConfig), "expected ErrConfig, got %T: %v", err, err) + } + if t.wantPlainErr { + require.False(suite.T(), errors.As(err, &errServer), "expected plain error, got ErrServer") + require.False(suite.T(), errors.As(err, &errConfig), "expected plain error, got ErrConfig") + } + }) + } +} + // In order for 'go test' to run this suite, we need to create // a normal test function and pass our suite to suite.Run func TestRequestsTestSuite(t *testing.T) {