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

Helm 3: registry login/logout #5597

Merged
merged 14 commits into from May 6, 2019

Conversation

@jdolitsky
Copy link
Member

commented Apr 13, 2019

Resolves #5461.

This leverages a new version of oras to add new Helm commands for logging into/out of a registry. I decided to keep everything under helm chart subcommand for now to keep all the registry work consistent (helm chart login, helm chart logout). We should have a planning session soon to discuss where all of the registry-related commands should end up.

Upon successful login, a new entry is placed in $HELM_HOME/registry/config.json, which is formatted and functions exactly like Docker CLI, including support for Docker credentials helpers such as osxkeychain (oras pulls in Docker as a library):

$ cat ~/.helm/registry/config.json
{
	"auths": {
		"localhost:5000": {}
	},
	"credsStore": "osxkeychain"
}

cc @sajayantony @SteveLasker @yuwaMSFT2 @shizhMSFT @jzelinskie @stevvooe

Example

Note: h3 in commands below refers to binary built from this change

First, start a registry with auth enabled:

$ htpasswd -cB -b auth.htpasswd myuser mypass
$ docker run -it --rm -p 5000:5000 \
    -v $(pwd)/auth.htpasswd:/etc/docker/registry/auth.htpasswd \
    -e REGISTRY_AUTH="{htpasswd: {realm: localhost, path: /etc/docker/registry/auth.htpasswd}}" \
    registry

Next, login using the new command (manually enter password):

$ h3 chart login -u myuser localhost:5000
Password:
Login succeeded

Save a chart in the local cache:

$ helm fetch stable/postgresql --untar
$ h3 chart save postgresql/ localhost:5000/mycharts/postgresql:latest
Name: postgresql
Version: 3.15.0
Meta: sha256:b32d02932fc66e26248c9f589ddb0667d91597db6800b05c0edc0ccb2eab1179
Content: sha256:2bfdce314c6de5fe58dc88d5d8785a2461fcc61ccc5ee640a3e502d59c352a83
latest: saved

Push chart to the registry:

$ h3 chart push localhost:5000/mycharts/postgresql:latest
The push refers to repository [localhost:5000/mycharts/postgresql]
Name: postgresql
Version: 3.15.0
Meta: sha256:b32d02932fc66e26248c9f589ddb0667d91597db6800b05c0edc0ccb2eab1179
Content: sha256:2bfdce314c6de5fe58dc88d5d8785a2461fcc61ccc5ee640a3e502d59c352a83
latest: pushed to remote (2 layers, 16.4 KiB total)

Logout of registry:

$ h3 chart logout localhost:5000
Logout succeeded

Try pushing again - DENIED!

$ h3 chart push  localhost:5000/mycharts/postgresql:latest
The push refers to repository [localhost:5000/mycharts/postgresql]
Name: postgresql
Version: 3.15.0
Meta: sha256:b32d02932fc66e26248c9f589ddb0667d91597db6800b05c0edc0ccb2eab1179
Content: sha256:2bfdce314c6de5fe58dc88d5d8785a2461fcc61ccc5ee640a3e502d59c352a83
Error: unexpected response: 401 Unauthorized

jdolitsky added some commits Apr 12, 2019

login/logout placeholders
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
use latest oras
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
use docker auth system
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
working login+push
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
working on tests
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
fix typo in htpasswd
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
rename credsfile to config.json
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
add flags for username/password
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>

@jdolitsky jdolitsky requested review from bacongobbler and adamreese Apr 13, 2019

@helm-bot helm-bot added the size/XL label Apr 13, 2019

@adamreese adamreese added the v3 label Apr 14, 2019

disable logout test broken on linux
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
@bacongobbler
Copy link
Member

left a comment

Looks good for a first pass!

Show resolved Hide resolved Gopkg.toml Outdated
Show resolved Hide resolved cmd/helm/root.go Outdated
}

