Apigee pending cbs uniformity changes - #775
Conversation
PR #729 is an independent generation of this bundle (registry issue #159). Of its six runbook tasks, exactly one reaches a finding nothing here could: environments hosting zero deployed API proxies. That is the environment axis of the inventory every other check reads from the proxy axis. check_failed_deployments asks "is this proxy deployed anywhere?" -- an org whose proxies are all deployed to prod answers yes for every proxy while an empty test environment sits serving nothing, and a hostname routed there returns an edge-level error rather than a proxy response. The new partialcoverage fixture is exactly that case, and asserts the coverage issue is raised AND that every proxy-side check stays silent. Deliberately counts deployments, not HEALTHY deployments. Gating on state == "READY" would make every environment look empty the moment the deployment status view became unreadable, and would re-report in this check what check_deployment_state already owns. Asserted from both sides: statusunknown and broken must both report zero coverage issues. Two correctness guards #729 does not have. A PROXIES filter removes deployments from the inventory, so an environment hosting only filtered-out proxies would look empty -- a finding manufactured by configuration; the check refuses to judge and says why. An ENVIRONMENTS filter is the opposite case: the topology records every environment unfiltered while deployments ARE filtered, so the environment list is narrowed to what the run actually looked at. Not adopted, with reasons recorded in the README: - Discovery as a task: it is suite setup here, deliberately. - Proxy deployment health, revision/approval state: already covered. - Health summary: raises no finding a per-condition check does not. - Runtime environment status: its Cloud Monitoring filter uses resource.labels.environment_name, which does not exist -- the Environment monitored resource has resource_container/org/env/location, so the query matches nothing and the check silently reports nothing. It also misreads environment/active (a GAUGE counting environments attached to an instance, not a per-environment liveness flag) and sums that gauge over the window. Read correctly it measures environment-to-instance attachment, which gcp-apigee-environment-health already checks directly through the Apigee API. Verified against the live metric and monitored-resource descriptors. - STALE_REVISION_THRESHOLD: #729 defaults it to 1, so a proxy one revision behind is never flagged. Adopting that would weaken the drift check. Offline tier 262 -> 301 assertions. 25 mutations, all detected against a green baseline, including four new ones: coverage silenced, coverage narrowed to READY-only, and each of the two scope-filter guards removed. Claude-Session: https://claude.ai/code/session_01NLcdGCtY4qZH8tW5MKDEvi Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* [Creator] [design-spec] gcp-apigee-traffic-health (#158) * [#731] Anchor gcp-apigee-traffic-health on the Apigee organization APIGEE_ORG was rendered from {{match_resource.resource_name}}, which is not an attribute runwhen-local builds. Because CustomUndefined.__str__ returns a placeholder rather than "", the SLX rendered APIGEE_ORG as the literal string missing_workspaceInfo_custom_variable: every Apigee API call targeted a non-existent org and operators saw the placeholder in the alias. Both templates now share one org resolution chain, in boolean mode and with the indexed payload materialised first so an absent .resource degrades instead of raising UndefinedError. Also in this change: - qualifiers ["project", "resource"] -> ["resource"]. The SLX name is built from the qualifier values and an Apigee X org is named after its project, so the old form rendered <project>-<project>-gcp-apigee-traffic-health-<hash>. - Auth gates on whether a token can be minted, not on whether activate-service-account succeeded, plus a key-shape probe that emits a sentinel and no key bytes. Without it, a run with no identity found nothing and reported healthy. - Discovery moved from a task into Suite Initialization; a missing scope file is now an error in each check rather than an empty organization. - Target servers are read as the bare array the API actually returns. `.targetServers[].name` matches no real response, so the target performance check evaluated nothing on every run while rendering as passed. - Throughput deviation is compared as a ratio in both directions. A drop is bounded at -100%, so the old abs(deviation) > 200 could never report a traffic collapse. - Findings are aggregated per failure mode; titles name the org and carry no counts or resource names. - Ships runbook-only: the SLI invoked a strict subset of the runbook's scripts with identical variables, and the SLO consumed the SLI's metric. - Adds an offline tier (fixtures built from the Apigee v1 discovery document) and a render tier (templates through runwhen-local's jinja2 config). Every assertion was mutation-tested: 22 mutations, all detected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pha15WRfir9fXdCbv5YoLB * [#731] Update SKILL-TEMPLATE.md to match the shipped bundle It still documented sli.robot and generate_traffic_summary.sh, both of which were removed, and listed task titles naming the project. Operator-facing documentation that tells someone to run a deleted script is worse than none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pha15WRfir9fXdCbv5YoLB * [#731] Wire THROUGHPUT_DEVIATION_PCT through, and assert the class Review finding: THROUGHPUT_DEVIATION_PCT was documented in README.md and the SKILL-TEMPLATE variable table as a tunable with default 200, and read by check_throughput.sh, but wired through none of the three places the platform needs it -- taskset configProvided, Import User Variable, and the env dict. The script's built-in default was therefore the only reachable value and an operator following the README could not change it. Confirmed by inspection: 0 occurrences in all three. Nothing was broken at the default, which is exactly why it survived: every behavioural assertion in the offline tier passed while the promise was unkeepable. Wired through all three, and added a CLASS assertion rather than only fixing the instance, per the review's second comment. Scenario H now extracts every variable the README's `## Configuration` section documents as operator config and asserts each is present in configProvided, in Import User Variable, and in the env dict. It is scoped to leading bullets inside that section, so prose mentions elsewhere (KEY_JSON, TOKEN_ABSENT, thresholds named in task descriptions) are not mistaken for config, and genuinely internal variables -- APIGEE_API, APIGEE_SCOPE_FILE -- are correctly ignored, as the review noted they should be. The behavioural scenarios cannot catch this class and adding more would not help: they export the variables themselves, so the scripts always see a value whether or not the runbook would ever supply one. That reasoning is recorded next to the new scenario. Audited the rest of both bundles for the same class: THROUGHPUT_DEVIATION_PCT here and SECURITY_WINDOW_HOURS in #733 were the only two. Verification: offline 130 passed / 0 failed (was 111), render 19/0, shellcheck clean, 26 mutations all detected -- including dropping the new variable from each of the three wiring points independently, and renaming the README heading, which proves the extraction cannot pass vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pha15WRfir9fXdCbv5YoLB * [#731] Harden the .test harness; stop leaking tokens into xtrace Second-pass review, all findings verified against the tree before changing anything. Items 1-4 (the ones flagged for merge): 1. build-infra reported success when it did nothing. `tf.secret` absent printed "Skipping" and exited 0, telling the caller the infrastructure existed when it did not. Now sources load-credentials.sh, which exits non-zero. 2. validate-generation-rules could not fail. `ajv ... && echo valid || echo invalid` swallowed the failure -- the block's exit status was the trailing `rm -rf` -- so an invalid rule printed "is invalid" and passed. Replaced with the environment-health model: checked `curl -fsS` for the schema, a `jq -e` sanity check on the download, a failure counter, and a guard that errors when the glob matches no rules. 3. default never reached discovery, and its description advertised terraform infra that was not in the chain. It now runs the credential-free tiers first, then build-infra, generate-rwl-config, run-rwl-discovery and validate-generation-rules. Without discovery a green `task` run said nothing about the generation rule, which is what most of this bundle's work was about. 4. No shared credential contract. Adopts load-credentials.sh from the merged siblings, so one tf.secret serves all four Apigee bundles and an ambient APIGEE_ORG that disagrees with the file is announced rather than silently retargeting a shared org. generate-rwl-config no longer defaults the org to "mock-org", which produced a workspaceInfo naming an organization that does not exist. Medium: 5. The Terraform provisioned a GCS bucket its own header called inert. Removed. The stub now publishes discovery ground truth instead -- gated resource type, expected SLX count, expected resourcePath -- so a discovery run has something machine-readable to be checked against. 6. Discovery image pinned to 0.11.11 with a pre-flight registry probe. On an older image discovery exits 0 with zero SLXs, which reads as "the rule matched nothing" when it is an image problem. Non-blocking item 8, tokens in the xtrace stream: fixed here even though it is pre-existing and collection-wide, because it is a live OAuth bearer credential written into captured output. As the review noted, the token leaks at the assignment as well as the request -- and also at the `[ -z "$token" ]` emptiness test, which is a third site: restoring tracing before that test just moves the leak. Verified zero occurrences of the stub's sentinel across all five scripts. The merged siblings still have it; not touched from this PR. Also updates .test/README.md, which still described the removed summary issue and the deleted bucket scaffold, and now leads with the credential-free tiers. Every new assertion is mutation-tested. Offline 152 passed / 0 failed (was 130), render 19/0, shellcheck clean, 34 mutations all detected -- including unwrapping each xtrace suppression point independently, reverting each harness fix, and re-adding the bucket. Not verified locally: the HCL. terraform is not installed in this environment, so .test/terraform was not syntax-checked or planned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pha15WRfir9fXdCbv5YoLB --------- Co-authored-by: rw-codebundle-agent[bot] <rw-codebundle-agent[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add gcp-apigee-security-config CodeBundle Monitors the security posture and access configuration of an Apigee organization: TLS keystore alias expiry, API product quota/rate limits, developer app access scope, Apigee security score/incidents, and target server TLS. Includes runbook, SLI, generation rules/templates, README, and test infrastructure. Closes #157 * [#733] Anchor gcp-apigee-security-config on the Apigee organization APIGEE_ORG was set to "{{project.name}}", so it rendered the PROJECT where the org was wanted. That works by coincidence on Apigee X, where an org is 1:1 with its project and the org ID is the project ID -- which is exactly why it survives testing and fails on any divergence: an explicit custom.apigee_org override, hybrid, or legacy Edge. The same file was already internally inconsistent, since its alias resolved the org correctly while the config value did not. Both templates now share one org resolution chain, in boolean mode and with the indexed payload materialised first so an absent .resource degrades instead of raising UndefinedError. Also in this change: - qualifiers ["project"] -> ["resource"]. gcp-hierarchy.yaml inserts project_id into the path only when `resource` is a qualifier, so the old form flattened resourcePath from gcp/<project>/<org> to gcp/<project> and never named the org. - Adds the scope tag, which was absent entirely, as lowercase `organization`. - Auth gates on whether a token can be minted, not on whether activate-service-account succeeded, plus a key-shape probe that emits a sentinel and no key bytes. Without it, every curl ran as no identity, every check found nothing, and the run reported a healthy org. - Target server TLS is read from the per-target-server document. The list endpoint returns a bare array of STRINGS and carries no sSLInfo, so the old code read .sSLInfo off a string, got an empty value, and its `[ -z ... ]` test then reported a plaintext backend for EVERY target server in the org without ever reading one. - Findings are aggregated per failure mode; titles name the org and carry no counts, scores, quota values or resource names. Consumer keys appear only as an 8-character prefix, and only in details. - Security metrics distinguish "not measured" from "measured as zero risk": an unpopulated Advanced API Security metric raises nothing. - awk replaces an interpreter dependency the runner image does not guarantee. - The security score task title named the project; it now names the org, like every other task. - Drops the keystore alias expiry check. gcp-apigee-environment-health already performs it, gating on the same resource type, so the same certificate raised the same finding twice against two SLXs for one org -- and this bundle's version never worked, reading .name off a bare array of strings and skipping every keystore. Reasoning recorded in the README and next to the tasks. - Drops the summary task, which raised no finding and only restated the others. - Ships runbook-only: the SLI invoked a strict subset of the runbook's scripts with identical variables and thresholds. - Adds a shared apigee_common.sh whose helpers separate the documented list endpoints (singular field names) from the undocumented ones (bare arrays). - Adds an offline tier (fixtures built from the Apigee v1 discovery document) and a render tier (templates through runwhen-local's jinja2 config). Every assertion was mutation-tested: 27 mutations, all detected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pha15WRfir9fXdCbv5YoLB * [#733] Update SKILL-TEMPLATE.md to match the shipped bundle It still documented sli.robot, check_keystore_tls.sh and generate_security_summary.sh, all of which were removed. Operator-facing documentation that tells someone to run a deleted script is worse than none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pha15WRfir9fXdCbv5YoLB * [#733] Wire SECURITY_WINDOW_HOURS through, and assert the class Review finding: SECURITY_WINDOW_HOURS was documented in README.md and the SKILL-TEMPLATE variable table as a tunable with default 6, and read by check_security_score.sh, but wired through none of the three places the platform needs it -- taskset configProvided, Import User Variable, and the env dict. The script's built-in default was therefore the only reachable value and an operator following the README could not change it. Confirmed by inspection: 0 occurrences in all three. Nothing was broken at the default, which is exactly why it survived: every behavioural assertion in the offline tier passed while the promise was unkeepable. Wired through all three, and added a CLASS assertion rather than only fixing the instance, per the review's second comment. Scenario G now extracts every variable the README's `## Configuration` section documents as operator config and asserts each is present in configProvided, in Import User Variable, and in the env dict. It is scoped to leading bullets inside that section, so prose mentions elsewhere (KEY_JSON, TOKEN_ABSENT, thresholds named in task descriptions) are not mistaken for config, and genuinely internal variables such as APIGEE_API are correctly ignored, as the review noted they should be. The behavioural scenarios cannot catch this class and adding more would not help: they export the variables themselves, so the scripts always see a value whether or not the runbook would ever supply one. That reasoning is recorded next to the new scenario. Audited the rest of both bundles for the same class: SECURITY_WINDOW_HOURS here and THROUGHPUT_DEVIATION_PCT in #731 were the only two. Verification: offline 130 passed / 0 failed (was 114), render 19/0, shellcheck clean, 31 mutations all detected -- including dropping the new variable from each of the three wiring points independently, and renaming the README heading, which proves the extraction cannot pass vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pha15WRfir9fXdCbv5YoLB * [#733] Stop task clean from disabling Apigee under the shared org Second-pass review, all findings verified against the tree before changing anything. BLOCKING items 1 and 2 -- the same root cause: google_project_service.apigee here and google_project_service.required ["apigee.googleapis.com"] in gcp-apigee-environment-health were the same GCP resource under two Terraform states. disable_on_destroy is unset and the provider default is true, and `task clean` runs `terraform destroy -auto-approve`, so tearing down this bundle would have attempted to DISABLE apigee.googleapis.com while a live organization, a runtime instance and three sibling bundles' fixtures depended on it. Resolved by dropping the ownership claim rather than by setting disable_on_destroy = false. This bundle is a read-only guest that never needed to enable anything, and single ownership is a stronger guarantee than two states that agree not to destroy. environment-health's bootstrap is now the documented prerequisite, which is the arrangement proxy-health already uses. High: 3. build-terraform-infra reported success when it did nothing. Now sources load-credentials.sh, which exits non-zero when credentials are absent. 4. validate-generation-rules could not fail. `ajv ... && echo valid || echo invalid` swallowed the failure. Replaced with the environment-health model: checked `curl -fsS`, a `jq -e` sanity check on the download, a failure counter, and a guard for an empty rule set. 5. Adopts the shared load-credentials.sh contract. This matters most here, as the review noted: this bundle reads a shared org, and a stale APIGEE_ORG left over from a sibling's run would silently retarget it. Medium: 6. Dropped the project-wide roles/iam.serviceAccountTokenCreator binding. It permitted impersonating every service account in the project; nothing here impersonates anything. 7. .test/README.md told operators to export a key that no resource created. Added google_service_account_key, gated on create_reader_key so the private key need not live in Terraform state, with the out-of-band gcloud alternative documented. 8. .test/README.md now leads with the credential-free tiers -- the ones that gate a PR -- and records that check_security_score cannot be demonstrated on an EVALUATION org at all, since apigee.googleapis.com/security/* only populates with the paid Advanced API Security add-on. It also no longer describes the removed keystore/cert-expiry scenario. 9. Discovery image pinned to 0.11.11 with a pre-flight registry probe. Non-blocking item 10, tokens in the xtrace stream: fixed here even though it is pre-existing and collection-wide, because it is a live OAuth bearer credential written into captured output. As the review noted, the token leaks at the assignment as well as the request -- and also at the `[ -z "$(apigee_token)" ]` guard, where a command substitution inside a test is expanded by xtrace before the test runs. Added apigee_have_token for that case. Verified zero occurrences of the stub's sentinel across all four scripts. The merged siblings still have it; not touched from this PR. Every new assertion is mutation-tested. Offline 156 passed / 0 failed (was 130), render 19/0, shellcheck clean, 40 mutations all detected -- including re-adding the google_project_service claim, re-adding the token-creator binding, removing the key resource, and unwrapping each xtrace suppression point. Not verified locally: the HCL. terraform is not installed in this environment, so .test/terraform was not syntax-checked or planned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pha15WRfir9fXdCbv5YoLB --------- Co-authored-by: rw-codebundle-agent[bot] <rw-codebundle-agent[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Checked this at 1. Completeness — what is and isn't hereCompared each bundle in this PR against the tip of the branch it supersedes:
So #733, #731 and #766 are fully represented and can be closed in favour of this PR once it lands. Two things worth confirming are deliberate: #729 is only partially adopted. Two of the five Apigee bundles are not in this PR. 2. Uniformity — current state across all fiveNone of the vocabulary work is in this PR yet. At
3. Proposed standard vocabularyTwelve names, identical in every bundle, same responsibilities, same exit semantics:
Two rules that make
4. Cross-cutting changesC1 —
|
|
Follow-up to the review above, covering the two Apigee bundles that are not in this PR — Posting here rather than on the merged PRs so the five-bundle picture stays in one place. If you'd rather track it separately, this converts cleanly into an issue. Why they belong in the conversationFive of the eight cross-cutting items apply to them, and on two axes the merged bundles are now behind the three in this PR:
So Two more axes I hadn't spotted — these affect all fiveOffline stub directory has three different names. Related, and this is the one that matters: only Offline runner filename varies four ways. Suggest:
|
| # | Change | Type |
|---|---|---|
| 1 | Add test-render — the only bundle in the family without a render tier |
new tier |
| 2 | Add validate-all-tests.sh and ci = test-offline → test-render → validate-generation-rules |
new |
| 3 | Rename test-issue-generation → test-live |
rename |
| 4 | Rename check-and-cleanup-terraform → check-and-cleanup-fixtures |
rename |
| 5 | internal: true on generate-certs, build-terraform-infra, import-keystore-alias |
visibility |
| 6 | Fold import-keystore-alias into build-infra — it provisions a fixture, and a separate step is a fixture people forget. default currently has to call it explicitly, which is the tell |
responsibility |
| 7 | Drop delete-slxs from clean (C1) |
🐞 |
| 8 | Pin RWL_IMAGE + adopt proxy-health's registry pre-flight (C3) |
drift |
| 9 | Refresh upload-slxs/delete-slxs/check-rwp-config from .test-tasks (C2) |
drift |
| 10 | xtrace guard — 8 scripts run set -x, none guard, and apigee_common.sh interpolates the token at 3 sites (C7) |
🔒 |
| 11 | default → standard order |
consistency |
Confirmed with its own stub, so no live token was involved:
++ curl -fsS -H 'Authorization: Bearer offline-fake-token' https://apigee.googleapis.com/v1/organizations/org
security-config's approach is the one to copy — centralising the guard in apigee_common.sh means the check scripts need no local change.
Keeps bootstrap-prerequisites / destroy-prerequisites. That asymmetry is deliberate: one Apigee org per project, four bundles are guests, and it is exactly what stops a guest's teardown disabling the API under the shared org — the bug fixed on #733.
gcp-apigee-product-governance
Closest of the five to the target already. Its check-and-cleanup-fixtures is the reference implementation for C6, and its offline tier is the reference for known-positive / known-negative / cannot-run coverage.
| # | Change | Type |
|---|---|---|
| 1 | Add ci — default inlines the tiers instead of naming them |
new |
| 2 | Drop the check-and-cleanup-terraform alias; keep check-and-cleanup-fixtures |
rename |
| 3 | internal: true on create-entitlement-fixtures |
visibility |
| 4 | Add test-live to default — the task exists, default skips it |
consistency |
| 5 | Drop delete-slxs from clean (C1) |
🐞 |
| 6 | Pin RWL_IMAGE + registry pre-flight (C3) |
drift |
| 7 | Refresh the three RW tasks (C2) | drift |
| 8 | Rename offline/stubs/ → offline/bin/, offline/run-offline-tests.sh → offline/run.sh |
naming |
| 9 | Preventive xtrace guard in apigee_common.sh — no script sets -x today, so nothing leaks, but the token is interpolated unguarded and the bundle is one set -x away from the same exposure |
🔒 preventive |
Suggested split
This reinforces the sequencing note from my previous comment. The rename-and-visibility pass (C4/C5/C6 + internal: true + file naming) touches all five bundles, changes no behaviour, and is reviewable in one sitting. Landing it first, as its own PR against main, means:
- the vocabulary is settled before this PR's content merges on top;
- there is no window where three bundles use one vocabulary and two use another;
- this PR stays about the bundles it actually changes.
C1 (five one-line edits) is worth folding into that same pass — it's the item with the highest day-to-day cost, since it breaks local teardown on every run.
C2/C3/C7 and the test-live work are better done per-bundle afterwards, with a live discovery run behind them.
|
Correction to my two comments above, on I listed C8 as "optional", and then in the What actually blocks it — and the fixNot idempotency. So the change is to take the prerequisite layer off Terraform and make it check-then-create over
Every one of those four is naturally idempotent as a check-then-create, and none needs state. That makes the block safely duplicable into all five bundles verbatim. The teardown guard is simpler than I saidI claimed Teardown order inside the task stays: peering → address → network → disable What this buys
Two things to get rightConcurrency. Org creation currently accepts only Copy drift — the real cost of the no-shared-library constraint, and this family has already demonstrated it: the bare-array Revised sequencingSlotting into the plan from my earlier comment:
C8 moves ahead of the drift fixes because it changes what |
…guard Addresses the review on #775 across all five Apigee bundles, including the two already on main that the PR did not touch -- uniformity across three of five is not uniformity. C1 `clean` no longer requires RunWhen Platform credentials. It called delete-slxs -> check-rwp-config, which exits 1 without RW_WORKSPACE/ RW_API_URL/RW_PAT, so clean-rwl-discovery never ran and a root-owned output/ was left behind after every local run. delete-slxs is dropped from the chain and stays available on its own. clean-rwl-discovery also stops running an unconditional `sudo rm -rf`, which failed outright without passwordless sudo even when there was nothing to remove. C2 The three RunWhen Platform tasks now come from ../../.test-tasks/Taskfile (as 49 other bundles already do) instead of five copies that had all drifted to the v3 /branches/main/ endpoint. Thin local wrappers keep the task names an operator types. Copying the bodies in verbatim was the other option and would have been ~300 lines x 5 of exactly the drift this commit is about; .test/ is harness, not shipped bundle content, so the no-shared- library constraint does not apply to it. C3 RWL_IMAGE pinned to 0.11.12 in all five, with proxy-health's registry pre-flight adopted everywhere. Verified 0.11.12 is the newest published tag (0.11.13 does not exist) and that its GCP resource-type registry carries gcp_apigee_organizations -- on an image without it, discovery exits 0 with zero SLXs, which reads as a bundle problem. C4/C5/C6 Standard vocabulary. run-mock-tests -> ci, test-issue-generation -> test-live, check-and-cleanup-terraform -> check-and-cleanup-fixtures, and the aliases are gone. Implementation sub-steps are internal: true, still reachable via `task --list-all`. `task --list` is now byte-identical across all five. C7 xtrace token guard. proxy-health ran `set -x` in 10 scripts with the token interpolated into curl at two sites in apigee_common.sh and no guard at all; env-health the same in 8. Both now disable tracing around the mint and the request and restore the PREVIOUS state rather than switching tracing on -- product-governance traces nothing, and gets the guard preventively. Verified against a canary token: 6 leaked lines -> 0 in both bundles. proxy-health also had no gcloud stub in its offline tier, so with GCP_ACCESS_TOKEN unset (which the nocreds scenario does deliberately) the fallback reached the real gcloud and, on any Linux host with ambient credentials, minted and traced a live token. Added, and the scenario now drives the no-token branch explicitly via TOKEN_FAIL instead of relying on gcloud being absent. C8 Prerequisites off Terraform. The APIs, VPC, peering range and Apigee org moved from environment-health's main.tf to apigee_prerequisites.sh, which is check-then-create over gcloud/REST and byte-identical in all five bundles. Five Terraform states cannot each own the same VPC -- with no state there is nothing to own, so every bundle is self-sufficient and teardown order stops mattering. Org creation accepts 409, and creates verify by describe rather than trusting an exit status, so concurrent bootstraps are a no-op rather than a coin flip. Teardown disables apigee and apigeeconnect only, never servicenetworking. check-shared-drift.sh checksums the duplicated files against the siblings and fails `task ci` on divergence. Enforced duplication is fine; silent drift is what hurts -- the bare-array /environments trap was rediscovered three times. Naming: offline/bin + offline/run.sh + render/run.sh + validate-all-tests.sh in all five (stub dirs were bin/stubs/mock, runners four different names). product-governance's validate-all-tests.sh was the LIVE tier under a name that means the offline aggregate everywhere else -- renamed to test-live.sh. test-live now exists in all five. proxy-health, security-config and traffic-health had no live assertion tier at all; each runs its checks against the fixtures in one shared working directory (they share a discovery cache) and asserts the artifacts are well-formed and that discovery actually established an inventory -- a 403 turns every list into [] and "nothing wrong" then reads identically to "nothing was looked at". No SLI/SLO added. Also corrected from the review: proxy-health's test-offline/test-render DO carry desc: and DO appear in `task --list` (via aliases), so that row of the uniformity table was wrong. Verification: `task ci` green in all five (offline + render + generation-rule validation against the live schema + drift check); render tier actually executed with jinja2 rather than skipping. Offline assertions 133/149/301/156/ 152 -> 168/185/337/191/185, the additions being the vocabulary, C1, C3 and gcloud-stub guards -- each negative-tested by reintroducing the defect. `task clean` exits 0 in all five with no credentials of any kind. shellcheck clean over all 34 changed shell files; terraform validate passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
|
Addressed at Two places the review was wrong, and one where I went the other wayThe C2's "copy those bodies in verbatim" — I used C7's "copy One trap worth recording, because the first version of the fix had it: factoring the guard into a helper does not work. A helper has to hand the saved state back through a command substitution, and Per item
C1 — reproduced before fixing: C3 — checked ghcr before pinning: C7 — confirmed with a canary token, before and after: The missing C8 — the four prerequisite resources are out of
One collision the review had not spotted
Verification
The render tier had been reporting Offline assertion counts, before → after:
The additions are static assertions on the vocabulary, C1, C3 and the stub directory — the thing that stops the five drifting apart again one convenience rename at a time. Each was negative-tested by reintroducing the defect: renaming Also: What is NOT verifiedNothing in C8 has run against real GCP. That is the one piece I would want a real run behind before this merges. |
…move The prerequisites section still described the APIs, VPC, peering range and Service Networking connection as Terraform resources in main.tf, and showed the `terraform apply -target=...` command bootstrap-prerequisites used to run. All four moved to apigee_prerequisites.sh in 3f85350, so that section documented a file state that no longer exists. Also corrected in the same section: disable_on_destroy no longer exists, `terraform destroy` no longer owns the reserved range, the runtime instances no longer carry a depends_on for destroy ordering (destroy-prerequisites refusing while the org exists is what enforces it now), and `task clean` no longer needs passwordless sudo. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
|
C9 — the fixture-ordering dependency that C8 did not remove. This supersedes the scope of C8 in my correction above: the substrate boundary moves from "the org" to "the org, its environments, and the runtime instance." The residual problemC8 made every bundle self-sufficient for APIs, VPC, peering and the org. It did not make them self-sufficient for the two Apigee environments, which are still created only by
Run them out of order and you get 404s that read as broken code. Only Why "let each bundle create its own environment" cannot workAn EVALUATION org is hard-capped, and both limits were hit empirically during #745 provisioning: Two environment slots, one instance slot, five bundles. Capped resources are inherently shared — which means they belong to the substrate by definition. Putting them in one bundle's fixtures was the original mis-assignment, and it is the same category error C8 fixed one level down. The fix1. Environments and the instance join the idempotent bootstrapFrom
Two environments serve all five. 2.
|
…te too
C8 made every bundle self-sufficient for APIs, VPC, peering and the org. It did
not make them self-sufficient for the two Apigee ENVIRONMENTS, which only
gcp-apigee-environment-health created. Four bundles need them, so four bundles
were still silent guests of a fifth -- the same category error one level up.
An EVALUATION organization is hard-capped:
the number of environments cannot exceed 2 for TRIAL subscription
the number of instance cannot exceed the limit 1
Two environment slots and one instance slot, five bundles. A capped resource is
inherently shared, which makes it substrate by definition -- "let each bundle
create its own" is not available.
The substrate boundary therefore moves from "the org" to "the org, its two
environments, and the runtime instance". apigee_prerequisites.sh now creates
all of it: idempotent, 409-tolerant, byte-identical in all five, drift-checked
by `task ci`. Environment and instance creates are long-running operations, so
each is polled to ACTIVE -- the POST returns before the resource is usable and
anything that moves on immediately 404s on the next call.
build-infra calls bootstrap-prerequisites itself. It is idempotent and no-ops
in seconds once satisfied, so which bundle you run first stops being something
anyone has to know -- the same argument as folding the keystore import into
build-infra.
apigee_preflight.sh (new, shared) asserts the contract BY NAME before any
fixture is created. proxy-health's preflight_apigee_org.sh was the only
substrate check in the family and it checked the wrong thing:
[ "$env_count" -eq 0 ] && fail "org has no environments."
That asserts "at least one" while the bundle needs two, and its bootstrap does
not fail without the second -- it does
if [ -n "$env2" ]; then deploy_revision ... "$env2" ...; fi
so with one environment the cross-environment drift fixture is SILENTLY
SKIPPED and check_revision_drift.sh then reports clean, not because there is no
drift but because nothing was created to drift. A count cannot catch that.
environment-health, security-config and traffic-health had no check at all, so
their failure mode was a wall of 404s that reads as broken code.
preflight_apigee_org.sh is replaced by the shared script.
THE SUBSTRATE CONTRACT is written into apigee_prerequisites.sh, carrying the
invariant that will otherwise be tidied away: apigee-env-unattached-* being
unattached is a FIXTURE, not a defect. It is environment-health's known-positive
for check_instance_attachments and proxy-health's second environment. Attaching
it makes that check pass because there is nothing to find, which is not the same
as passing because the org is healthy. preflight warns if it has been attached.
Because the slots are capped, the substrate names cannot vary per bundle: two
different suffixes would want four environments. APIGEE_SUBSTRATE_SUFFIX is
therefore shared, and _check_environment_slots fails naming the occupying
environments rather than letting a quota error do it.
google_apigee_instance.secondary is now opt-in behind enable_secondary_instance
(default false). It had no count, so on an EVALUATION org `terraform apply`
failed on it every time -- meaning this configuration had never completed end to
end there. Kept as a multi-region failover fixture for a PAID org, which is what
"stays" can only sensibly mean.
Verification: offline assertions 168/185/337/191/185 -> 183/200/352/206/200, the
additions covering the contract, the preflight semantics and the build-infra
wiring; each negative-tested by reintroducing the defect (dropping the bootstrap
call, deleting the invariant, weakening the preflight to a count). The preflight
itself is exercised against canned API responses through all six branches --
contract satisfied, one environment only, no instance, healthy env unattached,
unattached env wrongly attached, and the object-shaped /environments response.
`task ci` green in all five; terraform validate and fmt pass; shellcheck clean.
Still unverified against real GCP, now including the environment and instance
provisioning. The two empirical claims this rests on -- the EVALUATION caps, and
that a proxy deployed to an unattached environment still appears under
/deployments with environment and revision -- are the reviewer's observations,
recorded as such in the contract along with the caveat that deployment `state`
came back null on that topology.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
|
C9 addressed at Validated
env2=$(echo "$ENVS" | sed -n '2p')
...
if [ -n "$env2" ]; then
deploy_revision "${SUFFIX}-proxy-drift" "$env2" "$rev2"
fiSo on a one-environment org the fixture is silently skipped and
Your line numbers are from pre-C8 One thing that does not hold together
Worth confirming this is what you meant, because it also implies the "instance creation can race a freshly-created peering" note in the README was observed on a paid org, or was the cap failing rather than a race. What landedThe substrate boundary moved to org + both environments + the runtime instance. One thing your disposition table did not mention and that bit immediately: environment and instance creates are long-running operations. The POST returns before the resource is usable, so anything that creates one and moves on 404s on the very next call. Each is polled to
It warns rather than fails if The contract is written into One suffix across all five. Capped slots mean the substrate names cannot vary per bundle: two suffixes would want four environments. VerificationOffline assertions 168/185/337/191/185 → 183/200/352/206/200. New assertions negative-tested by reintroducing the defect — dropping the bootstrap call from The preflight itself is exercised against canned API responses through all six branches: contract satisfied, one environment only (the silent-skip case), no instance, healthy env unattached, unattached env wrongly attached, and the object-shaped
Still unprovenEverything in C8 and C9 remains unexecuted against real GCP, now including environment and instance provisioning. The two empirical claims underneath C9 — the I have written up the full bootstrap → build → teardown cycle as a runbook so that gap can actually be closed; happy to attach it here if useful. |
…by the substrate suffix
Introduced by C9 and caught while re-reading the runbook against the code.
test-live.sh built the expected environment name from TF_VAR_resource_suffix:
SUFFIX="${TF_VAR_resource_suffix:-${RESOURCE_SUFFIX:-test001}}"
assert_has "unattached environment" ... "apigee-env-unattached-${SUFFIX}"
but C9 names the environments from APIGEE_SUBSTRATE_SUFFIX, which is a
different value by design: the substrate slots are capped and shared, so all
five bundles must agree on that suffix, while each bundle's own fixtures are
free to use a per-run one. The moment the two diverge -- which is exactly the
case the contract documents and tells operators to set explicitly -- the
assertion looks for an environment nobody created.
This is the same trap the keystore-import comment already warns about one
task over, so the fix is the same: resolve the substrate suffix separately,
with the same precedence chain apigee_prerequisites.sh uses, and use it only
for the substrate name. The envgroup, target-server and keystore assertions
keep the per-run suffix, because those fixtures really are this bundle's.
The offline tier now asserts the split, so it cannot silently regress:
183 -> 186 assertions, negative-tested by reverting the name to ${SUFFIX}
(both the positive and the negative assertion fire).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
… that lied
Found by running `task ci` in codecollection-devtools from an interactive shell
that had sourced load-credentials.sh. Three separate defects, all of the same
family: a test that reports a different answer depending on where it runs.
1. THE OFFLINE TIERS INHERITED THE CALLER'S CREDENTIALS.
load-credentials.sh ends with
export APIGEE_ORG GCP_PROJECT_ID TF_VAR_org_id TF_VAR_project_id
and tf.secret is itself a file of `export TF_VAR_...` lines. Sourcing either --
the documented way to get credentials, and what every live task does -- leaves
them set for the rest of the shell session. The offline tier then read a REAL
organization where a fixture was intended:
* environment-health: the credential-contract scenario stopped failing on a
tf.secret naming no org, because load-credentials.sh prefers an ambient
APIGEE_ORG/TF_VAR_org_id over the file. try_load() sanitised APIGEE_ORG and
GCP_PROJECT_ID but not the TF_VAR_ spellings.
* product-governance: all four org-resolution scenarios resolved the ambient
org instead of the fixture's, so the projectId-filtering regression test
asserted nothing.
Seven assertions failed in a polluted shell and passed in a clean one. "No
cloud, no credentials, no spend" has to mean the tier cannot SEE the caller's
credentials, not that it politely declines to use them, so all five runners now
unset the whole set once, before any scenario runs.
2. grep OPTION INJECTION MADE AN ASSERTION PASS FOR THE WRONG REASON.
for unwanted in '- project' ...; do
if printf '%s' "$GR_CODE" | grep -qF "$unwanted"; then
The needle starts with a dash, so grep parsed it as an option, printed
"grep: invalid option -- ' '", exited non-zero, and the else-branch reported
PASS. The generation rule was never actually checked for `- project`. Every
grep taking a variable pattern now passes `--`.
3. SIGPIPE + pipefail TURNED A SUCCESSFUL MATCH INTO "not found".
proxy-health's assert_contains/assert_lacks did
printf '%s\n' "$haystack" | grep -qF -- "$needle"
Under `set -o pipefail`, grep -q exits on the first match while printf is still
writing, printf takes SIGPIPE (141), and pipefail reports the pipeline as
failed -- so a match reads as "not found". It only bites when the haystack is
large enough that printf has not finished, which is why it was invisible for
every small assertion, and it is image-dependent: GNU grep 3.8 in
codecollection-devtools triggers it, debian:stable-slim does not. In
assert_lacks the consequence is inverted and worse -- a present needle looks
absent, so the assertion PASSES on the very thing it forbids. Both now use a
herestring, which has no pipeline.
Verification: every tier green both in a clean environment and with
APIGEE_ORG/GCP_PROJECT_ID/TF_VAR_org_id/TF_VAR_project_id exported --
188/203/354/208/202, up from 186/200/352/206/200. The new assertions cover the
unset block and the `--` guard. shellcheck clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
`task ci` -- the credential-free gate this family is meant to run through --
could not complete in codecollection-devtools, the container everyone tests in.
validate-generation-rules shells out to ajv-cli, and that image ships no node,
no npm and no ajv, so the gate died on its last step with
Error: ajv is required but not installed.
ajv stays the preferred validator: it is the collection convention (44 of the 47
bundles that validate rules use it) and it is what CI has. What changes is that
its absence is no longer fatal when another validator exists. The body moves out
of five copies of the Taskfile into validate_generation_rules.sh -- shared,
byte-identical, drift-checked -- which picks ajv, falls back to Python's
jsonschema, and FAILS when neither is present, naming both install routes.
The two obvious alternatives are both wrong, and both already exist in this
collection:
* Skip when ajv is missing. The three non-ajv bundles here print
"Warning: ... Skipping validation" and exit 0. An absent check reports the
same green as a passing one -- the failure this family keeps designing
against.
* Downgrade to yaml.safe_load(). That proves the file is YAML, not that it
satisfies the schema, so a rule gating on a resource type that does not
exist sails through and discovery silently produces zero SLXs.
Equivalence checked, not assumed: ajv and jsonschema agree on all five bundles'
current rules, and both reject a rule with a non-string apiVersion and a scalar
where resourceTypes must be an array. jsonschema additionally reports every
error with its path, which ajv does not. The ajv-free path was exercised inside
the devtools image itself -- refusing correctly with no validator installed,
passing with `pip install jsonschema`, and rejecting a deliberately corrupted
rule.
The assertions that guarded the old inline body (checked schema download, JSON
sanity check, empty-ruleset error, no swallowed failures) now read the script
that owns them, plus new ones for the fallback and the never-skip rule.
One of those new assertions failed on first run, for the reason this codebase
has now recorded three times: the script's own header QUOTES "Skipping
validation" as the thing it must not do, so matching the whole file failed on
the prose. Comments are stripped first, as they already are for the generation
rule and the Taskfile.
Verification: `task ci` exits 0 in all five on a host with ajv; offline tiers
188/203/354/215/209; drift clean; task --list still byte-identical; shellcheck
clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
…t on PATH
`task clean` in codecollection-devtools ended with
"gcloud": executable file not found in $PATH
task: Failed to run task "clean": ... exit status 127
which names neither the problem nor the fix, and reads as a task crash.
activate-gcloud.sh accepts three credential routes and its header says failing
hard is right but "failing on the absence of one particular file, when another
valid path exists, is not". A missing gcloud broke that promise two ways:
* with gcp.json.secret present, the unguarded activate-service-account died
on a bare 127;
* without it, _ag_have_token swallowed the same 127 through `|| true`, so the
run reached the credentials advice at the bottom and told the operator to
supply credentials they already had.
Neither is a credentials problem. Every route this script accepts still needs
gcloud to spend the credential, so the binary is checked first and reported as
the tooling gap it is.
AND THE ADVICE HAD TO BE RIGHT, WHICH TOOK A CORRECTION.
I had previously reported that codecollection-devtools ships no gcloud, and
wrote the first version of this message accordingly. That was wrong, and wrong
because of how I measured: my probes ran `bash -lc`, and a LOGIN shell rebuilds
PATH from /etc/profile, discarding the image's ENV PATH. The SDK is there, at
~/google-cloud-sdk/bin, and a non-login `docker exec` finds it. So the common
case is not "not installed" but "installed and dropped by this shell" -- and
telling someone to install a tool they already have is worse than saying
nothing.
The check therefore looks before it advises: it probes the usual SDK locations
and either names the path it found with the exact `export PATH=...` line, or
points at the installer when it genuinely is not there. Both branches were
exercised -- the first inside the running devtools container reproducing the
operator's own login shell, the second in debian:stable-slim.
The two activate-service-account calls are also checked now, so a key file that
gcloud REJECTS says so instead of falling through to "no usable credentials".
node, npm and ajv really are absent from that image in both shell kinds, so the
preceding jsonschema-fallback commit stands unaffected.
Four harness cases cover it -- tooling-gap wording, no 127 masking, the
installed-but-off-PATH advice, and the genuinely-absent advice -- each
negative-tested by neutering the guard: 358 passed, dropping to 354 with 4
failures when reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
…ance Confirmed decision, so it stops being an inference and gets a guard. enable_secondary_instance (default false) already gated the second runtime instance and its attachment from C9. Nothing enforced that. Removing the count would be a one-line, plausible-looking edit that breaks EVERY build-infra on an EVALUATION organization -- the cap is one instance, and applying a second fails with "the number of instance cannot exceed the limit 1". That is precisely how this configuration came to have never completed end to end on an eval org in the first place, so it is worth a test rather than a comment. Six assertions: both resources gated, the variable present, its default really false, and -- since env-health's offline tier did not read its Terraform at all until now -- that this state no longer owns the primary instance or either substrate environment, which C9 moved out and five bundles now share. Each negative-tested by making the regression: removing the count fails "the second runtime instance is gated", flipping the default fails "the default really is false". terraform validate and fmt still pass; removing the count from both resources would also break `secondary[0]` at validate time, so the guard is layered rather than sole. terraform.tfvars.example did not mention the flag, so a paid-org operator had no way to discover it while instance_region sat above looking unconditional. Both are now documented together, with the cap that motivates the default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
|
It now has a guard ( Everything else since
|
…running op
Found by running bootstrap-prerequisites against a real project for the first
time. Bootstrap reported
attached 'apigee-env-healthy-apigee' to 'apigee-inst-primary-apigee'
Substrate ready:
and `task preflight`, immediately afterwards, disagreed in all five bundles:
PREFLIGHT FAILED: environment 'apigee-env-healthy-apigee' is not attached
preflight was right. The attachments list was {} and a fresh POST explained why:
400 FAILED_PRECONDITION -- the resource is locked by another operation that
is 25 percent completed so far where environment ... is currently being
attached ... by operation d9613ba8
Attaching an environment to a runtime instance is a long-running operation, the
same as the environment and instance creates this script already polls. That
fact is stated in this file's own header and was applied to two of the three
places it holds. _ensure_attachment posted, took the 200, printed "attached"
and returned -- so bootstrap declared the substrate ready while provisioning
was a quarter done, and every consumer that believed it would have deployed
into an environment that could not yet serve traffic.
The attachments list is also why retrying is not free: it stays EMPTY during
provisioning, so absent-because-provisioning is indistinguishable from
absent-because-missing and the existence check would happily POST again. Apigee
rejects that with 400 FAILED_PRECONDITION naming the in-flight operation --
which is now treated as "someone is already doing it" and falls through to the
same wait, the correct outcome for a concurrent bootstrap as well. Any other
400 is still a hard error.
Observed timings from this run, which the comments previously only guessed at:
org ~2.5 min, each environment under a minute, the runtime instance ~20 min,
the attachment several minutes more.
Everything else the run exercised behaved as designed: steps 1-4 no-op'd on
already-present APIs, VPC and peering; adopt_existing_range() reused the legacy
apigee-peering-<suffix> reservation rather than reserving a second /21; both
environments were created inside the two-slot EVALUATION cap; the single
instance slot was free. It was run from gcp-apigee-proxy-health, deliberately
NOT environment-health, which is the C8/C9 claim that the bootstrapping bundle
does not matter.
Verified: `task preflight` now exits 0 with "substrate contract satisfied" in
all five bundles against the live org. Offline tiers unchanged at
194/203/358/215/209, drift clean, shellcheck clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
…hecks
Three defects, all found by running the live tier against a real org for the
first time. It had never executed, so none of them had ever surfaced.
1. EVERY TASK FAILED IN SUITE SETUP, BEFORE ANY CHECK RAN.
[ WARN ] File 'discover_topology.sh' not found in '.../run'
FileNotFoundError: Could not find the robot file in any known locations.
7 tasks, 0 passed, 7 failed
RW.CLI resolves each script relative to the WORKING directory. This script runs
robot from a scratch dir on purpose -- the checks write their *_issues.json into
the cwd, and asserting on those artifacts beats scraping log.html -- but nothing
put the scripts there. The sibling it was modelled on avoids the problem by
running from the bundle root, which resolves the scripts and drops every issue
file into the source tree. Seeding copies into the scratch dir keeps both
properties, and is what gcp-apigee-proxy-health's offline harness already does
per scenario.
2. AN ABSENT TOPOLOGY WAS A SYNTAX ERROR, NOT A FAILED ASSERTION.
`[ "$(jq '.environments | length' ...)" -ge 2 ]` with no file left the left side
empty, so bash printed "integer expression expected" into the middle of the
results instead of the check reporting failure.
3. THE ASSERTIONS READ THE WRONG FIELD -- IN BOTH DIRECTIONS.
Titles carry the failure mode and the org scope only; the resource names live in
details. That was a deliberate change (one issue aggregates every affected
resource, so a title naming one of them is both wrong and unstable run to run),
and the offline tiers assert it directly -- "no contained resource name in any
title". The live tier was never re-run afterwards and still asserted the old
shape, so:
* the known-POSITIVES reported four fixtures as undetected when every one had
been found correctly, the names sitting in details the whole time;
* the known-NEGATIVES, which use assert_hasnt, passed without testing
anything at all -- a healthy fixture's name can never appear in a title, so
those two ticks were green for the wrong reason and would have stayed green
through a real regression.
A bodies() helper joins title + details + actual, and every resource-name
assertion now uses it.
Verified against runwhen-nonprod-sandbox: 16 passed, 0 failed, exit 0, with
APIGEE_ORG deliberately empty so the run exercises org auto-discovery. Every
known-positive fires, every known-negative stays silent. Offline tier gains a
guard for the seeding line (195 passed), negative-tested by removing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
1204a0a to
cbe7fd5
Compare
…ally
Found by running the live tier in the four bundles that had never run it.
proxy-health and security-config both died before their first assertion:
GCP_PROJECT_ID: Must set GCP_PROJECT_ID (or TF_VAR_project_id)
APIGEE_ORG: Must set APIGEE_ORG (or TF_VAR_org_id)
Nothing sourced the credential contract. Across the five bundles the task body
had drifted three ways:
* product-governance task sourced load-credentials.sh -> worked
* environment-health script sourced tf.secret itself -> worked
* proxy-health,
security-config,
traffic-health neither -> failed
Two of those spellings work and one does not, which is precisely what the
standard vocabulary exists to stop -- and the three that failed are the three
whose live tier had never been executed, so the drift was invisible.
load-credentials.sh is now sourced in the task, in all five, byte-identically.
The task is the right place: it is the documented contract ("source this from
.test tasks that need live credentials"), it normalises the organization name
across the several spellings tf.secret is written in, and it exits non-zero
when credentials are absent rather than letting the run proceed against
nothing. Sourcing it also exports every TF_VAR_ in the file, so the scripts
need nothing of their own.
The offline tier now asserts the task sources it and runs the script, so a
fourth spelling cannot appear unnoticed: 195/215/209 -> 197/217/211.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
C1 removed an unconditional `sudo rm -rf output` from clean-rwl-discovery. The
identical line twenty lines away in run-rwl-discovery was left alone, and it
killed the task on the first line, before it even pulled an image:
sudo: a password is required
Failed to remove output directory
task: Failed to run task "run-rwl-discovery": exit status 1
Same reasoning as C1: output/ is root-owned only because a PREVIOUS discovery
container created it. On a fresh checkout there is nothing to remove and sudo is
pure cost; on any host without passwordless sudo it is fatal. Skip when the
directory is absent, try plain rm, fall back to sudo, and if neither works name
the path to remove by hand instead of "Failed to remove output directory".
The C1 commit message even states the rule -- "an unconditional `sudo rm -rf`
made `task clean` fail outright on any machine without passwordless sudo" -- so
the miss was not a misunderstanding, it was not looking for the same pattern
elsewhere in the file. It stayed hidden because nothing had run discovery; it
failed the moment something did.
Guarded in the offline tier, keyed on the OLD form's error string rather than on
"sudo rm -rf output": the fixed form contains that too, as its fallback arm, so
the first version of the assertion failed against correct code.
197/217/211 -> 199/219/213. task --list still byte-identical across all five,
drift clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
Two bundles rendered with the generic default icon in the UI, because the icons
they named are not there:
icons/gcp/apigee_api_platform/apigee_api_platform.svg HTTP 200
icons/gcp/apigee/apigee.svg HTTP 403
icons/gcp/access-context-manager/access-context-manager.svg HTTP 403
gcp-apigee-traffic-health pointed at the second and gcp-apigee-security-config
at the third -- which is not an Apigee icon at all, but Access Context Manager,
a different GCP service. Both now use apigee_api_platform, matching the three
bundles that already did.
A missing icon fails silently: the UI substitutes its default, so nothing in a
discovery run, an upload, or any test tier reports it. It was visible only by
looking at five SLXs side by side in the workspace. The offline tier now asserts
the shared icon is used and neither wrong URL comes back.
199/219/213 -> 202/222/216.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HotdXokWQtJNYi2WPs4LkF
|
C8 and C9 are no longer unproven. The whole family has now been exercised end to end against a real GCP project — bootstrap, fixtures, live assertions, discovery and upload — and that run found eight defects that no amount of offline testing had surfaced. All eight are fixed and pushed. What ran
Deliberately bootstrapped from gcp-apigee-proxy-health, not environment-health — that is C8/C9's central claim, that the bootstrapping bundle no longer matters. Before C8 it was impossible: a second Terraform state cannot adopt the first's VPC. Timings, replacing the guesses in the comments: org ~2.5 min, each environment under a minute, runtime instance ~20 min, attachment several minutes more. Second run of the full bootstrap no-op'd in under a minute, which is what makes The EVALUATION caps behaved exactly as you described — two environment slots, one instance slot, both fitting. What discovery provedEvery SLX qualifies on The eight defects1. The instance attachment was never awaited ( 2-4. That third one is the one worth dwelling on. The known-positives reported four fixtures as undetected when every one had been found correctly. The known-negatives were worse: they use 5. 6. 7. Two SLXs used icons that do not exist ( 8. Left open deliberately — Probed the alternatives: I have not fixed it because the replacement is a semantic choice, not a rename — Unrelated finding: runwhen-local#832Apigee discovery fails on every ADC-authenticated run since 0.11.10 — StateOffline assertions 133/149/301/156/152 → 202/203/358/222/216, every addition negative-tested by reintroducing the defect it guards. The sandbox is still provisioned and billing — teardown is |
2 participants
No description provided.