From 47465f6ed5d456d0f07e9520b0fe7a7149c3f5e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Mon, 29 Jul 2024 15:36:57 +0200 Subject: [PATCH] Do not validate settings if publishing is disabled This commit improves the Maven Plugin to only validate the publishing settings if publishing is actually enabled. Closes gh-29756 --- .../boot/maven/BuildImageMojo.java | 7 +-- .../springframework/boot/maven/Docker.java | 13 +++-- .../boot/maven/DockerTests.java | 54 ++++++++++++++----- 3 files changed, 53 insertions(+), 21 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildImageMojo.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildImageMojo.java index 79b62bf53030..3d737fa7c219 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildImageMojo.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildImageMojo.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -246,9 +246,10 @@ public void execute() throws MojoExecutionException { private void buildImage() throws MojoExecutionException { Libraries libraries = getLibraries(Collections.emptySet()); try { - DockerConfiguration dockerConfiguration = (this.docker != null) ? this.docker.asDockerConfiguration() - : new Docker().asDockerConfiguration(); BuildRequest request = getBuildRequest(libraries); + DockerConfiguration dockerConfiguration = (this.docker != null) + ? this.docker.asDockerConfiguration(request.isPublish()) + : new Docker().asDockerConfiguration(request.isPublish()); Builder builder = new Builder(new MojoBuildLog(this::getLog), dockerConfiguration); builder.build(request); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/Docker.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/Docker.java index 53618609d4d7..a28403a179c9 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/Docker.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/Docker.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -140,14 +140,15 @@ void setPublishRegistry(DockerRegistry builderRegistry) { * Returns this configuration as a {@link DockerConfiguration} instance. This method * should only be called when the configuration is complete and will no longer be * changed. + * @param publish whether the image should be published * @return the Docker configuration */ - DockerConfiguration asDockerConfiguration() { + DockerConfiguration asDockerConfiguration(boolean publish) { DockerConfiguration dockerConfiguration = new DockerConfiguration(); dockerConfiguration = customizeHost(dockerConfiguration); dockerConfiguration = dockerConfiguration.withBindHostToBuilder(this.bindHostToBuilder); dockerConfiguration = customizeBuilderAuthentication(dockerConfiguration); - dockerConfiguration = customizePublishAuthentication(dockerConfiguration); + dockerConfiguration = customizePublishAuthentication(dockerConfiguration, publish); return dockerConfiguration; } @@ -180,7 +181,11 @@ private DockerConfiguration customizeBuilderAuthentication(DockerConfiguration d "Invalid Docker builder registry configuration, either token or username/password must be provided"); } - private DockerConfiguration customizePublishAuthentication(DockerConfiguration dockerConfiguration) { + private DockerConfiguration customizePublishAuthentication(DockerConfiguration dockerConfiguration, + boolean publish) { + if (!publish) { + return dockerConfiguration; + } if (this.publishRegistry == null || this.publishRegistry.isEmpty()) { return dockerConfiguration.withEmptyPublishRegistryAuthentication(); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/DockerTests.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/DockerTests.java index f2258b915dc3..65a162d8b3c7 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/DockerTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/DockerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,7 +37,7 @@ class DockerTests { @Test void asDockerConfigurationWithDefaults() { Docker docker = new Docker(); - DockerConfiguration dockerConfiguration = docker.asDockerConfiguration(); + DockerConfiguration dockerConfiguration = createDockerConfiguration(docker); assertThat(dockerConfiguration.getHost()).isNull(); assertThat(dockerConfiguration.getBuilderRegistryAuthentication()).isNull(); assertThat(decoded(dockerConfiguration.getPublishRegistryAuthentication().getAuthHeader())) @@ -53,14 +53,14 @@ void asDockerConfigurationWithHostConfiguration() { docker.setHost("docker.example.com"); docker.setTlsVerify(true); docker.setCertPath("/tmp/ca-cert"); - DockerConfiguration dockerConfiguration = docker.asDockerConfiguration(); + DockerConfiguration dockerConfiguration = createDockerConfiguration(docker); DockerHostConfiguration host = dockerConfiguration.getHost(); assertThat(host.getAddress()).isEqualTo("docker.example.com"); assertThat(host.isSecure()).isTrue(); assertThat(host.getCertificatePath()).isEqualTo("/tmp/ca-cert"); assertThat(host.getContext()).isNull(); assertThat(dockerConfiguration.isBindHostToBuilder()).isFalse(); - assertThat(docker.asDockerConfiguration().getBuilderRegistryAuthentication()).isNull(); + assertThat(createDockerConfiguration(docker).getBuilderRegistryAuthentication()).isNull(); assertThat(decoded(dockerConfiguration.getPublishRegistryAuthentication().getAuthHeader())) .contains("\"username\" : \"\"") .contains("\"password\" : \"\"") @@ -72,14 +72,14 @@ void asDockerConfigurationWithHostConfiguration() { void asDockerConfigurationWithContextConfiguration() { Docker docker = new Docker(); docker.setContext("test-context"); - DockerConfiguration dockerConfiguration = docker.asDockerConfiguration(); + DockerConfiguration dockerConfiguration = createDockerConfiguration(docker); DockerHostConfiguration host = dockerConfiguration.getHost(); assertThat(host.getContext()).isEqualTo("test-context"); assertThat(host.getAddress()).isNull(); assertThat(host.isSecure()).isFalse(); assertThat(host.getCertificatePath()).isNull(); assertThat(dockerConfiguration.isBindHostToBuilder()).isFalse(); - assertThat(docker.asDockerConfiguration().getBuilderRegistryAuthentication()).isNull(); + assertThat(createDockerConfiguration(docker).getBuilderRegistryAuthentication()).isNull(); assertThat(decoded(dockerConfiguration.getPublishRegistryAuthentication().getAuthHeader())) .contains("\"username\" : \"\"") .contains("\"password\" : \"\"") @@ -92,7 +92,7 @@ void asDockerConfigurationWithHostAndContextFails() { Docker docker = new Docker(); docker.setContext("test-context"); docker.setHost("docker.example.com"); - assertThatIllegalArgumentException().isThrownBy(docker::asDockerConfiguration) + assertThatIllegalArgumentException().isThrownBy(() -> createDockerConfiguration(docker)) .withMessageContaining("Invalid Docker configuration"); } @@ -103,13 +103,13 @@ void asDockerConfigurationWithBindHostToBuilder() { docker.setTlsVerify(true); docker.setCertPath("/tmp/ca-cert"); docker.setBindHostToBuilder(true); - DockerConfiguration dockerConfiguration = docker.asDockerConfiguration(); + DockerConfiguration dockerConfiguration = createDockerConfiguration(docker); DockerHostConfiguration host = dockerConfiguration.getHost(); assertThat(host.getAddress()).isEqualTo("docker.example.com"); assertThat(host.isSecure()).isTrue(); assertThat(host.getCertificatePath()).isEqualTo("/tmp/ca-cert"); assertThat(dockerConfiguration.isBindHostToBuilder()).isTrue(); - assertThat(docker.asDockerConfiguration().getBuilderRegistryAuthentication()).isNull(); + assertThat(createDockerConfiguration(docker).getBuilderRegistryAuthentication()).isNull(); assertThat(decoded(dockerConfiguration.getPublishRegistryAuthentication().getAuthHeader())) .contains("\"username\" : \"\"") .contains("\"password\" : \"\"") @@ -124,7 +124,7 @@ void asDockerConfigurationWithUserAuth() { new Docker.DockerRegistry("user1", "secret1", "https://docker1.example.com", "docker1@example.com")); docker.setPublishRegistry( new Docker.DockerRegistry("user2", "secret2", "https://docker2.example.com", "docker2@example.com")); - DockerConfiguration dockerConfiguration = docker.asDockerConfiguration(); + DockerConfiguration dockerConfiguration = createDockerConfiguration(docker); assertThat(decoded(dockerConfiguration.getBuilderRegistryAuthentication().getAuthHeader())) .contains("\"username\" : \"user1\"") .contains("\"password\" : \"secret1\"") @@ -142,7 +142,7 @@ void asDockerConfigurationWithIncompleteBuilderUserAuthFails() { Docker docker = new Docker(); docker.setBuilderRegistry( new Docker.DockerRegistry("user", null, "https://docker.example.com", "docker@example.com")); - assertThatIllegalArgumentException().isThrownBy(docker::asDockerConfiguration) + assertThatIllegalArgumentException().isThrownBy(() -> createDockerConfiguration(docker)) .withMessageContaining("Invalid Docker builder registry configuration"); } @@ -151,16 +151,25 @@ void asDockerConfigurationWithIncompletePublishUserAuthFails() { Docker docker = new Docker(); docker.setPublishRegistry( new Docker.DockerRegistry("user", null, "https://docker.example.com", "docker@example.com")); - assertThatIllegalArgumentException().isThrownBy(docker::asDockerConfiguration) + assertThatIllegalArgumentException().isThrownBy(() -> createDockerConfiguration(docker)) .withMessageContaining("Invalid Docker publish registry configuration"); } + @Test + void asDockerConfigurationWithIncompletePublishUserAuthDoesNotFailIfPublishIsDisabled() { + Docker docker = new Docker(); + docker.setPublishRegistry( + new Docker.DockerRegistry("user", null, "https://docker.example.com", "docker@example.com")); + DockerConfiguration dockerConfiguration = docker.asDockerConfiguration(false); + assertThat(dockerConfiguration.getPublishRegistryAuthentication()).isNull(); + } + @Test void asDockerConfigurationWithTokenAuth() { Docker docker = new Docker(); docker.setBuilderRegistry(new Docker.DockerRegistry("token1")); docker.setPublishRegistry(new Docker.DockerRegistry("token2")); - DockerConfiguration dockerConfiguration = docker.asDockerConfiguration(); + DockerConfiguration dockerConfiguration = createDockerConfiguration(docker); assertThat(decoded(dockerConfiguration.getBuilderRegistryAuthentication().getAuthHeader())) .contains("\"identitytoken\" : \"token1\""); assertThat(decoded(dockerConfiguration.getPublishRegistryAuthentication().getAuthHeader())) @@ -175,10 +184,27 @@ void asDockerConfigurationWithUserAndTokenAuthFails() { dockerRegistry.setToken("token"); Docker docker = new Docker(); docker.setBuilderRegistry(dockerRegistry); - assertThatIllegalArgumentException().isThrownBy(docker::asDockerConfiguration) + assertThatIllegalArgumentException().isThrownBy(() -> createDockerConfiguration(docker)) .withMessageContaining("Invalid Docker builder registry configuration"); } + @Test + void asDockerConfigurationWithUserAndTokenAuthDoesNotFailIfPublishingIsDisabled() { + Docker.DockerRegistry dockerRegistry = new Docker.DockerRegistry(); + dockerRegistry.setUsername("user"); + dockerRegistry.setPassword("secret"); + dockerRegistry.setToken("token"); + Docker docker = new Docker(); + docker.setPublishRegistry(dockerRegistry); + DockerConfiguration dockerConfiguration = docker.asDockerConfiguration(false); + assertThat(dockerConfiguration.getPublishRegistryAuthentication()).isNull(); + } + + private DockerConfiguration createDockerConfiguration(Docker docker) { + return docker.asDockerConfiguration(true); + + } + String decoded(String value) { return new String(Base64.getDecoder().decode(value)); }