-
Notifications
You must be signed in to change notification settings - Fork 86
Fix the golang package updater #997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v3_er
Are you sure you want to change the base?
Conversation
eyalk007
commented
Dec 15, 2025
- All tests passed. If this feature is not already covered by the tests, I added new tests.
- This pull request is on the dev branch.
- I used gofmt for formatting the code before submitting the pull request.
- Update documentation about new features / new supported technologies
packagehandlers/gopackagehandler.go
Outdated
|
|
||
| func (golang *GoPackageHandler) UpdateDependency(vulnDetails *utils.VulnerabilityDetails) error { | ||
| // Configure resolution from an Artifactory server if needed | ||
| if golang.depsRepo != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we want to stop support depsRepo, so those lines can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i say comment this needs to be back by q1 in my eyes
packagehandlers/gopackagehandler.go
Outdated
| ) | ||
|
|
||
| type GoPackageHandler struct { | ||
| CommonPackageHandler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we dont need and want the CommonPackageHandler right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i believe it should not exist
maybe in the future in a diff way with the parsing, but for now we should delete implementation package by package manager
| } | ||
|
|
||
| func (golang *GoPackageHandler) allowLockfileManipulation() []string { | ||
| return append(os.Environ(), "GOFLAGS=-mod=mod") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we have "GOFLAGS=-mod=mod" in a const in the beginning so it will be more clear which flags are being used for each package handler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feels like overkill
packagehandlers/gopackagehandler.go
Outdated
| "github.com/jfrog/jfrog-client-go/utils/log" | ||
| ) | ||
|
|
||
| type GoPackageHandler struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think ti change the PackageHandler name to something more clear? for example "GolangFixPrCreator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GolangPackageFixer
79d4322 to
5cd924b
Compare
… GoPackageUpdater - Add 'v' prefix to Go module versions (fixes: go get invalid version error) - Run 'go mod tidy' after 'go get' to ensure go.mod and go.sum consistency - Set GOFLAGS=-mod=mod for predictable module behavior - Add debug logging for command output (warnings, etc.) - Remove CommonPackageHandler dependency - Go handler is now fully independent - Rename GoPackageHandler -> GoPackageUpdater (better name for future dependency bump feature) - Change receiver from 'golang' to 'gpu' for clarity - Follows Dependabot/Renovate best practices
WHAT: - Detect vendor/modules.txt to identify projects using vendoring - Automatically run 'go mod vendor' after 'go mod tidy' when vendor exists - This keeps vendor/ directory in sync with go.mod/go.sum after dependency updates WHY: - Renovate and Dependabot both support vendor directories - When dependencies are updated, vendor/ needs to be regenerated - Following industry best practices from Renovate's postUpdateOptions HOW IT WORKS: 1. After running 'go get package@version' 2. Run 'go mod tidy' (already existed) 3. NEW: Check for vendor/modules.txt 4. NEW: If found, run 'go mod vendor' to update vendor/ directory TESTING: - Test repo: frogbot-test-multi-go (has vendor/ in both projects) - Expected: After fix, vendor/ directory should be updated in PR REFERENCES: - Renovate: config.postUpdateOptions includes 'gomodTidy' - Dependabot: Automatically detects and updates vendor directories - Analysis: dependency-update-tools-analysis-mermaid.md
7baec21 to
1df000e
Compare
- Remove depsRepo and serverDetails (not needed) - Remove Artifactory resolution code - Remove comments (anti-comment style) - Rename gopackagehandler.go -> gopackageupdater.go to match struct name
- Go handler doesn't need serverDetails or depsRepo - Empty implementation satisfies PackageHandler interface - Future: gradually remove from all handlers that don't need it