feat(kustomize): support git/repo artifact in kustomize bake manifest (…

…#449) * feat(kustomize): support git/repo artifact in kustomize bake manifest * address code review comments * address code review comments * address code review comments
spinnaker · Oct 29, 2019 · 2f92d63 · 2f92d63
1 parent a43835d
commit 2f92d63
Show file tree

Hide file tree

Showing 7 changed files with 115 additions and 18 deletions.
diff --git a/rosco-manifests/rosco-manifests.gradle b/rosco-manifests/rosco-manifests.gradle
@@ -9,6 +9,7 @@ dependencies {
   implementation "com.netflix.spinnaker.kork:kork-exceptions"
   implementation "com.netflix.spinnaker.kork:kork-security"
   implementation "commons-io:commons-io"
+  implementation "org.apache.commons:commons-compress:1.14"
   implementation "org.yaml:snakeyaml:1.25"
 
   implementation "com.squareup.retrofit:retrofit"

diff --git a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloader.java b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloader.java
@@ -18,8 +18,11 @@
 
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Path;
 
 public interface ArtifactDownloader {
-  void downloadArtifact(Artifact artifact, Path targetFile) throws IOException;
+  InputStream downloadArtifact(Artifact artifact) throws IOException;
+
+  void downloadArtifactToFile(Artifact artifact, Path targetFile) throws IOException;
 }
diff --git a/...manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java b/...manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java
@@ -24,19 +24,24 @@ public ArtifactDownloaderImpl(ClouddriverService clouddriverService) {
     this.clouddriverService = clouddriverService;
   }
 
-  public void downloadArtifact(Artifact artifact, Path targetFile) throws IOException {
+  public InputStream downloadArtifact(Artifact artifact) throws IOException {
+    Response response =
+        retrySupport.retry(() -> clouddriverService.fetchArtifact(artifact), 5, 1000, true);
+    if (response.getBody() == null) {
+      throw new IOException("Failure to fetch artifact: empty response");
+    }
+    return response.getBody().in();
+  }
+
+  public void downloadArtifactToFile(Artifact artifact, Path targetFile) throws IOException {
     try (OutputStream outputStream = Files.newOutputStream(targetFile)) {
-      Response response =
-          retrySupport.retry(() -> clouddriverService.fetchArtifact(artifact), 5, 1000, true);
-      if (response.getBody() != null) {
-        try (InputStream inputStream = response.getBody().in()) {
-          IOUtils.copy(inputStream, outputStream);
-        } catch (IOException e) {
-          throw new IOException(
-              String.format(
-                  "Failed to read input stream of downloaded artifact: %s. Error: %s",
-                  artifact, e.getMessage()));
-        }
+      try (InputStream inputStream = downloadArtifact(artifact)) {
+        IOUtils.copy(inputStream, outputStream);
+      } catch (IOException e) {
+        throw new IOException(
+            String.format(
+                "Failed to read input stream of downloaded artifact: %s. Error: %s",
+                artifact, e.getMessage()));
       }
     } catch (RetrofitError e) {
       throw new IOException(

diff --git a/...manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/helm/HelmTemplateUtils.java b/...manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/helm/HelmTemplateUtils.java
@@ -93,7 +93,7 @@ private Path downloadArtifactToTmpFile(BakeManifestEnvironment env, Artifact art
       throws IOException {
     String fileName = UUID.randomUUID().toString();
     Path targetPath = env.resolvePath(fileName);
-    artifactDownloader.downloadArtifact(artifact, targetPath);
+    artifactDownloader.downloadArtifactToFile(artifact, targetPath);
     return targetPath;
   }
 }
diff --git a/...in/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeBakeManifestRequest.java b/...in/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeBakeManifestRequest.java
@@ -25,4 +25,5 @@
 @EqualsAndHashCode(callSuper = true)
 public class KustomizeBakeManifestRequest extends BakeManifestRequest {
   private Artifact inputArtifact;
+  private String kustomizeFilePath;
 }
diff --git a/...src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java b/...src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java
@@ -22,18 +22,24 @@
 import com.netflix.spinnaker.rosco.manifests.ArtifactDownloader;
 import com.netflix.spinnaker.rosco.manifests.BakeManifestEnvironment;
 import com.netflix.spinnaker.rosco.manifests.kustomize.mapping.Kustomization;
+import java.io.BufferedInputStream;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.compress.archivers.ArchiveEntry;
+import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
+import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream;
 import org.apache.commons.io.FilenameUtils;
 import org.springframework.stereotype.Component;
 
@@ -50,13 +56,27 @@ public KustomizeTemplateUtils(
   }
 
   public BakeRecipe buildBakeRecipe(
-      BakeManifestEnvironment env, KustomizeBakeManifestRequest request) {
+      BakeManifestEnvironment env, KustomizeBakeManifestRequest request) throws IOException {
     BakeRecipe result = new BakeRecipe();
     result.setName(request.getOutputName());
     Artifact artifact = request.getInputArtifact();
     if (artifact == null) {
       throw new IllegalArgumentException("Exactly one input artifact must be provided to bake.");
     }
+
+    String artifactType = Optional.of(artifact.getType()).orElse("");
+    if ("git/repo".equals(artifactType)) {
+      return buildBakeRecipeFromGitRepo(env, request, artifact);
+    } else {
+      return oldBuildBakeRecipe(env, request, artifact);
+    }
+  }
+
+  // Keep the old logic for now. This will be removed as soon as the rest of the git/repo artifact
+  // PRs are merged
+  private BakeRecipe oldBuildBakeRecipe(
+      BakeManifestEnvironment env, KustomizeBakeManifestRequest request, Artifact artifact) {
+
     String kustomizationfilename = FilenameUtils.getName(artifact.getReference());
     if (kustomizationfilename == null
         || (kustomizationfilename != null
@@ -79,11 +99,78 @@ public BakeRecipe buildBakeRecipe(
     command.add("kustomize");
     command.add("build");
     command.add(templatePath.getParent().toString());
+
+    BakeRecipe result = new BakeRecipe();
     result.setCommand(command);
+    return result;
+  }
 
+  private BakeRecipe buildBakeRecipeFromGitRepo(
+      BakeManifestEnvironment env, KustomizeBakeManifestRequest request, Artifact artifact)
+      throws IOException {
+    // This is a redundant check for now, but it's here for when we soon remove the old logic of
+    // building from a github/file artifact type and instead, only support the git/repo artifact
+    // type
+    if (!"git/repo".equals(artifact.getType())) {
+      throw new IllegalArgumentException("The inputArtifact should be of type \"git/repo\".");
+    }
+
+    String kustomizeFilePath = request.getKustomizeFilePath();
+    if (kustomizeFilePath == null) {
+      throw new IllegalArgumentException("The bake request should contain a kustomize file path.");
+    }
+
+    InputStream inputStream;
+    try {
+      inputStream = artifactDownloader.downloadArtifact(artifact);
+    } catch (IOException e) {
+      throw new IOException("Failed to download git/repo artifact: " + e.getMessage(), e);
+    }
+
+    try {
+      extractArtifact(inputStream, env.resolvePath(""));
+    } catch (IOException e) {
+      throw new IOException("Failed to extract git/repo artifact: " + e.getMessage(), e);
+    }
+
+    List<String> command = new ArrayList<>();
+    command.add("kustomize");
+    command.add("build");
+    command.add(env.resolvePath(kustomizeFilePath).getParent().toString());
+
+    BakeRecipe result = new BakeRecipe();
+    result.setCommand(command);
     return result;
   }
 
+  // This being here is temporary until we find a better way to abstract it
+  private static void extractArtifact(InputStream inputStream, Path outputPath) throws IOException {
+    try (TarArchiveInputStream tarArchiveInputStream =
+        new TarArchiveInputStream(
+            new GzipCompressorInputStream(new BufferedInputStream(inputStream)))) {
+
+      ArchiveEntry archiveEntry;
+      while ((archiveEntry = tarArchiveInputStream.getNextEntry()) != null) {
+        Path archiveEntryOutput = validateArchiveEntry(archiveEntry.getName(), outputPath);
+        if (archiveEntry.isDirectory()) {
+          if (!Files.exists(archiveEntryOutput)) {
+            Files.createDirectory(archiveEntryOutput);
+          }
+        } else {
+          Files.copy(tarArchiveInputStream, archiveEntryOutput);
+        }
+      }
+    }
+  }
+
+  private static Path validateArchiveEntry(String archiveEntryName, Path outputPath) {
+    Path entryPath = outputPath.resolve(archiveEntryName);
+    if (!entryPath.normalize().startsWith(outputPath)) {
+      throw new IllegalStateException("Attempting to create a file outside of the staging path.");
+    }
+    return entryPath;
+  }
+
   protected void downloadArtifactToTmpFileStructure(
       BakeManifestEnvironment env, Artifact artifact, String referenceBaseURL) throws IOException {
     if (artifact.getReference() == null) {
@@ -93,7 +180,7 @@ protected void downloadArtifactToTmpFileStructure(
     Path artifactFilePath = env.resolvePath(artifactFileName);
     Path artifactParentDirectory = artifactFilePath.getParent();
     Files.createDirectories(artifactParentDirectory);
-    artifactDownloader.downloadArtifact(artifact, artifactFilePath);
+    artifactDownloader.downloadArtifactToFile(artifact, artifactFilePath);
   }
 
   private List<Artifact> getArtifacts(Artifact artifact) {

diff --git a/...fests/src/test/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImplTest.java b/...fests/src/test/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImplTest.java
@@ -48,7 +48,7 @@ public void downloadsArtifactContent() throws IOException {
     try (ArtifactDownloaderImplTest.AutoDeletingFile file = new AutoDeletingFile()) {
       when(clouddriverService.fetchArtifact(testArtifact))
           .thenReturn(successfulResponse(testContent));
-      artifactDownloader.downloadArtifact(testArtifact, file.path);
+      artifactDownloader.downloadArtifactToFile(testArtifact, file.path);
 
       Assertions.assertThat(file.path).hasContent(testContent);
     }
@@ -63,7 +63,7 @@ public void retries() throws IOException {
       when(clouddriverService.fetchArtifact(testArtifact))
           .thenThrow(RetrofitError.networkError("", new IOException("timeout")))
           .thenReturn(successfulResponse(testContent));
-      artifactDownloader.downloadArtifact(testArtifact, file.path);
+      artifactDownloader.downloadArtifactToFile(testArtifact, file.path);
 
       Assertions.assertThat(file.path).hasContent(testContent);
     }