-
Notifications
You must be signed in to change notification settings - Fork 205
HIP: Helm support Container Tools configuration (auth.json, registries.conf, etc) for OCI registry access management
#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?
Changes from 2 commits
1b89550
6335ac4
7588b79
850727a
b96cb0f
b00f334
ec384fb
7bd9734
5b21eee
8dce6d1
c1717df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,137 @@ | ||||||
| --- | ||||||
| hip: 9999 | ||||||
| title: "registries.conf support for OCI registry management" | ||||||
| authors: [ "George Jenkins <gvjenkins@gmail.com>", "Andy Block <andy@redhat.com>" ] | ||||||
|
||||||
| authors: [ "George Jenkins <gvjenkins@gmail.com>", "Andy Block <andy@redhat.com>" ] | |
| authors: [ "George Jenkins <gvjenkins@gmail.com>", "Andrew Block <andy.block@gmail.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.
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
Outdated
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. |
Outdated
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
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.
included / done
Outdated
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?
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 have a TODO, need to go grab the link
Outdated
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
Outdated
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?
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've added the file lookup specifications.
TODO: is whether we want to follow the same tmpfs pattern for linux systems as the specification
Outdated
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.)
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.
mattfarina marked this conversation as resolved.
Show resolved
Hide resolved
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
Outdated
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.
Outdated
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)
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.
Outdated
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
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've added an HELM_EXPERIMENTAL_OCI_CONTAINERS_CONFIG feature flag.
The goal (as written into the HIP) is for this to become default true. The alternative is a permanent feature flag style config (cli flag or environment var). I think we should only introduce such iff we think (from user feedback and implementation) that introducing Container Tools config support can't be done in a non-intrusive/non-backwards compatible manner.
Uh oh!
There was an error while loading. Please reload this page.