-
Notifications
You must be signed in to change notification settings - Fork 202
Helm support registries.conf for OCI configuration/auth info
#391
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
base: main
Are you sure you want to change the base?
Conversation
99c6d56 to
cea4a1d
Compare
mtrmac
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some drive-by notes.
hips/hip-XXXX.md
Outdated
| ```toml | ||
| # registries.conf | ||
| [[registry]] | ||
| prefix = "oci.example.com" | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In registries.conf, it’s not necessary to add a no-op entry for a registry. (In fact it’s currently somewhat annoying to do, although that would probably be best fixed in the parser package.)
| ``` | ||
|
|
||
| ```json | ||
| # auth.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better or worse, auth.json is intentionally placed into tmpfs filesystems cleared on reboot. (The tools may read other locations in the home directory, but they, without specific instruction otherwise, don’t update them.)
That might be a fairly disruptive change to users of “login”, so it probably should be discussed as such.
hips/hip-XXXX.md
Outdated
|
|
||
| Including support for repository prefixes (allowing different credentials for different prefixes), registry mirrors, and registry "aliasing". Features Helm would like to introduce. But is currenly blocked by a lack of mechanism to store detail | ||
|
|
||
| (in particular, the existing registry configuration Helm uses, Docker’s `$HOME/.docker/config.json`, etc do not support registry aliases nor prefixes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is do not support registry aliases nor prefixes the only limitation of Docker’s $HOME/.docker/config.json? I would expect that this proposal could clarify more on the problems and limitations here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FeynmanZhou are there other limitations you want to highlight?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what limitations @FeynmanZhou is referring to, but one limitation this may or may not address is namespaced auth. Like with Harbor, you might have one credential for oci://localhost/production and another for oci://localhost/development and config.json as I recall only supports auth at the domain level.
hips/hip-XXXX.md
Outdated
| ## Specification | ||
|
|
||
| Helm will utilize the `registries.conf` specification when determinging OCI registry information (authentication credentials, etc): | ||
| <https://github.com/containers/image/blob/main/docs/containers-registries.conf.5.md> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to reference a versioned link instead of main branch in case the link is broken in the main branch
hips/hip-XXXX.md
Outdated
|
|
||
| ## Open issues | ||
|
|
||
| - Support for registies.conf in ORAS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we use ORAS for this, do we need to get agreement from them before this moves forward?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mattfarina ,
@sabre1041 brought up this proposal to the ORAS community meeting on Mar 5. There is also a GitHub issue tracked in oras-go oras-project/oras-go#918.
As this is a dependency for ORAS from Helm v4, what's the expected timeline to getregisties.conf supported in oras-go?
scottrigby
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of supporting this in Helm.
We may want to note in this HIP that registry aliases would help with airgapped environments—an important use case for Helm users—where an in-cluster registry would replace external registries defined as the storage for chart dependencies.
There are existing means in Helm to overwrite a registry for container images in airgap—many charts make images/oci registries configurable, but even if they do not, this can be done with a post-renderer. However, there is currently no way to do this with the repository specified by chart dependencies without manually manipulating the Chart.yaml.
|
Note that the Podman tools that standardize registries.conf have been accepted as a CNCF Sandbox project: cncf/sandbox#309 |
|
Also note this related issue: helm/helm#13615 |
Signed-off-by: George Jenkins <[email protected]>
07c67e8 to
1f6c412
Compare
registries.conf for OCI configuration/auth inforegistries.conf for OCI configuration/auth info
Signed-off-by: George Jenkins <[email protected]>
1f6c412 to
6335ac4
Compare
hips/hip-XXXX.md
Outdated
| --- | ||
| hip: 9999 | ||
| title: "registries.conf support for OCI registry management" | ||
| authors: [ "George Jenkins <[email protected]>", "Andy Block <[email protected]>" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| authors: [ "George Jenkins <[email protected]>", "Andy Block <andy@redhat.com>" ] | |
| authors: [ "George Jenkins <[email protected]>", "Andrew Block <andy.block@gmail.com>" ] |
|
|
||
| - support for repository prefixes (allowing different credentials for different prefixes) | ||
| - registry mirrors | ||
| - registry "aliasing"—features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add:
- allowing/denying registries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
hips/hip-XXXX.md
Outdated
| - registry mirrors | ||
| - registry "aliasing"—features | ||
|
|
||
| Helm would like to introduce features depending on these functionalities. Supporting `registries.conf` would enable them without Helm having to create or implement its own mechanisms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Helm would like to introduce features depending on these functionalities. Supporting `registries.conf` would enable them without Helm having to create or implement its own mechanisms. | |
| Helm would like to introduce features leaning upon these functionalities. Supporting `registries.conf` would enable them without Helm having to create or implement its own mechanisms. |
hips/hip-XXXX.md
Outdated
| - registry "aliasing"—features | ||
|
|
||
| Helm would like to introduce features depending on these functionalities. Supporting `registries.conf` would enable them without Helm having to create or implement its own mechanisms. | ||
| With `registries.conf` having been standardized by other container ecosystem CLI tools (podman, buildah, skopeo, etc.). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also mention that these are also CNCF hosted tools/projects
hips/hip-XXXX.md
Outdated
|
|
||
| `registries.conf` was picked as being a format intended for consumption beyond the Docker application container ecosystem. | ||
|
|
||
| ORAS (the library Helm uses for supporting OCI functionality) has planned support for `registries.conf` client-side OCI registry management. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include reference(s) to the ORAS specific features?
hips/hip-XXXX.md
Outdated
| Helm will utilize the `registries.conf` specification when determining OCI registry information (authentication credentials, etc.): | ||
| <https://github.com/containers/image/blob/main/docs/containers-registries.conf.5.md> | ||
|
|
||
| `registries.conf` will be preferred either when a `registries.conf` file already exists on the user's system, or when Helm supports and a user utilizes functionality that cannot be supported by Docker's configuration file in the future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we indicate where we will search for entries?
hips/hip-XXXX.md
Outdated
| } | ||
| ``` | ||
|
|
||
| Helm will use ORAS v3 for updating (and reading) `registries.conf` (TODO: link to ORAS v3 `registries.conf` implementation). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we call out the fact that ORAS v3 development has yet to begin as of this date
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't characterize it as hasn't begun. An oras-go v2 branch was cut and there are several PRs out for v3. Nothing has merged though.
hips/hip-XXXX.md
Outdated
|
|
||
| To account for existing configuration in Docker’s registry configuration, Helm will prefer `registries.conf` when resolving OCI registries (including credentials), and fall back to the existing storage mechanism if `registries.conf` does not contain an entry for the required OCI registry (including if `registries.conf` does not exist). | ||
|
|
||
| Helm must expect (and even encourage) users to utilize other tooling to manage `registries.conf`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or at least provide some form of guidance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite following?
Do you mean guidance on using other tools to manage? (if so, I think this is a detail we can leave for docs)
hips/hip-XXXX.md
Outdated
|
|
||
| 1. A corrupt `registries.conf` will cause an error for existing workflows | ||
| 2. An invalid or incompatible with Helm `registries.conf` entry for the given OCI registry will cause the user's workflow to fail | ||
| 3. Helm’s preference for `registries.conf` will break users who assume credentials are stored in Docker’s registry configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to include an explicit option to opt out of the feature and fall back. That way, we are covered for all scenarios
| "auth": "Zm9vOmJhcgo=" | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to mention policy.json around here since that is used for allow/deny?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess along these lines, hopefully certs.d would be supported. That could be out of the scope for this document though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, I want to focus this HIP on initial registries.conf/auth.json support (ie. feature parity and switch over from Docker conf).
And leave extending to support future functionality to either:
- ORAS improvements
- Explicit HIPs for additional Helm behavior changes
hips/hip-XXXX.md
Outdated
| } | ||
| ``` | ||
|
|
||
| Helm will use ORAS v3 for updating (and reading) `registries.conf` (TODO: link to ORAS v3 `registries.conf` implementation). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't characterize it as hasn't begun. An oras-go v2 branch was cut and there are several PRs out for v3. Nothing has merged though.
| Helm's fallback to Docker's registry configuration ensures the vast majority of existing user workflows remain the same. | ||
|
|
||
| However, there are three potential incompatibility scenarios: | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is another possibility here where the user doesn't have a registries.conf but has a auth.json My understanding is the registries.conf is not required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, when I originally wrote this, I thought registries.conf was always paired with auth.json.
https://github.com/helm/community/pull/391/files#r1978339008 mentions the same
I have (significantly) reworked the language to include auth.json. Strictly, auth.json is all that is needed to replace docker/config.json. But the overall motivation is to be able to utilize features implemented by registries.conf.
Signed-off-by: George Jenkins <[email protected]>
|
|
||
| `auth.json`: | ||
|
|
||
| 1. Location specified in environment variable `REGISTRY_AUTH_FILE` if set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to consider that ${XDG_RUNTIME_DIR} is tmpfs on linux by default
https://github.com/helm/community/pull/391/files#r1978342074
Signed-off-by: George Jenkins <[email protected]>
Signed-off-by: George Jenkins <[email protected]>
Signed-off-by: George Jenkins <[email protected]>
No description provided.