Client setup (better login, config files and switch projects) #992

fabianofranz · 2015-02-12T14:47:44Z

No description provided.

deads2k · 2015-02-12T19:23:35Z

pkg/cmd/cli/config/smart_merge.go

Is this a straight copy of: https://github.com/openshift/origin/blob/master/pkg/cmd/experimental/login/smart_merge.go? Need to clean up the original?

Almost a straight copy, I just moved getUniqueName into this file.

----- Original Message -----

@@ -0,0 +1,157 @@
+package config

Is this a straight copy of:
https://github.com/openshift/origin/blob/master/pkg/cmd/experimental/login/smart_merge.go?
Need to clean up the original?

Reply to this email directly or view it on GitHub:
https://github.com/openshift/origin/pull/992/files#r24611203

Removed the original.

----- Original Message -----

@@ -0,0 +1,157 @@
+package config

Is this a straight copy of:
https://github.com/openshift/origin/blob/master/pkg/cmd/experimental/login/smart_merge.go?
Need to clean up the original?

Reply to this email directly or view it on GitHub:
https://github.com/openshift/origin/pull/992/files#r24611203

fabianofranz · 2015-02-13T20:49:13Z

pkg/cmd/cli/cmd/factory.go

@smarterclayton I need to intercept every API request in order to check for an eventual 401 Unauthorized and then be able to print a proper, user-friendly, message (you need to log in bla bla). Any feedback about this approach? Would you suggest something different than taking the clients and injecting it here in the Factory? Looks a little hacky, nothing protects us against people doing client.New / kclient.New directly.

You want to create a transport and pass it in to client.New on the config. Your transport should wrap http.DefaultTransport.

----- Original Message -----

if err != nil { return nil, nil, err }

oc, err := client.New(os)

transport, err := kclient.TransportFor(cfg)

@smarterclayton I need to intercept every API request in order to check for
an eventual 401 Unauthorized and then be able to print a proper,
user-friendly, message (you need to log in bla bla). Any feedback about
this approach? Would you suggest something different than taking the clients
and injecting it here in the Factory? Looks a little hacky, nothing protects
us against people doing client.New / kclient.New directly.

Reply to this email directly or view it on GitHub:
https://github.com/openshift/origin/pull/992/files#r24695535

Note that I need to wrap both the origin and the kube clients, so that it works when doing i.e. osc get <kubeobject>

You should be able to alter the ClientConfig before it is passed up

----- Original Message -----

if err != nil { return nil, nil, err }

oc, err := client.New(os)

transport, err := kclient.TransportFor(cfg)

Note that I need to wrap both the origin and the kube clients, so that it
works when doing i.e. osc get <kubeobject>

Reply to this email directly or view it on GitHub:
https://github.com/openshift/origin/pull/992/files#r24702754

fabianofranz · 2015-02-17T02:07:33Z

User login flow

Every osc command will try to use a config file from the expected locations (more info below). If a config file could not be found in any of the locations (and the individual configs were also not specified through flags like --server), commands will fail as "not configured". Example:

$ osc get templates
OpenShift is not configured. You need to run the login command in order to create a default config for your server and credentials:

  osc login [--username=<username>] [--password<password>] [--server=<server>]

You can also run this command again providing the path to a config file directly, either through the --config flag of the OPENSHIFTCONFIG environment variable.

The osc login command is the entry point to start the client configuration. Login will take several information from the user (server URL, username, password), either through flags or interactive prompt if flags were not provided, and will automatically create a config file in the home directory that will be used by every command after logging in.

The --project flag is also accepted by login and if provided the command will switch to the provided project after logging in.

Log in by providing username, password and server (and alternatively other optional flags that may apply), either through flags or interactive input.

$ osc login
Username: myself
Password: ********
Server: https://localhost:8443/
Logged into <server> as <username>.

$ osc login [--username <username>] [--password <password>] [--server <server>] [--project <project>]
Logged into <server> as <username>.

$ osc login [--username <username>] [--password <password>] [--server <server>] [--project <project>]
Already logged into <server> as <username>.

The command always checks for an existing and valid token for the given server and username and if exists, just make use of it. All flags are optional and if the command misses something that it needs and were not provided and not present in the context currently in use, the user will be prompted for input.

The server and username flags can also recognize existing clusters or users present in the config file in use by its keys. So users can also log in by providing clusters, users or contexts already known by the config file.

$ osc login [--context <context>]
Logged into <server> as <username>.

$ osc login [--username <user>]  [--password <password>] [--server <cluster>]
Logged into <server> as <username>.

TODO: Kerberos

Config loading order

Notice it may not make sense to support both the --kubeconfig and --openshiftconfig flags so we will probably only keep the latter. Same for the KUBECONFIG and OPENSHIFTCONFIG env vars. Alternative names are presented below.

~~--openshiftconfig or even~~ just --config flag
OPENSHIFTCONFIG ~~or OPENSHIFT_CONFIG~~ env var
.openshiftconfig file in current directory
~~~/.openshift/.openshiftconfig or just ~/.openshift/.config~~ ~/.config/openshift/.config file
~~--kubeconfig flag~~
KUBECONFIG env var
.kubeconfig file in current directory
~/.kube/.kubeconfig file

Progress

Login
- By username and password
- By existing context
- Save auth info to config file after logging in and make sure it's used
- Support all locations following the loading order (todo: handle file not writable)
- Switch to another user or server
- Reuse previous tokens when switching users
Config
- Config files provided by either flags, env vars, local or home directories (following the loading order above)
- OpenShift-specific configs
- Create a config file if one is not located in any of the supported locations
Other
- User-friendly message when running any command and auth fails
- Set the project in use after logging in
- Logout
- Move from experimental to osc login
- More tests
Project
- Command to switch between projects (osc project <project-name>)
- Must only use projects the user has access to (waiting for Make List Projects authorization aware #971)

FYI @smarterclayton @deads2k @jwforres @liggitt It's starting to look good, in case you want to check out and give it a try. ;)

smarterclayton · 2015-02-18T00:40:59Z

----- Original Message -----

User login flow

The --project flag is accepted by all flows and if provided the command
will switch to the provided project after logging in.

Log in by providing username, password and server (and alternatively other
optional flags that may apply). The command always checks for an existing
and valid token for the given server and username and if exists, just make
use of it. All flags are optional and if the command misses something that
it needs and were not provided and not present in the context currently in
use, the user will be prompted for input.
$ osc login [--username <username>] [--password <password>] [--server
<server>] [--project <project>]
Logged into <server> as <username>.

$ osc login
Username: myself
Password: ********
Server: https://localhost:8443/
Logged into <server> as <username>.

$ osc login [--username <username>] [--password <password>] [--server
<server>] [--project <project>]
Already logged into <server> as <username>.
The server and username flags can also recognize existing clusters or
users present in the config file in use by its keys. So users can also log
in by providing clusters, users or contexts already known by the config
file.
$ osc login [--context <context>]
Logged into <server> as <username>.

What happens if I pass user, context, and server?

$ osc login [--username ] [--password ] [--server ]
Logged into as .


3. TODO: Kerberos

Config loading order
-----

Notice it may not make sense to support *both* the `--kubeconfig` and
`--openshiftconfig` flags so we will probably only keep the latter. Same for
the `KUBECONFIG` and `OPENSHIFTCONFIG` env vars. Alternative names are
presented below.

1. `--openshiftconfig` or even just `--config` flag
2. `OPENSHIFTCONFIG` or `OPENSHIFT_CONFIG` env var

--config

./.openshiftconfig file

~/.openshift/.openshiftconfig or just ~/.openshift/.config file

Or .config/openshift/.config

We should be consistent with upstream though.

--kubeconfig flag

Don't support

KUBECONFIG env var

Support

./.openshiftconfig file

We should only support one config path

~/.kube/.kubeconfig file

We should support upstreams config path.

Progress

Login

By username and password

By existing context

Save auth info to config file after logging in and make sure it's
used

Support all locations following the loading order (todo: handle file
not writable)

Switch to another user or server

Reuse previous tokens when switching users

Config

Config files provided by either flags, env vars, local or home
directories (following the loading order above)

OpenShift-specific configs

Create a config file if one is not located in any of the supported
locations

Other

User-friendly message when running any command and auth fails

Set the project in use after logging in

Logout

Move from experimental to osc login

More tests

FYI @smarterclayton @deads2k @jwforres @liggitt It's starting to look good,
in case you want to check out and give it a try. ;)

Reply to this email directly or view it on GitHub:
#992 (comment)

fabianofranz · 2015-02-18T13:50:44Z

----- Original Message -----

----- Original Message -----
User login flow

