[jtroup]

ENG-8580

what

CloudRunService.get_label_params and CloudRunJob.get_label_params
build their request body by deepcopying the whole resource, because
replaceService and replaceJob replace the entire object rather
than patching it. A shared knative_body() helper now does that
copy, keeping only the fields the Knative schema defines at the root
of a Service or a Job — apiVersion, kind, metadata, spec and
status — and dropping c7n's own annotations wherever they sit,
including nested inside spec.

A root field that is neither of those raises rather than being
dropped. An allow-list under a whole object replace means a field
it does not recognise would be deleted from the live service on a
200, so the one case it cannot handle safely is the one case it
refuses to guess at: the action fails, and nothing is written.

status is still sent, and metadata.resourceVersion is still
echoed back, both unchanged from before. The latter deserves its
own look: the list endpoint can hand back a resourceVersion that
lags a recent write by another actor, and replaying a stale one
draws a 409 that aborts the run for every resource after it. That
is tracked in
ENG-8590.

This fixes the body, not every failure on this path. set-labels
and mark-for-op on gcp.cloud-run-job still fail, with a 404:
replaceJob is served only by the regional Cloud Run endpoint,
while the client is built against the global one. That is an
unrelated defect in the transport rather than the payload, and is
tracked in ENG-8589.

Every workspace package moves from .post3 to .post4 via make pkg-increment PKG_INCREMENT=post, which increments the root and the
twelve tool packages, regenerates c7n/version.py, and updates
uv.lock.

why

Every label mutation on gcp.cloud-run-service and
gcp.cloud-run-job failed:

Invalid JSON payload received. Unknown name "labels" at
'service': Cannot find field.

Two kinds of key reached the body. augment() flattens
metadata.labels to a synthetic top level labels key, which the
label action needs in order to read the resource's current labels
but which the api has no field for. Separately, the annotations c7n
keeps on a resource rode along on the deepcopy — c7n:MatchedFilters
from a plain value filter is enough to trigger the rejection. Both
set-labels and mark-for-op were affected, since both reach the
same get_label_params.

Annotations are not confined to the root. A list-item filter
annotates the objects it matched in place, so a policy filtering on
spec.template.spec.containers leaves c7n:MatchedFilters on a
container, several levels inside a field the body has to carry
verbatim. Removing only the root keys would leave that policy shape
failing with the same error at a different path, so the copy drops
annotations at every depth.

Only the colon form is recognised below the root. c7n.metrics is
the sole dotted key c7n writes and it is only ever set on the
resource itself, whereas kubernetes annotation keys — user
controlled, and carried in metadata.annotations — may contain a
dot but never a colon. Matching the dotted form while descending
would strip an annotation such as c7n.example.com/owner, and
under a whole object replace a key omitted from the body is a key
deleted from the service.

Keeping a whitelist of schema fields rather than removing the two
known offenders means neither a new annotation key nor another
synthetic key added by augment() can reintroduce the failure.

The version bump cuts the next prerelease, so the fix ships.

Fixes: cloud-custodian#11034

testing

New coverage asserts the outgoing request body directly, for both
resources and for both set-labels and mark-for-op: the body
carries exactly the five schema fields and neither contaminant key,
metadata.labels holds the merged label set, and the resource
itself keeps the synthetic labels key that the action reads its
current state from. The policy-level tests spy on the api call and
invoke it, so parameter names are still validated against the
discovery document and the body is still serialized. Helper-level
tests cover an annotation nested in a container inside spec, a
dotted user annotation surviving at both annotation sites, the
resource being copied rather than mutated, and an unrecognised root
field raising while the expected synthetic keys pass through.

The expected field set is spelled out in the tests rather than
imported from the module under test, so that widening the module's
own tuple fails them instead of travelling through them.

The existing replay tests could not catch this — the replay
harness discards request bodies and returns the recorded response
regardless of what was sent, which is why the broken behaviour
shipped with passing tests.

mark-for-op is exercised with op: notify, because neither
resource registers a stop or delete action for it to mark for.

Verified against a live project, since no replay test can observe
this. With the original body the api returns the reported 400,
naming both labels and c7n:MatchedFilters in one response;
with the allow-list the same policy succeeds and the label lands on
the service. A list-item policy reproduces the nested case —
Unknown name "c7n:MatchedFilters" at 'service.spec.template.spec.containers[0]' — and succeeds once
annotations are dropped at depth. The equivalent job policy still
fails with the 404 above, unchanged by this work.

The replace semantics were measured rather than assumed. Omitting a
single user annotation from an otherwise unchanged body returns 200
and removes that annotation from the service, so a field left out of
the body really is a field deleted. Matching the dotted prefix while
descending was confirmed to delete c7n.example.com/owner from a
live service on an ordinary set-labels run, which is what the
colon-only match below the root prevents; with the colon-only match
a service carrying both c7n.example.com/owner and an unrelated
team.example.com/oncall keeps both across a run that lands its
label.

docs

No docs needed. Bug fix with no schema or user-facing surface.

---

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

The PR makes Cloud Run label mutations construct schema-constrained replacement bodies while preserving the original resource object. It also advances the prerelease package versions consistently.

• Adds a shared helper that retains Knative root fields, removes Custodian-generated annotations recursively, and rejects unknown root fields.
• Applies the sanitized replacement body to both Cloud Run Service and Job label mutations.
• Adds direct request-body coverage for set-labels and mark-for-op, including nested annotations and user-owned dotted annotation keys.
• Updates package metadata and the lockfile from post3 to post4.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
tools/c7n_gcp/c7n_gcp/resources/cloudrun.py	Introduces schema-constrained, recursively sanitized replacement bodies and uses them for Service and Job label mutations.
tools/c7n_gcp/tests/test_cloudrun.py	Adds helper- and policy-level assertions covering outgoing body shape, nested annotations, copy semantics, unknown fields, and both label actions.
uv.lock	Updates only the editable project package versions to the new prerelease revision.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Listed Cloud Run resource] --> B[knative_body]
  B --> C{Unknown root field?}
  C -- Yes --> D[Raise PolicyExecutionError]
  C -- No --> E[Keep apiVersion, kind, metadata, spec, status]
  E --> F[Remove nested c7n-colon annotations]
  F --> G[Merge metadata labels]
  G --> H[replaceService or replaceJob]

Loading

_{Reviews (2): Last reviewed commit: "chore: bump version to post4" | Re-trigger Greptile}

[ajkerrigan]

Thanks, looks good and the comments may help other folks avoid some of the "wait, can't we..." thoughts that I had while looking this over 😅

[ajkerrigan]

Thanks - this is a helpful comment. The part that initially threw me here was that most other labeling operations specify something like updateMask='labels' and the error message in cloud-custodian#11034 had a URL with updateMask=metadata.labels in it but that option doesn't seem to be available to Python until v2 of this service.

The replaceService() docs do call out:

Only the spec and metadata labels and annotations are modifiable.

But "whole object replace" does still feel like the right way to think about it 👍

[howbazaar]

LGTM