Skip to content

refactor: decode assert artifact, get artifact and get policy responses into typed structs - #1246

Merged
sami-alajrami merged 6 commits into
mainfrom
refactor/typed-get-responses
Oct 5, 2026
Merged

sami-alajrami merged 6 commits into
mainfrom
refactor/typed-get-responses

Conversation

@dangrondahl

@dangrondahl dangrondahl commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

The table printers for assert artifact, get artifact and get policy now decode the API response into small typed structs. Until now they decoded into map[string]any and read fields through unchecked type assertions. With an assertion, a missing or renamed field panics the CLI. With a struct, it becomes a zero value, or a returned error where the field is genuinely required.

Each struct is declared next to its command's options, like the other types in cmd/kosli. It declares only the fields the printer reads, and timestamps are json.Number. -o json output is untouched: it still prints the raw server body.

Command Change
assert artifact assertArtifactResult with nested policy evaluations, rule evaluations and flows. The failure-message loop is now its own function, policyFailures.
get artifact artifactResponse with running, exited and history. formattedTimestamp now accepts a json.Number, because the artifacts API sends created_at as a numeric string. An empty one counts as missing, and anything else is parsed by the existing string case. trail_name and template_reference_name are *string, so their rows still print whenever the server sends the key, even with an empty value, as on main.
get policy policyResponse. A policy with no versions now returns policy '<name>' has no versions instead of panicking with index out of range.

These commits were written before #1206 and are rebased onto main here. This is groundwork for the typed listing renderer for the list commands, which will follow the same pattern.

Testing

  • 8 new unit tests for the printers. They cover missing fields, the for_control failure text, string timestamps, an empty json.Number, trail rows sent with empty values, and a policy with no versions.
  • The live-server suites run in CI.
  • I built main and this branch and diffed their output against a stub server serving recorded fixtures, in both table and JSON format:
Fixture Result
Full policy, artifact and non-compliant assert responses Identical
get artifact with trail_name and template_reference_name sent as "" Identical
get policy with versions: [] main panics (index out of range). The branch returns the error above.
get artifact without running, exited, history or URLs main panics (nil interface conversion). The branch renders the table with empty URL fields.
assert artifact with only {"compliant": true} main panics. The branch prints COMPLIANT.
-o json for every fixture Identical

Checklist

The table printer walked five levels of map[string]any with unchecked
type assertions, so a response missing any expected field panicked the
CLI. Typed structs make a missing field a zero value instead, document
the response shape in one place, and let the failure-message loop
become a standalone helper.
The table printer had fifteen unchecked type assertions, and a response
without running, exited or history lists panicked. Timestamps are
json.Number because the artifacts API returns created_at as a numeric
string while other endpoints return numbers; formattedTimestamp learns
to treat an empty json.Number as a missing value.
The table printer indexed versions[len-1] without a length check, so a
policy with no versions panicked. It now returns an error.
@github-actions github-actions Bot added go Pull requests that update go code refactor labels Oct 5, 2026
@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @dangrondahl's task in 1m 12s —— View job


Follow-up review complete

New commit 93851890 ("fix(assert artifact): decide the exit code from the compliant field alone") only touches cmd/kosli/assertArtifact.go and cmd/kosli/assertArtifact_test.go.

Previous findings touched by this commit:

  • -o json robustness regression (assertArtifact.go, full-struct decode for the exit code) — Resolved. assertCompliant now decodes only {"compliant": bool} via an anonymous struct, decoupled from assertArtifactResult. New tests (TestAssertCompliantDependsOnlyOnCompliant, TestAssertCompliantFailsWhenNotCompliant, TestAssertCompliantFailsWhenCompliantIsMissing) pin this, including a case with mismatched field types in policy_evaluations/flows. Thread resolved and replied on inline.

Previous findings not touched by this commit (already addressed in 7cc60129, prior to this review cycle, so out of scope here): the restoreLogger leak in assertArtifact_test.go, the json.Number/string duplication in cli_utils.go, and the trail_name/template_reference_name *string change in getArtifact.go. The getPolicy.go "no versions" design question was answered (kept as a hard error) and the author's reasoning stands — no action needed.

New issues in the touched lines: none found. The change is a minimal, well-targeted fix with direct test coverage.

Comment thread cmd/kosli/assertArtifact_test.go Outdated
Comment thread cmd/kosli/cli_utils.go Outdated
Comment thread cmd/kosli/getArtifact.go Outdated
Comment thread cmd/kosli/getPolicy.go
Move-only. The rest of cmd/kosli declares types before newXCmd, and
assertArtifactResult is used by run before its old declaration.
- get artifact prints the Trail and Name in template rows whenever the
  server sends the key, even with an empty value, as main does
- formattedTimestamp parses a json.Number through the string case
  instead of a second copy of the parse
- the assert printer tests restore the package logger they replace
@dangrondahl
dangrondahl marked this pull request as ready for review October 5, 2026 12:39
Comment thread cmd/kosli/assertArtifact.go Outdated
…lone

The compliance check decoded the whole response into the table printer's
types, so a type change in any nested field turned a compliant -o json
run into a decode error and a failing exit code.
@sami-alajrami
sami-alajrami merged commit 430b0d2 into main Oct 5, 2026
23 checks passed
@sami-alajrami
sami-alajrami deleted the refactor/typed-get-responses branch October 5, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update go code refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants