Support Google OAuth credentials for GKE auth using Google Service Accounts #580
Conversation
| <dependency> | ||
| <groupId>commons-io</groupId> | ||
| <artifactId>commons-io</artifactId> | ||
| <version>2.6</version> |
There was a problem hiding this comment.
Have to be careful here since this is part of Core. Probably you meant to use <exclude>s instead.
| <dependency> | ||
| <groupId>org.apache.httpcomponents</groupId> | ||
| <artifactId>httpclient</artifactId> | ||
| <version>4.5.5</version> |
There was a problem hiding this comment.
Ditto, or use the library plugin for this.
There was a problem hiding this comment.
I've added these deps here due to enforcer (Require upper bound dependencies error) and I am actually not sure how to apply excludes here.
Could you assist with that or share some info on using library plugin?
There was a problem hiding this comment.
If you needed to do this, the POM mistakes are probably upstream in the google-oauth plugin.
| @@ -240,7 +251,8 @@ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item item, @QueryPa | |||
| CredentialsMatchers.instanceOf(StandardUsernamePasswordCredentials.class), | |||
| CredentialsMatchers.instanceOf(TokenProducer.class), | |||
| CredentialsMatchers.instanceOf(StandardCertificateCredentials.class), | |||
| CredentialsMatchers.instanceOf(FileCredentials.class) | |||
| CredentialsMatchers.instanceOf(FileCredentials.class), | |||
| CredentialsMatchers.instanceOf(GoogleRobotCredentials.class) | |||
There was a problem hiding this comment.
This is not how we want to work with credentials. AFAIK there should be no need to touch the kubernetes plugin at all. Rather, the Google-related plugins should be implementing TokenProducer. That is how a generic plugin can support authentication with various credential types, including from AWS etc. I would have to refresh my memory on the details though.
There was a problem hiding this comment.
So, there is kubernetes-credentials-plugin, that defines TokenProducer. Should then my code be moved there, or google-oauth-plugin should implement that interface itself?
There was a problem hiding this comment.
There are bunch of credentials types implemented here: https://github.com/jenkinsci/kubernetes-credentials-plugin/tree/master/src/main/java/org/jenkinsci/plugins/kubernetes/credentials
One of them is TokenProducer implementation for OpenShift:
https://github.com/jenkinsci/kubernetes-credentials-plugin/blob/master/src/main/java/org/jenkinsci/plugins/kubernetes/credentials/OpenShiftBearerTokenCredentialImpl.java
There was a problem hiding this comment.
Not sure offhand, would need to spend time researching it. I had thought the official API here was via the authentication-tokens plugin, which allows for example docker-commons to pick up Registry credentials from multiple vendors, but now I see that kubernetes-credentials indeed defines its own interface, perhaps in ignorance of that system. @Vlatombe @stephenc opinions?
There was a problem hiding this comment.
I think we deprecated the OpenShift kind already, in #529. #268 is the older change. Asking for advice from @maxlaverse.
There was a problem hiding this comment.
authentication-tokens should be able to provide the mapping... you want to turn the credentials into something that knows how to add auth to the HTTP client library... that may mean providing implementations of that for all the supported credential types... but it also means that the support can be added by extension plugins rather than needing to modify the base all the time. See https://github.com/jenkinsci/gitea-plugin/blob/master/src/main/java/org/jenkinsci/plugin/gitea/authentication/GiteaAuthUserSource.java and https://github.com/jenkinsci/gitea-plugin/blob/d5aa571acd4fb9e5d2f9c39034b577b397980f0b/src/main/java/org/jenkinsci/plugin/gitea/GiteaSCMSource.java#L720 for an example
There was a problem hiding this comment.
I have a limited access to Internet for two weeks and didn’t really manage to go into that PR on my mobile @jglick
The only thing I can say is that all credential related classes have been moved into ‘kubernetes-credentials-plugins’. The OpenShift implementation is in that plugin as well iirc. You could probably do the same and implement the TokenProducer interface, and wouldn’t need any change here, right ?
That being said, I extracted this kubectl configuration feature from the ‘kubernetes-plugin’ in the ‘kubernetes-cli-plugin’ a while ago. If you want pipeline support and if there are any changes to do on those KubeCtlWrappper classes, you might want to open a PR there.
|
@jglick so, based on discussion above, would it be sufficient for now to move my changes to |
|
@dev-dsp that would be my understanding as well. |
|
I don't see any correct way to implement it in Implementing AuthenticationTokenSource interface however was much simplier and took ~70 loc in both kubernetes-credentials and kubernetes plugins (but with crutches to support all three methods - hardcoded plugins, AuthenticationTokenSource and TokenProducer). My suggestion is to merge my changes for now (since they are small and fit to the plugins current state), and after that rework all three plugins (k8s-credentials, k8s, and k8s-cli) to use |
|
It is always a pain to handle migration issues after the fact. I would recommend implementing a minimal support for AuthenticationTokens in kubernetes-plugin, to provide OauthToken in builder, or Then add your google oauth implementation directly to kubernetes-credentials, adding the required dependencies (google-oauth, authentications-token). Then existing implementations using TokenProducer could be gradually migrated to AuthenticationTokens. |
|
So I've opened jenkinsci/kubernetes-credentials-plugin#12 It contains classes defining different authentication types for Kubernetes along with Google OAuth Credentials token source. I already have code changes for |
|
@dev-dep Please open the PR, even if you reference the snapshot (the CI will fail but at least we'll be able to review). I filed jenkinsci/kubernetes-credentials-plugin#13 so that you will be able to refer to your changes using Jenkins incrementals (https://jenkins.io/blog/2018/05/15/incremental-deployment/) once it is merged. |
|
Superceded by #588 |
Support Google OAuth credentials for GKE auth using Google Service Accounts