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

Mount all ServiceAccount imagePullSecrets to allow builds to read the run image #865

Merged
merged 5 commits into from
Oct 22, 2021

Conversation

matthewmcnew
Copy link
Collaborator

@matthewmcnew matthewmcnew commented Oct 21, 2021

  • The build's ServiceAccount's imagePullSecrets will be used to pull the builder image but, are unavailable to read the run image which is likely in the same registry.
  • This also supports multiple builder image pull secrets configured directly on the build resource.
  • Mounts the imagePullSecrets inside the rebase build pod.
  • Refactor BuildPod interface to take a "BuildContext"

Resolves: #858

… run image

- The build's ServiceAccount's imagePullSecrets will be used to pull the builder image but, are unavailable to read the run image which is likely in the same registry.
- This also supports multiple builder image pull secrets configured directly on the build resource.
- Mounts the imagePullSecrets inside the rebase build pod.
@codecov-commenter
Copy link

Codecov Report

Merging #865 (56f3e9b) into main (7eff976) will increase coverage by 0.27%.
The diff coverage is 97.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #865      +/-   ##
==========================================
+ Coverage   67.68%   67.96%   +0.27%     
==========================================
  Files         113      113              
  Lines        5019     5044      +25     
==========================================
+ Hits         3397     3428      +31     
+ Misses       1259     1254       -5     
+ Partials      363      362       -1     
Impacted Files Coverage Δ
pkg/buildpod/generator.go 66.15% <87.50%> (+2.82%) ⬆️
pkg/apis/build/v1alpha2/build_pod.go 97.78% <100.00%> (+1.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7eff976...56f3e9b. Read the comment docs.

@@ -51,6 +52,7 @@ func init() {
flag.Var(&dockerCredentials, "basic-docker", "Basic authentication for docker of the form 'secretname=git.domain.com'")
flag.Var(&dockerCfgCredentials, "dockercfg", "Docker Cfg credentials in the form of the path to the credential")
flag.Var(&dockerConfigCredentials, "dockerconfig", "Docker Config JSON credentials in the form of the path to the credential")
flag.Var(&imagePullSecrets, "imagepull", "Builder Image pull credentials in the form of the path to the credential")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this appears to be a filename instead of the full path to the credential

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to the other path to credentials like dockercfg or dockerconfig and will attempt to read either a .dockerconfigjson or a .dockercfg file.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should add to each then that it is the path inside the build-secrets dir

Copy link
Collaborator

@tomkennedy513 tomkennedy513 Oct 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a big deal it just tripped me up for a second when i was looking at build_pod.go and saw we only passed in the name

Copy link
Collaborator Author

@matthewmcnew matthewmcnew Oct 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should add to each then that it is the path inside the build-secrets dir

Add what to each?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update each description to be something like "..in the form of the path to the credential in the /var/build-secrets directory"

cmd/build-init/main.go Outdated Show resolved Hide resolved
cmd/build-init/main.go Outdated Show resolved Hide resolved
@matthewmcnew matthewmcnew force-pushed the mount-image-pull-secrets branch from edf6b69 to d2816c9 Compare October 22, 2021 14:57
Copy link
Contributor

@tylerphelan tylerphelan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice refactor

@matthewmcnew matthewmcnew merged commit 65ce71d into main Oct 22, 2021
@matthewmcnew matthewmcnew deleted the mount-image-pull-secrets branch October 22, 2021 15:49
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.

Provide a build's service account's imagePullSecrets to pull a run image.
5 participants