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

Modify kubeadmin usage message, admins should not use kubeadmin via CLI #1513

Merged

Conversation

sallyom
Copy link
Contributor

@sallyom sallyom commented Apr 1, 2019

The kubeadmin user should only be used to temporarily access the
console, and only until an admin configures a proper Identity Provider.
There is no reason to login as the kubeadmin user via CLI. If oauth
is broken in a cluster, an admin can still access via CLI as system:admin,
but will not be able to access via kube:admin.

@enj https://jira.coreos.com/browse/AUTH-293

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 1, 2019
@sallyom sallyom force-pushed the modify-kubeadmin-msg branch 2 times, most recently from 5d10ae2 to 0d1b956 Compare April 1, 2019 18:50
@@ -427,8 +427,8 @@ func logComplete(directory, consoleURL string) error {
return err
}
logrus.Info("Install complete!")
logrus.Infof("Run 'export KUBECONFIG=%s' to manage the cluster with 'oc', the OpenShift CLI.", kubeconfig)
logrus.Infof("The cluster is ready when 'oc login -u kubeadmin -p %s' succeeds (wait a few minutes).", pw)
logrus.Infof("To access the cluster as the system:admin user, run 'export KUBECONFIG=%s'", kubeconfig)
Copy link
Member

@wking wking Apr 1, 2019

Choose a reason for hiding this comment

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

I think you want to mention oc in this line. Maybe:

logrus.Infof("To access the cluster as the system:admin user when using 'oc', run 'export KUBECONFIG=%s'", kubeconfig)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks good! thanks, updated

logrus.Infof("Run 'export KUBECONFIG=%s' to manage the cluster with 'oc', the OpenShift CLI.", kubeconfig)
logrus.Infof("The cluster is ready when 'oc login -u kubeadmin -p %s' succeeds (wait a few minutes).", pw)
logrus.Infof("To access the cluster as the system:admin user, run 'export KUBECONFIG=%s'", kubeconfig)
logrus.Info("Manage the cluster with 'oc', the OpenShift CLI. Run 'oc --help' for the menu.")
Copy link
Member

@wking wking Apr 1, 2019

Choose a reason for hiding this comment

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

Do we want this level of detail? If folks need to be reminded about --help, I'd much rather be linking them to external docs for using oc (or hope they say "huh, dunno what oc is. Ah, this next line is talking about a web console, that sounds like my style").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol yup, good point.

@wking
Copy link
Member

wking commented Apr 1, 2019

This looks good, but you need to bump here and here too.

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 2, 2019
@sallyom
Copy link
Contributor Author

sallyom commented Apr 2, 2019

@wking, updated w/ the other 2 files, thanks

The kubeadmin user should only be used to temporarily access the
console, and only until an admin configures a proper Identity Provider.
There is no reason to login as the kubeadmin user via CLI.  If oauth
is broken in a cluster, an admin can still access via CLI as system:admin,
but will not be able to access via kube:admin.
Copy link
Member

@wking wking left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 2, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sallyom, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 2, 2019
@openshift-merge-robot openshift-merge-robot merged commit 3d904d3 into openshift:master Apr 2, 2019
wking added a commit to wking/openshift-installer that referenced this pull request Apr 5, 2019
This was added in 4033577 (Append router CA to cluster CA in
kubeconfig, 2019-02-12, openshift#1242), where the motivation was [1]:

  With users created with an identity-provider, OAuth flow goes
  through router and router-ca is not trusted. This prohibits the user
  from using oc login from command line, without manually appending
  the router ca to the cluster ca. The openshift-ingress-operator
  creates the router-CA and this is not available until the very end
  of an install.

  This PR adds the router CA from configmap router-ca -n
  openshift-config-managed to the kubeconfig
  certificate-authority-data. This allows an identity-provided user to
  oc login from the terminal.

But the admin kubeconfig is already authenticated, so folks shouldn't
be using 'oc login' with that kubeconfig.  For example, see dee6929
(Modify kubeadmin usage message, admins should not use kubeadmin via
CLI, 2019-04-01, openshift#1513).  That leaves us with no use case for this
modification, and making the finish code watch-only sets us up to
rename away from the user-provided-infrastructure subcommand.

[1]: openshift#1242 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants