-
Notifications
You must be signed in to change notification settings - Fork 51
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
Decouple dependencies of tools/ and the rest of the codebase #1840
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.
This is a preparation step before making tooks/ its own module.
typo in commit msg: tooks/ --> tools
@oshoval don't bother reviewing it, it's WIP |
ddf1137
to
ee00f93
Compare
The module was moved from coreos org under prometheus-operator. Reflect that in imports, to allow `go mod tidy` to pass. Signed-off-by: Petr Horacek <[email protected]>
Otherwise `go mod tidy` fails due to broken libvmi import. Signed-off-by: Petr Horacek <[email protected]>
ee00f93
to
df8e705
Compare
47a0611
to
1f0d12c
Compare
aa0731d
to
cf357cd
Compare
cf357cd
to
ae6987f
Compare
This is a preparation step before making tooks/ its own module. This greatly reduces the amound of dependencies it the root go.mod, allowing for easier scanning and resolution of vulnerabilities. Signed-off-by: Petr Horacek <[email protected]>
Signed-off-by: Petr Horacek <[email protected]>
ae6987f
to
f5cec32
Compare
Quality Gate failedFailed conditions |
IPAM lane started to fail on random tests, not related to this PR i believe, The teardown from glance looks fine after each test. |
@@ -3,11 +3,11 @@ | |||
set -e | |||
|
|||
function fix() { | |||
git ls-files -- ':!vendor/' | xargs sed --follow-symlinks -i 's/[[:space:]]*$//' | |||
git ls-files -- ':!vendor/' ':!tools/vendor' | xargs sed --follow-symlinks -i 's/[[:space:]]*$//' |
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.
nit: in order to exclude /vendor/ folders that might exists in the future
git ls-files | grep -vE '^vendor/|/vendor/'
or maybe also
git ls-files -- ':!vendor' ':!**/vendor/**'
but just nice to have please, it can wait for once we have new vendor folders
Very nice, thank you We can just ignore failures on the IPAM for this PR imo, and fix it soon also sonar can be ignored please |
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.
/lgtm
Can you please rebase this PR ? Git actions doesn't auto rebase, it is always good please to rebase manually just before merge |
#1882 should fix the ipam flakiness, at least some of them are due to that (will run few times) |
PR needs rebase. Instructions 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. |
@phoracek sorry for conflicting with your PR, can you re-copy the conflicted files to the new location? |
Pull requests that are marked with After that period the bot marks them with the label /label needs-approver-review |
What this PR does / why we need it:
This PR introduces a dedicated subpackage for tools used in the project. This allows us to shed much of the vendored dependencies that are tied to the production code.
This is important, because it allows us to quickly evaluate whether any reported CVE/CWE affects the production code. Decoupling the dependencies also makes is easier to keep them up to date. Finally, making it clear which dependencies are used by the production code enables us to audit the list and decide whether we want to remove any of the dependencies.
Number of go lines vendored by the production code was cut by 96 %, from 384015 to 14460. Checked with
cd vendor && find . -name '*.go' | xargs wc -l
.Number of dependencies (including indirect) of the production code was cut by 30 %, from 806 to 566. Checked with
cat go.sum | awk '{print $1}' | sort | uniq | wc -l
.The list of dependencies we were able to shed off is here: https://gist.github.com/phoracek/aafc6cae0275291117b0c13e94c48e66
Special notes for your reviewer:
Review the PR commit by commit. The last commit contains all the vendoring and go.sum updates.
Release note: