Skip to content

Commit 0d18b39

Browse files
Fix: Add Gerrit DNS validation, CI_TEST_MODE
Add fast-fail DNS validation of the Gerrit server hostname after config display, before PR processing. This catches bogus derived hostnames early instead of letting the tool fail silently during cleanup. Implement CI_TEST_MODE leveraging DRY_RUN: when CI_TEST_MODE=true, force DRY_RUN=true and G2G_DRYRUN_DISABLE_NETWORK=true so all tool logic runs but Gerrit network operations are no-ops. Cleanup tasks are also suppressed since there is no server to query. Display a CI_TEST_MODE row with a test-tube emoji in the config table when active. Gate all cleanup operations on not ci_test_mode to prevent network calls to non-existent Gerrit servers. The early DNS validation is skipped when CI_TEST_MODE, G2G_TEST_MODE, or G2G_DRYRUN_DISABLE_NETWORK are active, consistent with existing dry-run preflight. Add CI_TEST_MODE as an action input in action.yaml and pass it to the CLI step. Update the reusable workflow to read vars.CI_TEST_MODE. Switch the testing.yaml happy-path test to CI_TEST_MODE=true. Update test fixtures using placeholder hostnames to set G2G_DRYRUN_DISABLE_NETWORK=true when popping G2G_TEST_MODE since they target validation logic not network reachability. Remove console output blank line before: - ✅ Operation completed! - ❌ Operation failed! Co-authored-by: Claude <claude@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
1 parent e240ec1 commit 0d18b39

File tree

9 files changed

+425
-16
lines changed

9 files changed

+425
-16
lines changed

.github/workflows/github2gerrit.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ jobs:
190190
DRY_RUN: ${{ inputs.DRY_RUN }}
191191
ISSUE_ID: ${{ inputs.ISSUE_ID }}
192192
ISSUE_ID_LOOKUP_JSON: ${{ vars.ISSUE_ID_LOOKUP_JSON }}
193+
CI_TEST_MODE: ${{ vars.CI_TEST_MODE }}
193194
PR_NUMBER: ${{ inputs.PR_NUMBER }}
194195

195196
github2gerrit:
@@ -220,6 +221,7 @@ jobs:
220221
ALLOW_DUPLICATES: ${{ inputs.ALLOW_DUPLICATES }}
221222
ISSUE_ID: ${{ inputs.ISSUE_ID }}
222223
ISSUE_ID_LOOKUP_JSON: ${{ inputs.ISSUE_ID_LOOKUP_JSON }}
224+
CI_TEST_MODE: ${{ vars.CI_TEST_MODE }}
223225
GERRIT_SERVER: ${{ inputs.GERRIT_SERVER }}
224226
GERRIT_SERVER_PORT: ${{ inputs.GERRIT_SERVER_PORT }}
225227
GERRIT_PROJECT: ${{ inputs.GERRIT_PROJECT }}

.github/workflows/testing.yaml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,10 @@ jobs:
3434

3535
- name: "Running local action: ${{ github.repository }}"
3636
uses: ./
37-
env:
38-
G2G_DRYRUN_DISABLE_NETWORK: 'true'
3937
with:
4038
GERRIT_KNOWN_HOSTS: 'dummy-host ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC...'
4139
GERRIT_SSH_PRIVKEY_G2G: 'dummy-key'
42-
DRY_RUN: 'true'
40+
CI_TEST_MODE: 'true'
4341
AUTOMATION_ONLY: 'false'
4442

4543
# yamllint disable-line rule:line-length

action.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ inputs:
6969
description: "Enable CI testing mode; overrides .gitreview, creates orphan commits"
7070
required: false
7171
default: "false"
72+
CI_TEST_MODE:
73+
description: "Enable CI test mode; all tool logic runs but Gerrit network operations are skipped"
74+
required: false
75+
default: "false"
7276
FORCE:
7377
description: "Force PR closure regardless of Gerrit change status (abandoned, etc)"
7478
required: false
@@ -338,6 +342,7 @@ runs:
338342
SYNC_ALL_OPEN_PRS: ${{ env.SYNC_ALL_OPEN_PRS }}
339343
PR_NUMBER: ${{ env.PR_NUMBER }}
340344
G2G_TEST_MODE: "false"
345+
CI_TEST_MODE: ${{ inputs.CI_TEST_MODE }}
341346
run: |
342347
# Run github2gerrit Python CLI
343348
set -euo pipefail

