enable support of AuthenticationTokens, add Google OAuth Credentials Token Source

[dev-dsp]

Part of work from jenkinsci/kubernetes-plugin#588

[jglick]

CC @Vlatombe

[dev-dsp]

for me it seems harder to maintain separated plugin for credentials, because using AuthenticationTokens API requires some model classes for converting Credentials to them. Now these models have to be defined here in order to share them between kubernetes and kubernetes-cli plugins.
wouldn't it be easier to merge everything to kubernetes and use its auth core in kubernetes-cli?

[dev-dsp]

@jglick @Vlatombe build of related kubernetes-plugin changes succeded with custom version from this PR: jenkinsci/kubernetes-plugin#588

[dev-dsp]

Do I have to write getter-setter tests for model classes to prevent coverage level decrease?

[jglick]

Do I have to write getter-setter tests for model classes to prevent coverage level decrease?

I am not the plugin maintainer but: sounds like busy work to me. Useful tests either exercise realistic end-to-end scenarios; or exercise complex blocks of code which do not obviously do what they claim to be doing. Would a plausible innocent-looking but incorrect change make the test fail? If not, it is just noise.

[Vlatombe]

Looks good overall (see my notes). I'll give it a try.

[dev-dsp]

@Vlatombe could you please review / merge?

Thanks

[Vlatombe]

Some minor tweaks but lgtm otherwise.

[Vlatombe]

@maxlaverse @carlossg are you able to review this? Looks ready to merge to me but I don't have karma on this repo.

[Vlatombe]

I proposed some changes in dev-dsp#1 to take into account the newly released kubernetes-client-api plugin

[dev-dsp]

Thanks @Vlatombe, I've merged your changes

[maxlaverse]

Hi !
I'll need time to have a deeper look. I want to be sure we're introducing something that make sense for both kubernetes and kubernetes-cli plugins but I don't have the full picture yet.

This plugin provides credentials that can be retrieved with the TokenProducer interface by other plugins. I only had a quick look but if I read it correctly, this PR introduces a set of credentials with a new interface that returns a serialized kubeconfig file with hard-coded values for fields that are configurable in kubernetes-cli. I want to have an idea how to actually make use of this new version of the plugin in kubernetes-cli.

Regarding the tests coverage I agree with @jglick and couldn't think of sensible tests to add.

[dev-dsp]

Hi @maxlaverse,

For example, I don't see any need in wrapping kubectl calls instead of building configuration with the kubernetes-client library. So that's how I reworked that part in kubernetes-plugin: https://github.com/jenkinsci/kubernetes-plugin/pull/588/files#diff-440b9ecce96aa26f91c957eb465f2b36L93

I found similar setup code in https://github.com/jenkinsci/kubernetes-cli-plugin/blob/master/src/main/java/org/jenkinsci/plugins/kubernetes/cli/kubeconfig/KubeConfigWriter.java, that now can be simplified. buildKubeConfig method currently returns string with dumped yaml, but that may be reworked to return ConfigBuilder with default values that are pre-set by KubernetesAuth and may be overwritten by user options.

[maxlaverse]

Hi @dev-dsp !
I finally had some time to have a deeper look at the PRs.

For example, I don't see any need in wrapping kubectl calls instead of building configuration with the kubernetes-client library.

Well, kubectl is pretty safe to rely on. It has a public interface that is documented with a deprecation policy that makes it very likely for this plugin to be compatible with new versions of the CLI without requiring any change. I haven't found the documentation for the kubeconfig file format yet which makes me worry about potential breaking changes that would require manual intervention to support new versions of the CLI.

That being said, if I'm not mistaken kubectl is still the best option to modify kubeconfig elements (e.g current context) when using raw kubeconfig credentials. Else we would have to first parse the kubeconfig in order to get a ConfigBuilder which would require the library to be always up-to-date with all potential field names. If that's right, I see little gain in introducing the fabric8 dependency instead of using kubectl calls.

I also had a look at your original PR jenkinsci/kubernetes-plugin#580.
Do I understand it correctly and GoogleRobotCredentials is simply a TokenProducer with a hard-coded URL ?

[dev-dsp]

Hello @maxlaverse .

There were no known kubeconfig compatibility issues (at least I could not find any), and its structure can not just suddenly change at one time. Btw, possible compatibility issues cause things like continuous integration with functional tests, which we do have here (and in fabric8 pipelines, for sure).
What we actually could do on top of all changes is to add apiVersion to generated kubeconfig file, since I see that as the only option for k8s guys for introducing such fundamental changes that may break compatibility.

When stepping aside of the way with using ConfigBuilder, we would require to pass all the pipeline-related things like Launcher and Context to some methods of kubernetes-credentials.KubernetesAuth that will operate with workspace (!). I don't find that way clearer neither for development, nor for usage/debug. I already faced with similar issue: it is not possible to use TokenProducer apart from code which it was designed for (getToken(serviceAddress, caCertData, skipTlsVerify): see https://github.com/jenkinsci/kubernetes-plugin/pull/588/files#diff-25470b9af9ef3dbc01915f9ebffdad92R54)

Regarding TokenProducer - it is not possible to implement that interface for GoogleRobotCredentials class, since the last one is final: jenkinsci/kubernetes-plugin#580 (comment)

[dev-dsp]

@maxlaverse any thoughts on that?

[Vlatombe]

I've given some time to prepare a downstream PR in kubernetes-cli, I'll file a couple of fixes against this PR.

[Vlatombe]

@dev-dsp Could you edit the PR description to refer to the current PR jenkinsci/kubernetes-plugin#588 ?

[maxlaverse]

Hi @dev-dsp !
I'm sorry for the very late reply. Those past months have been unusually busy. I just landed on this PR again after having received a few GitHub notifications for activities on Jenkins related repositories.

I must admit my opinion has shifted since. I understand that the dependency to the kubectl binary might be annoying coming from a plugin that is solely meant to manage Kubernetes credentials. I was probably too focused on the other plugin I maintain, kubernetes-cli-plugin, which works very well with the current implementation and requires the binary anyway. Regarding the potential upcoming incompatibilities with the kubeconfig format, we shall see.

@Vlatombe I saw you tried to adapt that other plugin, thanks for that! In the meantime, are you still happy with this PR ? In that case I would merge and release to allow @dev-dsp to make progress with jenkinsci/kubernetes-plugin#580.

[msglueck]

Bump! We would like to use this! Can we somehow help you guys out?

[Vlatombe]

@dev-dep I have dev-dsp#2 waiting on top of this one.

[dev-dsp]

Oh sorry. @Vlatombe, I merged that.

[dev-dsp]

(just a friendly reminder)

[maxlaverse]

Thank you all!
Released in 0.5.0