From 9103215bf3c9731c271ba71452a94cde7b438fac Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Wed, 5 Aug 2020 15:08:40 +0200 Subject: [PATCH 1/3] Don't return error if we can't parse the creds We just continue on to the next credentials --- aci/convert/registrycredentials.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aci/convert/registrycredentials.go b/aci/convert/registrycredentials.go index 5b666e93b..0e12147a9 100644 --- a/aci/convert/registrycredentials.go +++ b/aci/convert/registrycredentials.go @@ -73,8 +73,10 @@ func getRegistryCredentials(project compose.Project, registryLoader registryConf var registryCreds []containerinstance.ImageRegistryCredential for name, oneCred := range allCreds { parsedURL, err := url.Parse(name) + // Credentials can contain some garbage, we don't return the error here + // because we don't care about these garbage creds. if err != nil { - return nil, err + continue } hostname := parsedURL.Host From 280066a5f434071e4daf4c4b56101904139224b2 Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Wed, 5 Aug 2020 15:09:33 +0200 Subject: [PATCH 2/3] Move credential tests to gotest.tools --- aci/convert/registrycredentials_test.go | 127 ++++++++++++------------ 1 file changed, 61 insertions(+), 66 deletions(-) diff --git a/aci/convert/registrycredentials_test.go b/aci/convert/registrycredentials_test.go index 5b975bc24..225129827 100644 --- a/aci/convert/registrycredentials_test.go +++ b/aci/convert/registrycredentials_test.go @@ -20,129 +20,126 @@ import ( "strconv" "testing" + "github.com/Azure/azure-sdk-for-go/profiles/latest/containerinstance/mgmt/containerinstance" "github.com/Azure/go-autorest/autorest/to" "github.com/compose-spec/compose-go/types" cliconfigtypes "github.com/docker/cli/cli/config/types" - - "github.com/Azure/azure-sdk-for-go/profiles/latest/containerinstance/mgmt/containerinstance" - - . "github.com/onsi/gomega" "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/suite" + "gotest.tools/v3/assert" + "gotest.tools/v3/assert/cmp" ) const getAllCredentials = "getAllRegistryCredentials" -type RegistryConvertTestSuite struct { - suite.Suite - loader *MockRegistryLoader -} +func TestHubPrivateImage(t *testing.T) { + loader := &MockRegistryLoader{} + loader.On(getAllCredentials).Return(registry("https://index.docker.io", userPwdCreds("toto", "pwd")), nil) -func (suite *RegistryConvertTestSuite) BeforeTest(suiteName, testName string) { - suite.loader = &MockRegistryLoader{} -} - -func (suite *RegistryConvertTestSuite) TestHubPrivateImage() { - suite.loader.On(getAllCredentials).Return(registry("https://index.docker.io", userPwdCreds("toto", "pwd")), nil) - - creds, err := getRegistryCredentials(composeServices("gtardif/privateimg"), suite.loader) - Expect(err).To(BeNil()) - Expect(creds).To(Equal([]containerinstance.ImageRegistryCredential{ + creds, err := getRegistryCredentials(composeServices("gtardif/privateimg"), loader) + assert.NilError(t, err) + assert.DeepEqual(t, creds, []containerinstance.ImageRegistryCredential{ { Server: to.StringPtr(dockerHub), Username: to.StringPtr("toto"), Password: to.StringPtr("pwd"), }, - })) + }) } -func (suite *RegistryConvertTestSuite) TestRegistryNameWithoutProtocol() { - suite.loader.On(getAllCredentials).Return(registry("index.docker.io", userPwdCreds("toto", "pwd")), nil) +func TestRegistryNameWithoutProtocol(t *testing.T) { + loader := &MockRegistryLoader{} + loader.On(getAllCredentials).Return(registry("index.docker.io", userPwdCreds("toto", "pwd")), nil) - creds, err := getRegistryCredentials(composeServices("gtardif/privateimg"), suite.loader) - Expect(err).To(BeNil()) - Expect(creds).To(Equal([]containerinstance.ImageRegistryCredential{ + creds, err := getRegistryCredentials(composeServices("gtardif/privateimg"), loader) + assert.NilError(t, err) + assert.DeepEqual(t, creds, []containerinstance.ImageRegistryCredential{ { Server: to.StringPtr(dockerHub), Username: to.StringPtr("toto"), Password: to.StringPtr("pwd"), }, - })) + }) } -func (suite *RegistryConvertTestSuite) TestImageWithDotInName() { - suite.loader.On(getAllCredentials).Return(registry("index.docker.io", userPwdCreds("toto", "pwd")), nil) +func TestImageWithDotInName(t *testing.T) { + loader := &MockRegistryLoader{} + loader.On(getAllCredentials).Return(registry("index.docker.io", userPwdCreds("toto", "pwd")), nil) - creds, err := getRegistryCredentials(composeServices("my.image"), suite.loader) - Expect(err).To(BeNil()) - Expect(creds).To(Equal([]containerinstance.ImageRegistryCredential{ + creds, err := getRegistryCredentials(composeServices("my.image"), loader) + assert.NilError(t, err) + assert.DeepEqual(t, creds, []containerinstance.ImageRegistryCredential{ { Server: to.StringPtr(dockerHub), Username: to.StringPtr("toto"), Password: to.StringPtr("pwd"), }, - })) + }) } -func (suite *RegistryConvertTestSuite) TestAcrPrivateImage() { - suite.loader.On(getAllCredentials).Return(registry("https://mycontainerregistrygta.azurecr.io", tokenCreds("123456")), nil) +func TestAcrPrivateImage(t *testing.T) { + loader := &MockRegistryLoader{} + loader.On(getAllCredentials).Return(registry("https://mycontainerregistrygta.azurecr.io", tokenCreds("123456")), nil) - creds, err := getRegistryCredentials(composeServices("mycontainerregistrygta.azurecr.io/privateimg"), suite.loader) - Expect(err).To(BeNil()) - Expect(creds).To(Equal([]containerinstance.ImageRegistryCredential{ + creds, err := getRegistryCredentials(composeServices("mycontainerregistrygta.azurecr.io/privateimg"), loader) + assert.NilError(t, err) + assert.DeepEqual(t, creds, []containerinstance.ImageRegistryCredential{ { Server: to.StringPtr("mycontainerregistrygta.azurecr.io"), Username: to.StringPtr(tokenUsername), Password: to.StringPtr("123456"), }, - })) + }) } -func (suite *RegistryConvertTestSuite) TestAcrPrivateImageLinux() { +func TestAcrPrivateImageLinux(t *testing.T) { + loader := &MockRegistryLoader{} token := tokenCreds("123456") token.Username = tokenUsername - suite.loader.On(getAllCredentials).Return(registry("https://mycontainerregistrygta.azurecr.io", token), nil) + loader.On(getAllCredentials).Return(registry("https://mycontainerregistrygta.azurecr.io", token), nil) - creds, err := getRegistryCredentials(composeServices("mycontainerregistrygta.azurecr.io/privateimg"), suite.loader) - Expect(err).To(BeNil()) - Expect(creds).To(Equal([]containerinstance.ImageRegistryCredential{ + creds, err := getRegistryCredentials(composeServices("mycontainerregistrygta.azurecr.io/privateimg"), loader) + assert.NilError(t, err) + assert.DeepEqual(t, creds, []containerinstance.ImageRegistryCredential{ { Server: to.StringPtr("mycontainerregistrygta.azurecr.io"), Username: to.StringPtr(tokenUsername), Password: to.StringPtr("123456"), }, - })) + }) } -func (suite *RegistryConvertTestSuite) TestNoMoreRegistriesThanImages() { +func TestNoMoreRegistriesThanImages(t *testing.T) { + loader := &MockRegistryLoader{} configs := map[string]cliconfigtypes.AuthConfig{ "https://mycontainerregistrygta.azurecr.io": tokenCreds("123456"), "https://index.docker.io": userPwdCreds("toto", "pwd"), } - suite.loader.On(getAllCredentials).Return(configs, nil) + loader.On(getAllCredentials).Return(configs, nil) - creds, err := getRegistryCredentials(composeServices("mycontainerregistrygta.azurecr.io/privateimg"), suite.loader) - Expect(err).To(BeNil()) - Expect(creds).To(Equal([]containerinstance.ImageRegistryCredential{ + creds, err := getRegistryCredentials(composeServices("mycontainerregistrygta.azurecr.io/privateimg"), loader) + assert.NilError(t, err) + assert.DeepEqual(t, creds, []containerinstance.ImageRegistryCredential{ { Server: to.StringPtr("mycontainerregistrygta.azurecr.io"), Username: to.StringPtr(tokenUsername), Password: to.StringPtr("123456"), }, - })) + }) - creds, err = getRegistryCredentials(composeServices("someuser/privateimg"), suite.loader) - Expect(err).To(BeNil()) - Expect(creds).To(Equal([]containerinstance.ImageRegistryCredential{ + creds, err = getRegistryCredentials(composeServices("someuser/privateimg"), loader) + assert.NilError(t, err) + assert.DeepEqual(t, creds, []containerinstance.ImageRegistryCredential{ { Server: to.StringPtr(dockerHub), Username: to.StringPtr("toto"), Password: to.StringPtr("pwd"), }, - })) + }) + } -func (suite *RegistryConvertTestSuite) TestHubAndSeveralACRRegistries() { +func TestHubAndSeveralACRRegistries(t *testing.T) { + loader := &MockRegistryLoader{} configs := map[string]cliconfigtypes.AuthConfig{ "https://mycontainerregistry1.azurecr.io": tokenCreds("123456"), "https://mycontainerregistry2.azurecr.io": tokenCreds("456789"), @@ -150,21 +147,24 @@ func (suite *RegistryConvertTestSuite) TestHubAndSeveralACRRegistries() { "https://index.docker.io": userPwdCreds("toto", "pwd"), "https://other.registry.io": userPwdCreds("user", "password"), } - suite.loader.On(getAllCredentials).Return(configs, nil) + loader.On(getAllCredentials).Return(configs, nil) - creds, err := getRegistryCredentials(composeServices("mycontainerregistry1.azurecr.io/privateimg", "someuser/privateImg2", "mycontainerregistry2.azurecr.io/privateimg"), suite.loader) - Expect(err).To(BeNil()) - Expect(creds).To(ContainElement(containerinstance.ImageRegistryCredential{ + creds, err := getRegistryCredentials(composeServices("mycontainerregistry1.azurecr.io/privateimg", "someuser/privateImg2", "mycontainerregistry2.azurecr.io/privateimg"), loader) + assert.NilError(t, err) + + assert.Assert(t, cmp.Contains(creds, containerinstance.ImageRegistryCredential{ Server: to.StringPtr("mycontainerregistry1.azurecr.io"), Username: to.StringPtr(tokenUsername), Password: to.StringPtr("123456"), })) - Expect(creds).To(ContainElement(containerinstance.ImageRegistryCredential{ + + assert.Assert(t, cmp.Contains(creds, containerinstance.ImageRegistryCredential{ Server: to.StringPtr("mycontainerregistry2.azurecr.io"), Username: to.StringPtr(tokenUsername), Password: to.StringPtr("456789"), })) - Expect(creds).To(ContainElement(containerinstance.ImageRegistryCredential{ + + assert.Assert(t, cmp.Contains(creds, containerinstance.ImageRegistryCredential{ Server: to.StringPtr(dockerHub), Username: to.StringPtr("toto"), Password: to.StringPtr("pwd"), @@ -204,11 +204,6 @@ func tokenCreds(token string) cliconfigtypes.AuthConfig { } } -func TestRegistryConvertTestSuite(t *testing.T) { - RegisterTestingT(t) - suite.Run(t, new(RegistryConvertTestSuite)) -} - type MockRegistryLoader struct { mock.Mock } From ee4916cbf550d50aede71a3854a67dd1c90ac206 Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Wed, 5 Aug 2020 15:22:13 +0200 Subject: [PATCH 3/3] Add test for invalid registry name --- aci/convert/registrycredentials_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/aci/convert/registrycredentials_test.go b/aci/convert/registrycredentials_test.go index 225129827..ee3eaf839 100644 --- a/aci/convert/registrycredentials_test.go +++ b/aci/convert/registrycredentials_test.go @@ -61,6 +61,15 @@ func TestRegistryNameWithoutProtocol(t *testing.T) { }) } +func TestInvalidCredentials(t *testing.T) { + loader := &MockRegistryLoader{} + loader.On(getAllCredentials).Return(registry("18.195.159.6:444", userPwdCreds("toto", "pwd")), nil) + + creds, err := getRegistryCredentials(composeServices("gtardif/privateimg"), loader) + assert.NilError(t, err) + assert.Equal(t, len(creds), 0) +} + func TestImageWithDotInName(t *testing.T) { loader := &MockRegistryLoader{} loader.On(getAllCredentials).Return(registry("index.docker.io", userPwdCreds("toto", "pwd")), nil)