Skip to content

Commit

Permalink
Don't pass --platform to docker/podman
Browse files Browse the repository at this point in the history
The `--platform` flag is only ever passed to docker/podman when both the
user and server platform values are equal. When they mismatch, the
`--platform` flag is ommitted, and the server's native platform is used.

Since the `--platform` value is only ever passed when the user and
server platforms are equal, it shouldn't matter; however in practice
there is a podman bug, which causes a pull to occur whenver the
`--platform` flag is specified: containers/podman#15711

This bug will cause podman to always pull the earthly/buildkitd image from
docker hub, which will either 1) overwrite the local image if the image exists in docker hub,
and ultimately will cause our tests to run against an incorrect image
version, or 2) result in the following 404-error if the tag does not
exist:

    exit status 125: Trying to pull docker.io/earthly/buildkitd:dev-HEAD...
    Error: initializing source docker://earthly/buildkitd:dev-HEAD: reading manifest dev-HEAD in docker.io/earthly/buildkitd: manifest unknown: manifest unknown: exit status 125

This commit additionally updates the podman tests to use capsh rather
than grep for the `RUN --privileged` tests, as the Effective capabilities (CapEff)
bits are not always the same between docker and podman

Signed-off-by: Alex Couture-Beil <alex@earthly.dev>
  • Loading branch information
alexcb committed Dec 29, 2022
1 parent f7db424 commit bf316ab
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 55 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/reusable-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ jobs:
if: inputs.BINARY == 'docker'
- name: remove Docker
run: ${{inputs.SUDO}} apt-get purge docker-engine docker docker.io docker-ce docker-ce-cli ; ${{inputs.SUDO}} rm -rf /usr/bin/docker
if: inputs.binary == 'podman'
if: inputs.binary != 'docker'
- name: Install Podman (with apt-get)
run: ${{inputs.SUDO}} apt-get update && ${{inputs.SUDO}} apt-get install -y podman
if: inputs.binary == 'podman'
Expand Down
27 changes: 26 additions & 1 deletion ast/tests/privileged.ast.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@
],
"name": "FROM"
}
},
{
"command": {
"args": [
"apk",
"add",
"libcap"
],
"name": "RUN"
}
}
],
"targets": [
Expand Down Expand Up @@ -38,8 +48,23 @@
"grep",
"CapEff",
"|",
"awk",
"'{print",
"$2}'",
">",
"privileged-cap"
],
"name": "RUN"
}
},
{
"command": {
"args": [
"capsh",
"--decode=\"$(cat privileged-cap)\"",
"|",
"grep",
"0000003fffffffff"
"cap_sys_admin"
],
"name": "RUN"
}
Expand Down
5 changes: 3 additions & 2 deletions tests/allow-privileged-import.earth
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
VERSION 0.6
FROM alpine:3.15
RUN apk add libcap # for capsh

IMPORT --allow-privileged github.com/earthly/test-remote/privileged:main
IMPORT ./a/really/deep/subdir

test-remote-import:
COPY privileged+privileged/proc-status .
RUN cat proc-status | grep CapEff | grep 0000003fffffffff
RUN capsh --decode="$(cat proc-status | grep CapEff | awk '{print $2}')" | grep cap_sys_admin

test-relative-import:
COPY subdir+subdirprivileged/proc-status .
RUN cat proc-status | grep CapEff | grep 0000003fffffffff
RUN capsh --decode="$(cat proc-status | grep CapEff | awk '{print $2}')" | grep cap_sys_admin

test-remote-cmd:
DO privileged+PRIV
Expand Down
4 changes: 3 additions & 1 deletion tests/allow-privileged.earth
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
VERSION 0.6
FROM alpine:3.15
RUN apk add libcap # for capsh

reject-privileged-in-remote-repo-triggered-by-from-locally:
FROM github.com/earthly/test-remote/privileged:main+locally
Expand Down Expand Up @@ -55,7 +56,8 @@ allow-privileged-in-remote-repo-triggered-by-from-privileged:

allow-privileged-in-remote-repo-triggered-by-copy-privileged:
COPY --allow-privileged github.com/earthly/test-remote/privileged:main+privileged/proc-status .
RUN cat proc-status | grep CapEff | grep 0000003fffffffff
# checking for 0000003fffffffff might fail when running under podman, check the cap_sys_admin bit is set instead
RUN capsh --decode="$(cat proc-status | grep CapEff | awk '{print $2}')" | grep cap_sys_admin

allow-privileged-in-remote-repo-triggered-by-build-privileged:
BUILD --allow-privileged github.com/earthly/test-remote/privileged:main+privileged
Expand Down
8 changes: 7 additions & 1 deletion tests/privileged.earth
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
VERSION 0.6
FROM alpine:3.15
RUN apk add libcap # for capsh

test:
RUN cat /proc/self/status | grep CapEff | grep 00000000a80425fb
RUN --privileged cat /proc/self/status | grep CapEff | grep 0000003fffffffff

# when running under podman CapEff is not always 0000003fffffffff; but might instead be 000001ffffffffff
# use the capsh tool to decode this number and check the sys admin capability is permitted
RUN --privileged cat /proc/self/status | grep CapEff | awk '{print $2}' > privileged-cap
RUN capsh --decode="$(cat privileged-cap)" | grep cap_sys_admin
3 changes: 2 additions & 1 deletion tests/true-false-flag.earth
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ VERSION 0.6
hello:
FROM alpine:3.15
ARG PRIVILEGED=false
RUN --no-cache --privileged=$PRIVILEGED if cat /proc/self/status | grep CapEff | grep 0000003fffffffff >/dev/null; then echo "I have the power"; else echo "fight the power"; fi
# Note the 0000000000f00000 bit means cap_sys_pacct,cap_sys_admin,cap_sys_boot,cap_sys_nice are enabled
RUN --no-cache --privileged=$PRIVILEGED if cat /proc/self/status | grep CapEff | awk '{print $2}' | grep -w ..........f..... >/dev/null; then echo "I have the power"; else echo "fight the power"; fi

all:
BUILD +hello --PRIVILEGED=false
Expand Down
9 changes: 0 additions & 9 deletions util/containerutil/frontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"io"
"runtime"

"github.com/hashicorp/go-multierror"
"github.com/pkg/errors"
Expand Down Expand Up @@ -101,11 +100,3 @@ func frontendIfAvailable(ctx context.Context, feType string, cfg *FrontendConfig

return fe, nil
}

func getPlatform() string {
arch := runtime.GOARCH
if runtime.GOARCH == "arm" {
arch = "arm/v7"
}
return fmt.Sprintf("linux/%s", arch)
}
40 changes: 1 addition & 39 deletions util/containerutil/shell_shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"strconv"
"strings"

"github.com/containerd/containerd/platforms"
"github.com/earthly/earthly/conslogging"
"github.com/hashicorp/go-multierror"
_ "github.com/moby/buildkit/client/connhelper/dockercontainer" // Load "docker-container://" helper.
Expand Down Expand Up @@ -191,16 +190,8 @@ func (sf *shellFrontend) ContainerRun(ctx context.Context, containers ...Contain
args = append(args, "--publish", port)
}

platform := getPlatform()
supportsPlatform, platformCheckErr := sf.supportsPlatform(ctx, platform)
if platformCheckErr != nil {
err = multierror.Append(err, platformCheckErr)
}
if supportsPlatform {
args = append(args, "--platform", platform)
}

args = append(args, "-d") // Run detached, this feels implied by the API
args = append(args, "--pull", "missing")
args = append(args, "--name", container.NameOrID)
args = append(args, container.AdditionalArgs...)
args = append(args, sf.runCompatibilityArgs...)
Expand Down Expand Up @@ -302,35 +293,6 @@ func (sf *shellFrontend) commandContextOutput(ctx context.Context, args ...strin
return output, nil
}

func normalizePlatform(platform string) (string, error) {
parsedPlatform, err := platforms.Parse(platform)
if err != nil {
return "", errors.Wrapf(err, "failed to parse platform %s", platform)
}
platformSpec := platforms.Normalize(parsedPlatform)
return platforms.Format(platformSpec), nil
}

func (sf *shellFrontend) supportsPlatform(ctx context.Context, platform string) (bool, error) {
normalizedPlatform, err := normalizePlatform(platform)
if err != nil {
// Failing to normalize the platform means it may not be valid, so return false
sf.Console.VerbosePrintf("failed to normalize platform %s", platform)
return false, nil
}
frontendInfo, err := sf.FrontendInformation(ctx)
if err != nil {
return false, errors.Wrapf(err, "failed to get platform information")
}
normalizedServerPlatform, err := normalizePlatform(frontendInfo.ServerPlatform)
if err != nil {
// Failing to normalize the platform could mean its invalid, so return false
sf.Console.VerbosePrintf("failed to normalize server platform %s", frontendInfo.ServerPlatform)
return false, nil
}
return normalizedServerPlatform == normalizedPlatform, nil
}

func (sf *shellFrontend) setupAndValidateAddresses(feType string, cfg *FrontendConfig) (*FrontendURLs, error) {
calculatedBuildkitHost := cfg.BuildkitHostCLIValue
if cfg.BuildkitHostCLIValue == "" {
Expand Down

0 comments on commit bf316ab

Please sign in to comment.