feat(tasks): migrate the secret-rotation task from gke-labs - #145
Open
jessie1111101 wants to merge 2 commits into
Open
feat(tasks): migrate the secret-rotation task from gke-labs#145jessie1111101 wants to merge 2 commits into
jessie1111101 wants to merge 2 commits into
Conversation
Ports tasks/gcp/secret-rotation and its prebuilt OpenTofu stack, with the
prompt rewritten to the terse house style and a verification_spec added.
The prompt no longer names the ExternalSecrets operator, Cloud Secret
Manager, the db-credentials object, or the rotation sequence. It states the
situation (the credential is compromised), the two constraints (the old value
must stop working; the service must not be interrupted) and the deliverable.
Discovering that the credential enters the cluster through a sync operator --
and that editing the Kubernetes Secret directly is overwritten within
refreshInterval: 10s -- is the task.
verification_spec declares two objectives (weights 3.0 and 2.0) and three
safeguards, one catastrophic and two recoverable. The objectives are that no
ExternalSecret in the namespace still resolves remoteRef.version to the
compromised "1", and that every ExternalSecret reports Ready=True; the pin
alone can be satisfied by pointing at a version that does not exist or that
the bound service account cannot read, which leaves the workload on a stale
Secret while the spec reads correctly.
Every "still there" safeguard matches by label selector rather than by name:
a single-object get of a deleted object exits non-zero, which reports as
status error, and an errored entry leaves both sides of the correctness
fraction -- so the exact destruction the safeguard exists to catch would drop
out of the score instead of scoring it. Each none[] group carries an existence
guard, because none over an empty match set inverts "nothing matched" into a
pass.
Two migration fixes: the original prompt used {{GKE_CLUSTER_NAME}} and
{{GCP_PROJECT_ID}}, neither of which this harness substitutes, so the agent
would have received two literal placeholders; and agent-rules.md is cut back
to how-to-work rules, since the original named the operator, the sequencing
and the remediation, making it an answer key.
namespace is pinned in infrastructure.variables. {{NAMESPACE}} resolves as env
NAMESPACE -> that variable -> the harness default, while the tofu variable
only ever comes from the map, so exporting NAMESPACE elsewhere desynchronises
the prompt and verifiers from the stack.
Known gap, documented in the README rather than papered over: the Secret
Manager side of the rotation -- a new version created, the compromised one
destroyed after the replacement is in use -- is a gcloud call, and no
registered verifier can make one. It stays in expected_output and is
informational, because VerificationCorrectness takes precedence over
ChecklistScore once any objective exists. A command verifier closes it.
Signed-off-by: Jessie Liu <jssl@google.com>
kubernetes-prow
Bot
added
the
kind/feature
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jessie1111101 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (24)
Comment |
kubernetes-prow
Bot
added
the
cncf-cla: yes
kubernetes-prow
Bot
added
the
needs-ok-to-test
|
Hi @jessie1111101. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
kubernetes-prow
Bot
added
the
size/XL
Two runs on dedicated GKE clusters, in the layout gke-labs/devops-bench#244 established and kubernetes-sigs#141/kubernetes-sigs#142 used: gemini-3.7-flash-openclaw-mcp google-vertex/gemini-3.7-flash claude-opus-5-openclaw-mcp anthropic-vertex/claude-opus-5 Both score OutcomeScore 1.0 at VerificationCoverage 1.0, with all five verification entries passing. Two things are worth reading out of that. The rotation objective works as designed. Its reason line, identically on both runs, is `across_matches=none: db-credentials: spec.data.[0]: '2' eq '1' is False` -- the agent moved the version pin and the inversion turned the failing inner match into a pass. A pin left at the compromised version fails here. The claude-opus-5 run scores ChecklistScore 0.75 and OutcomeScore 1.0 anyway, which is the _CORRECTNESS_KEYS fallback behaving as documented: with objectives declared, VerificationCorrectness is the correctness term and the checklist is informational. The item it missed asks for a surged rollout; the agent did not roll out at all, because db-secret-viewer mounts the credential and the kubelet refreshes a projected volume in place. The item grades a mechanism where it should grade continuity of service, and the judge penalised the correct answer. Rewriting it is a follow-up on this task, not a change to these numbers. Two caveats these runs carry that the v16/v17 batches did not, both recorded in the per-run READMEs: the kubernetes-sigs#72 container sandbox is kind-only so these ran unsandboxed with hand-rolled isolation, and BENCH_USE_MCP=true here is backed by a real AGENT_MCP_SERVER rather than being a bare flag. No task, harness, or infra code changes. Signed-off-by: Jessie Liu <jssl@google.com>
kubernetes-prow
Bot
added
size/XXL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third and last of the three leftover gke-labs GCP task migrations, after #143 (optimize-scale) and #144 (multi-region-failover). Ports
tasks/gcp/secret-rotationandtf/prebuilt/secret-rotation, rewrites the prompt to the terse house style, and adds averification_spec.Closes #39.
The prompt
The original told the agent which operator was in play, which secret to touch, what order to do it in, and how to remediate. What is left states the situation and the constraints:
Dropped: "ExternalSecrets operator", "Cloud Secret Manager", the name
db-credentials, the rollback instruction, and the cluster-access boilerplate. Discovering that the credential enters the cluster through a sync operator — and that editing the Kubernetes Secret directly gets overwritten withinrefreshInterval: 10s— is the task.agent-rules.mdgot the same treatment. It named the operator, the sequencing ("destroy last") and the remediation ("rolling restarts"), which made it an answer key. It is now six how-to-work rules with the task-specific procedure removed. It is opt-in viaAGENT_RULES_TEXTand is not loaded by default; the header says so.verification_spec
Two objectives, total weight 5.0:
ExternalSecretin the namespace resolvesspec.data[*].remoteRef.versionto"1"ExternalSecretin the namespace reportsReady=TrueThree safeguards:
db-credentialsAvailablecondition is notTrueat the end of the rungcp-storeClusterSecretStoreis goneThe objectives are paired deliberately. The version pin alone can be satisfied by repointing at a version that does not exist, or at one the bound service account cannot read — either leaves the workload mounting a stale Secret while the spec reads correctly.
Ready=Trueis the operator's own statement that it fetched the referenced version and wrote it to the target, so it is what separates a rotation from an edit.Objective 1 is spelled "no element resolves to
1" rather than "every element equals2".across_matches: nonequantifies over the elements ofspec.data[*], and an element that does not resolveremoteRef.versiontrivially conforms — so an agent that dropped the field entirely (defaulting to latest) passes, which is correct.op: newould fail on that same missing field andop: absentwould fail on an explicit"2".Safeguard shapes
Every "still there" check matches by label selector, never by
resource_name. A single-objectkubectl getof a deleted object exits non-zero,get_resourceraises, andresource_propertyreports statuserror— and an errored entry leaves both sides of the correctness fraction, so the exact destruction the safeguard exists to catch would drop out of the score instead of scoring it. A selector that matches nothing is a cleanfail.Each
none[...]group carries an existence guard in front of it._check()returns"fail"on an empty object set before anyacross_matchesreduction, and an enclosingnoneinverts that into a pass — so without the guard, deleting everything is the winning move.Migration fixes
{{GKE_CLUSTER_NAME}}and{{GCP_PROJECT_ID}}. The substituted set is{{PROJECT_ID}},{{CLUSTER_NAME}},{{APP_LOCATION}},{{TARGET_DEPLOYMENT_NAME}},{{NAMESPACE}}— the agent would have received two literal{{...}}strings. Same bug as the one fixed in feat(tasks): migrate multi-region-failover with a terse prompt and safeguards #144.namespaceis pinned ininfrastructure.variables.{{NAMESPACE}}resolves as envNAMESPACE→ that variable → the harness default, while the tofu variable only ever comes from that map, so exportingNAMESPACEto anything else points the prompt and the verifiers at a namespace the stack never created. Called out in the task comment and the README.Known gap
The Secret Manager side of the rotation — a new version was created, and the compromised version was destroyed or disabled after the replacement was in use — is a
gcloudcall, and no registered verifier can make one. It stays inexpected_output, which is markedJudge the following, none gating:because that is now what it is:VerificationCorrectnesstakes precedence overChecklistScoreoutright, so once any objective exists the checklist is informational. Revocation of the compromised credential does not move the score.Two related things the end state also cannot show: whether availability held during the rotation (the safeguard reads the final state, so a rotation that dropped every pod for a minute and recovered still passes), and whether the workload is serving the new value (no baseline to compare against).
This is written into the README rather than papered over.
add-http-probe-and-sar-sweep— acommand/httpverifier — is the follow-up that closes it, and it is the same follow-up named in #144.Testing
parse_entrieson the spec: 5 entries, 0 errors — 2 objectives (3.0, 2.0), 1 catastrophic, 2 recoverable.hack/boilerplate.pyclean; Apache headers applied to the 7.tffiles./kind feature