feat(tools): add go-dependency-analyzer skill#20247
Open
janisz wants to merge 9 commits into
Open
Conversation
User request: Create a Go dependency analyzer skill for StackRox codebase to support CVE triage, security analysis, and dependency impact assessment. Key features: - Branching workflow for direct vs transitive dependencies - Wrapper pattern detection (e.g., zap via pkg/logging) - Mandatory goda visualization for dependency trees - Replace directive tracking for StackRox forks - Team assignment recommendations based on component usage Tested with pgx, docker, prometheus, zap, protoc-gen-validate, and kubevirt.io/containerized-data-importer-api dependencies. Skill designed for team sharing via repository .claude/skills/ directory. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
janisz
changed the title
User feedback: Component-to-team mapping should be based on .github/CODEOWNERS, remove redundant "Recommended Action" sections, and handle dependencies that can be both direct and indirect. Changes: - Use .github/CODEOWNERS for team assignment instead of hardcoded mapping - Remove "Recommended Action" checkbox sections from all report templates - Add nuance for dependencies that can be both direct and indirect - Update transitive dependency team assignment to target teams using intermediate packages - Updated all 7 examples to reflect new team assignment approach Reasoning: - CODEOWNERS is the source of truth for code ownership - "Recommended Action" sections were redundant and not actionable - Dependencies can appear as both direct and transitive simultaneously - Transitive deps should be assigned to teams that introduced them via intermediate dependencies Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Simplify skill review findings: - Fix typo: missing space after 'on' in Example 6 - Standardize phrasing: 'From CODEOWNERS' across all examples - Simplify verbose explanations while keeping essential info - Improve readability and consistency Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
🚀 Build Images ReadyImages are ready for commit c2b236d. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-793-gc2b236da67 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #20247 +/- ##
==========================================
+ Coverage 49.79% 49.83% +0.04%
==========================================
Files 2770 2773 +3
Lines 210223 210736 +513
==========================================
+ Hits 104673 105018 +345
- Misses 97829 97957 +128
- Partials 7721 7761 +40
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
User request: "make this skill compact... to bare minimum to make it work correctly" Achieved 33.4% reduction (760 → 506 lines) while preserving all critical functionality. **Removed (254 lines total):** - Advanced Techniques section (66 lines) - complete duplication of Steps 2-7 - Example 3: yaml.v3 (23 lines) - redundant with test-only pattern - Example 4: Go deprecation (28 lines) - out of scope for dependency analysis - Example 7: Transitive dependency (52 lines) - duplicates Step 2A workflow - Example 3 "Key Findings" section (15 lines) - repeated table data - CODEOWNERS examples (8 lines) - users can read actual file - Verbose explanations in Steps 4-5 (62 lines) - consolidated to essentials **Streamlined:** - Step 4: Wrapper detection workflow (43 → 14 lines) - Step 5: Function analysis (35 → 7 lines) - Removed redundant prose, consolidated bash commands **Preserved:** - All critical branching logic (direct vs transitive) - MANDATORY commands (go mod why, goda graph) - Wrapper pattern detection workflow - Both report templates (direct and transitive) - CODEOWNERS team assignment - All essential troubleshooting Skill maintains full functionality, faster to read and understand. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…yzer User request: add line about how to install goda Added Prerequisites section before Instructions to make tool requirement clear upfront, rather than only mentioning it in Troubleshooting. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
4 tasks
rhybrillou
approved these changes
May 6, 2026
Comment on lines
+43
to
+52
| ```bash | ||
| # Check if dependency is in go.mod (direct or indirect) | ||
| grep -i "module-name" go.mod | ||
|
|
||
| # Check for replace directives (StackRox forks) | ||
| grep -i "module-name" go.mod | grep "=>" | ||
|
|
||
| # Check if indirect | ||
| grep -i "module-name" go.mod | grep "// indirect" | ||
| ``` |
Contributor
There was a problem hiding this comment.
Should only the root go.mod be considered, or can the other go.mod files in the repository be relevant here ?
Contributor
Author
There was a problem hiding this comment.
Good point, If we have a CVE to fix then we can fix it everywhere on the other hand tools are fine as we not ship them. I'll add a note on that
| - Format: `Our package → Intermediate package → module-name` | ||
| - Example: `central/externalbackups → cloud.google.com/go/storage → protoc-gen-validate` | ||
|
|
||
| 2. **Check if we use the intermediate package:** |
Contributor
There was a problem hiding this comment.
Should there be logic to isolate the test-only dependencies and process them differently ?
| - No direct maintenance needed | ||
| - Updated automatically when [intermediate package] updates | ||
| - Monitor for security issues but low priority (transitive) | ||
| ``` |
Contributor
There was a problem hiding this comment.
Can it make sense to bump the indirect dependencies anyway to protect the intermediate packages from the possible vulnerabilities in the target dependency ?
| go list -m module-name | ||
|
|
||
| # MANDATORY: Show why dependency is needed | ||
| go mod why -m module-name |
Contributor
There was a problem hiding this comment.
Does go mod why provide a comprehensive view of why a dependency is used or would it be better to rely on the dependency graph to have a better coverage of the target dependency usage ?
Contributor
Author
There was a problem hiding this comment.
Unfortunately go mod is limited that's why I used goda
| | **zap** | v1.18.2 (fork) | Direct | 7 | pkg/logging, operator | CRITICAL | | ||
| ``` | ||
|
|
||
| ### Example 6: Wrapper pattern (infrastructure library) |
Contributor
There was a problem hiding this comment.
Where are examples 4 and 5 ?
User request: "Look at current PR comments and plan how to fix them all" Addresses all 5 review comments from PR #20247: 1. Line 52: Added multi-module workspace clarification - Explains StackRox monorepo uses single root go.mod - Documents tool vs production dependency priority for CVEs 2. Line 89: Added test-only transitive dependency guidance - Shows how to detect test-only transitive chains - Documents lower CVE priority for test infrastructure 3. Line 117: Added indirect dependency bumping strategy - Criteria for when to proactively bump transitive deps - Trade-offs between direct bumping vs upstream fixes 4. Line 130: Added go mod why vs goda tool comparison - Explains go mod why limitation (shows one path) - Documents why goda is needed (shows all paths) - Provides example scenario demonstrating the gap 5. Line 355: Added missing Examples 4 and 5 - Example 4: Transitive dependency (test infrastructure) - Example 5: Replace directive (StackRox fork) Total additions: ~155 lines of clarifications and examples. No changes to existing skill logic or workflow. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixes confirmed bugs from code review that would cause incorrect dependency analysis or confusing output: CRITICAL fixes: - Remove /... suffix from goda reach() patterns (lines 88, 160, 198) - Pattern fails for modules without subpackages (e.g., gopkg.in/yaml.v2) - Returns empty results instead of showing dependency usage HIGH priority fixes: - Remove -i flag from grep commands on module paths (lines 45, 48, 51) - Go module paths are case-sensitive - Case-insensitive matching causes wrong module identification in CVE triage MEDIUM priority fixes: - Add --exclude-dir=test to production grep commands (lines 100, 201, 205) - Test fixture files in /test/ directories were counted as production code - Inflates CVE priority for test-only dependencies - Add handling for "does not need" from go mod why (after line 183) - Occurs when dependency in go.mod but not actually used - Provides guidance for stale entries and build-tag-excluded imports - Add go.mod existence validation (before line 42) - Pre-flight check prevents confusing errors when run from wrong directory LOW priority fixes: - Add "Action:" section to Example 4 (line 447) - Template showed Action section but example omitted it - Ensures template/example consistency Total changes: 33 insertions, 13 deletions Impact: Fixes command syntax errors and adds missing edge case handling Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…yzer After implementing command fixes in commit 8786da1, second code review found 5 documentation sections still showing old incorrect patterns. Main workflow commands were correctly fixed, but explanatory text, troubleshooting sections, and examples weren't updated to match. Documentation fixes: - Line 179: Remove /... from goda pattern in "Why Both Tools Are Needed" - Was: `goda graph "reach(./..., module-name/...)"` - Now: `goda graph "reach(./..., module-name)"` - Line 601: Remove -i flag from troubleshooting grep guidance - Was: `grep -i "pgx"` (case-insensitive) - Now: `grep "pgx"` (case-sensitive) + clarifying note - Matches fix: Go module paths are case-sensitive - Line 634: Remove /... from troubleshooting goda example - Was: `goda list "reach(./central/..., pgx/...)"` - Now: `goda list "reach(./central/..., pgx)"` - Lines 535, 537: Add standard exclusion flags to Example 6 - Added --exclude-dir=vendor and --exclude-dir=test - Matches methodology in Step 4 - Line 338: Add --exclude-dir=test to Example 1 - Now matches production code search pattern Total changes: 6 lines modified (6 insertions, 6 deletions) Impact: Ensures all documentation teaches same correct patterns Resolves inconsistency where users reading examples/troubleshooting would learn old incorrect approaches that were fixed in main workflow. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…lyzer Problem: - Skill wasn't being invoked for dependency questions like "why do we need X?" - Trigger conditions were only in frontmatter description (easy to miss) - No prominent guidance on when to use the skill Solution: - Added "When to Use This Skill" section immediately after title - Listed common question patterns that should trigger skill invocation - Made it explicit: "ALWAYS use this skill when user asks ANY question about Go dependencies" - Includes examples: "Why do we need X?", "What uses X?", "Is X used in production?" This ensures AI agents invoke the skill for all dependency-related questions, preventing manual analysis when a structured workflow exists. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@janisz: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Skill for using
go mod ...andgodato learn about dependencies and their usage useful when dealing with CVE reports in dependencies.User-facing documentation
Testing and quality
Automated testing
How I validated my change
/go-dependency-analyzer analyze kubevirt.io/containerized-data-importer-api ● Dependency Analysis: kubevirt.io/containerized-data-importer-api (TRANSITIVE) Dependency: kubevirt.io/containerized-data-importer-api v1.64.0 Status: TRANSITIVE ONLY - Not directly imported Dependency Chain: Our code (StackRox) ↓ imports kubevirt.io/api v1.8.2 (KubeVirt virtual machine API) ↓ depends on kubevirt.io/containerized-data-importer-api v1.64.0 (CDI API for persistent storage) Why It Exists: From go mod why: - pkg/virtualmachine imports kubevirt.io/api/core/v1 (KubeVirt core API) - KubeVirt API depends on containerized-data-importer-api/pkg/apis/core/v1beta1 for DataVolume specs - CDI (Containerized Data Importer) is KubeVirt's persistent storage subsystem for VM disks Our Usage of Intermediate Package: - We import: kubevirt.io/api (KubeVirt virtual machine APIs) - Used in: 5 production files (direct kubevirt.io/api imports) - Plus: 7 production files via pkg/virtualmachine wrapper - Components: Central (VM pipeline), Sensor (VM monitoring) - Files (direct kubevirt.io/api usage): - pkg/virtualmachine/api.go - VM type definitions - sensor/kubernetes/listener/resources/virtualmachine/dispatcher/*.go - VM event dispatching (4 files) What Intermediate Uses It For: - KubeVirt API uses CDI API to define DataVolume types for VM persistent storage - When VMs reference persistent disks, they use CDI DataVolume specs - We monitor VM resources through KubeVirt API, which includes CDI DataVolume references - CDI API defines schemas for volume provisioning, cloning, and importing Recommended Action: - ✅ No direct action needed - transitive dependency - ✅ Monitor for security updates (managed by kubevirt.io/api) - ✅ Low priority - updated automatically with KubeVirt API Team Assignment: N/A - Managed via kubevirt.io/api updates (If CVE: assign to @sensor-team who owns VM monitoring feature in sensor/kubernetes/listener/resources/virtualmachine)