From eeeadc683f8322aa121216cff5bebb188ae88d6b Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 09:49:27 +0200 Subject: [PATCH 01/26] refactor(evaluations): move the wait loop into a generic poll package The rego-policy publish needs the same backoff wait as evaluations. No behaviour change. Co-Authored-By: Claude Opus 5.5 --- internal/evaluations/wait.go | 74 +++++---------------- internal/evaluations/wait_test.go | 18 ------ internal/poll/poll.go | 90 ++++++++++++++++++++++++++ internal/poll/poll_test.go | 104 ++++++++++++++++++++++++++++++ 4 files changed, 209 insertions(+), 77 deletions(-) create mode 100644 internal/poll/poll.go create mode 100644 internal/poll/poll_test.go diff --git a/internal/evaluations/wait.go b/internal/evaluations/wait.go index 860c3cd34..1b245d0cc 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,14 +22,8 @@ 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. +// WaitOptions bounds how long an evaluation is asked about and how often, with +// the same meaning as poll.Options. A zero field takes its default. type WaitOptions struct { Timeout time.Duration Initial time.Duration @@ -93,58 +88,19 @@ 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 + evaluation, err := poll.Until(ctx, poll.Options(options.withDefaults()), + func() (*Evaluation, bool, error) { + evaluation, err := c.Get(org, id) + if err != nil { + return nil, false, err + } + return evaluation, evaluation.IsTerminal(), nil + }) - 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 - } - - 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) - } -} - -// 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 + var expired *poll.ExpiredError + if errors.As(err, &expired) { + return nil, &StillPendingError{Org: org, ID: id, Waited: expired.Waited} } - return max + return evaluation, err } diff --git a/internal/evaluations/wait_test.go b/internal/evaluations/wait_test.go index 70f9c64ff..d9936e564 100644 --- a/internal/evaluations/wait_test.go +++ b/internal/evaluations/wait_test.go @@ -232,24 +232,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() diff --git a/internal/poll/poll.go b/internal/poll/poll.go new file mode 100644 index 000000000..7dd23199f --- /dev/null +++ b/internal/poll/poll.go @@ -0,0 +1,90 @@ +// Package poll asks the server about something that finishes asynchronously +// until it is done, backing off between reads. +package poll + +import ( + "context" + "fmt" + "time" +) + +// Options bounds how long a read is repeated and how often. Every field must be +// positive; callers own their defaults, since each resource has its own budget. +// +// 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 Options struct { + Timeout time.Duration + Initial time.Duration + Max time.Duration +} + +// ExpiredError says the budget ran out before the read reported done. It +// carries no value, so an unfinished answer is never mistaken for a final one. +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 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 Until[T any](ctx context.Context, options Options, read func() (T, bool, error)) (T, error) { + var zero T + // 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 zero, err + } + + value, done, err := read() + if err != nil { + // Transient trouble was already retried inside the HTTP client, so + // what reaches here will not improve by being asked again. + return zero, err + } + if done { + return value, nil + } + + select { + case <-ctx.Done(): + return zero, 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 zero, &ExpiredError{Waited: time.Since(started).Round(time.Millisecond)} + case <-time.After(interval): + } + interval = nextInterval(interval, options.Max) + } +} + +// nextInterval doubles the wait between reads, never past max. Backing off +// keeps a slow resource 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 +} diff --git a/internal/poll/poll_test.go b/internal/poll/poll_test.go new file mode 100644 index 000000000..08025bc73 --- /dev/null +++ b/internal/poll/poll_test.go @@ -0,0 +1,104 @@ +package poll + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// Short enough that a test that must wait the budget out finishes quickly, and +// long enough that a test that must not hit the budget never does. +func testOptions() Options { + return Options{ + Timeout: 2 * time.Second, + Initial: time.Millisecond, + Max: 2 * time.Millisecond, + } +} + +// sequence answers each read with the next state in turn, and repeats the +// last one once it runs 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) + } +} From aa85f6eef90caf3dc8822e2efcb2c38c0a6c1d6e Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 09:57:25 +0200 Subject: [PATCH 02/26] fix(requests): accept 202 Accepted as a success The rego-policy publish returns 202 Accepted, which Do reported as an error. Only 200, 201 and 202 are accepted: callers read the body as the whole answer, which a 204 (no body) or a 206/207 (part of one) is not. Co-Authored-By: Claude Opus 5.5 --- internal/requests/requests.go | 13 ++++++++++- internal/requests/requests_test.go | 37 ++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/internal/requests/requests.go b/internal/requests/requests.go index 719862ea6..e9c1d62a5 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,17 @@ func (c *Client) PayloadOutput(req *http.Request, jsonFields map[string]any, mes return nil } +// isSuccess accepts only the statuses whose body is the whole answer, which is +// how every caller reads it. A 204 has no body and a 206 or 207 only part of +// one, so treating those as success would hand a caller an answer it is not. +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{ From a3b09cac863e5dd3bd633ec650ef34be9fc8979b Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 10:20:50 +0200 Subject: [PATCH 03/26] feat(regopolicies): publish a rego policy version Adds the client for PUT /api/v2/rego-policies/{org}/{identifier}, which stores the next version as pending. Co-Authored-By: Claude Opus 5.5 --- internal/regopolicies/client.go | 105 ++++++++++++++++ internal/regopolicies/client_test.go | 177 +++++++++++++++++++++++++++ 2 files changed, 282 insertions(+) create mode 100644 internal/regopolicies/client.go create mode 100644 internal/regopolicies/client_test.go diff --git a/internal/regopolicies/client.go b/internal/regopolicies/client.go new file mode 100644 index 000000000..41d6155aa --- /dev/null +++ b/internal/regopolicies/client.go @@ -0,0 +1,105 @@ +// Package regopolicies talks to the Kosli rego policy API: it publishes a +// versioned Rego bundle under an identifier, and reads its versions back. It +// compiles and validates nothing itself; the server does that asynchronously. +package regopolicies + +import ( + "encoding/json" + "fmt" + "net/http" + "net/url" + + "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 describes one publish of a policy. An empty Name or +// Description is left out, so the server keeps the value it has. +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"` +} + +// 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) { + endpoint, err := url.JoinPath(c.host, "api/v2/rego-policies", org, request.Identifier) + if err != nil { + return nil, err + } + + response, err := c.http.Do(&requests.RequestParams{ + Method: http.MethodPut, + URL: endpoint, + Token: c.token, + DryRun: c.dryRun, + 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) +} + +// The payload type is separate from PublishRequest because the server forbids +// unknown fields: the wire shape has to be stated exactly here. +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..65d6ee93b --- /dev/null +++ b/internal/regopolicies/client_test.go @@ -0,0 +1,177 @@ +package regopolicies + +import ( + "encoding/json" + "errors" + "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" +) + +// received is what the fake server saw, so a test can assert on the request +// rather than only on what came back. +type received struct { + hits int + method string + path 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.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) +} From 32460f88e29681073a234af549495f2ae76320ba Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 10:27:17 +0200 Subject: [PATCH 04/26] feat(regopolicies): read a version back and wait for its validation Co-Authored-By: Claude Opus 5.5 --- internal/regopolicies/client.go | 48 +++++++++ internal/regopolicies/version_test.go | 94 +++++++++++++++++ internal/regopolicies/wait.go | 75 ++++++++++++++ internal/regopolicies/wait_test.go | 141 ++++++++++++++++++++++++++ 4 files changed, 358 insertions(+) create mode 100644 internal/regopolicies/version_test.go create mode 100644 internal/regopolicies/wait.go create mode 100644 internal/regopolicies/wait_test.go diff --git a/internal/regopolicies/client.go b/internal/regopolicies/client.go index 41d6155aa..bfd3aeac6 100644 --- a/internal/regopolicies/client.go +++ b/internal/regopolicies/client.go @@ -5,9 +5,11 @@ package regopolicies import ( "encoding/json" + "errors" "fmt" "net/http" "net/url" + "strconv" "github.com/kosli-dev/cli/internal/requests" ) @@ -42,6 +44,26 @@ type Version struct { 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"` + OPAVersion string `json:"opa_version"` + // Set only on a valid version; ParamsSchema only when the policy declares one. + Entrypoint string `json:"entrypoint"` + ParamsSchema map[string]any `json:"params_schema"` + // Set only on a rejected or failed version. + Error *ValidationError `json:"error"` +} + +// 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: the +// set belongs to the evaluator, 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. @@ -86,6 +108,32 @@ func (c *Client) Publish(org string, request PublishRequest) (*Version, error) { 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 := url.JoinPath(c.host, "api/v2/rego-policies", org, identifier, + "versions", strconv.Itoa(version)) + if err != nil { + return nil, err + } + + // DryRun is deliberately not set: 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 { + // The shared client answers a suppressed request with nothing at all, + // so a later change that suppresses this read meets this, not a panic. + return nil, errors.New("no rego policy version was read back") + } + return decodeVersion(response.Body) +} + // The payload type is separate from PublishRequest because the server forbids // unknown fields: the wire shape has to be stated exactly here. type publishPayload struct { diff --git a/internal/regopolicies/version_test.go b/internal/regopolicies/version_test.go new file mode 100644 index 000000000..98002038e --- /dev/null +++ b/internal/regopolicies/version_test.go @@ -0,0 +1,94 @@ +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 means the server has added one; reading it as terminal +// would report an unfinished validation as 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) + } +} diff --git a/internal/regopolicies/wait.go b/internal/regopolicies/wait.go new file mode 100644 index 000000000..0ed4a2a28 --- /dev/null +++ b/internal/regopolicies/wait.go @@ -0,0 +1,75 @@ +package regopolicies + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/kosli-dev/cli/internal/poll" +) + +// DefaultWaitOptions gives validation the same thirty seconds the platform +// states for an evaluation. A bundle at the size cap takes about ten seconds to +// compile, so this is ample without hiding a stuck version. +var DefaultWaitOptions = WaitOptions{ + Timeout: 30 * time.Second, + Initial: 500 * time.Millisecond, + Max: 5 * time.Second, +} + +// WaitOptions bounds how long a version is asked about and how often, with the +// same meaning as poll.Options. A zero field takes its default. +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 +} + +// StillPendingError says the wait expired before the version was validated. It +// carries no version, so an unfinished validation is never read as an outcome. +type StillPendingError struct { + Org string + Identifier string + Version int + 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 /api/v2/rego-policies/%s/%s/versions/%d", + e.Version, e.Identifier, e.Waited, e.Org, e.Identifier, e.Version) +} + +// 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) { + read, err := poll.Until(ctx, poll.Options(options.withDefaults()), + 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{ + Org: org, Identifier: identifier, Version: version, 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..535599971 --- /dev/null +++ b/internal/regopolicies/wait_test.go @@ -0,0 +1,141 @@ +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 answers each request with the next response in turn, and +// repeats the last one 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 +} + +// Short enough that a test that must wait the budget out finishes quickly, and +// long enough that a test that must not hit the budget never does. +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) +} + +// A rejected policy is an answer, not a failure to get one: the caller decides +// what it means. +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, "my-org", stillPending.Org) + 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 /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 TestWaitOptionsFallBackToTheDefaultBudget(t *testing.T) { + filled := WaitOptions{}.withDefaults() + + require.Equal(t, DefaultWaitOptions, filled) + require.Equal(t, 30*time.Second, filled.Timeout) +} From e78f8c3493a67147df9de11935257323d313b8d4 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 10:38:47 +0200 Subject: [PATCH 05/26] feat(create): hidden kosli create rego-policy, publish and wait for a valid version Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/create.go | 1 + cmd/kosli/createRegoPolicy.go | 116 ++++++++++ cmd/kosli/createRegoPolicy_test.go | 201 ++++++++++++++++++ cmd/kosli/root.go | 4 + .../testdata/empty-flag-audit-coverage.json | 8 +- hack/empty-flag-audit/spec.json | 26 +++ 6 files changed, 355 insertions(+), 1 deletion(-) create mode 100644 cmd/kosli/createRegoPolicy.go create mode 100644 cmd/kosli/createRegoPolicy_test.go diff --git a/cmd/kosli/create.go b/cmd/kosli/create.go index 5db4fa714..5ff32d440 100644 --- a/cmd/kosli/create.go +++ b/cmd/kosli/create.go @@ -21,6 +21,7 @@ func newCreateCmd(out io.Writer) *cobra.Command { newCreateEnvironmentCmd(out), newCreateFlowCmd(out), newCreatePolicyCmd(out), + newCreateRegoPolicyCmd(out), newCreateAttestationTypeCmd(out), newCreateControlCmd(out), newCreateApiKeyCmd(out), diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go new file mode 100644 index 000000000..08e755aba --- /dev/null +++ b/cmd/kosli/createRegoPolicy.go @@ -0,0 +1,116 @@ +package main + +import ( + "context" + "fmt" + "io" + + "github.com/kosli-dev/cli/internal/docgen" + "github.com/kosli-dev/cli/internal/regopolicies" + "github.com/spf13/cobra" +) + +const createRegoPolicyShortDesc = `Publish a new version of a rego policy in a Kosli org.` + +const createRegoPolicyLongDesc = createRegoPolicyShortDesc + ` +The policy is addressed by IDENTIFIER, which cannot be changed after the first +publish. PATH is a Rego file or a directory, sent as one bundle in the same way +as ^kosli evaluate policy --policy^ sends it. + +Kosli validates the new version after it is stored. The command waits for that +result, and exits 0 only when the version is valid.` + +const createRegoPolicyExample = ` +# publish the first version of a rego policy from a directory: +kosli create 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 +` + +// regoPolicyWaitOptions is how long validation is waited for, and the seam a +// test shortens it through. +var regoPolicyWaitOptions = regopolicies.WaitOptions{} + +type createRegoPolicyOptions struct { + name string + description string + inputSchema string + comment string +} + +func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { + o := new(createRegoPolicyOptions) + cmd := &cobra.Command{ + Use: "rego-policy IDENTIFIER PATH", + Short: createRegoPolicyShortDesc, + Long: createRegoPolicyLongDesc, + Example: createRegoPolicyExample, + Args: cobra.ExactArgs(2), + // Hidden while the server endpoint is behind a feature flag. + Hidden: true, + Annotations: map[string]string{docgen.DocHiddenAnnotation: "", 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) + + err := RequireFlags(cmd, []string{"input-schema"}) + if err != nil { + logger.Error("failed to configure required flags: %v", err) + } + + return cmd +} + +func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) error { + 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 { + // A dry run sent nothing, so there is no version to wait for. + return nil + } + + version, err := client.WaitForTerminal(context.Background(), global.Org, identifier, + published.Version, regoPolicyWaitOptions) + if err != nil { + return err + } + if version.Status != regopolicies.StatusValid { + return fmt.Errorf("rego policy '%s' version %d is %s", identifier, version.Version, version.Status) + } + + _, err = fmt.Fprintf(out, "rego policy '%s' version %d is valid (%s)\n", + identifier, version.Version, version.Digest) + return err +} diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go new file mode 100644 index 000000000..4c3eef931 --- /dev/null +++ b/cmd/kosli/createRegoPolicy_test.go @@ -0,0 +1,201 @@ +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"}` +) + +// fakeRegoPolicies records each publish and answers each version read with the +// next body in reads, repeating the last one once they run out. +type fakeRegoPolicies struct { + published []map[string]any + publishURL string + reads []string + 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) + _, _ = fmt.Fprint(w, regoPolicyPending) + + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/versions/"): + fake.readURL = r.URL.Path + w.WriteHeader(http.StatusOK) + _, _ = 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 CreateRegoPolicyCommandTestSuite struct { + suite.Suite + originalWait regopolicies.WaitOptions +} + +func (suite *CreateRegoPolicyCommandTestSuite) SetupTest() { + suite.originalWait = regoPolicyWaitOptions + regoPolicyWaitOptions = regopolicies.WaitOptions{ + Timeout: 2 * time.Second, + Initial: time.Millisecond, + Max: 2 * time.Millisecond, + } +} + +func (suite *CreateRegoPolicyCommandTestSuite) TearDownTest() { + regoPolicyWaitOptions = suite.originalWait +} + +func (suite *CreateRegoPolicyCommandTestSuite) createCmd(host, args string) string { + return fmt.Sprintf("create rego-policy %s --host %s --org test-org --api-token test-token --max-api-retries 0", + args, host) +} + +func (suite *CreateRegoPolicyCommandTestSuite) TestItIsHiddenFromHelp() { + _, combined, _, _, err := executeCommandC("create --help") + + require.NoError(suite.T(), err) + require.NotContains(suite.T(), combined, "rego-policy") + require.Contains(suite.T(), combined, "flow", "the rest of the help still renders") +} + +func (suite *CreateRegoPolicyCommandTestSuite) TestItPublishesTheFileAndReportsTheValidVersion() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, combined, _, _, err := executeCommandC(suite.createCmd(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 *CreateRegoPolicyCommandTestSuite) TestItLeavesOutTheNameAndDescriptionWhenNotGiven() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, _, _, _, err := executeCommandC(suite.createCmd(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 *CreateRegoPolicyCommandTestSuite) TestItWaitsThroughPendingReads() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyPending, regoPolicyPending, regoPolicyValid) + + _, combined, _, _, err := executeCommandC(suite.createCmd(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.Contains(suite.T(), combined, "is valid") +} + +// A directory travels as `evaluate policy` sends it, so the same directory +// gives the same digest on both paths. +func (suite *CreateRegoPolicyCommandTestSuite) 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.createCmd(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"]) +} + +// Until the outcomes are told apart, nothing but a valid version may read as +// one. +func (suite *CreateRegoPolicyCommandTestSuite) TestAVersionThatIsNotValidFails() { + rejected := strings.Replace(regoPolicyValid, `"status":"valid"`, `"status":"rejected"`, 1) + server, _ := newFakeRegoPolicies(suite.T(), rejected) + + _, combined, _, _, err := executeCommandC(suite.createCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "rejected") + require.NotContains(suite.T(), combined, "is valid") +} + +func (suite *CreateRegoPolicyCommandTestSuite) TestItNeedsAnIdentifierAndAPath() { + _, _, _, _, err := executeCommandC(suite.createCmd("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 *CreateRegoPolicyCommandTestSuite) TestItNeedsAnInputSchema() { + _, _, _, _, err := executeCommandC(suite.createCmd("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 TestCreateRegoPolicyCommandTestSuite(t *testing.T) { + suite.Run(t, new(CreateRegoPolicyCommandTestSuite)) +} diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index f20ad3d77..feb0f398a 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." diff --git a/cmd/kosli/testdata/empty-flag-audit-coverage.json b/cmd/kosli/testdata/empty-flag-audit-coverage.json index 46192fdd7..dc66e5f00 100644 --- a/cmd/kosli/testdata/empty-flag-audit-coverage.json +++ b/cmd/kosli/testdata/empty-flag-audit-coverage.json @@ -561,6 +561,12 @@ "dry-run": "bool", "type": "string" }, + "create rego-policy": { + "comment": "string", + "description": "string", + "input-schema": "string", + "name": "string" + }, "create service-account": { "description": "string", "dry-run": "bool", @@ -608,7 +614,7 @@ "name": "string", "output": "string", "params": "string", - "policy": "string", + "policy": "stringArray", "trail": "string" }, "evaluate trail": { diff --git a/hack/empty-flag-audit/spec.json b/hack/empty-flag-audit/spec.json index 5a9379663..1084fd8e1 100644 --- a/hack/empty-flag-audit/spec.json +++ b/hack/empty-flag-audit/spec.json @@ -1521,6 +1521,32 @@ ] ] }, + "create 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", + "input-schema", + "name" + ], + "flag_values": { + "comment": "probe-comment", + "description": "probe-description", + "input-schema": "trail", + "name": "probe-name" + }, + "setup": [], + "verify": [] + }, "create service-account": { "args": [ "{account}" From 154086610524ffdd49a105f7038dc8e2a62f061e Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 10:43:42 +0200 Subject: [PATCH 06/26] feat(create): tell rejected, failed and still-pending rego policy versions apart Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 24 +++++++++- cmd/kosli/createRegoPolicy_test.go | 71 ++++++++++++++++++++++++++---- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 08e755aba..8e97fe543 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -107,10 +107,32 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er return err } if version.Status != regopolicies.StatusValid { - return fmt.Errorf("rego policy '%s' version %d is %s", identifier, version.Version, version.Status) + return notValidError(identifier, version) } _, err = fmt.Fprintf(out, "rego policy '%s' version %d is valid (%s)\n", identifier, version.Version, version.Digest) return err } + +// notValidError reports a version that will never be evaluated. A rejected +// version is the policy's fault and a failed one is Kosli's, so the two are +// worded apart: the author of a failed version has nothing to fix. +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/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go index 4c3eef931..ff26b1865 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/createRegoPolicy_test.go @@ -25,6 +25,15 @@ const ( `"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 records each publish and answers each version read with the @@ -166,18 +175,64 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestADirectoryTravelsKeyedByRelat require.Equal(suite.T(), "trails", fake.published[0]["input_schema"]) } -// Until the outcomes are told apart, nothing but a valid version may read as -// one. -func (suite *CreateRegoPolicyCommandTestSuite) TestAVersionThatIsNotValidFails() { - rejected := strings.Replace(regoPolicyValid, `"status":"valid"`, `"status":"rejected"`, 1) - server, _ := newFakeRegoPolicies(suite.T(), rejected) - +func (suite *CreateRegoPolicyCommandTestSuite) publishExpectingError(reads ...string) (string, error) { + server, _ := newFakeRegoPolicies(suite.T(), reads...) _, combined, _, _, err := executeCommandC(suite.createCmd(server.URL, "pr-approved testdata/policies/allow-all.rego --input-schema trail")) - require.Error(suite.T(), err) - require.Contains(suite.T(), err.Error(), "rejected") require.NotContains(suite.T(), combined, "is valid") + return combined, err +} + +func (suite *CreateRegoPolicyCommandTestSuite) 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 *CreateRegoPolicyCommandTestSuite) 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()) +} + +// A failed version is our fault, not the policy's: the author must not go +// looking for a defect in the policy, and publishing again is the way back. +func (suite *CreateRegoPolicyCommandTestSuite) 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 *CreateRegoPolicyCommandTestSuite) 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 *CreateRegoPolicyCommandTestSuite) 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.Contains(suite.T(), err.Error(), + "read it later with GET /api/v2/rego-policies/test-org/pr-approved/versions/1") } func (suite *CreateRegoPolicyCommandTestSuite) TestItNeedsAnIdentifierAndAPath() { From 43f5f015487d220e1d07f72047b04ede37034a58 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 10:48:22 +0200 Subject: [PATCH 07/26] feat(create): refuse an unknown input schema and a policy URL before publishing Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 12 ++++++++ cmd/kosli/createRegoPolicy_test.go | 48 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 8e97fe543..3bf314287 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io" + "slices" "github.com/kosli-dev/cli/internal/docgen" "github.com/kosli-dev/cli/internal/regopolicies" @@ -35,6 +36,8 @@ kosli create rego-policy pr-approved ./policies/pr-approved/ \ // test shortens it through. var regoPolicyWaitOptions = regopolicies.WaitOptions{} +var regoPolicyInputSchemas = []string{"trail", "trails"} + type createRegoPolicyOptions struct { name string description string @@ -79,6 +82,15 @@ func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { } func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) error { + if !slices.Contains(regoPolicyInputSchemas, o.inputSchema) { + return fmt.Errorf("--input-schema must be one of %v, got %q", regoPolicyInputSchemas, o.inputSchema) + } + // A published version is cited by decisions, so its source must be what + // the author has on disk, not whatever a URL serves at the moment. + 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 diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go index ff26b1865..1d721efd2 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/createRegoPolicy_test.go @@ -235,6 +235,54 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAWaitThatExpiresSaysHowToRead "read it later with GET /api/v2/rego-policies/test-org/pr-approved/versions/1") } +// refusedBeforeSending runs args against a fake server and returns the error, +// requiring that nothing reached the server. +func (suite *CreateRegoPolicyCommandTestSuite) refusedBeforeSending(args string) error { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, _, _, _, err := executeCommandC(suite.createCmd(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 *CreateRegoPolicyCommandTestSuite) 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 *CreateRegoPolicyCommandTestSuite) 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 *CreateRegoPolicyCommandTestSuite) 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, +// so an empty value is refused like on every other flag. +func (suite *CreateRegoPolicyCommandTestSuite) 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 *CreateRegoPolicyCommandTestSuite) TestItNeedsAnIdentifierAndAPath() { _, _, _, _, err := executeCommandC(suite.createCmd("http://localhost:1", "pr-approved --input-schema trail")) From 6c95937c4ee1631e059f20eba81d021fb530f26d Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 10:53:15 +0200 Subject: [PATCH 08/26] feat(create): add --output json and --dry-run to create rego-policy Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 30 ++++++++++-- cmd/kosli/createRegoPolicy_test.go | 47 +++++++++++++++++++ .../testdata/empty-flag-audit-coverage.json | 4 +- hack/empty-flag-audit/spec.json | 8 +++- internal/regopolicies/client.go | 10 ++-- 5 files changed, 88 insertions(+), 11 deletions(-) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 3bf314287..00f0d4e47 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -2,11 +2,13 @@ package main import ( "context" + "encoding/json" "fmt" "io" "slices" "github.com/kosli-dev/cli/internal/docgen" + "github.com/kosli-dev/cli/internal/output" "github.com/kosli-dev/cli/internal/regopolicies" "github.com/spf13/cobra" ) @@ -43,6 +45,7 @@ type createRegoPolicyOptions struct { description string inputSchema string comment string + output string } func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { @@ -72,6 +75,8 @@ func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { 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 { @@ -82,6 +87,12 @@ func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { } func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) error { + // Refused before publishing: a format refused where it is printed would + // leave a version published by a command that reported an error. + 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) } @@ -122,9 +133,22 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er return notValidError(identifier, version) } - _, err = fmt.Fprintf(out, "rego policy '%s' version %d is valid (%s)\n", - identifier, version.Version, version.Digest) - return err + return printVersion(out, version) +} + +var regoPolicyPrinters = map[string]func(io.Writer, *regopolicies.Version) error{ + "table": func(out io.Writer, version *regopolicies.Version) error { + _, err := fmt.Fprintf(out, "rego policy '%s' version %d is valid (%s)\n", + version.Identifier, version.Version, version.Digest) + return err + }, + "json": func(out io.Writer, version *regopolicies.Version) error { + raw, err := json.Marshal(version) + if err != nil { + return err + } + return output.PrintJson(string(raw), out, 0) + }, } // notValidError reports a version that will never be evaluated. A rejected diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go index 1d721efd2..632b890c6 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/createRegoPolicy_test.go @@ -283,6 +283,53 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAnEmptyDescriptionIsRefused() require.Contains(suite.T(), err.Error(), "flag '--description' was given an empty value") } +func (suite *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsTheValidVersion() { + server, _ := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, _, stdout, _, err := executeCommandC(suite.createCmd(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") +} + +// Refused before publishing: a format refused where it is printed would leave +// a version published by a command that reported an error. +func (suite *CreateRegoPolicyCommandTestSuite) 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 *CreateRegoPolicyCommandTestSuite) TestADryRunSendsNothingAndWaitsForNothing() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) + + _, combined, _, _, err := executeCommandC(suite.createCmd(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") +} + func (suite *CreateRegoPolicyCommandTestSuite) TestItNeedsAnIdentifierAndAPath() { _, _, _, _, err := executeCommandC(suite.createCmd("http://localhost:1", "pr-approved --input-schema trail")) diff --git a/cmd/kosli/testdata/empty-flag-audit-coverage.json b/cmd/kosli/testdata/empty-flag-audit-coverage.json index dc66e5f00..a887eba86 100644 --- a/cmd/kosli/testdata/empty-flag-audit-coverage.json +++ b/cmd/kosli/testdata/empty-flag-audit-coverage.json @@ -564,8 +564,10 @@ "create rego-policy": { "comment": "string", "description": "string", + "dry-run": "bool", "input-schema": "string", - "name": "string" + "name": "string", + "output": "string" }, "create service-account": { "description": "string", diff --git a/hack/empty-flag-audit/spec.json b/hack/empty-flag-audit/spec.json index 1084fd8e1..9e731d4d2 100644 --- a/hack/empty-flag-audit/spec.json +++ b/hack/empty-flag-audit/spec.json @@ -1535,14 +1535,18 @@ "flags_to_test": [ "comment", "description", + "dry-run", "input-schema", - "name" + "name", + "output" ], "flag_values": { "comment": "probe-comment", "description": "probe-description", + "dry-run": "true", "input-schema": "trail", - "name": "probe-name" + "name": "probe-name", + "output": "json" }, "setup": [], "verify": [] diff --git a/internal/regopolicies/client.go b/internal/regopolicies/client.go index bfd3aeac6..b6f2539d6 100644 --- a/internal/regopolicies/client.go +++ b/internal/regopolicies/client.go @@ -45,13 +45,13 @@ type Version struct { CreatedAt float64 `json:"created_at"` CreatedBy string `json:"created_by"` // Nil while the version is pending. - ValidatedAt *float64 `json:"validated_at"` - OPAVersion string `json:"opa_version"` + 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"` - ParamsSchema map[string]any `json:"params_schema"` + 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"` + Error *ValidationError `json:"error,omitempty"` } // IsTerminal reports whether the version has left pending, which it does once. From 68fff87dbda3ef84efb309b4c59ced4a0ca6b086 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 10:57:14 +0200 Subject: [PATCH 09/26] docs(create): explain versions, outcomes and exit codes in create rego-policy help Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 54 +++++++++++++++++++++++++++++++---- cmd/kosli/lifecycle_test.go | 16 +++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 00f0d4e47..521126543 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -16,12 +16,32 @@ import ( const createRegoPolicyShortDesc = `Publish a new version of a rego policy in a Kosli org.` const createRegoPolicyLongDesc = createRegoPolicyShortDesc + ` -The policy is addressed by IDENTIFIER, which cannot be changed after the first -publish. PATH is a Rego file or a directory, sent as one bundle in the same way -as ^kosli evaluate policy --policy^ sends it. - -Kosli validates the new version after it is stored. The command waits for that -result, and exits 0 only when the version is valid.` +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. + +An evaluation runs the highest-numbered valid version, so a rejected or failed +version leaves the policy in service unchanged.` const createRegoPolicyExample = ` # publish the first version of a rego policy from a directory: @@ -32,6 +52,28 @@ kosli create rego-policy pr-approved ./policies/pr-approved/ \ --comment "Initial version" \ --api-token yourAPIToken \ --org yourOrgName + +# publish a new version of an existing rego policy from one file: +kosli create 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 create 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 create rego-policy pr-approved ./policies/pr-approved/ \ + --input-schema trail \ + --dry-run \ + --api-token yourAPIToken \ + --org yourOrgName ` // regoPolicyWaitOptions is how long validation is waited for, and the seam a diff --git a/cmd/kosli/lifecycle_test.go b/cmd/kosli/lifecycle_test.go index 39d40176f..738f533c5 100644 --- a/cmd/kosli/lifecycle_test.go +++ b/cmd/kosli/lifecycle_test.go @@ -50,6 +50,22 @@ func TestLifecycleAttestDecisionIsBetaAndVisible(t *testing.T) { } } +// Hidden while the rego-policies API is behind the +// is-server-side-evaluation-enabled flag on the server. +func TestLifecycleCreateRegoPolicyIsBetaAndHidden(t *testing.T) { + global = &GlobalOpts{} + cmd := newCreateRegoPolicyCmd(io.Discard) + if !cmd.Hidden { + t.Error("expected create rego-policy to be Hidden") + } + if !isBeta(cmd) { + t.Error("expected create rego-policy to be beta") + } + if !isDocHidden(cmd) { + t.Error("expected create rego-policy to be left out of the docs") + } +} + func TestLifecycleSnapshotGKEIsBetaAndVisible(t *testing.T) { cmd := newSnapshotGKECmd(io.Discard) if cmd.Hidden { From 9ac6624fae528fa6002b1c64045d3691c7306e1a Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 11:10:43 +0200 Subject: [PATCH 10/26] fix(regopolicies): keep the identifier in one path segment A slash in an identifier was read as a path separator, and "../x" left the org's route. The identifier and org are now escaped, and "", "." and "..", which escaping cannot keep in place, are refused before sending. Co-Authored-By: Claude Opus 5.5 --- internal/regopolicies/client.go | 25 +++++++++++++++-- internal/regopolicies/client_test.go | 40 +++++++++++++++++++++++++++ internal/regopolicies/version_test.go | 19 +++++++++++++ internal/regopolicies/wait.go | 11 ++++++-- internal/regopolicies/wait_test.go | 11 ++++++++ 5 files changed, 100 insertions(+), 6 deletions(-) diff --git a/internal/regopolicies/client.go b/internal/regopolicies/client.go index b6f2539d6..27dbfa18b 100644 --- a/internal/regopolicies/client.go +++ b/internal/regopolicies/client.go @@ -81,7 +81,11 @@ func NewClient(httpClient *requests.Client, host, token string, dryRun bool) *Cl // 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) { - endpoint, err := url.JoinPath(c.host, "api/v2/rego-policies", org, request.Identifier) + 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 } @@ -111,8 +115,11 @@ func (c *Client) Publish(org string, request PublishRequest) (*Version, error) { // 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 := url.JoinPath(c.host, "api/v2/rego-policies", org, identifier, - "versions", strconv.Itoa(version)) + route, err := policyRoute(org, identifier) + if err != nil { + return nil, err + } + endpoint, err := url.JoinPath(c.host, route, "versions", strconv.Itoa(version)) if err != nil { return nil, err } @@ -134,6 +141,18 @@ func (c *Client) GetVersion(org, identifier string, version int) (*Version, erro return decodeVersion(response.Body) } +// policyRoute is the escaped API path of one policy. The identifier is a single +// segment, so a slash in it is escaped rather than read as a separator, and the +// names a URL path resolves on its own are refused: escaping cannot keep "." +// or ".." in their segment. +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 +} + // The payload type is separate from PublishRequest because the server forbids // unknown fields: the wire shape has to be stated exactly here. type publishPayload struct { diff --git a/internal/regopolicies/client_test.go b/internal/regopolicies/client_test.go index 65d6ee93b..d35205188 100644 --- a/internal/regopolicies/client_test.go +++ b/internal/regopolicies/client_test.go @@ -3,6 +3,7 @@ package regopolicies import ( "encoding/json" "errors" + "fmt" "io" "net/http" "net/http/httptest" @@ -19,6 +20,7 @@ type received struct { hits int method string path string + escapedPath string authHeader string contentType string body map[string]any @@ -31,6 +33,7 @@ func newFakeServer(t *testing.T, status int, responseBody string) (*httptest.Ser 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") @@ -175,3 +178,40 @@ func TestPublishSendsNothingOnADryRun(t *testing.T) { require.Nil(t, version) require.Equal(t, 0, seen.hits) } + +// The identifier is one path segment: a slash in it must not reach another +// route, least of all one outside the org. +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) + }) + } +} + +// Escaping cannot keep these in their segment: a URL path resolves them. +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) + }) + } +} diff --git a/internal/regopolicies/version_test.go b/internal/regopolicies/version_test.go index 98002038e..88a677c49 100644 --- a/internal/regopolicies/version_test.go +++ b/internal/regopolicies/version_test.go @@ -92,3 +92,22 @@ func TestOnlyAValidatedVersionIsTerminal(t *testing.T) { 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 index 0ed4a2a28..09d513b76 100644 --- a/internal/regopolicies/wait.go +++ b/internal/regopolicies/wait.go @@ -49,9 +49,14 @@ type StillPendingError struct { } func (e *StillPendingError) Error() string { - return fmt.Sprintf( - "version %d of rego policy %s is still pending after %s; read it later with GET /api/v2/rego-policies/%s/%s/versions/%d", - e.Version, e.Identifier, e.Waited, e.Org, e.Identifier, e.Version) + route, err := policyRoute(e.Org, e.Identifier) + if err != nil { + // Unreachable: the wait reads the version through the same route first. + return fmt.Sprintf("version %d of rego policy %s is still pending after %s", + e.Version, e.Identifier, e.Waited) + } + return fmt.Sprintf("version %d of rego policy %s is still pending after %s; read it later with GET %s/versions/%d", + e.Version, e.Identifier, e.Waited, route, e.Version) } // WaitForTerminal polls until the version is validated, the caller gives up, or diff --git a/internal/regopolicies/wait_test.go b/internal/regopolicies/wait_test.go index 535599971..8893b3f23 100644 --- a/internal/regopolicies/wait_test.go +++ b/internal/regopolicies/wait_test.go @@ -139,3 +139,14 @@ func TestWaitOptionsFallBackToTheDefaultBudget(t *testing.T) { require.Equal(t, DefaultWaitOptions, filled) require.Equal(t, 30*time.Second, filled.Timeout) } + +// The route in the message is one a reader can paste, so it is escaped the way +// the request was. +func TestStillPendingNamesTheEscapedRoute(t *testing.T) { + err := &StillPendingError{Org: "my-org", Identifier: "a/b", Version: 2, Waited: time.Second} + + require.Equal(t, + "version 2 of rego policy a/b is still pending after 1s; "+ + "read it later with GET /api/v2/rego-policies/my-org/a%2Fb/versions/2", + err.Error()) +} From 01cefecd6150c11124a8ca3f71f0026d5218efd9 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 11:11:09 +0200 Subject: [PATCH 11/26] fix(create): do not poll a version the publish already answered as terminal Publishing content the newest valid version already holds answers with that version, so the extra read was wasted. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 13 ++++++++---- cmd/kosli/createRegoPolicy_test.go | 34 ++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 521126543..6bce6ab1b 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -166,10 +166,15 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er return nil } - version, err := client.WaitForTerminal(context.Background(), global.Org, identifier, - published.Version, regoPolicyWaitOptions) - if err != nil { - return err + // A publish of content the newest version already holds answers with that + // version, which can already be terminal. + version := published + if !version.IsTerminal() { + version, err = client.WaitForTerminal(context.Background(), global.Org, identifier, + published.Version, regoPolicyWaitOptions) + if err != nil { + return err + } } if version.Status != regopolicies.StatusValid { return notValidError(identifier, version) diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go index 632b890c6..93ab81450 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/createRegoPolicy_test.go @@ -39,12 +39,14 @@ const ( // fakeRegoPolicies records each publish and answers each version read with the // next body in reads, repeating the last one once they run out. type fakeRegoPolicies struct { - published []map[string]any - publishURL string - reads []string - readCount int - readURL string - unexpected int + // What the publish answers with; empty means a pending version. + publishAnswer string + published []map[string]any + publishURL string + reads []string + readCount int + readURL string + unexpected int } func newFakeRegoPolicies(t *testing.T, reads ...string) (*httptest.Server, *fakeRegoPolicies) { @@ -60,7 +62,11 @@ func newFakeRegoPolicies(t *testing.T, reads ...string) (*httptest.Server, *fake fake.published = append(fake.published, body) fake.publishURL = r.URL.Path w.WriteHeader(http.StatusAccepted) - _, _ = fmt.Fprint(w, regoPolicyPending) + 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 @@ -330,6 +336,20 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestADryRunSendsNothingAndWaitsFo require.NotContains(suite.T(), combined, "is valid") } +// Publishing content the newest valid version already holds stores nothing and +// answers with that version, so there is nothing left to wait for. +func (suite *CreateRegoPolicyCommandTestSuite) TestAPublishAnsweredWithAValidVersionIsNotPolled() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyPending) + fake.publishAnswer = regoPolicyValid + + _, combined, _, _, err := executeCommandC(suite.createCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail")) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), 0, fake.readCount) + require.Contains(suite.T(), combined, "rego policy 'pr-approved' version 1 is valid (sha256:abc123)") +} + func (suite *CreateRegoPolicyCommandTestSuite) TestItNeedsAnIdentifierAndAPath() { _, _, _, _, err := executeCommandC(suite.createCmd("http://localhost:1", "pr-approved --input-schema trail")) From aa78c6d6c920f6a0cc8bf23072f73cdcd1d8f57c Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 11:11:16 +0200 Subject: [PATCH 12/26] fix(regopolicies): report a 409 on publish instead of retrying it The server re-runs a publish that loses a race, so a 409 is a refusal. Retrying it as a lock delayed the answer and lost the server's message. Co-Authored-By: Claude Opus 5.5 --- internal/regopolicies/client.go | 3 +++ internal/regopolicies/client_test.go | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/internal/regopolicies/client.go b/internal/regopolicies/client.go index 27dbfa18b..9edaee1fc 100644 --- a/internal/regopolicies/client.go +++ b/internal/regopolicies/client.go @@ -95,6 +95,9 @@ func (c *Client) Publish(org string, request PublishRequest) (*Version, error) { 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, diff --git a/internal/regopolicies/client_test.go b/internal/regopolicies/client_test.go index d35205188..5017bd315 100644 --- a/internal/regopolicies/client_test.go +++ b/internal/regopolicies/client_test.go @@ -215,3 +215,21 @@ func TestPublishRefusesAnIdentifierThatIsNotASegment(t *testing.T) { }) } } + +// A 409 from this endpoint is a refusal, not a lock to wait out: the server +// re-runs a lost race itself. Retrying would delay the answer and lose its words. +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) +} From 470d9da838f91c4b0eda19c5767fc07f22a84762 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 11:22:55 +0200 Subject: [PATCH 13/26] refactor: hold comments to the comment standard Keep only the fact each comment states that the code cannot show, and drop the comments that restate a test name or what a helper does. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 20 +++++--------- cmd/kosli/createRegoPolicy_test.go | 19 +++---------- cmd/kosli/lifecycle_test.go | 3 +- internal/evaluations/wait.go | 3 +- internal/poll/poll.go | 40 ++++++--------------------- internal/poll/poll_test.go | 6 ++-- internal/regopolicies/client.go | 26 +++++++---------- internal/regopolicies/client_test.go | 7 ----- internal/regopolicies/version_test.go | 3 +- internal/regopolicies/wait.go | 11 +++----- internal/regopolicies/wait_test.go | 10 ++----- internal/requests/requests.go | 5 ++-- 12 files changed, 43 insertions(+), 110 deletions(-) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 6bce6ab1b..981c8aa24 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -76,8 +76,7 @@ kosli create rego-policy pr-approved ./policies/pr-approved/ \ --org yourOrgName ` -// regoPolicyWaitOptions is how long validation is waited for, and the seam a -// test shortens it through. +// A variable so that tests can shorten the wait. var regoPolicyWaitOptions = regopolicies.WaitOptions{} var regoPolicyInputSchemas = []string{"trail", "trails"} @@ -98,7 +97,7 @@ func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { Long: createRegoPolicyLongDesc, Example: createRegoPolicyExample, Args: cobra.ExactArgs(2), - // Hidden while the server endpoint is behind a feature flag. + // Hidden while the server gates the endpoint behind is-server-side-evaluation-enabled. Hidden: true, Annotations: map[string]string{docgen.DocHiddenAnnotation: "", betaCLIAnnotation: ""}, PreRunE: func(cmd *cobra.Command, args []string) error { @@ -129,8 +128,7 @@ func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { } func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) error { - // Refused before publishing: a format refused where it is printed would - // leave a version published by a command that reported an 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) @@ -138,8 +136,7 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er if !slices.Contains(regoPolicyInputSchemas, o.inputSchema) { return fmt.Errorf("--input-schema must be one of %v, got %q", regoPolicyInputSchemas, o.inputSchema) } - // A published version is cited by decisions, so its source must be what - // the author has on disk, not whatever a URL serves at the moment. + // 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") } @@ -162,12 +159,11 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er return err } if published == nil { - // A dry run sent nothing, so there is no version to wait for. + // Nil on a dry run. return nil } - // A publish of content the newest version already holds answers with that - // version, which can already be terminal. + // 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, @@ -198,9 +194,7 @@ var regoPolicyPrinters = map[string]func(io.Writer, *regopolicies.Version) error }, } -// notValidError reports a version that will never be evaluated. A rejected -// version is the policy's fault and a failed one is Kosli's, so the two are -// worded apart: the author of a failed version has nothing to fix. +// 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 { diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go index 93ab81450..72e674a36 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/createRegoPolicy_test.go @@ -36,10 +36,9 @@ const ( `"error":{"kind":"lambda_timeout","message":"validation timed out after 3 attempts"}}` ) -// fakeRegoPolicies records each publish and answers each version read with the -// next body in reads, repeating the last one once they run out. +// fakeRegoPolicies answers version reads with the next of reads, repeating the last. type fakeRegoPolicies struct { - // What the publish answers with; empty means a pending version. + // Empty means the publish answers with a pending version. publishAnswer string published []map[string]any publishURL string @@ -161,8 +160,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestItWaitsThroughPendingReads() require.Contains(suite.T(), combined, "is valid") } -// A directory travels as `evaluate policy` sends it, so the same directory -// gives the same digest on both paths. +// The same directory must give the same digest here as in `evaluate policy`. func (suite *CreateRegoPolicyCommandTestSuite) TestADirectoryTravelsKeyedByRelativePath() { dir := suite.T().TempDir() require.NoError(suite.T(), os.MkdirAll(filepath.Join(dir, "lib"), 0o755)) @@ -208,8 +206,6 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestARejectedVersionWithNoReasonS err.Error()) } -// A failed version is our fault, not the policy's: the author must not go -// looking for a defect in the policy, and publishing again is the way back. func (suite *CreateRegoPolicyCommandTestSuite) TestAFailedVersionIsNotBlamedOnThePolicy() { _, err := suite.publishExpectingError(regoPolicyFailed) @@ -241,8 +237,6 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAWaitThatExpiresSaysHowToRead "read it later with GET /api/v2/rego-policies/test-org/pr-approved/versions/1") } -// refusedBeforeSending runs args against a fake server and returns the error, -// requiring that nothing reached the server. func (suite *CreateRegoPolicyCommandTestSuite) refusedBeforeSending(args string) error { server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) @@ -280,8 +274,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestABundleOverTheFileCapIsRefuse 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, -// so an empty value is refused like on every other flag. +// An unset variable in `--description "$DESC"` must not clear a description. func (suite *CreateRegoPolicyCommandTestSuite) TestAnEmptyDescriptionIsRefused() { err := suite.refusedBeforeSending( `pr-approved testdata/policies/allow-all.rego --input-schema trail --description ""`) @@ -313,8 +306,6 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsTheValidVersion() }, printed, "fields the server left out stay out") } -// Refused before publishing: a format refused where it is printed would leave -// a version published by a command that reported an error. func (suite *CreateRegoPolicyCommandTestSuite) TestAnUnknownOutputFormatIsRefused() { err := suite.refusedBeforeSending( "pr-approved testdata/policies/allow-all.rego --input-schema trail --output yaml") @@ -336,8 +327,6 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestADryRunSendsNothingAndWaitsFo require.NotContains(suite.T(), combined, "is valid") } -// Publishing content the newest valid version already holds stores nothing and -// answers with that version, so there is nothing left to wait for. func (suite *CreateRegoPolicyCommandTestSuite) TestAPublishAnsweredWithAValidVersionIsNotPolled() { server, fake := newFakeRegoPolicies(suite.T(), regoPolicyPending) fake.publishAnswer = regoPolicyValid diff --git a/cmd/kosli/lifecycle_test.go b/cmd/kosli/lifecycle_test.go index 738f533c5..2749bfcd1 100644 --- a/cmd/kosli/lifecycle_test.go +++ b/cmd/kosli/lifecycle_test.go @@ -50,8 +50,7 @@ func TestLifecycleAttestDecisionIsBetaAndVisible(t *testing.T) { } } -// Hidden while the rego-policies API is behind the -// is-server-side-evaluation-enabled flag on the server. +// Hidden while the server gates the endpoint behind is-server-side-evaluation-enabled. func TestLifecycleCreateRegoPolicyIsBetaAndHidden(t *testing.T) { global = &GlobalOpts{} cmd := newCreateRegoPolicyCmd(io.Discard) diff --git a/internal/evaluations/wait.go b/internal/evaluations/wait.go index 1b245d0cc..00d7c2264 100644 --- a/internal/evaluations/wait.go +++ b/internal/evaluations/wait.go @@ -22,8 +22,7 @@ var DefaultWaitOptions = WaitOptions{ Max: 5 * time.Second, } -// WaitOptions bounds how long an evaluation is asked about and how often, with -// the same meaning as poll.Options. A zero field takes its default. +// WaitOptions are poll.Options in which a zero field takes its default. type WaitOptions struct { Timeout time.Duration Initial time.Duration diff --git a/internal/poll/poll.go b/internal/poll/poll.go index 7dd23199f..21622c056 100644 --- a/internal/poll/poll.go +++ b/internal/poll/poll.go @@ -1,5 +1,4 @@ -// Package poll asks the server about something that finishes asynchronously -// until it is done, backing off between reads. +// Package poll repeats a read, backing off, until it reports done. package poll import ( @@ -8,22 +7,15 @@ import ( "time" ) -// Options bounds how long a read is repeated and how often. Every field must be -// positive; callers own their defaults, since each resource has its own budget. -// -// 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. +// 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 } -// ExpiredError says the budget ran out before the read reported done. It -// carries no value, so an unfinished answer is never mistaken for a final one. +// ExpiredError says the budget ran out before the read reported done. type ExpiredError struct { Waited time.Duration } @@ -33,32 +25,24 @@ func (e *ExpiredError) Error() string { } // 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 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. +// 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 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. + // Measured, because a slow read can overrun the budget. 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. + // 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 { - // Transient trouble was already retried inside the HTTP client, so - // what reaches here will not improve by being asked again. + // The HTTP client has already retried transient errors. return zero, err } if done { @@ -69,9 +53,6 @@ func Until[T any](ctx context.Context, options Options, read func() (T, bool, er case <-ctx.Done(): return zero, 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 zero, &ExpiredError{Waited: time.Since(started).Round(time.Millisecond)} case <-time.After(interval): } @@ -79,9 +60,6 @@ func Until[T any](ctx context.Context, options Options, read func() (T, bool, er } } -// nextInterval doubles the wait between reads, never past max. Backing off -// keeps a slow resource 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 diff --git a/internal/poll/poll_test.go b/internal/poll/poll_test.go index 08025bc73..24c5851fd 100644 --- a/internal/poll/poll_test.go +++ b/internal/poll/poll_test.go @@ -9,8 +9,7 @@ import ( "github.com/stretchr/testify/require" ) -// Short enough that a test that must wait the budget out finishes quickly, and -// long enough that a test that must not hit the budget never does. +// testOptions has a Timeout that expires only when a test shortens it. func testOptions() Options { return Options{ Timeout: 2 * time.Second, @@ -19,8 +18,7 @@ func testOptions() Options { } } -// sequence answers each read with the next state in turn, and repeats the -// last one once it runs out. +// 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) { diff --git a/internal/regopolicies/client.go b/internal/regopolicies/client.go index 9edaee1fc..4457836eb 100644 --- a/internal/regopolicies/client.go +++ b/internal/regopolicies/client.go @@ -1,6 +1,5 @@ -// Package regopolicies talks to the Kosli rego policy API: it publishes a -// versioned Rego bundle under an identifier, and reads its versions back. It -// compiles and validates nothing itself; the server does that asynchronously. +// Package regopolicies publishes versioned Rego bundles to Kosli and reads their +// validation back. package regopolicies import ( @@ -23,8 +22,7 @@ const ( StatusFailed = "failed" ) -// PublishRequest describes one publish of a policy. An empty Name or -// Description is left out, so the server keeps the value it has. +// PublishRequest leaves an empty Name or Description out, so the server keeps its value. type PublishRequest struct { Identifier string Name string @@ -59,8 +57,8 @@ 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: the -// set belongs to the evaluator, and an unknown kind must still reach the user. +// 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"` @@ -127,7 +125,7 @@ func (c *Client) GetVersion(org, identifier string, version int) (*Version, erro return nil, err } - // DryRun is deliberately not set: a read writes nothing. + // Not a dry run: a read writes nothing. response, err := c.http.Do(&requests.RequestParams{ Method: http.MethodGet, URL: endpoint, @@ -137,17 +135,14 @@ func (c *Client) GetVersion(org, identifier string, version int) (*Version, erro return nil, err } if response == nil { - // The shared client answers a suppressed request with nothing at all, - // so a later change that suppresses this read meets this, not a panic. + // 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 is the escaped API path of one policy. The identifier is a single -// segment, so a slash in it is escaped rather than read as a separator, and the -// names a URL path resolves on its own are refused: escaping cannot keep "." -// or ".." in their segment. +// 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 "", ".", "..": @@ -156,8 +151,7 @@ func policyRoute(org, identifier string) (string, error) { return "/api/v2/rego-policies/" + url.PathEscape(org) + "/" + url.PathEscape(identifier), nil } -// The payload type is separate from PublishRequest because the server forbids -// unknown fields: the wire shape has to be stated exactly here. +// 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"` diff --git a/internal/regopolicies/client_test.go b/internal/regopolicies/client_test.go index 5017bd315..637b149b9 100644 --- a/internal/regopolicies/client_test.go +++ b/internal/regopolicies/client_test.go @@ -14,8 +14,6 @@ import ( "github.com/stretchr/testify/require" ) -// received is what the fake server saw, so a test can assert on the request -// rather than only on what came back. type received struct { hits int method string @@ -179,8 +177,6 @@ func TestPublishSendsNothingOnADryRun(t *testing.T) { require.Equal(t, 0, seen.hits) } -// The identifier is one path segment: a slash in it must not reach another -// route, least of all one outside the org. func TestPublishKeepsTheIdentifierInOneSegment(t *testing.T) { for identifier, escaped := range map[string]string{ "a/b": "/api/v2/rego-policies/my-org/a%2Fb", @@ -199,7 +195,6 @@ func TestPublishKeepsTheIdentifierInOneSegment(t *testing.T) { } } -// Escaping cannot keep these in their segment: a URL path resolves them. func TestPublishRefusesAnIdentifierThatIsNotASegment(t *testing.T) { for _, identifier := range []string{"", ".", ".."} { t.Run(identifier, func(t *testing.T) { @@ -216,8 +211,6 @@ func TestPublishRefusesAnIdentifierThatIsNotASegment(t *testing.T) { } } -// A 409 from this endpoint is a refusal, not a lock to wait out: the server -// re-runs a lost race itself. Retrying would delay the answer and lose its words. 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+`"}`) diff --git a/internal/regopolicies/version_test.go b/internal/regopolicies/version_test.go index 88a677c49..9e722e6f3 100644 --- a/internal/regopolicies/version_test.go +++ b/internal/regopolicies/version_test.go @@ -79,8 +79,7 @@ func TestGetVersionSurfacesAMissingVersion(t *testing.T) { require.Equal(t, message, apiError.Message) } -// An unknown status means the server has added one; reading it as terminal -// would report an unfinished validation as an outcome. +// 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, diff --git a/internal/regopolicies/wait.go b/internal/regopolicies/wait.go index 09d513b76..531e39252 100644 --- a/internal/regopolicies/wait.go +++ b/internal/regopolicies/wait.go @@ -9,17 +9,15 @@ import ( "github.com/kosli-dev/cli/internal/poll" ) -// DefaultWaitOptions gives validation the same thirty seconds the platform -// states for an evaluation. A bundle at the size cap takes about ten seconds to -// compile, so this is ample without hiding a stuck version. +// 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 bounds how long a version is asked about and how often, with the -// same meaning as poll.Options. A zero field takes its default. +// WaitOptions are poll.Options in which a zero field takes its default. type WaitOptions struct { Timeout time.Duration Initial time.Duration @@ -39,8 +37,7 @@ func (o WaitOptions) withDefaults() WaitOptions { return o } -// StillPendingError says the wait expired before the version was validated. It -// carries no version, so an unfinished validation is never read as an outcome. +// StillPendingError says the wait expired before the version was validated. type StillPendingError struct { Org string Identifier string diff --git a/internal/regopolicies/wait_test.go b/internal/regopolicies/wait_test.go index 8893b3f23..4721a8627 100644 --- a/internal/regopolicies/wait_test.go +++ b/internal/regopolicies/wait_test.go @@ -17,8 +17,7 @@ type fakeResponse struct { body string } -// newSequencedServer answers each request with the next response in turn, and -// repeats the last one once the sequence runs out. +// newSequencedServer repeats the last response once the sequence runs out. func newSequencedServer(t *testing.T, responses ...fakeResponse) (*httptest.Server, *received) { t.Helper() seen := &received{} @@ -36,8 +35,7 @@ func newSequencedServer(t *testing.T, responses ...fakeResponse) (*httptest.Serv return server, seen } -// Short enough that a test that must wait the budget out finishes quickly, and -// long enough that a test that must not hit the budget never does. +// testWaitOptions has a Timeout that expires only when a test shortens it. func testWaitOptions() WaitOptions { return WaitOptions{ Timeout: 2 * time.Second, @@ -66,8 +64,6 @@ func TestWaitReturnsTheVersionOnceItIsValidated(t *testing.T) { require.Equal(t, "/api/v2/rego-policies/my-org/pr-approved/versions/3", seen.path) } -// A rejected policy is an answer, not a failure to get one: the caller decides -// what it means. func TestWaitReturnsARejectedVersionAsAnOutcome(t *testing.T) { server, _ := newSequencedServer(t, fakeResponse{http.StatusOK, pendingBody}, @@ -140,8 +136,6 @@ func TestWaitOptionsFallBackToTheDefaultBudget(t *testing.T) { require.Equal(t, 30*time.Second, filled.Timeout) } -// The route in the message is one a reader can paste, so it is escaped the way -// the request was. func TestStillPendingNamesTheEscapedRoute(t *testing.T) { err := &StillPendingError{Org: "my-org", Identifier: "a/b", Version: 2, Waited: time.Second} diff --git a/internal/requests/requests.go b/internal/requests/requests.go index e9c1d62a5..06d183b3a 100644 --- a/internal/requests/requests.go +++ b/internal/requests/requests.go @@ -407,9 +407,8 @@ func (c *Client) PayloadOutput(req *http.Request, jsonFields map[string]any, mes return nil } -// isSuccess accepts only the statuses whose body is the whole answer, which is -// how every caller reads it. A 204 has no body and a 206 or 207 only part of -// one, so treating those as success would hand a caller an answer it is not. +// 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: From 9cafca374eed5097726de15910c50e24c62058aa Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 11:40:29 +0200 Subject: [PATCH 14/26] fix(create): print the version as JSON for every result, not only a valid one With --output json, a rejected or failed version left stdout empty, so a script could not read error.kind or error.message. The exit code is still 1. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 8 ++++++++ cmd/kosli/createRegoPolicy_test.go | 33 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 981c8aa24..701c05b72 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -39,6 +39,8 @@ seconds for the result: - 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 printed for each of the three results, +with the reason for a rejected or failed one. An evaluation runs the highest-numbered valid version, so a rejected or failed version leaves the policy in service unchanged.` @@ -173,6 +175,12 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er } } if version.Status != regopolicies.StatusValid { + // JSON carries the reason for a script to read; the table has only the error. + if o.output == "json" { + if err := printVersion(out, version); err != nil { + return err + } + } return notValidError(identifier, version) } diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go index 72e674a36..0ba185a1b 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/createRegoPolicy_test.go @@ -282,6 +282,39 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAnEmptyDescriptionIsRefused() require.Contains(suite.T(), err.Error(), "flag '--description' was given an empty value") } +func (suite *CreateRegoPolicyCommandTestSuite) 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.createCmd(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 *CreateRegoPolicyCommandTestSuite) TestTableOutputPrintsNothingForAVersionThatIsNotValid() { + server, _ := newFakeRegoPolicies(suite.T(), regoPolicyRejected) + + _, _, stdout, _, err := executeCommandC(suite.createCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail")) + + require.Error(suite.T(), err) + require.Empty(suite.T(), stdout) +} + func (suite *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsTheValidVersion() { server, _ := newFakeRegoPolicies(suite.T(), regoPolicyValid) From 08ee1f441c95b464815c38f5fa1a291764eb5e77 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 11:40:44 +0200 Subject: [PATCH 15/26] fix(regopolicies): give the full URL, with the host, in the still-pending message A route without a host left a user on a non-default --host to fill it in. The message and GetVersion now build the URL in one place. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy_test.go | 5 +++-- internal/regopolicies/client.go | 14 +++++++++----- internal/regopolicies/wait.go | 15 ++++++++------- internal/regopolicies/wait_test.go | 7 ++++--- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go index 0ba185a1b..b683959f8 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/createRegoPolicy_test.go @@ -233,8 +233,9 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAWaitThatExpiresSaysHowToRead _, err := suite.publishExpectingError(regoPolicyPending) require.Contains(suite.T(), err.Error(), "version 1 of rego policy pr-approved is still pending after") - require.Contains(suite.T(), err.Error(), - "read it later with GET /api/v2/rego-policies/test-org/pr-approved/versions/1") + 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 *CreateRegoPolicyCommandTestSuite) refusedBeforeSending(args string) error { diff --git a/internal/regopolicies/client.go b/internal/regopolicies/client.go index 4457836eb..ad9103efa 100644 --- a/internal/regopolicies/client.go +++ b/internal/regopolicies/client.go @@ -116,11 +116,7 @@ func (c *Client) Publish(org string, request PublishRequest) (*Version, error) { // 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) { - route, err := policyRoute(org, identifier) - if err != nil { - return nil, err - } - endpoint, err := url.JoinPath(c.host, route, "versions", strconv.Itoa(version)) + endpoint, err := versionURL(c.host, org, identifier, version) if err != nil { return nil, err } @@ -151,6 +147,14 @@ func policyRoute(org, identifier string) (string, error) { 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"` diff --git a/internal/regopolicies/wait.go b/internal/regopolicies/wait.go index 531e39252..b61dd0560 100644 --- a/internal/regopolicies/wait.go +++ b/internal/regopolicies/wait.go @@ -39,6 +39,7 @@ func (o WaitOptions) withDefaults() WaitOptions { // StillPendingError says the wait expired before the version was validated. type StillPendingError struct { + Host string Org string Identifier string Version int @@ -46,14 +47,14 @@ type StillPendingError struct { } func (e *StillPendingError) Error() string { - route, err := policyRoute(e.Org, e.Identifier) + pending := fmt.Sprintf("version %d of rego policy %s is still pending after %s", + e.Version, e.Identifier, e.Waited) + endpoint, err := versionURL(e.Host, e.Org, e.Identifier, e.Version) if err != nil { - // Unreachable: the wait reads the version through the same route first. - return fmt.Sprintf("version %d of rego policy %s is still pending after %s", - e.Version, e.Identifier, e.Waited) + // Unreachable: the wait reads the version from the same URL first. + return pending } - return fmt.Sprintf("version %d of rego policy %s is still pending after %s; read it later with GET %s/versions/%d", - e.Version, e.Identifier, e.Waited, route, e.Version) + return pending + "; read it later with GET " + endpoint } // WaitForTerminal polls until the version is validated, the caller gives up, or @@ -71,7 +72,7 @@ func (c *Client) WaitForTerminal(ctx context.Context, org, identifier string, ve var expired *poll.ExpiredError if errors.As(err, &expired) { return nil, &StillPendingError{ - Org: org, Identifier: identifier, Version: version, Waited: expired.Waited} + Host: c.host, Org: org, Identifier: identifier, Version: version, Waited: expired.Waited} } return read, err } diff --git a/internal/regopolicies/wait_test.go b/internal/regopolicies/wait_test.go index 4721a8627..2e7c3d52e 100644 --- a/internal/regopolicies/wait_test.go +++ b/internal/regopolicies/wait_test.go @@ -109,7 +109,7 @@ func TestWaitGivesUpNamingTheVersionAndHowToReadItLater(t *testing.T) { 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 /api/v2/rego-policies/my-org/pr-approved/versions/1", + "; read it later with GET "+server.URL+"/api/v2/rego-policies/my-org/pr-approved/versions/1", err.Error()) } @@ -137,10 +137,11 @@ func TestWaitOptionsFallBackToTheDefaultBudget(t *testing.T) { } func TestStillPendingNamesTheEscapedRoute(t *testing.T) { - err := &StillPendingError{Org: "my-org", Identifier: "a/b", Version: 2, Waited: time.Second} + err := &StillPendingError{ + Host: "https://app.kosli.com", Org: "my-org", Identifier: "a/b", Version: 2, Waited: time.Second} require.Equal(t, "version 2 of rego policy a/b is still pending after 1s; "+ - "read it later with GET /api/v2/rego-policies/my-org/a%2Fb/versions/2", + "read it later with GET https://app.kosli.com/api/v2/rego-policies/my-org/a%2Fb/versions/2", err.Error()) } From 732f7d786fb27e0401ca4d604ef94e8ec21911b0 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 11:44:56 +0200 Subject: [PATCH 16/26] refactor(poll): one WaitOptions type, with defaults filled by Options.Or Co-Authored-By: Claude Opus 5.5 --- internal/evaluations/wait.go | 23 +++-------------------- internal/evaluations/wait_test.go | 7 ++----- internal/poll/poll.go | 14 ++++++++++++++ internal/poll/poll_test.go | 12 ++++++++++++ internal/regopolicies/wait.go | 23 +++-------------------- internal/regopolicies/wait_test.go | 7 ++----- 6 files changed, 36 insertions(+), 50 deletions(-) diff --git a/internal/evaluations/wait.go b/internal/evaluations/wait.go index 00d7c2264..bb1978c22 100644 --- a/internal/evaluations/wait.go +++ b/internal/evaluations/wait.go @@ -22,25 +22,8 @@ var DefaultWaitOptions = WaitOptions{ Max: 5 * time.Second, } -// WaitOptions are poll.Options in which a zero field takes its default. -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 @@ -88,7 +71,7 @@ func (c *Client) Get(org, id string) (*Evaluation, error) { // 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. func (c *Client) WaitForTerminal(ctx context.Context, org, id string, options WaitOptions) (*Evaluation, error) { - evaluation, err := poll.Until(ctx, poll.Options(options.withDefaults()), + evaluation, err := poll.Until(ctx, options.Or(DefaultWaitOptions), func() (*Evaluation, bool, error) { evaluation, err := c.Get(org, id) if err != nil { diff --git a/internal/evaluations/wait_test.go b/internal/evaluations/wait_test.go index d9936e564..7843d52a0 100644 --- a/internal/evaluations/wait_test.go +++ b/internal/evaluations/wait_test.go @@ -232,9 +232,6 @@ func TestWaitStopsOnAReadItCannotRetry(t *testing.T) { require.Equal(t, 2, seen.hits, "a failed read ends the wait rather than being polled through") } -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 index 21622c056..0e83b8be8 100644 --- a/internal/poll/poll.go +++ b/internal/poll/poll.go @@ -15,6 +15,20 @@ type Options struct { 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 diff --git a/internal/poll/poll_test.go b/internal/poll/poll_test.go index 24c5851fd..2079ead1f 100644 --- a/internal/poll/poll_test.go +++ b/internal/poll/poll_test.go @@ -100,3 +100,15 @@ func TestNextIntervalDoublesUpToTheCeiling(t *testing.T) { "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/wait.go b/internal/regopolicies/wait.go index b61dd0560..561b4f4eb 100644 --- a/internal/regopolicies/wait.go +++ b/internal/regopolicies/wait.go @@ -17,25 +17,8 @@ var DefaultWaitOptions = WaitOptions{ Max: 5 * time.Second, } -// WaitOptions are poll.Options in which a zero field takes its default. -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 before the version was validated. type StillPendingError struct { @@ -60,7 +43,7 @@ func (e *StillPendingError) Error() string { // 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) { - read, err := poll.Until(ctx, poll.Options(options.withDefaults()), + read, err := poll.Until(ctx, options.Or(DefaultWaitOptions), func() (*Version, bool, error) { read, err := c.GetVersion(org, identifier, version) if err != nil { diff --git a/internal/regopolicies/wait_test.go b/internal/regopolicies/wait_test.go index 2e7c3d52e..902748ac9 100644 --- a/internal/regopolicies/wait_test.go +++ b/internal/regopolicies/wait_test.go @@ -129,11 +129,8 @@ func TestWaitStopsOnAReadItCannotRetry(t *testing.T) { require.Equal(t, 2, seen.hits) } -func TestWaitOptionsFallBackToTheDefaultBudget(t *testing.T) { - filled := WaitOptions{}.withDefaults() - - require.Equal(t, DefaultWaitOptions, filled) - require.Equal(t, 30*time.Second, filled.Timeout) +func TestTheDefaultBudgetIsThirtySeconds(t *testing.T) { + require.Equal(t, 30*time.Second, DefaultWaitOptions.Timeout) } func TestStillPendingNamesTheEscapedRoute(t *testing.T) { From 96beeb946bc23a11d149cd22d3dae36453d71d48 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 11:46:51 +0200 Subject: [PATCH 17/26] fix(evaluations): give the full URL, with the host, in the still-pending message The same message format as create rego-policy, so a user on a non-default --host can use the URL as it is. Co-Authored-By: Claude Opus 5.5 --- internal/evaluations/wait.go | 19 ++++++++++++++----- internal/evaluations/wait_test.go | 1 + 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/internal/evaluations/wait.go b/internal/evaluations/wait.go index bb1978c22..7b37c02a2 100644 --- a/internal/evaluations/wait.go +++ b/internal/evaluations/wait.go @@ -30,20 +30,25 @@ type WaitOptions = poll.Options // 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 { + Host string Org string ID 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) + pending := fmt.Sprintf("evaluation %s is still pending after %s", e.ID, e.Waited) + endpoint, err := evaluationURL(e.Host, e.Org, e.ID) + if err != nil { + // Unreachable: the wait reads the evaluation from the same URL first. + return pending + } + return pending + "; read it later with GET " + endpoint } // 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 } @@ -82,7 +87,11 @@ func (c *Client) WaitForTerminal(ctx context.Context, org, id string, options Wa var expired *poll.ExpiredError if errors.As(err, &expired) { - return nil, &StillPendingError{Org: org, ID: id, Waited: expired.Waited} + return nil, &StillPendingError{Host: c.host, Org: org, ID: id, Waited: expired.Waited} } return evaluation, err } + +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 7843d52a0..559e01ba3 100644 --- a/internal/evaluations/wait_test.go +++ b/internal/evaluations/wait_test.go @@ -199,6 +199,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") } From 2c1d383adca259fa3315e699430c078d3fb6ad28 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 11:58:56 +0200 Subject: [PATCH 18/26] fix(create): print the pending version as JSON when the wait expires A script reading stdout had no version number to read the version later. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 27 ++++++++++++++++++++------- cmd/kosli/createRegoPolicy_test.go | 15 +++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 701c05b72..0e3030fa9 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "io" "slices" @@ -39,8 +40,8 @@ seconds for the result: - 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 printed for each of the three results, -with the reason for a rejected or failed one. +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.` @@ -170,16 +171,19 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er if !version.IsTerminal() { version, err = client.WaitForTerminal(context.Background(), global.Org, identifier, published.Version, regoPolicyWaitOptions) + var stillPending *regopolicies.StillPendingError + if errors.As(err, &stillPending) { + if printErr := o.printIfJSON(out, published); printErr != nil { + return printErr + } + } if err != nil { return err } } if version.Status != regopolicies.StatusValid { - // JSON carries the reason for a script to read; the table has only the error. - if o.output == "json" { - if err := printVersion(out, version); err != nil { - return err - } + if err := o.printIfJSON(out, version); err != nil { + return err } return notValidError(identifier, version) } @@ -187,6 +191,15 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er return printVersion(out, version) } +// 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 *createRegoPolicyOptions) printIfJSON(out io.Writer, version *regopolicies.Version) error { + if o.output != "json" { + return nil + } + return regoPolicyPrinters["json"](out, version) +} + var regoPolicyPrinters = map[string]func(io.Writer, *regopolicies.Version) error{ "table": func(out io.Writer, version *regopolicies.Version) error { _, err := fmt.Fprintf(out, "rego policy '%s' version %d is valid (%s)\n", diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go index b683959f8..36ea59d10 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/createRegoPolicy_test.go @@ -227,6 +227,21 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAFailedVersionWithNoReasonIsS err.Error()) } +func (suite *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsThePendingVersionWhenTheWaitExpires() { + regoPolicyWaitOptions.Timeout = 20 * time.Millisecond + server, _ := newFakeRegoPolicies(suite.T(), regoPolicyPending) + + _, _, stdout, _, err := executeCommandC(suite.createCmd(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 *CreateRegoPolicyCommandTestSuite) TestAWaitThatExpiresSaysHowToReadTheVersionLater() { regoPolicyWaitOptions.Timeout = 20 * time.Millisecond From 9749077f4341c2094f06076cb210f73a82481676 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 12:01:16 +0200 Subject: [PATCH 19/26] fix(create): say when a publish stored nothing new Publishing the files and input schema of the newest valid version answers with that version. The table output now says that nothing new was stored, and that --comment was not kept when one was given. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 25 +++++++++++++++++++------ cmd/kosli/createRegoPolicy_test.go | 29 +++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 0e3030fa9..62c125c06 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -188,7 +188,15 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er return notValidError(identifier, version) } - return printVersion(out, version) + note := "" + if published.IsTerminal() { + // A new version is always stored pending, so a terminal answer is an existing one. + note = "nothing new was stored, as the files and input schema are the same as this version" + if o.comment != "" { + note += ", so --comment was not kept" + } + } + return printVersion(out, version, note) } // printIfJSON gives a script the version even when the command fails, as JSON @@ -197,16 +205,21 @@ func (o *createRegoPolicyOptions) printIfJSON(out io.Writer, version *regopolici if o.output != "json" { return nil } - return regoPolicyPrinters["json"](out, version) + return regoPolicyPrinters["json"](out, version, "") } -var regoPolicyPrinters = map[string]func(io.Writer, *regopolicies.Version) error{ - "table": func(out io.Writer, version *regopolicies.Version) error { - _, err := fmt.Fprintf(out, "rego policy '%s' version %d is valid (%s)\n", +// The JSON printer ignores note: its output is the server's answer as sent. +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) error { + "json": func(out io.Writer, version *regopolicies.Version, _ string) error { raw, err := json.Marshal(version) if err != nil { return err diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/createRegoPolicy_test.go index 36ea59d10..1cb5a7b6c 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/createRegoPolicy_test.go @@ -157,7 +157,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestItWaitsThroughPendingReads() require.NoError(suite.T(), err) require.Equal(suite.T(), 3, fake.readCount) - require.Contains(suite.T(), combined, "is valid") + 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`. @@ -377,15 +377,28 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestADryRunSendsNothingAndWaitsFo } func (suite *CreateRegoPolicyCommandTestSuite) TestAPublishAnsweredWithAValidVersionIsNotPolled() { - server, fake := newFakeRegoPolicies(suite.T(), regoPolicyPending) - fake.publishAnswer = regoPolicyValid + for _, test := range []struct { + name, flags, want string + }{ + {"without a comment", "", + "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\n"}, + {"with a comment", ` --comment "Second try"`, + "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, so --comment was not kept\n"}, + } { + suite.Run(test.name, func() { + server, fake := newFakeRegoPolicies(suite.T(), regoPolicyPending) + fake.publishAnswer = regoPolicyValid - _, combined, _, _, err := executeCommandC(suite.createCmd(server.URL, - "pr-approved testdata/policies/allow-all.rego --input-schema trail")) + _, combined, _, _, err := executeCommandC(suite.createCmd(server.URL, + "pr-approved testdata/policies/allow-all.rego --input-schema trail"+test.flags)) - require.NoError(suite.T(), err) - require.Equal(suite.T(), 0, fake.readCount) - require.Contains(suite.T(), combined, "rego policy 'pr-approved' version 1 is valid (sha256:abc123)") + require.NoError(suite.T(), err) + require.Equal(suite.T(), 0, fake.readCount) + require.Equal(suite.T(), test.want, combined) + }) + } } func (suite *CreateRegoPolicyCommandTestSuite) TestItNeedsAnIdentifierAndAPath() { From c25a4ccaccf56039d8690ebfab32935bc684b89c Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 12:03:53 +0200 Subject: [PATCH 20/26] refactor: build the still-pending URL once, when the error is made Both StillPendingError types rebuilt the URL in Error() and carried a fallback branch that could not run. They now hold the URL itself. Co-Authored-By: Claude Opus 5.5 --- internal/evaluations/wait.go | 18 ++++++++---------- internal/evaluations/wait_test.go | 1 + internal/regopolicies/wait.go | 19 ++++++++----------- internal/regopolicies/wait_test.go | 18 ++++++++++-------- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/internal/evaluations/wait.go b/internal/evaluations/wait.go index 7b37c02a2..3ce542e84 100644 --- a/internal/evaluations/wait.go +++ b/internal/evaluations/wait.go @@ -30,20 +30,14 @@ type WaitOptions = poll.Options // 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 { - Host string - Org string ID string + URL string Waited time.Duration } func (e *StillPendingError) Error() string { - pending := fmt.Sprintf("evaluation %s is still pending after %s", e.ID, e.Waited) - endpoint, err := evaluationURL(e.Host, e.Org, e.ID) - if err != nil { - // Unreachable: the wait reads the evaluation from the same URL first. - return pending - } - return pending + "; read it later with GET " + endpoint + 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. @@ -76,6 +70,10 @@ func (c *Client) Get(org, id string) (*Evaluation, error) { // 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. func (c *Client) WaitForTerminal(ctx context.Context, org, id string, options WaitOptions) (*Evaluation, error) { + 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) @@ -87,7 +85,7 @@ func (c *Client) WaitForTerminal(ctx context.Context, org, id string, options Wa var expired *poll.ExpiredError if errors.As(err, &expired) { - return nil, &StillPendingError{Host: c.host, Org: org, ID: id, Waited: expired.Waited} + return nil, &StillPendingError{ID: id, URL: endpoint, Waited: expired.Waited} } return evaluation, err } diff --git a/internal/evaluations/wait_test.go b/internal/evaluations/wait_test.go index 559e01ba3..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". diff --git a/internal/regopolicies/wait.go b/internal/regopolicies/wait.go index 561b4f4eb..cb46ecbba 100644 --- a/internal/regopolicies/wait.go +++ b/internal/regopolicies/wait.go @@ -22,27 +22,24 @@ type WaitOptions = poll.Options // StillPendingError says the wait expired before the version was validated. type StillPendingError struct { - Host string - Org string Identifier string Version int + URL string Waited time.Duration } func (e *StillPendingError) Error() string { - pending := fmt.Sprintf("version %d of rego policy %s is still pending after %s", - e.Version, e.Identifier, e.Waited) - endpoint, err := versionURL(e.Host, e.Org, e.Identifier, e.Version) - if err != nil { - // Unreachable: the wait reads the version from the same URL first. - return pending - } - return pending + "; read it later with GET " + endpoint + 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) @@ -55,7 +52,7 @@ func (c *Client) WaitForTerminal(ctx context.Context, org, identifier string, ve var expired *poll.ExpiredError if errors.As(err, &expired) { return nil, &StillPendingError{ - Host: c.host, Org: org, Identifier: identifier, Version: version, Waited: expired.Waited} + 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 index 902748ac9..2a579be43 100644 --- a/internal/regopolicies/wait_test.go +++ b/internal/regopolicies/wait_test.go @@ -103,7 +103,7 @@ func TestWaitGivesUpNamingTheVersionAndHowToReadItLater(t *testing.T) { 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, "my-org", stillPending.Org) + 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) @@ -133,12 +133,14 @@ func TestTheDefaultBudgetIsThirtySeconds(t *testing.T) { require.Equal(t, 30*time.Second, DefaultWaitOptions.Timeout) } -func TestStillPendingNamesTheEscapedRoute(t *testing.T) { - err := &StillPendingError{ - Host: "https://app.kosli.com", Org: "my-org", Identifier: "a/b", Version: 2, Waited: time.Second} +func TestStillPendingNamesTheEscapedURL(t *testing.T) { + server, _ := newSequencedServer(t, fakeResponse{http.StatusOK, pendingBody}) + options := testWaitOptions() + options.Timeout = 20 * time.Millisecond - require.Equal(t, - "version 2 of rego policy a/b is still pending after 1s; "+ - "read it later with GET https://app.kosli.com/api/v2/rego-policies/my-org/a%2Fb/versions/2", - err.Error()) + _, 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") } From 7854b7ab87dfbfd03d3a847872b15c362a183016 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 12:19:18 +0200 Subject: [PATCH 21/26] fix(create): generate no docs page for the hidden rego-policy command The docHidden annotation still generates a page, with hidden: true; like evaluate policy, this command gets none until it is unhidden. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/createRegoPolicy.go | 3 +-- cmd/kosli/lifecycle_test.go | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/cmd/kosli/createRegoPolicy.go b/cmd/kosli/createRegoPolicy.go index 62c125c06..24458683e 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/createRegoPolicy.go @@ -8,7 +8,6 @@ import ( "io" "slices" - "github.com/kosli-dev/cli/internal/docgen" "github.com/kosli-dev/cli/internal/output" "github.com/kosli-dev/cli/internal/regopolicies" "github.com/spf13/cobra" @@ -102,7 +101,7 @@ func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { Args: cobra.ExactArgs(2), // Hidden while the server gates the endpoint behind is-server-side-evaluation-enabled. Hidden: true, - Annotations: map[string]string{docgen.DocHiddenAnnotation: "", betaCLIAnnotation: ""}, + Annotations: map[string]string{betaCLIAnnotation: ""}, PreRunE: func(cmd *cobra.Command, args []string) error { err := RequireGlobalFlags(global, []string{"Org", "ApiToken"}) if err != nil { diff --git a/cmd/kosli/lifecycle_test.go b/cmd/kosli/lifecycle_test.go index 2749bfcd1..86996de96 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" @@ -60,8 +61,17 @@ func TestLifecycleCreateRegoPolicyIsBetaAndHidden(t *testing.T) { if !isBeta(cmd) { t.Error("expected create rego-policy to be beta") } - if !isDocHidden(cmd) { - t.Error("expected create rego-policy to be left out of the docs") + + 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) } } From f273e21752b91f82e428557b891cc1960ab60354 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 12:22:33 +0200 Subject: [PATCH 22/26] refactor: rename create rego-policy to publish rego-policy Each call publishes a version, the first one included, which is what the server API and the help already call it. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/create.go | 1 - cmd/kosli/lifecycle_test.go | 8 +- cmd/kosli/publish.go | 22 ++++ ...eateRegoPolicy.go => publishRegoPolicy.go} | 30 ++--- ...licy_test.go => publishRegoPolicy_test.go} | 110 ++++++++++-------- cmd/kosli/root.go | 1 + .../testdata/empty-flag-audit-coverage.json | 16 +-- hack/empty-flag-audit/spec.json | 2 +- 8 files changed, 112 insertions(+), 78 deletions(-) create mode 100644 cmd/kosli/publish.go rename cmd/kosli/{createRegoPolicy.go => publishRegoPolicy.go} (89%) rename cmd/kosli/{createRegoPolicy_test.go => publishRegoPolicy_test.go} (74%) diff --git a/cmd/kosli/create.go b/cmd/kosli/create.go index 5ff32d440..5db4fa714 100644 --- a/cmd/kosli/create.go +++ b/cmd/kosli/create.go @@ -21,7 +21,6 @@ func newCreateCmd(out io.Writer) *cobra.Command { newCreateEnvironmentCmd(out), newCreateFlowCmd(out), newCreatePolicyCmd(out), - newCreateRegoPolicyCmd(out), newCreateAttestationTypeCmd(out), newCreateControlCmd(out), newCreateApiKeyCmd(out), diff --git a/cmd/kosli/lifecycle_test.go b/cmd/kosli/lifecycle_test.go index 86996de96..7f938fa9e 100644 --- a/cmd/kosli/lifecycle_test.go +++ b/cmd/kosli/lifecycle_test.go @@ -52,14 +52,14 @@ func TestLifecycleAttestDecisionIsBetaAndVisible(t *testing.T) { } // Hidden while the server gates the endpoint behind is-server-side-evaluation-enabled. -func TestLifecycleCreateRegoPolicyIsBetaAndHidden(t *testing.T) { +func TestLifecyclePublishRegoPolicyIsBetaAndHidden(t *testing.T) { global = &GlobalOpts{} - cmd := newCreateRegoPolicyCmd(io.Discard) + cmd := newPublishRegoPolicyCmd(io.Discard) if !cmd.Hidden { - t.Error("expected create rego-policy to be Hidden") + t.Error("expected publish rego-policy to be Hidden") } if !isBeta(cmd) { - t.Error("expected create rego-policy to be beta") + t.Error("expected publish rego-policy to be beta") } dir := t.TempDir() 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/createRegoPolicy.go b/cmd/kosli/publishRegoPolicy.go similarity index 89% rename from cmd/kosli/createRegoPolicy.go rename to cmd/kosli/publishRegoPolicy.go index 24458683e..8f924c7d2 100644 --- a/cmd/kosli/createRegoPolicy.go +++ b/cmd/kosli/publishRegoPolicy.go @@ -13,9 +13,9 @@ import ( "github.com/spf13/cobra" ) -const createRegoPolicyShortDesc = `Publish a new version of a rego policy in a Kosli org.` +const publishRegoPolicyShortDesc = `Publish a new version of a rego policy in a Kosli org.` -const createRegoPolicyLongDesc = createRegoPolicyShortDesc + ` +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. @@ -45,9 +45,9 @@ 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 createRegoPolicyExample = ` +const publishRegoPolicyExample = ` # publish the first version of a rego policy from a directory: -kosli create rego-policy pr-approved ./policies/pr-approved/ \ +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" \ @@ -56,14 +56,14 @@ kosli create rego-policy pr-approved ./policies/pr-approved/ \ --org yourOrgName # publish a new version of an existing rego policy from one file: -kosli create rego-policy pr-approved ./policies/pr-approved.rego \ +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 create rego-policy release-ready ./policies/release-ready/ \ +kosli publish rego-policy release-ready ./policies/release-ready/ \ --input-schema trails \ --name "Release ready" \ --output json \ @@ -71,7 +71,7 @@ kosli create rego-policy release-ready ./policies/release-ready/ \ --org yourOrgName # show what would be published, without sending it: -kosli create rego-policy pr-approved ./policies/pr-approved/ \ +kosli publish rego-policy pr-approved ./policies/pr-approved/ \ --input-schema trail \ --dry-run \ --api-token yourAPIToken \ @@ -83,7 +83,7 @@ var regoPolicyWaitOptions = regopolicies.WaitOptions{} var regoPolicyInputSchemas = []string{"trail", "trails"} -type createRegoPolicyOptions struct { +type publishRegoPolicyOptions struct { name string description string inputSchema string @@ -91,13 +91,13 @@ type createRegoPolicyOptions struct { output string } -func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { - o := new(createRegoPolicyOptions) +func newPublishRegoPolicyCmd(out io.Writer) *cobra.Command { + o := new(publishRegoPolicyOptions) cmd := &cobra.Command{ Use: "rego-policy IDENTIFIER PATH", - Short: createRegoPolicyShortDesc, - Long: createRegoPolicyLongDesc, - Example: createRegoPolicyExample, + 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, @@ -129,7 +129,7 @@ func newCreateRegoPolicyCmd(out io.Writer) *cobra.Command { return cmd } -func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) error { +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 { @@ -200,7 +200,7 @@ func (o *createRegoPolicyOptions) run(out io.Writer, identifier, path string) er // 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 *createRegoPolicyOptions) printIfJSON(out io.Writer, version *regopolicies.Version) error { +func (o *publishRegoPolicyOptions) printIfJSON(out io.Writer, version *regopolicies.Version) error { if o.output != "json" { return nil } diff --git a/cmd/kosli/createRegoPolicy_test.go b/cmd/kosli/publishRegoPolicy_test.go similarity index 74% rename from cmd/kosli/createRegoPolicy_test.go rename to cmd/kosli/publishRegoPolicy_test.go index 1cb5a7b6c..e901d480f 100644 --- a/cmd/kosli/createRegoPolicy_test.go +++ b/cmd/kosli/publishRegoPolicy_test.go @@ -82,12 +82,12 @@ func newFakeRegoPolicies(t *testing.T, reads ...string) (*httptest.Server, *fake return server, fake } -type CreateRegoPolicyCommandTestSuite struct { +type PublishRegoPolicyCommandTestSuite struct { suite.Suite originalWait regopolicies.WaitOptions } -func (suite *CreateRegoPolicyCommandTestSuite) SetupTest() { +func (suite *PublishRegoPolicyCommandTestSuite) SetupTest() { suite.originalWait = regoPolicyWaitOptions regoPolicyWaitOptions = regopolicies.WaitOptions{ Timeout: 2 * time.Second, @@ -96,27 +96,39 @@ func (suite *CreateRegoPolicyCommandTestSuite) SetupTest() { } } -func (suite *CreateRegoPolicyCommandTestSuite) TearDownTest() { +func (suite *PublishRegoPolicyCommandTestSuite) TearDownTest() { regoPolicyWaitOptions = suite.originalWait } -func (suite *CreateRegoPolicyCommandTestSuite) createCmd(host, args string) string { - return fmt.Sprintf("create rego-policy %s --host %s --org test-org --api-token test-token --max-api-retries 0", +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) } -func (suite *CreateRegoPolicyCommandTestSuite) TestItIsHiddenFromHelp() { - _, combined, _, _, err := executeCommandC("create --help") +// 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, "rego-policy") - require.Contains(suite.T(), combined, "flow", "the rest of the help still renders") + require.NotContains(suite.T(), combined, "publish") + require.Contains(suite.T(), combined, "create", "the rest of the help still renders") } -func (suite *CreateRegoPolicyCommandTestSuite) TestItPublishesTheFileAndReportsTheValidVersion() { +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.createCmd(server.URL, + _, 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"`)) @@ -138,10 +150,10 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestItPublishesTheFileAndReportsT require.Contains(suite.T(), combined, "rego policy 'pr-approved' version 1 is valid (sha256:abc123)") } -func (suite *CreateRegoPolicyCommandTestSuite) TestItLeavesOutTheNameAndDescriptionWhenNotGiven() { +func (suite *PublishRegoPolicyCommandTestSuite) TestItLeavesOutTheNameAndDescriptionWhenNotGiven() { server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) - _, _, _, _, err := executeCommandC(suite.createCmd(server.URL, + _, _, _, _, err := executeCommandC(suite.publishCmd(server.URL, "pr-approved testdata/policies/allow-all.rego --input-schema trail")) require.NoError(suite.T(), err) @@ -149,10 +161,10 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestItLeavesOutTheNameAndDescript require.NotContains(suite.T(), fake.published[0], "description") } -func (suite *CreateRegoPolicyCommandTestSuite) TestItWaitsThroughPendingReads() { +func (suite *PublishRegoPolicyCommandTestSuite) TestItWaitsThroughPendingReads() { server, fake := newFakeRegoPolicies(suite.T(), regoPolicyPending, regoPolicyPending, regoPolicyValid) - _, combined, _, _, err := executeCommandC(suite.createCmd(server.URL, + _, combined, _, _, err := executeCommandC(suite.publishCmd(server.URL, "pr-approved testdata/policies/allow-all.rego --input-schema trail")) require.NoError(suite.T(), err) @@ -161,14 +173,14 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestItWaitsThroughPendingReads() } // The same directory must give the same digest here as in `evaluate policy`. -func (suite *CreateRegoPolicyCommandTestSuite) TestADirectoryTravelsKeyedByRelativePath() { +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.createCmd(server.URL, + _, _, _, _, err := executeCommandC(suite.publishCmd(server.URL, "pr-approved "+dir+" --input-schema trails")) require.NoError(suite.T(), err) @@ -179,16 +191,16 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestADirectoryTravelsKeyedByRelat require.Equal(suite.T(), "trails", fake.published[0]["input_schema"]) } -func (suite *CreateRegoPolicyCommandTestSuite) publishExpectingError(reads ...string) (string, error) { +func (suite *PublishRegoPolicyCommandTestSuite) publishExpectingError(reads ...string) (string, error) { server, _ := newFakeRegoPolicies(suite.T(), reads...) - _, combined, _, _, err := executeCommandC(suite.createCmd(server.URL, + _, 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 *CreateRegoPolicyCommandTestSuite) TestARejectedVersionNamesTheErrorInThePolicy() { +func (suite *PublishRegoPolicyCommandTestSuite) TestARejectedVersionNamesTheErrorInThePolicy() { _, err := suite.publishExpectingError(regoPolicyRejected) require.Equal(suite.T(), @@ -197,7 +209,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestARejectedVersionNamesTheError err.Error()) } -func (suite *CreateRegoPolicyCommandTestSuite) TestARejectedVersionWithNoReasonSaysSo() { +func (suite *PublishRegoPolicyCommandTestSuite) TestARejectedVersionWithNoReasonSaysSo() { _, err := suite.publishExpectingError( strings.Replace(regoPolicyValid, `"status":"valid"`, `"status":"rejected"`, 1)) @@ -206,7 +218,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestARejectedVersionWithNoReasonS err.Error()) } -func (suite *CreateRegoPolicyCommandTestSuite) TestAFailedVersionIsNotBlamedOnThePolicy() { +func (suite *PublishRegoPolicyCommandTestSuite) TestAFailedVersionIsNotBlamedOnThePolicy() { _, err := suite.publishExpectingError(regoPolicyFailed) require.Equal(suite.T(), @@ -217,7 +229,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAFailedVersionIsNotBlamedOnTh require.NotContains(suite.T(), err.Error(), "rejected") } -func (suite *CreateRegoPolicyCommandTestSuite) TestAFailedVersionWithNoReasonIsStillNotBlamedOnThePolicy() { +func (suite *PublishRegoPolicyCommandTestSuite) TestAFailedVersionWithNoReasonIsStillNotBlamedOnThePolicy() { _, err := suite.publishExpectingError( strings.Replace(regoPolicyValid, `"status":"valid"`, `"status":"failed"`, 1)) @@ -227,11 +239,11 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAFailedVersionWithNoReasonIsS err.Error()) } -func (suite *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsThePendingVersionWhenTheWaitExpires() { +func (suite *PublishRegoPolicyCommandTestSuite) TestJsonOutputIsThePendingVersionWhenTheWaitExpires() { regoPolicyWaitOptions.Timeout = 20 * time.Millisecond server, _ := newFakeRegoPolicies(suite.T(), regoPolicyPending) - _, _, stdout, _, err := executeCommandC(suite.createCmd(server.URL, + _, _, stdout, _, err := executeCommandC(suite.publishCmd(server.URL, "pr-approved testdata/policies/allow-all.rego --input-schema trail --output json")) require.Error(suite.T(), err) @@ -242,7 +254,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsThePendingVersion require.Equal(suite.T(), float64(1), printed["version"]) } -func (suite *CreateRegoPolicyCommandTestSuite) TestAWaitThatExpiresSaysHowToReadTheVersionLater() { +func (suite *PublishRegoPolicyCommandTestSuite) TestAWaitThatExpiresSaysHowToReadTheVersionLater() { regoPolicyWaitOptions.Timeout = 20 * time.Millisecond _, err := suite.publishExpectingError(regoPolicyPending) @@ -253,10 +265,10 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAWaitThatExpiresSaysHowToRead err.Error()) } -func (suite *CreateRegoPolicyCommandTestSuite) refusedBeforeSending(args string) error { +func (suite *PublishRegoPolicyCommandTestSuite) refusedBeforeSending(args string) error { server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) - _, _, _, _, err := executeCommandC(suite.createCmd(server.URL, args)) + _, _, _, _, err := executeCommandC(suite.publishCmd(server.URL, args)) require.Error(suite.T(), err) require.Empty(suite.T(), fake.published, "nothing may be published") @@ -264,21 +276,21 @@ func (suite *CreateRegoPolicyCommandTestSuite) refusedBeforeSending(args string) return err } -func (suite *CreateRegoPolicyCommandTestSuite) TestAnUnknownInputSchemaIsRefused() { +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 *CreateRegoPolicyCommandTestSuite) TestAPolicyAtAURLIsRefused() { +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 *CreateRegoPolicyCommandTestSuite) TestABundleOverTheFileCapIsRefused() { +func (suite *PublishRegoPolicyCommandTestSuite) TestABundleOverTheFileCapIsRefused() { dir := suite.T().TempDir() for i := range maxPolicyBundleFiles + 1 { name := filepath.Join(dir, fmt.Sprintf("p%03d.rego", i)) @@ -291,14 +303,14 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestABundleOverTheFileCapIsRefuse } // An unset variable in `--description "$DESC"` must not clear a description. -func (suite *CreateRegoPolicyCommandTestSuite) TestAnEmptyDescriptionIsRefused() { +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 *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsPrintedForAVersionThatIsNotValid() { +func (suite *PublishRegoPolicyCommandTestSuite) TestJsonOutputIsPrintedForAVersionThatIsNotValid() { for _, test := range []struct { name, body, status, kind string }{ @@ -308,7 +320,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsPrintedForAVersio suite.Run(test.name, func() { server, _ := newFakeRegoPolicies(suite.T(), test.body) - _, _, stdout, _, err := executeCommandC(suite.createCmd(server.URL, + _, _, stdout, _, err := executeCommandC(suite.publishCmd(server.URL, "pr-approved testdata/policies/allow-all.rego --input-schema trail --output json")) require.Error(suite.T(), err) @@ -321,20 +333,20 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsPrintedForAVersio } } -func (suite *CreateRegoPolicyCommandTestSuite) TestTableOutputPrintsNothingForAVersionThatIsNotValid() { +func (suite *PublishRegoPolicyCommandTestSuite) TestTableOutputPrintsNothingForAVersionThatIsNotValid() { server, _ := newFakeRegoPolicies(suite.T(), regoPolicyRejected) - _, _, stdout, _, err := executeCommandC(suite.createCmd(server.URL, + _, _, 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 *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsTheValidVersion() { +func (suite *PublishRegoPolicyCommandTestSuite) TestJsonOutputIsTheValidVersion() { server, _ := newFakeRegoPolicies(suite.T(), regoPolicyValid) - _, _, stdout, _, err := executeCommandC(suite.createCmd(server.URL, + _, _, stdout, _, err := executeCommandC(suite.publishCmd(server.URL, "pr-approved testdata/policies/allow-all.rego --input-schema trail --output json")) require.NoError(suite.T(), err) @@ -355,17 +367,17 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestJsonOutputIsTheValidVersion() }, printed, "fields the server left out stay out") } -func (suite *CreateRegoPolicyCommandTestSuite) TestAnUnknownOutputFormatIsRefused() { +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 *CreateRegoPolicyCommandTestSuite) TestADryRunSendsNothingAndWaitsForNothing() { +func (suite *PublishRegoPolicyCommandTestSuite) TestADryRunSendsNothingAndWaitsForNothing() { server, fake := newFakeRegoPolicies(suite.T(), regoPolicyValid) - _, combined, _, _, err := executeCommandC(suite.createCmd(server.URL, + _, combined, _, _, err := executeCommandC(suite.publishCmd(server.URL, "pr-approved testdata/policies/allow-all.rego --input-schema trail --dry-run")) require.NoError(suite.T(), err) @@ -376,7 +388,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestADryRunSendsNothingAndWaitsFo require.NotContains(suite.T(), combined, "is valid") } -func (suite *CreateRegoPolicyCommandTestSuite) TestAPublishAnsweredWithAValidVersionIsNotPolled() { +func (suite *PublishRegoPolicyCommandTestSuite) TestAPublishAnsweredWithAValidVersionIsNotPolled() { for _, test := range []struct { name, flags, want string }{ @@ -391,7 +403,7 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAPublishAnsweredWithAValidVer server, fake := newFakeRegoPolicies(suite.T(), regoPolicyPending) fake.publishAnswer = regoPolicyValid - _, combined, _, _, err := executeCommandC(suite.createCmd(server.URL, + _, combined, _, _, err := executeCommandC(suite.publishCmd(server.URL, "pr-approved testdata/policies/allow-all.rego --input-schema trail"+test.flags)) require.NoError(suite.T(), err) @@ -401,22 +413,22 @@ func (suite *CreateRegoPolicyCommandTestSuite) TestAPublishAnsweredWithAValidVer } } -func (suite *CreateRegoPolicyCommandTestSuite) TestItNeedsAnIdentifierAndAPath() { - _, _, _, _, err := executeCommandC(suite.createCmd("http://localhost:1", +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 *CreateRegoPolicyCommandTestSuite) TestItNeedsAnInputSchema() { - _, _, _, _, err := executeCommandC(suite.createCmd("http://localhost: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 TestCreateRegoPolicyCommandTestSuite(t *testing.T) { - suite.Run(t, new(CreateRegoPolicyCommandTestSuite)) +func TestPublishRegoPolicyCommandTestSuite(t *testing.T) { + suite.Run(t, new(PublishRegoPolicyCommandTestSuite)) } diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index feb0f398a..700d2fe80 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -592,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 a887eba86..839257c27 100644 --- a/cmd/kosli/testdata/empty-flag-audit-coverage.json +++ b/cmd/kosli/testdata/empty-flag-audit-coverage.json @@ -561,14 +561,6 @@ "dry-run": "bool", "type": "string" }, - "create rego-policy": { - "comment": "string", - "description": "string", - "dry-run": "bool", - "input-schema": "string", - "name": "string", - "output": "string" - }, "create service-account": { "description": "string", "dry-run": "bool", @@ -789,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 9e731d4d2..af6358cb4 100644 --- a/hack/empty-flag-audit/spec.json +++ b/hack/empty-flag-audit/spec.json @@ -1521,7 +1521,7 @@ ] ] }, - "create rego-policy": { + "publish rego-policy": { "args": [ "{policy}", "cmd/kosli/testdata/policies/allow-all.rego" From c711ef4959aedf6a99e3de63a1f38d26fe600909 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 12:36:27 +0200 Subject: [PATCH 23/26] fix(publish): name the published version when it cannot be read back The version is already stored when the read fails, so the error now says which one, and --output json still prints it. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/publishRegoPolicy.go | 12 ++++---- cmd/kosli/publishRegoPolicy_test.go | 43 ++++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/cmd/kosli/publishRegoPolicy.go b/cmd/kosli/publishRegoPolicy.go index 8f924c7d2..19dcc1a91 100644 --- a/cmd/kosli/publishRegoPolicy.go +++ b/cmd/kosli/publishRegoPolicy.go @@ -170,14 +170,16 @@ func (o *publishRegoPolicyOptions) run(out io.Writer, identifier, path string) e if !version.IsTerminal() { version, err = client.WaitForTerminal(context.Background(), global.Org, identifier, published.Version, regoPolicyWaitOptions) - var stillPending *regopolicies.StillPendingError - if errors.As(err, &stillPending) { + if err != nil { if printErr := o.printIfJSON(out, published); printErr != nil { return printErr } - } - if err != nil { - return err + 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 { diff --git a/cmd/kosli/publishRegoPolicy_test.go b/cmd/kosli/publishRegoPolicy_test.go index e901d480f..b1a61d73b 100644 --- a/cmd/kosli/publishRegoPolicy_test.go +++ b/cmd/kosli/publishRegoPolicy_test.go @@ -43,9 +43,11 @@ type fakeRegoPolicies struct { published []map[string]any publishURL string reads []string - readCount int - readURL string - unexpected int + // 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) { @@ -69,7 +71,11 @@ func newFakeRegoPolicies(t *testing.T, reads ...string) (*httptest.Server, *fake case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/versions/"): fake.readURL = r.URL.Path - w.WriteHeader(http.StatusOK) + 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++ @@ -239,6 +245,35 @@ func (suite *PublishRegoPolicyCommandTestSuite) TestAFailedVersionWithNoReasonIs err.Error()) } +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) From 67e6240b1141233fd5c1486f606b29b9afb6b310 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 12:37:29 +0200 Subject: [PATCH 24/26] fix(publish): say a pending publish stored nothing when its comment was dropped A new version keeps the comment it was sent with, so an answer with a different comment is an existing version, pending or not. The comment clause now appears only when the comment really was not kept. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/publishRegoPolicy.go | 8 ++++--- cmd/kosli/publishRegoPolicy_test.go | 34 +++++++++++++++++++---------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/cmd/kosli/publishRegoPolicy.go b/cmd/kosli/publishRegoPolicy.go index 19dcc1a91..022ec8385 100644 --- a/cmd/kosli/publishRegoPolicy.go +++ b/cmd/kosli/publishRegoPolicy.go @@ -189,11 +189,13 @@ func (o *publishRegoPolicyOptions) run(out io.Writer, identifier, path string) e 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() { - // A new version is always stored pending, so a terminal answer is an existing one. + if published.IsTerminal() || commentDropped { note = "nothing new was stored, as the files and input schema are the same as this version" - if o.comment != "" { + if commentDropped { note += ", so --comment was not kept" } } diff --git a/cmd/kosli/publishRegoPolicy_test.go b/cmd/kosli/publishRegoPolicy_test.go index b1a61d73b..aa68409d2 100644 --- a/cmd/kosli/publishRegoPolicy_test.go +++ b/cmd/kosli/publishRegoPolicy_test.go @@ -423,26 +423,38 @@ func (suite *PublishRegoPolicyCommandTestSuite) TestADryRunSendsNothingAndWaitsF require.NotContains(suite.T(), combined, "is valid") } -func (suite *PublishRegoPolicyCommandTestSuite) TestAPublishAnsweredWithAValidVersionIsNotPolled() { +// 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, flags, want string + name, answer, flags, want string + reads int }{ - {"without a comment", "", - "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\n"}, - {"with a comment", ` --comment "Second try"`, - "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, so --comment was not kept\n"}, + {"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(), regoPolicyPending) - fake.publishAnswer = regoPolicyValid + 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(), 0, fake.readCount) + require.Equal(suite.T(), test.reads, fake.readCount, "a terminal answer is not polled") require.Equal(suite.T(), test.want, combined) }) } From 14bcab29fd6f84a96cf49e57eaa33b4b18a6144f Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 12:37:54 +0200 Subject: [PATCH 25/26] docs(publish): correct what the JSON printer comment claims The JSON output is built from the Version struct, not the server's body as sent, so it holds only the fields Version declares. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/publishRegoPolicy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kosli/publishRegoPolicy.go b/cmd/kosli/publishRegoPolicy.go index 022ec8385..59161f638 100644 --- a/cmd/kosli/publishRegoPolicy.go +++ b/cmd/kosli/publishRegoPolicy.go @@ -211,7 +211,7 @@ func (o *publishRegoPolicyOptions) printIfJSON(out io.Writer, version *regopolic return regoPolicyPrinters["json"](out, version, "") } -// The JSON printer ignores note: its output is the server's answer as sent. +// 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)", From f3da36f812e4b54e181fd76344691dda5016b2e5 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Mon, 12 Oct 2026 12:38:41 +0200 Subject: [PATCH 26/26] fix(publish): stop when the publish answer names no version A 2xx without a version decoded to 0, and the command read versions/0 and reported whatever came back. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/publishRegoPolicy.go | 4 ++++ cmd/kosli/publishRegoPolicy_test.go | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/cmd/kosli/publishRegoPolicy.go b/cmd/kosli/publishRegoPolicy.go index 59161f638..588ae5ab1 100644 --- a/cmd/kosli/publishRegoPolicy.go +++ b/cmd/kosli/publishRegoPolicy.go @@ -164,6 +164,10 @@ func (o *publishRegoPolicyOptions) run(out io.Writer, identifier, path string) e // 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 diff --git a/cmd/kosli/publishRegoPolicy_test.go b/cmd/kosli/publishRegoPolicy_test.go index aa68409d2..1c444b452 100644 --- a/cmd/kosli/publishRegoPolicy_test.go +++ b/cmd/kosli/publishRegoPolicy_test.go @@ -245,6 +245,18 @@ func (suite *PublishRegoPolicyCommandTestSuite) TestAFailedVersionWithNoReasonIs 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