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
[WIP] chore: pin go deps with go.mod #5535
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: karlkfi 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 |
Looks like |
Those container images do not seem to exist in GCR. |
Looks like it might be due to docker/buildx#1613 which can be mitigated by setting |
ad51164
to
64fee65
Compare
Yes, That is right. Those container images were built when tests were executed, and images aren't pushed to the registry. Maybe that tests success if I rerun it. |
It doesn't pass when re-run. It doesn't pass locally. The whole repo is private now for some reason. It looks like the tests are using |
Alright, so the real error is from docker build because it doesn't like my new go.work module, which is specific to this PR:
|
Figured it out. It's because It looks like the Dockerfile needs to be built from the root in order for the kyaml replace directive to work. So I think we have to remove site from dockerignore, or move tools to be a top level directory. Anyone got a preference? Yall might want the tools dir for other dependencies in different dockerfiles, so maybe top level is best? The downside is that then we also have to change the docker build for the site to also run from the root dir, so it can copy in the tools... |
64fee65
to
e01fbd3
Compare
Looks like I'm gonna have to do this for all the modules to really fix the supply chain issues and pin to versions with checksums. But I got lint/build/test happy now at least. It turns out when you add a new Go module it needs to be added to |
Looks like we already have Can you write what you want in this file? |
Using the same tools.go for all modules means that all the Dockerfiles need to be built from the repo root and copy all repo files into the docker context, which will mean slower docker build times. But if that's ok, it has the benefit that all the tools are centralized with the same versions in the top level go.mod. |
e01fbd3
to
8bac054
Compare
c46c11c
to
96c3bca
Compare
Looks like mdrip requires Golang 1.21+ So we're going to have to bump the prow image. |
/cc @koba1t |
/hold wait to update Golang 1.21 |
@koba1t What do I need to move this forward? Do I need to upgrade to Go 1.21 in a separate PR? AFAICT, the CI needs to be updated first in order to unblock this or any other PR updating the Go here to 1.21. |
Anything I can do to move this along? |
I think so. |
Use go.mod & go.sum to pin the version and checksum for installed commands and their source dependencies. Modify hacks/for-each-module.sh to count found and skipped modules to avoid needing to hardcode multiple expected values. Modify `make container-image` to update the pinned version of hugo to match HUGO_VERSION in netlify.toml, instead of passing in the version as a build-arg. This should help reduce risk of importing newer dependency versions that haven't passed vulnerability checks. Disable abandoned linters to fix `make lint`.
96c3bca
to
1e73578
Compare
@karlkfi: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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/test-infra repository. I understand the commands that are listed here. |
Now that Go 1.21 is merged, I'm gonna break up this PR into some smaller ones that should be easier to review. |
Extracted:
Once those are merged, I'll make another to handle the |
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/test-infra repository. |
Extracted #5636 |
I think the only unique things left in here are:
None of these are particularly critical. So I think we can just call this done and leave those for other people to pick up as needed. |
Use go.mod & go.sum to pin the version and checksum for installed
commands and their source dependencies.
Modify hacks/for-each-module.sh to count found and skipped modules
to avoid needing to hardcode multiple expected values.
Modify
make container-image
to update the pinned version of hugoto match HUGO_VERSION in netlify.toml, instead of passing in the
version as a build-arg.
This should help reduce risk of importing newer dependency versions
that haven't passed vulnerability checks.
Disable abandoned linters to fix
make lint
.Depends on #5555