src/github2gerrit/cli.py

Lines changed: 102 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2126,8 +2126,76 @@ def _process() -> None:
21262126
raise converted_error from exc
21272127

21282128
gh = _read_github_context()
2129+
2130+
# --- CI_TEST_MODE: leverage DRY_RUN infrastructure ---
2131+
# CI_TEST_MODE reuses the existing DRY_RUN + G2G_DRYRUN_DISABLE_NETWORK
2132+
# code paths so that all tool logic runs but Gerrit network operations
2133+
# are no-ops. Cleanup tasks (abandoned-PR / Gerrit-change sweeps) are
2134+
# also suppressed because they would hit a non-existent server.
2135+
ci_test_mode = env_bool("CI_TEST_MODE", False)
2136+
2137+
# G2G_TEST_MODE is an internal unit-test flag that short-circuits after
2138+
# config validation (no network at all). It must also suppress the
2139+
# early DNS probe to avoid failures against placeholder hostnames.
2140+
g2g_test_mode = env_bool("G2G_TEST_MODE", False)
2141+
2142+
if ci_test_mode:
2143+
log.info(
2144+
"🧪 CI_TEST_MODE enabled: forcing DRY_RUN=true and "
2145+
"G2G_DRYRUN_DISABLE_NETWORK=true"
2146+
)
2147+
os.environ["DRY_RUN"] = "true"
2148+
os.environ["G2G_DRYRUN_DISABLE_NETWORK"] = "true"
2149+
# Rebuild inputs so the rest of the pipeline sees dry_run=True
2150+
data = _load_effective_inputs()
2151+
2152+
# Display config AFTER CI_TEST_MODE evaluation so the table
2153+
# reflects the actual runtime values (e.g. DRY_RUN forced true).
21292154
_display_effective_config(data, gh)
21302155

2156+
# --- Early Gerrit server DNS validation (fast-fail) ---
2157+
# Validate the Gerrit server BEFORE any PR processing or workspace setup.
2158+
# This prevents wasted work when the server is unreachable or bogus.
2159+
# Runs for both normal and DRY_RUN modes — DRY_RUN still queries Gerrit
2160+
# for read-only checks (preflight probes, cleanup scans) so the server
2161+
# must actually resolve.
2162+
#
2163+
# Skipped when:
2164+
# - CI_TEST_MODE: no real Gerrit server exists for this repository
2165+
# - G2G_TEST_MODE: internal unit-test short-circuit; never reaches Gerrit
2166+
# - G2G_DRYRUN_DISABLE_NETWORK: explicitly opted out of all network probes
2167+
# (consistent with its effect inside _dry_run_preflight)
2168+
dryrun_disable_network = env_bool("G2G_DRYRUN_DISABLE_NETWORK", False)
2169+
2170+
if not ci_test_mode and not g2g_test_mode and not dryrun_disable_network:
2171+
gerrit_host = data.gerrit_server or ""
2172+
if gerrit_host:
2173+
try:
2174+
_validator = Orchestrator(workspace=Path("."))
2175+
_validator.validate_gerrit_server(gerrit_host)
2176+
log.debug(
2177+
"✅ Gerrit server '%s' DNS validation passed",
2178+
gerrit_host.strip(),
2179+
)
2180+
except OrchestratorError as exc:
2181+
log.warning(
2182+
"❌ Gerrit server validation failed for '%s'",
2183+
gerrit_host,
2184+
)
2185+
safe_console_print(
2186+
f"❌ Gerrit server '{gerrit_host}' could not be resolved. "
2187+
f"Cannot proceed without a valid Gerrit server.",
2188+
style="red",
2189+
)
2190+
exit_with_error(
2191+
ExitCode.CONFIGURATION_ERROR,
2192+
message=(
2193+
f"Gerrit server DNS validation failed for "
2194+
f"'{gerrit_host}'"
2195+
),
2196+
exception=exc,
2197+
)
2198+
21312199
# Detect PR operation mode for routing
21322200
operation_mode = gh.get_operation_mode()
21332201
if operation_mode != models.PROperationMode.UNKNOWN:
@@ -2164,8 +2232,10 @@ def _process() -> None:
21642232
)
21652233

21662234
# First, abandon the specific Gerrit change for this closed PR
2235+
# Skip in CI_TEST_MODE: no Gerrit server to query
21672236
if (
2168-
gh.pr_number
2237+
not ci_test_mode
2238+
and gh.pr_number
21692239
and data.gerrit_server
21702240
and data.gerrit_project
21712241
and gh.repository
@@ -2209,7 +2279,8 @@ def _process() -> None:
22092279
)
22102280

22112281
# Run abandoned PR cleanup if enabled
2212-
if FORCE_ABANDONED_CLEANUP:
2282+
# Skip in CI_TEST_MODE: no Gerrit server to query
2283+
if FORCE_ABANDONED_CLEANUP and not ci_test_mode:
22132284
try:
22142285
log.debug("Running abandoned PR cleanup...")
22152286
if gh.repository and "/" in gh.repository:
@@ -2225,7 +2296,8 @@ def _process() -> None:
22252296
log.warning("Abandoned PR cleanup failed: %s", exc)
22262297

22272298
# Run Gerrit cleanup if enabled
2228-
if FORCE_GERRIT_CLEANUP:
2299+
# Skip in CI_TEST_MODE: no Gerrit server to query
2300+
if FORCE_GERRIT_CLEANUP and not ci_test_mode:
22292301
try:
22302302
log.debug("Running Gerrit cleanup for closed GitHub PRs...")
22312303
if data.gerrit_server and data.gerrit_project:
@@ -2285,7 +2357,8 @@ def _process() -> None:
22852357
# Run cleanup tasks for Gerrit events and legacy G2G_GERRIT_CHANGE_URL
22862358
if gerrit_event_change_url or gerrit_change_url:
22872359
# Run abandoned PR cleanup if enabled
2288-
if FORCE_ABANDONED_CLEANUP:
2360+
# Skip in CI_TEST_MODE: no Gerrit server to query
2361+
if FORCE_ABANDONED_CLEANUP and not ci_test_mode:
22892362
try:
22902363
log.debug("Running abandoned PR cleanup...")
22912364
if gh.repository and "/" in gh.repository:
@@ -2301,7 +2374,8 @@ def _process() -> None:
23012374
log.warning("Abandoned PR cleanup failed: %s", exc)
23022375

23032376
# Run Gerrit cleanup if enabled
2304-
if FORCE_GERRIT_CLEANUP:
2377+
# Skip in CI_TEST_MODE: no Gerrit server to query
2378+
if FORCE_GERRIT_CLEANUP and not ci_test_mode:
23052379
try:
23062380
log.debug("Running Gerrit cleanup for closed GitHub PRs...")
23072381
if data.gerrit_server and data.gerrit_project:
@@ -2322,7 +2396,8 @@ def _process() -> None:
23222396
_process_close_merged_prs(data, gh)
23232397

23242398
# Run abandoned PR cleanup if enabled
2325-
if FORCE_ABANDONED_CLEANUP:
2399+
# Skip in CI_TEST_MODE: no Gerrit server to query
2400+
if FORCE_ABANDONED_CLEANUP and not ci_test_mode:
23262401
try:
23272402
log.debug("Running abandoned PR cleanup...")
23282403
if gh.repository and "/" in gh.repository:
@@ -2338,7 +2413,8 @@ def _process() -> None:
23382413
log.warning("Abandoned PR cleanup failed: %s", exc)
23392414

23402415
# Run Gerrit cleanup if enabled
2341-
if FORCE_GERRIT_CLEANUP:
2416+
# Skip in CI_TEST_MODE: no Gerrit server to query
2417+
if FORCE_GERRIT_CLEANUP and not ci_test_mode:
23422418
try:
23432419
log.info("Running Gerrit cleanup for closed GitHub PRs...")
23442420
if data.gerrit_server and data.gerrit_project:
@@ -2383,7 +2459,8 @@ def _process() -> None:
23832459
log.debug("Processing completed ✅")
23842460

23852461
# Run abandoned PR cleanup if enabled
2386-
if FORCE_ABANDONED_CLEANUP:
2462+
# Skip in CI_TEST_MODE: no Gerrit server to query
2463+
if FORCE_ABANDONED_CLEANUP and not ci_test_mode:
23872464
try:
23882465
log.debug("Running abandoned PR cleanup...")
23892466
if gh.repository and "/" in gh.repository:
@@ -2399,7 +2476,8 @@ def _process() -> None:
23992476
log.warning("Abandoned PR cleanup failed: %s", exc)
24002477

24012478
# Run Gerrit cleanup if enabled
2402-
if FORCE_GERRIT_CLEANUP:
2479+
# Skip in CI_TEST_MODE: no Gerrit server to query
2480+
if FORCE_GERRIT_CLEANUP and not ci_test_mode:
24032481
try:
24042482
log.info("Running Gerrit cleanup for closed GitHub PRs...")
24052483
if data.gerrit_server and data.gerrit_project:
@@ -2567,7 +2645,8 @@ def _process() -> None:
25672645
pipeline_success, result = _process_single(data, gh, progress_tracker)
25682646

25692647
# Run abandoned PR cleanup if enabled and pipeline was successful
2570-
if pipeline_success and FORCE_ABANDONED_CLEANUP:
2648+
# Skip in CI_TEST_MODE: no Gerrit server to query
2649+
if pipeline_success and FORCE_ABANDONED_CLEANUP and not ci_test_mode:
25712650
try:
25722651
log.debug("Running abandoned PR cleanup...")
25732652
# Extract owner and repo from gh.repository (format: "owner/repo")
@@ -2585,7 +2664,8 @@ def _process() -> None:
25852664
log.warning("Abandoned PR cleanup failed: %s", exc)
25862665

25872666
# Run Gerrit cleanup if enabled and pipeline was successful
2588-
if pipeline_success and FORCE_GERRIT_CLEANUP:
2667+
# Skip in CI_TEST_MODE: no Gerrit server to query
2668+
if pipeline_success and FORCE_GERRIT_CLEANUP and not ci_test_mode:
25892669
try:
25902670
log.debug("Running Gerrit cleanup for closed GitHub PRs...")
25912671
if data.gerrit_server and data.gerrit_project:
@@ -2612,10 +2692,11 @@ def _process() -> None:
26122692
# Show summary after progress tracker is stopped
26132693
if show_progress and RICH_AVAILABLE:
26142694
summary = progress_tracker.get_summary() if progress_tracker else {}
2695+
print()
26152696
safe_console_print(
2616-
"\n✅ Operation completed!"
2697+
"✅ Operation completed!"
26172698
if pipeline_success
2618-
else "\n❌ Operation failed!",
2699+
else "❌ Operation failed!",
26192700
style="green" if pipeline_success else "red",
26202701
)
26212702
safe_console_print(
@@ -2864,6 +2945,9 @@ def _get_ssh_agent_status() -> str:
28642945

28652946
def _display_effective_config(data: Inputs, gh: GitHubContext) -> None:
28662947
"""Display effective configuration in a formatted table."""
2948+
# Use env_bool for consistent boolean parsing across the codebase
2949+
ci_test_mode_enabled = env_bool("CI_TEST_MODE", False)
2950+
28672951
# Detect mode and display prominently
28682952
github_mode = _is_github_mode()
28692953
mode_label = "GITHUB_MODE" if github_mode else "CLI_MODE"
@@ -2924,6 +3008,11 @@ def _display_effective_config(data: Inputs, gh: GitHubContext) -> None:
29243008
# Mode first - always show
29253009
config_info[mode_label] = mode_description
29263010

3011+
# Show CI_TEST_MODE regardless of operation mode so logs always
3012+
# indicate when the run is using test infrastructure.
3013+
if ci_test_mode_enabled:
3014+
config_info["CI_TEST_MODE"] = "🧪"
3015+
29273016
if is_closing_pr_mode:
29283017
# In PR closing mode, only show minimal relevant config
29293018
if data.dry_run:

src/github2gerrit/core.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1658,6 +1658,38 @@ def __init__(
16581658
# Public API
16591659
# ---------------
16601660

1661+
def validate_gerrit_server(self, gerrit_host: str | None) -> None:
1662+
"""Validate that the Gerrit server hostname can be resolved via DNS.
1663+
1664+
This provides a fast-fail mechanism to catch invalid or unreachable
1665+
Gerrit servers early, before any work is done.
1666+
1667+
Args:
1668+
gerrit_host: The Gerrit server hostname to validate.
1669+
May be ``None`` or empty, in which case an error is raised.
1670+
1671+
Raises:
1672+
OrchestratorError: If the hostname is empty/None or cannot
1673+
be resolved.
1674+
"""
1675+
if not gerrit_host or not gerrit_host.strip():
1676+
raise OrchestratorError(_MSG_MISSING_GERRIT_SERVER)
1677+
1678+
host = gerrit_host.strip()
1679+
try:
1680+
socket.getaddrinfo(host, None)
1681+
log.debug("DNS resolution for Gerrit host '%s' succeeded", host)
1682+
except socket.gaierror as exc:
1683+
log.warning(
1684+
"Gerrit server '%s' could not be resolved via DNS. "
1685+
"This typically means either the server hostname is "
1686+
"incorrect or there is no Gerrit server associated "
1687+
"with this repository.",
1688+
host,
1689+
)
1690+
msg = f"DNS resolution failed for '{host}'"
1691+
raise OrchestratorError(msg) from exc
1692+
16611693
def execute(
16621694
self,
16631695
inputs: Inputs,

tests/test_cli.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ def test_no_pr_context_exits_2(tmp_path: Path) -> None:
180180
env["GITHUB_EVENT_NAME"] = "workflow_dispatch"
181181
# Disable test mode to ensure non-zero exit on missing PR context
182182
env.pop("G2G_TEST_MODE", None)
183+
# Skip early DNS validation — this test targets PR context validation,
184+
# not network reachability of the derived placeholder hostname
185+
env["G2G_DRYRUN_DISABLE_NETWORK"] = "true"
183186
# Force non-bulk path to avoid GitHub API token requirement
184187
env["SYNC_ALL_OPEN_PRS"] = "false"
185188
# Set PR_NUMBER to empty to trigger validation error
@@ -238,6 +241,9 @@ def test_validation_fails_when_no_organization_and_missing_gerrit_params(
238241
env.pop("GITHUB_EVENT_NAME", None)
239242
# Disable test mode to ensure validation errors are properly caught
240243
env.pop("G2G_TEST_MODE", None)
244+
# Skip early DNS validation — this test targets input validation,
245+
# not network reachability of placeholder hostnames
246+
env["G2G_DRYRUN_DISABLE_NETWORK"] = "true"
241247
# Use empty config file to avoid interference from real config
242248
empty_config = tmp_path / "config.txt"
243249
empty_config.write_text("", encoding="utf-8")
@@ -294,6 +300,9 @@ def test_validation_local_cli_requires_derivation_disabled(
294300
env["G2G_ENABLE_DERIVATION"] = "false" # Explicitly disable derivation
295301
# Disable test mode to see validation behavior
296302
env.pop("G2G_TEST_MODE", None)
303+
# Skip early DNS validation — this test targets derivation validation,
304+
# not network reachability of placeholder hostnames
305+
env["G2G_DRYRUN_DISABLE_NETWORK"] = "true"
297306
# Use empty config file to avoid interference from real config
298307
empty_config = tmp_path / "config.txt"
299308
empty_config.write_text("", encoding="utf-8")

tests/test_cli_outputs_file.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@ def _base_env_with_event(tmp_path: Path) -> dict[str, str]:
111111
"GITHUB_HEAD_REF": "feature",
112112
# Ensure real execution path (not short-circuited)
113113
"G2G_TEST_MODE": "false",
114+
# Skip early DNS validation — tests mock the Gerrit layer,
115+
# the derived placeholder hostname is not resolvable
116+
"G2G_DRYRUN_DISABLE_NETWORK": "true",
114117
# Disable automation-only mode for tests
115118
"AUTOMATION_ONLY": "false",
116119
}
@@ -216,6 +219,8 @@ def test_multi_pr_url_mode_writes_aggregated_outputs(
216219
"GITHUB_TOKEN": "dummy",
217220
"DRY_RUN": "true",
218221
"G2G_TEST_MODE": "false",
222+
# Skip early DNS validation — tests mock the Gerrit layer
223+
"G2G_DRYRUN_DISABLE_NETWORK": "true",
219224
"GITHUB_OUTPUT": str(outputs_file),
220225
}
221226

tests/test_cli_url_and_dryrun.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ def _base_env() -> dict[str, str]:
3131
# Token not needed since we mock build_client for bulk mode
3232
"GITHUB_TOKEN": "dummy",
3333
"DRY_RUN": "true",
34+
# Skip early DNS validation — tests mock the Gerrit layer,
35+
# the derived placeholder hostname is not resolvable
36+
"G2G_DRYRUN_DISABLE_NETWORK": "true",
3437
# Disable automation-only mode for tests
3538
"AUTOMATION_ONLY": "false",
3639
}

0 commit comments

Comments
 (0)