Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Auth token file support #418

Merged
merged 20 commits into from
Mar 20, 2017
Merged

Auth token file support #418

merged 20 commits into from
Mar 20, 2017

Conversation

danielfm
Copy link
Contributor

@danielfm danielfm commented Mar 16, 2017

This PR adds support for authentication via static token files, as documented here.

Authentication via static token files, although not very flexible, is a pretty simple solution for users getting started with Kubernetes and/or want to have some mechanism to give access to different users, but do not want the complexity overhead of a more flexible solution, such as dex.

This PR will also be the foundation which #414 will be laid on.

Remaining TODOs:

  • Write tests
  • Document new feature (including required steps when upgrading an existing cluster, if any)
  • Spawn a few test clusters to validate the changes (with and without custom tokens to validate both scenarios)
  • Validate auth tokens file according to the current specification[1]

[1] The token file is a csv file with a minimum of 3 columns: token, user name, user uid, followed by optional group names. Note, if you have more than one group the column must be double quoted

Daniel Fernandes Martins added 5 commits March 16, 2017 12:21
Since we'll be encrypting things that are not TLS certificates, it's
time to rename the function in order to avoid any confusion.
The decryption of the token file should also happen provided it is put
at the correct location.

There are a couple of FIXME annotations that will be addressed in
future commits.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 16, 2017
@codecov-io
Copy link

codecov-io commented Mar 16, 2017

Codecov Report

Merging #418 into master will increase coverage by 1.66%.
The diff coverage is 69.62%.

@@            Coverage Diff            @@
##           master    #418      +/-   ##
=========================================
+ Coverage   38.53%   40.2%   +1.66%     
=========================================
  Files          30      31       +1     
  Lines        2351    2480     +129     
=========================================
+ Hits          906     997      +91     
- Misses       1324    1347      +23     
- Partials      121     136      +15
Impacted Files Coverage Δ
core/controlplane/config/config.go 65.18% <52.63%> (-0.24%) ⬇️
core/controlplane/config/token_config.go 72.41% <72.41%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95614a4...bd7569a. Read the comment docs.

@danielfm
Copy link
Contributor Author

danielfm commented Mar 17, 2017

@mumoshu et al: I still want to add extra validation to ensure operators can only create clusters if the provided auth tokens file is valid according to the specs, but overall I think this PR is ready for review.

Please let me know if there's anything I missed.

@danielfm danielfm changed the title [wip] Auth token file support Auth token file support Mar 17, 2017
@danielfm
Copy link
Contributor Author

All done! Waiting for reviews.

@danielfm
Copy link
Contributor Author

danielfm commented Mar 17, 2017

The tests are kinda flaky right now, this is the error I've been seeing:

failed to create cluster driver : failed to load node pool #0: failed getting AMI for config: error getting ami data for channel stable: failed to get AMI data: stable: invalid status code: 409

I'll try again soon.

Daniel Fernandes Martins added 4 commits March 17, 2017 18:21
Before generating the encrypted version of the auth token file, parse
it as a CSV and look for problems, such as comments and unmatching
number of columns.

If any problem is found, an error is raised, causing the cluster not
to be created.
@@ -1178,7 +1203,7 @@ func (c ControllerSettings) Valid() error {
func (c Experimental) Valid() error {
for _, taint := range c.Taints {
if taint.Effect != "NoSchedule" && taint.Effect != "PreferNoSchedule" {
return fmt.Errorf("Effect must be NoSchdule or PreferNoSchedule, but was %s", taint.Effect)
return fmt.Errorf("Effect must be NoSchedule or PreferNoSchedule, but was %s", taint.Effect)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch 👍

@@ -1045,7 +1070,7 @@ func (c DeploymentSettings) Valid() (*DeploymentValidationResult, error) {
return &DeploymentValidationResult{vpcNet: vpcNet}, nil
}

func (c DeploymentSettings) TLSAssetsEncryptionEnabled() bool {
func (c DeploymentSettings) AssetsEncryptionEnabled() bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for paying attention to the naming 👍

@mumoshu
Copy link
Contributor

mumoshu commented Mar 20, 2017

@danielfm LGTM 👍 Thanks for your contribution!

Btw sry for the flake. The fix for the it is planned in #396

@mumoshu mumoshu merged commit 181c918 into kubernetes-retired:master Mar 20, 2017
@mumoshu mumoshu added this to the v0.9.5-rc.4 milestone Mar 20, 2017
@danielfm danielfm deleted the auth-token branch March 21, 2017 14:48
mumoshu added a commit to mumoshu/kube-aws that referenced this pull request Mar 22, 2017
camilb added a commit to camilb/kube-aws that referenced this pull request Apr 5, 2017
* kubernetes-incubator/master: (29 commits)
  Emit errors when kube-aws sees unexpected keys in cluster.yaml Resolves kubernetes-retired#404
  Tag controller nodes appropriately with `kubernetes.io/role`. Resolves kubernetes-retired#370
  Make Container Linux AMI fetching a bit more reliable
  Stop locksmithd errors on etcd nodes
  Upgrade heapster to version 1.3.0 (kubernetes-retired#420)
  Auth token file support (kubernetes-retired#418)
  Update README.md
  Update README accordingly to the new git repo
  AWS China region support (kubernetes-retired#390)
  Conform as a Kubernetes Incubator Project
  Fixed typo in template
  upgrade aws-sdk to latest version Fix kubernetes-retired#388
  Upgrade Kubernetes version to v1.5.4
  Fix assumed public hostnames for EC2 instances in us-east-1
  Fix assumed public hostnames for EC2 instances in us-east-1
  typo
  fix: etcdDataVolumeEncrypted not creating encrypted volumes fixes kubernetes-retired#383
  Allow disabling wait signals fixes kubernetes-retired#371
  Update file paths in readme
  Fix an issue with glue security group documentation
  ...
kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Mar 27, 2018
Adds support for authentication via static token files, as documented [here](https://kubernetes.io/docs/admin/authentication/#static-token-file).

The token file is a csv file with a minimum of 3 columns: token, user name, user uid, followed by optional group names. Note, if you have more than one group the column must be double quoted.

Authentication via static token files, although not very flexible, is a pretty simple solution for users getting started with Kubernetes and/or want to have some mechanism to give access to different users, but do not want the complexity overhead of a more flexible solution, such as [dex](https://github.com/coreos/dex).

This change will also be the foundation which kubernetes-retired#414 will be laid on.

A more detailed change list follows:

* Renamed function

Since we'll be encrypting things that are not TLS certificates, it's
time to rename the function in order to avoid any confusion.

The function name suffix implied it created a file, which was not the
case.

* Updated test error message

* Renamed systemd unit to match name changes

* Defined a location in which to put the token auth file

The decryption of the token file should also happen provided it is put
at the correct location.

* Should only attempt to decrypt tokens if the auth token file exists

* Fixed typo

* Added tests

* Updated docs

* Fixed script

* Validate auth token file

Before generating the encrypted version of the auth token file, parse
it as a CSV and look for problems, such as comments and unmatching
number of columns.

If any problem is found, an error is raised, causing the cluster not
to be created.
kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Mar 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants