Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Commit

Permalink
Merge pull request #160 from spotify/validate-recreate-SA-keys
Browse files Browse the repository at this point in the history 
validate & recreate SA keys if necessary
  • Loading branch information
@honnix
honnix committed May 4, 2017
2 parents 2206cb5 + 6ead180 commit 228e7b1
Show file tree
Hide file tree
Showing 3 changed files with 298 additions and 58 deletions.
28 changes: 28 additions & 0 deletions styx-scheduler-service/src/main/java/com/spotify/styx/ServiceAccountKeyManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

package com.spotify.styx;

import com.google.api.client.googleapis.json.GoogleJsonResponseException;
import com.google.api.services.iam.v1.Iam;
import com.google.api.services.iam.v1.model.CreateServiceAccountKeyRequest;
import com.google.api.services.iam.v1.model.ServiceAccountKey;
Expand All @@ -43,6 +44,33 @@ public ServiceAccountKey createP12Key(String serviceAccount) throws IOException
.setPrivateKeyType("TYPE_PKCS12_FILE"));
}

public boolean serviceAccountExists(String serviceAccount) throws IOException {
try {
iam.projects().serviceAccounts().get("projects/-/serviceAccounts/" + serviceAccount)
.execute();
return true;
} catch (GoogleJsonResponseException e) {
if (e.getStatusCode() == 404) {
return false;
}
throw e;
}
}

public boolean keyExists(String keyName) throws IOException {
try {
iam.projects().serviceAccounts().keys()
.get(keyName)
.execute();
return true;
} catch (GoogleJsonResponseException e) {
if (e.getStatusCode() == 404) {
return false;
}
throw e;
}
}

