Skip to content

Centralize AWF container IP topology constants and remove duplicated literals#4947

Merged
lpcox merged 3 commits into
mainfrom
copilot/fix-duplicate-container-ip-addresses
Jun 14, 2026
Merged

Centralize AWF container IP topology constants and remove duplicated literals#4947
lpcox merged 3 commits into
mainfrom
copilot/fix-duplicate-container-ip-addresses

Conversation

Copilot AI commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

The container IP topology (172.30.0.{10,20,30,40,50} plus subnet) was duplicated across multiple modules, creating drift risk when network values change. This PR consolidates those values into shared exports and updates callers to consume a single source of truth.

  • Shared network topology constants

    • Added per-container IP exports to src/host-iptables-shared.ts alongside existing subnet/gateway constants:
      • SQUID_IP, AGENT_IP, API_PROXY_IP, DOH_PROXY_IP, CLI_PROXY_IP
  • Replaced duplicated literals in runtime code

    • src/host-iptables-network.ts: ensureFirewallNetwork() now returns IPs from shared constants.
    • src/config-writer.ts: networkConfig now uses shared constants for subnet + all sidecar/container IPs.
    • src/cli-workflow.ts: DoH and CLI proxy host-iptables inputs now use DOH_PROXY_IP and CLI_PROXY_IP.
  • Coverage for constant contract

    • src/host-iptables-shared.test.ts: added focused assertions that exported network constants match the expected fixed topology.
// src/host-iptables-shared.ts
export const NETWORK_SUBNET = '172.30.0.0/24';
export const SQUID_IP = '172.30.0.10';
export const AGENT_IP = '172.30.0.20';
export const API_PROXY_IP = '172.30.0.30';
export const DOH_PROXY_IP = '172.30.0.40';
export const CLI_PROXY_IP = '172.30.0.50';

Copilot AI changed the title [WIP] Refactor hardcoded container IP addresses into named constants Centralize AWF container IP topology constants and remove duplicated literals Jun 14, 2026
Copilot finished work on behalf of lpcox June 14, 2026 17:09
Copilot AI requested a review from lpcox June 14, 2026 17:09
@lpcox lpcox marked this pull request as ready for review June 14, 2026 17:10
Copilot AI review requested due to automatic review settings June 14, 2026 17:10
@github-actions

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.60% 96.64% 📈 +0.04%
Statements 96.47% 96.52% 📈 +0.05%
Functions 98.80% 98.80% ➡️ +0.00%
Branches 91.18% 91.21% 📈 +0.03%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 85.3% → 85.5% (+0.19%) 85.3% → 85.5% (+0.19%)
src/workdir-setup.ts 92.6% → 94.4% (+1.85%) 92.6% → 94.4% (+1.85%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR centralizes the fixed AWF container network topology (subnet + per-container reserved IPs) into shared exports, and updates runtime call sites to use those shared constants to reduce drift risk when network values change.

Changes:

  • Added per-container reserved IP constants (SQUID_IP, AGENT_IP, API_PROXY_IP, DOH_PROXY_IP, CLI_PROXY_IP) to src/host-iptables-shared.ts.
  • Replaced duplicated hard-coded IP/subnet literals in ensureFirewallNetwork(), config generation, and CLI workflow wiring with shared constants.
  • Added a focused unit test asserting the exported constants match the expected fixed topology.
Show a summary per file
File Description
src/host-iptables-shared.ts Introduces shared exports for reserved container IPs alongside existing network constants.
src/host-iptables-shared.test.ts Adds a unit test to lock down the shared constant contract.
src/host-iptables-network.ts Uses shared constants when returning the reserved IPs from ensureFirewallNetwork().
src/config-writer.ts Uses shared constants when writing networkConfig for compose/config generation.
src/cli-workflow.ts Uses shared constants for DoH and CLI proxy host-iptables inputs.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

Comment on lines 8 to +12
export const NETWORK_SUBNET = '172.30.0.0/24';
export const AWF_NETWORK_GATEWAY = '172.30.0.1';
export const SQUID_IP = '172.30.0.10';
export const AGENT_IP = '172.30.0.20';
export const API_PROXY_IP = '172.30.0.30';
Comment on lines +25 to +29
expect(NETWORK_SUBNET).toBe('172.30.0.0/24');
expect(SQUID_IP).toBe('172.30.0.10');
expect(AGENT_IP).toBe('172.30.0.20');
expect(API_PROXY_IP).toBe('172.30.0.30');
expect(DOH_PROXY_IP).toBe('172.30.0.40');
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)

  • GitHub MCP connectivity (PRs: Fix direct BYOK mode): ✅
  • HTTP 200/301 from github.com: ✅
  • File I/O test: ✅
  • BYOK inference: ✅

Overall PASS

@Copilot

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)

@github-actions

Copy link
Copy Markdown
Contributor

@Copilot @lpcox
Centralize AWF container IP topology constants and remove duplicated literals
✅ GitHub MCP connectivity
✅ GitHub.com HTTP
✅ File write/read
✅ BYOK inference
Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra
PASS

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)

@github-actions

This comment has been minimized.

- Import getLocalDockerEnv from docker-host directly instead of the
  docker-manager barrel to avoid circular dependency
- Replace literal IP/subnet strings in host-iptables-network.test.ts
  with imported constants for single-source-of-truth consistency

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #4947 ·

@github-actions

Copy link
Copy Markdown
Contributor

🔥 Smoke Test Results

PR: Centralize AWF container IP topology constants and remove duplicated literals
Author: @Copilot | Assignees: @lpcox @Copilot

Test Result
GitHub MCP connectivity
GitHub.com HTTP connectivity
File write/read

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

🔥 Smoke Test: Copilot PAT — PASS

Test Result
GitHub MCP connectivity
GitHub.com HTTP connectivity
File write/read

Overall: PASS — Auth mode: PAT (COPILOT_GITHUB_TOKEN)

cc @lpcox @Copilot

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) Mode ✅ PASS

Test Results:

  • ✅ GitHub MCP connectivity
  • ✅ GitHub.com connectivity (HTTP 200)
  • ✅ File write/read test
  • ✅ BYOK inference test (direct mode via api-proxy → api.githubcopilot.com)

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) with credential isolation: sidecar holds real key, agent sees placeholder only.

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor

Chroot Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.16.0 v22.22.3
Go go1.22.12 go1.22.12

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot.

  • Go matches correctly
  • Python patch version mismatch (3.12.13 vs 3.12.3)
  • Node.js major version mismatch (v24 vs v22)

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor

Centralize AWF container IP topology constants and remove duplicated literals
✅ Refactor OpenAI BYOK base URL parsing to reuse shared proxy URL normalization
✅ refactor(api-proxy): split proxy-request.js into http-client.js and body-handler.js
✅ GitHub title check
✅ smoke file write/read
✅ npm ci && npm run build
PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING (host.docker.internal:6379) ❌ Timed out
PostgreSQL pg_isready (host.docker.internal:5432) ❌ No response
PostgreSQL SELECT 1 ❌ No response

Overall: ❌ FAIL

host.docker.internal resolves to 172.17.0.1 but both services are unreachable — likely not running as GitHub Actions service containers in this workflow run.

🔌 Service connectivity validated by Smoke Services

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results: Gemini Engine

Overall Status: PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@lpcox lpcox merged commit 498d0f1 into main Jun 14, 2026
109 of 133 checks passed
@lpcox lpcox deleted the copilot/fix-duplicate-container-ip-addresses branch June 14, 2026 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants