feat(managementapi): serve annotations on the container manifest detail (S17 Container Manifest Reads plan: 19/20)

Why

Step 19 of the container manifest reads plan. annotations is a stored container_manifests column no response served. The detail route carries every other field of the manifest resource, so the Annotations section of the redesigned version detail page has nothing to read. This serves it there and keeps it off the list: at the caps the push path enforces, a hundred-row page would carry 59.06 MB of annotations (AC #129 (closed)).

What

The guard is on length, not nil. datastore.ContainerManifest documents its NULL reading as a zero-length json.RawMessage, which admits a non-nil empty value. That value fails to marshal, and transport.WriteJSON has already written the 200 and the Content-Type and drops the Encode error, so a nil check would answer a NULL column with 200 and an empty body, every other field lost with it. TestContainerManifestDetail_Hosted_Annotations/*_NULL_column_read_back_zero-length is that case.

Absence is test-enforced, not schema-enforced. kin-openapi leaves additionalProperties open on both manifest leaves, so response validation cannot catch a list element that wrongly carried the key (D2). Both list arms assert it absent instead.

required and properties sit at the detail leaf's own top level beside allOf, matching Step 10. Nested in an allOf branch, TestContract_ContainerManifestSchemas_ShareOneBase's "require every property you declare" arm would pass vacuously.

The property's three ceilings (128 keys, 512-byte keys, 4,096-byte values) are the stored column's own, so the 59.06 MB figure behind the list's withholding is derivable from the document.

No route changed, so guardrail 12 adds no e2e scenario and guardrail 21 no run-recipe edit. No operation was added, renamed, or removed, so the Bruno collection is unchanged.

Spec coverage

AC Criterion Tests
#128 (closed) Detail carries every list field plus annotations (this step's share of the four detail fields) TestContainerManifestDetail_{Hosted_PopulatedColumns,Hosted_NullColumns,Remote_ServesCachedRow,ReferrerRowIsServed,PerFormat}, through assertContainerManifestDetailShape re-keyed on containerManifestDetailResourceKeys
#129 (closed) annotations on the detail, null with no key pushed and {} when empty, absent from every list element rather than null TestContainerManifestDetail_Hosted_Annotations (four readings x docker/oci), TestContainerManifestList_ResourceFields, TestContainerRemoteManifestList_ResourceFields
#137 (closed) Remote detail serves annotations null TestContainerManifestDetail_Remote_Annotations (docker/oci), TestContainerManifestDetail_Remote_ServesCachedRow
Step 19 clause Test
null when no annotations key was pushed ..._Hosted_Annotations/{docker,oci}_no_annotations_key_pushed
The store's documented zero-length NULL reading still serves null rather than failing the whole marshal ..._Hosted_Annotations/{docker,oci}_NULL_column_read_back_zero-length
{} when it carried an empty one ..._Hosted_Annotations/{docker,oci}_empty_object_pushed
The stored object served verbatim ..._Hosted_Annotations/{docker,oci}_populated_object, ..._Hosted_PopulatedColumns
Absent from every list element, hosted and remote TestContainerManifestList_ResourceFields, TestContainerRemoteManifestList_ResourceFields
Property on the detail leaf and no other, required, object-or-null, string values, at the column's three ceilings TestContract_ContainerManifestDetailSchema_DeclaresAnnotations
A populated body validated through openapi3filter.ValidateResponse TestContainerHandlers_ResponsesMatchOpenAPIContract/manifest_detail_200

Reviewable LOC

Group Files Insertions Deletions
Source api/openapi/v1.yaml, internal/managementapi/container_resources.go 40 4
Test 5 files under internal/managementapi 219 25

288 lines, under guardrail 18's threshold.

Test plan

mise exec -- env -u GOROOT go test ./internal/managementapi/... -count=1
ARTIFACT_REGISTRY_DATABASE_TEST_DSN="postgres://<user>:<pw>@127.0.0.1:<port>/<db>?sslmode=disable" \
  mise exec -- env -u GOROOT go test -tags=integration ./internal/managementapi/... -count=1
mise exec -- env -u GOROOT golangci-lint run --build-tags=integration \
  --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false ./internal/managementapi/...
npx --yes @redocly/cli@2.34.0 lint --config api/openapi/redocly.yaml

All green locally: unit 22s, integration 104s against postgres:17-alpine at CI's .pg-service-options, both lint legs 0 issues, redocly valid.

Context for LLM agents

Rejected alternatives.

  • Nil check instead of a length check. containerManifestFromModel leaves Annotations nil for a NULL column today, so a nil check is green on this tree. It is wrong against the contract datastore.ContainerManifest states (len == 0), and the failure mode is a silent 200 with an empty body rather than an error, because WriteJSON drops the Encode error after writing the header. The guard defends the documented contract, not current behavior.
  • omitempty on the field. Absence travels as null on a present key across this whole surface, and the schema's required lists lean on it. An omitted key would fail both the key-set pin and openapi3filter.ValidateResponse.
  • Serving annotations on the list too. AC #129 (closed) forbids it, and the Page weight table prices it at 59.06 MB per hundred-row page. datastore.ContainerManifestListRow has no such field, so the list is structurally incapable of reading the column.
  • Declaring the property inside a second allOf branch. TestContract_ContainerManifestSchemas_ShareOneBase reads schema.Properties and schema.Required at the leaf's top level, so a branch-nested declaration would make its strongest arm pass vacuously.

Non-goals.

  • tags, children, and parent_digests, the detail's other three fields, are Steps 10 (merged), 18, and 16.
  • The column's null-vs-empty round trip through PostgreSQL is already proven by TestCreateContainerManifest_AnnotationsRoundTrip; this MR adds no datastore test.
  • maxProperties: 128 is attributed to container.manifest_max_annotation_keys, a configurable knob, following S17's own wording. The hardcoded ceiling actually lives in container_manifest_annotations.schema.json. Raised in review and declined as spec-faithful under guardrail 1.

Rebase note. Step 10 (!2540 (merged)) merged mid-review and changed the detail's shape. The branch was rebased onto it: containerManifestDetailResourceKeys is now one declaration of 14 entries, both mapping functions carry annotations through Step 10's facts parameter, and the detail leaf declares both tags and annotations.

🤖 Generated with Claude Code

Related to #1150 (closed)

Merge request reports

Loading
Loading