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.yamlAll 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.
containerManifestFromModelleavesAnnotationsnil for a NULL column today, so a nil check is green on this tree. It is wrong against the contractdatastore.ContainerManifeststates (len == 0), and the failure mode is a silent200with an empty body rather than an error, becauseWriteJSONdrops the Encode error after writing the header. The guard defends the documented contract, not current behavior. omitemptyon the field. Absence travels asnullon a present key across this whole surface, and the schema'srequiredlists lean on it. An omitted key would fail both the key-set pin andopenapi3filter.ValidateResponse.- Serving
annotationson the list too. AC #129 (closed) forbids it, and the Page weight table prices it at 59.06 MB per hundred-row page.datastore.ContainerManifestListRowhas no such field, so the list is structurally incapable of reading the column. - Declaring the property inside a second
allOfbranch.TestContract_ContainerManifestSchemas_ShareOneBasereadsschema.Propertiesandschema.Requiredat the leaf's top level, so a branch-nested declaration would make its strongest arm pass vacuously.
Non-goals.
tags,children, andparent_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: 128is attributed tocontainer.manifest_max_annotation_keys, a configurable knob, following S17's own wording. The hardcoded ceiling actually lives incontainer_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.
Related to #1150 (closed)