Skip to content
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

Set GOPROXY in cloudbuild.yaml #1331

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

torredil
Copy link
Member

@torredil torredil commented Aug 2, 2022

Signed-off-by: Eddie Torres torredil@amazon.com

What is this PR about? / Why do we need it?

  • The vendor directory was removed in Remove /vendor directory #1328 to address Remove /vendor directory #1326. As a result of this change, we need to account for two scenarios:

    1. For security, a user might want to prevent outgoing connections outside their internal network. Addressed in Pass GOPROXY to image builder #1330.
    2. If GOPROXY is undefined or set to "direct", then go get will use a direct connection to the VCS (e.g github.com). This is causing our build to timeout, since go mod download takes nearly 30 minutes to complete. Addressed by this PR.

Why do we need it?

  • Dependencies are stored in immutable storage, thus avoiding the risk that any dependency might disappear in the future.
  • Protection against actors who might inject malicious code since Go modules cannot be deleted or overridden once stored in the GOPROXY.
  • Significantly faster to resolve dependencies since GOPROXY serves the source code over HTTP, thus not requiring the overhead when pulling from a VCS which fetches the whole repository.

What testing is done?

  • CI

Signed-off-by: Eddie Torres <torredil@amazon.com>
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 2, 2022
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 2, 2022
@torredil
Copy link
Member Author

torredil commented Aug 2, 2022

/retest

@rdpsin
Copy link
Contributor

rdpsin commented Aug 2, 2022

/lgtm
/approve
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 2, 2022
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 2, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rdpsin, torredil

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gtxu
Copy link
Contributor

gtxu commented Aug 2, 2022

/lgtm
/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 2, 2022
@k8s-ci-robot k8s-ci-robot merged commit 03fe188 into kubernetes-sigs:master Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants