Skip to content

Commit 15ec9f0

Browse files
[v3-3-test] Catch lang-SDK Go example module drift before it reaches main (#70568) (#70624)
* Catch lang-SDK Go example module drift before it reaches main kubernetes-tests/lang_sdk/go_example is a separate Go module that resolves the SDK through a `replace` onto ../../../go-sdk, so it carries its own copy of the SDK's indirect requirements. Nothing re-tidies it when a dependency moves inside /go-sdk, and Dependabot bumps exactly one module per PR. The blast radius is what makes this worth guarding. Go refuses to build an inconsistent module, so once such a bump merges, "Kubernetes tests / K8S Lang-SDK" fails at the Build Go bundle step on every pull request until someone notices and tidies the module by hand — not just on the PR that caused it. Dependabot security updates do not consult .github/dependabot.yml, so no per-directory configuration prevents this, and a second Dependabot PR for the example module would merge at a different time and leave main red in between. The drift has to fail the bump PR itself. The check is `go mod tidy -diff` in the example module: it asks exactly the question the failing CI step asks, never writes to the working tree, and exits non-zero when the module is untidy. * Let prek provide the Go toolchain for the tidy check Static checks run on a runner whose preinstalled toolchains are deleted to free disk space before prek starts, so the check could never find `go` there and failed on every run. Asking prek for the toolchain is how the Go SDK's own tidy hook already gets one, and it pins the same version everywhere. (cherry picked from commit bce20ff) Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
1 parent bacf9cf commit 15ec9f0

3 files changed

Lines changed: 242 additions & 0 deletions

File tree

.pre-commit-config.yaml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,23 @@ repos:
284284
^\.pre-commit-config\.yaml$
285285
pass_filenames: false
286286
require_serial: true
287+
- id: check-go-example-mod-tidy
288+
name: Check lang-SDK Go example module is tidy against the Go SDK
289+
entry: ./scripts/ci/prek/check_go_example_mod_tidy.py
290+
# golang so prek provisions the toolchain the check needs: static checks run on a
291+
# runner whose preinstalled toolchains are wiped to free disk space, and the SDK's
292+
# own `go mod tidy` hook gets its Go the same way.
293+
language: golang
294+
# The example module keeps its own copy of the SDK's indirect requirements
295+
# (it resolves the SDK through a `replace`), so a dependency moving in
296+
# go-sdk/go.mod leaves it stale. Watching both modules' manifests is enough:
297+
# any new requirement in the SDK necessarily lands in go-sdk/go.mod first.
298+
files: >
299+
(?x)
300+
^go-sdk/go\.(mod|sum)$|
301+
^kubernetes-tests/lang_sdk/go_example/go\.(mod|sum)$
302+
pass_filenames: false
303+
require_serial: true
287304
- id: check-partition-mapper-defaults-in-sync
288305
name: Check partition-mapper core/SDK sync (FanOutMapper table + SegmentWindow/FixedKeyMapper)
289306
entry: ./scripts/ci/prek/check_partition_mapper_defaults_in_sync.py
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
#!/usr/bin/env python3
2+
# Licensed to the Apache Software Foundation (ASF) under one
3+
# or more contributor license agreements. See the NOTICE file
4+
# distributed with this work for additional information
5+
# regarding copyright ownership. The ASF licenses this file
6+
# to you under the Apache License, Version 2.0 (the
7+
# "License"); you may not use this file except in compliance
8+
# with the License. You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# Unless required by applicable law or agreed to in writing,
13+
# software distributed under the License is distributed on an
14+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
# KIND, either express or implied. See the License for the
16+
# specific language governing permissions and limitations
17+
# under the License.
18+
"""
19+
Keep the lang-SDK Go example module tidy against the Go SDK.
20+
21+
``kubernetes-tests/lang_sdk/go_example`` is a **separate** Go module that
22+
resolves the SDK from the in-repo sources::
23+
24+
replace github.com/apache/airflow/go-sdk => ../../../go-sdk
25+
26+
Because of that ``replace`` it carries its own copy of the SDK's indirect
27+
requirements. Nothing re-tidies it when a dependency moves inside
28+
``/go-sdk`` — and Dependabot bumps exactly one module per PR. The example
29+
module is then left pinning the old versions, Go refuses to build an
30+
inconsistent module, and ``Kubernetes tests / K8S Lang-SDK`` fails at the
31+
"Build Go bundle" step::
32+
33+
go: updates to go.mod needed; to update it:
34+
go mod tidy
35+
36+
The damage is not limited to the bump PR: once it merges, that job is red on
37+
*every* pull request until someone notices and tidies the example module by
38+
hand. This happened with #70226 (``google.golang.org/grpc`` 1.79.3 -> 1.82.1
39+
in ``/go-sdk`` only) and was cleaned up after the fact by #70561.
40+
41+
Note that Dependabot **security** updates do not consult
42+
``.github/dependabot.yml`` at all, so no amount of per-directory config
43+
prevents this — and a second Dependabot PR for the example module would merge
44+
at a different time, leaving ``main`` red in between. The drift has to fail
45+
the bump PR itself, which is what this check does.
46+
47+
The check is ``go mod tidy -diff`` in the example module: it is the exact
48+
question the failing CI step asks, it never writes to the working tree, and it
49+
exits non-zero when the module is untidy.
50+
51+
Run from the repo root:
52+
53+
uv run --project scripts python scripts/ci/prek/check_go_example_mod_tidy.py
54+
55+
Exits 0 if the example module is tidy, 1 otherwise.
56+
"""
57+
58+
from __future__ import annotations
59+
60+
import os
61+
import pathlib
62+
import shutil
63+
import subprocess
64+
import sys
65+
66+
REPO_ROOT = pathlib.Path(__file__).resolve().parents[3]
67+
EXAMPLE_MODULE = pathlib.Path("kubernetes-tests/lang_sdk/go_example")
68+
GO_SDK_MODULE = pathlib.Path("go-sdk")
69+
70+
71+
def run_tidy_diff(module_dir: pathlib.Path, go_binary: str = "go") -> tuple[int, str]:
72+
"""Ask Go whether ``module_dir`` is tidy. Returns ``(returncode, combined_output)``."""
73+
completed = subprocess.run(
74+
[go_binary, "mod", "tidy", "-diff"],
75+
cwd=module_dir,
76+
capture_output=True,
77+
text=True,
78+
check=False,
79+
)
80+
return completed.returncode, (completed.stdout + completed.stderr).strip()
81+
82+
83+
def format_report(returncode: int, output: str) -> tuple[int, str]:
84+
"""Turn a ``go mod tidy -diff`` result into ``(exit_code, report)``."""
85+
if returncode == 0:
86+
return 0, f"OK: {EXAMPLE_MODULE} is tidy against {GO_SDK_MODULE}."
87+
lines = [
88+
f"ERROR: {EXAMPLE_MODULE} is not tidy.",
89+
"",
90+
f"It is a separate Go module that resolves the SDK via a `replace` onto {GO_SDK_MODULE},",
91+
"so it keeps its own copy of the SDK's indirect requirements. A dependency moved in",
92+
f"{GO_SDK_MODULE} without this module being re-tidied, which breaks the",
93+
"'Kubernetes tests / K8S Lang-SDK' bundle build on every pull request once merged.",
94+
"",
95+
"Fix it in this PR by running:",
96+
"",
97+
f" (cd {EXAMPLE_MODULE} && go mod tidy)",
98+
"",
99+
"and committing the resulting go.mod / go.sum changes.",
100+
"",
101+
"`go mod tidy -diff` reported:",
102+
"",
103+
output or "(no output)",
104+
]
105+
return 1, "\n".join(lines)
106+
107+
108+
def main() -> int:
109+
module_dir = REPO_ROOT / EXAMPLE_MODULE
110+
if not (module_dir / "go.mod").is_file():
111+
print(f"ERROR: {EXAMPLE_MODULE}/go.mod not found — has the example module moved?")
112+
return 1
113+
if shutil.which("go") is None:
114+
if os.environ.get("CI"):
115+
print("ERROR: `go` is not on PATH but this is a CI run — the toolchain is required here.")
116+
return 1
117+
print(f"SKIPPED: `go` is not on PATH, cannot verify that {EXAMPLE_MODULE} is tidy.")
118+
return 0
119+
returncode, output = run_tidy_diff(module_dir)
120+
exit_code, report = format_report(returncode, output)
121+
print(report)
122+
return exit_code
123+
124+
125+
if __name__ == "__main__":
126+
sys.exit(main())
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
from __future__ import annotations
18+
19+
import subprocess
20+
from unittest import mock
21+
22+
import check_go_example_mod_tidy as checker
23+
import pytest
24+
25+
# Trimmed to the shape that matters: the drift #70226 introduced and #70561 cleaned up.
26+
GRPC_DRIFT_DIFF = """\
27+
diff current/go.mod tidy/go.mod
28+
--- current/go.mod
29+
+++ tidy/go.mod
30+
@@ -37,9 +37,9 @@
31+
- google.golang.org/grpc v1.79.3 // indirect
32+
+ google.golang.org/grpc v1.82.1 // indirect
33+
"""
34+
35+
36+
def test_tidy_module_passes():
37+
exit_code, report = checker.format_report(0, "")
38+
39+
assert exit_code == 0
40+
assert "is tidy" in report
41+
42+
43+
def test_untidy_module_fails_with_the_fix_command_and_the_diff():
44+
exit_code, report = checker.format_report(1, GRPC_DRIFT_DIFF)
45+
46+
assert exit_code == 1
47+
assert "is not tidy" in report
48+
assert "(cd kubernetes-tests/lang_sdk/go_example && go mod tidy)" in report
49+
# The reason the contributor cares: this is what turns K8S Lang-SDK red for everyone.
50+
assert "K8S Lang-SDK" in report
51+
assert "google.golang.org/grpc v1.82.1" in report
52+
53+
54+
def test_untidy_module_without_diff_output_still_reports():
55+
exit_code, report = checker.format_report(1, "")
56+
57+
assert exit_code == 1
58+
assert "(no output)" in report
59+
60+
61+
@mock.patch("check_go_example_mod_tidy.subprocess.run", autospec=True)
62+
def test_run_tidy_diff_never_writes_to_the_working_tree(mock_run, tmp_path):
63+
mock_run.return_value = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="")
64+
65+
checker.run_tidy_diff(tmp_path)
66+
67+
args = mock_run.call_args.args[0]
68+
assert args == ["go", "mod", "tidy", "-diff"]
69+
assert mock_run.call_args.kwargs["cwd"] == tmp_path
70+
71+
72+
@mock.patch("check_go_example_mod_tidy.subprocess.run", autospec=True)
73+
def test_run_tidy_diff_combines_stdout_and_stderr(mock_run, tmp_path):
74+
mock_run.return_value = subprocess.CompletedProcess(
75+
args=[], returncode=1, stdout="diff current/go.mod tidy/go.mod\n", stderr="go: downloading\n"
76+
)
77+
78+
returncode, output = checker.run_tidy_diff(tmp_path)
79+
80+
assert returncode == 1
81+
assert "diff current/go.mod tidy/go.mod" in output
82+
assert "go: downloading" in output
83+
84+
85+
@pytest.mark.parametrize(
86+
("ci_env", "expected_exit", "expected_text"),
87+
[
88+
pytest.param({"CI": "true"}, 1, "this is a CI run", id="ci-fails-loudly"),
89+
pytest.param({}, 0, "SKIPPED", id="local-skips"),
90+
],
91+
)
92+
@mock.patch("check_go_example_mod_tidy.shutil.which", return_value=None)
93+
def test_missing_go_toolchain(mock_which, ci_env, expected_exit, expected_text, monkeypatch, capsys):
94+
monkeypatch.delenv("CI", raising=False)
95+
for key, value in ci_env.items():
96+
monkeypatch.setenv(key, value)
97+
98+
assert checker.main() == expected_exit
99+
assert expected_text in capsys.readouterr().out

0 commit comments

Comments
 (0)