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

Add glob support for extra file permissions #2345

Merged
merged 7 commits into from
Mar 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions jib-gradle-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ All notable changes to this project will be documented in this file.

### Added

- Glob pattern support for `jib.extraDirectories.permissions`. ([#1200](https://github.com/GoogleContainerTools/jib/issues/1200))

### Changed

- `jib.container.creationTime` now accepts more timezone formats:`+HHmm`. This allows for easier configuration of creationTime by external systems. ([#2320](https://github.com/GoogleContainerTools/jib/issues/2320))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.google.cloud.tools.jib.gradle;

import com.google.cloud.tools.jib.api.buildplan.AbsoluteUnixPath;
import com.google.cloud.tools.jib.api.buildplan.FilePermissions;
import com.google.cloud.tools.jib.api.buildplan.ImageFormat;
import com.google.cloud.tools.jib.plugins.common.AuthProperty;
Expand Down Expand Up @@ -162,7 +161,7 @@ public List<Path> getExtraDirectories() {
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@
import com.google.api.client.http.HttpTransport;
import com.google.cloud.tools.jib.ProjectInfo;
import com.google.cloud.tools.jib.api.LogEvent;
import com.google.cloud.tools.jib.api.buildplan.AbsoluteUnixPath;
import com.google.cloud.tools.jib.api.buildplan.FilePermissions;
import com.google.cloud.tools.jib.plugins.common.ProjectProperties;
import com.google.cloud.tools.jib.plugins.common.UpdateChecker;
import com.google.common.util.concurrent.Futures;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -114,19 +113,17 @@ static void disableHttpLogging() {
}

/**
* Validates and converts a {@code String->String} file-path-to-file-permissions map to an
* equivalent {@code AbsoluteUnixPath->FilePermission} map.
* Converts a {@code String->String} file-path-to-file-permissions map to an equivalent {@code
* String->FilePermission} map.
*
* @param stringMap the map to convert (example entry: {@code "/path/on/container" -> "755"})
* @return the converted map
*/
static Map<AbsoluteUnixPath, FilePermissions> convertPermissionsMap(
Map<String, String> stringMap) {
Map<AbsoluteUnixPath, FilePermissions> permissionsMap = new HashMap<>();
static Map<String, FilePermissions> convertPermissionsMap(Map<String, String> stringMap) {
// Order is important, so use a LinkedHashMap
Map<String, FilePermissions> permissionsMap = new LinkedHashMap<>();
for (Map.Entry<String, String> entry : stringMap.entrySet()) {
AbsoluteUnixPath key = AbsoluteUnixPath.get(entry.getKey());
FilePermissions value = FilePermissions.fromOctalString(entry.getValue());
permissionsMap.put(key, value);
permissionsMap.put(entry.getKey(), FilePermissions.fromOctalString(entry.getValue()));
}
return permissionsMap;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,9 @@ public void testIsWarProject() {
public void testConvertPermissionsMap() {
Assert.assertEquals(
ImmutableMap.of(
AbsoluteUnixPath.get("/test/folder/file1"),
"/test/folder/file1",
FilePermissions.fromOctalString("123"),
AbsoluteUnixPath.get("/test/file2"),
"/test/file2",
FilePermissions.fromOctalString("456")),
TaskCommon.convertPermissionsMap(
ImmutableMap.of("/test/folder/file1", "123", "/test/file2", "456")));
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 @@ -5,6 +5,8 @@ All notable changes to this project will be documented in this file.

### Added

- Glob pattern support for `<extraDirectories><permissions>`. ([#1200](https://github.com/GoogleContainerTools/jib/issues/1200))

### Changed

- `<container><creationTime>` now accepts more timezone formats:`+HHmm`. This allows for easier configuration of creationTime by external systems. ([#2320](https://github.com/GoogleContainerTools/jib/issues/2320))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.google.cloud.tools.jib.maven;

import com.google.cloud.tools.jib.api.buildplan.AbsoluteUnixPath;
import com.google.cloud.tools.jib.api.buildplan.FilePermissions;
import com.google.cloud.tools.jib.api.buildplan.ImageFormat;
import com.google.cloud.tools.jib.plugins.common.AuthProperty;
Expand Down Expand Up @@ -167,7 +166,7 @@ public List<Path> getExtraDirectories() {
}

@Override
public Map<AbsoluteUnixPath, FilePermissions> getExtraDirectoryPermissions() {
public Map<String, FilePermissions> getExtraDirectoryPermissions() {
return MojoCommon.convertPermissionsList(jibPluginConfiguration.getExtraDirectoryPermissions());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import com.google.cloud.tools.jib.ProjectInfo;
import com.google.cloud.tools.jib.api.LogEvent;
import com.google.cloud.tools.jib.api.buildplan.AbsoluteUnixPath;
import com.google.cloud.tools.jib.api.buildplan.FilePermissions;
import com.google.cloud.tools.jib.maven.JibPluginConfiguration.PermissionConfiguration;
import com.google.cloud.tools.jib.plugins.common.ProjectProperties;
Expand All @@ -30,7 +29,7 @@
import com.google.common.util.concurrent.Futures;
import java.nio.file.Path;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -100,24 +99,23 @@ static List<Path> getExtraDirectories(JibPluginConfiguration jibPluginConfigurat
}

/**
* Validates and converts a list of {@link PermissionConfiguration} to an equivalent {@code
* AbsoluteUnixPath->FilePermission} map.
* Converts a list of {@link PermissionConfiguration} to an equivalent {@code
* String->FilePermission} map.
*
* @param permissionList the list to convert
* @return the resulting map
*/
@VisibleForTesting
static Map<AbsoluteUnixPath, FilePermissions> convertPermissionsList(
static Map<String, FilePermissions> convertPermissionsList(
List<PermissionConfiguration> permissionList) {
Map<AbsoluteUnixPath, FilePermissions> permissionsMap = new HashMap<>();
// Order is important, so use a LinkedHashMap
Map<String, FilePermissions> permissionsMap = new LinkedHashMap<>();
for (PermissionConfiguration permission : permissionList) {
if (!permission.getFile().isPresent() || !permission.getMode().isPresent()) {
throw new IllegalArgumentException(
"Incomplete <permission> configuration; requires <file> and <mode> fields to be set");
}
AbsoluteUnixPath key = AbsoluteUnixPath.get(permission.getFile().get());
FilePermissions value = FilePermissions.fromOctalString(permission.getMode().get());
permissionsMap.put(key, value);
permissionsMap.put(
permission.getFile().get(), FilePermissions.fromOctalString(permission.getMode().get()));
}
return permissionsMap;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@
import com.google.cloud.tools.jib.api.buildplan.RelativeUnixPath;
import com.google.cloud.tools.jib.filesystem.DirectoryWalker;
import java.io.IOException;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.nio.file.Paths;
import java.time.Instant;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.function.BiFunction;
import java.util.function.Predicate;
Expand All @@ -46,22 +50,41 @@ public class JavaContainerBuilderHelper {
*/
public static FileEntriesLayer extraDirectoryLayerConfiguration(
Path extraDirectory,
Map<AbsoluteUnixPath, FilePermissions> extraDirectoryPermissions,
Map<String, FilePermissions> extraDirectoryPermissions,
BiFunction<Path, AbsoluteUnixPath, Instant> modificationTimeProvider)
throws IOException {
FileEntriesLayer.Builder builder =
FileEntriesLayer.builder().setName(LayerType.EXTRA_FILES.getName());
Map<PathMatcher, FilePermissions> pathMatchers = new LinkedHashMap<>();
for (Map.Entry<String, FilePermissions> entry : extraDirectoryPermissions.entrySet()) {
pathMatchers.put(
FileSystems.getDefault().getPathMatcher("glob:" + entry.getKey()), entry.getValue());
}

new DirectoryWalker(extraDirectory)
.filterRoot()
.walk(
localPath -> {
AbsoluteUnixPath pathOnContainer =
AbsoluteUnixPath.get("/").resolve(extraDirectory.relativize(localPath));
Instant modificationTime = modificationTimeProvider.apply(localPath, pathOnContainer);
FilePermissions permissions = extraDirectoryPermissions.get(pathOnContainer);
FilePermissions permissions =
extraDirectoryPermissions.get(pathOnContainer.toString());
if (permissions == null) {
// Check for matching globs
Path containerPath = Paths.get(pathOnContainer.toString());
for (Map.Entry<PathMatcher, FilePermissions> entry : pathMatchers.entrySet()) {
if (entry.getKey().matches(containerPath)) {
builder.addEntry(
localPath, pathOnContainer, entry.getValue(), modificationTime);
return;
}
}

// Add with default permissions
builder.addEntry(localPath, pathOnContainer, modificationTime);
} else {
// Add with explicit permissions
builder.addEntry(localPath, pathOnContainer, permissions, modificationTime);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

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

import com.google.cloud.tools.jib.api.buildplan.AbsoluteUnixPath;
import com.google.cloud.tools.jib.api.buildplan.FilePermissions;
import com.google.cloud.tools.jib.api.buildplan.ImageFormat;
import java.nio.file.Path;
Expand Down Expand Up @@ -81,7 +80,7 @@ public interface RawConfiguration {

List<Path> getExtraDirectories();

Map<AbsoluteUnixPath, FilePermissions> getExtraDirectoryPermissions();
Map<String, FilePermissions> getExtraDirectoryPermissions();

Optional<Path> getDockerExecutable();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@
import com.google.cloud.tools.jib.api.buildplan.AbsoluteUnixPath;
import com.google.cloud.tools.jib.api.buildplan.FileEntriesLayer;
import com.google.cloud.tools.jib.api.buildplan.FileEntry;
import com.google.cloud.tools.jib.api.buildplan.FilePermissions;
import com.google.cloud.tools.jib.configuration.BuildContext;
import com.google.cloud.tools.jib.filesystem.FileOperations;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.Resources;
import com.google.common.util.concurrent.MoreExecutors;
import java.io.IOException;
Expand All @@ -41,6 +43,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.junit.Assert;
Expand Down Expand Up @@ -97,6 +100,76 @@ public void testExtraDirectoryLayerConfiguration() throws URISyntaxException, IO
layerConfiguration.getEntries());
}

@Test
public void testExtraDirectoryLayerConfiguration_globPermissions()
throws URISyntaxException, IOException {
Path extraFilesDirectory = Paths.get(Resources.getResource("core/layer").toURI());
Map<String, FilePermissions> permissionsMap =
ImmutableMap.of(
"/a",
FilePermissions.fromOctalString("123"),
"/a/*",
FilePermissions.fromOctalString("456"),
"**/bar",
FilePermissions.fromOctalString("765"));
FileEntriesLayer fileEntriesLayer =
JavaContainerBuilderHelper.extraDirectoryLayerConfiguration(
extraFilesDirectory, permissionsMap, (ignored1, ignored2) -> Instant.EPOCH);
assertExtractionPathsUnordered(
Arrays.asList("/a", "/a/b", "/a/b/bar", "/c", "/c/cat", "/foo"),
fileEntriesLayer.getEntries());

Map<AbsoluteUnixPath, FilePermissions> expectedPermissions =
ImmutableMap.<AbsoluteUnixPath, FilePermissions>builder()
.put(AbsoluteUnixPath.get("/a"), FilePermissions.fromOctalString("123"))
.put(AbsoluteUnixPath.get("/a/b"), FilePermissions.fromOctalString("456"))
.put(AbsoluteUnixPath.get("/a/b/bar"), FilePermissions.fromOctalString("765"))
.put(AbsoluteUnixPath.get("/c"), FilePermissions.DEFAULT_FOLDER_PERMISSIONS)
.put(AbsoluteUnixPath.get("/c/cat"), FilePermissions.DEFAULT_FILE_PERMISSIONS)
.put(AbsoluteUnixPath.get("/foo"), FilePermissions.DEFAULT_FILE_PERMISSIONS)
.build();
for (FileEntry entry : fileEntriesLayer.getEntries()) {
Assert.assertEquals(
expectedPermissions.get(entry.getExtractionPath()), entry.getPermissions());
}
}

@Test
public void testExtraDirectoryLayerConfiguration_overlappingPermissions()
throws URISyntaxException, IOException {
Path extraFilesDirectory = Paths.get(Resources.getResource("core/layer").toURI());
Map<String, FilePermissions> permissionsMap =
ImmutableMap.of(
"/a**",
FilePermissions.fromOctalString("123"),
// Should be ignored, since first match takes priority
"/a/b**",
FilePermissions.fromOctalString("000"),
// Should override first match since explicit path is used instead of glob
"/a/b/bar",
FilePermissions.fromOctalString("765"));
FileEntriesLayer fileEntriesLayer =
JavaContainerBuilderHelper.extraDirectoryLayerConfiguration(
extraFilesDirectory, permissionsMap, (ignored1, ignored2) -> Instant.EPOCH);
assertExtractionPathsUnordered(
Arrays.asList("/a", "/a/b", "/a/b/bar", "/c", "/c/cat", "/foo"),
fileEntriesLayer.getEntries());

Map<AbsoluteUnixPath, FilePermissions> expectedPermissions =
ImmutableMap.<AbsoluteUnixPath, FilePermissions>builder()
.put(AbsoluteUnixPath.get("/a"), FilePermissions.fromOctalString("123"))
.put(AbsoluteUnixPath.get("/a/b"), FilePermissions.fromOctalString("123"))
.put(AbsoluteUnixPath.get("/a/b/bar"), FilePermissions.fromOctalString("765"))
.put(AbsoluteUnixPath.get("/c"), FilePermissions.DEFAULT_FOLDER_PERMISSIONS)
.put(AbsoluteUnixPath.get("/c/cat"), FilePermissions.DEFAULT_FILE_PERMISSIONS)
.put(AbsoluteUnixPath.get("/foo"), FilePermissions.DEFAULT_FILE_PERMISSIONS)
.build();
for (FileEntry entry : fileEntriesLayer.getEntries()) {
Assert.assertEquals(
expectedPermissions.get(entry.getExtractionPath()), entry.getPermissions());
}
}

@Test
public void testFromExplodedWar()
throws URISyntaxException, IOException, InvalidImageReferenceException,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ public void testPluginConfigurationProcessor_extraDirectory()
Path extraDirectory = Paths.get(Resources.getResource("core/layer").toURI());
Mockito.when(rawConfiguration.getExtraDirectories()).thenReturn(Arrays.asList(extraDirectory));
Mockito.when(rawConfiguration.getExtraDirectoryPermissions())
.thenReturn(
ImmutableMap.of(AbsoluteUnixPath.get("/foo"), FilePermissions.fromOctalString("123")));
.thenReturn(ImmutableMap.of("/foo", FilePermissions.fromOctalString("123")));

BuildContext buildContext = getBuildContext(processCommonConfiguration());
List<FileEntry> extraFiles =
Expand Down