Skip to content

Commit

Permalink
fix : Jib ignores DOCKER_REGISTRY environment variable (#1617)
Browse files Browse the repository at this point in the history 
JibBuildService should check all available registry options before
submitting ImageConfiguration to Jib.

Signed-off-by: Rohan Kumar <[email protected]>
  • Loading branch information
@rohanKanojia
rohanKanojia committed Feb 28, 2023
1 parent 920fde8 commit 1c0cc00
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 31 deletions.
4 changes: 4 additions & 0 deletions doc/changelog.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ChangeLog
* **0.42-SNAPSHOT** :
- Support `docker run --platform` ([1641](https://github.com/fabric8io/docker-maven-plugin/issues/1641)) @chonton
- Update buildx documentation to amplify the build behavior ([1646](https://github.com/fabric8io/docker-maven-plugin/pull/1646)) @chonton
- Default to native platform when creating container ([1645](https://github.com/fabric8io/docker-maven-plugin/pull/1645)) @chonton
- JIB ignores the `DOCKER_REGISTRY` environment variable ([1617](https://github.com/fabric8io/docker-maven-plugin/issues/1617)) @rohanKanojia

* **0.41.0** (2023-02-06):
- multi-arch build should use provided repository ([1597](https://github.com/fabric8io/docker-maven-plugin/issues/1597)) @merikan
Expand Down
43 changes: 28 additions & 15 deletions src/main/java/io/fabric8/maven/docker/service/JibBuildService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ public class JibBuildService {
private static final List<String> DEFAULT_DOCKER_REGISTRIES = Arrays.asList(
"docker.io", "index.docker.io", "registry.hub.docker.com"
);
private static final String PUSH_REGISTRY = ".docker.push.registry";
private static final String ARCHIVE_FILE_NAME = "docker-build";
private final Logger log;
private final ServiceHub serviceHub;
Expand All @@ -51,10 +50,10 @@ public void build(String jibImageFormat, ImageConfiguration imageConfig, Registr
if (imageConfig.getBuildConfiguration().isDockerFileMode()) {
throw new MojoExecutionException("Dockerfile mode is not supported with JIB build strategy");
}
prependRegistry(imageConfig, mojoParameters.getProject().getProperties().getProperty(PUSH_REGISTRY));
prepareImageConfiguration(imageConfig, registryConfig);
BuildDirs buildDirs = new BuildDirs(imageConfig.getName(), mojoParameters);
final Credential pullRegistryCredential = getRegistryCredentials(
registryConfig, false, imageConfig, log);
registryConfig, false, imageConfig);
final JibContainerBuilder containerBuilder = containerFromImageConfiguration(jibImageFormat, imageConfig, pullRegistryCredential);

File dockerTarArchive = getAssemblyTarArchive(imageConfig, serviceHub, mojoParameters, log);
Expand Down Expand Up @@ -85,11 +84,11 @@ public void build(String jibImageFormat, ImageConfiguration imageConfig, Registr
public void push(Collection<ImageConfiguration> imageConfigs, int retries, RegistryService.RegistryConfig registryConfig, boolean skipTag) throws MojoExecutionException {
try {
for (ImageConfiguration imageConfiguration : imageConfigs) {
prependRegistry(imageConfiguration, registryConfig.getRegistry());
prepareImageConfiguration(imageConfiguration, registryConfig);
log.info("This push refers to: %s", imageConfiguration.getName());
JibServiceUtil.jibPush(
imageConfiguration,
getRegistryCredentials(registryConfig, true, imageConfiguration, log),
getRegistryCredentials(registryConfig, true, imageConfiguration),
getBuildTarArchive(imageConfiguration, mojoParameters),
log
);
Expand Down Expand Up @@ -117,21 +116,14 @@ static File getAssemblyTarArchive(ImageConfiguration imageConfig, ServiceHub ser
}

static Credential getRegistryCredentials(
RegistryService.RegistryConfig registryConfig, boolean isPush, ImageConfiguration imageConfiguration, Logger log)
RegistryService.RegistryConfig registryConfig, boolean isPush, ImageConfiguration imageConfiguration)
throws MojoExecutionException {

String registry;
if (isPush) {
registry = EnvUtil.firstRegistryOf(
new ImageName(imageConfiguration.getName()).getRegistry(),
imageConfiguration.getRegistry(),
registryConfig.getRegistry()
);
registry = getConfiguredPushRegistry(registryConfig, imageConfiguration);
} else {
registry = EnvUtil.firstRegistryOf(
new ImageName(getBaseImage(imageConfiguration)).getRegistry(),
registryConfig.getRegistry()
);
registry = getConfiguredPullRegistry(registryConfig, imageConfiguration);
}
if (registry == null || DEFAULT_DOCKER_REGISTRIES.contains(registry)) {
registry = DOCKER_LOGIN_DEFAULT_REGISTRY; // Let's assume docker is default registry.
Expand All @@ -150,4 +142,25 @@ static File getBuildTarArchive(ImageConfiguration imageConfiguration, MojoParame
BuildDirs buildDirs = new BuildDirs(imageConfiguration.getName(), mojoParameters);
return new File(buildDirs.getTemporaryRootDirectory(), ARCHIVE_FILE_NAME + "." + ArchiveCompression.none.getFileSuffix());
}

static ImageConfiguration prepareImageConfiguration(ImageConfiguration imageConfig, RegistryService.RegistryConfig registryConfig) {
String configuredRegistry = getConfiguredPushRegistry(registryConfig, imageConfig);
prependRegistry(imageConfig, configuredRegistry);
return imageConfig;
}

private static String getConfiguredPushRegistry(RegistryService.RegistryConfig registryConfig, ImageConfiguration imageConfiguration) {
return EnvUtil.firstRegistryOf(
new ImageName(imageConfiguration.getName()).getRegistry(),
imageConfiguration.getRegistry(),
registryConfig.getRegistry()
);
}

private static String getConfiguredPullRegistry(RegistryService.RegistryConfig registryConfig, ImageConfiguration imageConfiguration) {
return EnvUtil.firstRegistryOf(
new ImageName(getBaseImage(imageConfiguration)).getRegistry(),
registryConfig.getRegistry()
);
}
}
2 changes: 1 addition & 1 deletion src/main/java/io/fabric8/maven/docker/util/JibServiceUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ static Set<String> getAllImageTags(List<String> tags, String imageName) {
}

static ImageFormat getImageFormat(String jibImageFormat) {
if (jibImageFormat != null && jibImageFormat.toLowerCase().equalsIgnoreCase("oci")) {
if (jibImageFormat != null && jibImageFormat.equalsIgnoreCase("oci")) {
return ImageFormat.OCI;
}
return ImageFormat.Docker;
Expand Down
50 changes: 35 additions & 15 deletions src/test/java/io/fabric8/maven/docker/service/JibBuildServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
import java.nio.file.Path;
import java.util.Collections;
import java.util.List;
import java.util.Properties;

import com.google.cloud.tools.jib.api.Credential;
import com.google.cloud.tools.jib.api.TarImage;

import io.fabric8.maven.docker.util.EnvUtil;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.project.MavenProject;
import org.apache.maven.settings.Settings;
Expand All @@ -33,6 +32,10 @@
import io.fabric8.maven.docker.util.Logger;
import io.fabric8.maven.docker.util.MojoParameters;

import static io.fabric8.maven.docker.service.JibBuildService.prepareImageConfiguration;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mockStatic;

@ExtendWith(MockitoExtension.class)
class JibBuildServiceTest {

Expand Down Expand Up @@ -70,11 +73,11 @@ void testGetRegistryCredentialsForPush() throws MojoExecutionException {
mockAuthConfigFactory(true, registryConfig);
// When
Credential credential = JibBuildService.getRegistryCredentials(
registryConfig, true, imageConfiguration, logger);
registryConfig, true, imageConfiguration);
// Then
Assertions.assertNotNull(credential);
Assertions.assertEquals("testuserpush", credential.getUsername());
Assertions.assertEquals("testpass", credential.getPassword());
assertEquals("testuserpush", credential.getUsername());
assertEquals("testpass", credential.getPassword());
}

@Test
Expand All @@ -90,11 +93,11 @@ void testGetRegistryCredentialsForPull() throws MojoExecutionException {
mockAuthConfigFactory(false, registryConfig);
// When
Credential credential = JibBuildService.getRegistryCredentials(
registryConfig, false, imageConfiguration, logger);
registryConfig, false, imageConfiguration);
// Then
Assertions.assertNotNull(credential);
Assertions.assertEquals("testuserpull", credential.getUsername());
Assertions.assertEquals("testpass", credential.getPassword());
assertEquals("testuserpull", credential.getUsername());
assertEquals("testpass", credential.getPassword());
}

@Test
Expand All @@ -109,7 +112,7 @@ void testGetBuildTarArchive() throws IOException {

// Then
Assertions.assertNotNull(tarArchive);
Assertions.assertEquals(new File("/target/test/testimage/0.0.1/tmp/docker-build.tar").getPath(),
assertEquals(new File("/target/test/testimage/0.0.1/tmp/docker-build.tar").getPath(),
tarArchive.getAbsolutePath().substring(projectBaseDir.getAbsolutePath().length()));
}

Expand All @@ -126,7 +129,7 @@ void testGetAssemblyTarArchive() throws IOException, MojoExecutionException {

// Then
Assertions.assertNotNull(tarArchive);
Assertions.assertEquals(new File("/target/test/testimage/0.0.1/tmp/docker-build.tar").getPath(),
assertEquals(new File("/target/test/testimage/0.0.1/tmp/docker-build.tar").getPath(),
tarArchive.getAbsolutePath().substring(projectBaseDir.toString().length()));
}

Expand All @@ -138,12 +141,12 @@ void testPrependRegistry() {
JibBuildService.prependRegistry(imageConfiguration, "quay.io");
// Then
Assertions.assertNotNull(imageConfiguration);
Assertions.assertEquals("quay.io/test/testimage:0.0.1", imageConfiguration.getName());
assertEquals("quay.io/test/testimage:0.0.1", imageConfiguration.getName());
}

@Test
void testPushWithNoConfigurations() {
try (MockedStatic<JibServiceUtil> jibServiceUtilMock = Mockito.mockStatic(JibServiceUtil.class)) {
try (MockedStatic<JibServiceUtil> jibServiceUtilMock = mockStatic(JibServiceUtil.class)) {
// Given
jibServiceUtilMock
.when(() -> JibServiceUtil.jibPush(Mockito.any(ImageConfiguration.class), Mockito.any(Credential.class), Mockito.any(File.class), Mockito.any(Logger.class)))
Expand All @@ -159,7 +162,6 @@ void testPushWithNoConfigurations() {
@Test
void testBuildCallsBuildContainer(@TempDir Path tmpDir) throws Exception {
// ARRANGE
Mockito.when(project.getProperties()).thenReturn(new Properties());
setupServiceHubExpectations(tmpDir.toFile());
setupDockerAssemblyExpectations(tmpDir);
final RegistryService.RegistryConfig registryConfig = new RegistryService.RegistryConfig.Builder()
Expand All @@ -170,7 +172,7 @@ void testBuildCallsBuildContainer(@TempDir Path tmpDir) throws Exception {
JibBuildService jibBuildService = new JibBuildService(serviceHub, params, logger);
ImageConfiguration imageConfiguration = getImageConfiguration();

try (MockedStatic<JibServiceUtil> jibServiceUtilMock = Mockito.mockStatic(JibServiceUtil.class)) {
try (MockedStatic<JibServiceUtil> jibServiceUtilMock = mockStatic(JibServiceUtil.class)) {
jibServiceUtilMock.when(() -> JibServiceUtil.getBaseImage(imageConfiguration)).thenCallRealMethod();
// ACT
jibBuildService.build("docker", imageConfiguration, registryConfig);
Expand All @@ -183,7 +185,7 @@ void testBuildCallsBuildContainer(@TempDir Path tmpDir) throws Exception {
@Test
@Disabled("Cannot intercept JibServiceUtil.pushImage() to prevent actual image creation")
void testPushWithConfiguration(@TempDir Path tmpDir) throws Exception {
try (MockedStatic<JibServiceUtil> jibServiceUtilMock = Mockito.mockStatic(JibServiceUtil.class)) {
try (MockedStatic<JibServiceUtil> jibServiceUtilMock = mockStatic(JibServiceUtil.class)) {
// Given
setupServiceHubExpectations(tmpDir.toFile());
setupDockerAssemblyExpectations(tmpDir);
Expand All @@ -204,6 +206,24 @@ void testPushWithConfiguration(@TempDir Path tmpDir) throws Exception {
}
}

@Test
void prepareImageConfiguration_whenDockerRegistryPropertySet_thenImageConfigurationShouldHaveRegistry() {
try (MockedStatic<EnvUtil> envUtilMockedStatic = mockStatic(EnvUtil.class)) {
// Given
envUtilMockedStatic.when(() -> EnvUtil.firstRegistryOf(null, null, null)).thenReturn("registry-from-docker-registry-env.io");
ImageConfiguration imageConfiguration = new ImageConfiguration.Builder()
.name("test/foo:bar")
.build();
RegistryService.RegistryConfig registryConfig = new RegistryService.RegistryConfig();

// When
ImageConfiguration result = prepareImageConfiguration(imageConfiguration, registryConfig);

// Then
assertEquals("registry-from-docker-registry-env.io/test/foo:bar", result.getName());
}
}

private ImageConfiguration getImageConfiguration() {
return new ImageConfiguration.Builder()
.name("test/testimage:0.0.1")
Expand Down

0 comments on commit 1c0cc00

Please sign in to comment.