private ServiceAccountKey createKey(String serviceAccount,
CreateServiceAccountKeyRequest request)
throws IOException {
Expand Down
162 changes: 110 additions & 52 deletions styx-scheduler-service/src/main/java/com/spotify/styx/docker/KubernetesDockerRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static com.spotify.styx.docker.KubernetesPodEventTranslator.translate;
import static com.spotify.styx.serialization.Json.OBJECT_MAPPER;
import static com.spotify.styx.state.RunState.State.RUNNING;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toSet;

import com.fasterxml.jackson.core.JsonProcessingException;
Expand Down Expand Up @@ -71,7 +72,6 @@
import io.fabric8.kubernetes.client.Watcher;
import java.io.IOException;
import java.net.ProtocolException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
Expand All @@ -84,16 +84,12 @@
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* A {@link DockerRunner} implementation that submits container executions to a Kubernetes cluster.
*/
class KubernetesDockerRunner implements DockerRunner {

private static final Logger logger = LoggerFactory.getLogger(KubernetesDockerRunner.class);

private static final String NAMESPACE = "default";
static final String STYX_RUN = "styx-run";
static final String STYX_WORKFLOW_INSTANCE_ANNOTATION = "styx-workflow-instance";
Expand All @@ -109,13 +105,13 @@ class KubernetesDockerRunner implements DockerRunner {
static final String TRIGGER_TYPE = "STYX_TRIGGER_TYPE";
private static final int DEFAULT_POLL_PODS_INTERVAL_SECONDS = 60;
static final String STYX_WORKFLOW_SA_ENV_VARIABLE = "GOOGLE_APPLICATION_CREDENTIALS";
private static final String STYX_WORKFLOW_SA_SECRET_ANNOTATION = "styx-wf-sa";
private static final String STYX_WORKFLOW_SA_ID_ANNOTATION = "styx-wf-sa";
private static final String STYX_WORKFLOW_SA_JSON_KEY_NAME_ANNOTATION = "styx-wf-sa-json-key-name";
private static final String STYX_WORKFLOW_SA_P12_KEY_NAME_ANNOTATION = "styx-wf-sa-p12-key-name";
static final String STYX_WORKFLOW_SA_SECRET_NAME = "styx-wf-sa-keys";
private static final String STYX_WORKFLOW_SA_JSON_KEY = "styx-wf-sa.json";
private static final String STYX_WORKFLOW_SA_P12_KEY = "styx-wf-sa.p12";
private static final String STYX_WORKFLOW_SA_SECRET_MOUNT_PATH =
static final String STYX_WORKFLOW_SA_SECRET_MOUNT_PATH =
"/etc/" + STYX_WORKFLOW_SA_SECRET_NAME + "/";

private static final ThreadFactory THREAD_FACTORY = new ThreadFactoryBuilder()
Expand All @@ -134,6 +130,8 @@ class KubernetesDockerRunner implements DockerRunner {

private Watch watch;

private final Object secretMutationLock = new Object() {};

KubernetesDockerRunner(NamespacedKubernetesClient client, StateManager stateManager, Stats stats,
ServiceAccountKeyManager serviceAccountKeyManager, int pollPodsIntervalSeconds) {
this.stateManager = Objects.requireNonNull(stateManager);
Expand All @@ -159,56 +157,29 @@ public String start(WorkflowInstance workflowInstance, RunSpec runSpec) throws I
}
}

private void ensureSecrets(WorkflowInstance workflowInstance, RunSpec runSpec) {
if (runSpec.serviceAccount().isPresent()) {
final String serviceAccount = runSpec.serviceAccount().get();
final String secretName = buildSecretName(serviceAccount);
final Secret secret = client.secrets().withName(secretName).get();
if (secret == null) {
final ServiceAccountKey jsonKey;
final ServiceAccountKey p12Key;
try {
jsonKey = serviceAccountKeyManager.createJsonKey(serviceAccount);
p12Key = serviceAccountKeyManager.createP12Key(serviceAccount);
} catch (IOException e) {
logger.error("[AUDIT] Failed to create keys for {}", serviceAccount, e);
throw new InvalidExecutionException("Failed to create keys for " + serviceAccount);
}

final Map<String, String> keys = ImmutableMap.of(
STYX_WORKFLOW_SA_JSON_KEY, jsonKey.getPrivateKeyData(),
STYX_WORKFLOW_SA_P12_KEY, p12Key.getPrivateKeyData()
);

final Map<String, String> annotations = ImmutableMap.of(
STYX_WORKFLOW_SA_JSON_KEY_NAME_ANNOTATION, jsonKey.getName(),
STYX_WORKFLOW_SA_P12_KEY_NAME_ANNOTATION, p12Key.getName(),
STYX_WORKFLOW_SA_SECRET_ANNOTATION, serviceAccount
);

client.secrets().create(new SecretBuilder()
.withNewMetadata()
.withName(secretName)
.withAnnotations(annotations)
.endMetadata()
.withData(keys)
.build());
LOG.info("[AUDIT] Secret {} created for {} referred to by workflow {}", secretName,
serviceAccount, workflowInstance.workflowId());
} else {
LOG.info("[AUDIT] Workflow {} refers to secret {} of {}", workflowInstance.workflowId(),
secretName, serviceAccount);
}
}
private void ensureSecrets(WorkflowInstance workflowInstance, RunSpec runSpec) throws IOException {
ensureServiceAccountKeySecret(workflowInstance, runSpec);
ensureCustomSecret(workflowInstance, runSpec);
}

if (runSpec.secret().isPresent()) {
final WorkflowConfiguration.Secret specSecret = runSpec.secret().get();
private void ensureCustomSecret(WorkflowInstance workflowInstance, RunSpec runSpec) {
runSpec.secret().ifPresent(specSecret -> {
if (specSecret.name().startsWith(STYX_WORKFLOW_SA_SECRET_NAME)) {
LOG.warn("[AUDIT] Workflow {} refers to secret {} with managed service account key secret name prefix, "
+ "denying execution", specSecret.name());
throw new InvalidExecutionException(
"Referenced secret '" + specSecret.name() + "' has the managed service account key secret name prefix");
}

// if it ever happens, that feels more like a hack than pure luck so let's be paranoid
if (STYX_WORKFLOW_SA_SECRET_MOUNT_PATH.equals(specSecret.mountPath())) {
LOG.error("[AUDIT] Workflow {} tries to mount secret {} to the reserved path",
workflowInstance.workflowId(), specSecret.name());
throw new InvalidExecutionException(
"Referenced secret '" + specSecret.name() + "' has the mount path "
+ STYX_WORKFLOW_SA_SECRET_MOUNT_PATH + " defined that is reserved");
}

final Secret secret = client.secrets().withName(specSecret.name()).get();
if (secret == null) {
LOG.error("[AUDIT] Workflow {} refers to a non-existent secret {}",
Expand All @@ -219,12 +190,99 @@ private void ensureSecrets(WorkflowInstance workflowInstance, RunSpec runSpec) {
LOG.info("[AUDIT] Workflow {} refers to secret {}",
workflowInstance.workflowId(), specSecret.name());
}
});
}

private void ensureServiceAccountKeySecret(WorkflowInstance workflowInstance, RunSpec runSpec) throws IOException {
if (!runSpec.serviceAccount().isPresent()) {
return;
}

final String serviceAccount = runSpec.serviceAccount().get();

// Check that the service account exists
final boolean serviceAccountExists = serviceAccountKeyManager.serviceAccountExists(serviceAccount);
if (!serviceAccountExists) {
LOG.error("[AUDIT] Workflow {} refers to non-existent service account {}", workflowInstance.workflowId(),
serviceAccount);
throw new InvalidExecutionException("Referenced service account " + serviceAccount + " was not found");
}

final String secretName = buildSecretName(serviceAccount);

LOG.info("[AUDIT] Workflow {} refers to secret {} storing keys of {}",
workflowInstance.workflowId(), secretName, serviceAccount);

// TODO: shard locking to regain concurrency
synchronized (secretMutationLock) {

// Check if we have a valid service account key secret already
final Secret existingSecret = client.secrets().withName(secretName).get();
if (existingSecret != null) {
if (serviceAccountKeysExist(existingSecret)) {
return;
}

LOG.info("[AUDIT] Service account keys have been deleted for {}, recreating",
serviceAccount);

// Need to delete this secret before creating a new one
client.secrets().delete(existingSecret);
}

// Create service account keys and secret
final ServiceAccountKey jsonKey;
final ServiceAccountKey p12Key;
try {
jsonKey = serviceAccountKeyManager.createJsonKey(serviceAccount);
p12Key = serviceAccountKeyManager.createP12Key(serviceAccount);
} catch (IOException e) {
LOG.error("[AUDIT] Failed to create keys for {}", serviceAccount, e);
throw e;
}

final Map<String, String> keys = ImmutableMap.of(
STYX_WORKFLOW_SA_JSON_KEY, jsonKey.getPrivateKeyData(),
STYX_WORKFLOW_SA_P12_KEY, p12Key.getPrivateKeyData()
);

final Map<String, String> annotations = ImmutableMap.of(
STYX_WORKFLOW_SA_JSON_KEY_NAME_ANNOTATION, jsonKey.getName(),
STYX_WORKFLOW_SA_P12_KEY_NAME_ANNOTATION, p12Key.getName(),
STYX_WORKFLOW_SA_ID_ANNOTATION, serviceAccount
);

final Secret newSecret = new SecretBuilder()
.withNewMetadata()
.withName(secretName)
.withAnnotations(annotations)
.endMetadata()
.withData(keys)
.build();

client.secrets().create(newSecret);

LOG.info("[AUDIT] Secret {} created to store keys of {} referred by workflow {}",
secretName, serviceAccount, workflowInstance.workflowId());
}
}

private boolean serviceAccountKeysExist(Secret secret) {
final Map<String, String> annotations = secret.getMetadata().getAnnotations();
final String jsonKeyName = annotations.get(STYX_WORKFLOW_SA_JSON_KEY_NAME_ANNOTATION);
final String p12KeyName = annotations.get(STYX_WORKFLOW_SA_P12_KEY_NAME_ANNOTATION);

try {
return serviceAccountKeyManager.keyExists(jsonKeyName)
&& serviceAccountKeyManager.keyExists(p12KeyName);
} catch (IOException e) {
throw new RuntimeException(e);
}
}

private static String buildSecretName(String serviceAccount) {
return STYX_WORKFLOW_SA_SECRET_NAME + '-'
+ Hashing.sha256().hashString(serviceAccount, StandardCharsets.UTF_8);
+ Hashing.sha256().hashString(serviceAccount, UTF_8);
}

@VisibleForTesting
Expand Down Expand Up @@ -330,7 +388,7 @@ public void close() throws IOException {
try {
executor.awaitTermination(30, TimeUnit.SECONDS);
} catch (InterruptedException e) {
logger.warn("Failed to terminate executor", e);
LOG.warn("Failed to terminate executor", e);
}
}

Expand Down
Loading

0 comments on commit 228e7b1

Please sign in to comment.