-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Changes from all commits
28b1732
d177116
0bc22e6
0f8c548
0c3ca98
a4163e2
2d22dce
558864f
b2f9332
93b87f4
f843269
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -54,6 +63,7 @@ public class KubeConfig { | |
String currentNamespace; | ||
Object preferences; | ||
ConfigPersister persister; | ||
private File file; | ||
|
||
public static void registerAuthenticator(Authenticator auth) { | ||
synchronized (authenticators) { | ||
|
@@ -185,6 +195,7 @@ public String getPassword() { | |
return getData(currentUser, "password"); | ||
} | ||
|
||
@SuppressWarnings("unchecked") | ||
public String getAccessToken() { | ||
if (currentUser == null) { | ||
return null; | ||
|
@@ -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"); | ||
} | ||
|
@@ -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)) { | ||
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why swallow the exception here? wouldn't you rather throw? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 On the other hand There was a problem hiding this comment. Choose a reason for hiding this commentThe 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... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
@@ -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; | ||
} | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 theR
.