Skip to content

Commit

Permalink
Fix fabric8io#2308: kubeconfig with external authentication command d…
Browse files Browse the repository at this point in the history 
…oesn't work
  • Loading branch information
@rohanKanojia
rohanKanojia committed Aug 7, 2020
1 parent fa4e44d commit 31af911
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -3,6 +3,7 @@
### 4.10-SNAPSHOT
#### Bugs
* Fix #2373: Unable to create a Template on OCP3
* Fix #2308: Fix kubernetes client `Config` loading KUBECONFIG with external authentication command
* Fix #2316: Cannot load resource from stream without apiVersion

#### Improvements
Expand Down
95 changes: 60 additions & 35 deletions kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java
View file
Expand Up @@ -600,41 +600,11 @@ private static boolean loadFromKubeconfig(Config config, String context, String
} else if (config.getOauthTokenProvider() == null) { // https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins
ExecConfig exec = currentAuthInfo.getExec();
if (exec != null) {
String apiVersion = exec.getApiVersion();
if ("client.authentication.k8s.io/v1alpha1".equals(apiVersion) || "client.authentication.k8s.io/v1beta1".equals(apiVersion)) {
List<String> argv = new ArrayList<String>();
String command = exec.getCommand();
if (command.contains("/") && !command.startsWith("/") && kubeconfigPath != null && !kubeconfigPath.isEmpty()) {
// Appears to be a relative path; normalize. Spec is vague about how to detect this situation.
command = Paths.get(kubeconfigPath).resolveSibling(command).normalize().toString();
}
argv.add(command);
List<String> args = exec.getArgs();
if (args != null) {
argv.addAll(args);
}
ProcessBuilder pb = new ProcessBuilder(argv);
List<ExecEnvVar> env = exec.getEnv();
if (env != null) {
Map<String, String> environment = pb.environment();
env.forEach(var -> environment.put(var.getName(), var.getValue()));
}
// TODO check behavior of tty & stdin
Process p = pb.start();
if (p.waitFor() != 0) {
LOGGER.warn(IOHelpers.readFully(p.getErrorStream()));
}
ExecCredential ec = Serialization.unmarshal(p.getInputStream(), ExecCredential.class);
if (!apiVersion.equals(ec.apiVersion)) {
LOGGER.warn("Wrong apiVersion {} vs. {}", ec.apiVersion, apiVersion);
}
if (ec.status != null && ec.status.token != null) {
config.setOauthToken(ec.status.token);
} else {
LOGGER.warn("No token returned");
}
} else { // TODO v1beta1?
LOGGER.warn("Unsupported apiVersion: {}", apiVersion);
ExecCredential ec = getExecCredentialFromExecConfig(exec, kubeconfigPath);
if (ec != null && ec.status != null && ec.status.token != null) {
config.setOauthToken(ec.status.token);
} else {
LOGGER.warn("No token returned");
}
}
}
Expand All @@ -651,6 +621,61 @@ private static boolean loadFromKubeconfig(Config config, String context, String
return false;
}

protected static ExecCredential getExecCredentialFromExecConfig(ExecConfig exec, String kubeconfigPath) throws IOException, InterruptedException {
String apiVersion = exec.getApiVersion();
if ("client.authentication.k8s.io/v1alpha1".equals(apiVersion) || "client.authentication.k8s.io/v1beta1".equals(apiVersion)) {
List<ExecEnvVar> env = exec.getEnv();
// TODO check behavior of tty & stdin
ProcessBuilder pb = new ProcessBuilder(getAuthenticatorCommandFromExecConfig(exec, kubeconfigPath, Utils.getSystemPathVariable()));
if (env != null) {
Map<String, String> environment = pb.environment();
env.forEach(var -> environment.put(var.getName(), var.getValue()));
}
Process p = pb.start();
if (p.waitFor() != 0) {
LOGGER.warn(IOHelpers.readFully(p.getErrorStream()));
}
ExecCredential ec = Serialization.unmarshal(p.getInputStream(), ExecCredential.class);
if (!apiVersion.equals(ec.apiVersion)) {
LOGGER.warn("Wrong apiVersion {} vs. {}", ec.apiVersion, apiVersion);
} else {
return ec;
}
} else { // TODO v1beta1?
LOGGER.warn("Unsupported apiVersion: {}", apiVersion);
}
return null;
}

protected static List<String> getAuthenticatorCommandFromExecConfig(ExecConfig exec, String kubeconfigPath, String systemPathValue) {
String command = exec.getCommand();
if (command.contains("/") && !command.startsWith("/") && kubeconfigPath != null && !kubeconfigPath.isEmpty()) {
// Appears to be a relative path; normalize. Spec is vague about how to detect this situation.
command = Paths.get(kubeconfigPath).resolveSibling(command).normalize().toString();
}
List<String> argv = new ArrayList<>(Utils.getCommandPlatformPrefix());
command = getCommandWithFullyQualifiedPath(command, systemPathValue);
List<String> args = exec.getArgs();
if (args != null) {
argv.add(command + " " + String.join( " ", args));
}
return argv;
}

protected static String getCommandWithFullyQualifiedPath(String command, String pathValue) {
String[] pathParts = pathValue.split(File.pathSeparator);

// Iterate through path in order to find executable file
for (String pathPart : pathParts) {
File commandFile = new File(pathPart + File.separator + command);
if (commandFile.exists()) {
return commandFile.getAbsolutePath();
}
}

return command;
}

private static Context setCurrentContext(String context, Config config, io.fabric8.kubernetes.api.model.Config kubeConfig) {
if (context != null) {
kubeConfig.setCurrentContext(context);
Expand Down
55 changes: 50 additions & 5 deletions kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/Utils.java
View file
Expand Up @@ -32,6 +32,7 @@
import java.net.URLEncoder;
import java.nio.file.Paths;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
Expand All @@ -53,6 +54,10 @@ public class Utils {

private static final Logger LOGGER = LoggerFactory.getLogger(Utils.class);
private static final String ALL_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789";
public static final String WINDOWS = "win";
public static final String OS_NAME = "os.name";
public static final String PATH_WINDOWS = "Path";
public static final String PATH_UNIX = "PATH";

private Utils() {
}
Expand Down Expand Up @@ -101,7 +106,7 @@ public static boolean getSystemPropertyOrEnvVar(String systemPropertyName, Boole
}

public static int getSystemPropertyOrEnvVar(String systemPropertyName, int defaultValue) {
String result = getSystemPropertyOrEnvVar(systemPropertyName, new Integer(defaultValue).toString());
String result = getSystemPropertyOrEnvVar(systemPropertyName, Integer.toString(defaultValue));
return Integer.parseInt(result);
}

Expand Down Expand Up @@ -186,7 +191,7 @@ public static boolean shutdownExecutorService(ExecutorService executorService) {
if (LOGGER.isDebugEnabled()) {
List<Runnable> tasks = executorService.shutdownNow();
if (!tasks.isEmpty()) {
LOGGER.debug("ExecutorService was not cleanly shutdown, after waiting for 10 seconds. Number of remaining tasks:" + tasks.size());
LOGGER.debug("ExecutorService was not cleanly shutdown, after waiting for 10 seconds. Number of remaining tasks: {}", tasks.size());
}
}
} catch (InterruptedException e) {
Expand All @@ -213,7 +218,7 @@ public static void closeQuietly(Iterable<Closeable> closeables) {
c.close();
}
} catch (IOException e) {
LOGGER.debug("Error closing:" + c);
LOGGER.debug("Error closing: {}", c);
}
}
}
Expand Down Expand Up @@ -331,7 +336,7 @@ public static String getProperty(Map<String, Object> properties, String property
* @param str Url as string
* @return returns encoded string
*/
public static final String toUrlEncoded(String str) {
public static String toUrlEncoded(String str) {
try {
return URLEncoder.encode(str, "UTF-8");
} catch (UnsupportedEncodingException exception) {
Expand All @@ -341,7 +346,7 @@ public static final String toUrlEncoded(String str) {
}

public static String getPluralFromKind(String kind) {
StringBuffer pluralBuffer = new StringBuffer(kind.toLowerCase(Locale.ROOT));
StringBuilder pluralBuffer = new StringBuilder(kind.toLowerCase(Locale.ROOT));
switch (kind) {
case "ComponentStatus":
case "Ingress":
Expand Down Expand Up @@ -425,4 +430,44 @@ public static String interpolateString(String templateInput, Map<String, String>
.reduce(Function.identity(), Function::andThen)
.apply(Objects.requireNonNull(templateInput, "templateInput is required"));
}

/**
* Check whether platform is windows or not
*
* @return boolean value indicating whether OS is Windows or not.
*/
public static boolean isWindowsOperatingSystem() {
return getOperatingSystemFromSystemProperty().toLowerCase().contains(WINDOWS);
}

/**
* Get system PATH variable
*
* @return a string containing value of PATH
*/
public static String getSystemPathVariable() {
return System.getenv(isWindowsOperatingSystem() ? PATH_WINDOWS : PATH_UNIX);
}

/**
* Returns prefixes needed to invoke specified command
* in a subprocess.
*
* @return a list of strings containing prefixes
*/
public static List<String> getCommandPlatformPrefix() {
List<String> platformPrefixParts = new ArrayList<>();
if (Utils.isWindowsOperatingSystem()) {
platformPrefixParts.add("cmd.exe");
platformPrefixParts.add("/c");
} else {
platformPrefixParts.add("sh");
platformPrefixParts.add("-c");
}
return platformPrefixParts;
}

private static String getOperatingSystemFromSystemProperty() {
return System.getProperty(OS_NAME);
}
}
53 changes: 52 additions & 1 deletion kubernetes-client/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java
View file
Expand Up @@ -16,6 +16,8 @@

package io.fabric8.kubernetes.client;

import io.fabric8.kubernetes.api.model.ExecConfig;
import io.fabric8.kubernetes.api.model.ExecConfigBuilder;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.fabric8.kubernetes.client.utils.Utils;
import okhttp3.OkHttpClient;
Expand All @@ -28,8 +30,8 @@

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.nio.file.Paths;
Expand Down Expand Up @@ -577,4 +579,53 @@ private void assertConfig(Config config) {
assertEquals("/path/to/keystore", config.getKeyStoreFile());
assertEquals("keystorePassphrase", config.getKeyStorePassphrase());
}

@Test
void testGetAuthenticatorCommandFromExecConfig() throws IOException {
// Given
File commandFolder = Files.createTempDirectory("test").toFile();
File commandFile = new File(commandFolder, "aws");
boolean isNewFileCreated = commandFile.createNewFile();
String systemPathValue = getTestPathValue(commandFolder);
ExecConfig execConfig = new ExecConfigBuilder()
.withApiVersion("client.authentication.k8s.io/v1alpha1")
.addToArgs("--region", "us-west2", "eks", "get-token", "--cluster-name", "api-eks.example.com")
.withCommand("aws")
.build();

// When
List<String> processBuilderArgs = Config.getAuthenticatorCommandFromExecConfig(execConfig, "~/.kube/config", systemPathValue);

// Then
assertTrue(isNewFileCreated);
assertNotNull(processBuilderArgs);
assertEquals(3, processBuilderArgs.size());
assertPlatformPrefixes(processBuilderArgs);
List<String> commandParts = Arrays.asList(processBuilderArgs.get(2).split(" "));
assertEquals(commandFile.getAbsolutePath(), commandParts.get(0));
assertEquals("--region", commandParts.get(1));
assertEquals("us-west2", commandParts.get(2));
assertEquals("eks", commandParts.get(3));
assertEquals("get-token", commandParts.get(4));
assertEquals("--cluster-name", commandParts.get(5));
assertEquals("api-eks.example.com", commandParts.get(6));
}

private void assertPlatformPrefixes(List<String> processBuilderArgs) {
List<String> platformArgsExpected = Utils.getCommandPlatformPrefix();
assertEquals(platformArgsExpected.get(0), processBuilderArgs.get(0));
assertEquals(platformArgsExpected.get(1), processBuilderArgs.get(1));
}

private String getTestPathValue(File commandFolder) {
if (Utils.isWindowsOperatingSystem()) {
return "C:\\Program Files\\Java\\jdk14.0_23\\bin" + File.pathSeparator +
commandFolder.getAbsolutePath() + File.pathSeparator +
"C:\\Program Files\\Apache Software Foundation\\apache-maven-3.3.1";
} else {
return "/usr/java/jdk-14.0.1/bin" + File.pathSeparator +
commandFolder.getAbsolutePath() + File.pathSeparator +
"/opt/apache-maven/bin";
}
}
}
36 changes: 34 additions & 2 deletions kubernetes-client/src/test/java/io/fabric8/kubernetes/client/internal/UtilsTest.java
View file
Expand Up @@ -65,16 +65,21 @@
import io.fabric8.kubernetes.api.model.storage.v1beta1.CSIDriver;
import io.fabric8.kubernetes.api.model.storage.v1beta1.CSINode;
import io.fabric8.kubernetes.client.utils.Utils;
import org.apache.commons.lang.SystemUtils;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import java.io.File;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;

class UtilsTest {
Expand Down Expand Up @@ -103,12 +108,12 @@ void existingEnvVarShouldReturnValueFromConvertedSysPropName() {

@Test
void existingEnvVarShouldReturnBooleanValueFromConvertedSysPropName() {
assertEquals(true, Utils.getSystemPropertyOrEnvVar("env.var.exists.boolean", false));
Assertions.assertTrue(Utils.getSystemPropertyOrEnvVar("env.var.exists.boolean", false));
}

@Test
void missingEnvVarShouldReturnDefaultValue() {
assertEquals(true, Utils.getSystemPropertyOrEnvVar("DONT_EXIST", true));
Assertions.assertTrue(Utils.getSystemPropertyOrEnvVar("DONT_EXIST", true));
}

@Test
Expand Down Expand Up @@ -339,4 +344,31 @@ void isNotNullNoneAreNullTest() {
// Then
assertTrue(result);
}

@Test
@DisplayName("test getting system path")
void testGetSystemPathVariable() {
// When
String pathVariable = Utils.getSystemPathVariable();

// Then
assertNotNull(pathVariable);
assertTrue(pathVariable.contains(File.pathSeparator));
}

@Test
@DisplayName("Should get command prefix")
void testGetCommandPlatformPrefix() {
List<String> commandPrefix = Utils.getCommandPlatformPrefix();

assertNotNull(commandPrefix);
assertEquals(2, commandPrefix.size());
if (SystemUtils.IS_OS_WINDOWS) {
assertEquals("cmd.exe", commandPrefix.get(0));
assertEquals("/c", commandPrefix.get(1));
} else {
assertEquals("sh", commandPrefix.get(0));
assertEquals("-c", commandPrefix.get(1));
}
}
}

0 comments on commit 31af911

Please sign in to comment.