Add support for client-type ExecCredentials #453
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cben
reviewed
Jul 31, 2020
lib/kubeclient/exec_credentials.rb
Outdated
| return if has_client_credentials | ||
|
|
||
| has_token = status.key?('token') | ||
| raise 'exec plugin didn\'t return a token' unless has_token |
There was a problem hiding this comment.
At this point, I think you can say it returned neither token nor a client certificate.
There was a problem hiding this comment.
Done. Also added check to ensure a token XOR client creds are returned.
cben
reviewed
Jul 31, 2020
lib/kubeclient/config.rb
Outdated
| @@ -142,16 +149,15 @@ def fetch_user_key_data(user) | |||
| File.read(ext_file_path(user['client-key'])) | |||
| elsif user.key?('client-key-data') | |||
| Base64.decode64(user['client-key-data']) | |||
| elsif user.key?('clientKeyData') | |||
| user['clientKeyData'] | |||
There was a problem hiding this comment.
It's slightly confusing to have 2 similarly named keys, one base64, one not.
Although both spellings come from k8s.
The reuse of user is clever but WDYT of putting it in a sub-field instead?
user.exec_result = ExecCredentials.run(exec_opts)
It does mean you'll have to check .token in 2 places, but I think the data flow will be more readable.
|
This is great, thanks! |
cben
added a commit
that referenced
this pull request
Aug 3, 2020
Add support for client-type ExecCredentials
Merged
cben
reviewed
Aug 3, 2020
| @@ -4,6 +4,10 @@ Notable changes to this project will be documented in this file. | |||
| The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). | |||
| Kubeclient release versioning follows [SemVer](https://semver.org/). | |||
|
|
|||
| ## 4.9.0 - 2020-08-03 | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This library currently supports
token-type ExecCredentials, but there is actually another flavor for TLS client auth. With this flavor, thestatusfield includes PEM-encodedclientKeyDataandclientCertificateDatarather than atoken. Reference:This PR adds support for TLS client auth while maintaining support for token ExecCredentials. To accomplish this, the credentials provided by the
statusfield, regardless of flavor, are passed around as if it were auser.