diff --git a/cmd/kosli/lifecycle_test.go b/cmd/kosli/lifecycle_test.go index 39d40176f..7f938fa9e 100644 --- a/cmd/kosli/lifecycle_test.go +++ b/cmd/kosli/lifecycle_test.go @@ -2,6 +2,7 @@ package main import ( "io" + "path/filepath" "strings" "testing" @@ -50,6 +51,30 @@ func TestLifecycleAttestDecisionIsBetaAndVisible(t *testing.T) { } } +// Hidden while the server gates the endpoint behind is-server-side-evaluation-enabled. +func TestLifecyclePublishRegoPolicyIsBetaAndHidden(t *testing.T) { + global = &GlobalOpts{} + cmd := newPublishRegoPolicyCmd(io.Discard) + if !cmd.Hidden { + t.Error("expected publish rego-policy to be Hidden") + } + if !isBeta(cmd) { + t.Error("expected publish rego-policy to be beta") + } + + dir := t.TempDir() + if _, _, _, _, err := executeCommandC("docs --dir " + dir); err != nil { + t.Fatalf("docs generation failed: %v", err) + } + pages, err := filepath.Glob(filepath.Join(dir, "*rego-policy*")) + if err != nil { + t.Fatal(err) + } + if len(pages) > 0 { + t.Errorf("expected no docs page for a hidden rego-policy command, got %v", pages) + } +} + func TestLifecycleSnapshotGKEIsBetaAndVisible(t *testing.T) { cmd := newSnapshotGKECmd(io.Discard) if cmd.Hidden { diff --git a/cmd/kosli/publish.go b/cmd/kosli/publish.go new file mode 100644 index 000000000..6abc06400 --- /dev/null +++ b/cmd/kosli/publish.go @@ -0,0 +1,22 @@ +package main + +import ( + "io" + + "github.com/spf13/cobra" +) + +const publishDesc = `All Kosli publish commands.` + +func newPublishCmd(out io.Writer) *cobra.Command { + cmd := &cobra.Command{ + Use: "publish", + Short: publishDesc, + Long: publishDesc, + } + + cmd.AddCommand( + newPublishRegoPolicyCmd(out), + ) + return cmd +} diff --git a/cmd/kosli/publishRegoPolicy.go b/cmd/kosli/publishRegoPolicy.go new file mode 100644 index 000000000..588ae5ab1 --- /dev/null +++ b/cmd/kosli/publishRegoPolicy.go @@ -0,0 +1,256 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "slices" + + "github.com/kosli-dev/cli/internal/output" + "github.com/kosli-dev/cli/internal/regopolicies" + "github.com/spf13/cobra" +) + +const publishRegoPolicyShortDesc = `Publish a new version of a rego policy in a Kosli org.` + +const publishRegoPolicyLongDesc = publishRegoPolicyShortDesc + ` +The policy is addressed by IDENTIFIER, which decisions cite and which cannot be +changed after the first publish. ^--name^ is required on the first publish only. +Only an org admin can publish. + +PATH is a Rego file or a directory on this machine. A directory is sent as one +bundle, by the same rules as ^kosli evaluate policy --policy^: files in +dot-directories are left out, and the bundle can hold at most 100 files and +1 MiB. + +Each publish stores the next version, with ^--comment^ kept on it. A change to +the files or to ^--input-schema^ makes a new version. When the files and the +input schema are the same as the newest version, and that version is pending or +valid, nothing new is stored and that version is reported. A change to only +^--name^ or ^--description^ makes no new version. + +Kosli validates each version after it is stored, and the command waits up to 30 +seconds for the result: + - valid: the version can be evaluated. The command exits 0. + - rejected: the policy is at fault, for example it does not compile. The + command prints the reason and exits 1. + - failed: Kosli could not validate the version. Publish again to retry. The + command exits 1. +If the wait expires, the command exits 1 and says how to read the version later. +With ^--output json^, the version is always printed: with the reason for a +rejected or failed one, and as pending when the wait expires. + +An evaluation runs the highest-numbered valid version, so a rejected or failed +version leaves the policy in service unchanged.` + +const publishRegoPolicyExample = ` +# publish the first version of a rego policy from a directory: +kosli publish rego-policy pr-approved ./policies/pr-approved/ \ + --input-schema trail \ + --name "Pull request approved" \ + --description "Every change reached main through an approved PR" \ + --comment "Initial version" \ + --api-token yourAPIToken \ + --org yourOrgName + +# publish a new version of an existing rego policy from one file: +kosli publish rego-policy pr-approved ./policies/pr-approved.rego \ + --input-schema trail \ + --comment "Also require two approvers" \ + --api-token yourAPIToken \ + --org yourOrgName + +# publish a policy written against a set of trails, and print the version as JSON: +kosli publish rego-policy release-ready ./policies/release-ready/ \ + --input-schema trails \ + --name "Release ready" \ + --output json \ + --api-token yourAPIToken \ + --org yourOrgName + +# show what would be published, without sending it: +kosli publish rego-policy pr-approved ./policies/pr-approved/ \ + --input-schema trail \ + --dry-run \ + --api-token yourAPIToken \ + --org yourOrgName +` + +// A variable so that tests can shorten the wait. +var regoPolicyWaitOptions = regopolicies.WaitOptions{} + +var regoPolicyInputSchemas = []string{"trail", "trails"} + +type publishRegoPolicyOptions struct { + name string + description string + inputSchema string + comment string + output string +} + +func newPublishRegoPolicyCmd(out io.Writer) *cobra.Command { + o := new(publishRegoPolicyOptions) + cmd := &cobra.Command{ + Use: "rego-policy IDENTIFIER PATH", + Short: publishRegoPolicyShortDesc, + Long: publishRegoPolicyLongDesc, + Example: publishRegoPolicyExample, + Args: cobra.ExactArgs(2), + // Hidden while the server gates the endpoint behind is-server-side-evaluation-enabled. + Hidden: true, + Annotations: map[string]string{betaCLIAnnotation: ""}, + PreRunE: func(cmd *cobra.Command, args []string) error { + err := RequireGlobalFlags(global, []string{"Org", "ApiToken"}) + if err != nil { + return ErrorBeforePrintingUsage(cmd, err.Error()) + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + return o.run(out, args[0], args[1]) + }, + } + + cmd.Flags().StringVar(&o.inputSchema, "input-schema", "", regoPolicyInputSchemaFlag) + cmd.Flags().StringVar(&o.name, "name", "", regoPolicyNameFlag) + cmd.Flags().StringVar(&o.description, "description", "", regoPolicyDescriptionFlag) + cmd.Flags().StringVar(&o.comment, "comment", "", regoPolicyCommentFlag) + cmd.Flags().StringVarP(&o.output, "output", "o", "table", outputFlag) + addDryRunFlag(cmd) + + err := RequireFlags(cmd, []string{"input-schema"}) + if err != nil { + logger.Error("failed to configure required flags: %v", err) + } + + return cmd +} + +func (o *publishRegoPolicyOptions) run(out io.Writer, identifier, path string) error { + // Checked before publishing, so a bad format cannot fail a command whose version is stored. + printVersion, known := regoPolicyPrinters[o.output] + if !known { + return fmt.Errorf("unsupported output format: %s. Valid formats are: [table, json]", o.output) + } + if !slices.Contains(regoPolicyInputSchemas, o.inputSchema) { + return fmt.Errorf("--input-schema must be one of %v, got %q", regoPolicyInputSchemas, o.inputSchema) + } + // Decisions cite a published version, so its source must be on disk, not behind a URL. + if isRemotePolicyRef(path) { + return fmt.Errorf("PATH takes a file or a directory on this machine, not a URL") + } + + files, err := policyBundle([]string{path}) + if err != nil { + return err + } + + client := regopolicies.NewClient(kosliClient, global.Host, global.ApiToken, global.DryRun) + published, err := client.Publish(global.Org, regopolicies.PublishRequest{ + Identifier: identifier, + Name: o.name, + Description: o.description, + InputSchema: o.inputSchema, + Comment: o.comment, + Files: files, + }) + if err != nil { + return err + } + if published == nil { + // Nil on a dry run. + return nil + } + if published.Version < 1 { + // Versions count from 1, so any read would be of a version that does not exist. + return errors.New("the Kosli server accepted the publish but named no version, so it cannot be read back") + } + + // Publishing the content of the newest version answers with that version, which can be terminal. + version := published + if !version.IsTerminal() { + version, err = client.WaitForTerminal(context.Background(), global.Org, identifier, + published.Version, regoPolicyWaitOptions) + if err != nil { + if printErr := o.printIfJSON(out, published); printErr != nil { + return printErr + } + var stillPending *regopolicies.StillPendingError + if errors.As(err, &stillPending) { + return err + } + return fmt.Errorf("rego policy '%s' version %d was published, but could not be read back: %w", + identifier, published.Version, err) + } + } + if version.Status != regopolicies.StatusValid { + if err := o.printIfJSON(out, version); err != nil { + return err + } + return notValidError(identifier, version) + } + + // A new version is stored pending and with the comment it was sent, so a + // terminal answer, or one with another comment, is an existing version. + commentDropped := o.comment != "" && published.Comment != o.comment + note := "" + if published.IsTerminal() || commentDropped { + note = "nothing new was stored, as the files and input schema are the same as this version" + if commentDropped { + note += ", so --comment was not kept" + } + } + return printVersion(out, version, note) +} + +// printIfJSON gives a script the version even when the command fails, as JSON +// carries what the error text does not, such as the version number. +func (o *publishRegoPolicyOptions) printIfJSON(out io.Writer, version *regopolicies.Version) error { + if o.output != "json" { + return nil + } + return regoPolicyPrinters["json"](out, version, "") +} + +// The JSON printer ignores note, so a script reads only the fields of Version. +var regoPolicyPrinters = map[string]func(out io.Writer, version *regopolicies.Version, note string) error{ + "table": func(out io.Writer, version *regopolicies.Version, note string) error { + line := fmt.Sprintf("rego policy '%s' version %d is valid (%s)", + version.Identifier, version.Version, version.Digest) + if note != "" { + line += "; " + note + } + _, err := fmt.Fprintln(out, line) + return err + }, + "json": func(out io.Writer, version *regopolicies.Version, _ string) error { + raw, err := json.Marshal(version) + if err != nil { + return err + } + return output.PrintJson(string(raw), out, 0) + }, +} + +// notValidError words a rejected version as the policy's fault and a failed one as Kosli's. +func notValidError(identifier string, version *regopolicies.Version) error { + subject := fmt.Sprintf("rego policy '%s' version %d", identifier, version.Version) + switch version.Status { + case regopolicies.StatusRejected: + if version.Error == nil { + return fmt.Errorf("%s was rejected, and Kosli gave no reason", subject) + } + return fmt.Errorf("%s was rejected (%s): %s", subject, version.Error.Kind, version.Error.Message) + case regopolicies.StatusFailed: + reason := ", with no reason given" + if version.Error != nil { + reason = fmt.Sprintf(" (%s): %s", version.Error.Kind, version.Error.Message) + } + return fmt.Errorf("validation of %s failed on the Kosli side%s; "+ + "this is not a fault in the policy, publish again to retry", subject, reason) + } + return fmt.Errorf("%s is %s", subject, version.Status) +} diff --git a/cmd/kosli/publishRegoPolicy_test.go b/cmd/kosli/publishRegoPolicy_test.go new file mode 100644 index 000000000..1c444b452 --- /dev/null +++ b/cmd/kosli/publishRegoPolicy_test.go @@ -0,0 +1,493 @@ +package main + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/kosli-dev/cli/internal/regopolicies" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +const ( + regoPolicyPending = `{"identifier":"pr-approved","version":1,"status":"pending",` + + `"digest":"sha256:abc123","input_schema":"trail","comment":"",` + + `"created_at":1757000000.5,"created_by":"Jane Doe"}` + regoPolicyValid = `{"identifier":"pr-approved","version":1,"status":"valid",` + + `"digest":"sha256:abc123","input_schema":"trail","comment":"",` + + `"created_at":1757000000.5,"created_by":"Jane Doe","validated_at":1757000002.25,` + + `"opa_version":"1.21.1","entrypoint":"policy"}` + regoPolicyRejected = `{"identifier":"pr-approved","version":1,"status":"rejected",` + + `"digest":"sha256:abc123","input_schema":"trail","comment":"",` + + `"created_at":1757000000.5,"created_by":"Jane Doe","validated_at":1757000002.25,` + + `"opa_version":"1.21.1","error":{"kind":"compile",` + + `"message":"policy.rego:4: rego_parse_error: unexpected eof token"}}` + regoPolicyFailed = `{"identifier":"pr-approved","version":1,"status":"failed",` + + `"digest":"sha256:abc123","input_schema":"trail","comment":"",` + + `"created_at":1757000000.5,"created_by":"Jane Doe","validated_at":1757000040.0,` + + `"error":{"kind":"lambda_timeout","message":"validation timed out after 3 attempts"}}` +) + +// fakeRegoPolicies answers version reads with the next of reads, repeating the last. +type fakeRegoPolicies struct { + // Empty means the publish answers with a pending version. + publishAnswer string + published []map[string]any + publishURL string + reads []string + // Zero means a version read answers 200. + readStatus int + readCount int + readURL string + unexpected int +} + +func newFakeRegoPolicies(t *testing.T, reads ...string) (*httptest.Server, *fakeRegoPolicies) { + t.Helper() + fake := &fakeRegoPolicies{reads: reads} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPut && strings.HasPrefix(r.URL.Path, "/api/v2/rego-policies/"): + raw, err := io.ReadAll(r.Body) + require.NoError(t, err) + var body map[string]any + require.NoError(t, json.Unmarshal(raw, &body)) + fake.published = append(fake.published, body) + fake.publishURL = r.URL.Path + w.WriteHeader(http.StatusAccepted) + answer := fake.publishAnswer + if answer == "" { + answer = regoPolicyPending + } + _, _ = fmt.Fprint(w, answer) + + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/versions/"): + fake.readURL = r.URL.Path + status := fake.readStatus + if status == 0 { + status = http.StatusOK + } + w.WriteHeader(status) + _, _ = fmt.Fprint(w, fake.reads[min(fake.readCount, len(fake.reads)-1)]) + fake.readCount++ + + default: + fake.unexpected++ + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + return server, fake +} + +type PublishRegoPolicyCommandTestSuite struct { + suite.Suite + originalWait regopolicies.WaitOptions +} + +func (suite *PublishRegoPolicyCommandTestSuite) SetupTest() { + suite.originalWait = regoPolicyWaitOptions + regoPolicyWaitOptions = regopolicies.WaitOptions{ + Timeout: 2 * time.Second, + Initial: time.Millisecond, + Max: 2 * time.Millisecond, + } +} + +func (suite *PublishRegoPolicyCommandTestSuite) TearDownTest() { + regoPolicyWaitOptions = suite.originalWait +} + +func (suite *PublishRegoPolicyCommandTestSuite) publishCmd(host, args string) string { + return fmt.Sprintf("publish rego-policy %s --host %s --org test-org --api-token test-token --max-api-retries 0", + args, host) +} + +// The publish verb has no visible subcommand, so it is hidden as well. +func (suite *PublishRegoPolicyCommandTestSuite) TestItIsHiddenFromHelp() { + _, combined, _, _, err := executeCommandC("--help") + + require.NoError(suite.T(), err) + require.NotContains(suite.T(), combined, "publish") + require.Contains(suite.T(), combined, "create", "the rest of the help still renders") +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestItIsNoLongerACreateCommand() { + root, err := newRootCmd(io.Discard, io.Discard, nil) + require.NoError(suite.T(), err) + create, _, err := root.Find([]string{"create"}) + require.NoError(suite.T(), err) + + for _, sub := range create.Commands() { + require.NotEqual(suite.T(), "rego-policy", sub.Name()) + } +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestItPublishesTheFileAndReportsTheValidVersion() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, combined, _, _, err := executeCommandC(suite.publishCmd(server.URL, + `pr-approved testdata/policies/allow-all.rego --input-schema trail `+ + `--name "Pull request approved" --description "Approved PRs only" --comment "Initial version"`)) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), 0, fake.unexpected) + require.Equal(suite.T(), "/api/v2/rego-policies/test-org/pr-approved", fake.publishURL) + require.Len(suite.T(), fake.published, 1) + + published := fake.published[0] + require.Equal(suite.T(), "trail", published["input_schema"]) + require.Equal(suite.T(), "Pull request approved", published["name"]) + require.Equal(suite.T(), "Approved PRs only", published["description"]) + require.Equal(suite.T(), "Initial version", published["comment"]) + files := published["files"].(map[string]any) + require.Len(suite.T(), files, 1) + require.Contains(suite.T(), files["allow-all.rego"], "package policy") + + require.Equal(suite.T(), "/api/v2/rego-policies/test-org/pr-approved/versions/1", fake.readURL) + require.Contains(suite.T(), combined, "rego policy 'pr-approved' version 1 is valid (sha256:abc123)") +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestItLeavesOutTheNameAndDescriptionWhenNotGiven() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, _, _, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail")) + + require.NoError(suite.T(), err) + require.NotContains(suite.T(), fake.published[0], "name") + require.NotContains(suite.T(), fake.published[0], "description") +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestItWaitsThroughPendingReads() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyPending, regoPolicyPending, regoPolicyValid) + + _, combined, _, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail")) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), 3, fake.readCount) + require.Equal(suite.T(), "rego policy 'pr-approved' version 1 is valid (sha256:abc123)\n", combined) +} + +// The same directory must give the same digest here as in `evaluate policy`. +func (suite *PublishRegoPolicyCommandTestSuite) TestADirectoryTravelsKeyedByRelativePath() { + dir := suite.T().TempDir() + require.NoError(suite.T(), os.MkdirAll(filepath.Join(dir, "lib"), 0o755)) + require.NoError(suite.T(), os.WriteFile(filepath.Join(dir, "policy.rego"), []byte("package policy\n"), 0o644)) + require.NoError(suite.T(), os.WriteFile(filepath.Join(dir, "lib", "helpers.rego"), []byte("package lib\n"), 0o644)) + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, _, _, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved "+dir+" --input-schema trails")) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), map[string]any{ + "policy.rego": "package policy\n", + "lib/helpers.rego": "package lib\n", + }, fake.published[0]["files"]) + require.Equal(suite.T(), "trails", fake.published[0]["input_schema"]) +} + +func (suite *PublishRegoPolicyCommandTestSuite) publishExpectingError(reads ...string) (string, error) { + server, _ := newFakeRegoPolicies(suite.T(), reads...) + _, combined, _, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail")) + require.Error(suite.T(), err) + require.NotContains(suite.T(), combined, "is valid") + return combined, err +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestARejectedVersionNamesTheErrorInThePolicy() { + _, err := suite.publishExpectingError(regoPolicyRejected) + + require.Equal(suite.T(), + "rego policy 'pr-approved' version 1 was rejected (compile): "+ + "policy.rego:4: rego_parse_error: unexpected eof token", + err.Error()) +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestARejectedVersionWithNoReasonSaysSo() { + _, err := suite.publishExpectingError( + strings.Replace(regoPolicyValid, `"status":"valid"`, `"status":"rejected"`, 1)) + + require.Equal(suite.T(), + "rego policy 'pr-approved' version 1 was rejected, and Kosli gave no reason", + err.Error()) +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestAFailedVersionIsNotBlamedOnThePolicy() { + _, err := suite.publishExpectingError(regoPolicyFailed) + + require.Equal(suite.T(), + "validation of rego policy 'pr-approved' version 1 failed on the Kosli side (lambda_timeout): "+ + "validation timed out after 3 attempts; "+ + "this is not a fault in the policy, publish again to retry", + err.Error()) + require.NotContains(suite.T(), err.Error(), "rejected") +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestAFailedVersionWithNoReasonIsStillNotBlamedOnThePolicy() { + _, err := suite.publishExpectingError( + strings.Replace(regoPolicyValid, `"status":"valid"`, `"status":"failed"`, 1)) + + require.Equal(suite.T(), + "validation of rego policy 'pr-approved' version 1 failed on the Kosli side, with no reason given; "+ + "this is not a fault in the policy, publish again to retry", + err.Error()) +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestAnAnswerWithNoVersionIsNotReadBack() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + fake.publishAnswer = `{"identifier":"pr-approved","status":"pending"}` + + _, _, _, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail")) + + require.EqualError(suite.T(), err, + "the Kosli server accepted the publish but named no version, so it cannot be read back") + require.Equal(suite.T(), 0, fake.readCount) +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestAFailedReadNamesTheVersionThatWasPublished() { + for _, test := range []struct { + name, output string + }{ + {"table", "table"}, + {"json", "json"}, + } { + suite.Run(test.name, func() { + server, fake := newFakeRegoPolicies(suite.T(), `{"message":"Rego policy 'pr-approved' does not exist"}`) + fake.readStatus = http.StatusNotFound + + _, _, stdout, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail --output "+test.output)) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), + "rego policy 'pr-approved' version 1 was published, but could not be read back: ") + require.Contains(suite.T(), err.Error(), "does not exist") + if test.output == "json" { + var printed map[string]any + require.NoError(suite.T(), json.Unmarshal([]byte(stdout), &printed), "stdout is JSON: %s", stdout) + require.Equal(suite.T(), "pending", printed["status"]) + } else { + require.Empty(suite.T(), stdout) + } + }) + } +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestJsonOutputIsThePendingVersionWhenTheWaitExpires() { + regoPolicyWaitOptions.Timeout = 20 * time.Millisecond + server, _ := newFakeRegoPolicies(suite.T(), regoPolicyPending) + + _, _, stdout, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail --output json")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "still pending") + var printed map[string]any + require.NoError(suite.T(), json.Unmarshal([]byte(stdout), &printed), "stdout is JSON: %s", stdout) + require.Equal(suite.T(), "pending", printed["status"]) + require.Equal(suite.T(), float64(1), printed["version"]) +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestAWaitThatExpiresSaysHowToReadTheVersionLater() { + regoPolicyWaitOptions.Timeout = 20 * time.Millisecond + + _, err := suite.publishExpectingError(regoPolicyPending) + + require.Contains(suite.T(), err.Error(), "version 1 of rego policy pr-approved is still pending after") + require.Regexp(suite.T(), + `read it later with GET http://127\.0\.0\.1:\d+/api/v2/rego-policies/test-org/pr-approved/versions/1$`, + err.Error()) +} + +func (suite *PublishRegoPolicyCommandTestSuite) refusedBeforeSending(args string) error { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, _, _, _, err := executeCommandC(suite.publishCmd(server.URL, args)) + + require.Error(suite.T(), err) + require.Empty(suite.T(), fake.published, "nothing may be published") + require.Equal(suite.T(), 0, fake.unexpected) + return err +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestAnUnknownInputSchemaIsRefused() { + err := suite.refusedBeforeSending( + "pr-approved testdata/policies/allow-all.rego --input-schema many") + + require.Equal(suite.T(), `--input-schema must be one of [trail trails], got "many"`, err.Error()) +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestAPolicyAtAURLIsRefused() { + err := suite.refusedBeforeSending( + "pr-approved https://example.com/policy.rego --input-schema trail") + + require.Equal(suite.T(), "PATH takes a file or a directory on this machine, not a URL", err.Error()) +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestABundleOverTheFileCapIsRefused() { + dir := suite.T().TempDir() + for i := range maxPolicyBundleFiles + 1 { + name := filepath.Join(dir, fmt.Sprintf("p%03d.rego", i)) + require.NoError(suite.T(), os.WriteFile(name, []byte("package p\n"), 0o644)) + } + + err := suite.refusedBeforeSending("pr-approved " + dir + " --input-schema trail") + + require.Equal(suite.T(), "policy bundle holds 101 files, over the limit of 100", err.Error()) +} + +// An unset variable in `--description "$DESC"` must not clear a description. +func (suite *PublishRegoPolicyCommandTestSuite) TestAnEmptyDescriptionIsRefused() { + err := suite.refusedBeforeSending( + `pr-approved testdata/policies/allow-all.rego --input-schema trail --description ""`) + + require.Contains(suite.T(), err.Error(), "flag '--description' was given an empty value") +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestJsonOutputIsPrintedForAVersionThatIsNotValid() { + for _, test := range []struct { + name, body, status, kind string + }{ + {"rejected", regoPolicyRejected, "rejected", "compile"}, + {"failed", regoPolicyFailed, "failed", "lambda_timeout"}, + } { + suite.Run(test.name, func() { + server, _ := newFakeRegoPolicies(suite.T(), test.body) + + _, _, stdout, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail --output json")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), test.status) + var printed map[string]any + require.NoError(suite.T(), json.Unmarshal([]byte(stdout), &printed), "stdout is JSON: %s", stdout) + require.Equal(suite.T(), test.status, printed["status"]) + require.Equal(suite.T(), test.kind, printed["error"].(map[string]any)["kind"]) + }) + } +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestTableOutputPrintsNothingForAVersionThatIsNotValid() { + server, _ := newFakeRegoPolicies(suite.T(), regoPolicyRejected) + + _, _, stdout, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail")) + + require.Error(suite.T(), err) + require.Empty(suite.T(), stdout) +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestJsonOutputIsTheValidVersion() { + server, _ := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, _, stdout, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail --output json")) + require.NoError(suite.T(), err) + + var printed map[string]any + require.NoError(suite.T(), json.Unmarshal([]byte(stdout), &printed), "stdout is JSON: %s", stdout) + require.Equal(suite.T(), map[string]any{ + "identifier": "pr-approved", + "version": float64(1), + "status": "valid", + "digest": "sha256:abc123", + "input_schema": "trail", + "comment": "", + "created_at": 1757000000.5, + "created_by": "Jane Doe", + "validated_at": 1757000002.25, + "opa_version": "1.21.1", + "entrypoint": "policy", + }, printed, "fields the server left out stay out") +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestAnUnknownOutputFormatIsRefused() { + err := suite.refusedBeforeSending( + "pr-approved testdata/policies/allow-all.rego --input-schema trail --output yaml") + + require.Equal(suite.T(), "unsupported output format: yaml. Valid formats are: [table, json]", err.Error()) +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestADryRunSendsNothingAndWaitsForNothing() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, combined, _, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail --dry-run")) + + require.NoError(suite.T(), err) + require.Empty(suite.T(), fake.published) + require.Equal(suite.T(), 0, fake.readCount) + require.Contains(suite.T(), combined, "THIS IS A DRY-RUN") + require.Contains(suite.T(), combined, "/api/v2/rego-policies/test-org/pr-approved") + require.NotContains(suite.T(), combined, "is valid") +} + +// A new version keeps the comment it was published with, so a different +// comment on the answer means the server stored nothing. +func (suite *PublishRegoPolicyCommandTestSuite) TestItSaysWhenAPublishStoredNothingNew() { + const unchanged = "rego policy 'pr-approved' version 1 is valid (sha256:abc123); nothing new was stored, " + + "as the files and input schema are the same as this version" + withComment := func(body, comment string) string { + return strings.Replace(body, `"comment":""`, `"comment":"`+comment+`"`, 1) + } + for _, test := range []struct { + name, answer, flags, want string + reads int + }{ + {"valid, no comment given", regoPolicyValid, "", + unchanged + "\n", 0}, + {"valid, same comment", withComment(regoPolicyValid, "First"), ` --comment "First"`, + unchanged + "\n", 0}, + {"valid, other comment", regoPolicyValid, ` --comment "Second try"`, + unchanged + ", so --comment was not kept\n", 0}, + {"pending, other comment", withComment(regoPolicyPending, "First"), ` --comment "Second try"`, + unchanged + ", so --comment was not kept\n", 1}, + {"new version, same comment", withComment(regoPolicyPending, "Second try"), ` --comment "Second try"`, + "rego policy 'pr-approved' version 1 is valid (sha256:abc123)\n", 1}, + } { + suite.Run(test.name, func() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + fake.publishAnswer = test.answer + + _, combined, _, _, err := executeCommandC(suite.publishCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail"+test.flags)) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), test.reads, fake.readCount, "a terminal answer is not polled") + require.Equal(suite.T(), test.want, combined) + }) + } +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestItNeedsAnIdentifierAndAPath() { + _, _, _, _, err := executeCommandC(suite.publishCmd("http://localhost:1", + "pr-approved --input-schema trail")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "accepts 2 arg(s), received 1") +} + +func (suite *PublishRegoPolicyCommandTestSuite) TestItNeedsAnInputSchema() { + _, _, _, _, err := executeCommandC(suite.publishCmd("http://localhost:1", + "pr-approved testdata/policies/allow-all.rego")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), `required flag(s) "input-schema" not set`) +} + +func TestPublishRegoPolicyCommandTestSuite(t *testing.T) { + suite.Run(t, new(PublishRegoPolicyCommandTestSuite)) +} diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index f20ad3d77..700d2fe80 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -158,6 +158,10 @@ Paths the list already matches stay excluded whatever is later added there, so k policyDecisionTrailFlag = "[conditional] The Kosli trail the decision is recorded in. Only required with --control." policyDecisionNameFlag = "[defaulted] The attestation name the decision is recorded under. Defaults to -decision." policyDecisionFingerprintFlag = "[optional] The SHA256 fingerprint of the artifact the decision is about. Without it the decision is about the trail." + regoPolicyInputSchemaFlag = "The shape of the input the policy is written against: trail (one trail) or trails (a set of trails)." + regoPolicyNameFlag = "[conditional] The display name of the rego policy. Only required when the identifier is new; without it the current name is kept." + regoPolicyDescriptionFlag = "[optional] The description of the rego policy. Without it the current description is kept." + regoPolicyCommentFlag = "[optional] Why this version is published. It is kept with the version." outputFlagWithMarkdown = "[defaulted] The format of the output. Valid formats are: [table, json, markdown]." searchByNameFlag = "[optional] Only list flows whose name contains this substring. The Kosli API supports alphanumeric characters and '-'." ignoreCaseFlag = "[optional] Perform case-insensitive matching for --name. By default matching is case sensitive." @@ -588,6 +592,7 @@ func newRootCmd(out, errOut io.Writer, args []string) (*cobra.Command, error) { // New syntax commands newGetCmd(out), newCreateCmd(out), + newPublishCmd(out), newBeginCmd(out), newAttestCmd(out), newReportCmd(out), diff --git a/cmd/kosli/testdata/empty-flag-audit-coverage.json b/cmd/kosli/testdata/empty-flag-audit-coverage.json index 46192fdd7..839257c27 100644 --- a/cmd/kosli/testdata/empty-flag-audit-coverage.json +++ b/cmd/kosli/testdata/empty-flag-audit-coverage.json @@ -608,7 +608,7 @@ "name": "string", "output": "string", "params": "string", - "policy": "string", + "policy": "stringArray", "trail": "string" }, "evaluate trail": { @@ -781,6 +781,14 @@ "start": "string", "start-ts": "float64" }, + "publish rego-policy": { + "comment": "string", + "description": "string", + "dry-run": "bool", + "input-schema": "string", + "name": "string", + "output": "string" + }, "rename environment": { "dry-run": "bool" }, diff --git a/hack/empty-flag-audit/spec.json b/hack/empty-flag-audit/spec.json index 5a9379663..af6358cb4 100644 --- a/hack/empty-flag-audit/spec.json +++ b/hack/empty-flag-audit/spec.json @@ -1521,6 +1521,36 @@ ] ] }, + "publish rego-policy": { + "args": [ + "{policy}", + "cmd/kosli/testdata/policies/allow-all.rego" + ], + "flags": { + "input-schema": "trail" + }, + "baseline_ok": false, + "error": "the version is validated away from this machine, and the server the audit runs against does not offer it", + "needs": "server-side evaluation", + "flags_to_test": [ + "comment", + "description", + "dry-run", + "input-schema", + "name", + "output" + ], + "flag_values": { + "comment": "probe-comment", + "description": "probe-description", + "dry-run": "true", + "input-schema": "trail", + "name": "probe-name", + "output": "json" + }, + "setup": [], + "verify": [] + }, "create service-account": { "args": [ "{account}" diff --git a/internal/evaluations/wait.go b/internal/evaluations/wait.go index 860c3cd34..3ce542e84 100644 --- a/internal/evaluations/wait.go +++ b/internal/evaluations/wait.go @@ -8,6 +8,7 @@ import ( "net/url" "time" + "github.com/kosli-dev/cli/internal/poll" "github.com/kosli-dev/cli/internal/requests" ) @@ -21,52 +22,27 @@ var DefaultWaitOptions = WaitOptions{ Max: 5 * time.Second, } -// WaitOptions bounds how long an evaluation is asked about and how often. A -// zero field takes its default. -// -// Timeout governs the polling, not any single read. A read is bounded by the -// HTTP client's own retry policy instead, which has no overall deadline, so a -// server that answers very slowly can overrun this budget by one read. Holding -// to it exactly would mean giving the read a deadline of its own, which the -// shared client cannot take today. -type WaitOptions struct { - Timeout time.Duration - Initial time.Duration - Max time.Duration -} - -func (o WaitOptions) withDefaults() WaitOptions { - if o.Timeout <= 0 { - o.Timeout = DefaultWaitOptions.Timeout - } - if o.Initial <= 0 { - o.Initial = DefaultWaitOptions.Initial - } - if o.Max <= 0 { - o.Max = DefaultWaitOptions.Max - } - return o -} +// WaitOptions are poll.Options in which a field that is not set takes its default. +type WaitOptions = poll.Options // StillPendingError says the wait expired with the evaluation unfinished. It // is deliberately not a verdict and carries no result: an evaluation that has // not answered must never be reported as a denial. The id is on it so the // caller can point at the evaluation that is still running. type StillPendingError struct { - Org string ID string + URL string Waited time.Duration } func (e *StillPendingError) Error() string { - return fmt.Sprintf( - "evaluation %s is still pending after %s; read it later with GET /api/v2/evaluations/%s/%s", - e.ID, e.Waited, e.Org, e.ID) + return fmt.Sprintf("evaluation %s is still pending after %s; read it later with GET %s", + e.ID, e.Waited, e.URL) } // Get reads one evaluation. A pending one is a normal answer, not an error. func (c *Client) Get(org, id string) (*Evaluation, error) { - endpoint, err := url.JoinPath(c.host, "api/v2/evaluations", org, id) + endpoint, err := evaluationURL(c.host, org, id) if err != nil { return nil, err } @@ -93,58 +69,27 @@ func (c *Client) Get(org, id string) (*Evaluation, error) { // WaitForTerminal polls until the evaluation finishes, the caller gives up, or // the budget expires. It returns an evaluation only when that evaluation is // terminal, so a caller never has to ask whether the answer it holds is final. -// -// Cancelling ctx stops the polling but cannot abort a read already in flight, -// for the same reason the budget cannot: the read carries no context. Both take -// effect at the next turn of the loop. func (c *Client) WaitForTerminal(ctx context.Context, org, id string, options WaitOptions) (*Evaluation, error) { - options = options.withDefaults() - // Measured rather than assumed: a read carries no deadline of its own, so - // the time actually spent can exceed the budget, and reporting the budget - // would understate the wait to the one person asking whether the platform - // met it. - started := time.Now() - expired := time.After(options.Timeout) - interval := options.Initial - - for { - // Checked before the read, so a caller who has already given up costs - // the server nothing. - if err := ctx.Err(); err != nil { - return nil, err - } - - evaluation, err := c.Get(org, id) - if err != nil { - // Transient trouble was already retried inside the HTTP client, so - // what reaches here will not improve by being asked again. - return nil, err - } - if evaluation.IsTerminal() { - return evaluation, nil - } + endpoint, err := evaluationURL(c.host, org, id) + if err != nil { + return nil, err + } + evaluation, err := poll.Until(ctx, options.Or(DefaultWaitOptions), + func() (*Evaluation, bool, error) { + evaluation, err := c.Get(org, id) + if err != nil { + return nil, false, err + } + return evaluation, evaluation.IsTerminal(), nil + }) - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-expired: - // Rounded: the point of the number is that the budget was spent, and - // six digits of nanoseconds are noise in that sentence. An overrun - // still shows, which is why it is measured at all. - return nil, &StillPendingError{ - Org: org, ID: id, Waited: time.Since(started).Round(time.Millisecond)} - case <-time.After(interval): - } - interval = nextInterval(interval, options.Max) + var expired *poll.ExpiredError + if errors.As(err, &expired) { + return nil, &StillPendingError{ID: id, URL: endpoint, Waited: expired.Waited} } + return evaluation, err } -// nextInterval doubles the wait between reads, never past max. Backing off -// keeps a slow evaluation from being asked about hundreds of times, while the -// first few reads stay close together so a fast one answers promptly. -func nextInterval(current, max time.Duration) time.Duration { - if doubled := current * 2; doubled < max { - return doubled - } - return max +func evaluationURL(host, org, id string) (string, error) { + return url.JoinPath(host, "api/v2/evaluations", org, id) } diff --git a/internal/evaluations/wait_test.go b/internal/evaluations/wait_test.go index 70f9c64ff..1b4c5fe9b 100644 --- a/internal/evaluations/wait_test.go +++ b/internal/evaluations/wait_test.go @@ -189,6 +189,7 @@ func TestWaitGivesUpWithTheEvaluationIdAndNoVerdict(t *testing.T) { var stillPending *StillPendingError require.True(t, errors.As(err, &stillPending), "expected a still-pending error, got %T", err) require.Equal(t, "01JABCDEF", stillPending.ID) + require.Equal(t, server.URL+"/api/v2/evaluations/my-org/01JABCDEF", stillPending.URL) // The time actually spent, not the budget: a read carries no deadline of // its own, so the two can differ and only the first answers "did the // platform meet its promise". @@ -199,6 +200,7 @@ func TestWaitGivesUpWithTheEvaluationIdAndNoVerdict(t *testing.T) { "no nanosecond noise in a sentence about a budget") require.Contains(t, err.Error(), "01JABCDEF") require.Contains(t, err.Error(), "still pending") + require.Contains(t, err.Error(), "read it later with GET "+server.URL+"/api/v2/evaluations/my-org/01JABCDEF") require.NotContains(t, err.Error(), "denied") } @@ -232,27 +234,6 @@ func TestWaitStopsOnAReadItCannotRetry(t *testing.T) { require.Equal(t, 2, seen.hits, "a failed read ends the wait rather than being polled through") } -func TestNextIntervalDoublesUpToTheCeiling(t *testing.T) { - const max = 5 * time.Second - - for _, test := range []struct { - current time.Duration - want time.Duration - }{ - {500 * time.Millisecond, time.Second}, - {time.Second, 2 * time.Second}, - {2 * time.Second, 4 * time.Second}, - {4 * time.Second, max}, - {max, max}, - } { - require.Equal(t, test.want, nextInterval(test.current, max), - "doubling %s", test.current) - } -} - -func TestWaitOptionsFallBackToTheAgreedBudget(t *testing.T) { - filled := WaitOptions{}.withDefaults() - - require.Equal(t, DefaultWaitOptions, filled) - require.Equal(t, 30*time.Second, filled.Timeout, "the enqueue-to-terminal ceiling") +func TestTheDefaultBudgetIsTheAgreedOne(t *testing.T) { + require.Equal(t, 30*time.Second, DefaultWaitOptions.Timeout, "the enqueue-to-terminal ceiling") } diff --git a/internal/poll/poll.go b/internal/poll/poll.go new file mode 100644 index 000000000..0e83b8be8 --- /dev/null +++ b/internal/poll/poll.go @@ -0,0 +1,82 @@ +// Package poll repeats a read, backing off, until it reports done. +package poll + +import ( + "context" + "fmt" + "time" +) + +// Options must have every field positive. Timeout bounds the polling, not a read +// in flight, so one slow read can overrun it. +type Options struct { + Timeout time.Duration + Initial time.Duration + Max time.Duration +} + +// Or fills each field that is not positive from defaults. +func (o Options) Or(defaults Options) Options { + if o.Timeout <= 0 { + o.Timeout = defaults.Timeout + } + if o.Initial <= 0 { + o.Initial = defaults.Initial + } + if o.Max <= 0 { + o.Max = defaults.Max + } + return o +} + +// ExpiredError says the budget ran out before the read reported done. +type ExpiredError struct { + Waited time.Duration +} + +func (e *ExpiredError) Error() string { + return fmt.Sprintf("still pending after %s", e.Waited) +} + +// Until calls read until it reports done, read fails, ctx ends, or the budget +// expires. It returns a value only when read reported it done. Cancelling ctx +// cannot abort a read in flight. +func Until[T any](ctx context.Context, options Options, read func() (T, bool, error)) (T, error) { + var zero T + // Measured, because a slow read can overrun the budget. + started := time.Now() + expired := time.After(options.Timeout) + interval := options.Initial + + for { + // Checked first, so a cancelled caller sends no read. + if err := ctx.Err(); err != nil { + return zero, err + } + + value, done, err := read() + if err != nil { + // The HTTP client has already retried transient errors. + return zero, err + } + if done { + return value, nil + } + + select { + case <-ctx.Done(): + return zero, ctx.Err() + case <-expired: + return zero, &ExpiredError{Waited: time.Since(started).Round(time.Millisecond)} + case <-time.After(interval): + } + interval = nextInterval(interval, options.Max) + } +} + +func nextInterval(current, max time.Duration) time.Duration { + if doubled := current * 2; doubled < max { + return doubled + } + return max +} diff --git a/internal/poll/poll_test.go b/internal/poll/poll_test.go new file mode 100644 index 000000000..2079ead1f --- /dev/null +++ b/internal/poll/poll_test.go @@ -0,0 +1,114 @@ +package poll + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// testOptions has a Timeout that expires only when a test shortens it. +func testOptions() Options { + return Options{ + Timeout: 2 * time.Second, + Initial: time.Millisecond, + Max: 2 * time.Millisecond, + } +} + +// sequence repeats the last state once the states run out. +func sequence(states ...string) (func() (string, bool, error), *int) { + reads := 0 + return func() (string, bool, error) { + state := states[min(reads, len(states)-1)] + reads++ + return state, state != "pending", nil + }, &reads +} + +func TestUntilReturnsTheFirstValueThatIsDone(t *testing.T) { + read, reads := sequence("pending", "pending", "valid") + + value, err := Until(context.Background(), testOptions(), read) + + require.NoError(t, err) + require.Equal(t, "valid", value) + require.Equal(t, 3, *reads) +} + +func TestUntilStopsOnAFailedRead(t *testing.T) { + failure := errors.New("read failed") + reads := 0 + read := func() (string, bool, error) { + reads++ + if reads == 2 { + return "", false, failure + } + return "pending", false, nil + } + + value, err := Until(context.Background(), testOptions(), read) + + require.ErrorIs(t, err, failure) + require.Empty(t, value) + require.Equal(t, 2, reads, "a failed read ends the wait rather than being polled through") +} + +func TestUntilStopsWhenTheCallerGivesUp(t *testing.T) { + read, reads := sequence("pending") + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + value, err := Until(ctx, testOptions(), read) + + require.ErrorIs(t, err, context.Canceled) + require.Empty(t, value) + require.Equal(t, 0, *reads, "a cancelled wait reads nothing") +} + +func TestUntilGivesUpWithTheTimeItWaitedAndNoValue(t *testing.T) { + read, _ := sequence("pending") + options := testOptions() + options.Timeout = 20 * time.Millisecond + + value, err := Until(context.Background(), options, read) + + require.Empty(t, value, "a wait that expired must never carry a value") + var expired *ExpiredError + require.True(t, errors.As(err, &expired), "expected an expired error, got %T", err) + // Rounded to the millisecond before it is reported, so allow that much. + require.GreaterOrEqual(t, expired.Waited+time.Millisecond, options.Timeout) + require.Equal(t, expired.Waited.Round(time.Millisecond), expired.Waited) +} + +func TestNextIntervalDoublesUpToTheCeiling(t *testing.T) { + const max = 5 * time.Second + + for _, test := range []struct { + current time.Duration + want time.Duration + }{ + {500 * time.Millisecond, time.Second}, + {time.Second, 2 * time.Second}, + {2 * time.Second, 4 * time.Second}, + {4 * time.Second, max}, + {max, max}, + } { + require.Equal(t, test.want, nextInterval(test.current, max), + "doubling %s", test.current) + } +} + +func TestOrFillsOnlyTheFieldsThatAreNotSet(t *testing.T) { + defaults := Options{Timeout: 30 * time.Second, Initial: 500 * time.Millisecond, Max: 5 * time.Second} + + require.Equal(t, defaults, Options{}.Or(defaults)) + require.Equal(t, + Options{Timeout: time.Second, Initial: 500 * time.Millisecond, Max: 5 * time.Second}, + Options{Timeout: time.Second}.Or(defaults)) + require.Equal(t, + Options{Timeout: 30 * time.Second, Initial: 500 * time.Millisecond, Max: 5 * time.Second}, + Options{Timeout: -time.Second}.Or(defaults), "a negative field is not set") +} diff --git a/internal/regopolicies/client.go b/internal/regopolicies/client.go new file mode 100644 index 000000000..ad9103efa --- /dev/null +++ b/internal/regopolicies/client.go @@ -0,0 +1,173 @@ +// Package regopolicies publishes versioned Rego bundles to Kosli and reads their +// validation back. +package regopolicies + +import ( + "encoding/json" + "errors" + "fmt" + "net/http" + "net/url" + "strconv" + + "github.com/kosli-dev/cli/internal/requests" +) + +// The statuses a version reports. A version leaves pending exactly once: +// rejected means the policy is at fault, failed means the server is. +const ( + StatusPending = "pending" + StatusValid = "valid" + StatusRejected = "rejected" + StatusFailed = "failed" +) + +// PublishRequest leaves an empty Name or Description out, so the server keeps its value. +type PublishRequest struct { + Identifier string + Name string + Description string + InputSchema string + Comment string + Files map[string]string +} + +// Version is one published version of a policy, as the server reports it. +type Version struct { + Identifier string `json:"identifier"` + Version int `json:"version"` + Status string `json:"status"` + Digest string `json:"digest"` + InputSchema string `json:"input_schema"` + Comment string `json:"comment"` + CreatedAt float64 `json:"created_at"` + CreatedBy string `json:"created_by"` + // Nil while the version is pending. + ValidatedAt *float64 `json:"validated_at,omitempty"` + OPAVersion string `json:"opa_version,omitempty"` + // Set only on a valid version; ParamsSchema only when the policy declares one. + Entrypoint string `json:"entrypoint,omitempty"` + ParamsSchema map[string]any `json:"params_schema,omitempty"` + // Set only on a rejected or failed version. + Error *ValidationError `json:"error,omitempty"` +} + +// IsTerminal reports whether the version has left pending, which it does once. +func (v *Version) IsTerminal() bool { + return v.Status == StatusValid || v.Status == StatusRejected || v.Status == StatusFailed +} + +// ValidationError says why a version is not valid. Kind is a plain string because +// the evaluator owns the set, and an unknown kind must still reach the user. +type ValidationError struct { + Kind string `json:"kind"` + Message string `json:"message"` +} + +// Client is a Kosli API client scoped to the rego policy endpoints. +type Client struct { + http *requests.Client + host string + token string + dryRun bool +} + +func NewClient(httpClient *requests.Client, host, token string, dryRun bool) *Client { + return &Client{http: httpClient, host: host, token: token, dryRun: dryRun} +} + +// Publish stores the next version of a policy and returns it pending. A dry run +// sends nothing and returns no version. +func (c *Client) Publish(org string, request PublishRequest) (*Version, error) { + route, err := policyRoute(org, request.Identifier) + if err != nil { + return nil, err + } + endpoint, err := url.JoinPath(c.host, route) + if err != nil { + return nil, err + } + + response, err := c.http.Do(&requests.RequestParams{ + Method: http.MethodPut, + URL: endpoint, + Token: c.token, + DryRun: c.dryRun, + // The server re-runs a publish that loses a race, so a 409 here is a + // refusal to report, not a lock to wait out. + DisableConflictRetry: true, + Payload: publishPayload{ + Name: request.Name, + Description: request.Description, + InputSchema: request.InputSchema, + Comment: request.Comment, + Files: request.Files, + }, + }) + if err != nil { + return nil, err + } + if response == nil { + return nil, nil + } + return decodeVersion(response.Body) +} + +// GetVersion reads one version of a policy. A pending one is a normal answer, +// not an error. +func (c *Client) GetVersion(org, identifier string, version int) (*Version, error) { + endpoint, err := versionURL(c.host, org, identifier, version) + if err != nil { + return nil, err + } + + // Not a dry run: a read writes nothing. + response, err := c.http.Do(&requests.RequestParams{ + Method: http.MethodGet, + URL: endpoint, + Token: c.token, + }) + if err != nil { + return nil, err + } + if response == nil { + // Do returns no response for a dry-run request. + return nil, errors.New("no rego policy version was read back") + } + return decodeVersion(response.Body) +} + +// policyRoute escapes the identifier into one path segment. It refuses "." and +// "..", which a URL path resolves however they are escaped. +func policyRoute(org, identifier string) (string, error) { + switch identifier { + case "", ".", "..": + return "", fmt.Errorf("%q is not a valid rego policy identifier", identifier) + } + return "/api/v2/rego-policies/" + url.PathEscape(org) + "/" + url.PathEscape(identifier), nil +} + +func versionURL(host, org, identifier string, version int) (string, error) { + route, err := policyRoute(org, identifier) + if err != nil { + return "", err + } + return url.JoinPath(host, route, "versions", strconv.Itoa(version)) +} + +// The server refuses unknown fields, so the wire shape is stated here exactly. +type publishPayload struct { + Name string `json:"name,omitempty"` + Description string `json:"description,omitempty"` + InputSchema string `json:"input_schema"` + Comment string `json:"comment"` + Files map[string]string `json:"files"` +} + +func decodeVersion(body string) (*Version, error) { + var version Version + if err := json.Unmarshal([]byte(body), &version); err != nil { + return nil, fmt.Errorf("failed to parse the rego policy version: %w", err) + } + return &version, nil +} diff --git a/internal/regopolicies/client_test.go b/internal/regopolicies/client_test.go new file mode 100644 index 000000000..637b149b9 --- /dev/null +++ b/internal/regopolicies/client_test.go @@ -0,0 +1,228 @@ +package regopolicies + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/kosli-dev/cli/internal/logger" + "github.com/kosli-dev/cli/internal/requests" + "github.com/stretchr/testify/require" +) + +type received struct { + hits int + method string + path string + escapedPath string + authHeader string + contentType string + body map[string]any +} + +func newFakeServer(t *testing.T, status int, responseBody string) (*httptest.Server, *received) { + t.Helper() + seen := &received{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen.hits++ + seen.method = r.Method + seen.path = r.URL.Path + seen.escapedPath = r.URL.EscapedPath() + seen.authHeader = r.Header.Get("Authorization") + seen.contentType = r.Header.Get("Content-Type") + + raw, err := io.ReadAll(r.Body) + require.NoError(t, err) + if len(raw) > 0 { + require.NoError(t, json.Unmarshal(raw, &seen.body)) + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, err = w.Write([]byte(responseBody)) + require.NoError(t, err) + })) + t.Cleanup(server.Close) + return server, seen +} + +func newTestClient(t *testing.T, host string, dryRun bool) *Client { + t.Helper() + // No retries, so a 5xx surfaces as itself instead of being retried away. + httpClient, err := requests.NewKosliClient("", 0, false, logger.NewLogger(io.Discard, io.Discard, false)) + require.NoError(t, err) + return NewClient(httpClient, host, "test-token", dryRun) +} + +func aPublishRequest() PublishRequest { + return PublishRequest{ + Identifier: "pr-approved", + InputSchema: "trail", + Comment: "Initial version", + Files: map[string]string{"policy.rego": "package policy\n\nallow := true\n"}, + } +} + +const pendingBody = `{"identifier":"pr-approved","version":1,"status":"pending",` + + `"digest":"sha256:abc123","input_schema":"trail","comment":"Initial version",` + + `"created_at":1757000000.5,"created_by":"Jane Doe"}` + +func TestPublishSendsTheRequestTheAPIExpects(t *testing.T) { + server, seen := newFakeServer(t, http.StatusAccepted, pendingBody) + + _, err := newTestClient(t, server.URL, false).Publish("my-org", aPublishRequest()) + require.NoError(t, err) + + require.Equal(t, 1, seen.hits) + require.Equal(t, http.MethodPut, seen.method) + require.Equal(t, "/api/v2/rego-policies/my-org/pr-approved", seen.path) + require.Equal(t, "Bearer test-token", seen.authHeader) + require.Contains(t, seen.contentType, "application/json") +} + +func TestPublishSendsExactlyTheAgreedPayload(t *testing.T) { + server, seen := newFakeServer(t, http.StatusAccepted, pendingBody) + + _, err := newTestClient(t, server.URL, false).Publish("my-org", aPublishRequest()) + require.NoError(t, err) + + // The server forbids unknown fields, so anything extra is a refusal. + require.Equal(t, map[string]any{ + "input_schema": "trail", + "comment": "Initial version", + "files": map[string]any{"policy.rego": "package policy\n\nallow := true\n"}, + }, seen.body) +} + +func TestPublishSendsAnEmptyCommentRatherThanLeavingItOut(t *testing.T) { + server, seen := newFakeServer(t, http.StatusAccepted, pendingBody) + request := aPublishRequest() + request.Comment = "" + + _, err := newTestClient(t, server.URL, false).Publish("my-org", request) + require.NoError(t, err) + + require.Contains(t, seen.body, "comment") + require.Equal(t, "", seen.body["comment"]) +} + +func TestPublishSendsTheNameAndDescriptionOnlyWhenGiven(t *testing.T) { + server, seen := newFakeServer(t, http.StatusAccepted, pendingBody) + request := aPublishRequest() + request.Name = "Pull request approved" + request.Description = "Every change reached main through an approved PR" + + _, err := newTestClient(t, server.URL, false).Publish("my-org", request) + require.NoError(t, err) + + require.Equal(t, "Pull request approved", seen.body["name"]) + require.Equal(t, "Every change reached main through an approved PR", seen.body["description"]) +} + +func TestPublishReadsThePendingVersionBack(t *testing.T) { + server, _ := newFakeServer(t, http.StatusAccepted, pendingBody) + + version, err := newTestClient(t, server.URL, false).Publish("my-org", aPublishRequest()) + require.NoError(t, err) + + require.Equal(t, &Version{ + Identifier: "pr-approved", + Version: 1, + Status: StatusPending, + Digest: "sha256:abc123", + InputSchema: "trail", + Comment: "Initial version", + CreatedAt: 1757000000.5, + CreatedBy: "Jane Doe", + }, version) +} + +func TestPublishSurfacesARefusalInTheServersWords(t *testing.T) { + for _, test := range []struct { + name string + status int + message string + }{ + {"bad input", http.StatusBadRequest, "Input schema 'many' is not one of 'trail', 'trails'"}, + {"not an admin", http.StatusForbidden, + "User 'jane' doesn't have permission to publish rego policies in organization 'my-org'"}, + {"over a cap", http.StatusRequestEntityTooLarge, + "Policy bundle is 1100000 bytes, over the 1048576 byte limit"}, + } { + t.Run(test.name, func(t *testing.T) { + server, _ := newFakeServer(t, test.status, `{"message":"`+test.message+`"}`) + + version, err := newTestClient(t, server.URL, false).Publish("my-org", aPublishRequest()) + + require.Nil(t, version) + var apiError *requests.APIError + require.True(t, errors.As(err, &apiError), "expected an API error, got %T", err) + require.Equal(t, test.status, apiError.StatusCode) + require.Equal(t, test.message, apiError.Message) + }) + } +} + +func TestPublishSendsNothingOnADryRun(t *testing.T) { + server, seen := newFakeServer(t, http.StatusAccepted, pendingBody) + + version, err := newTestClient(t, server.URL, true).Publish("my-org", aPublishRequest()) + + require.NoError(t, err) + require.Nil(t, version) + require.Equal(t, 0, seen.hits) +} + +func TestPublishKeepsTheIdentifierInOneSegment(t *testing.T) { + for identifier, escaped := range map[string]string{ + "a/b": "/api/v2/rego-policies/my-org/a%2Fb", + "../other/x": "/api/v2/rego-policies/my-org/..%2Fother%2Fx", + } { + t.Run(identifier, func(t *testing.T) { + server, seen := newFakeServer(t, http.StatusAccepted, pendingBody) + request := aPublishRequest() + request.Identifier = identifier + + _, err := newTestClient(t, server.URL, false).Publish("my-org", request) + require.NoError(t, err) + + require.Equal(t, escaped, seen.escapedPath) + }) + } +} + +func TestPublishRefusesAnIdentifierThatIsNotASegment(t *testing.T) { + for _, identifier := range []string{"", ".", ".."} { + t.Run(identifier, func(t *testing.T) { + server, seen := newFakeServer(t, http.StatusAccepted, pendingBody) + request := aPublishRequest() + request.Identifier = identifier + + version, err := newTestClient(t, server.URL, false).Publish("my-org", request) + + require.Nil(t, version) + require.EqualError(t, err, fmt.Sprintf("%q is not a valid rego policy identifier", identifier)) + require.Equal(t, 0, seen.hits) + }) + } +} + +func TestPublishDoesNotRetryAConflict(t *testing.T) { + const message = "Rego policy 'pr-approved' is being published by another request" + server, seen := newFakeServer(t, http.StatusConflict, `{"message":"`+message+`"}`) + httpClient, err := requests.NewKosliClient("", 2, false, logger.NewLogger(io.Discard, io.Discard, false)) + require.NoError(t, err) + + version, err := NewClient(httpClient, server.URL, "test-token", false).Publish("my-org", aPublishRequest()) + + require.Nil(t, version) + var apiError *requests.APIError + require.True(t, errors.As(err, &apiError), "expected an API error, got %T: %v", err, err) + require.Equal(t, http.StatusConflict, apiError.StatusCode) + require.Equal(t, message, apiError.Message) + require.Equal(t, 1, seen.hits) +} diff --git a/internal/regopolicies/version_test.go b/internal/regopolicies/version_test.go new file mode 100644 index 000000000..9e722e6f3 --- /dev/null +++ b/internal/regopolicies/version_test.go @@ -0,0 +1,112 @@ +package regopolicies + +import ( + "errors" + "net/http" + "testing" + + "github.com/kosli-dev/cli/internal/requests" + "github.com/stretchr/testify/require" +) + +const validBody = `{"identifier":"pr-approved","version":3,"status":"valid",` + + `"digest":"sha256:abc123","input_schema":"trail","comment":"Initial version",` + + `"created_at":1757000000.5,"created_by":"Jane Doe","validated_at":1757000002.25,` + + `"opa_version":"1.21.1","entrypoint":"policy",` + + `"params_schema":{"type":"object","properties":{"threshold":{"type":"integer"}}}}` + +func TestGetVersionReadsAValidVersionBack(t *testing.T) { + server, seen := newFakeServer(t, http.StatusOK, validBody) + + version, err := newTestClient(t, server.URL, false).GetVersion("my-org", "pr-approved", 3) + require.NoError(t, err) + + require.Equal(t, http.MethodGet, seen.method) + require.Equal(t, "/api/v2/rego-policies/my-org/pr-approved/versions/3", seen.path) + require.Equal(t, "Bearer test-token", seen.authHeader) + + validatedAt := 1757000002.25 + require.Equal(t, &Version{ + Identifier: "pr-approved", + Version: 3, + Status: StatusValid, + Digest: "sha256:abc123", + InputSchema: "trail", + Comment: "Initial version", + CreatedAt: 1757000000.5, + CreatedBy: "Jane Doe", + ValidatedAt: &validatedAt, + OPAVersion: "1.21.1", + Entrypoint: "policy", + ParamsSchema: map[string]any{ + "type": "object", + "properties": map[string]any{"threshold": map[string]any{"type": "integer"}}, + }, + }, version) +} + +const rejectedBody = `{"identifier":"pr-approved","version":4,"status":"rejected",` + + `"digest":"sha256:def456","input_schema":"trail","comment":"",` + + `"created_at":1757000000.5,"created_by":"Jane Doe","validated_at":1757000002.25,` + + `"opa_version":"1.21.1","error":{"kind":"compile",` + + `"message":"policy.rego:4: rego_parse_error: unexpected eof token"}}` + +func TestGetVersionReadsWhyAVersionIsNotValid(t *testing.T) { + server, _ := newFakeServer(t, http.StatusOK, rejectedBody) + + version, err := newTestClient(t, server.URL, false).GetVersion("my-org", "pr-approved", 4) + require.NoError(t, err) + + require.Equal(t, StatusRejected, version.Status) + require.Equal(t, &ValidationError{ + Kind: "compile", + Message: "policy.rego:4: rego_parse_error: unexpected eof token", + }, version.Error) + require.Empty(t, version.Entrypoint) + require.Nil(t, version.ParamsSchema) +} + +func TestGetVersionSurfacesAMissingVersion(t *testing.T) { + const message = "Version 9 of rego policy 'pr-approved' does not exist in org 'my-org'" + server, _ := newFakeServer(t, http.StatusNotFound, `{"message":"`+message+`"}`) + + version, err := newTestClient(t, server.URL, false).GetVersion("my-org", "pr-approved", 9) + + require.Nil(t, version) + var apiError *requests.APIError + require.True(t, errors.As(err, &apiError), "expected an API error, got %T", err) + require.Equal(t, http.StatusNotFound, apiError.StatusCode) + require.Equal(t, message, apiError.Message) +} + +// An unknown status is one the server added later, not an outcome. +func TestOnlyAValidatedVersionIsTerminal(t *testing.T) { + for status, terminal := range map[string]bool{ + StatusPending: false, + StatusValid: true, + StatusRejected: true, + StatusFailed: true, + "validating": false, + } { + require.Equal(t, terminal, (&Version{Status: status}).IsTerminal(), status) + } +} + +func TestGetVersionKeepsTheIdentifierInOneSegment(t *testing.T) { + server, seen := newFakeServer(t, http.StatusOK, validBody) + + _, err := newTestClient(t, server.URL, false).GetVersion("my-org", "../other/x", 3) + require.NoError(t, err) + + require.Equal(t, "/api/v2/rego-policies/my-org/..%2Fother%2Fx/versions/3", seen.escapedPath) +} + +func TestGetVersionRefusesAnIdentifierThatIsNotASegment(t *testing.T) { + server, seen := newFakeServer(t, http.StatusOK, validBody) + + version, err := newTestClient(t, server.URL, false).GetVersion("my-org", "..", 3) + + require.Nil(t, version) + require.EqualError(t, err, `".." is not a valid rego policy identifier`) + require.Equal(t, 0, seen.hits) +} diff --git a/internal/regopolicies/wait.go b/internal/regopolicies/wait.go new file mode 100644 index 000000000..cb46ecbba --- /dev/null +++ b/internal/regopolicies/wait.go @@ -0,0 +1,58 @@ +package regopolicies + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/kosli-dev/cli/internal/poll" +) + +// DefaultWaitOptions allows 30 s: a bundle at the size cap takes about 10 s to +// validate. +var DefaultWaitOptions = WaitOptions{ + Timeout: 30 * time.Second, + Initial: 500 * time.Millisecond, + Max: 5 * time.Second, +} + +// WaitOptions are poll.Options in which a field that is not set takes its default. +type WaitOptions = poll.Options + +// StillPendingError says the wait expired before the version was validated. +type StillPendingError struct { + Identifier string + Version int + URL string + Waited time.Duration +} + +func (e *StillPendingError) Error() string { + return fmt.Sprintf("version %d of rego policy %s is still pending after %s; read it later with GET %s", + e.Version, e.Identifier, e.Waited, e.URL) +} + +// WaitForTerminal polls until the version is validated, the caller gives up, or +// the budget expires. It returns a version only when that version is terminal. +func (c *Client) WaitForTerminal(ctx context.Context, org, identifier string, version int, options WaitOptions) (*Version, error) { + endpoint, err := versionURL(c.host, org, identifier, version) + if err != nil { + return nil, err + } + read, err := poll.Until(ctx, options.Or(DefaultWaitOptions), + func() (*Version, bool, error) { + read, err := c.GetVersion(org, identifier, version) + if err != nil { + return nil, false, err + } + return read, read.IsTerminal(), nil + }) + + var expired *poll.ExpiredError + if errors.As(err, &expired) { + return nil, &StillPendingError{ + Identifier: identifier, Version: version, URL: endpoint, Waited: expired.Waited} + } + return read, err +} diff --git a/internal/regopolicies/wait_test.go b/internal/regopolicies/wait_test.go new file mode 100644 index 000000000..2a579be43 --- /dev/null +++ b/internal/regopolicies/wait_test.go @@ -0,0 +1,146 @@ +package regopolicies + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/kosli-dev/cli/internal/requests" + "github.com/stretchr/testify/require" +) + +type fakeResponse struct { + status int + body string +} + +// newSequencedServer repeats the last response once the sequence runs out. +func newSequencedServer(t *testing.T, responses ...fakeResponse) (*httptest.Server, *received) { + t.Helper() + seen := &received{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + next := responses[min(seen.hits, len(responses)-1)] + seen.hits++ + seen.path = r.URL.Path + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(next.status) + _, err := w.Write([]byte(next.body)) + require.NoError(t, err) + })) + t.Cleanup(server.Close) + return server, seen +} + +// testWaitOptions has a Timeout that expires only when a test shortens it. +func testWaitOptions() WaitOptions { + return WaitOptions{ + Timeout: 2 * time.Second, + Initial: time.Millisecond, + Max: 2 * time.Millisecond, + } +} + +const unknownStatusBody = `{"identifier":"pr-approved","version":1,"status":"validating",` + + `"digest":"sha256:abc123","input_schema":"trail","comment":"",` + + `"created_at":1757000000.5,"created_by":"Jane Doe"}` + +func TestWaitReturnsTheVersionOnceItIsValidated(t *testing.T) { + server, seen := newSequencedServer(t, + fakeResponse{http.StatusOK, pendingBody}, + fakeResponse{http.StatusOK, pendingBody}, + fakeResponse{http.StatusOK, validBody}, + ) + + version, err := newTestClient(t, server.URL, false). + WaitForTerminal(context.Background(), "my-org", "pr-approved", 3, testWaitOptions()) + + require.NoError(t, err) + require.Equal(t, StatusValid, version.Status) + require.Equal(t, 3, seen.hits) + require.Equal(t, "/api/v2/rego-policies/my-org/pr-approved/versions/3", seen.path) +} + +func TestWaitReturnsARejectedVersionAsAnOutcome(t *testing.T) { + server, _ := newSequencedServer(t, + fakeResponse{http.StatusOK, pendingBody}, + fakeResponse{http.StatusOK, rejectedBody}, + ) + + version, err := newTestClient(t, server.URL, false). + WaitForTerminal(context.Background(), "my-org", "pr-approved", 4, testWaitOptions()) + + require.NoError(t, err) + require.Equal(t, StatusRejected, version.Status) + require.Equal(t, "compile", version.Error.Kind) +} + +func TestWaitKeepsGoingThroughAnUnrecognisedStatus(t *testing.T) { + server, seen := newSequencedServer(t, + fakeResponse{http.StatusOK, unknownStatusBody}, + fakeResponse{http.StatusOK, validBody}, + ) + + version, err := newTestClient(t, server.URL, false). + WaitForTerminal(context.Background(), "my-org", "pr-approved", 3, testWaitOptions()) + + require.NoError(t, err) + require.Equal(t, StatusValid, version.Status) + require.Equal(t, 2, seen.hits) +} + +func TestWaitGivesUpNamingTheVersionAndHowToReadItLater(t *testing.T) { + server, _ := newSequencedServer(t, fakeResponse{http.StatusOK, pendingBody}) + options := testWaitOptions() + options.Timeout = 20 * time.Millisecond + + version, err := newTestClient(t, server.URL, false). + WaitForTerminal(context.Background(), "my-org", "pr-approved", 1, options) + + require.Nil(t, version, "a wait that expired must never carry a version") + var stillPending *StillPendingError + require.True(t, errors.As(err, &stillPending), "expected a still-pending error, got %T", err) + require.Equal(t, server.URL+"/api/v2/rego-policies/my-org/pr-approved/versions/1", stillPending.URL) + require.Equal(t, "pr-approved", stillPending.Identifier) + require.Equal(t, 1, stillPending.Version) + require.GreaterOrEqual(t, stillPending.Waited+time.Millisecond, options.Timeout) + require.Equal(t, + "version 1 of rego policy pr-approved is still pending after "+stillPending.Waited.String()+ + "; read it later with GET "+server.URL+"/api/v2/rego-policies/my-org/pr-approved/versions/1", + err.Error()) +} + +func TestWaitStopsOnAReadItCannotRetry(t *testing.T) { + server, seen := newSequencedServer(t, + fakeResponse{http.StatusOK, pendingBody}, + fakeResponse{http.StatusNotFound, `{"message":"Rego policy 'pr-approved' does not exist"}`}, + ) + + version, err := newTestClient(t, server.URL, false). + WaitForTerminal(context.Background(), "my-org", "pr-approved", 1, testWaitOptions()) + + require.Nil(t, version) + var apiError *requests.APIError + require.True(t, errors.As(err, &apiError)) + require.Equal(t, http.StatusNotFound, apiError.StatusCode) + require.Equal(t, 2, seen.hits) +} + +func TestTheDefaultBudgetIsThirtySeconds(t *testing.T) { + require.Equal(t, 30*time.Second, DefaultWaitOptions.Timeout) +} + +func TestStillPendingNamesTheEscapedURL(t *testing.T) { + server, _ := newSequencedServer(t, fakeResponse{http.StatusOK, pendingBody}) + options := testWaitOptions() + options.Timeout = 20 * time.Millisecond + + _, err := newTestClient(t, server.URL, false). + WaitForTerminal(context.Background(), "my-org", "a/b", 2, options) + + require.ErrorContains(t, err, + "read it later with GET "+server.URL+"/api/v2/rego-policies/my-org/a%2Fb/versions/2") +} diff --git a/internal/requests/requests.go b/internal/requests/requests.go index 719862ea6..06d183b3a 100644 --- a/internal/requests/requests.go +++ b/internal/requests/requests.go @@ -327,7 +327,7 @@ func (c *Client) Do(p *RequestParams) (*HTTPResponse, error) { c.Logger.Debug("request made to %s and got status %d", req.URL, resp.StatusCode) - if resp.StatusCode != 200 && resp.StatusCode != 201 { + if !isSuccess(resp.StatusCode) { var respBody any err := json.Unmarshal([]byte(body), &respBody) if err != nil { @@ -407,6 +407,16 @@ func (c *Client) PayloadOutput(req *http.Request, jsonFields map[string]any, mes return nil } +// isSuccess accepts only statuses whose body is the whole answer, as every caller +// reads it: a 204 has no body, and a 206 or 207 only part of one. +func isSuccess(status int) bool { + switch status { + case http.StatusOK, http.StatusCreated, http.StatusAccepted: + return true + } + return false +} + func customCheckRetry(ctx context.Context, resp *http.Response, err error) (bool, error) { // Get the default retry policy for errors and certain status codes. // It will retry on 5xx, 429 (rate limit) and we add 409 (lock conflict) via a custom check. diff --git a/internal/requests/requests_test.go b/internal/requests/requests_test.go index 5343b19f1..266bd25e8 100644 --- a/internal/requests/requests_test.go +++ b/internal/requests/requests_test.go @@ -30,6 +30,17 @@ func (suite *RequestsTestSuite) SetupSuite() { Put("/artifacts/1"). Reply(201). BodyString(`{"sha": "8b4fd747df6882b897aa514af7b40571a7508cc78a8d48ae2c12f9f4bcb1598f","name": "artifact"}`) + suite.fakeService.NewHandler(). + Put("/accepted/"). + Reply(202). + BodyString(`{"status": "pending"}`) + suite.fakeService.NewHandler(). + Delete("/no-content/"). + Reply(204) + suite.fakeService.NewHandler(). + Get("/partial/"). + Reply(206). + BodyString(`{"message": "partial`) suite.fakeService.NewHandler(). Put("/html"). Reply(200). @@ -293,6 +304,32 @@ func (suite *RequestsTestSuite) TestDo() { }, expectedBody: `{"sha": "8b4fd747df6882b897aa514af7b40571a7508cc78a8d48ae2c12f9f4bcb1598f","name": "artifact"}`, }, + { + name: "202 is a success and returns its body", + params: &RequestParams{ + Method: http.MethodPut, + URL: suite.fakeService.ResolveURL("/accepted/"), + }, + expectedBody: `{"status": "pending"}`, + }, + { + name: "204 is not a success: callers read the body as the answer", + params: &RequestParams{ + Method: http.MethodDelete, + URL: suite.fakeService.ResolveURL("/no-content/"), + }, + wantError: true, + expectedErrorMsg: "unexpected end of JSON input", + }, + { + name: "206 is not a success: a partial body is not the answer", + params: &RequestParams{ + Method: http.MethodGet, + URL: suite.fakeService.ResolveURL("/partial/"), + }, + wantError: true, + expectedErrorMsg: "unexpected end of JSON input", + }, { name: "GET request to 404 endpoint", params: &RequestParams{