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

Commit

Permalink
validate & recreate SA keys if necessary
Browse files Browse the repository at this point in the history
  • Loading branch information
Daniel Norberg committed May 3, 2017
1 parent 0944275 commit a585405
Show file tree
Hide file tree
Showing 3 changed files with 266 additions and 56 deletions.
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,34 @@ 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
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 @@ -67,7 +68,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 @@ -80,16 +80,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 Down Expand Up @@ -130,6 +126,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 @@ -155,50 +153,13 @@ 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());
Expand All @@ -215,12 +176,101 @@ 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;
}

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 {} 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.warn("[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_SECRET_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 for {} referred to 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 @@ -318,7 +368,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 a585405

Please sign in to comment.