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/stage generates different Dockerfile depending on docker version #1187

Closed
eed3si9n opened this issue Jan 10, 2019 · 6 comments
Closed
Labels

Comments

@eed3si9n
Copy link
Member

steps

$ eval $(minikube docker-env)
$ sbt
> Docker/stage

expected

Docker/stage either fails or generates an identical Dockerfile with or without running eval $(minikube docker-env).

ADD --chown=daemon:daemon opt /opt

actual

The content of Dockerfile is different.

ADD opt /opt
RUN ["chown", "-R", "daemon:daemon", "."]

This causes deployment to fail on OpenShift.

notes

Ref #1044

https://github.com/sbt/sbt-native-packager/blob/v1.3.15/src/main/scala/com/typesafe/sbt/packager/docker/DockerPlugin.scala#L226-L230

    if (dockerVersion.exists(DockerSupport.chownFlag)) {
      Seq(Cmd("ADD", s"--chown=$daemonUser:$daemonGroup $files /$files"))
    } else {
      Seq(Cmd("ADD", s"$files /$files"), makeChown(daemonUser, daemonGroup, "." :: Nil))
    }

It turns out that when I run eval $(minikube docker-env) it was detecting "17.12.1-ce".

@muuki88
Copy link
Contributor

muuki88 commented Jan 11, 2019

Thanks for the detailed bug report ❤️

From your description it looks like you have two very different docker versions on your host and in Kubernetes. If you want to support a fixed version of docker you can set the dockerVersion setting.

dockerVersion := Some("10.0.0")

or set it to None to use no newer features. Maybe we should point that out in the docs explicitly.

I'm not sure what happens if you build docker images with different docker daemon version. Are they backwards compatible? What's the error on open shift. From my little docker experience I thought the Dockerfile is only relevant during build time.

@muuki88 muuki88 added the docker label Jan 11, 2019
@eed3si9n
Copy link
Member Author

I think it makes sense for the build to occasionally outsource its work to external system like docker, but as a general principle the build should be repeatable.
I suggest we create a setting for dockerCompatibilityVersion and set it to whatever the version that introduced --chown. If we detect any version below it we can error out asking the user to either bump their Docker or lower the dockerCompatibilityVersion to "17.0".

What's the error on open shift. From my little docker experience I thought the Dockerfile is only relevant during build time.

On OpenShift, I get /opt/docker/bin/name-of-app: Permission denied.

Given that OpenShift runs containers using an arbitrarily assigned user ID, I actually don't think ADD --chown=daemon:daemon opt /opt would work either, but the failure let me realize this repeatability issue.

OKD-Specific Guidelines says:

Adding the following to your Dockerfile sets the directory and file permissions to allow users in the root group to access them in the built image:

RUN chgrp -R 0 /some/directory && \
    chmod -R g=u /some/directory

@muuki88
Copy link
Contributor

muuki88 commented Jan 12, 2019

Thanks for the clarification.

The dockerVersion setting is actually used for exactly this. While I really like the name dockerCompatibilityVersion, we don't really need it to solve this issue. If you set the dockerVersion then you get a repeatable build. What you don't get is a good error message (e.g. dockerCompatibilityVersion is higher than your actual dockerVersion). I favor less settings in this case.

Regarding the OpenShift permission issue. Should this be part of this issue? Or is this rather a feature request to remove the chown part completely?

@eed3si9n
Copy link
Member Author

Regarding the OpenShift permission issue. Should this be part of this issue? Or is this rather a feature request to remove the chown part completely?

Yea, let's split the OpenShift compatibility into another issue.

I favor less settings in this case.

Do you think it's a good design that sbt-native-packager produces different docker image depending on the docker environment at the time? My opinion is that it's counter to the immutable infrastructure idea that Docker tries to implement, and the default should be safer.

If we want to do this using one setting what we can do is let it fail when it detects older Docker servers with and error message like

Docker server 17.0.0 was detected. update Docker server or set `dockerVersion := Some("17.0.0")`

@muuki88
Copy link
Contributor

muuki88 commented Jan 14, 2019

Do you think it's a good design that sbt-native-packager produces different docker image depending on the docker environment at the time? My opinion is that it's counter to the immutable infrastructure idea that Docker tries to implement, and the default should be safer.

