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

[kots] Allow users to upload a .docker/config.json file #12174

Merged
merged 1 commit into from
Aug 19, 2022

Conversation

Pothulapati
Copy link
Contributor

@Pothulapati Pothulapati commented Aug 17, 2022

Description

This PR updates the kots UI to add a new config option to upload registry
credentials irrespective of the registry being used, which is then merged
into a single config.json file and passed as the registry secret which
is then used across the workspace image builder components.

Related Issue(s)

Fixes #12136

How to test

Upload a dockerconfigjson file through the new option,
Screenshot from 2022-08-18 19-03-10

and see those credentials in the secret by running

kubectl -n gitpod get secret builtin-registry-auth --output="jsonpath={.data.\.dockerconfigjson}" | base64 -d

Release Notes

[kots] Allow users to upload a `.docker/config.json` file

Documentation

Werft options:

  • /werft with-preview

@Pothulapati Pothulapati force-pushed the tar/kots-dockerconfig branch from 5a04f3f to e963107 Compare August 18, 2022 04:57
@roboquat roboquat added size/M and removed size/S labels Aug 18, 2022
@Pothulapati Pothulapati force-pushed the tar/kots-dockerconfig branch 11 times, most recently from 81ecde6 to 21b2ed1 Compare August 18, 2022 12:07
@Pothulapati Pothulapati marked this pull request as ready for review August 18, 2022 12:08
@Pothulapati Pothulapati requested a review from a team August 18, 2022 12:08
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Aug 18, 2022
@Pothulapati
Copy link
Contributor Author

Pothulapati commented Aug 18, 2022

/werft run publish-to-kots

👍 started the job as gitpod-build-tar-kots-dockerconfig.20
(with .werft/ from main)

@@ -174,8 +184,7 @@ spec:
kubectl create secret docker-registry container-registry \
--namespace "{{repl Namespace }}" \
--from-file=.dockerconfigjson=/tmp/container-registry-secret \
-o yaml --dry-run=client | \
kubectl replace --namespace "{{repl Namespace }}" --force -f -
-o yaml --dry-run=client > "${GITPOD_OBJECTS}/templates/gitpod.yaml"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this to save the secret in the gitpod.yaml, instead of applying so that we can post process it if the user submits more docker configs through the new option

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a reasonable approach. This means that we'll have a secret at the start of the YAML file, but that shouldn't be a problem as it's what we're doing anyway

@lucasvaltl
Copy link
Contributor

@Pothulapati what does the UX, specifically the user flow, look like here? When does a user set this and how - do they need to also set something else for this to take Affect?

@Pothulapati
Copy link
Contributor Author

Pothulapati commented Aug 18, 2022

@lucasvaltl This adds a new config option in the registry section of the KOTS UI (irrespective of in-cluster or external registry) through which a user can upload a file.

Screenshot from 2022-08-18 19-03-10

do they need to also set something else for this to take Affect?

Not really, because this is automatically set everywhere to be used. They will have to obviously use the container image somewhere to actually use the functionality. 🤔

@@ -79,6 +79,16 @@ spec:
fi

echo "Gitpod: Generate the base Installer config"
echo "Gitpod: Create a Helm template directory"
Copy link
Contributor

Choose a reason for hiding this comment

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

The echo "Gitpod: Generate the base Installer config" line should go above the /app/installer init > "${CONFIG_FILE}" so that the logging is representative of what's going on

@@ -174,8 +184,7 @@ spec:
kubectl create secret docker-registry container-registry \
--namespace "{{repl Namespace }}" \
--from-file=.dockerconfigjson=/tmp/container-registry-secret \
-o yaml --dry-run=client | \
kubectl replace --namespace "{{repl Namespace }}" --force -f -
-o yaml --dry-run=client > "${GITPOD_OBJECTS}/templates/gitpod.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a reasonable approach. This means that we'll have a secret at the start of the YAML file, but that shouldn't be a problem as it's what we're doing anyway

@Pothulapati Pothulapati force-pushed the tar/kots-dockerconfig branch 4 times, most recently from 706b2a6 to 4caf78e Compare August 19, 2022 12:22
Copy link
Contributor

@mrsimonemms mrsimonemms left a comment

Choose a reason for hiding this comment

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

/hold

Works beautifully. I've respectfully made a couple of suggestions on the config wording, but approved so you can merge when ready

install/kots/manifests/kots-config.yaml Outdated Show resolved Hide resolved
install/kots/manifests/kots-config.yaml Outdated Show resolved Hide resolved
@Pothulapati Pothulapati force-pushed the tar/kots-dockerconfig branch from 4caf78e to 5f32584 Compare August 19, 2022 13:13
@Pothulapati
Copy link
Contributor Author

This is how the option looks now!

Screenshot from 2022-08-19 18-47-40

Screenshot from 2022-08-19 18-48-11

when: '{{repl ConfigOptionEquals "reg_docker_config_enable" "1" }}'
type: file
required: true
help_text: Docker [config JSON file](https://docs.docker.com/engine/reference/commandline/cli/#sample-configuration-file) with auth credentials used to access private registries, for workspace images.
Copy link
Contributor

@lucasvaltl lucasvaltl Aug 19, 2022

Choose a reason for hiding this comment

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

Suggested change
help_text: Docker [config JSON file](https://docs.docker.com/engine/reference/commandline/cli/#sample-configuration-file) with auth credentials used to access private registries, for workspace images.
help_text: Docker [config JSON file](https://docs.docker.com/engine/reference/commandline/cli/#sample-configuration-file) with auth credentials used to access private registries used for pulling base workspace images.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

credentials used to access private registries used for pulling base workspace images.

Happy to update, but used to <> used for <> sounds a bit confusing?

help_text: This is useful when you have base workspace images in private registries other than the above configured ones.

- name: reg_docker_config
title: Registry credentials
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
title: Registry credentials
title: Private base image registry credentials

title: Configure additional registry credentials for pulling workspace images
type: bool
default: "0"
help_text: This is useful when you have base workspace images in private registries other than the above configured ones.
Copy link
Contributor

@lucasvaltl lucasvaltl Aug 19, 2022

Choose a reason for hiding this comment

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

Suggested change
help_text: This is useful when you have base workspace images in private registries other than the above configured ones.
help_text: This is useful when you want to use base workspace images in private registries other than the above configured ones or the [default base workspace images stored on Docker Hub](https://github.com/gitpod-io/workspace-images).

Copy link
Contributor

@lucasvaltl lucasvaltl left a comment

Choose a reason for hiding this comment

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

Added some suggestions that you can merge right in. Otherwise LGTM, although I think we should hack around KOTS a bit to create a better separation between the push and pull registries here.

@mrsimonemms
Copy link
Contributor

/unhold

@roboquat roboquat merged commit 2e8fb27 into main Aug 19, 2022
@roboquat roboquat deleted the tar/kots-dockerconfig branch August 19, 2022 13:39
Pothulapati added a commit that referenced this pull request Aug 25, 2022
Follow up to #12174,
and improves the help_text around the newly added fields.

Signed-off-by: Tarun Pothulapati <[email protected]>
Pothulapati added a commit that referenced this pull request Aug 25, 2022
…owList`

Follow upto #12174

This PR updates the installer logic to also load the auth's reigstry
URL's into `.containerRegistry.privateBaseImageAllowList`.

Signed-off-by: Tarun Pothulapati <[email protected]>
nandajavarma pushed a commit that referenced this pull request Aug 26, 2022
…owList`

Follow upto #12174

This PR updates the installer logic to also load the auth's reigstry
URL's into `.containerRegistry.privateBaseImageAllowList`.

Signed-off-by: Tarun Pothulapati <[email protected]>

Co-authored-by: Simon Emms <[email protected]>
Co-authored-by: Simon Emms <[email protected]>
nandajavarma pushed a commit that referenced this pull request Aug 26, 2022
…owList`

Follow upto #12174

This PR updates the installer logic to also load the auth's reigstry
URL's into `.containerRegistry.privateBaseImageAllowList`.

Signed-off-by: Tarun Pothulapati <[email protected]>

Co-authored-by: Simon Emms <[email protected]>
Co-authored-by: Simon Emms <[email protected]>
nandajavarma added a commit that referenced this pull request Aug 26, 2022
…owList`

Follow upto #12174

This PR updates the installer logic to also load the auth's reigstry
URL's into `.containerRegistry.privateBaseImageAllowList`.

Signed-off-by: Tarun Pothulapati <[email protected]>

Co-authored-by: Simon Emms <[email protected]>
Co-authored-by: Nandaja Varma <[email protected]>
nandajavarma added a commit that referenced this pull request Aug 26, 2022
…owList`

Follow upto #12174

This PR updates the installer logic to also load the auth's reigstry
URL's into `.containerRegistry.privateBaseImageAllowList`.

Signed-off-by: Tarun Pothulapati <[email protected]>

Co-authored-by: Simon Emms <[email protected]>
Co-authored-by: Nandaja Varma <[email protected]>
roboquat pushed a commit that referenced this pull request Aug 26, 2022
…owList`

Follow upto #12174

This PR updates the installer logic to also load the auth's reigstry
URL's into `.containerRegistry.privateBaseImageAllowList`.

Signed-off-by: Tarun Pothulapati <[email protected]>

Co-authored-by: Simon Emms <[email protected]>
Co-authored-by: Nandaja Varma <[email protected]>
mrsimonemms pushed a commit that referenced this pull request Aug 26, 2022
…owList`

Follow upto #12174

This PR updates the installer logic to also load the auth's reigstry
URL's into `.containerRegistry.privateBaseImageAllowList`.

Signed-off-by: Tarun Pothulapati <[email protected]>

Co-authored-by: Simon Emms <[email protected]>
Co-authored-by: Nandaja Varma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/M team: delivery Issue belongs to the self-hosted team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[kots] Allow adding additional credentials for private registries in KOTS
4 participants