-
Notifications
You must be signed in to change notification settings - Fork 445
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 support for --chown flag for ADD/COPY Docker commands #1044
Changes from 1 commit
fecf78c
d5472d1
c78eec6
9eac2d5
3fc8c88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,9 @@ import com.typesafe.sbt.SbtNativePackager.Universal | |
import com.typesafe.sbt.packager.Compat._ | ||
import com.typesafe.sbt.packager.{MappingsHelper, Stager} | ||
|
||
import scala.sys.process.Process | ||
import scala.util.Try | ||
|
||
/** | ||
* == Docker Plugin == | ||
* | ||
|
@@ -80,6 +83,9 @@ object DockerPlugin extends AutoPlugin { | |
dockerEntrypoint := Seq("bin/%s" format executableScriptName.value), | ||
dockerCmd := Seq(), | ||
dockerExecCommand := Seq("docker"), | ||
dockerVersion := Try(Process(dockerExecCommand.value ++ Seq("version --format '{{.Server.Version}}'")).!!) | ||
.toOption.map(_.trim) | ||
.flatMap(DockerVersion.parse), | ||
dockerBuildOptions := Seq("--force-rm") ++ Seq("-t", dockerAlias.value.versioned) ++ ( | ||
if (dockerUpdateLatest.value) | ||
Seq("-t", dockerAlias.value.latest) | ||
|
@@ -96,7 +102,7 @@ object DockerPlugin extends AutoPlugin { | |
val generalCommands = makeFrom(dockerBaseImage.value) +: makeMaintainer((maintainer in Docker).value).toSeq | ||
|
||
generalCommands ++ | ||
Seq(makeWorkdir(dockerBaseDirectory), makeAdd(dockerBaseDirectory), makeChown(user, group, "." :: Nil)) ++ | ||
Seq(makeWorkdir(dockerBaseDirectory)) ++ makeAdd(dockerVersion.value, dockerBaseDirectory, user, group) ++ | ||
dockerLabels.value.map(makeLabel) ++ | ||
makeExposePorts(dockerExposedPorts.value, dockerExposedUdpPorts.value) ++ | ||
makeVolumes(dockerExposedVolumes.value, user, group) ++ | ||
|
@@ -179,18 +185,34 @@ object DockerPlugin extends AutoPlugin { | |
Cmd("WORKDIR", dockerBaseDirectory) | ||
|
||
/** | ||
* @param dockerBaseDirectory, the installation directory | ||
* @param dockerVersion | ||
* @param dockerBaseDirectory the installation directory | ||
* @param daemonUser | ||
* @param daemonGroup | ||
* @return ADD command adding all files inside the installation directory | ||
*/ | ||
private final def makeAdd(dockerBaseDirectory: String): CmdLike = { | ||
private final def makeAdd(dockerVersion: Option[DockerVersion], dockerBaseDirectory: String, | ||
daemonUser: String, daemonGroup: String): Seq[CmdLike] = { | ||
|
||
/** | ||
* This is the file path of the file in the Docker image, and does not depend on the OS where the image | ||
* is being built. This means that it needs to be the Unix file separator even when the image is built | ||
* on e.g. Windows systems. | ||
*/ | ||
val files = dockerBaseDirectory.split(UnixSeparatorChar)(1) | ||
Cmd("ADD", s"$files /$files") | ||
|
||
dockerVersion match { | ||
case Some(DockerVersion(major, minor, _, _)) if major >= 17 && minor >= 9 => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The At this point I'm fine with putting this into a if statement. If we have more of this special case, we should object DockerSupport {
def chownFlag(version: DockerVersion): Boolean = ...
}
|
||
Seq( | ||
Cmd("ADD", s"--chown=$daemonUser:$daemonGroup $files /$files") | ||
) | ||
|
||
case _ => | ||
Seq( | ||
Cmd("ADD", s"$files /$files"), | ||
makeChown(daemonUser, daemonGroup, "." :: Nil) | ||
) | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -278,7 +300,7 @@ object DockerPlugin extends AutoPlugin { | |
} | ||
|
||
/** | ||
* uses the `mappings in Unversial` to generate the | ||
* uses the `mappings in Universal` to generate the | ||
* `mappings in Docker`. | ||
*/ | ||
def mapGenericFilesToDocker: Seq[Setting[_]] = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package com.typesafe.sbt.packager.docker | ||
|
||
import scala.util.matching.Regex | ||
|
||
|
||
case class DockerVersion(major: Int, minor: Int, patch: Int, release: Option[String]) | ||
|
||
object DockerVersion { | ||
private val DockerVersionPattern: Regex = "^'([0-9]+).([0-9]+).([0-9]+)-?([-a-z]+)?'$".r | ||
|
||
def parse(version: String): Option[DockerVersion] = { | ||
Option(version).collect { | ||
case DockerVersionPattern(major, minor, patch, release) => | ||
new DockerVersion(major.toInt, minor.toInt, patch.toInt, Option(release)) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guys, I don't think this works...
I believe it needs to be
Seq("version", "--format", "'{{.Server.Version}}'")
, otherwise your shell will add another set of ticks around the whole thing and Docker will report that it does not know the command.I will try to confirm this and send a PR is necessary. I'd love to use this feature :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm me, already fixed somewhere else :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#1052 and @mrfyda ftw! :D