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

Inconsistent use of .kubernetes_auth file format and code #2302

Closed
erictune opened this issue Nov 11, 2014 · 16 comments · Fixed by #2340
Closed

Inconsistent use of .kubernetes_auth file format and code #2302

erictune opened this issue Nov 11, 2014 · 16 comments · Fixed by #2340

Comments

@erictune
Copy link
Member

Several things do or will use a .kubernetes_auth file:

  • kubectl
  • cmd/e2e/e2e.go
  • kubecfg
  • I'd like kubelet to use it too.
  • I'd like things in pods that talk to the API to use it too (e.g scheduler running in a pod on a non-master node).

They use different ways to specify its location:

  • kubectl has a persistent flag called auth-path
  • e2e test uses --auth_config
  • kubecfg uses --auth

There are several routines for loading the file:

  • kubecfg uses kubecfg.LoadAuthInfo, which does prompting for user/pass if the file does not exist
  • kubectl uses kubectl.LoadAuthInfo, which is identical to kubecfg code.
  • e2e test uses ``kubecfg.LoadAuthInfo`

There are several format definitions:

  • kubecfg uses kubecfg.AuthInfo
  • kubectl uses kubectl.AuthInfo which is identical to kubcfg's
    • e2e test uses kubecfg.AuthInfo indirectly.
      The Marshalled file contents then get copied from the kubec*.AuthInfo into the almost-but-not-quite same-named fields of Config in pkg/client.

Questions:

  1. Can we standardize on a flag name for the path to the .kubernetes_auth file?
  2. Should there be a single format for a .kubernetes_auth file?
  3. Should the go type that defines that format be the same file or a copy of the code in pkg/client? If the former, should the type in pkg/client be put in a less obscure location than pkg/client/helpers.go.
  4. Should there be a single parser for a .kubernetes_auth file, to also be used by kubelet, scheduler, etc?
@erictune
Copy link
Member Author

@ghodss @smarterclayton @derekwaynecarr I think you all might know something about this.

@lavalamp
Copy link
Member

Opinions:

  1. -auth-path
  2. Yes; pkg/kubectl's (kubecfg is deprecated), but move it to pkg/client.
  3. The type should exist in one place, somewhere in pkg/client
  4. Yes.

@derekwaynecarr
Copy link
Member

I thought configuration information was going to be centralized and kubernetes_auth was being moved?

@erictune
Copy link
Member Author

@derekwaynecarr pointer to discussion or elaborate please.

@derekwaynecarr
Copy link
Member

@erictune - sorry, see #1755 (comment)

@erictune
Copy link
Member Author

Okay, reading #1755 it seems like there are two use cases here:

  • Remember arbitrarily many preferences, including, but not at all limited to, auth. For use by a human-centric tool like kubectl.
  • Distribute auth info in a standard format to clients in pods. (e.g. for Service account proposal. #2297) Needs to work for golang client (pkg/client) and any conforming alternate-language implementations.

There is a slight overlap because people might want to run kubectl in a pod from a bash script. But I think that can be addressed with a flag or subcommand to kubectl.

So, what I think makes sense is to:

  • move auth file format definition and parsing and client factory to pkg/client/authfile
  • leave Kubectl as is so it can evolve its file format as needed.

@smarterclayton
Copy link
Contributor

Makes sense

On Nov 12, 2014, at 12:53 PM, Eric Tune notifications@github.com wrote:

Okay, reading #1755 it seems like there are two use cases here:

Remember arbitrarily many preferences, including, but not at all limited to, auth. For use by a human-centric tool like kubectl.
Distribute auth info in a standard format to clients in pods. (e.g. for #2297) Needs to work for golang client (pkg/client) and any conforming alternate-language implementations.
There is a slight overlap because people might want to run kubectl in a pod from a bash script. But I think that can be addressed with a flag or subcommand to kubectl.

So, what I think makes sense is to:

move auth file format definition and parsing and client factory to pkg/client/authfile
leave Kubectl as is so it can evolve its file format as needed.

Reply to this email directly or view it on GitHub.

@ghodss
Copy link
Contributor

ghodss commented Nov 12, 2014

@erictune So you're proposing is that the file formats may be different, but ultimately even kubectl would use pkg/client/authfile to create a client? Or authfile is used for everything but kubectl, which is on its own? Either is fine with me, I would just want to keep the flexibility that kubectl would be allowed to keep all of its connection/auth information in a format best suited for it, e.g. in the format described in #1755 (comment).

@erictune
Copy link
Member Author

I agree with you that "kubectl would be allowed to keep all of its connection/auth information in a format best suited for it".

But, it will need to have an option to import from the format defined by pkg/client/authfile.

@erictune
Copy link
Member Author

By import, I mean read file X, and either use just for current command, or save in kubectl's own file for later reuse.

@ghodss
Copy link
Contributor

ghodss commented Nov 12, 2014

Sounds good.

@erictune
Copy link
Member Author

#2340 removes the duplicate format definitions and some duplicate code.

Still need to standardize on flag and search path for the .kubernetes_auth file.

@erictune
Copy link
Member Author

And figure out what to do with client.BindClientConfigFlags.

@erictune
Copy link
Member Author

BindClientConfigFlags has flags that correspond to most of the fields of the authfile, plus --master.
But, we should get --master from the kubernetes service so that it allows for master pod changes, even in setups without DNS.

@smarterclayton
Copy link
Contributor

As we've been centralizing our Openshift commands I've been moving things into this pattern:

https://github.com/openshift/origin/blob/master/pkg/cmd/util/clientcmd/clientcmd.go

So a client cli binds their flags the same way to get a client object against a master. I was considering moving it upstream once Brendan's standalone was done. Things left include making it respect an authfile and other concerns. Removing bind can follow that.

On Nov 12, 2014, at 8:12 PM, Eric Tune notifications@github.com wrote:

And figure out what to do with client.BindClientConfigFlags.


Reply to this email directly or view it on GitHub.

@smarterclayton
Copy link
Contributor

We should take env consistently in place of arguments - I opened spf13/cobra#33 to make that cleaner.

On Nov 12, 2014, at 8:14 PM, Eric Tune notifications@github.com wrote:

BindClientConfigFlags has flags that correspond to most of the fields of the authfile, plus --master.
But, we should get --master from the kubernetes service so that it allows for master pod changes, even in setups without DNS.


Reply to this email directly or view it on GitHub.

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 a pull request may close this issue.

5 participants