Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Gradle Property and Provider to enable lazy evaluation for jib.extraDirectories parameters #3737

Merged
merged 24 commits into from
Aug 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a55e53a
Update permissions type from Map to MapProperty
emmileaf Aug 16, 2022
019fd65
Convert path.from and path.into to property types
emmileaf Aug 16, 2022
3402b29
WIP: add/fix unit tests
emmileaf Aug 16, 2022
63319af
WIP: changes to setFrom to accept Provider type
emmileaf Aug 17, 2022
5e6f237
WIP: comment out added tests for more debugging
emmileaf Aug 17, 2022
a5f1aee
WIP: clean up updated setFrom to allow for lazy eval
emmileaf Aug 17, 2022
56cb69e
WIP: fix added unit test, can lazy eval using setFrom but not setPaths
emmileaf Aug 17, 2022
b43c295
Add extra directories to lazy evaluation testing project
emmileaf Aug 17, 2022
7370713
Switch setFrom to method overloading
emmileaf Aug 18, 2022
1360011
Update setPaths to allow for lazy eval
emmileaf Aug 18, 2022
1819eb3
Update unit tests to check for both specification options
emmileaf Aug 18, 2022
b2815b7
Remove setPermissions test
emmileaf Aug 19, 2022
798882d
Fix file formatting
emmileaf Aug 19, 2022
b79d5ec
Switch from anonymous inner classes to lambdas
emmileaf Aug 22, 2022
925d5c3
Fix test compatibility with windows
emmileaf Aug 22, 2022
af4bb2c
Coverage: add tests for extraDirectories in JibExtensionTest
emmileaf Aug 23, 2022
4d986fa
Rename test file for committing test extra directory
emmileaf Aug 24, 2022
bd9183d
Update changelog and readme
emmileaf Aug 24, 2022
11d9dd1
Move object to parameters logic into helper method
emmileaf Aug 24, 2022
c7e131e
Fix javadoc checkstyle violations
emmileaf Aug 24, 2022
f380733
Rename added tests
emmileaf Aug 24, 2022
96197cf
Rename helper method
emmileaf Aug 24, 2022
25e47a5
Updated test to use stricter assert statement for path.from
emmileaf Aug 25, 2022
6f0a3b7
Update test file with more informative content
emmileaf Aug 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions jib-gradle-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.
### Added

- Included `imagePushed` field to image metadata json output file which provides information on whether an image was pushed by Jib. Note that the output file is `build/jib-image.json` by default or configurable with `jib.outputPaths.imageJson`. ([#3641](https://github.com/GoogleContainerTools/jib/pull/3641))
- Added lazy evaluation for `jib.extraDirectories` parameters using Gradle Property and Provider. ([#3737](https://github.com/GoogleContainerTools/jib/issues/3737))

### Changed

Expand Down
22 changes: 22 additions & 0 deletions jib-gradle-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,28 @@ Using `paths` as a closure, you may also specify the target of the copy and incl
}
```

You can also configure `paths` and `permissions` through [lazy configuration in Gradle](https://docs.gradle.org/current/userguide/lazy_configuration.html), using providers in `build.gradle`:

```groovy
extraDirectories {
paths = project.provider { 'src/main/custom-extra-dir' }
permissions = project.provider { ['/path/on/container/to/fileA': '755'] }
}
```

```groovy
extraDirectories {
paths {
path {
from = project.provider { 'src/main/custom-extra-dir' }
into = project.provider { '/dest-in-container' }
includes = project.provider { ['*.txt', '**/*.txt'] }
excludes = project.provider { ['hidden.txt'] }
}
}
}
```

### Authentication Methods

Pushing/pulling from private registries require authorization credentials.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.inject.Inject;
import org.gradle.api.Action;
import org.gradle.api.Project;
import org.gradle.api.model.ObjectFactory;
import org.gradle.api.provider.ListProperty;
import org.gradle.api.provider.MapProperty;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.Internal;

Expand All @@ -40,14 +43,15 @@ public class ExtraDirectoriesParameters {

private ListProperty<ExtraDirectoryParameters> paths;
private ExtraDirectoryParametersSpec spec;
private Map<String, String> permissions = Collections.emptyMap();
private MapProperty<String, String> permissions;

@Inject
public ExtraDirectoriesParameters(ObjectFactory objects, Project project) {
this.objects = objects;
this.project = project;
paths = objects.listProperty(ExtraDirectoryParameters.class).empty();
spec = objects.newInstance(ExtraDirectoryParametersSpec.class, project, paths);
permissions = objects.mapProperty(String.class, String.class).empty();
}

public void paths(Action<? super ExtraDirectoryParametersSpec> action) {
Expand Down Expand Up @@ -92,10 +96,28 @@ public List<ExtraDirectoryParameters> getPaths() {
* @param paths paths to set.
*/
public void setPaths(Object paths) {
this.paths.set(
project.files(paths).getFiles().stream()
.map(file -> new ExtraDirectoryParameters(objects, project, file.toPath(), "/"))
.collect(Collectors.toList()));
this.paths.set(convertToExtraDirectoryParametersList(paths));
}

/**
* Sets paths, for lazy evaluation where {@code paths} is a {@link Provider} of a suitable object.
*
* @param paths provider of paths to set
* @see #setPaths(Object)
*/
public void setPaths(Provider<Object> paths) {
mpeddada1 marked this conversation as resolved.
Show resolved Hide resolved
this.paths.set(paths.map(this::convertToExtraDirectoryParametersList));
}

/**
* Helper method to convert {@code Object} to {@code List<ExtraDirectoryParameters>} in {@code
* setFrom}.
*/
@Nonnull
private List<ExtraDirectoryParameters> convertToExtraDirectoryParametersList(Object obj) {
return project.files(obj).getFiles().stream()
.map(file -> new ExtraDirectoryParameters(objects, project, file.toPath(), "/"))
.collect(Collectors.toList());
}

/**
Expand All @@ -106,15 +128,15 @@ public void setPaths(Object paths) {
* @return the permissions map from path on container to file permissions
*/
@Input
public Map<String, String> getPermissions() {
public MapProperty<String, String> getPermissions() {
String property = System.getProperty(PropertyNames.EXTRA_DIRECTORIES_PERMISSIONS);
if (property != null) {
return ConfigurationPropertyValidator.parseMapProperty(property);
Map<String, String> parsedPermissions =
ConfigurationPropertyValidator.parseMapProperty(property);
if (!parsedPermissions.equals(permissions.get())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this check even matter? There is no harm in setting permissions to an equivalent object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True! I saw that both PR #3242 and #3034 both included this check in the getter, and my understanding was that we wanted to avoid setting a property multiple times here?

permissions.set(parsedPermissions);
}
}
return permissions;
}

public void setPermissions(Map<String, String> permissions) {
this.permissions = permissions;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,56 +24,68 @@
import org.gradle.api.Project;
import org.gradle.api.model.ObjectFactory;
import org.gradle.api.provider.ListProperty;
import org.gradle.api.provider.Property;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.Internal;

/** Configuration of an extra directory. */
public class ExtraDirectoryParameters implements ExtraDirectoriesConfiguration {

private Project project;
private Path from = Paths.get("");
private String into = "/";
private Property<Path> from;
private Property<String> into;
private ListProperty<String> includes;
private ListProperty<String> excludes;

@Inject
public ExtraDirectoryParameters(ObjectFactory objects, Project project) {
this.project = project;
includes = objects.listProperty(String.class).empty();
excludes = objects.listProperty(String.class).empty();
this.from = objects.property(Path.class).value(Paths.get(""));
this.into = objects.property(String.class).value("/");
this.includes = objects.listProperty(String.class).empty();
this.excludes = objects.listProperty(String.class).empty();
}

ExtraDirectoryParameters(ObjectFactory objects, Project project, Path from, String into) {
this(objects, project);
this.from = from;
this.into = into;
this.from = objects.property(Path.class).value(from);
this.into = objects.property(String.class).value(into);
}

@Input
public String getFromString() {
// Gradle warns about @Input annotations on File objects, so we have to expose a getter for a
// String to make them go away.
return from.toString();
return from.get().toString();
}

@Override
@Internal
public Path getFrom() {
return from;
return from.get();
}

public void setFrom(Object from) {
this.from = project.file(from).toPath();
this.from.set(project.file(from).toPath());
}

public void setFrom(Provider<Object> from) {
this.from.set(from.map(obj -> project.file(obj).toPath()));
}

@Override
@Input
public String getInto() {
return into;
return into.get();
}

public void setInto(String into) {
this.into = into;
this.into.set(into);
}

public void setInto(Provider<String> into) {
this.into.set(into);
}

@Input
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ public List<? extends ExtraDirectoriesConfiguration> getExtraDirectories() {

@Override
public Map<String, FilePermissions> getExtraDirectoryPermissions() {
return TaskCommon.convertPermissionsMap(jibExtension.getExtraDirectories().getPermissions());
return TaskCommon.convertPermissionsMap(
jibExtension.getExtraDirectories().getPermissions().get());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import java.util.List;
import java.util.Properties;
import org.gradle.api.Project;
import org.gradle.api.provider.Provider;
import org.gradle.api.provider.ProviderFactory;
import org.gradle.testfixtures.ProjectBuilder;
import org.junit.Before;
import org.junit.Rule;
Expand Down Expand Up @@ -241,23 +243,42 @@ public void testExtraDirectories_default() {
assertThat(testJibExtension.getExtraDirectories().getPaths()).hasSize(1);
assertThat(testJibExtension.getExtraDirectories().getPaths().get(0).getFrom())
.isEqualTo(fakeProject.getProjectDir().toPath().resolve("src/main/jib"));
assertThat(testJibExtension.getExtraDirectories().getPermissions()).isEmpty();
assertThat(testJibExtension.getExtraDirectories().getPermissions().get()).isEmpty();
}

@Test
public void testExtraDirectories() {
testJibExtension.extraDirectories(
extraDirectories -> {
extraDirectories.setPaths("test/path");
extraDirectories.setPermissions(ImmutableMap.of("file1", "123", "file2", "456"));
});

assertThat(testJibExtension.getExtraDirectories().getPaths()).hasSize(1);
assertThat(testJibExtension.getExtraDirectories().getPaths().get(0).getFrom())
.isEqualTo(fakeProject.getProjectDir().toPath().resolve("test/path"));
assertThat(testJibExtension.getExtraDirectories().getPermissions())
.containsExactly("file1", "123", "file2", "456")
.inOrder();
}

@Test
public void testExtraDirectories_lazyEvaluation_setFromInto() {
testJibExtension.extraDirectories(
extraDirectories ->
extraDirectories.paths(
paths -> {
ProviderFactory providerFactory = fakeProject.getProviders();
Provider<Object> from = providerFactory.provider(() -> "test/path");
Provider<String> into = providerFactory.provider(() -> "/target");
paths.path(
path -> {
path.setFrom(from);
path.setInto(into);
});
}));

assertThat(testJibExtension.getExtraDirectories().getPaths()).hasSize(1);
assertThat(testJibExtension.getExtraDirectories().getPaths().get(0).getFrom())
.isEqualTo(fakeProject.getProjectDir().toPath().resolve("test/path"));
assertThat(testJibExtension.getExtraDirectories().getPaths().get(0).getInto())
.isEqualTo("/target");
}

@Test
Expand Down Expand Up @@ -309,6 +330,23 @@ public void testExtraDirectories_stringListForPaths() {
.isEqualTo(fakeProject.getProjectDir().toPath().resolve("another/path"));
}

@Test
public void testExtraDirectories_lazyEvaluation_StringListForPaths() {
testJibExtension.extraDirectories(
extraDirectories -> {
ProviderFactory providerFactory = fakeProject.getProviders();
Provider<Object> paths =
providerFactory.provider(() -> Arrays.asList("test/path", "another/path"));
extraDirectories.setPaths(paths);
});

assertThat(testJibExtension.getExtraDirectories().getPaths()).hasSize(2);
assertThat(testJibExtension.getExtraDirectories().getPaths().get(0).getFrom())
.isEqualTo(fakeProject.getProjectDir().toPath().resolve("test/path"));
assertThat(testJibExtension.getExtraDirectories().getPaths().get(1).getFrom())
.isEqualTo(fakeProject.getProjectDir().toPath().resolve("another/path"));
}

@Test
public void testExtraDirectories_fileListForPaths() {
testJibExtension.extraDirectories(
Expand Down Expand Up @@ -456,7 +494,7 @@ public void testProperties() {
assertThat(testJibExtension.getExtraDirectories().getPaths().get(1).getFrom())
.isEqualTo(Paths.get("/bar/baz"));
System.setProperty("jib.extraDirectories.permissions", "/foo/bar=707,/baz=456");
assertThat(testJibExtension.getExtraDirectories().getPermissions())
assertThat(testJibExtension.getExtraDirectories().getPermissions().get())
.containsExactly("/foo/bar", "707", "/baz", "456")
.inOrder();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,37 @@ public void testLazyEvalForLabels() {
"labels contain values [firstkey:updated-first-label, secondKey:updated-second-label]");
}

@Test
public void testLazyEvalForExtraDirectories() {
BuildResult checkExtraDirectories =
testProject.build("check-extra-directories", "-Djib.console=plain");
assertThat(checkExtraDirectories.getOutput()).contains("[/updated:755]");
assertThat(checkExtraDirectories.getOutput()).contains("updated-custom-extra-dir");
}

@Test
public void testLazyEvalForExtraDirectories_individualPaths() throws IOException {
BuildResult checkExtraDirectories =
testProject.build(
"check-extra-directories", "-b=build-extra-dirs.gradle", "-Djib.console=plain");

Path extraDirectoryPath =
testProject
.getProjectRoot()
.resolve("src")
.resolve("main")
.resolve("updated-custom-extra-dir")
.toRealPath();
assertThat(checkExtraDirectories.getOutput())
.contains("extraDirectories (from): [" + extraDirectoryPath + "]");
assertThat(checkExtraDirectories.getOutput())
.contains("extraDirectories (into): [/updated-custom-into-dir]");
assertThat(checkExtraDirectories.getOutput())
.contains("extraDirectories (includes): [[include.txt]]");
assertThat(checkExtraDirectories.getOutput())
.contains("extraDirectories (excludes): [[exclude.txt]]");
}

private Project createProject(String... plugins) {
Project project =
ProjectBuilder.builder().withProjectDir(testProjectRoot.getRoot()).withName("root").build();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
plugins {
id 'java'
id 'com.google.cloud.tools.jib'
}

sourceCompatibility = 1.8
targetCompatibility = 1.8

repositories {
mavenCentral()
}

dependencies {
implementation 'com.google.guava:guava:23.6-jre'
}

project.ext.value = 'original'

project.afterEvaluate {
project.ext.value = 'updated'
project.ext.getCustomIncludes = { -> return ['include.txt'] }
project.ext.getCustomExcludes = { -> return ['exclude.txt'] }
}

jib {
extraDirectories {
paths {
path {
from = project.provider { 'src/main/' + project.ext.value + '-custom-extra-dir' }
into = project.provider { '/' + project.ext.value + '-custom-into-dir' }
includes = project.provider { -> project.ext.getCustomIncludes() }
excludes = project.provider { -> project.ext.getCustomExcludes() }
}
}
}
}

tasks.register('check-extra-directories') {
List<Object> from = project.extensions.getByName('jib')['extraDirectories']['paths'].collect{ path -> path['from']}
List<Object> into = project.extensions.getByName('jib')['extraDirectories']['paths'].collect{ path -> path['into']}
List<Object> includes = project.extensions.getByName('jib')['extraDirectories']['paths'].collect{ path -> path['includes'].get()}
List<Object> excludes = project.extensions.getByName('jib')['extraDirectories']['paths'].collect{ path -> path['excludes'].get()}
println('extraDirectories (from): ' + from)
println('extraDirectories (into): ' + into)
println('extraDirectories (includes): ' + includes)
println('extraDirectories (excludes): ' + excludes)
}
Loading