Skip to content

Add create helpers for users, identities, and mappings from the CLI#8715

Merged
openshift-bot merged 1 commit intoopenshift:masterfrom
liggitt:create-user
May 4, 2016
Merged

Add create helpers for users, identities, and mappings from the CLI#8715
openshift-bot merged 1 commit intoopenshift:masterfrom
liggitt:create-user

Conversation

@liggitt
Copy link
Copy Markdown
Contributor

@liggitt liggitt commented May 2, 2016

Adds creation helpers for User, Identity, and UserIdentityMapping API objects. Lets you do:

oc create user ajones --full-name="Adam Jones"
oc create identity acme_ldap:adamjones
oc create useridentitymapping acme_ldap:adamjones ajones

oc describe user ajones

Name:           ajones
Created:        2 seconds ago
Labels:         <none>
Annotations:    <none>
Full Name:      Adam Jones
Identities:     acme_ldap:adamjones

This allows scripting user provisioning for use with the "lookup" mappingMethod.

Note that the identity provider name and identity provider username must exactly match what is determined from the IDP at login time.

@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented May 2, 2016

[test]

@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented May 2, 2016

@stevekuznetsov

====


== oc create useridentitymapping
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure if this is the best way to expose this feature to users. Are we expecting users to create identities and not map them? When are users creating identities and not using identity providers? I'm not against having these commands in the CLI as I'm sure there are a lot of complex use cases that they enable, but I wonder if we need something different for Joe User.

Copy link
Copy Markdown
Contributor Author

@liggitt liggitt May 2, 2016

Choose a reason for hiding this comment

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

Are we expecting users to create identities and not map them? When are users creating identities and not using identity providers?

I would expect them to use these in a script to provision a user one piece at a time, in conjunction with a cluster configured with the "lookup" mappingMethod

I wonder if we need something different for Joe User.

These are not for Joe User... only a cluster admin would have the ability to even run these. As you point out, most of the time, users/identities will get auto-created on login. A cluster admin that needs these is already in advanced territory, so I'd rather just give them the primitives to build exactly what they need.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Understood. Let's be clear in the help text about when/why you'd use these tools. Otherwise code LGTM. Lots of boilerplate for create, but I'm not really sure if we want or need a better abstraction.

@smarterclayton
Copy link
Copy Markdown
Contributor

LGTM

UserRecommendedName = "user"

userLong = `
Create a user.`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Expound on the help to indicate when I'd want to do use this (and the other) commands. This is only likely to be used if my master-config is set to avoid any auto-provisioning, right? We don't want people thinking they should do this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated help

@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented May 3, 2016

[merge]

@openshift-bot
Copy link
Copy Markdown
Contributor

Evaluated for origin test up to 2ab359c

@openshift-bot
Copy link
Copy Markdown
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3553/)

@deads2k
Copy link
Copy Markdown
Contributor

deads2k commented May 3, 2016

since you're retagging anyway, how about a test to make sure this always creates a valid object in test/cmd/basicresources. Nothing fancy, just a default object in case validation rules change we don't want this to rot.

@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented May 3, 2016

are the created objects in test/cmd/admin.sh not sufficient?

@deads2k
Copy link
Copy Markdown
Contributor

deads2k commented May 3, 2016

are the created objects in test/cmd/admin.sh not sufficient?

Didn't notice them.

@smarterclayton
Copy link
Copy Markdown
Contributor

smarterclayton commented May 4, 2016 via email

@bparees
Copy link
Copy Markdown
Contributor

bparees commented May 4, 2016

Standard error from the command: error: no match for
"centos/ruby-22-centos7"

Looks like a registry flake to me.

@smarterclayton
Copy link
Copy Markdown
Contributor

Can you prove it in the openshift logs? We haven't had a flake recently, I
want to be sure.

On Tue, May 3, 2016 at 9:03 PM, Ben Parees notifications@github.com wrote:

'''Standard error from the command: error: no match for
"centos/ruby-22-centos7" '''

Looks like a registry flake to me.

Ben Parees | OpenShift

On May 3, 2016 20:58, "Clayton Coleman" notifications@github.com wrote:

@mfojtik @bparees

https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5796/
failed due to something build related


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8715 (comment)


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#8715 (comment)

@liggitt
Copy link
Copy Markdown
Contributor Author

liggitt commented May 4, 2016

not sure if this is related:

I0503 18:39:56.364203 12073 importer.go:410] unable to access tag "latest" for repository &importer.importRepository{Ref:api.DockerImageReference{Registry:"", Namespace:"centos", Name:"ruby-22-centos7", Tag:"", ID:""}, Registry:(*url.URL)(0xc823251a70), Name:"centos/ruby-22-centos7", Insecure:false, Tags:[]importer.importTag{importer.importTag{Name:"latest", Image:(*api.Image)(nil), Err:error(nil)}}, Digests:[]importer.importDigest(nil), MaximumTags:0, AdditionalTags:[]string(nil), Err:error(nil)}: errcode.Errors{errcode.Error{Code:1011, Message:"manifest unknown", Detail:map[string]interface {}{"DriverName":"s3", "Enclosed":map[string]interface {}{"Op":"Get", "URL":"https://s3-external-1.amazonaws.com/docker-images-prod/registry-v2/docker/registry/v2/repositories/centos/ruby-22-centos7/_manifests/revisions/sha256/2ddda7d210e4355c40672721fa8e51f4501a8cdf74aa27e0a56013f5f1c20188/link", "Err":map[string]interface {}{}}}}}

@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented May 4, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5801/) (Image: devenv-rhel7_4092)

@openshift-bot
Copy link
Copy Markdown
Contributor

Evaluated for origin merge up to 2ab359c

@bparees
Copy link
Copy Markdown
Contributor

bparees commented May 4, 2016

what @liggitt found seems likely to be the cause. My only question would be when that error occurs, what is returned to new-app? If new-app gets "invalid", "notfound", or "unauthorized" then it gives up. If it got any other error, it should have fallen back to querying the docker registry directly.

i'm guessing that error message resulted in a "not found" or "unauthorized" response, but maybe it shouldn't have.

@bparees
Copy link
Copy Markdown
Contributor

bparees commented May 4, 2016

confirmed in the code, importer.go is going to return that as a NotFound based on the Manifest Unknown error code it received. I'm still not clear on why importer.go got the error it did, but that's why new-app ultimately failed.

@smarterclayton
Copy link
Copy Markdown
Contributor

Ok, maybe we chalk this up to "a push happened at the same time" or "the
docker hub sucks"

On May 3, 2016, at 11:12 PM, Ben Parees notifications@github.com wrote:

confirmed in the code, importer.go is going to return that as a NotFound
based on the Manifest Unknown error code it received. I'm still not clear
on why importer.go got the error it did, but that's why new-app ultimately
failed.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#8715 (comment)

@openshift-bot openshift-bot merged commit 9a867c9 into openshift:master May 4, 2016
@liggitt liggitt deleted the create-user branch May 5, 2016 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants