From 24adbbb548b9200a3803fa763142ac7f36bdede3 Mon Sep 17 00:00:00 2001 From: Kathryn Lewis Date: Thu, 17 Oct 2019 12:21:10 -0700 Subject: [PATCH 1/4] feat(kustomize): support git/repo artifact in kustomize bake manifest --- rosco-manifests/rosco-manifests.gradle | 1 + .../KustomizeBakeManifestRequest.java | 1 + .../kustomize/KustomizeTemplateUtils.java | 82 ++++++++++++++++++- 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/rosco-manifests/rosco-manifests.gradle b/rosco-manifests/rosco-manifests.gradle index 0d42a8e58..a3eec8499 100644 --- 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/kustomize/KustomizeBakeManifestRequest.java b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeBakeManifestRequest.java index b05d3b0e5..c22235419 100644 --- a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeBakeManifestRequest.java +++ b/rosco-manifests/src/main/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/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java index 6390e0f7f..4bcedf298 100644 --- a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java +++ b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java @@ -22,6 +22,7 @@ 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.net.URI; @@ -31,9 +32,13 @@ 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 +55,28 @@ 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(""); + switch (artifactType) { + case "git/repo": + return buildBakeRecipeFromGitRepo(env, request, artifact); + default: + 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,71 @@ 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."); + } + + Path tarPath = env.resolvePath("repo.tar.gz"); + try { + artifactDownloader.downloadArtifact(artifact, tarPath); + } catch (IOException e) { + throw new IOException("Failed to download git/repo artifact: " + e.getMessage(), e); + } + try { + extractArtifact(tarPath, env.resolvePath("")); + } catch (IOException e) { + throw new IOException("Failed to extract git/repo artifact: " + e.getMessage(), e); + } + + List 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 + public static void extractArtifact(Path inputTar, Path outputPath) throws IOException { + TarArchiveInputStream tarArchiveInputStream = + new TarArchiveInputStream( + new GzipCompressorInputStream(new BufferedInputStream(Files.newInputStream(inputTar)))); + + ArchiveEntry archiveEntry = null; + while ((archiveEntry = tarArchiveInputStream.getNextEntry()) != null) { + Path archiveEntryOutput = outputPath.resolve(archiveEntry.getName()); + if (archiveEntry.isDirectory()) { + if (!Files.exists(archiveEntryOutput)) { + Files.createDirectory(archiveEntryOutput); + } + } else { + Files.copy(tarArchiveInputStream, archiveEntryOutput); + } + } + + tarArchiveInputStream.close(); + } + protected void downloadArtifactToTmpFileStructure( BakeManifestEnvironment env, Artifact artifact, String referenceBaseURL) throws IOException { if (artifact.getReference() == null) { From 9f128c4801c2e073c80d59f49e8a2112f8ca066f Mon Sep 17 00:00:00 2001 From: Kathryn Lewis Date: Tue, 22 Oct 2019 19:04:05 -0700 Subject: [PATCH 2/4] address code review comments --- .../rosco/manifests/ArtifactDownloader.java | 5 +- .../manifests/ArtifactDownloaderImpl.java | 13 ++++- .../manifests/helm/HelmTemplateUtils.java | 2 +- .../kustomize/KustomizeTemplateUtils.java | 47 ++++++++++--------- .../manifests/ArtifactDownloaderImplTest.java | 4 +- 5 files changed, 43 insertions(+), 28 deletions(-) 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 index 761943ebf..7fc62928d 100644 --- 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/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java index 92f51b16f..031b456c3 100644 --- a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java +++ b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java @@ -23,7 +23,18 @@ 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) { + return response.getBody().in(); + } else { + log.error("Failure to fetch artifact: empty response"); + return null; + } + } + + 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); diff --git a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/helm/HelmTemplateUtils.java b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/helm/HelmTemplateUtils.java index 1820fbd79..abe616a2a 100644 --- a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/helm/HelmTemplateUtils.java +++ b/rosco-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/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java index 4bcedf298..43eda341a 100644 --- a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java +++ b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java @@ -25,6 +25,7 @@ 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; @@ -64,11 +65,10 @@ public BakeRecipe buildBakeRecipe( } String artifactType = Optional.of(artifact.getType()).orElse(""); - switch (artifactType) { - case "git/repo": - return buildBakeRecipeFromGitRepo(env, request, artifact); - default: - return oldBuildBakeRecipe(env, request, artifact); + if ("git/repo".equals(artifactType)) { + return buildBakeRecipeFromGitRepo(env, request, artifact); + } else { + return oldBuildBakeRecipe(env, request, artifact); } } @@ -120,15 +120,15 @@ private BakeRecipe buildBakeRecipeFromGitRepo( throw new IllegalArgumentException("The bake request should contain a kustomize file path."); } - Path tarPath = env.resolvePath("repo.tar.gz"); + InputStream inputStream; try { - artifactDownloader.downloadArtifact(artifact, tarPath); + inputStream = artifactDownloader.downloadArtifact(artifact); } catch (IOException e) { throw new IOException("Failed to download git/repo artifact: " + e.getMessage(), e); } try { - extractArtifact(tarPath, env.resolvePath("")); + extractArtifact(inputStream, env.resolvePath("")); } catch (IOException e) { throw new IOException("Failed to extract git/repo artifact: " + e.getMessage(), e); } @@ -144,24 +144,25 @@ private BakeRecipe buildBakeRecipeFromGitRepo( } // This being here is temporary until we find a better way to abstract it - public static void extractArtifact(Path inputTar, Path outputPath) throws IOException { - TarArchiveInputStream tarArchiveInputStream = + private static void extractArtifact(InputStream inputStream, Path outputPath) throws IOException { + try (TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream( - new GzipCompressorInputStream(new BufferedInputStream(Files.newInputStream(inputTar)))); - - ArchiveEntry archiveEntry = null; - while ((archiveEntry = tarArchiveInputStream.getNextEntry()) != null) { - Path archiveEntryOutput = outputPath.resolve(archiveEntry.getName()); - if (archiveEntry.isDirectory()) { - if (!Files.exists(archiveEntryOutput)) { - Files.createDirectory(archiveEntryOutput); + new GzipCompressorInputStream(new BufferedInputStream(inputStream)))) { + + ArchiveEntry archiveEntry; + while ((archiveEntry = tarArchiveInputStream.getNextEntry()) != null) { + Path archiveEntryOutput = outputPath.resolve(archiveEntry.getName()); + if (archiveEntry.isDirectory()) { + if (!Files.exists(archiveEntryOutput)) { + Files.createDirectory(archiveEntryOutput); + } + } else { + Files.copy(tarArchiveInputStream, archiveEntryOutput); } - } else { - Files.copy(tarArchiveInputStream, archiveEntryOutput); } + } catch (Exception e) { + System.out.println(e); } - - tarArchiveInputStream.close(); } protected void downloadArtifactToTmpFileStructure( @@ -173,7 +174,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 getArtifacts(Artifact artifact) { diff --git a/rosco-manifests/src/test/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImplTest.java b/rosco-manifests/src/test/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImplTest.java index c1d57a90e..8608ac08a 100644 --- a/rosco-manifests/src/test/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImplTest.java +++ b/rosco-manifests/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); } From 38fa3f629333776e806c84c0a55b67515abbc9db Mon Sep 17 00:00:00 2001 From: Kathryn Lewis Date: Fri, 25 Oct 2019 18:18:39 -0700 Subject: [PATCH 3/4] address code review comments --- .../manifests/ArtifactDownloaderImpl.java | 28 ++++++++----------- .../kustomize/KustomizeTemplateUtils.java | 16 +++++++---- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java index f17a136fb..9aeb725dc 100644 --- a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java +++ b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java @@ -27,28 +27,24 @@ public ArtifactDownloaderImpl(ClouddriverService clouddriverService) { public InputStream downloadArtifact(Artifact artifact) throws IOException { Response response = retrySupport.retry(() -> clouddriverService.fetchArtifact(artifact), 5, 1000, true); - if (response.getBody() != null) { - return response.getBody().in(); + if (response.getBody() == null) { + throw new IOException("Failure to fetch artifact: empty response"); } else { - log.error("Failure to fetch artifact: empty response"); - return null; + try (InputStream inputStream = response.getBody().in()) { + return inputStream; + } catch (IOException e) { + throw new IOException( + String.format( + "Failed to read input stream of downloaded artifact: %s. Error: %s", + artifact, e.getMessage())); + } } } 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())); - } - } + InputStream inputStream = downloadArtifact(artifact); + IOUtils.copy(inputStream, outputStream); } catch (RetrofitError e) { throw new IOException( String.format("Failed to download artifact: %s. Error: %s", artifact, e.getMessage())); diff --git a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java index 43eda341a..8e4ef01c3 100644 --- a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java +++ b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/kustomize/KustomizeTemplateUtils.java @@ -109,8 +109,8 @@ 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 + // 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\"."); } @@ -151,7 +151,7 @@ private static void extractArtifact(InputStream inputStream, Path outputPath) th ArchiveEntry archiveEntry; while ((archiveEntry = tarArchiveInputStream.getNextEntry()) != null) { - Path archiveEntryOutput = outputPath.resolve(archiveEntry.getName()); + Path archiveEntryOutput = validateArchiveEntry(archiveEntry.getName(), outputPath); if (archiveEntry.isDirectory()) { if (!Files.exists(archiveEntryOutput)) { Files.createDirectory(archiveEntryOutput); @@ -160,11 +160,17 @@ private static void extractArtifact(InputStream inputStream, Path outputPath) th Files.copy(tarArchiveInputStream, archiveEntryOutput); } } - } catch (Exception e) { - System.out.println(e); } } + 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) { From 95330b6444a78ae11bb85fc935242734c650b948 Mon Sep 17 00:00:00 2001 From: Kathryn Lewis Date: Tue, 29 Oct 2019 08:40:56 -0700 Subject: [PATCH 4/4] address code review comments --- .../manifests/ArtifactDownloaderImpl.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java index 9aeb725dc..74bd69d8a 100644 --- a/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java +++ b/rosco-manifests/src/main/java/com/netflix/spinnaker/rosco/manifests/ArtifactDownloaderImpl.java @@ -29,22 +29,20 @@ public InputStream downloadArtifact(Artifact artifact) throws IOException { retrySupport.retry(() -> clouddriverService.fetchArtifact(artifact), 5, 1000, true); if (response.getBody() == null) { throw new IOException("Failure to fetch artifact: empty response"); - } else { - try (InputStream inputStream = response.getBody().in()) { - return inputStream; + } + return response.getBody().in(); + } + + public void downloadArtifactToFile(Artifact artifact, Path targetFile) throws IOException { + try (OutputStream outputStream = Files.newOutputStream(targetFile)) { + 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())); } - } - } - - public void downloadArtifactToFile(Artifact artifact, Path targetFile) throws IOException { - try (OutputStream outputStream = Files.newOutputStream(targetFile)) { - InputStream inputStream = downloadArtifact(artifact); - IOUtils.copy(inputStream, outputStream); } catch (RetrofitError e) { throw new IOException( String.format("Failed to download artifact: %s. Error: %s", artifact, e.getMessage()));