From 55b0cedd8203dc322e2f1805be98a099357fbb1e Mon Sep 17 00:00:00 2001 From: Appu Date: Thu, 18 Jun 2020 10:08:05 -0400 Subject: [PATCH] Add binary path to staging (#842) * Allow binaries in java11 standard buildpath --- .../AppYamlProjectStageConfiguration.java | 2 +- .../operations/AppYamlProjectStaging.java | 52 +++++- .../operations/AppYamlProjectStagingTest.java | 165 +++++++++++++----- .../cloud/tools/project/AppYamlTest.java | 20 ++- 4 files changed, 185 insertions(+), 54 deletions(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/configuration/AppYamlProjectStageConfiguration.java b/src/main/java/com/google/cloud/tools/appengine/configuration/AppYamlProjectStageConfiguration.java index 79d4b9983..9c79757e0 100644 --- a/src/main/java/com/google/cloud/tools/appengine/configuration/AppYamlProjectStageConfiguration.java +++ b/src/main/java/com/google/cloud/tools/appengine/configuration/AppYamlProjectStageConfiguration.java @@ -85,7 +85,7 @@ public static Builder builder() { /** * Create a new builder for AppYamlProjectStageConfiguration. * - * @deprecated Use newBuilder().appEngineDirectory(appEngineDirectory).artifact(artifact) + * @deprecated Use builder().appEngineDirectory(appEngineDirectory).artifact(artifact) * .stagingDirectory(stagingDirectory) instead. */ @Deprecated diff --git a/src/main/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStaging.java b/src/main/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStaging.java index 2a7af0e79..ab843a06a 100644 --- a/src/main/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStaging.java +++ b/src/main/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStaging.java @@ -74,15 +74,29 @@ public void stageArchive(AppYamlProjectStageConfiguration config) throws AppEngi String runtime = findRuntime(config); if ("flex".equals(env)) { stageFlexibleArchive(config, runtime); - } else if ("java11".equals(runtime)) { - stageStandardArchive(config); - } else { - // I don't know how to deploy this + return; + } + if ("java11".equals(runtime)) { + boolean isJar = config.getArtifact().getFileName().toString().endsWith(".jar"); + if (isJar) { + stageStandardArchive(config); + return; + } + if (hasCustomEntrypoint(config)) { + stageStandardBinary(config); + return; + } + // I cannot deploy non-jars without custom entrypoints throw new AppEngineException( - "Cannot process application with runtime: " - + runtime - + (Strings.isNullOrEmpty(env) ? "" : " and env: " + env)); + "Cannot process application with runtime: java11." + + " A custom entrypoint must be defined in your app.yaml for non-jar artifact: " + + config.getArtifact().toString()); } + // I don't know how to deploy this + throw new AppEngineException( + "Cannot process application with runtime: " + + runtime + + (Strings.isNullOrEmpty(env) ? "" : " and env: " + env)); } catch (IOException ex) { throw new AppEngineException(ex); } @@ -108,6 +122,15 @@ void stageStandardArchive(AppYamlProjectStageConfiguration config) copyArtifactJarClasspath(config, copyService); } + @VisibleForTesting + void stageStandardBinary(AppYamlProjectStageConfiguration config) + throws IOException, AppEngineException { + CopyService copyService = new CopyService(); + copyExtraFiles(config, copyService); + copyAppEngineContext(config, copyService); + copyArtifact(config, copyService); + } + @VisibleForTesting @Nullable static String findEnv(AppYamlProjectStageConfiguration config) @@ -198,7 +221,6 @@ static void copyExtraFiles(AppYamlProjectStageConfiguration config, CopyService private static void copyArtifact(AppYamlProjectStageConfiguration config, CopyService copyService) throws IOException, AppEngineException { - // Copy the JAR/WAR file to staging. Path artifact = config.getArtifact(); if (Files.exists(artifact)) { Path stagingDirectory = config.getStagingDirectory(); @@ -248,6 +270,20 @@ static void copyArtifactJarClasspath( } } + @VisibleForTesting + // for non jar artifacts we want to ensure the entrypoint is custom + static boolean hasCustomEntrypoint(AppYamlProjectStageConfiguration config) + throws IOException, AppEngineException { + // verify that app.yaml that contains entrypoint: + if (config.getAppEngineDirectory() == null) { + throw new AppEngineException("Invalid Staging Configuration: missing App Engine directory"); + } + Path appYamlFile = config.getAppEngineDirectory().resolve(APP_YAML); + try (InputStream input = Files.newInputStream(appYamlFile)) { + return AppYaml.parse(input).getEntrypoint() != null; + } + } + @VisibleForTesting static class CopyService { void copyDirectory(Path src, Path dest, List excludes) throws IOException { diff --git a/src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java b/src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java index 2beff47d4..42f1c749d 100644 --- a/src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java +++ b/src/test/java/com/google/cloud/tools/appengine/operations/AppYamlProjectStagingTest.java @@ -16,10 +16,13 @@ package com.google.cloud.tools.appengine.operations; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.doCallRealMethod; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -44,7 +47,6 @@ import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; /** Test the {@link AppYamlProjectStaging} functionality. */ @@ -73,7 +75,7 @@ public void setUp() throws IOException { ImmutableList.of( temporaryFolder.newFolder().toPath(), temporaryFolder.newFolder().toPath()); stagingDirectory = temporaryFolder.newFolder().toPath(); - artifact = temporaryFolder.newFile("artifact").toPath(); + artifact = temporaryFolder.newFile("artifact.jar").toPath(); dockerFile = dockerDirectory.resolve("Dockerfile"); Files.createFile(dockerFile); @@ -96,28 +98,81 @@ public void testStageArchive_flexPath() throws IOException, AppEngineException { StandardOpenOption.CREATE_NEW); // mock to watch internal calls - AppYamlProjectStaging mock = Mockito.mock(AppYamlProjectStaging.class); - Mockito.doCallRealMethod().when(mock).stageArchive(config); + AppYamlProjectStaging mock = mock(AppYamlProjectStaging.class); + doCallRealMethod().when(mock).stageArchive(config); mock.stageArchive(config); verify(mock).stageFlexibleArchive(config, "test_runtime"); } @Test - public void testStageArchive_standardPath() throws IOException, AppEngineException { + public void testStageArchive_java11StandardPath() throws IOException, AppEngineException { Files.write( appEngineDirectory.resolve("app.yaml"), "runtime: java11\n".getBytes(StandardCharsets.UTF_8), StandardOpenOption.CREATE_NEW); // mock to watch internal calls - AppYamlProjectStaging mock = Mockito.mock(AppYamlProjectStaging.class); - Mockito.doCallRealMethod().when(mock).stageArchive(config); + AppYamlProjectStaging mock = mock(AppYamlProjectStaging.class); + doCallRealMethod().when(mock).stageArchive(config); mock.stageArchive(config); verify(mock).stageStandardArchive(config); } + @Test + public void testStageArchive_java11StandardBinaryPath() throws IOException, AppEngineException { + config = + AppYamlProjectStageConfiguration.builder() + .appEngineDirectory(appEngineDirectory) + .artifact(temporaryFolder.newFile("myscript.sh").toPath()) + .stagingDirectory(stagingDirectory) + .extraFilesDirectories(extraFilesDirectories) + .build(); + + Files.write( + appEngineDirectory.resolve("app.yaml"), + "runtime: java11\nentrypoint: anything\n".getBytes(StandardCharsets.UTF_8), + StandardOpenOption.CREATE_NEW); + + // mock to watch internal calls + AppYamlProjectStaging mock = mock(AppYamlProjectStaging.class); + doCallRealMethod().when(mock).stageArchive(config); + + mock.stageArchive(config); + verify(mock).stageStandardBinary(config); + } + + @Test + public void testStageArchive_java11BinaryWithoutEntrypoint() + throws IOException, AppEngineException { + Path nonJarArtifact = temporaryFolder.newFile("myscript.sh").toPath(); + config = + AppYamlProjectStageConfiguration.builder() + .appEngineDirectory(appEngineDirectory) + .artifact(nonJarArtifact) + .stagingDirectory(stagingDirectory) + .extraFilesDirectories(extraFilesDirectories) + .build(); + + Files.write( + appEngineDirectory.resolve("app.yaml"), + "runtime: java11\n".getBytes(StandardCharsets.UTF_8), + StandardOpenOption.CREATE_NEW); + + AppYamlProjectStaging testStaging = new AppYamlProjectStaging(); + + try { + testStaging.stageArchive(config); + fail(); + } catch (AppEngineException ex) { + assertEquals( + "Cannot process application with runtime: java11. A custom entrypoint must be defined in your app.yaml for non-jar artifact: " + + nonJarArtifact.toString(), + ex.getMessage()); + } + } + @Test public void testStageArchive_unknown() throws IOException { Files.write( @@ -131,7 +186,7 @@ public void testStageArchive_unknown() throws IOException { testStaging.stageArchive(config); fail(); } catch (AppEngineException ex) { - Assert.assertEquals("Cannot process application with runtime: moose", ex.getMessage()); + assertEquals("Cannot process application with runtime: moose", ex.getMessage()); } } @@ -279,7 +334,7 @@ public void testCopyExtraFiles_nonExistantDirectory() throws IOException { AppYamlProjectStaging.copyExtraFiles(badExtraFilesConfig, copyService); fail(); } catch (AppEngineException ex) { - Assert.assertEquals( + assertEquals( "Extra files directory does not exist. Location: " + extraFilesDirectory, ex.getMessage()); } @@ -302,7 +357,7 @@ public void testCopyExtraFiles_directoryIsActuallyAFile() throws IOException { AppYamlProjectStaging.copyExtraFiles(badExtraFilesConfig, copyService); fail(); } catch (AppEngineException ex) { - Assert.assertEquals( + assertEquals( "Extra files location is not a directory. Location: " + extraFilesDirectory, ex.getMessage()); } @@ -370,8 +425,10 @@ public void testCopyAppEngineContext_appYamlInAppEngineDirectory() public void testFindRuntime_malformedAppYaml() throws IOException { Path file = appEngineDirectory.resolve("app.yaml"); - Files.createFile(file); - Files.write(file, ": m a l f o r m e d !".getBytes(StandardCharsets.UTF_8)); + Files.write( + file, + ": m a l f o r m e d !".getBytes(StandardCharsets.UTF_8), + StandardOpenOption.CREATE_NEW); try { AppYamlProjectStaging.findRuntime(config); @@ -381,28 +438,48 @@ public void testFindRuntime_malformedAppYaml() throws IOException { } } + @Test + public void testHasCustomEntrypoint_true() throws IOException, AppEngineException { + Path file = appEngineDirectory.resolve("app.yaml"); + Files.write( + file, + "entrypoint: custom custom".getBytes(StandardCharsets.UTF_8), + StandardOpenOption.CREATE_NEW); + + assertTrue(AppYamlProjectStaging.hasCustomEntrypoint(config)); + } + + @Test + public void testHasCustomEntrypoint_false() throws IOException, AppEngineException { + Path file = appEngineDirectory.resolve("app.yaml"); + Files.write( + file, "runtime: java".getBytes(StandardCharsets.UTF_8), StandardOpenOption.CREATE_NEW); + + Assert.assertFalse(AppYamlProjectStaging.hasCustomEntrypoint(config)); + } + @Test public void testCopyArtifactJarClasspath_noClasspath() throws IOException { AppYamlProjectStaging.copyArtifactJarClasspath( - AppYamlProjectStageConfiguration.builder( - appEngineDirectory, - Paths.get("src/test/resources/jars/libs/simpleLib.jar"), - stagingDirectory) + AppYamlProjectStageConfiguration.builder() + .appEngineDirectory(appEngineDirectory) + .artifact(Paths.get("src/test/resources/jars/libs/simpleLib.jar")) + .stagingDirectory(stagingDirectory) .build(), copyService); verifyNoInteractions(copyService); - Assert.assertEquals(0, handler.getLogs().size()); + assertEquals(0, handler.getLogs().size()); } @Test public void testCopyArtifactJarClasspath_withClasspathEntries() throws IOException { AppYamlProjectStaging.copyArtifactJarClasspath( - AppYamlProjectStageConfiguration.builder( - appEngineDirectory, - Paths.get("src/test/resources/jars/complexLib.jar"), - stagingDirectory) + AppYamlProjectStageConfiguration.builder() + .appEngineDirectory(appEngineDirectory) + .artifact(Paths.get("src/test/resources/jars/complexLib.jar")) + .stagingDirectory(stagingDirectory) .build(), copyService); @@ -412,16 +489,16 @@ public void testCopyArtifactJarClasspath_withClasspathEntries() throws IOExcepti stagingDirectory.resolve("libs/simpleLib.jar")); verifyNoMoreInteractions(copyService); - Assert.assertEquals(0, handler.getLogs().size()); + assertEquals(0, handler.getLogs().size()); } @Test public void testCopyArtifactJarClasspath_withMissingClasspathEntries() throws IOException { AppYamlProjectStaging.copyArtifactJarClasspath( - AppYamlProjectStageConfiguration.builder( - appEngineDirectory, - Paths.get("src/test/resources/jars/complexLibMissingEntryManifest.jar"), - stagingDirectory) + AppYamlProjectStageConfiguration.builder() + .appEngineDirectory(appEngineDirectory) + .artifact(Paths.get("src/test/resources/jars/complexLibMissingEntryManifest.jar")) + .stagingDirectory(stagingDirectory) .build(), copyService); @@ -433,9 +510,9 @@ public void testCopyArtifactJarClasspath_withMissingClasspathEntries() throws IO // check for warning about missing jars List logs = handler.getLogs(); - Assert.assertEquals(1, logs.size()); - Assert.assertEquals(Level.WARNING, logs.get(0).getLevel()); - Assert.assertEquals( + assertEquals(1, logs.size()); + assertEquals(Level.WARNING, logs.get(0).getLevel()); + assertEquals( "Could not copy 'Class-Path' jar: " + Paths.get("src/test/resources/jars/libs/missing.jar") + " referenced in MANIFEST.MF", @@ -451,10 +528,10 @@ public void testCopyArtifactJarClasspath_targetAlreadyExists() throws IOExceptio Files.createFile(simpleLibTarget); AppYamlProjectStaging.copyArtifactJarClasspath( - AppYamlProjectStageConfiguration.builder( - appEngineDirectory, - Paths.get("src/test/resources/jars/complexLib.jar"), - stagingDirectory) + AppYamlProjectStageConfiguration.builder() + .appEngineDirectory(appEngineDirectory) + .artifact(Paths.get("src/test/resources/jars/complexLib.jar")) + .stagingDirectory(stagingDirectory) .build(), copyService); @@ -463,9 +540,9 @@ public void testCopyArtifactJarClasspath_targetAlreadyExists() throws IOExceptio // check for warning about overwriting jars List logs = handler.getLogs(); - Assert.assertEquals(1, logs.size()); - Assert.assertEquals(Level.FINE, logs.get(0).getLevel()); - Assert.assertEquals( + assertEquals(1, logs.size()); + assertEquals(Level.FINE, logs.get(0).getLevel()); + assertEquals( "Overwriting 'Class-Path' jar: " + simpleLibTarget + " with " @@ -482,20 +559,20 @@ public void testCopyService_copiesToExistingFile() throws IOException { Path srcDir = root.resolve("srcDir"); Files.createDirectory(srcDir); Path srcFile = srcDir.resolve("srcFile"); - Files.createFile(srcFile); - Files.write(srcFile, "some content".getBytes(StandardCharsets.UTF_8)); + Files.write( + srcFile, "some content".getBytes(StandardCharsets.UTF_8), StandardOpenOption.CREATE_NEW); Path destDir = root.resolve("destDir"); Files.createDirectory(destDir); Path destFile = destDir.resolve("destFile"); Files.createFile(destFile); - Assert.assertTrue(Files.exists(destDir)); - Assert.assertTrue(Files.exists(destFile)); + assertTrue(Files.exists(destDir)); + assertTrue(Files.exists(destFile)); copier.copyFileAndReplace(srcFile, destFile); - Assert.assertArrayEquals(Files.readAllBytes(srcFile), Files.readAllBytes(destFile)); + assertArrayEquals(Files.readAllBytes(srcFile), Files.readAllBytes(destFile)); } @Test @@ -506,8 +583,8 @@ public void testCopyService_createsParentsWhenNecessary() throws IOException { Path srcDir = root.resolve("srcDir"); Files.createDirectory(srcDir); Path srcFile = srcDir.resolve("srcFile"); - Files.createFile(srcFile); - Files.write(srcFile, "some content".getBytes(StandardCharsets.UTF_8)); + Files.write( + srcFile, "some content".getBytes(StandardCharsets.UTF_8), StandardOpenOption.CREATE_NEW); Path destDir = root.resolve("destDir"); Path destFile = destDir.resolve("destFile"); @@ -517,6 +594,6 @@ public void testCopyService_createsParentsWhenNecessary() throws IOException { copier.copyFileAndReplace(srcFile, destFile); - Assert.assertArrayEquals(Files.readAllBytes(srcFile), Files.readAllBytes(destFile)); + assertArrayEquals(Files.readAllBytes(srcFile), Files.readAllBytes(destFile)); } } diff --git a/src/test/java/com/google/cloud/tools/project/AppYamlTest.java b/src/test/java/com/google/cloud/tools/project/AppYamlTest.java index 8a2fd51eb..d134a2991 100644 --- a/src/test/java/com/google/cloud/tools/project/AppYamlTest.java +++ b/src/test/java/com/google/cloud/tools/project/AppYamlTest.java @@ -78,7 +78,25 @@ public void testGetEntrypoint_success() throws AppEngineException { @Test public void testGetEntrypoint_nullBecauseWrongType() throws AppEngineException { - InputStream appYaml = asStream("runtime: [goose, moose]\np2: v2"); + InputStream appYaml = asStream("entrypoint: [goose, moose]\np2: v2"); + Assert.assertNull(AppYaml.parse(appYaml).getRuntime()); + } + + @Test + public void testGetEntrypoint_nullBecauseNull() throws AppEngineException { + InputStream appYaml = asStream("entrypoint: null\np2: v2"); + Assert.assertNull(AppYaml.parse(appYaml).getRuntime()); + } + + @Test + public void testGetEntrypoint_nullBecauseNothing() throws AppEngineException { + InputStream appYaml = asStream("entrypoint:\np2: v2"); + Assert.assertNull(AppYaml.parse(appYaml).getRuntime()); + } + + @Test + public void testGetEntrypoint_nullBecauseEmptyList() throws AppEngineException { + InputStream appYaml = asStream("entrypoint: []\np2: v2"); Assert.assertNull(AppYaml.parse(appYaml).getRuntime()); }