From 04c678b099c6c1bbd48dd5de34b7e4dd1e37118d Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 19 Aug 2020 13:56:41 +0200 Subject: [PATCH 1/5] Do not require refresh token to obtain ACR token. Especially this will make the ACR auto-login feature work with Service Principal login, that does not have a refresh token Signed-off-by: Guillaume Tardif --- aci/convert/registry_credentials.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/aci/convert/registry_credentials.go b/aci/convert/registry_credentials.go index a5e468aa..5083e892 100644 --- a/aci/convert/registry_credentials.go +++ b/aci/convert/registry_credentials.go @@ -150,10 +150,9 @@ func (c cliRegistryHelper) autoLoginAcr(registry string) error { } data := url.Values{ - "grant_type": {"access_token_refresh_token"}, + "grant_type": {"access_token"}, "service": {registry}, "tenant": {tenantID}, - "refresh_token": {token.RefreshToken}, "access_token": {token.AccessToken}, } repoAuthURL := fmt.Sprintf("https://%s/oauth2/exchange", registry) @@ -162,7 +161,7 @@ func (c cliRegistryHelper) autoLoginAcr(registry string) error { return err } if res.StatusCode != 200 { - return errors.Errorf("error while renewing access token, status : %s", res.Status) + return errors.Errorf("error while accessing ACR token from Azure login, status : %s", res.Status) } bits, err := ioutil.ReadAll(res.Body) if err != nil { From 5cab129c10e01496f0c5bbafe7f6a85965edaae7 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 19 Aug 2020 14:09:38 +0200 Subject: [PATCH 2/5] Add E2E test on deploying ACR images Signed-off-by: Guillaume Tardif --- aci/convert/registry_credentials.go | 8 ++++---- tests/aci-e2e/e2e-aci_test.go | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/aci/convert/registry_credentials.go b/aci/convert/registry_credentials.go index 5083e892..964db78a 100644 --- a/aci/convert/registry_credentials.go +++ b/aci/convert/registry_credentials.go @@ -150,10 +150,10 @@ func (c cliRegistryHelper) autoLoginAcr(registry string) error { } data := url.Values{ - "grant_type": {"access_token"}, - "service": {registry}, - "tenant": {tenantID}, - "access_token": {token.AccessToken}, + "grant_type": {"access_token"}, + "service": {registry}, + "tenant": {tenantID}, + "access_token": {token.AccessToken}, } repoAuthURL := fmt.Sprintf("https://%s/oauth2/exchange", registry) res, err := http.Post(repoAuthURL, "application/x-www-form-urlencoded", strings.NewReader(data.Encode())) diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index 232c5a1f..0a34298c 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -550,6 +550,24 @@ func TestRunEnvVars(t *testing.T) { }) } +func TestDeployACRImage(t *testing.T) { + c := NewParallelE2eCLI(t, binDir) + _, _ = setupTestResourceGroup(t, c, "runAcr") + + t.Run("run", func(t *testing.T) { + cmd := c.NewDockerCmd("run", "-d", "dockerregistrygta.azurecr.io/hello-aci") + res := icmd.RunCmd(cmd) + res.Assert(t, icmd.Success) + out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + container := strings.TrimSpace(out[len(out)-1]) + t.Logf("Container name: %q", container) + waitForStatus(t, c, container, "Terminated") + + res = c.RunDockerCmd("logs", container) + assert.Assert(t, strings.Contains(res.Stdout(), "Hello from Docker!")) + }) +} + func setupTestResourceGroup(t *testing.T, c *E2eCLI, tName string) (string, string) { startTime := strconv.Itoa(int(time.Now().Unix())) rg := "E2E-" + tName + "-" + startTime From a8548de5dd15692a98122bb897c2f45e639a50bf Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 20 Aug 2020 10:55:49 +0200 Subject: [PATCH 3/5] Debug for CI failing tests (green locally) Signed-off-by: Guillaume Tardif --- aci/convert/registry_credentials.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/aci/convert/registry_credentials.go b/aci/convert/registry_credentials.go index 964db78a..ff86cd9e 100644 --- a/aci/convert/registry_credentials.go +++ b/aci/convert/registry_credentials.go @@ -70,7 +70,8 @@ func getRegistryCredentials(project compose.Project, helper registryHelper) ([]c for _, registry := range acrRegistries { err := helper.autoLoginAcr(registry) if err != nil { - fmt.Printf("Could not automatically login to %s from your Azure login. Assuming you already logged in to the ACR registry\n", registry) + return nil, err + //fmt.Printf("Could not automatically login to %s from your Azure login. Assuming you already logged in to the ACR registry\n", registry) } } @@ -79,6 +80,13 @@ func getRegistryCredentials(project compose.Project, helper registryHelper) ([]c return nil, err } var registryCreds []containerinstance.ImageRegistryCredential + b, err := json.MarshalIndent(allCreds, "", " ") + if err != nil { + fmt.Println("ERROR WHILE GETTING ALL CREDS") + fmt.Println(err) + } + fmt.Println("** ALL CREDS " + string(b)) + for name, oneCred := range allCreds { parsedURL, err := url.Parse(name) // Credentials can contain some garbage, we don't return the error here @@ -136,6 +144,7 @@ func getUsedRegistries(project compose.Project) (map[string]bool, []string) { } func (c cliRegistryHelper) autoLoginAcr(registry string) error { + fmt.Println("Fetching ACR login for " + registry) loginService, err := login.NewAzureLoginService() if err != nil { return err @@ -172,6 +181,10 @@ func (c cliRegistryHelper) autoLoginAcr(registry string) error { if err := json.Unmarshal(bits, &newToken); err != nil { return err } + fmt.Println("docker login for " + registry) + fmt.Println(newToken.RefreshToken) cmd := exec.Command("docker", "login", "-p", newToken.RefreshToken, "-u", tokenUsername, registry) - return cmd.Run() + loginResult, err := cmd.CombinedOutput() + fmt.Println("docker login : " + string(loginResult)) + return err } From dc1e31385812fbfda2351003734a78517cc7a2d6 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 21 Aug 2020 11:57:25 +0200 Subject: [PATCH 4/5] Revert "Debug for CI failing tests (green locally)" This reverts commit 1d875674357c7c5a5c26349f62c6f2ef71f74e38. Signed-off-by: Guillaume Tardif --- aci/convert/registry_credentials.go | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/aci/convert/registry_credentials.go b/aci/convert/registry_credentials.go index ff86cd9e..964db78a 100644 --- a/aci/convert/registry_credentials.go +++ b/aci/convert/registry_credentials.go @@ -70,8 +70,7 @@ func getRegistryCredentials(project compose.Project, helper registryHelper) ([]c for _, registry := range acrRegistries { err := helper.autoLoginAcr(registry) if err != nil { - return nil, err - //fmt.Printf("Could not automatically login to %s from your Azure login. Assuming you already logged in to the ACR registry\n", registry) + fmt.Printf("Could not automatically login to %s from your Azure login. Assuming you already logged in to the ACR registry\n", registry) } } @@ -80,13 +79,6 @@ func getRegistryCredentials(project compose.Project, helper registryHelper) ([]c return nil, err } var registryCreds []containerinstance.ImageRegistryCredential - b, err := json.MarshalIndent(allCreds, "", " ") - if err != nil { - fmt.Println("ERROR WHILE GETTING ALL CREDS") - fmt.Println(err) - } - fmt.Println("** ALL CREDS " + string(b)) - for name, oneCred := range allCreds { parsedURL, err := url.Parse(name) // Credentials can contain some garbage, we don't return the error here @@ -144,7 +136,6 @@ func getUsedRegistries(project compose.Project) (map[string]bool, []string) { } func (c cliRegistryHelper) autoLoginAcr(registry string) error { - fmt.Println("Fetching ACR login for " + registry) loginService, err := login.NewAzureLoginService() if err != nil { return err @@ -181,10 +172,6 @@ func (c cliRegistryHelper) autoLoginAcr(registry string) error { if err := json.Unmarshal(bits, &newToken); err != nil { return err } - fmt.Println("docker login for " + registry) - fmt.Println(newToken.RefreshToken) cmd := exec.Command("docker", "login", "-p", newToken.RefreshToken, "-u", tokenUsername, registry) - loginResult, err := cmd.CombinedOutput() - fmt.Println("docker login : " + string(loginResult)) - return err + return cmd.Run() } From 0f4d766f197663ba3132afd8fbc489a0f6dcc6aa Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 21 Aug 2020 12:01:22 +0200 Subject: [PATCH 5/5] Revert "Add E2E test on deploying ACR images" This reverts commit 5cab129c10e01496f0c5bbafe7f6a85965edaae7. Signed-off-by: Guillaume Tardif --- tests/aci-e2e/e2e-aci_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index 0a34298c..232c5a1f 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -550,24 +550,6 @@ func TestRunEnvVars(t *testing.T) { }) } -func TestDeployACRImage(t *testing.T) { - c := NewParallelE2eCLI(t, binDir) - _, _ = setupTestResourceGroup(t, c, "runAcr") - - t.Run("run", func(t *testing.T) { - cmd := c.NewDockerCmd("run", "-d", "dockerregistrygta.azurecr.io/hello-aci") - res := icmd.RunCmd(cmd) - res.Assert(t, icmd.Success) - out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") - container := strings.TrimSpace(out[len(out)-1]) - t.Logf("Container name: %q", container) - waitForStatus(t, c, container, "Terminated") - - res = c.RunDockerCmd("logs", container) - assert.Assert(t, strings.Contains(res.Stdout(), "Hello from Docker!")) - }) -} - func setupTestResourceGroup(t *testing.T, c *E2eCLI, tName string) (string, string) { startTime := strconv.Itoa(int(time.Now().Unix())) rg := "E2E-" + tName + "-" + startTime