Skip to content

Add support for Kubernetes username/password auth#2308

Merged
silvin-lubecki merged 1 commit into
docker:masterfrom
simonferquel:support-username-password
Feb 6, 2020
Merged

Add support for Kubernetes username/password auth#2308
silvin-lubecki merged 1 commit into
docker:masterfrom
simonferquel:support-username-password

Conversation

@simonferquel

Copy link
Copy Markdown
Contributor

- What I did
Added support for username/password Kubernetes authentication. The main motivation being to support k3s Kubernetes distributions.

- How I did it
Don't discard username/password fields when loading a kubeconfig file

- How to verify it
The PR comes with a unit test (same style as gke and eks test cases)

- Description for the changelog

  • Docker stack --orchestrator=kubernetes support now support username/password Kubernetes authentication (adding support for k3s)

This is required for supporting some Kubernetes distributions such as
rancher/k3s.

It comes with a test case validating correct parsing of a k3s kubeconfig
file

Signed-off-by: Simon Ferquel <[email protected]>
@simonferquel
simonferquel force-pushed the support-username-password branch from a316640 to 17e651d Compare February 4, 2020 10:31

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SGTM

not a big fan of the plain username/password storing, but I guess that's something on the kubernetes side (and I can see it come in handy for simple setups / local development)

}
}
var usernamePassword *UsernamePassword
if clientcfg.Username != "" || clientcfg.Password != "" {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need the check here? Of both are empty, we'll just set an empty string (which is the default for strings)

oh, nevermind; you created a pointer so that we don't set the UsernamePassword field if we don't use it

@@ -21,6 +21,13 @@ type EndpointMeta struct {
DefaultNamespace string `json:",omitempty"`
AuthProvider *clientcmdapi.AuthProviderConfig `json:",omitempty"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious; what's AuthProvider used for; is username/password not some form of that? (looking at https://github.com/kubernetes/client-go/blob/master/tools/clientcmd/api/types.go#L153-L177 it also has some sanitisation built in, so was wondering)

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.

AuthProvider is for authentication plugins (such as GKE authentication plugin). They allow to put arbitrary json in the kubeconfig file and feed the plugin with that so that it can setup the rest.Client accordingly. It is not related to the basic auth scheme.

@simonferquel

Copy link
Copy Markdown
Contributor Author

@silvin-lubecki would probably want to have a look as well.

@silvin-lubecki silvin-lubecki left a comment

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.

LGTM, thank you @simonferquel !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants