ci: gate the cross-module dependency graph on an allowlist - #1881
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe change adds a Bash validator for Terratest module dependency allowlists. It checks direct and indirect dependency consistency, then runs in the v2 checks and release pre-flight workflows. ChangesModule dependency validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant check-module-deps.sh
participant GoModFiles
GitHubActions->>check-module-deps.sh: Run dependency validation
check-module-deps.sh->>GoModFiles: Read module dependency declarations
GoModFiles-->>check-module-deps.sh: Return direct and indirect requirements
check-module-deps.sh-->>GitHubActions: Return validation status
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
Follow-up to #1875. Nothing currently stops the dependency cleanup from being undone.
Gap
check-acyclic-deps.shenforces layering only.awsandk8sare both tier 3, sok8simportingawsfor one EC2 call never violated it, and it survived until a user noticed that depending onk8spulled in 23 AWS service SDKs.The second half is staleness. When
k8sdroppedaws,helmkept it as an indirect require, becausego-mod-tidy-checkruns at the repo root and diffs only the rootgo.modandgo.sum.helmcarried 72aws-sdk-go-v2go.sumentries for code no module imported.Change
scripts/check-module-deps.sh, wired intov2-checks.ymlandcreate-release.yml:ALLOWED_EDGES, so adding one is deliberate and reviewed.The allowlist is seeded from the current graph: 26 edges, each with a note where the reason is not obvious.
Verification
Both historical regressions were replayed and both fail the check:
Clean on current main. shellcheck clean.
Summary by CodeRabbit
Bug Fixes
Chores