// Adapted from https://github.com/deislabs/oras
func getUsernamePassword(usernameOpt string, passwordOpt string, passwordFromStdinOpt bool) (string, string, error) {

This comment has been minimized.

Copy link
@yuwaMSFT2

yuwaMSFT2 Apr 17, 2019

just one usability suggestion: Docker cli recently had a change that if user tries to do docker login myregistry.io without user name and password, it first tries to read from the credential store and use the stored one for login. This comes very handy. Will helm login support the same?

This comment has been minimized.

Copy link
@jdolitsky

jdolitsky Apr 17, 2019

Author Member

im not sure i understand - what is the "stored one", and why log into something that youve already authed against?

This comment has been minimized.

Copy link
@yuwaMSFT2

yuwaMSFT2 Apr 17, 2019

The first time you do log in, the credential is stored; the next time you do login without username/pwd, the stored credential is retrieved and used for login.

The login in certain systems has certain significance. For example, when you do login, a token may be retrieved which is used in subsequent requests. But the token may expire after some time. Doing login again basically retrieve a new token.

Or server can do some special logic on login, say refreshing cached permission, etc.

Or I simply forgot if I did login already:)

// TODO: handle errors
credentialsFile := filepath.Join(settings.Home.Registry(), registry.CredentialsFileBasename)
client, _ := auth.NewClient(credentialsFile)
resolver, _ := client.Resolver(context.Background())

This comment has been minimized.

Copy link
@yuwaMSFT2

yuwaMSFT2 Apr 17, 2019

It seems currently oras resolver only has default options. Do we plan to have a way to configure different resolver options, like PlainHTTP (for example running a test registry on HTTP but not locahost)?

This comment has been minimized.

Copy link
@jdolitsky

jdolitsky Apr 17, 2019

Author Member

I'd be hesitant to encourage any use of http from the onset. This can of course come later if needed

This comment has been minimized.

Copy link
@yuwaMSFT2

yuwaMSFT2 Apr 17, 2019

I agree with you. The point is not the PlainHTTP support. Just point out the possibility that user may want to extend the default resolver, which resolves the host name of the server, scheme, and auth.

I understand it's not necessary for now; just brought it up so we think about possibility of extension (probably plugin model).

Show resolved Hide resolved pkg/registry/client_test.go Outdated

jdolitsky added some commits Apr 22, 2019

upgrade to oras 0.4.0
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
re-enable logout test
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
@jdolitsky

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2019

Ok, updated to released oras version (0.4.0). Should be ok to merge.

@yuwaMSFT2 in regards to the other asks - in the interest in getting this merged, can you add something to the issue queue for us to continue this discussion? Then if it makes sense we can work on a follow-up

panic for uncaught errors
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
@yuwaMSFT2

This comment has been minimized.

Copy link

commented Apr 22, 2019

@jdolitsky Sure please go ahead with the merge. I can open separate issues.

@SteveLasker

This comment has been minimized.

Copy link

commented Apr 26, 2019

looks great Josh.
For the UX, knowing this is a placeholder, I’m wondering about
helm login
helm repo login
helm chart login feels like i’m logging into a chart, whatever that might mean

Great to see all the progress

@bacongobbler

This comment has been minimized.

Copy link
Member

commented Apr 26, 2019

for now, we're moving all of the OCI integration parts into helm chart * commands. We'll eventually move them into the mainline CLI (i.e. helm login) once we figure out the migration plan from chart repositories to OCI registries.

@SteveLasker

This comment has been minimized.

Copy link

commented Apr 26, 2019

Makes sense. I thought I remembered we came to a solution where both would work, but I'm sure there's some important details here to call out.

@bacongobbler bacongobbler added this to Awaiting Review in Helm 3 May 1, 2019

move login/logout to new registry subcommand
Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
@jdolitsky

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

I've gone ahead and moved these into a new helm registry subcommand (based on some side convos)

We now have

helm registry login
helm registry logout

and

helm chart pull
helm chart push
helm chart list
helm chart save
helm chart export
helm chart remove

We will likely link helm push to helm chart push and helm pull to helm chart pull, but moving the others may cause confusion (for example, helm login might indicate one is logging into a cluster vs. a registry)

@adamreese
Copy link
Member

left a comment

LGTM

@jdolitsky please squash before merging

Helm 3 automation moved this from Awaiting Review to LGTM May 6, 2019

@jdolitsky jdolitsky merged commit a12a396 into helm:dev-v3 May 6, 2019

2 checks passed

DCO DCO
Details
ci/circleci: build Your tests passed on CircleCI!
Details

Helm 3 automation moved this from LGTM to Done May 6, 2019

@jdolitsky jdolitsky deleted the jdolitsky:feat-v3/registry-login branch May 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
6 participants
You can’t perform that action at this time.