From 2f92d6360a3aa8bea05e064b9726237834427b66 Mon Sep 17 00:00:00 2001 From: Kathryn Lewis Date: Tue, 29 Oct 2019 10:10:05 -0700 Subject: [PATCH] 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 --- rosco-manifests/rosco-manifests.gradle | 1 + .../rosco/manifests/ArtifactDownloader.java | 5 +- .../manifests/ArtifactDownloaderImpl.java | 29 +++--- .../manifests/helm/HelmTemplateUtils.java | 2 +- .../KustomizeBakeManifestRequest.java | 1 + .../kustomize/KustomizeTemplateUtils.java | 91 ++++++++++++++++++- .../manifests/ArtifactDownloaderImplTest.java | 4 +- 7 files changed, 115 insertions(+), 18 deletions(-) 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/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 a05693a88..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 @@ -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/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/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..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 @@ -22,8 +22,10 @@ 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; @@ -31,9 +33,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 +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 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 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); }