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

control-service: builder images load secrets from k8s #1358

Merged
merged 12 commits into from
Nov 29, 2022

Conversation

murphp15
Copy link
Collaborator

@murphp15 murphp15 commented Nov 28, 2022

Why

I wrote this PR with the intent of being able to pass extra docker creds to to the image builder so that we would be able to pull a private base image from a docker repo.
The builder now contains login details for 2 registries. The registry it pushes to and the registry it pulls base image from.

However this PR also introduces the concept of builder secrets being sourced from k8s which leads to much cleaner separation between the control plane and the builder image. I think this will lead to much better support for custom build images in the future.

I'm not sure where to add the documentation about the builder-secrets. So I created a ticket to follow up on this later.
#1357

How was this been tested

tested extensively locally.
I added a test to check that the base image can be pulled from a private repo but in fact it is pulling from the same repo as it is pushing to and so it is could let bugs through related to the auth not working for the base image repo.

closes #1333

murphp15 and others added 4 commits November 28, 2022 17:09
Why

What

How was this tested
tested extensivly locally and it works very well and leads to faster dev iteration cycles.

Signed-off-by: murphp15 <[email protected]>
Why

What

How was this tested
tested extensivly locally and it works very well and leads to faster dev iteration cycles.

Signed-off-by: murphp15 <[email protected]>
Why

What

How was this tested
tested extensivly locally and it works very well and leads to faster dev iteration cycles.

Signed-off-by: murphp15 <[email protected]>
@antoniivanov
Copy link
Collaborator

Currently our main user facing documentation of the Control Service configuration is primary though helm chart values.yaml so this would need to be documented there as well.

but we do need more user-friendly one and examples /tutorials - so the ticket that you opened can be about that.

Copy link
Collaborator

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

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

Looks goot to me.

Do we need to port/refactor some of the other configuration options to come from k8s secret file ?

@murphp15
Copy link
Collaborator Author

Do we need to port/refactor some of the other configuration options to come from k8s secret file ?

I am not going to do this now, but in the future we can cleanup those values.

@murphp15
Copy link
Collaborator Author

Currently our main user facing documentation of the Control Service configuration is primary though helm chart values.yaml so this would need to be documented there as well.

but we do need more user-friendly one and examples /tutorials - so the ticket that you opened can be about that.

Yeah I always do the helm changes in a second PR. I will add the documentation there when I am doing it.

murphp15 and others added 6 commits November 28, 2022 19:09
Why

What

How was this tested
tested extensivly locally and it works very well and leads to faster dev iteration cycles.

Signed-off-by: murphp15 <[email protected]>
Why

What

How was this tested
tested extensivly locally and it works very well and leads to faster dev iteration cycles.

Signed-off-by: murphp15 <[email protected]>
@murphp15 murphp15 enabled auto-merge (squash) November 29, 2022 10:21
@murphp15 murphp15 merged commit 152f772 into main Nov 29, 2022
@murphp15 murphp15 deleted the person/murphp15/builder-images-load-secrets-from-k8s branch November 29, 2022 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The builder job should support pulling the base image from private repositories
4 participants