Base entrypoint image on distroless #2562

imjasonh · 2020-05-06T14:48:30Z

The only reason it was based on busybox was to have access to cp,
which it used to copy its binary to /tekton/tools for use in later
steps.

Instead of relying on cp, this adds a "cp mode" to the entrypoint
binary itself. When invoked with the positional args cp <src> <dst>,
it copies src to dst using Go's os and io packages.

/hold for discussion

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

[y] Includes tests (if functionality changed/added)
[n] Includes docs (if user facing)
[y] Commit messages follow commit message best practices

See the contribution guide for more details.

Release Notes

Entrypoint image is no longer based on busybox, now based on distroless like other images.

tekton-robot · 2020-05-06T14:48:34Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign imjasonh
You can assign the PR to them by writing /assign @imjasonh in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

chmouel · 2020-05-07T09:04:33Z

cmd/entrypoint/main.go

+	}
+	defer s.Close()
+
+	d, err := os.OpenFile(dst, os.O_RDWR|os.O_CREATE, 0777)


is 777 what we want? isnt that 644 the default?

The default for os.Create is 0666. With that, or 0644, subsequent steps fail with exec: \"/tekton/tools/entrypoint\": permission denied": unknown because the other group doesn't have permission to execute.

0667 works, or as little as 0311 (owner can write, anybody can execute), but I didn't really feel strongly about limiting permissions on the executable. I can knock it down to 0311 though.

The only reason it was based on busybox was to have access to `cp`, which it used to copy its binary to /tekton/tools for use in later steps. Instead of relying on `cp`, this adds a "cp mode" to the entrypoint binary itself. When invoked with the positional args `cp <src> <dst>`, it copies src to dst using Go's os and io packages.

mattmoor · 2020-05-07T21:00:04Z

If you were to just create ./cmd/tools/cp with the needed CLI surface, then you could avoid the entrypoint juggling when using this, and I bet the image would be microscopic.

imjasonh · 2020-05-11T15:41:54Z

If you were to just create ./cmd/tools/cp with the needed CLI surface, then you could avoid the entrypoint juggling when using this, and I bet the image would be microscopic.

That would only work if I somehow got the cp binary into the same step as the entrypoint binary, to be able to copy it out to the shared volume. I guess that would be possible with (pseudocode):

initContainers:
# Use `cp` to copy `cp` to /tekton/tools
- image: ko://github.com/tektoncd/pipeline/cmd/cp
  command: ['/ko-app/cp', '/ko-app/cp', '/tekton/tools/cp']
  volumeMounts:
  - name: tekton-tools
    mountPath: /tekton/tools
# Use `cp` to copy `entrypoint` to /tekton/tools
- image: ko://github.com/tektoncd/pipeline/cmd/entrypoint
  command: ['/tekton/tools/cp', '/ko-app/entrypoint', '/tekton/tools/entrypoint']
  volumeMounts:
  - name: tekton-tools
    mountPath: /tekton/tools
...

Is that more straightforward than having a cp-mode for the single entrypoint binary? Maybe? I'm not sure.

mattmoor · 2020-05-11T15:47:32Z

Yeah, sorry wires crossed in my head. I thought this was to enable more direct usage of cp, but this is exactly what I did in warmimage for rewriting entrypoints 😇

imjasonh · 2020-05-11T15:51:35Z

Yeah, sorry wires crossed in my head. I thought this was to enable more direct usage of cp, but this is exactly what I did in warmimage for rewriting entrypoints 😇

For future reference, warm-image's sleeper binary takes a -mode flag that tells it to either sleep ~forever, or copy the binary itself to a destination:

https://github.com/mattmoor/warm-image/blob/8f6fd7efd2ebc47745f0a679e7b327c50722c089/cmd/sleeper/main.go#L31

imjasonh · 2020-05-12T14:22:18Z

Closing in favor of #2606 for now.

imjasonh · 2020-09-26T21:08:40Z

With ko gaining multi-arch support, this change is becoming a bit more relevant, since it makes every TaskRun require a cp binary in the base image, which makes it harder to support multi-arch support in Tekton.

I've rebased this on master and I'd like to see if we think this is still the right way forward.

/reopen

But still:
/hold

tekton-robot · 2020-09-26T21:08:41Z

@imjasonh: Failed to re-open PR: state cannot be changed. The entrypoint-cp branch was force-pushed or recreated.

In response to this:

With ko gaining multi-arch support, this change is becoming a bit more relevant, since it makes every TaskRun require a cp binary in the base image, which makes it harder to support multi-arch support in Tekton.

I've rebased this on master and I'd like to see if we think this is still the right way forward.

/reopen

But still:
/hold

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

imjasonh · 2020-09-26T21:11:21Z

Can't reopen this, created #3286 instead 🤷‍♂️

tekton-robot requested review from dibyom and a user May 6, 2020 14:48

tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 6, 2020

imjasonh force-pushed the entrypoint-cp branch 2 times, most recently from 25b2feb to 7adde75 Compare May 6, 2020 18:03

chmouel reviewed May 7, 2020

View reviewed changes

imjasonh force-pushed the entrypoint-cp branch from 7adde75 to d51fa66 Compare May 7, 2020 13:58

imjasonh closed this May 12, 2020

afrittoli mentioned this pull request May 19, 2020

Run tekton containers as nonroot 🐰 #2606

Merged

3 tasks

imjasonh mentioned this pull request Jun 4, 2020

Create a binary for Affinity Assistant used as a placeholder pod #2640

Closed

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 26, 2020

imjasonh mentioned this pull request Sep 26, 2020

Base entrypoint image on distroless #3286

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Base entrypoint image on distroless #2562

Base entrypoint image on distroless #2562

imjasonh commented May 6, 2020

tekton-robot commented May 6, 2020

chmouel May 7, 2020

imjasonh May 7, 2020

mattmoor commented May 7, 2020

imjasonh commented May 11, 2020 •

edited

Loading

mattmoor commented May 11, 2020

imjasonh commented May 11, 2020

imjasonh commented May 12, 2020

imjasonh commented Sep 26, 2020

tekton-robot commented Sep 26, 2020

imjasonh commented Sep 26, 2020

Base entrypoint image on distroless #2562

Base entrypoint image on distroless #2562

Conversation

imjasonh commented May 6, 2020

Submitter Checklist

Release Notes

tekton-robot commented May 6, 2020

chmouel May 7, 2020

Choose a reason for hiding this comment

imjasonh May 7, 2020

Choose a reason for hiding this comment

mattmoor commented May 7, 2020

imjasonh commented May 11, 2020 • edited Loading

mattmoor commented May 11, 2020

imjasonh commented May 11, 2020

imjasonh commented May 12, 2020

imjasonh commented Sep 26, 2020

tekton-robot commented Sep 26, 2020

imjasonh commented Sep 26, 2020

imjasonh commented May 11, 2020 •

edited

Loading