Skip to content

Conversation

@mcpherrinm
Copy link
Contributor

@mcpherrinm mcpherrinm commented Oct 26, 2023

Remove the "netaccess" container from the docker-compose dev environment.

It isn't needed during a regular 'docker compose up' developer environment, and only really serves as a way to use the same tools image in CI. Two checks run during CI are the govulncheck and verifying go mod tidy / go vendor. Neither of these checks require anything from the custom image other than Golang itself, which can be provided directly from the CI environment.

If a developer is working inside the existing containers, they can still run go mod tidy; go mod vendor themselves, which is a standard Golang workflow and thus is simpler than using the netaccess image via docker compose.

This adds extra complexity around the docker use here. Verifying vendor
and govulncheck can be done outside of the dev docker container.
@jsha
Copy link
Contributor

jsha commented Oct 26, 2023

This makes sense to me.

@mcpherrinm mcpherrinm changed the title [WIP] a little experiment... Remove the "netaccess" container from the docker-compose dev environment. Oct 27, 2023
@mcpherrinm mcpherrinm requested a review from jsha October 27, 2023 23:52
@mcpherrinm mcpherrinm marked this pull request as ready for review October 27, 2023 23:52
@mcpherrinm mcpherrinm requested a review from a team as a code owner October 27, 2023 23:52
@mcpherrinm
Copy link
Contributor Author

I pushed (and reverted) a few deliberately bad commits to make sure the vendoring and tidy checks failed as expected.

@jsha jsha merged commit 5b3c84d into main Nov 1, 2023
@jsha jsha deleted the mattm-no-netaccess branch November 1, 2023 22:11
@jsha
Copy link
Contributor

jsha commented Nov 1, 2023

Thanks for the improvements!

pgporada pushed a commit that referenced this pull request Nov 13, 2023
Having govulncheck prevent a PR from merging means that circumstances
entirely outside our control can grind Boulder development to a halt
until they are addressed. When the vulnerability is within Go itself, it
prevents PRs from being merged until we do a production deploy, because
we want our CI to always match what is in production. This is too
strict.

This PR removes govulncheck from the set of jobs depended upon by our
Boulder CI Test Matrix meta-job. It also adds vendorcheck, which was
accidentally omitted in #7123.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants