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.

Sign up for GitHub

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 ability to use public GCR repos without being authenticated #1140

Merged
merged 7 commits into from
Mar 24, 2020
Merged

Add ability to use public GCR repos without being authenticated #1140

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0cfc5c6
Remove importing gcr images into docker registry
samos123 Mar 14, 2020
8a02001
Add ability to use public GCR image
samos123 Mar 16, 2020
6626869
Add comments
samos123 Mar 17, 2020
8518b37
Add unit tests for CheckPushPermissions
samos123 Mar 21, 2020
c56f16b
Create constant for Docker conf location
samos123 Mar 21, 2020
c2393df
Cleanup push_test CheckPushPermission unit tests
samos123 Mar 23, 2020
1fb1edd
Test: Remove DockerConf after CheckPushPermission
samos123 Mar 23, 2020
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
2 changes: 0 additions & 2 deletions deploy/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ WORKDIR /go/src/github.com/GoogleContainerTools/kaniko
# Get GCR credential helper
ADD https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/v1.5.0/docker-credential-gcr_linux_amd64-1.5.0.tar.gz /usr/local/bin/
RUN tar -C /usr/local/bin/ -xvzf /usr/local/bin/docker-credential-gcr_linux_amd64-1.5.0.tar.gz
RUN docker-credential-gcr configure-docker
# Get Amazon ECR credential helper
RUN go get -u github.com/awslabs/amazon-ecr-credential-helper/ecr-login/cli/docker-credential-ecr-login
RUN make -C /go/src/github.com/awslabs/amazon-ecr-credential-helper linux-amd64
Expand All @@ -37,7 +36,6 @@ COPY --from=0 /usr/local/bin/docker-credential-gcr /kaniko/docker-credential-gcr
COPY --from=0 /go/src/github.com/awslabs/amazon-ecr-credential-helper/bin/linux-amd64/docker-credential-ecr-login /kaniko/docker-credential-ecr-login
COPY --from=0 /usr/local/bin/docker-credential-acr-linux /kaniko/docker-credential-acr-linux
COPY files/ca-certificates.crt /kaniko/ssl/certs/
COPY --from=0 /root/.docker/config.json /kaniko/.docker/config.json
ENV HOME /root
ENV USER /root
ENV PATH /usr/local/bin:/kaniko
Expand Down
2 changes: 0 additions & 2 deletions deploy/Dockerfile_debug
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ WORKDIR /go/src/github.com/GoogleContainerTools/kaniko
# Get GCR credential helper
ADD https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/v1.5.0/docker-credential-gcr_linux_amd64-1.5.0.tar.gz /usr/local/bin/
RUN tar -C /usr/local/bin/ -xvzf /usr/local/bin/docker-credential-gcr_linux_amd64-1.5.0.tar.gz
RUN docker-credential-gcr configure-docker
# Get Amazon ECR credential helper
RUN go get -u github.com/awslabs/amazon-ecr-credential-helper/ecr-login/cli/docker-credential-ecr-login
RUN make -C /go/src/github.com/awslabs/amazon-ecr-credential-helper linux-amd64
Expand All @@ -43,7 +42,6 @@ COPY --from=1 /distroless/bazel-bin/experimental/busybox/busybox/ /busybox/
# Declare /busybox as a volume to get it automatically whitelisted
VOLUME /busybox
COPY files/ca-certificates.crt /kaniko/ssl/certs/
COPY --from=0 /root/.docker/config.json /kaniko/.docker/config.json
ENV HOME /root
ENV USER /root
ENV PATH /usr/local/bin:/kaniko:/busybox
Expand Down
2 changes: 0 additions & 2 deletions deploy/Dockerfile_warmer
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ WORKDIR /go/src/github.com/GoogleContainerTools/kaniko
# Get GCR credential helper
ADD https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/v1.5.0/docker-credential-gcr_linux_amd64-1.5.0.tar.gz /usr/local/bin/
RUN tar -C /usr/local/bin/ -xvzf /usr/local/bin/docker-credential-gcr_linux_amd64-1.5.0.tar.gz
RUN docker-credential-gcr configure-docker
# Get Amazon ECR credential helper
RUN go get -u github.com/awslabs/amazon-ecr-credential-helper/ecr-login/cli/docker-credential-ecr-login
RUN make -C /go/src/github.com/awslabs/amazon-ecr-credential-helper linux-amd64
Expand All @@ -33,7 +32,6 @@ COPY --from=0 /go/src/github.com/GoogleContainerTools/kaniko/out/warmer /kaniko/
COPY --from=0 /usr/local/bin/docker-credential-gcr /kaniko/docker-credential-gcr
COPY --from=0 /go/src/github.com/awslabs/amazon-ecr-credential-helper/bin/linux-amd64/docker-credential-ecr-login /kaniko/docker-credential-ecr-login
COPY files/ca-certificates.crt /kaniko/ssl/certs/
COPY --from=0 /root/.docker/config.json /kaniko/.docker/config.json
ENV HOME /root
ENV USER /root
ENV PATH /usr/local/bin:/kaniko
Expand Down
9 changes: 0 additions & 9 deletions integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,6 @@ func launchTests(m *testing.M) (int, error) {

RunOnInterrupt(func() { DeleteFromBucket(fileInBucket) })
defer DeleteFromBucket(fileInBucket)
} else {
var err error
var migratedFiles []string
if migratedFiles, err = MigrateGCRRegistry(dockerfilesPath, allDockerfiles, config.imageRepo); err != nil {
RollbackMigratedFiles(dockerfilesPath, migratedFiles)
return 1, errors.Wrap(err, "Fail to migrate dockerfiles from gcs")
}
RunOnInterrupt(func() { RollbackMigratedFiles(dockerfilesPath, migratedFiles) })
defer RollbackMigratedFiles(dockerfilesPath, migratedFiles)
}
if err := buildRequiredImages(); err != nil {
return 1, errors.Wrap(err, "Error while building images")
Expand Down
155 changes: 0 additions & 155 deletions integration/migrate_gcr.go
View file
Open in desktop

This file was deleted.

25 changes: 22 additions & 3 deletions pkg/executor/push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"io/ioutil"
"net/http"
"os"
"os/exec"
"path/filepath"
"strings"
"time"
Expand Down Expand Up @@ -52,6 +53,7 @@ type withUserAgent struct {

const (
UpstreamClientUaKey = "UPSTREAM_CLIENT_TYPE"
DockerConfLocation = "/kaniko/.docker/config.json"
)

func (w *withUserAgent) RoundTrip(r *http.Request) (*http.Response, error) {
Expand Down Expand Up @@ -98,6 +100,13 @@ var defaultX509Handler systemCertLoader = func() CertPool {
}
}

// for testing
var (
fs = afero.NewOsFs()
execCommand = exec.Command
checkRemotePushPermission = remote.CheckPushPermission
)

// CheckPushPermissions checks that the configured credentials can be used to
// push to every specified destination.
func CheckPushPermissions(opts *config.KanikoOptions) error {
Expand All @@ -115,6 +124,18 @@ func CheckPushPermissions(opts *config.KanikoOptions) error {
continue
}

// Historically kaniko was pre-configured by default with gcr credential helper,
// in here we keep the backwards compatibility by enabling the GCR helper only
// when gcr.io is in one of the destinations.
if strings.Contains(destRef.RegistryStr(), "gcr.io") {
samos123 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@tejal29 tejal29 Mar 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Can you please test to push to a public GCR repo one needs to be authenticated? My gut is mostly yes in which case this is good. If not, then we might need to add this is documentation.

It would be great if you can add tests for this function.

  1. You can declare 2 variables
// for testing
var (
execute = execCommad
checkRemotePushPermissions = remote.CheckPushPermission
stat = os.Stat
)

In your tests, you could create mock for these and make sure,

  1. when mockOsStat return os.ErrNotExist, execute is called
  2. when mockOsStat return nil and some random fileinfo, execute is not called.

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 added the testing. I noticed we were already using afero in push.go and push_test.go so there wasn't a need to mock os.Stat. Thank you so much for detailed recommendations on how to test, really helpful.

There is no way to make a GCR repo pushable by the public. You can only push if you're explicitly given permission to push to a GCR repo. For reference: https://cloud.google.com/container-registry/docs/access-control or did you mean something else?

// Checking for existence of docker.config as it's normally required for
// authenticated registries and prevent overwriting user provided docker conf
if _, err := fs.Stat(DockerConfLocation); os.IsNotExist(err) {
if err := execCommand("docker-credential-gcr", "configure-docker").Run(); err != nil {
return errors.Wrap(err, "error while configuring docker-credential-gcr helper")
}
}
}
registryName := destRef.Repository.Registry.Name()
if opts.Insecure || opts.InsecureRegistries.Contains(registryName) {
newReg, err := name.NewRegistry(registryName, name.WeakValidation, name.Insecure)
Expand All @@ -124,7 +145,7 @@ func CheckPushPermissions(opts *config.KanikoOptions) error {
destRef.Repository.Registry = newReg
}
tr := makeTransport(opts, registryName, defaultX509Handler)
if err := remote.CheckPushPermission(destRef, creds.GetKeychain(), tr); err != nil {
if err := checkRemotePushPermission(destRef, creds.GetKeychain(), tr); err != nil {
return errors.Wrapf(err, "checking push permission for %q", destRef)
}
checked[destRef.Context().RepositoryStr()] = true
Expand Down Expand Up @@ -231,8 +252,6 @@ func DoPush(image v1.Image, opts *config.KanikoOptions) error {
return writeImageOutputs(image, destRefs)
}

var fs = afero.NewOsFs()

func writeImageOutputs(image v1.Image, destRefs []name.Tag) error {
dir := os.Getenv("BUILDER_OUTPUT")
if dir == "" {
Expand Down
66 changes: 66 additions & 0 deletions pkg/executor/push_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ import (
"io/ioutil"
"net/http"
"os"
"os/exec"
"path/filepath"
"testing"

"github.com/GoogleContainerTools/kaniko/pkg/config"
"github.com/GoogleContainerTools/kaniko/testutil"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
"github.com/google/go-containerregistry/pkg/v1/layout"
"github.com/google/go-containerregistry/pkg/v1/random"
Expand Down Expand Up @@ -307,3 +309,67 @@ func Test_makeTransport(t *testing.T) {
})
}
}

var calledExecCommand = false
var calledCheckPushPermission = false

func setCalledFalse() {
calledExecCommand = false
calledCheckPushPermission = false
}

func fakeExecCommand(command string, args ...string) *exec.Cmd {
calledExecCommand = true
cs := []string{"-test.run=TestHelperProcess", "--", command}
cs = append(cs, args...)
cmd := exec.Command(os.Args[0], cs...)
cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"}
return cmd
}

func fakeCheckPushPermission(ref name.Reference, kc authn.Keychain, t http.RoundTripper) error {
calledCheckPushPermission = true
return nil
}

func TestCheckPushPermissions(t *testing.T) {
tests := []struct {
Destination string
ShouldCallExecCommand bool
ExistingConfig bool
}{
{"gcr.io/test-image", true, false},
{"gcr.io/test-image", false, true},
{"localhost:5000/test-image", false, false},
{"localhost:5000/test-image", false, true},
}

execCommand = fakeExecCommand
checkRemotePushPermission = fakeCheckPushPermission
for _, test := range tests {
testName := fmt.Sprintf("%s_ExistingDockerConf_%v", test.Destination, test.ExistingConfig)
t.Run(testName, func(t *testing.T) {
fs = afero.NewMemMapFs()
opts := config.KanikoOptions{
Destinations: []string{test.Destination},
}
if test.ExistingConfig {
afero.WriteFile(fs, DockerConfLocation, []byte(""), os.FileMode(0644))
samos123 marked this conversation as resolved.
Show resolved Hide resolved
}
CheckPushPermissions(&opts)
if test.ShouldCallExecCommand != calledExecCommand {
t.Errorf("Expected calledExecCommand to be %v however it was %v",
calledExecCommand, test.ShouldCallExecCommand)
}
setCalledFalse()
})
}
}

func TestHelperProcess(t *testing.T) {
if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
return
}
fmt.Fprintf(os.Stdout, "fake result")
os.Exit(0)
}