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
3 changes: 3 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.22.0</version>
<configuration>
<trimStackTrace>false</trimStackTrace> <!-- SUREFIRE-1226 workaround -->
</configuration>
</plugin>
<plugin>
<groupId>com.coveo</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,23 @@ public static ClientBuilder standard() throws IOException {
public static ClientBuilder standard(boolean persistConfig) throws IOException {
final File kubeConfig = findConfigFromEnv();
if (kubeConfig != null) {
try (FileReader kubeConfigReader = new FileReader(kubeConfig)) {
try (FileReader kubeConfigReader = new FileReader(kubeConfig)) { // TODO UTF-8
KubeConfig kc = KubeConfig.loadKubeConfig(kubeConfigReader);
if (persistConfig) {
kc.setPersistConfig(new FilePersister(kubeConfig));
}
kc.setFile(kubeConfig);
return kubeconfig(kc);
}
}
final File config = findConfigInHomeDir();
if (config != null) {
try (FileReader configReader = new FileReader(config)) {
try (FileReader configReader = new FileReader(config)) { // TODO UTF-8
KubeConfig kc = KubeConfig.loadKubeConfig(configReader);
if (persistConfig) {
kc.setPersistConfig(new FilePersister(config));
}
kc.setFile(kubeConfig);
return kubeconfig(kc);
}
}
Expand Down
7 changes: 5 additions & 2 deletions util/src/main/java/io/kubernetes/client/util/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import io.kubernetes.client.ApiClient;
import io.kubernetes.client.util.credentials.AccessTokenAuthentication;
import io.kubernetes.client.util.credentials.UsernamePasswordAuthentication;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.io.InputStream;
Expand Down Expand Up @@ -73,11 +74,13 @@ public static ApiClient fromToken(String url, String token, boolean validateSSL)
}

public static ApiClient fromConfig(String fileName) throws IOException {
return fromConfig(new FileReader(fileName));
KubeConfig config = KubeConfig.loadKubeConfig(new FileReader(fileName)); // TODO UTF-8
config.setFile(new File(fileName));
return fromConfig(config);
}

public static ApiClient fromConfig(InputStream stream) throws IOException {
return fromConfig(new InputStreamReader(stream));
return fromConfig(new InputStreamReader(stream)); // TODO UTF-8
}

public static ApiClient fromConfig(Reader input) throws IOException {
Expand Down
121 changes: 121 additions & 0 deletions util/src/main/java/io/kubernetes/client/util/KubeConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,26 @@
*/
package io.kubernetes.client.util;

import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParseException;
import com.google.gson.JsonParser;
import io.kubernetes.client.util.authenticators.Authenticator;
import io.kubernetes.client.util.authenticators.AzureActiveDirectoryAuthenticator;
import io.kubernetes.client.util.authenticators.GCPAuthenticator;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.apache.commons.codec.binary.Base64;
import org.slf4j.Logger;
Expand Down Expand Up @@ -54,6 +63,7 @@ public class KubeConfig {
String currentNamespace;
Object preferences;
ConfigPersister persister;
private File file;

public static void registerAuthenticator(Authenticator auth) {
synchronized (authenticators) {
Expand Down Expand Up @@ -185,6 +195,7 @@ public String getPassword() {
return getData(currentUser, "password");
}

@SuppressWarnings("unchecked")
public String getAccessToken() {
if (currentUser == null) {
return null;
Expand Down Expand Up @@ -214,6 +225,11 @@ public String getAccessToken() {
}
}
}
String tokenViaExecCredential =
tokenViaExecCredential((Map<String, Object>) currentUser.get("exec"));
if (tokenViaExecCredential != null) {
return tokenViaExecCredential;
}
if (currentUser.containsKey("token")) {
return (String) currentUser.get("token");
}
Expand All @@ -229,6 +245,102 @@ public String getAccessToken() {
return null;
}

/**
* Attempt to create an access token by running a configured external program.
*
* @see <a
* 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;
}
String apiVersion = (String) execMap.get("apiVersion");
if (!"client.authentication.k8s.io/v1beta1".equals(apiVersion)
&& !"client.authentication.k8s.io/v1alpha1".equals(apiVersion)) {
log.error("Unrecognized user.exec.apiVersion: {}", apiVersion);
return null;
}
String command = (String) execMap.get("command");
JsonElement root = runExec(command, (List) execMap.get("args"), (List) execMap.get("env"));
if (root == null) {
return null;
}
if (!"ExecCredential".equals(root.getAsJsonObject().get("kind").getAsString())) {
log.error("Unrecognized kind in response");
return null;
}
if (!apiVersion.equals(root.getAsJsonObject().get("apiVersion").getAsString())) {
log.error("Mismatched apiVersion in response");
return null;
}
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;
}
log.debug("Obtained a token from {}", command);
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) {
List<String> argv = new ArrayList<>();
if (command.contains("/") || command.contains("\\")) {
// Spec is unclear on what should be treated as a “relative command path”.
// This clause should cover anything not resolved from $PATH / %Path%.
Path resolvedCommand = file.toPath().getParent().resolve(command).normalize();
if (!Files.exists(resolvedCommand)) {
log.error("No such file: {}", resolvedCommand);
return null;
}
// Not checking isRegularFile or isExecutable here; leave that to ProcessBuilder.start.
log.debug("Resolved {} to {}", command, resolvedCommand);
argv.add(resolvedCommand.toString());
} else {
argv.add(command);
}
if (args != null) {
argv.addAll(args);
}
ProcessBuilder pb = new ProcessBuilder(argv);
if (env != null) {
for (Map<String, String> entry : env) {
pb.environment().put(entry.get("name"), entry.get("value"));
}
}
pb.redirectError(ProcessBuilder.Redirect.INHERIT);
try {
Process proc = pb.start();
JsonElement root;
try (InputStream is = proc.getInputStream();
Reader r = new InputStreamReader(is, StandardCharsets.UTF_8)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation is weird here, is this really correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What looks incorrect? I do not have a choice about indentation—it is whatever mvn fmt:format picks.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, the 'R' aligned with the '(' looks weird to me, but if mvn:format says so, I guess it's right :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah my preference would be to line up the I and the R.

root = new JsonParser().parse(r);
} catch (JsonParseException x) {
log.error("Failed to parse output of " + command, x);
return null;
}
int r = proc.waitFor();
if (r != 0) {
log.error("{} failed with exit code {}", command, r);
return null;
}
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;
}
}

public boolean verifySSL() {
if (currentCluster == null) {
return false;
Expand All @@ -248,6 +360,15 @@ public void setPersistConfig(ConfigPersister persister) {
this.persister = persister;
}

/**
* Indicates a file from which this configuration was loaded.
*
* @param file a file path, available for use when resolving relative file paths
*/
public void setFile(File file) {
this.file = file;
}

public void setPreferences(Object preferences) {
this.preferences = preferences;
}
Expand Down
99 changes: 99 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 @@ -273,4 +273,103 @@ public void testRefreshToken() {
String token = config.getAccessToken();
assertEquals(token, fake.token);
}

private static final String KUBECONFIG_EXEC =
"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: echo\n"
+ " args:\n"
+ " - >-\n"
+ " {\"apiVersion\": \"client.authentication.k8s.io/v1beta1\", \"kind\": \"ExecCredential\", \"status\": {\"token\": \"abc123\"}}\n";

@Test
public void testExecCredentials() throws Exception {
KubeConfig kc = KubeConfig.loadKubeConfig(new StringReader(KUBECONFIG_EXEC));
kc.setFile(folder.newFile()); // just making sure it is ignored
assertEquals("abc123", kc.getAccessToken());
}

@Test
public void testExecCredentialsAlpha1() throws Exception {
KubeConfig kc =
KubeConfig.loadKubeConfig(new StringReader(KUBECONFIG_EXEC.replace("v1beta1", "v1alpha1")));
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());
}

private static final String KUBECONFIG_EXEC_BASEDIR =
"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: ./bin/authenticate\n";

private static final String AUTH_SCRIPT =
"#!/bin/sh\n"
+ "echo '{\"apiVersion\": \"client.authentication.k8s.io/v1beta1\", \"kind\": \"ExecCredential\", \"status\": {\"token\": \"abc123\"}}'\n";

@Test
public void testExecCredentialsBasedir() throws Exception {
File basedir = folder.newFolder();
File config = new File(basedir, ".kubeconfig");
try (FileWriter writer = new FileWriter(config)) {
writer.write(KUBECONFIG_EXEC_BASEDIR);
writer.flush();
}
File bindir = new File(basedir, "bin");
bindir.mkdir();
File script = new File(bindir, "authenticate");
try (FileWriter writer = new FileWriter(script)) {
writer.write(AUTH_SCRIPT);
writer.flush();
}
script.setExecutable(true);
try (FileReader reader = new FileReader(config)) {
KubeConfig kc = KubeConfig.loadKubeConfig(reader);
kc.setFile(config);
assertEquals("abc123", kc.getAccessToken());
}
}
}