Skip to content

Commit

Permalink
Throw exception if extraDirectories config contains invalid path (#3550)
Browse files Browse the repository at this point in the history
* Throw exception if extraDirectories config contains invalid path

* Rename ExtraDirectoryNotFoundException

* Only throws extraDirectory not found exception if it's not a jib default directory

* Address review comments

* Update jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/PluginConfigurationProcessor.java

Co-authored-by: Chanseok Oh <[email protected]>

* Update jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/PluginConfigurationProcessor.java

Co-authored-by: Chanseok Oh <[email protected]>

* Fix compile error

* Update changelog

* Merge branch 'master' into validate-extra-directories-paths-config

* Fix tests

* Fix javadoc

* Fix code style checks

Co-authored-by: Chanseok Oh <[email protected]>
  • Loading branch information
tommysitu and chanseokoh authored Jan 18, 2022
1 parent 9388312 commit 3f2caac
Show file tree
Hide file tree
Showing 18 changed files with 152 additions and 32 deletions.
2 changes: 1 addition & 1 deletion config/checkstyle/copyright-java.header
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
^/\*$
^ \* Copyright 20(17|18|19|20|21) Google LLC\.$
^ \* Copyright 20(17|18|19|20|21|22) Google LLC\.$
^ \*$
^ \* Licensed under the Apache License, Version 2\.0 \(the "License"\); you may not$
^ \* use this file except in compliance with the License\. You may obtain a copy of$
Expand Down
1 change: 1 addition & 0 deletions jib-gradle-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file.
### Changed

- Changed the default base image of the Jib CLI `jar` command from the `adoptopenjdk` images to the [`eclipse-temurin`](https://hub.docker.com/_/eclipse-temurin) on Docker Hub. Note that Temurin (by Adoptium) is the new name of AdoptOpenJDK. ([#3483](https://github.com/GoogleContainerTools/jib/issues/3483))
- Build will fail if `extraDirectories.paths` contain `from` directory that doesn't exist locally ([#3542](https://github.com/GoogleContainerTools/jib/issues/3542))

### Fixed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.cloud.tools.jib.docker.DockerClient;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.plugins.common.BuildStepsExecutionException;
import com.google.cloud.tools.jib.plugins.common.ExtraDirectoryNotFoundException;
import com.google.cloud.tools.jib.plugins.common.HelpfulSuggestions;
import com.google.cloud.tools.jib.plugins.common.IncompatibleBaseImageJavaVersionException;
import com.google.cloud.tools.jib.plugins.common.InvalidAppRootException;
Expand Down Expand Up @@ -180,6 +181,11 @@ public void buildDocker()
throw new GradleException(
HelpfulSuggestions.forInvalidImageReference(ex.getInvalidReference()), ex);

} catch (ExtraDirectoryNotFoundException ex) {
throw new GradleException(
"extraDirectories.paths contain \"from\" directory that doesn't exist locally: "
+ ex.getPath(),
ex);
} finally {
tempDirectoryProvider.close();
TaskCommon.finishUpdateChecker(projectProperties, updateCheckFuture);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.cloud.tools.jib.api.InvalidImageReferenceException;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.plugins.common.BuildStepsExecutionException;
import com.google.cloud.tools.jib.plugins.common.ExtraDirectoryNotFoundException;
import com.google.cloud.tools.jib.plugins.common.HelpfulSuggestions;
import com.google.cloud.tools.jib.plugins.common.IncompatibleBaseImageJavaVersionException;
import com.google.cloud.tools.jib.plugins.common.InvalidAppRootException;
Expand Down Expand Up @@ -175,6 +176,11 @@ public void buildImage()
throw new GradleException(
HelpfulSuggestions.forInvalidImageReference(ex.getInvalidReference()), ex);

} catch (ExtraDirectoryNotFoundException ex) {
throw new GradleException(
"extraDirectories.paths contain \"from\" directory that doesn't exist locally: "
+ ex.getPath(),
ex);
} finally {
tempDirectoryProvider.close();
TaskCommon.finishUpdateChecker(projectProperties, updateCheckFuture);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.cloud.tools.jib.api.InvalidImageReferenceException;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.plugins.common.BuildStepsExecutionException;
import com.google.cloud.tools.jib.plugins.common.ExtraDirectoryNotFoundException;
import com.google.cloud.tools.jib.plugins.common.HelpfulSuggestions;
import com.google.cloud.tools.jib.plugins.common.IncompatibleBaseImageJavaVersionException;
import com.google.cloud.tools.jib.plugins.common.InvalidAppRootException;
Expand Down Expand Up @@ -197,6 +198,11 @@ public void buildTar()
throw new GradleException(
HelpfulSuggestions.forInvalidImageReference(ex.getInvalidReference()), ex);

} catch (ExtraDirectoryNotFoundException ex) {
throw new GradleException(
"extraDirectories.paths contain \"from\" directory that doesn't exist locally: "
+ ex.getPath(),
ex);
} finally {
tempDirectoryProvider.close();
TaskCommon.finishUpdateChecker(projectProperties, updateCheckFuture);
Expand Down
2 changes: 2 additions & 0 deletions jib-maven-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ All notable changes to this project will be documented in this file.
### Changed

- Changed the default base image of the Jib CLI `jar` command from the `adoptopenjdk` images to the [`eclipse-temurin`](https://hub.docker.com/_/eclipse-temurin) on Docker Hub. Note that Temurin (by Adoptium) is the new name of AdoptOpenJDK. ([#3483](https://github.com/GoogleContainerTools/jib/issues/3483))
- Build will fail if `<extraDirectories><paths>` contain `from` directory that doesn't exist locally ([#3542](https://github.com/GoogleContainerTools/jib/issues/3542))


### Fixed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.cloud.tools.jib.docker.DockerClient;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.plugins.common.BuildStepsExecutionException;
import com.google.cloud.tools.jib.plugins.common.ExtraDirectoryNotFoundException;
import com.google.cloud.tools.jib.plugins.common.HelpfulSuggestions;
import com.google.cloud.tools.jib.plugins.common.IncompatibleBaseImageJavaVersionException;
import com.google.cloud.tools.jib.plugins.common.InvalidAppRootException;
Expand Down Expand Up @@ -165,6 +166,11 @@ public void execute() throws MojoExecutionException, MojoFailureException {
} catch (BuildStepsExecutionException ex) {
throw new MojoExecutionException(ex.getMessage(), ex.getCause());

} catch (ExtraDirectoryNotFoundException ex) {
throw new MojoExecutionException(
"<extraDirectories><paths> contain \"from\" directory that doesn't exist locally: "
+ ex.getPath(),
ex);
} finally {
tempDirectoryProvider.close();
MojoCommon.finishUpdateChecker(projectProperties, updateCheckFuture);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.cloud.tools.jib.api.buildplan.ImageFormat;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.plugins.common.BuildStepsExecutionException;
import com.google.cloud.tools.jib.plugins.common.ExtraDirectoryNotFoundException;
import com.google.cloud.tools.jib.plugins.common.HelpfulSuggestions;
import com.google.cloud.tools.jib.plugins.common.IncompatibleBaseImageJavaVersionException;
import com.google.cloud.tools.jib.plugins.common.InvalidAppRootException;
Expand Down Expand Up @@ -179,6 +180,11 @@ public void execute() throws MojoExecutionException, MojoFailureException {
} catch (BuildStepsExecutionException ex) {
throw new MojoExecutionException(ex.getMessage(), ex.getCause());

} catch (ExtraDirectoryNotFoundException ex) {
throw new MojoExecutionException(
"<extraDirectories><paths> contain \"from\" directory that doesn't exist locally: "
+ ex.getPath(),
ex);
} finally {
tempDirectoryProvider.close();
MojoCommon.finishUpdateChecker(projectProperties, updateCheckFuture);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.cloud.tools.jib.api.InvalidImageReferenceException;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.plugins.common.BuildStepsExecutionException;
import com.google.cloud.tools.jib.plugins.common.ExtraDirectoryNotFoundException;
import com.google.cloud.tools.jib.plugins.common.HelpfulSuggestions;
import com.google.cloud.tools.jib.plugins.common.IncompatibleBaseImageJavaVersionException;
import com.google.cloud.tools.jib.plugins.common.InvalidAppRootException;
Expand Down Expand Up @@ -157,6 +158,11 @@ public void execute() throws MojoExecutionException, MojoFailureException {
} catch (BuildStepsExecutionException ex) {
throw new MojoExecutionException(ex.getMessage(), ex.getCause());

} catch (ExtraDirectoryNotFoundException ex) {
throw new MojoExecutionException(
"<extraDirectories><paths> contain \"from\" directory that doesn't exist locally: "
+ ex.getPath(),
ex);
} finally {
tempDirectoryProvider.close();
MojoCommon.finishUpdateChecker(projectProperties, updateCheckFuture);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ public void testFilesMojo_multiModuleComplexService() throws VerificationExcepti
complexServiceRoot.resolve("src/main/resources2").toString(),
complexServiceRoot.resolve("src/main/jib1").toString(),
complexServiceRoot.resolve("src/main/jib2").toString(),
Paths.get("/").toAbsolutePath().resolve("some/random/absolute/path/jib3").toString(),
// this test expects standard .m2 locations
Paths.get(
System.getProperty("user.home"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,20 @@ public void testSyncMapMojo_multiProjectOutput() throws IOException, Verificatio
generated.get(1));

List<FileTemplate> direct = parsed.getDirect();
Assert.assertEquals(1, direct.size());
Assert.assertEquals(3, direct.size());
assertFilePaths(
m2.resolve(
"com/google/cloud/tools/tiny-test-lib/0.0.1-SNAPSHOT/tiny-test-lib-0.0.1-SNAPSHOT.jar"),
AbsoluteUnixPath.get("/app/libs/tiny-test-lib-0.0.1-SNAPSHOT.jar"),
direct.get(0));
assertFilePaths(
projectRoot.resolve("complex-service/src/main/jib1/foo"),
AbsoluteUnixPath.get("/foo"),
direct.get(1));
assertFilePaths(
projectRoot.resolve("complex-service/src/main/jib2/bar"),
AbsoluteUnixPath.get("/bar"),
direct.get(2));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
<paths>
<path>src/main/jib1</path> <!-- should resolve to this projectRoot -->
<path>${project.basedir}/src/main/jib2</path> <!-- provided as absolute within project -->
<path>/some/random/absolute/path/jib3</path> <!-- provided as absolute outside of project -->
</paths>
</extraDirectories>
</configuration>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
foo
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bar
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright 2022 Google LLC.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.google.cloud.tools.jib.plugins.common;

/** Indicates that the {@code extraDirectories.paths.path} is not found. */
public class ExtraDirectoryNotFoundException extends Exception {

private final String path;

public ExtraDirectoryNotFoundException(String message, String path) {
super(message);
this.path = path;
}

public String getPath() {
return path;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public class PluginConfigurationProcessor {

private static final String JIB_CLASSPATH_FILE = "jib-classpath-file";
private static final String JIB_MAIN_CLASS_FILE = "jib-main-class-file";
private static final Path DEFAULT_JIB_DIR = Paths.get("src").resolve("main").resolve("jib");

private PluginConfigurationProcessor() {}

Expand Down Expand Up @@ -122,6 +123,8 @@ private PluginConfigurationProcessor() {}
* parsed
* @throws InvalidCreationTimeException if configured creation time could not be parsed
* @throws JibPluginExtensionException if an error occurred while running plugin extensions
* @throws ExtraDirectoryNotFoundException if the extra directory specified for the build is not
* found
*/
public static JibBuildRunner createJibBuildRunnerForDockerDaemonImage(
RawConfiguration rawConfiguration,
Expand All @@ -134,7 +137,7 @@ public static JibBuildRunner createJibBuildRunnerForDockerDaemonImage(
InvalidContainerVolumeException, IncompatibleBaseImageJavaVersionException,
NumberFormatException, InvalidContainerizingModeException,
InvalidFilesModificationTimeException, InvalidCreationTimeException,
JibPluginExtensionException {
ExtraDirectoryNotFoundException, JibPluginExtensionException {
ImageReference targetImageReference =
getGeneratedTargetDockerTag(rawConfiguration, projectProperties, helpfulSuggestions);
DockerDaemonImage targetImage = DockerDaemonImage.named(targetImageReference);
Expand Down Expand Up @@ -194,6 +197,8 @@ public static JibBuildRunner createJibBuildRunnerForDockerDaemonImage(
* parsed
* @throws InvalidCreationTimeException if configured creation time could not be parsed
* @throws JibPluginExtensionException if an error occurred while running plugin extensions
* @throws ExtraDirectoryNotFoundException if the extra directory specified for the build is not
* found
*/
public static JibBuildRunner createJibBuildRunnerForTarImage(
RawConfiguration rawConfiguration,
Expand All @@ -206,7 +211,7 @@ public static JibBuildRunner createJibBuildRunnerForTarImage(
InvalidContainerVolumeException, IncompatibleBaseImageJavaVersionException,
NumberFormatException, InvalidContainerizingModeException,
InvalidFilesModificationTimeException, InvalidCreationTimeException,
JibPluginExtensionException {
JibPluginExtensionException, ExtraDirectoryNotFoundException {
ImageReference targetImageReference =
getGeneratedTargetDockerTag(rawConfiguration, projectProperties, helpfulSuggestions);
TarImage targetImage =
Expand Down Expand Up @@ -260,6 +265,8 @@ public static JibBuildRunner createJibBuildRunnerForTarImage(
* parsed
* @throws InvalidCreationTimeException if configured creation time could not be parsed
* @throws JibPluginExtensionException if an error occurred while running plugin extensions
* @throws ExtraDirectoryNotFoundException if the extra directory specified for the build is not
* found
*/
public static JibBuildRunner createJibBuildRunnerForRegistryImage(
RawConfiguration rawConfiguration,
Expand All @@ -272,7 +279,7 @@ public static JibBuildRunner createJibBuildRunnerForRegistryImage(
InvalidContainerVolumeException, IncompatibleBaseImageJavaVersionException,
NumberFormatException, InvalidContainerizingModeException,
InvalidFilesModificationTimeException, InvalidCreationTimeException,
JibPluginExtensionException {
JibPluginExtensionException, ExtraDirectoryNotFoundException {
Optional<String> image = rawConfiguration.getToImage();
Preconditions.checkArgument(image.isPresent());

Expand Down Expand Up @@ -340,14 +347,16 @@ public static JibBuildRunner createJibBuildRunnerForRegistryImage(
* @throws InvalidFilesModificationTimeException if configured modification time could not be
* parsed
* @throws InvalidCreationTimeException if configured creation time could not be parsed
* @throws ExtraDirectoryNotFoundException if the extra directory specified for the build is not
* found
*/
public static String getSkaffoldSyncMap(
RawConfiguration rawConfiguration, ProjectProperties projectProperties, Set<Path> excludes)
throws IOException, InvalidCreationTimeException, InvalidImageReferenceException,
IncompatibleBaseImageJavaVersionException, InvalidPlatformException,
InvalidContainerVolumeException, MainClassInferenceException, InvalidAppRootException,
InvalidWorkingDirectoryException, InvalidFilesModificationTimeException,
InvalidContainerizingModeException {
InvalidContainerizingModeException, ExtraDirectoryNotFoundException {
JibContainerBuilder jibContainerBuilder =
processCommonConfiguration(
rawConfiguration, ignored -> Optional.empty(), projectProperties);
Expand Down Expand Up @@ -406,7 +415,7 @@ static JibContainerBuilder processCommonConfiguration(
IncompatibleBaseImageJavaVersionException, IOException, InvalidImageReferenceException,
InvalidContainerizingModeException, MainClassInferenceException, InvalidPlatformException,
InvalidContainerVolumeException, InvalidWorkingDirectoryException,
InvalidCreationTimeException {
InvalidCreationTimeException, ExtraDirectoryNotFoundException {

// Create and configure JibContainerBuilder
ModificationTimeProvider modificationTimeProvider =
Expand Down Expand Up @@ -446,6 +455,8 @@ static JibContainerBuilder processCommonConfiguration(
extraDirectory.getExcludesList(),
rawConfiguration.getExtraDirectoryPermissions(),
modificationTimeProvider));
} else if (!from.endsWith(DEFAULT_JIB_DIR)) {
throw new ExtraDirectoryNotFoundException(from.toString(), from.toString());
}
}
return jibContainerBuilder;
Expand All @@ -461,7 +472,8 @@ static JibContainerBuilder processCommonConfiguration(
IOException, InvalidWorkingDirectoryException, InvalidPlatformException,
InvalidContainerVolumeException, IncompatibleBaseImageJavaVersionException,
NumberFormatException, InvalidContainerizingModeException,
InvalidFilesModificationTimeException, InvalidCreationTimeException {
InvalidFilesModificationTimeException, InvalidCreationTimeException,
ExtraDirectoryNotFoundException {
JibSystemProperties.checkHttpTimeoutProperty();
JibSystemProperties.checkProxyPortProperty();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public void testAsList_credentialHelperPath() throws IOException {
.dockerCredentialHelper(fakeCredentialHelperPath.toString());

Files.delete(fakeCredentialHelperPath);
Exception ex = assertThrows(FileNotFoundException.class, () -> credentialRetrievers.asList());
Exception ex = assertThrows(FileNotFoundException.class, credentialRetrievers::asList);
assertThat(ex)
.hasMessageThat()
.isEqualTo("Specified credential helper was not found: " + fakeCredentialHelperPath);
Expand Down Expand Up @@ -294,7 +294,7 @@ public void testCredentialHelper_cmdExtension() throws IOException {
DefaultCredentialRetrievers credentialRetrievers =
new DefaultCredentialRetrievers(mockCredentialRetrieverFactory, properties, environment)
.setCredentialHelper(pathWithoutCmd.toString());
Exception ex = assertThrows(FileNotFoundException.class, () -> credentialRetrievers.asList());
Exception ex = assertThrows(FileNotFoundException.class, credentialRetrievers::asList);
assertThat(ex).hasMessageThat().startsWith("Specified credential helper was not found:");
assertThat(ex).hasMessageThat().endsWith("foo");

Expand Down Expand Up @@ -333,7 +333,7 @@ public void testCredentialHelper_exeExtension() throws IOException {
DefaultCredentialRetrievers credentialRetrievers =
new DefaultCredentialRetrievers(mockCredentialRetrieverFactory, properties, environment)
.setCredentialHelper(pathWithoutExe.toString());
Exception ex = assertThrows(FileNotFoundException.class, () -> credentialRetrievers.asList());
Exception ex = assertThrows(FileNotFoundException.class, credentialRetrievers::asList);
assertThat(ex).hasMessageThat().startsWith("Specified credential helper was not found:");
assertThat(ex).hasMessageThat().endsWith("foo");

Expand Down
Loading

0 comments on commit 3f2caac

Please sign in to comment.