check mount path

spotify · May 3, 2017 · e03edd0 · e03edd0
1 parent b597cd5
commit e03edd0
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 3 deletions.
diff --git a/styx-scheduler-service/src/main/java/com/spotify/styx/docker/KubernetesDockerRunner.java b/styx-scheduler-service/src/main/java/com/spotify/styx/docker/KubernetesDockerRunner.java
@@ -107,7 +107,7 @@ class KubernetesDockerRunner implements DockerRunner {
   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()
@@ -166,6 +166,15 @@ private void ensureCustomSecret(WorkflowInstance workflowInstance, RunSpec runSp
         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(
+            STYX_WORKFLOW_SA_SECRET_MOUNT_PATH + " is a reserved mount path");
+      }
+
       final Secret secret = client.secrets().withName(specSecret.name()).get();
       if (secret == null) {
         LOG.error("[AUDIT] Workflow {} refers to a non-existent secret {}",
@@ -187,8 +196,8 @@ private void ensureServiceAccountKeySecret(WorkflowInstance workflowInstance, Ru
     final String serviceAccount = runSpec.serviceAccount().get();
 
     // Check that the service account exists
-    final boolean serviceAccountExists = !serviceAccountKeyManager.serviceAccountExists(serviceAccount);
-    if (serviceAccountExists) {
+    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");

diff --git a/styx-scheduler-service/src/test/java/com/spotify/styx/docker/KubernetesDockerRunnerTest.java b/styx-scheduler-service/src/test/java/com/spotify/styx/docker/KubernetesDockerRunnerTest.java
@@ -108,6 +108,12 @@ public class KubernetesDockerRunnerTest {
                                                                  empty(),
                                                                  Optional.of(SERVICE_ACCOUNT),
                                                                  empty());
+  private static final RunSpec RUN_SPEC_WITH_SECRET_AND_SA = RunSpec.create("busybox",
+                                                                            ImmutableList.copyOf(new String[0]),
+                                                                            false,
+                                                                            Optional.of(WorkflowConfiguration.Secret.create("secret1", KubernetesDockerRunner.STYX_WORKFLOW_SA_SECRET_MOUNT_PATH)),
+                                                                            Optional.of(SERVICE_ACCOUNT),
+                                                                            empty());
 
   @Mock NamespacedKubernetesClient k8sClient;
 
@@ -198,6 +204,24 @@ public void shouldThrowIfSecretNotExist() throws IOException, StateManager.IsClo
     kdr.start(WORKFLOW_INSTANCE, RUN_SPEC_WITH_SECRET);
   }
 
+  @Test(expected = InvalidExecutionException.class)
+  public void shouldThrowIfMountToReservedPath() throws IOException, StateManager.IsClosed {
+    when(secrets.withName(any(String.class))).thenReturn(namedResource);
+    when(namedResource.get()).thenReturn(null);
+    when(k8sClient.secrets()).thenReturn(secrets);
+    Pod createdPod = KubernetesDockerRunner.createPod(WORKFLOW_INSTANCE, RUN_SPEC_WITH_SECRET_AND_SA);
+    when(pods.create(any(Pod.class))).thenReturn(createdPod);
+
+    stateManager.receive(Event.terminate(WORKFLOW_INSTANCE, Optional.of(0)));
+    stateManager.receive(Event.success(WORKFLOW_INSTANCE));
+    kdr.close();
+
+    // Start a new runner
+    kdr = new KubernetesDockerRunner(k8sClient, stateManager, stats, serviceAccountKeyManager);
+    kdr.init();
+    kdr.start(WORKFLOW_INSTANCE, RUN_SPEC_WITH_SECRET_AND_SA);
+  }
+
   @Test
   public void shouldMountSecret() throws IOException, StateManager.IsClosed {
     final Pod pod = KubernetesDockerRunner.createPod(WORKFLOW_INSTANCE, RUN_SPEC_WITH_SECRET);