Skip to content
This repository has been archived by the owner on Mar 31, 2022. It is now read-only.

Add support for supplying build-args, fixes #22 #41

Merged
merged 2 commits into from
Jul 10, 2017
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
19 changes: 18 additions & 1 deletion plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
<name>Dockerfile Maven Plugin</name>
<description>Adds support for building Dockerfiles in Maven</description>

<properties>
<docker-client.version>8.8.0</docker-client.version>
</properties>

<dependencyManagement>
<dependencies>
<dependency>
Expand All @@ -29,7 +33,7 @@
<groupId>com.spotify</groupId>
<artifactId>docker-client</artifactId>
<classifier>shaded</classifier>
<version>8.8.0</version>
<version>${docker-client.version}</version>
</dependency>
<dependency>
<groupId>com.google.auth</groupId>
Expand Down Expand Up @@ -64,6 +68,12 @@
<version>3.0.0</version>
</dependency>

<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>2.8.0</version>
</dependency>

<dependency>
<groupId>org.apache.maven.plugin-tools</groupId>
<artifactId>maven-plugin-annotations</artifactId>
Expand Down Expand Up @@ -125,6 +135,13 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-invoker-plugin</artifactId>
<version>1.9</version>
<dependencies>
<dependency>
<groupId>com.spotify</groupId>
<artifactId>docker-client</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed as a dependency of maven-invoker-plugin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The invoker-plugin executes the verify.groovy scripts which don't seem to inherit the dependencies in the pom. I had to add it here to be able to use the DefaultDockerClient in the test.

Copy link
Member

Choose a reason for hiding this comment

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

Ah that makes sense, thanks for the clarification

<version>${docker-client.version}</version>
</dependency>
</dependencies>
<configuration>
<cloneProjectsTo>${project.build.directory}/it</cloneProjectsTo>
<pomIncludes>
Expand Down
6 changes: 6 additions & 0 deletions plugin/src/it/basic-with-build-args/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FROM scratch
MAINTAINER David Flemström <dflemstr@spotify.com>

ARG IMAGE_VERSION

LABEL version=${IMAGE_VERSION}
58 changes: 58 additions & 0 deletions plugin/src/it/basic-with-build-args/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
-/-/-
Dockerfile Maven Plugin
%%
Copyright (C) 2015 - 2016 Spotify AB
%%
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-\-\-
-->

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>com.spotify.it</groupId>
<artifactId>basic-with-build-args</artifactId>
<version>1.0-SNAPSHOT</version>

<description>A simple IT verifying a basic use case with build arguments.</description>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

<build>
<plugins>
<plugin>
<groupId>@project.groupId@</groupId>
<artifactId>@project.artifactId@</artifactId>
<version>@project.version@</version>
<executions>
<execution>
<id>default</id>
<goals>
<goal>build</goal>
</goals>
<configuration>
<buildArgs>
<IMAGE_VERSION>0.0.1</IMAGE_VERSION>
</buildArgs>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
28 changes: 28 additions & 0 deletions plugin/src/it/basic-with-build-args/verify.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* -/-/-
* Dockerfile Maven Plugin
* %%
* Copyright (C) 2015 - 2016 Spotify AB
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* -\-\-
*/
import com.spotify.docker.client.DefaultDockerClient

File imageIdFile = new File(basedir, "target/docker/image-id")
String imageId = imageIdFile.text.replaceAll("\\s", "")

DefaultDockerClient dockerClient = DefaultDockerClient.fromEnv().build()
imageInfo = dockerClient.inspectImage(imageId)

assert imageInfo.config().labels().get("version") == "0.0.1"
24 changes: 22 additions & 2 deletions plugin/src/main/java/com/spotify/plugin/dockerfile/BuildMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,16 @@

package com.spotify.plugin.dockerfile;

import com.google.gson.Gson;
import com.spotify.docker.client.DockerClient;
import com.spotify.docker.client.exceptions.DockerException;
import java.io.File;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Map;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.apache.maven.plugin.MojoExecutionException;
Expand Down Expand Up @@ -82,6 +86,12 @@ public class BuildMojo extends AbstractDockerMojo {
@Parameter(property = "dockerfile.build.noCache", defaultValue = "false")
private boolean noCache;

/**
* Custom build arguments
*/
@Parameter(property = "dockerfile.buildArgs")
private Map<String,String> buildArgs;

@Override
public void execute(DockerClient dockerClient)
throws MojoExecutionException, MojoFailureException {
Expand All @@ -93,7 +103,7 @@ public void execute(DockerClient dockerClient)
}

final String imageId = buildImage(
dockerClient, log, verbose, contextDirectory, repository, tag, pullNewerImage, noCache);
dockerClient, log, verbose, contextDirectory, repository, tag, pullNewerImage, noCache, buildArgs);
Copy link
Member

@mattnworb mattnworb Jul 7, 2017

Choose a reason for hiding this comment

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

buildArgs is now the seventh argument passed to this method that is an instance variable, perhaps it is time to make buildImage non-static so we don't have to have so many parameters passed to a method in the same class (which is never used by outside callers).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds like a good idea. If you want I could give it a go, unless you think it's outside the PR scope.

Copy link
Member

Choose a reason for hiding this comment

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

either option works for me


if (imageId == null) {
log.warn("Docker build was successful, but no image was built");
Expand Down Expand Up @@ -124,7 +134,8 @@ static String buildImage(@Nonnull DockerClient dockerClient,
@Nullable String repository,
@Nonnull String tag,
boolean pullNewerImage,
boolean noCache)
boolean noCache,
@Nullable Map<String,String> buildArgs)
throws MojoExecutionException, MojoFailureException {

log.info(MessageFormat.format("Building Docker context {0}", contextDirectory));
Expand All @@ -145,6 +156,15 @@ static String buildImage(@Nonnull DockerClient dockerClient,
buildParameters.add(DockerClient.BuildParam.noCache());
}

if (buildArgs != null && !buildArgs.isEmpty()) {
try {
final String encodedBuildArgs = URLEncoder.encode(new Gson().toJson(buildArgs), "utf-8");
Copy link
Member

@mattnworb mattnworb Jul 7, 2017

Choose a reason for hiding this comment

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

I could be wrong but I think Jersey (used by docker-client) encodes the name/value of the BuildParam as query params before they are passed to the Docker Remote API. Are you sure the encoding is needed here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was surprised by this as well but without the encoding it complains about " in the request:

[INFO] [ERROR] Failed to execute goal com.spotify:dockerfile-maven-plugin:1.3.2-ooyala-build-args:build (default) on project basic-with-build-args: Execution default of goal com.spotify:dockerfile-maven-plugin:1.3.2-ooyala-build-args:build failed: Illegal character """ at position 21 is not allowed as a start of a name in a path template "pull=true&buildargs={"IMAGE_VERSION":"0.0.1"}". -> [Help 1]

buildParameters.add(new DockerClient.BuildParam("buildargs", encodedBuildArgs));
} catch (UnsupportedEncodingException e) {
throw new MojoExecutionException("Could not build image", e);
}
}

final DockerClient.BuildParam[] buildParametersArray =
buildParameters.toArray(new DockerClient.BuildParam[buildParameters.size()]);

Expand Down