Add job to ensure expected test networks are created#591
Open
aramprice wants to merge 2 commits intoubuntu-jammyfrom
Open
Add job to ensure expected test networks are created#591aramprice wants to merge 2 commits intoubuntu-jammyfrom
aramprice wants to merge 2 commits intoubuntu-jammyfrom
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR introduces a new Concourse job Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
aramprice
force-pushed
the
ensure-integration-network
branch
2 times, most recently
from
73e988a to
f1f7ced
Compare
aramprice
changed the title
aramprice
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a manually triggered Concourse job to create/ensure the GCP integration subnetwork and firewall rule required by the pipeline’s GCP integration/BATs flows, without relying on Terraform state.
Changes:
- Added a new Concourse task (
ensure-integration-network) that usesgcloudto create the expected subnet and firewall rule if they don’t exist. - Added a new
infrastructuregroup andensure-integration-networkjob toci/pipelines/builder.yml. - Minor YAML cleanup/formatting adjustments in
ci/pipelines/builder.yml.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ci/tasks/gcp/ensure-integration-network.yml | Defines the Concourse task interface/params for ensuring the GCP integration network resources. |
| ci/tasks/gcp/ensure-integration-network.sh | Implements idempotent gcloud logic to create the subnet and firewall rule. |
| ci/pipelines/builder.yml | Wires the new manual job into the pipeline under a new infrastructure group. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/pipelines/builder.yml`:
- Line 1214: Remove the extraneous blank line that caused the YAMLlint
`empty-lines` error: delete the extra empty line so there are no consecutive
blank lines in the pipeline YAML (ensuring the builder pipeline block around the
area referenced no longer has a blank-only line), then run YAMLlint to confirm
the `too many blank lines (1 > 0)` violation is resolved.
- Around line 101-106: The job plan references an image resource
"bosh-integration-image" for the task ensure-integration-network but never
fetches it; add a `- get: bosh-integration-image` step before the `- task:
ensure-integration-network` entry in the plan so Concourse can resolve the image
resource, ensuring the `image: bosh-integration-image` field in the
ensure-integration-network task will point to a fetched artifact.
In `@ci/tasks/gcp/ensure-integration-network.sh`:
- Around line 17-50: The script currently only checks resource names (via gcloud
... --format="value(name)"), which can hide drift; update the subnet and
firewall validation (the blocks using gcloud compute networks subnets describe
and gcloud compute firewall-rules describe for SUBNET_NAME) to fetch and compare
critical attributes (for subnet: network, region, ipCidrRange / SUBNET_CIDR,
privateIpGoogleAccess, stackType; for firewall: network, allowed/protocols,
sourceRanges/SUBNET_CIDR, targetTags) using --format flags and then fail-fast or
reconcile when any mismatch is detected (e.g., exit non-zero with a clear error
if values differ, or delete+recreate to reconcile), referencing SUBNET_NAME,
SUBNET_CIDR, GCP_NETWORK_NAME and target-tags in the checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 52184cda-ce37-432f-a470-bb6317013afa
📒 Files selected for processing (3)
ci/pipelines/builder.ymlci/tasks/gcp/ensure-integration-network.shci/tasks/gcp/ensure-integration-network.yml
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
aramprice
force-pushed
the
ensure-integration-network
branch
2 times, most recently
from
d0df8e9 to
008969f
Compare
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
aramprice
force-pushed
the
ensure-integration-network
branch
from
008969f to
75d87e0
Compare
aramprice
force-pushed
the
ensure-integration-network
branch
from
75d87e0 to
a6d330d
Compare
aramprice
force-pushed
the
ensure-integration-network
branch
from
a6d330d to
8680c9a
Compare
Comment on lines
+54
to
+56
| exit 1 | ||
| fi | ||
|
|
Member
Author
There was a problem hiding this comment.
@copilot please suggest a fix
aramprice
force-pushed
the
ensure-integration-network
branch
from
8680c9a to
da6f688
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ci/tasks/gcp/ensure-integration-network.sh (1)
59-67:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate full firewall field sets, not only first entries.
Line 61 only checks
allowed[0]andsourceRanges[0]. A drifted rule with extra protocols or extra source ranges can still pass this check.Suggested hardening
current_fw="$(gcloud compute firewall-rules describe "${SUBNET_NAME}" \ --project="${GCP_PROJECT_ID}" \ - --format='csv[no-heading](network.basename(),direction,allowed[0].IPProtocol,sourceRanges[0],disabled)' \ + --format='csv[no-heading](network.basename(),direction,disabled)' \ 2>"${gcloud_stderr}")" && fw_exists=true || fw_exists=false if ${fw_exists}; then - expected_fw="${GCP_NETWORK_NAME},INGRESS,all,${SUBNET_CIDR},False" + expected_fw="${GCP_NETWORK_NAME},INGRESS,False" if [[ "${current_fw}" != "${expected_fw}" ]]; then echo "ERROR: Firewall rule '${SUBNET_NAME}' exists but is misconfigured." echo " Expected: ${expected_fw}" echo " Actual: ${current_fw}" exit 1 fi + current_protocols="$(gcloud compute firewall-rules describe "${SUBNET_NAME}" \ + --project="${GCP_PROJECT_ID}" \ + --format='value(allowed[].IPProtocol)' \ + | tr ',;' '\n' | LC_ALL=C sort | tr '\n' ',' | sed 's/,$//')" + expected_protocols="all" + if [[ "${current_protocols}" != "${expected_protocols}" ]]; then + echo "ERROR: Firewall rule '${SUBNET_NAME}' has wrong allowed protocols." + echo " Expected: ${expected_protocols}" + echo " Actual: ${current_protocols}" + exit 1 + fi + current_sources="$(gcloud compute firewall-rules describe "${SUBNET_NAME}" \ + --project="${GCP_PROJECT_ID}" \ + --format='value(sourceRanges.list())' \ + | tr ',;' '\n' | LC_ALL=C sort | tr '\n' ',' | sed 's/,$//')" + expected_sources="${SUBNET_CIDR}" + if [[ "${current_sources}" != "${expected_sources}" ]]; then + echo "ERROR: Firewall rule '${SUBNET_NAME}' has wrong source ranges." + echo " Expected: ${expected_sources}" + echo " Actual: ${current_sources}" + exit 1 + fi#!/bin/bash set -euo pipefail # Verify that first-element-only checks are currently used. rg -n -C2 'allowed\[0\]\.IPProtocol|sourceRanges\[0\]' ci/tasks/gcp/ensure-integration-network.sh # Show the firewall validation block for manual inspection. sed -n '57,90p' ci/tasks/gcp/ensure-integration-network.sh🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/tasks/gcp/ensure-integration-network.sh` around lines 59 - 67, The current check builds current_fw using allowed[0].IPProtocol and sourceRanges[0], which misses extra protocols or ranges; update the firewall validation to compare the full allowed and sourceRanges sets instead of only the first element. Replace the CSV format that uses allowed[0].IPProtocol and sourceRanges[0] with either a JSON output (gcloud ... --format=json) and parse/normalize allowed[*].IPProtocol and sourceRanges[*] (e.g., collect, sort and dedupe) or use gcloud joins to emit all entries, then construct expected_fw from ${GCP_NETWORK_NAME},INGRESS,<sorted-protocols>,<sorted-ranges>,False and compare against current_fw; modify the logic around current_fw, fw_exists, and expected_fw (references: current_fw, fw_exists, expected_fw, SUBNET_NAME, SUBNET_CIDR, GCP_NETWORK_NAME) so misconfigured rules with extra protocols or ranges are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@ci/tasks/gcp/ensure-integration-network.sh`:
- Around line 59-67: The current check builds current_fw using
allowed[0].IPProtocol and sourceRanges[0], which misses extra protocols or
ranges; update the firewall validation to compare the full allowed and
sourceRanges sets instead of only the first element. Replace the CSV format that
uses allowed[0].IPProtocol and sourceRanges[0] with either a JSON output (gcloud
... --format=json) and parse/normalize allowed[*].IPProtocol and sourceRanges[*]
(e.g., collect, sort and dedupe) or use gcloud joins to emit all entries, then
construct expected_fw from
${GCP_NETWORK_NAME},INGRESS,<sorted-protocols>,<sorted-ranges>,False and compare
against current_fw; modify the logic around current_fw, fw_exists, and
expected_fw (references: current_fw, fw_exists, expected_fw, SUBNET_NAME,
SUBNET_CIDR, GCP_NETWORK_NAME) so misconfigured rules with extra protocols or
ranges are detected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e7428e2-ca14-4394-8ef6-f0cbf064d3f0
📒 Files selected for processing (3)
ci/pipelines/builder.ymlci/tasks/gcp/ensure-integration-network.shci/tasks/gcp/ensure-integration-network.yml
Comment on lines
+84
to
+89
| fi | ||
| echo "Firewall rule '${SUBNET_NAME}' already exists and matches expected configuration." | ||
| elif grep -q "was not found" "${gcloud_stderr}"; then | ||
| echo "Creating firewall rule '${SUBNET_NAME}'..." | ||
| gcloud compute firewall-rules create "${SUBNET_NAME}" \ | ||
| --network="${GCP_NETWORK_NAME}" \ |
Member
Author
There was a problem hiding this comment.
@copilot please propose a fix or resolve this
Why
---
The bats and test-stemcells-ipv4 jobs assume a GCP subnetwork named
stemcell-builder-integration-<subnet_int> exists in the bosh-concourse
VPC, with a /24 at 10.100.<subnet_int>.0/24, private Google access, and
IPV4_ONLY stack type. They also require a matching ingress firewall rule
(all-protocol, source CIDR → tags test-stemcells-bats/bat) so that
compilation VMs and BAT deployment VMs can reach the BOSH director's
NATS server. Until now both resources had to be created and maintained
out of band; their absence caused consistent compilation-VM agent
timeouts (builds 466–475).
What
----
* ci/tasks/gcp/ensure-integration-network.sh
- Authenticates via GCP_JSON_KEY service account.
- Derives SUBNET_NAME and SUBNET_CIDR from SUBNET_INT.
- Captures stderr via mktemp temp file (cleaned up by trap on EXIT)
so that gcloud failures are classified: "was not found" → create the
resource; anything else → print the error and exit non-zero. This
prevents auth/permission/transient API errors from being silently
misinterpreted as "resource missing".
- Subnetwork: single gcloud describe call captures exit code (for
existence) and attributes (for drift detection). Validates network,
ipCidrRange, privateIpGoogleAccess, and stackType; exits non-zero
with a clear diff on any mismatch.
- Firewall rule: same stderr-capture pattern. Validates network,
direction, allowed[0].IPProtocol (must be "all"), sourceRanges[0],
and disabled (must be False) in one describe call. Validates
targetTags in a second describe call, sorting both sides before
comparison to be order-insensitive. Both 'test-stemcells-bats' and
'bat' tags are required, mirroring the existing
stemcell-builder-integration-22 rule.
* ci/tasks/gcp/ensure-integration-network.yml
- Concourse task definition. All params (GCP_JSON_KEY, GCP_PROJECT_ID,
GCP_REGION, GCP_NETWORK_NAME, SUBNET_INT) are required; no defaults,
values are provided explicitly by the pipeline.
* ci/pipelines/builder.yml
- New infrastructure group containing the new job.
- New job ensure-integration-network:
* serial: true, manual trigger only.
* Gets bosh-stemcells-ci and bosh-integration-image, then runs the
task with GCP_REGION=europe-north2 and
GCP_NETWORK_NAME=bosh-concourse passed explicitly.
* No passed: constraint on existing jobs; run on demand when the
subnet/firewall needs to be created or reconciled.
Verification
------------
* ytt -f ci/pipelines/builder.yml -f ci/pipelines/vars.yml renders
successfully.
* fly validate-pipeline -c <rendered> reports "looks good".
Co-authored-by: Cursor <cursoragent@cursor.com>
aramprice
force-pushed
the
ensure-integration-network
branch
from
da6f688 to
2152465
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
aramprice
requested review from
a team,
colins,
Copilot and
selzoc
and removed request for
a team
Comment on lines
+63
to
+93
| current_fw="$(gcloud compute firewall-rules describe "${SUBNET_NAME}" \ | ||
| --project="${GCP_PROJECT_ID}" \ | ||
| --format='csv[no-heading](network.basename(),direction,allowed[0].IPProtocol,sourceRanges[0],disabled)' \ | ||
| 2>"${gcloud_stderr}")" && fw_exists=true || fw_exists=false | ||
|
|
||
| if ${fw_exists}; then | ||
| expected_fw="${GCP_NETWORK_NAME},INGRESS,all,${SUBNET_CIDR},False" | ||
| if [[ "${current_fw}" != "${expected_fw}" ]]; then | ||
| echo "ERROR: Firewall rule '${SUBNET_NAME}' exists but is misconfigured." | ||
| echo " Expected: ${expected_fw}" | ||
| echo " Actual: ${current_fw}" | ||
| exit 1 | ||
| fi | ||
| # Validate target tags independently; sort before comparing since order is not deterministic | ||
| current_tags="$(gcloud compute firewall-rules describe "${SUBNET_NAME}" \ | ||
| --project="${GCP_PROJECT_ID}" \ | ||
| --format='value(targetTags.list())' \ | ||
| 2>"${gcloud_stderr}" \ | ||
| | tr ',;' '\n' | LC_ALL=C sort | tr '\n' ',' | sed 's/,$//')" && current_tags_read=true || current_tags_read=false | ||
| if ! ${current_tags_read}; then | ||
| echo "ERROR: gcloud describe failed while reading target tags for firewall rule '${SUBNET_NAME}':" | ||
| cat "${gcloud_stderr}" >&2 | ||
| exit 1 | ||
| fi | ||
| expected_tags="$(printf '%s\n' ${FIREWALL_TAGS//,/ } | LC_ALL=C sort | tr '\n' ',' | sed 's/,$//')" | ||
| if [[ "${current_tags}" != "${expected_tags}" ]]; then | ||
| echo "ERROR: Firewall rule '${SUBNET_NAME}' has wrong target tags." | ||
| echo " Expected: ${expected_tags}" | ||
| echo " Actual: ${current_tags}" | ||
| exit 1 | ||
| fi |
Member
Author
There was a problem hiding this comment.
@copilot the python dependency is not worth the complexity. Please resolve this or propose an option that uses shell and jq
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
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.
Adds a manually-triggered Concourse job ensure-integration-network (in a new infrastructure pipeline group) that idempotently creates the subnet and firewall rule using gcloud — no Terraform state or GCS bucket required. The script checks whether each resource exists before attempting creation, mirroring the existing stemcell-builder-integration-22 firewall rule (all-protocol INGRESS from the subnet CIDR to VMs tagged test-stemcells-bats/bat).