Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No default behavior for CONJUR_VERSION parameter #70

Closed
JfcAtCyberArk opened this issue Jan 30, 2020 · 9 comments · Fixed by cyberark/secretless-broker#1148 or cyberark/secrets-provider-for-k8s#69

Comments

@JfcAtCyberArk
Copy link

Hi there,
There is no default behavior if the CONJUR_VERSION is not set, or not 4 or 5.
Code reference:
https://github.com/cyberark/conjur-authn-k8s-client/blob/master/pkg/authenticator/authenticator.go#L230

In that case, the authenticator fails with the following entry in the logs:
CAKC011I Login request to:

It's really not intuitive.

Actual version of DAP is 11.2.1 - to be aligned with CyberArk suite, version 4 is really far now.
I think a default behavior would make sense.

Following DockerHub links may also be required an update:
https://hub.docker.com/r/cyberark/conjur-kubernetes-authenticator/
https://hub.docker.com/r/cyberark/conjur-authn-k8s-client

Many thanks!

JFC

@izgeri
Copy link
Contributor

izgeri commented Jan 31, 2020

Thanks for adding this issue @JfcAtCyberArk - I 100% agree and generally think most of our integrations should have this behavior (default to 5, but allow specifying 4 if using an older version of Conjur Enterprise v4.x)

To be totally clear, CONJUR_VERSION should be set to 5 unless you're using an older version of Conjur Enterprise v4.x (in which case, CONJUR_VERSION=4). The CONJUR_VERSION variable really refers to the Conjur API version being used by the server, which is 5 for DAP 10.x+ and Conjur OSS.

I'll see what we can do about getting a fix for this into our queue to address soon.

@izgeri
Copy link
Contributor

izgeri commented Feb 10, 2020

Note:

AC for making this update:

  • Update the authn-k8s client to default the CONJUR_VERSION value to 5, unless otherwise specified
  • Ensure there's a new version available with this update
  • Submit PRs to k8s secrets provider, secretless, and container-dap seedfetcher to bump the versions in those projects
  • Review the documentation to see which pages in the conjur and/or secretless docs give this requirement, and file docs issues to remove this requirement from the documentation as appropriate

@BradleyBoutcher BradleyBoutcher self-assigned this Feb 12, 2020
@izgeri
Copy link
Contributor

izgeri commented Feb 18, 2020

These changes are available in v0.16.1 (I actually think this functionality was available as early as v0.13.0), but we'll aim to propagate them to the downstream projects with PRs now (cc @BradleyBoutcher)

sgnn7 added a commit to cyberark/secretless-broker that referenced this issue Feb 19, 2020
…nt#70-bump-authn-k8s-client-version

Bump authn-k8s client to v0.16.1
izgeri pushed a commit to cyberark/secrets-provider-for-k8s that referenced this issue Mar 2, 2020
-bump-authn-k8s-client-version

Bump authn-k8s client version
@JfcAtCyberArk
Copy link
Author

Hi @izgeri @BradleyBoutcher,
I think the current implementation of the switch is confusing to me.

What I meant by a "Default Behavior" was that any other value than '4' would use '5' behind the scenes but it is not the case.

I just ran into an issue because my authenticator was failing when I provided it CONJUR_VERSION=11.4.0 and I had to come back to the source code to understand why.

I feel like I did not explain enough what I meant by a default behavior, sorry for that.
Still, WDYT? Is there a point refusing a value different from 4 or 5?

Please let me know!

JFC

@JfcAtCyberArk
Copy link
Author

I also think dockerhub doc is not really clear.
Maybe it should mention that the variable is optional - and that the only reason to use it is to set it to 4.

@izgeri
Copy link
Contributor

izgeri commented Apr 23, 2020

@JfcAtCyberArk all of our clients and tools expect the value to be either 4 or 5, and no other value.

At current, the authn-k8s client defaults to 5 if no value is specified, and errors if the value is not 4 or 5.

I've updated both DockerHub pages for this project to list CONJUR_VERSION at the bottom of the table, be clear that it's optional, and that '4' should only be used for Conjur Enterprise v4. I'll plan to remove these entries altogether once we deprecate authn-k8s for Conjur Enterprise v4, which I expect will happen soon.

Hope this makes sense, but please feel free to weigh in if you have any other suggestions.

@JfcAtCyberArk
Copy link
Author

@izgeri It totally makes sense!
I had something in mind where whatever is set for CONJUR_VERSION, "5" will be used except if "4" was set. Then, any Conjur/DAP user seeing a script or a plugin where there is a "CONJUR_VERSION" variable to set could set it to the version he is actually using.
I understand it is not the best approach because the "CONJUR_VERSION" variable is intended to disappear and at one point, users will have to stop using it. The sooner the better.
Thanks a lot for updating the DockerHub pages!

JFC

@izgeri
Copy link
Contributor

izgeri commented Apr 23, 2020

@JfcAtCyberArk you will be glad to hear we are trying to default this to 5 everywhere possible, and to remove it from the DAP / Conjur OSS documentation once we do. If there is a project where this is especially valuable for you and you don't see an existing gh issue for that change, please file one and let us know so we can update it soon!

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