I agree with you. The issue with docker is that it's changing things rather quickly (deprecating, adding, removing cli flags). While rpm and debian kept stable since I started working with sbt-native-packager, docker changed a lot.

I would argue that you can expect a certain build environment to be present to produce a certain artifact. The build pipeline is part of the immutable infrastructure. The same docker version should create the same Dockerfile.

If we want to do this using one setting what we can do is let it fail when it detects older Docker servers with and error message like

You are right. We can validate the set version to the actual detected version and error out if they don't match. The question is how do we validate?

  • Check if the Dockerfiles would be equal?
  • Check only the major version?
  • Check all DockerSupport checks?

There is already a validateDockerVersion validation task, which runs before the actual packaging and prints warnings or errors. We could simply extend this logic. WDYT?

eed3si9n added a commit to eed3si9n/sbt-native-packager that referenced this issue Jan 18, 2019
Fixes sbt#1189

This implements `dockerPermissionStrategy` setting which decides how file permissions are set for the working directory inside the Docker image generated by sbt-native-packager. The strategies are:

- `DockerPermissionStrategy.MultiStage` (default): uses multi-stage Docker build to call chmod ahead of time.
- `DockerPermissionStrategy.None`: does not attempt to change the file permissions, and use the host machine's file mode bits.
- `DockerPermissionStrategy.Run`: calls `RUN` in the Dockerfile. This has regression on the resulting Docker image file size.
- `DockerPermissionStrategy.CopyChown`: calls `COPY --chown` in the Dockerfile. Provided as a backward compatibility.

For `MultiStage` and `Run` strategies, `dockerChmodType` is used in addition to call `chmod` during Docker build.

- `DockerChmodType.UserGroupReadExecute`: chmod -R u=rX,g=rX
- `DockerChmodType.UserGroupWriteExecute`: chmod -R u=rwX,g=rwX
- `DockerChmodType.SyncGroupToUser`: chmod -R g=u
- `DockerChmodType.Custom`: Custom argument provided by the user.

During staging Docker package validation is called to check if the selected strategy is compatible with the deteted Docker version.
This fixes the current repeatability issue reported as sbt#1187. If the incompatibility is detected, the user is advised to
either upgrade their Docker, pick another strategy, or override the `dockerVersion` setting.

`daemonGroup` is set to `root` instead of copying the value from the `daemonUser` setting.
This matches the semantics of `USER` as well as OpenShift, which uses gid=0.
eed3si9n added a commit to eed3si9n/sbt-native-packager that referenced this issue Jan 18, 2019
Fixes sbt#1189

This implements a non-root Docker container that's safer by default and compatible with Red Hat OpenShift.
Current `ADD --chown=daemon:daemon opt /opt` nominally implements non-root image,
but by giving ownership of the working directory to the `daemon` user, it reduces the safety.
Instead we should keep the file ownership by Docker's `root`, and use `chmod` to grant access to `daemon`.

The challenge is calling `chmod` without incurring the fs layer overhead (sbt#883).
[Multi-stage builds](https://docs.docker.com/develop/develop-images/multistage-build/) can be used to pre-stage
the files with desired file permissions.

This adds new `dockerPermissionStrategy` setting which decides how file permissions are set for the working directory inside the Docker image generated by sbt-native-packager. The strategies are:

- `DockerPermissionStrategy.MultiStage` (default): uses multi-stage Docker build to call chmod ahead of time.
- `DockerPermissionStrategy.None`: does not attempt to change the file permissions, and use the host machine's file mode bits.
- `DockerPermissionStrategy.Run`: calls `RUN` in the Dockerfile. This has regression on the resulting Docker image file size.
- `DockerPermissionStrategy.CopyChown`: calls `COPY --chown` in the Dockerfile. Provided as a backward compatibility.

For `MultiStage` and `Run` strategies, `dockerChmodType` is used in addition to call `chmod` during Docker build.

- `DockerChmodType.UserGroupReadExecute`: chmod -R u=rX,g=rX
- `DockerChmodType.UserGroupWriteExecute`: chmod -R u=rwX,g=rwX
- `DockerChmodType.SyncGroupToUser`: chmod -R g=u
- `DockerChmodType.Custom`: Custom argument provided by the user.

Here's an example to change these settings:

```scala
import com.typesafe.sbt.packager.docker._

dockerPermissionStrategy := DockerPermissionStrategy.Run
dockerChmodType          := DockerChmodType.UserGroupWriteExecute
```

During `docker:stage`, Docker package validation is called to check if the selected strategy is compatible with the deteted Docker version.
This fixes the current repeatability issue reported as sbt#1187. If the incompatibility is detected, the user is advised to
either upgrade their Docker, pick another strategy, or override the `dockerVersion` setting.

`daemonGroup` is set to `root` instead of copying the value from the `daemonUser` setting.
This matches the semantics of `USER` as well as OpenShift, which uses gid=0.
eed3si9n added a commit to eed3si9n/sbt-native-packager that referenced this issue Jan 18, 2019
Fixes sbt#1189

This implements a non-root Docker container that's safer by default and compatible with Red Hat OpenShift.
Current `ADD --chown=daemon:daemon opt /opt` nominally implements non-root image,
but by giving ownership of the working directory to the `daemon` user, it reduces the safety.
Instead we should keep the file ownership by Docker's `root`, and use `chmod` to grant access to `daemon`.

The challenge is calling `chmod` without incurring the fs layer overhead (sbt#883).
[Multi-stage builds](https://docs.docker.com/develop/develop-images/multistage-build/) can be used to pre-stage
the files with desired file permissions.

This adds new `dockerPermissionStrategy` setting which decides how file permissions are set for the working directory inside the Docker image generated by sbt-native-packager. The strategies are:

- `DockerPermissionStrategy.MultiStage` (default): uses multi-stage Docker build to call chmod ahead of time.
- `DockerPermissionStrategy.None`: does not attempt to change the file permissions, and use the host machine's file mode bits.
- `DockerPermissionStrategy.Run`: calls `RUN` in the Dockerfile. This has regression on the resulting Docker image file size.
- `DockerPermissionStrategy.CopyChown`: calls `COPY --chown` in the Dockerfile. Provided as a backward compatibility.

For `MultiStage` and `Run` strategies, `dockerChmodType` is used in addition to call `chmod` during Docker build.

- `DockerChmodType.UserGroupReadExecute` (default): chmod -R u=rX,g=rX
- `DockerChmodType.UserGroupWriteExecute`: chmod -R u=rwX,g=rwX
- `DockerChmodType.SyncGroupToUser`: chmod -R g=u
- `DockerChmodType.Custom`: Custom argument provided by the user.

Some application will require writing files to the working directory.
In that case the setting should be changed as follows:

```scala
import com.typesafe.sbt.packager.docker.DockerChmodType

dockerChmodType := DockerChmodType.UserGroupWriteExecute
```

During `docker:stage`, Docker package validation is called to check if the selected strategy is compatible with the deteted Docker version.
This fixes the current repeatability issue reported as sbt#1187. If the incompatibility is detected, the user is advised to
either upgrade their Docker, pick another strategy, or override the `dockerVersion` setting.

`daemonGroup` is set to `root` instead of copying the value from the `daemonUser` setting.
This matches the semantics of `USER` as well as OpenShift, which uses gid=0.
eed3si9n added a commit to eed3si9n/sbt-native-packager that referenced this issue Jan 18, 2019
Fixes sbt#1189

This implements a non-root Docker container that's safer by default and compatible with Red Hat OpenShift.
Current `ADD --chown=daemon:daemon opt /opt` nominally implements non-root image,
but by giving ownership of the working directory to the `daemon` user, it reduces the safety.
Instead we should use `chmod` to default to read-only access unless the build user opts into writable working directory.

The challenge is calling `chmod` without incurring the fs layer overhead (sbt#883).
[Multi-stage builds](https://docs.docker.com/develop/develop-images/multistage-build/) can be used to pre-stage
the files with desired file permissions.

This adds new `dockerPermissionStrategy` setting which decides how file permissions are set for the working directory inside the Docker image generated by sbt-native-packager. The strategies are:

- `DockerPermissionStrategy.MultiStage` (default): uses multi-stage Docker build to call chmod ahead of time.
- `DockerPermissionStrategy.None`: does not attempt to change the file permissions, and use the host machine's file mode bits.
- `DockerPermissionStrategy.Run`: calls `RUN` in the Dockerfile. This has regression on the resulting Docker image file size.
- `DockerPermissionStrategy.CopyChown`: calls `COPY --chown` in the Dockerfile. Provided as a backward compatibility.

For `MultiStage` and `Run` strategies, `dockerChmodType` is used in addition to call `chmod` during Docker build.

- `DockerChmodType.UserGroupReadExecute` (default): chmod -R u=rX,g=rX
- `DockerChmodType.UserGroupWriteExecute`: chmod -R u=rwX,g=rwX
- `DockerChmodType.SyncGroupToUser`: chmod -R g=u
- `DockerChmodType.Custom`: Custom argument provided by the user.

Some application will require writing files to the working directory.
In that case the setting should be changed as follows:

```scala
import com.typesafe.sbt.packager.docker.DockerChmodType

dockerChmodType := DockerChmodType.UserGroupWriteExecute
```

During `docker:stage`, Docker package validation is called to check if the selected strategy is compatible with the deteted Docker version.
This fixes the current repeatability issue reported as sbt#1187. If the incompatibility is detected, the user is advised to
either upgrade their Docker, pick another strategy, or override the `dockerVersion` setting.

`daemonGroup` is set to `root` instead of copying the value from the `daemonUser` setting.
This matches the semantics of `USER` as well as OpenShift, which uses gid=0.
eed3si9n added a commit to eed3si9n/sbt-native-packager that referenced this issue Jan 18, 2019
Fixes sbt#1189

This implements a non-root Docker container that's safer by default and compatible with Red Hat OpenShift.
Current `ADD --chown=daemon:daemon opt /opt` nominally implements non-root image,
but by giving ownership of the working directory to the `daemon` user, it reduces the safety.
Instead we should use `chmod` to default to read-only access unless the build user opts into writable working directory.

The challenge is calling `chmod` without incurring the fs layer overhead (sbt#883).
[Multi-stage builds](https://docs.docker.com/develop/develop-images/multistage-build/) can be used to pre-stage
the files with desired file permissions.

This adds new `dockerPermissionStrategy` setting which decides how file permissions are set for the working directory inside the Docker image generated by sbt-native-packager. The strategies are:

- `DockerPermissionStrategy.MultiStage` (default): uses multi-stage Docker build to call chmod ahead of time.
- `DockerPermissionStrategy.None`: does not attempt to change the file permissions, and use the host machine's file mode bits.
- `DockerPermissionStrategy.Run`: calls `RUN` in the Dockerfile. This has regression on the resulting Docker image file size.
- `DockerPermissionStrategy.CopyChown`: calls `COPY --chown` in the Dockerfile. Provided as a backward compatibility.

For `MultiStage` and `Run` strategies, `dockerChmodType` is used in addition to call `chmod` during Docker build.

- `DockerChmodType.UserGroupReadExecute` (default): chmod -R u=rX,g=rX
- `DockerChmodType.UserGroupWriteExecute`: chmod -R u=rwX,g=rwX
- `DockerChmodType.SyncGroupToUser`: chmod -R g=u
- `DockerChmodType.Custom`: Custom argument provided by the user.

Some application will require writing files to the working directory.
In that case the setting should be changed as follows:

```scala
import com.typesafe.sbt.packager.docker.DockerChmodType

dockerChmodType := DockerChmodType.UserGroupWriteExecute
```

During `docker:stage`, Docker package validation is called to check if the selected strategy is compatible with the deteted Docker version.
This fixes the current repeatability issue reported as sbt#1187. If the incompatibility is detected, the user is advised to
either upgrade their Docker, pick another strategy, or override the `dockerVersion` setting.

`daemonGroup` is set to `root` instead of copying the value from the `daemonUser` setting.
This matches the semantics of `USER` as well as OpenShift, which uses gid=0.
eed3si9n added a commit to eed3si9n/sbt-native-packager that referenced this issue Jan 22, 2019
Fixes sbt#1189

This implements a non-root Docker container that's safer by default and compatible with Red Hat OpenShift.
Current `ADD --chown=daemon:daemon opt /opt` nominally implements non-root image,
but by giving ownership of the working directory to the `daemon` user, it reduces the safety.
Instead we should use `chmod` to default to read-only access unless the build user opts into writable working directory.

The challenge is calling `chmod` without incurring the fs layer overhead (sbt#883).
[Multi-stage builds](https://docs.docker.com/develop/develop-images/multistage-build/) can be used to pre-stage
the files with desired file permissions.

This adds new `dockerPermissionStrategy` setting which decides how file permissions are set for the working directory inside the Docker image generated by sbt-native-packager. The strategies are:

- `DockerPermissionStrategy.MultiStage` (default): uses multi-stage Docker build to call chmod ahead of time.
- `DockerPermissionStrategy.None`: does not attempt to change the file permissions, and use the host machine's file mode bits.
- `DockerPermissionStrategy.Run`: calls `RUN` in the Dockerfile. This has regression on the resulting Docker image file size.
- `DockerPermissionStrategy.CopyChown`: calls `COPY --chown` in the Dockerfile. Provided as a backward compatibility.

For `MultiStage` and `Run` strategies, `dockerChmodType` is used in addition to call `chmod` during Docker build.

- `DockerChmodType.UserGroupReadExecute` (default): chmod -R u=rX,g=rX
- `DockerChmodType.UserGroupWriteExecute`: chmod -R u=rwX,g=rwX
- `DockerChmodType.SyncGroupToUser`: chmod -R g=u
- `DockerChmodType.Custom`: Custom argument provided by the user.

Some application will require writing files to the working directory.
In that case the setting should be changed as follows:

```scala
import com.typesafe.sbt.packager.docker.DockerChmodType

dockerChmodType := DockerChmodType.UserGroupWriteExecute
```

During `docker:stage`, Docker package validation is called to check if the selected strategy is compatible with the deteted Docker version.
This fixes the current repeatability issue reported as sbt#1187. If the incompatibility is detected, the user is advised to
either upgrade their Docker, pick another strategy, or override the `dockerVersion` setting.

`daemonGroup` is set to `root` instead of copying the value from the `daemonUser` setting.
This matches the semantics of `USER` as well as OpenShift, which uses gid=0.
muuki88 pushed a commit that referenced this issue Jan 24, 2019
* Validate Docker packaging

* implement dockerPermissionStrategy

Fixes #1189

This implements a non-root Docker container that's safer by default and compatible with Red Hat OpenShift.
Current `ADD --chown=daemon:daemon opt /opt` nominally implements non-root image,
but by giving ownership of the working directory to the `daemon` user, it reduces the safety.
Instead we should use `chmod` to default to read-only access unless the build user opts into writable working directory.

The challenge is calling `chmod` without incurring the fs layer overhead (#883).
[Multi-stage builds](https://docs.docker.com/develop/develop-images/multistage-build/) can be used to pre-stage
the files with desired file permissions.

This adds new `dockerPermissionStrategy` setting which decides how file permissions are set for the working directory inside the Docker image generated by sbt-native-packager. The strategies are:

- `DockerPermissionStrategy.MultiStage` (default): uses multi-stage Docker build to call chmod ahead of time.
- `DockerPermissionStrategy.None`: does not attempt to change the file permissions, and use the host machine's file mode bits.
- `DockerPermissionStrategy.Run`: calls `RUN` in the Dockerfile. This has regression on the resulting Docker image file size.
- `DockerPermissionStrategy.CopyChown`: calls `COPY --chown` in the Dockerfile. Provided as a backward compatibility.

For `MultiStage` and `Run` strategies, `dockerChmodType` is used in addition to call `chmod` during Docker build.

- `DockerChmodType.UserGroupReadExecute` (default): chmod -R u=rX,g=rX
- `DockerChmodType.UserGroupWriteExecute`: chmod -R u=rwX,g=rwX
- `DockerChmodType.SyncGroupToUser`: chmod -R g=u
- `DockerChmodType.Custom`: Custom argument provided by the user.

Some application will require writing files to the working directory.
In that case the setting should be changed as follows:

```scala
import com.typesafe.sbt.packager.docker.DockerChmodType

dockerChmodType := DockerChmodType.UserGroupWriteExecute
```

During `docker:stage`, Docker package validation is called to check if the selected strategy is compatible with the deteted Docker version.
This fixes the current repeatability issue reported as #1187. If the incompatibility is detected, the user is advised to
either upgrade their Docker, pick another strategy, or override the `dockerVersion` setting.

`daemonGroup` is set to `root` instead of copying the value from the `daemonUser` setting.
This matches the semantics of `USER` as well as OpenShift, which uses gid=0.

* improve the names in file-permission scritped test

* add comment on globalSettings
@muuki88
Copy link
Contributor

muuki88 commented Feb 12, 2019

This should be fixed in 1.3.18 as we can configure the various permissions and docker build strategies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants