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

Docker publishRegistry in Maven plugin configuration is validated when publish option is false #29756

Closed
cdprete opened this issue Feb 12, 2022 · 6 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@cdprete
Copy link

cdprete commented Feb 12, 2022

Hi.
I'm using Spring Boot 2.6.3 and I've the spring-boot-maven-plugin configured in the following way:

                <plugin>
                    <groupId>org.springframework.boot</groupId>
                    <artifactId>spring-boot-maven-plugin</artifactId>
                    <version>${spring-boot.version}</version>
                    <executions>
                        <execution>
                            <goals>
                                <goal>repackage</goal>
                            </goals>
                        </execution>
                        <execution>
                            <id>build-image</id>
                            <goals>
                                <goal>build-image</goal>
                            </goals>
                            <configuration>
                                <image>
                                    <name>${docker.image-name}</name>
                                </image>
                                <docker>
                                    <publishRegistry>
                                        <username>${docker.credentials.username}</username>
                                        <password>${docker.credentials.password}</password>
                                    </publishRegistry>
                                </docker>
                            </configuration>
                        </execution>
                    </executions>
                </plugin>

where the registry credentials are simply empty as

<docker.credentials.username />
<docker.credentials.password />

I've tried to set the publish flag dynamically with Groovy in order to set it to false if one of the credentials is missing.
Everything works fine if both the credentials are missing. As soon as one is specified, the publishRegistry block complains even though the publish flag is false.

The Groovy part is

                <plugin>
                    <groupId>org.codehaus.gmavenplus</groupId>
                    <artifactId>gmavenplus-plugin</artifactId>
                    <version>1.13.1</version>
                    <dependencies>
                        <dependency>
                            <groupId>org.codehaus.groovy</groupId>
                            <artifactId>groovy-all</artifactId>
                            <version>3.0.9</version>
                            <type>pom</type>
                        </dependency>
                    </dependencies>
                    <executions>
                        <execution>
                            <id>check-if-image-must-be-published</id>
                            <phase>validate</phase>
                            <goals>
                                <goal>execute</goal>
                            </goals>
                        </execution>
                    </executions>
                    <configuration>
                        <scripts>
                            <script><![CDATA[
                                def dockerCredentialsUsernamePropertyName = 'docker.credentials.username'
                                def dockerCredentialsPasswordPropertyName = 'docker.credentials.password'
                                def dockerCredentialsUsername = checkPropertyValue(dockerCredentialsUsernamePropertyName)
                                def dockerCredentialsPassword = checkPropertyValue(dockerCredentialsPasswordPropertyName)
                                if(!dockerCredentialsUsername || !dockerCredentialsPassword) {
                                    project.properties['spring-boot.build-image.publish'] = false
                                }

                                String checkPropertyValue(String name) {
                                    def value = getPropertyValue(name)
                                    if(!value)  {
                                        log.warn "The property '$name' is not set or it's blank, therefore no image will be pushed."
                                    }

                                    return value
                                }

                                String getPropertyValue(String name) {
                                    // property was defined from command line e.g.: -DpropertyName=value
                                    def value = session.userProperties[name]?.trim()
                                    if (!value) {
                                        value = project.properties[name]?.trim()
                                    }
                                    return value
                                }
                            ]]></script>
                        </scripts>
                    </configuration>
                </plugin>

Ideally, if the publish flag is false, I would expect that the registry is completely ignored since it will not be used anyway.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 12, 2022
@wilkinsona
Copy link
Member

As with #29706, you can save us all some time by providing a minimal sample that reproduces the problem. It would also be useful if you shared the build output when the problem occurs.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Feb 12, 2022
cdprete pushed a commit to cdprete/github_spring_boot_issue_29756 that referenced this issue Feb 12, 2022
cdprete pushed a commit to cdprete/github_spring_boot_issue_29756 that referenced this issue Feb 12, 2022
@cdprete
Copy link
Author

cdprete commented Feb 12, 2022

@wilkinsona I gave you here already all the needed building blocks ;)
Anyway, here there is a demo for it: https://github.com/cdprete/github_spring_boot_issue_29756
As you can see from the README, the publishRegistry block is evaluated anyway, even if the spring-boot.build-image.publish flag is set to false.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 12, 2022
@snicoll snicoll changed the title [maven-plugin] Allow to build an image without pushing it Allow to build an image with the Maven plugin without pushing it Feb 14, 2022
@scottfrederick scottfrederick changed the title Allow to build an image with the Maven plugin without pushing it Docker publishRegistry in Maven plugin configuration is validated when publish option is false Feb 14, 2022
@scottfrederick
Copy link
Contributor

@cdprete Is there a real-world reason for providing either the username or password alone, instead of always providing neither or both, or is this something you happened up when testing? The plugin is working as designed now, and I'm not sure that ignoring invalid configuration that might not be used is a good idea. I'll label it so the rest of the team can provide opinions on this also.

@scottfrederick scottfrederick added the for: team-attention An issue we'd like other members of the team to review label Feb 14, 2022
@cdprete
Copy link
Author

cdprete commented Feb 15, 2022

@scottfrederick I want to prevent build failures if not all the properties have been provided. Moreover, if I set the plugin to skip the publishing, I assume it should not really care what it's under the publishRegistry block.

In the ideal world, you would have 2 different goals bound to 2 different phases. One for building the image (done on package for example) and one for publishing it (done on deploy for example).

@snicoll
Copy link
Member

snicoll commented Feb 21, 2022

In the ideal world, you would have 2 different goals bound to 2 different phases. One for building the image (done on package for example) and one for publishing it (done on deploy for example).

The Spring Boot plugin is not a general purpose plugin for Docker operations so we are not going to do that, see #26187.

I agree that we should not attempt to validate that part of a plugin configuration is valid if the feature is disabled. I can see now that everything is backed by a DockerConfiguration object. Perhaps this can be refactored to lazily request things rather than doing this upfront?

@cdprete

This comment was marked as resolved.

@philwebb philwebb added type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Mar 2, 2022
@philwebb philwebb added this to the 2.5.x milestone Mar 2, 2022
@wilkinsona wilkinsona modified the milestones: 2.5.x, 2.6.x May 19, 2022
@wilkinsona wilkinsona modified the milestones: 2.6.x, 2.7.x Nov 24, 2022
@philwebb philwebb modified the milestones: 2.7.x, 3.1.x Nov 8, 2023
@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.2.x May 20, 2024
@snicoll snicoll self-assigned this Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

6 participants