Skip to content

Commit c189d00

Browse files
committed
ci: gate the cross-module dependency graph on an allowlist
check-acyclic-deps.sh enforces layering, which is necessary but not sufficient. aws and k8s are both tier 3, so k8s importing aws for a single EC2 call never violated the tier rule, and it survived until a user reported that depending on k8s pulled in 23 AWS service SDKs (#1875). check-module-deps.sh requires every cross-module edge to be listed on purpose, so adding one is a deliberate act with a reviewer attached. It also catches the second half of that problem. When k8s stopped requiring aws, helm kept aws as a stale indirect, because nothing tidies submodule go.mod files: go-mod-tidy-check runs at the repo root and diffs only the root go.mod and go.sum. helm carried 72 aws-sdk-go-v2 go.sum entries for code no module imported. An indirect require unreachable through the declared direct graph is now an error. Both regressions were replayed against the script and both fail it.
1 parent 5eea79a commit c189d00

3 files changed

Lines changed: 172 additions & 0 deletions

File tree

.github/workflows/create-release.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ jobs:
4545
ref: ${{ inputs.ref }}
4646
persist-credentials: false
4747
- run: bash scripts/check-acyclic-deps.sh
48+
- run: bash scripts/check-module-deps.sh
4849
- run: bash scripts/check-single-source.sh
4950
- run: bash scripts/check-siv-placement.sh
5051

.github/workflows/v2-checks.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ jobs:
2323
persist-credentials: false
2424
- run: bash scripts/check-acyclic-deps.sh
2525

26+
module-deps:
27+
name: Cross-module dependency allowlist
28+
runs-on: ubuntu-latest
29+
steps:
30+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
31+
with:
32+
persist-credentials: false
33+
- run: bash scripts/check-module-deps.sh
34+
2635
single-source:
2736
name: Single source of truth per package path
2837
runs-on: ubuntu-latest

scripts/check-module-deps.sh

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
#!/usr/bin/env bash
2+
#
3+
# Checks the cross-module dependency graph against an explicit allowlist, and catches stale indirect requires.
4+
#
5+
# check-acyclic-deps.sh enforces layering: a module may not import from a strictly higher tier. That is necessary but
6+
# not sufficient. aws and k8s are both tier 3, so k8s importing aws for a single EC2 call never violated the tier
7+
# rule, and it went unnoticed until a user reported that depending on k8s pulled in 23 AWS service SDKs (#1875).
8+
#
9+
# This script enforces the complementary rule: every cross-module edge is listed below on purpose. Adding one is a
10+
# deliberate act with a reviewer attached, not something that happens because an import was convenient.
11+
#
12+
# It also catches the second half of that problem. When k8s stopped requiring aws, helm kept carrying aws as an
13+
# indirect require, because nothing tidies submodule go.mod files: the go-mod-tidy-check workflow runs at the repo
14+
# root and diffs only the root go.mod and go.sum. helm shipped 72 aws-sdk-go-v2 entries in its go.sum for code no
15+
# module imported. An indirect require that is not reachable through the declared direct graph is stale.
16+
#
17+
# No -e: accumulate every violation and report them all, rather than aborting on the first.
18+
set -uo pipefail
19+
20+
# Permitted direct cross-module requires, as "importer:importee".
21+
#
22+
# Test-only edges still appear here, because a test dependency is a real entry in go.mod. Keeping the graph small is
23+
# the point of the v2 split, so treat an addition as a design decision: prefer injecting behaviour over taking the
24+
# dependency. modules/k8s/kubectl_options.go NodePublicIPLookup is the worked example.
25+
ALLOWED_EDGES=(
26+
"aws:core"
27+
"aws:ssh" # Ec2Keypair embeds ssh.KeyPair; SCP helpers
28+
"azure:core"
29+
"database:core"
30+
"dnshelper:core"
31+
"docker:core"
32+
"docker:httphelper" # test only
33+
"gcp:core"
34+
"helm:core"
35+
"helm:httphelper" # test only
36+
"helm:k8s"
37+
"httphelper:core"
38+
"k8s:core"
39+
"k8s:httphelper" # test only
40+
"opa:core"
41+
"packer:core"
42+
"ssh:core"
43+
"terraform:core"
44+
"terraform:httphelper" # test only
45+
"terraform:opa"
46+
"terraform:ssh" # Options.SshAgent
47+
"terragrunt:core"
48+
"terragrunt:terraform"
49+
"teststructure:core"
50+
"teststructure:opa"
51+
"teststructure:terraform"
52+
)
53+
54+
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
55+
modules_dir="${repo_root}/modules"
56+
57+
if [ ! -d "$modules_dir" ]; then
58+
echo "check-module-deps: no modules/ directory; nothing to check"
59+
exit 0
60+
fi
61+
62+
is_allowed() {
63+
local edge="$1"
64+
for allowed in "${ALLOWED_EDGES[@]}"; do
65+
# Strip the trailing comment before comparing.
66+
[ "${allowed%%[[:space:]]*}" = "$edge" ] && return 0
67+
done
68+
return 1
69+
}
70+
71+
# terratest_requires <go.mod> <direct|indirect> prints one module name per line.
72+
terratest_requires() {
73+
local gomod="$1" kind="$2"
74+
if [ "$kind" = "direct" ]; then
75+
grep 'gruntwork-io/terratest/modules/' "$gomod" | grep -v '^module' | grep -v '// indirect' || true
76+
else
77+
grep 'gruntwork-io/terratest/modules/' "$gomod" | grep -v '^module' | grep '// indirect' || true
78+
fi | sed -E 's|.*/modules/([a-z0-9]+)/v2.*|\1|' | sort -u
79+
}
80+
81+
declare -A DIRECT_EDGES=()
82+
modules=()
83+
84+
for dir in "$modules_dir"/*/; do
85+
module="$(basename "$dir")"
86+
[ -f "${dir}go.mod" ] || continue
87+
modules+=("$module")
88+
DIRECT_EDGES[$module]="$(terratest_requires "${dir}go.mod" direct | tr '\n' ' ')"
89+
done
90+
91+
exit_code=0
92+
seen_edges=()
93+
94+
# 1. Every declared direct edge must be allowlisted.
95+
for module in "${modules[@]}"; do
96+
for importee in ${DIRECT_EDGES[$module]}; do
97+
edge="${module}:${importee}"
98+
seen_edges+=("$edge")
99+
100+
if ! is_allowed "$edge"; then
101+
echo "::error file=modules/${module}/go.mod::undeclared cross-module dependency '${edge}'. If this is intended, add it to ALLOWED_EDGES in scripts/check-module-deps.sh with a short note saying why. Prefer injecting behaviour over taking the dependency."
102+
exit_code=1
103+
fi
104+
done
105+
done
106+
107+
# 2. Every allowlisted edge must still exist, so the list documents the real graph rather than history.
108+
for allowed in "${ALLOWED_EDGES[@]}"; do
109+
edge="${allowed%%[[:space:]]*}"
110+
found=0
111+
112+
for seen in "${seen_edges[@]}"; do
113+
[ "$seen" = "$edge" ] && found=1 && break
114+
done
115+
116+
if [ "$found" -eq 0 ]; then
117+
echo "::error file=scripts/check-module-deps.sh::allowlisted edge '${edge}' no longer exists. Remove it from ALLOWED_EDGES so the list keeps describing the real graph."
118+
exit_code=1
119+
fi
120+
done
121+
122+
# 3. Every indirect terratest require must be reachable through the declared direct graph. An unreachable one is a
123+
# stale entry left behind when some other module dropped the dependency, and it drags that module's whole
124+
# transitive tree into every consumer's go.sum.
125+
reachable_from() {
126+
local start="$1"
127+
local -A visited=()
128+
local queue=("$start") current
129+
130+
while [ ${#queue[@]} -gt 0 ]; do
131+
current="${queue[0]}"
132+
queue=("${queue[@]:1}")
133+
[ -n "${visited[$current]+set}" ] && continue
134+
visited[$current]=1
135+
136+
for next in ${DIRECT_EDGES[$current]:-}; do
137+
queue+=("$next")
138+
done
139+
done
140+
141+
echo "${!visited[@]}"
142+
}
143+
144+
for module in "${modules[@]}"; do
145+
indirect="$(terratest_requires "${modules_dir}/${module}/go.mod" indirect | tr '\n' ' ')"
146+
[ -z "$indirect" ] && continue
147+
148+
reachable=" $(reachable_from "$module") "
149+
150+
for importee in $indirect; do
151+
if [[ "$reachable" != *" ${importee} "* ]]; then
152+
echo "::error file=modules/${module}/go.mod::stale indirect require '${importee}': no module in ${module}'s dependency graph requires it any more. Run 'go mod tidy' for this module and commit the result."
153+
exit_code=1
154+
fi
155+
done
156+
done
157+
158+
if [ "$exit_code" -eq 0 ]; then
159+
echo "module-deps check: OK (${#seen_edges[@]} cross-module edges, all allowlisted, no stale indirects)"
160+
fi
161+
162+
exit "$exit_code"

0 commit comments

Comments
 (0)