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

#238: support client.authentication.k8s.io/v1beta1 ExecCredential #512

Merged
merged 11 commits into from
Mar 8, 2019
Prev Previous commit
Next Next commit
Handle environment variables.
  • Loading branch information
jglick committed Mar 4, 2019
commit a4163e252326f84bcd73daf191ab3678adcd3a93
45 changes: 27 additions & 18 deletions util/src/main/java/io/kubernetes/client/util/KubeConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ public String getAccessToken() {
* href="https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins">
* Authenticating » client-go credential plugins</a>
*/
@SuppressWarnings("unchecked")
private String tokenViaExecCredential(Map<String, Object> execMap) {
if (execMap == null) {
return null;
Expand All @@ -260,8 +261,28 @@ private String tokenViaExecCredential(Map<String, Object> execMap) {
return null;
}
String command = (String) execMap.get("command");
List<Map<String, String>> env = (List) execMap.get("env");
List<String> args = (List) execMap.get("args");
JsonElement root = runExec(command, (List) execMap.get("args"), (List) execMap.get("env"));
if (root == null) {
return null;
}
// TODO verify .apiVersion and .kind = ExecCredential
JsonObject status = root.getAsJsonObject().get("status").getAsJsonObject();
JsonElement token = status.get("token");
if (token == null) {
// TODO handle clientCertificateData/clientKeyData
// (KubeconfigAuthentication is not yet set up for that to be dynamic)
log.warn("No token produced by {}", command);
return null;
}
return token.getAsString();
// TODO cache tokens between calls, up to .status.expirationTimestamp
// TODO a 401 is supposed to force a refresh,
// but KubeconfigAuthentication hardcodes AccessTokenAuthentication which does not support that
// and anyway ClientBuilder only calls Authenticator.provide once per ApiClient;
// we would need to do it on every request
}

private JsonElement runExec(String command, List<String> args, List<Map<String, String>> env) {
// TODO relativize command to basedir of config file (requires KubeConfig to be given a basedir)
List<String> argv = new ArrayList<>();
argv.add(command);
Expand All @@ -270,7 +291,9 @@ private String tokenViaExecCredential(Map<String, Object> execMap) {
}
ProcessBuilder pb = new ProcessBuilder(argv);
if (env != null) {
// TODO apply
for (Map<String, String> entry : env) {
pb.environment().put(entry.get("name"), entry.get("value"));
}
}
pb.redirectError(ProcessBuilder.Redirect.INHERIT);
try {
Expand All @@ -288,25 +311,11 @@ private String tokenViaExecCredential(Map<String, Object> execMap) {
log.error("{} failed with exit code {}", command, r);
return null;
}
// TODO verify .apiVersion and .kind = ExecCredential
JsonObject status = root.getAsJsonObject().get("status").getAsJsonObject();
JsonElement token = status.get("token");
if (token == null) {
// TODO handle clientCertificateData/clientKeyData
// (KubeconfigAuthentication is not yet set up for that to be dynamic)
log.warn("No token produced by {}", command);
return null;
}
return token.getAsString();
return root;
} catch (IOException | InterruptedException x) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why swallow the exception here? wouldn't you rather throw?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not swallowed—it is sent to the log.

As to whether it is appropriate to throw exceptions out of getAccessToken (presumably wrapped in RuntimeException?) as opposed to logging and proceeding with other authentication modes and ultimately returning null (unauthenticated), I have no strong opinion. I was following the example of existing code which proceeds despite errors (Failed to read token file for example).

On the other hand AzureActiveDirectoryAuthenticator.refresh appears to throw an undeclared exception out of getAccessToken. KubeconfigAuthentication.<init> is permitted to throw IOException so perhaps getAccessToken should as well? That would mean reworking some existing code which catches and logs exceptions, and adding IOException to the signature of Authenticator.refresh.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, sounds like it's messy, let's take it as a follow-on cleanup exercise...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted in PR description.

log.error("Failed to run " + command, x);
return null;
}
// TODO cache tokens between calls, up to .status.expirationTimestamp
// TODO a 401 is supposed to force a refresh,
// but KubeconfigAuthentication hardcodes AccessTokenAuthentication which does not support that
// and anyway ClientBuilder only calls Authenticator.provide once per ApiClient;
// we would need to do it on every request
}

public boolean verifySSL() {
Expand Down
27 changes: 27 additions & 0 deletions util/src/test/java/io/kubernetes/client/util/KubeConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -296,4 +296,31 @@ public void testExecCredentials() throws Exception {
KubeConfig kc = KubeConfig.loadKubeConfig(new StringReader(KUBECONFIG_EXEC));
assertEquals("abc123", kc.getAccessToken());
}

private static final String KUBECONFIG_EXEC_ENV =
"apiVersion: v1\n"
+ "current-context: c\n"
+ "contexts:\n"
+ "- name: c\n"
+ " context:\n"
+ " user: u\n"
+ "users:\n"
+ "- name: u\n"
+ " user:\n"
+ " exec:\n"
+ " apiVersion: client.authentication.k8s.io/v1beta1\n"
+ " command: sh\n"
+ " env:\n"
+ " - name: TOK\n"
+ " value: abc\n"
+ " args:\n"
+ " - -c\n"
+ " - >-\n"
+ " echo '{\"apiVersion\": \"client.authentication.k8s.io/v1beta1\", \"kind\": \"ExecCredential\", \"status\": {\"token\": \"'$TOK'123\"}}'\n";

@Test
public void testExecCredentialsEnv() throws Exception {
KubeConfig kc = KubeConfig.loadKubeConfig(new StringReader(KUBECONFIG_EXEC_ENV));
assertEquals("abc123", kc.getAccessToken());
}
}