The --project flag is accepted by all flows and if provided the command
will switch to the provided project after logging in.

Log in by providing username, password and server (and alternatively
other
optional flags that may apply). The command always checks for an existing
and valid token for the given server and username and if exists, just
make
use of it. All flags are optional and if the command misses something that
it needs and were not provided and not present in the context currently in
use, the user will be prompted for input.
$ osc login [--username <username>] [--password <password>] [--server
<server>] [--project <project>]
Logged into <server> as <username>.

$ osc login
Username: myself
Password: ********
Server: https://localhost:8443/
Logged into <server> as <username>.

$ osc login [--username <username>] [--password <password>] [--server
<server>] [--project <project>]
Already logged into <server> as <username>.
The server and username flags can also recognize existing clusters
or
users present in the config file in use by its keys. So users can also log
in by providing clusters, users or contexts already known by the config
file.
$ osc login [--context <context>]
Logged into <server> as <username>.
What happens if I pass user, context, and server?

I believe they should be mutually exclusive - if context were provided don't accept the other flags. Let's not fall into merging configs provided by different levels of the priority stack again.

$ osc login [--username ] [--password ] [--server
]
Logged into as .
3. TODO: Kerberos

Config loading order
-----

Notice it may not make sense to support *both* the `--kubeconfig` and
`--openshiftconfig` flags so we will probably only keep the latter. Same
for
the `KUBECONFIG` and `OPENSHIFTCONFIG` env vars. Alternative names are
presented below.

1. `--openshiftconfig` or even just `--config` flag
2. `OPENSHIFTCONFIG` or `OPENSHIFT_CONFIG` env var
--config

./.openshiftconfig file

~/.openshift/.openshiftconfig or just ~/.openshift/.config file

Or .config/openshift/.config

We should be consistent with upstream though.

--kubeconfig flag

Don't support

KUBECONFIG env var

Support

./.openshiftconfig file

We should only support one config path

~/.kube/.kubeconfig file

We should support upstreams config path.

Progress

Login

By username and password

By existing context

Save auth info to config file after logging in and make sure it's
used

Support all locations following the loading order (todo: handle
file
not writable)

Switch to another user or server

Reuse previous tokens when switching users

Config

Config files provided by either flags, env vars, local or home
directories (following the loading order above)

OpenShift-specific configs

Create a config file if one is not located in any of the supported
locations

Other

User-friendly message when running any command and auth fails

Set the project in use after logging in

Logout

Move from experimental to osc login

More tests

FYI @smarterclayton @deads2k @jwforres @liggitt It's starting to look
good,
in case you want to check out and give it a try. ;)

Reply to this email directly or view it on GitHub:
#992 (comment)

Reply to this email directly or view it on GitHub:
#992 (comment)

fabianofranz · 2015-02-19T14:29:19Z

@smarterclayton @deads2k Check how this proposal for the config file loading sound (notice the loading follows this order of priority).

First try using OpenShift locations:

path provided through the --config flag
path provided through the OPENSHIFTCONFIG env var
.openshiftconfig file in current directory
in home dir, ~/.config/openshift/.config file

If not found in any of the above, try the Kube locations (except for the --kubeconfig flag which we don't support:

path provided through the KUBECONFIG env var
.kubeconfig file in current directory
in home dir, ~/.kube/.kubeconfig file

Notice that if a config file were not found in any of the above locations we create one, using the OpenShift config file in home dir (in the proposal above, ~/.config/openshift/.config).

This is already implemented (although I need to do a big review of the code, a few things like the loaders are disconnected, etc), so when we close the deal I'll update it.

fabianofranz · 2015-03-14T01:17:16Z

@deads2k Ok, all 3 blockers addressed. Please check handle certs and tokens and report deserialization errors. All logic around wrapping errors was removed. The new errors.go is still there, but now only targeting to provide pretty messages and Is*Error kind of methods. Used only in login for now.

deads2k · 2015-03-16T12:12:08Z

pkg/cmd/cli/config/store.go

ReadFile can fail for reasons other than FileNotFound (permissions as an example). Those other kinds of failures should be reported. Only suppress the FileNotFound case.

Good catch, fixed.

deads2k · 2015-03-16T12:15:08Z

Two more comments and then we'll merge this take the rest as follow ons.

liggitt · 2015-03-16T13:31:07Z

hack/test-cmd.sh

shouldn't need to copy these files... the kubeconfig inlines the cert/key/roots data

Good catch, fixed.

liggitt · 2015-03-16T13:35:08Z

(fine to tweak the test sh files post-merge)

fabianofranz · 2015-03-16T15:22:04Z

@deads2k all set.

fabianofranz · 2015-03-16T17:32:54Z

@deads2k Project no longer required for a successful setup.

deads2k · 2015-03-16T17:45:45Z

There's a few TODOs, but this is a good starting point. [merge]

openshift-bot · 2015-03-16T17:50:36Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1188/) (Image: devenv-fedora_1053)

ups UPSTREAM: simplify client config loading logic

openshift-bot · 2015-03-16T18:20:49Z

Evaluated for origin up to aa0ba81

smarterclayton · 2015-03-16T18:45:14Z

Fabiano, email to public list please detailing the new function and how people can give feedback (i.e. how it works)

----- Original Message -----

There's a few TODOs, but this is a good starting point. [merge]

Reply to this email directly or view it on GitHub:
#992 (comment)

Merged by openshift-bot

…service-catalog/' changes from 568a7b9..8f07b7b 8f07b7b origin: add required patches ee57bfb Cleanup of ups broker example + making controller follow the OSB API (openshift#807) 45a11ed Revert "Rename our resources to have ServiceCatalog prefix (openshift#1054)" (openshift#1061) 4e47ec1 Rename our resources to have ServiceCatalog prefix (openshift#1054) 2bb334a Rebase on 1.7 API machinery (openshift#944) 5780b59 Run broker reconciler when spec is changed. (openshift#1026) 9c22d04 Merge branch 'pr/1006' d077915 check number of expected events before dereferencing to avoid panic (openshift#1052) 90d615f Merge branch 'pr/1055' bb6d6d8 fix log output to use formatted output (openshift#1056) c7abc81 Adding examples to the README ccc93c9 Remove different-org rule for LGTM (openshift#1050) be04cd5 Allow for a period in the GUID of the External ID (openshift#1034) 8c246df Make it so that binding.spec.secretName defaults to binding name (openshift#851) 6745418 Bump OSB Client (openshift#1049) 8346a0d apiserver etcd healthcheck as suggested to address k/k#48215 (openshift#1039) 11d0d4a use GKE's latest 1.6.X cluster version for Jenkins (openshift#1036) 7d71b5b Cross-build all the things! 8ec0874 RBAC setup behind the aggregator. (openshift#936) 0864a2e Upsert retry loop for Secret, set/check ownerReference for Secret owned by Binding (openshift#979) 6be9886 add info about weekly calls (openshift#1027) a242b26 add OSB API Header version flag (openshift#1014) 66e2ce6 Update REVIEWING doc with changes to LGTM process (openshift#1016) 699e016 Writing the returned progress description from the broker (openshift#998) 02642f4 Adding target to test on the host (openshift#1020) 78ca572 v0.0.13 (openshift#1024) 9e79ec2 use GKE's default K8S version for Jenkins (openshift#1023) d3c915a Fix curl on API server start error (openshift#1015) b50be75 Merge branch 'pr/1013' 2c98ba1 Using tag URLs 687f091 Parameterizing the priority fields 34ed5cd update apiregistration yaml to v1.7 final (openshift#1011) 91fa1ad make e2e look for pods' existence before checking status (openshift#1012) 0f90705 explicitly disable leader election if it is not enabled (openshift#965) f5761e7 controller-manager health checks (openshift#694) da260f2 Add logging for normal Unbind errors (openshift#992) 4c916a5 make the apiserver test use tls (openshift#991) 1a62ecc refactor reconcileBroker (openshift#986) cc179bc Add logging for normal Bind errors (openshift#993) a1458dd add parameterization for user-broker image to e2e tests (openshift#995) fb15891 Bump OSB client (openshift#1000) 79d5206 v0.0.12 (openshift#996) 39c7407 Merge branch 'pr/975' a553b2d Merge branch 'pr/974' d573339 reconcileBinding error checking (openshift#973) 39a1061 Making events and actions checks generic (openshift#960) 73136a4 Bump osb client (openshift#971) 878a987 reconcileInstance error checking in unit-tests 4991d57 reconcileBroker error checking in unit-tests 9ed6812 Extract methods for binding test setup (openshift#961) b69a1ee Make ups-broker return valid unbind response (openshift#964) 8b37d2f Releasing 0.0.11 (openshift#962) 52fec8b Merge branch 'pr/954' d49cdeb Swap client 445fa71 Add dependency on pmorie/go-open-service-broker-client 9f743b2 Instructions for enabling API Aggregation (openshift#895) 512508d Use correct infof calls in controller_manager (openshift#950) 77943ba fix regex that determines if a tag is deployable (openshift#947) 8a226b8 Updates for v0.0.10 release (openshift#943) REVERT: 568a7b9 origin build: add origin tooling git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog git-subtree-split: 8f07b7bbf3acb2b557f23596a92b5e775ae9321c

This simply differentiates a broker failure response as outlined in the spec versus some other unknown error.

fabianofranz force-pushed the osc_login_further branch 4 times, most recently from c2a2a9b to 1b15d90 Compare February 12, 2015 19:22

deads2k reviewed Feb 12, 2015
View reviewed changes

fabianofranz force-pushed the osc_login_further branch 5 times, most recently from 94cadd0 to 687d2d7 Compare February 13, 2015 20:19

fabianofranz reviewed Feb 13, 2015
View reviewed changes

fabianofranz force-pushed the osc_login_further branch 4 times, most recently from 34bc767 to 5aaea00 Compare February 17, 2015 01:14

fabianofranz force-pushed the osc_login_further branch 7 times, most recently from 9a458a4 to ba7c490 Compare February 19, 2015 02:38

fabianofranz force-pushed the osc_login_further branch 4 times, most recently from 5443865 to ae8cd9e Compare February 19, 2015 19:46

fabianofranz force-pushed the osc_login_further branch 2 times, most recently from fd7c54b to 2be67a3 Compare March 14, 2015 01:12

fabianofranz force-pushed the osc_login_further branch 2 times, most recently from 579dff2 to 585abf1 Compare March 15, 2015 16:40

deads2k reviewed Mar 16, 2015
View reviewed changes

liggitt reviewed Mar 16, 2015
View reviewed changes

fabianofranz force-pushed the osc_login_further branch 2 times, most recently from 9777ab5 to 1d2f6fb Compare March 16, 2015 15:20

fabianofranz added 4 commits March 16, 2015 15:16

UPSTREAM: loader allows multiple sets of loading rules

80bab11

ups UPSTREAM: simplify client config loading logic

Introducing client projects switching - 'osc project'

9a34d66

Introducing client login and setup - 'osc login'

2de77d1

Project is not required for a successful login

aa0ba81

fabianofranz force-pushed the osc_login_further branch from bcefbe8 to aa0ba81 Compare March 16, 2015 18:16

openshift-bot pushed a commit that referenced this pull request Mar 16, 2015

Merge pull request #992 from fabianofranz/osc_login_further

0ac00c6

Merged by openshift-bot

openshift-bot merged commit 0ac00c6 into openshift:master Mar 16, 2015

deads2k mentioned this pull request Mar 17, 2015

allow any number of kubeconfig files kubernetes/kubernetes#5541

Merged

fabianofranz mentioned this pull request Mar 31, 2015

Better error messages on osc #1516

Closed

jpeeler pushed a commit to jpeeler/origin that referenced this pull request Feb 1, 2018

Add logging for normal Unbind errors (openshift#992)

da260f2

This simply differentiates a broker failure response as outlined in the spec versus some other unknown error.

Client setup (better login, config files and switch projects) #992

Client setup (better login, config files and switch projects) #992

Uh oh!

Conversation

fabianofranz commented Feb 12, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabianofranz commented Feb 17, 2015

User login flow

Config loading order

Progress

Uh oh!

smarterclayton commented Feb 18, 2015

User login flow

Progress

Uh oh!

fabianofranz commented Feb 18, 2015

User login flow

Progress

Uh oh!

fabianofranz commented Feb 19, 2015

Uh oh!

fabianofranz commented Mar 14, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deads2k commented Mar 16, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liggitt commented Mar 16, 2015

Uh oh!

fabianofranz commented Mar 16, 2015

Uh oh!

fabianofranz commented Mar 16, 2015

Uh oh!

deads2k commented Mar 16, 2015

Uh oh!

openshift-bot commented Mar 16, 2015

Uh oh!

openshift-bot commented Mar 16, 2015

Uh oh!

smarterclayton commented Mar 16, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants