Add google cloud configuration object for kubeclient #88
Conversation
xldenis
force-pushed
the
gcp-auth-kubeclient
branch
from
46e6b8f
to
e1fb1ad
Compare
| @@ -28,7 +29,7 @@ def build_v1beta1_kubeclient(context) | |||
| end | |||
|
|
|||
| def _build_kubeclient(api_version:, context:, endpoint_path: nil) | |||
| config = Kubeclient::Config.read(ENV.fetch("KUBECONFIG")) | |||
| config = GoogleConfig.read(ENV.fetch("KUBECONFIG")) | |||
There was a problem hiding this comment.
I'm not sold about naming because someone reading this code could think that it only supports GKE config. What about KubeconfigWithGoogleSupport or whatever?
There was a problem hiding this comment.
I dont have any particular attachment to the name, though I'm not a huge fan of enormous names either.
There was a problem hiding this comment.
What do you think about GoogleConfig incorrectly telling someone that the tool only supports google auth?
There was a problem hiding this comment.
What about just ConfigWithGoogle ? I'm just not a huge fan of having KubernetesDeploy::KubeclientBuilder::KubeconfigWithGoogleSupport
| scopes = ['https://www.googleapis.com/auth/cloud-platform'] | ||
| authorization = Google::Auth.get_application_default(scopes) | ||
|
|
||
| authorization.apply({}) |
There was a problem hiding this comment.
Why does it need an empty apply? Can you add a comment to the code with the reason?
There was a problem hiding this comment.
Well apply adds the access token to a set of headers in a hash. Since we just want to recover the token we don't need to pass in anything.
| WebMock.allow_net_connect! | ||
| end | ||
|
|
||
| def test_auth_use_default_gcp_success |
There was a problem hiding this comment.
I think it also makes sense to check that the config works as usual if auth provider isn't GCP.
| class GoogleConfig < Kubeclient::Config | ||
| def fetch_user_auth_options(user) | ||
| if user['auth-provider'] && (user['auth-provider']['name'] == 'gcp') | ||
| { bearer_token: self.get_token } |
xldenis
force-pushed
the
gcp-auth-kubeclient
branch
6 times, most recently
from
559afa1
to
ec5fdbc
Compare
xldenis
force-pushed
the
gcp-auth-kubeclient
branch
from
ec5fdbc
to
1237b20
Compare
|
I've addressed the review comments the configuration is now called |
| end | ||
|
|
||
| def set_google_env_vars | ||
| ENV["GOOGLE_PRIVATE_KEY"] ||= "FAKE" |
There was a problem hiding this comment.
There's probably a reason why it's necessary, but I'd prefer them to be reset on teardown
There was a problem hiding this comment.
Its because the googleauth gem does a bunch of magic inspection to determine how to load configuration for a host (ie checking if creds are locally stored on disk, whether this is a GCE host, etc...).
Kubeclient doesn't support GCE authentication and there is currently no plan to merge the PR that would add support for it. To solve our problem I've added a small wrapper class based off the kubeclient PR. It provides a subclass of the
Kubeclient::Configwhich will look for the approriate auth type and if found will fetch a new (1 hour lifetime) token.Kubeclient GCE PR
@jeremywadsack, since you were the author of the original PR you may be interested in this.