From 000711c7f1c2157efde342111391a08e4bfe06c7 Mon Sep 17 00:00:00 2001 From: Aman Singh Date: Wed, 2 Nov 2022 09:48:29 +0530 Subject: [PATCH 1/8] fixed public url in code for acr --- cmd/kaniko-acr/main.go | 103 ++++++++++++++++++++++++++++++--------- pkg/artifact/artifact.go | 2 + 2 files changed, 81 insertions(+), 24 deletions(-) diff --git a/cmd/kaniko-acr/main.go b/cmd/kaniko-acr/main.go index 172bbe6..913ff5e 100644 --- a/cmd/kaniko-acr/main.go +++ b/cmd/kaniko-acr/main.go @@ -9,6 +9,7 @@ import ( "net/http" "net/url" "os" + "strings" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" @@ -139,6 +140,11 @@ func main() { Usage: "Azure Tenant Id", EnvVar: "TENANT_ID", }, + cli.StringFlag{ + Name: "subscription-id", + Usage: "Azure Subscription Id", + EnvVar: "SUBSCRIPTION_ID", + }, cli.StringFlag{ Name: "client-id", Usage: "Azure Client Id", @@ -210,11 +216,12 @@ func run(c *cli.Context) error { registry := c.String("registry") noPush := c.Bool("no-push") - err := createDockerConfig( + publicUrl, err := setupAuth( c.String("tenant-id"), c.String("client-id"), c.String("client-cert"), c.String("client-secret"), + c.String("subscription-id"), registry, noPush, ) @@ -250,7 +257,7 @@ func run(c *cli.Context) error { Artifact: kaniko.Artifact{ Tags: c.StringSlice("tags"), Repo: c.String("repo"), - Registry: c.String("registry"), + Registry: publicUrl, ArtifactFile: c.String("artifact-file"), RegistryType: artifact.Docker, }, @@ -258,46 +265,45 @@ func run(c *cli.Context) error { return plugin.Exec() } -func createDockerConfig(tenantId, clientId, cert, - clientSecret, registry string, noPush bool) error { +func setupAuth(tenantId, clientId, cert, + clientSecret, subscriptionId, registry string, noPush bool) (string, error) { if registry == "" { - return fmt.Errorf("registry must be specified") + return "", fmt.Errorf("registry must be specified") } if noPush { - return nil + return "", nil } // case of client secret or cert based auth if clientId != "" { // only setup auth when pushing or credentials are defined - token, err := getACRToken(tenantId, clientId, clientSecret, cert, registry) + token, publicUrl, err := getACRToken(subscriptionId, tenantId, clientId, clientSecret, cert, registry) if err != nil { - return errors.Wrap(err, "failed to fetch ACR Token") + return "", errors.Wrap(err, "failed to fetch ACR Token") } err = docker.CreateDockerCfgFile(username, token, registry, dockerConfigPath) if err != nil { - return errors.Wrap(err, "failed to create docker config") + return "", errors.Wrap(err, "failed to create docker config") } + return publicUrl, nil } else { - return fmt.Errorf("managed authentication is not supported") + return "", fmt.Errorf("managed authentication is not supported") } - - return nil } -func getACRToken(tenantId, clientId, clientSecret, cert, registry string) (string, error) { +func getACRToken(subscriptionId, tenantId, clientId, clientSecret, cert, registry string) (string, string, error) { if tenantId == "" { - return "", fmt.Errorf("tenantId can't be empty for AAD authentication") + return "", "", fmt.Errorf("tenantId can't be empty for AAD authentication") } if clientId == "" { - return "", fmt.Errorf("clientId can't be empty for AAD authentication") + return "", "", fmt.Errorf("clientId can't be empty for AAD authentication") } if clientSecret == "" && cert == "" { - return "", fmt.Errorf("one of client secret or cert should be defined") + return "", "", fmt.Errorf("one of client secret or cert should be defined") } // in case of authentication via cert @@ -309,21 +315,22 @@ func getACRToken(tenantId, clientId, clientSecret, cert, registry string) (strin } if err := os.Setenv(clientIdEnv, clientId); err != nil { - return "", errors.Wrap(err, "failed to set env variable client Id") + return "", "", errors.Wrap(err, "failed to set env variable client Id") } if err := os.Setenv(clientSecretKeyEnv, clientSecret); err != nil { - return "", errors.Wrap(err, "failed to set env variable client secret") + return "", "", errors.Wrap(err, "failed to set env variable client secret") } if err := os.Setenv(tenantKeyEnv, tenantId); err != nil { - return "", errors.Wrap(err, "failed to set env variable tenant Id") + return "", "", errors.Wrap(err, "failed to set env variable tenant Id") } if err := os.Setenv(certPathEnv, ACRCertPath); err != nil { - return "", errors.Wrap(err, "failed to set env variable cert path") + return "", "", errors.Wrap(err, "failed to set env variable cert path") } env, err := azidentity.NewEnvironmentCredential(nil) if err != nil { - return "", errors.Wrap(err, "failed to get env credentials from azure") + return "", "", errors.Wrap(err, "failed to get env credentials from azure") } + policy := policy.TokenRequestOptions{ Scopes: []string{"https://management.azure.com/.default"}, } @@ -334,14 +341,20 @@ func getACRToken(tenantId, clientId, clientSecret, cert, registry string) (strin azToken, err := env.GetToken(context.Background(), policy) if err != nil { - return "", errors.Wrap(err, "failed to fetch access token") + return "", "", errors.Wrap(err, "failed to fetch access token") + } + + fmt.Fprintf(os.Stderr, "aman %s\n", azToken.Token) + publicUrl, err := getPublicUrl(azToken.Token, registry, subscriptionId) + if err != nil { + return "", "", errors.Wrap(err, "failed to fetch access token") } ACRToken, err := fetchACRToken(tenantId, azToken.Token, registry) if err != nil { - return "", errors.Wrap(err, "failed to fetch ACR token") + return "", "", errors.Wrap(err, "failed to fetch ACR token") } - return ACRToken, nil + return ACRToken, publicUrl, nil } func fetchACRToken(tenantId, token, registry string) (string, error) { @@ -385,3 +398,45 @@ func setupACRCert(cert string) error { } return nil } + +func getPublicUrl(token, registryUrl, subscriptionId string) (string, error) { + finalUrl := "https://portal.azure.com/#view/Microsoft_Azure_ContainerRegistries/TagMetadataBlade/registryId/subscriptions/%s/resourceGroups/%s/providers/Microsoft.ContainerRegistry/" + registry := strings.Split(registryUrl, ".")[0] + fmt.Fprintf(os.Stderr, "aman %s %s %s\n", registry, registryUrl, subscriptionId) + burl := "https://management.azure.com/subscriptions/" + + subscriptionId + "/resources?$filter=resourceType%20eq%20'Microsoft.ContainerRegistry/registries'%20and%20name%20eq%20'" + + registry + "'&api-version=2021-04-01&$select=id" + + method := "GET" + + client := &http.Client{} + req, err := http.NewRequest(method, burl, nil) + + if err != nil { + fmt.Println(err) + return "", errors.Wrap(err, "failed to create request for getting container registry setting") + } + req.Header.Add("Authorization", "Bearer "+token) + res, err := client.Do(req) + if err != nil { + fmt.Println(err) + return "", errors.Wrap(err, "failed to send request for getting container registry setting") + } + defer res.Body.Close() + + fmt.Fprintf(os.Stderr, "aman %d %s %s\n", res.StatusCode, strings.Split(subscriptionId, "b"), burl) + + var response strct + err = json.NewDecoder(res.Body).Decode(&response) + if err != nil { + return "", errors.Wrap(err, "failed to send request for getting container registry setting") + } + resourceGroup := response.Value[0].ID + return fmt.Sprintf(finalUrl, subscriptionId, resourceGroup), nil +} + +type strct struct { + Value []struct { + ID string `json:"id"` + } `json:"value"` +} diff --git a/pkg/artifact/artifact.go b/pkg/artifact/artifact.go index 3e4ecf9..fd166fa 100644 --- a/pkg/artifact/artifact.go +++ b/pkg/artifact/artifact.go @@ -30,6 +30,7 @@ type ( Data struct { RegistryType RegistryTypeEnum `json:"registryType"` RegistryUrl string `json:"registryUrl"` + PublicUrl string `json:"publicUrl"` Images []Image `json:"images"` } DockerArtifact struct { @@ -51,6 +52,7 @@ func WritePluginArtifactFile(registryType RegistryTypeEnum, artifactFilePath, re RegistryUrl: registryUrl, Images: images, } + fmt.Fprintf(os.Stderr, "public Url is %s\n", registryUrl) dockerArtifact := DockerArtifact{ Kind: dockerArtifactV1, From 69e789b294ce0c5fe6519c5319639e576e162918 Mon Sep 17 00:00:00 2001 From: Aman Singh Date: Wed, 2 Nov 2022 12:23:26 +0530 Subject: [PATCH 2/8] removed prints statements --- cmd/kaniko-acr/main.go | 4 ---- pkg/artifact/artifact.go | 1 - 2 files changed, 5 deletions(-) diff --git a/cmd/kaniko-acr/main.go b/cmd/kaniko-acr/main.go index 913ff5e..c48a3b9 100644 --- a/cmd/kaniko-acr/main.go +++ b/cmd/kaniko-acr/main.go @@ -344,7 +344,6 @@ func getACRToken(subscriptionId, tenantId, clientId, clientSecret, cert, registr return "", "", errors.Wrap(err, "failed to fetch access token") } - fmt.Fprintf(os.Stderr, "aman %s\n", azToken.Token) publicUrl, err := getPublicUrl(azToken.Token, registry, subscriptionId) if err != nil { return "", "", errors.Wrap(err, "failed to fetch access token") @@ -402,7 +401,6 @@ func setupACRCert(cert string) error { func getPublicUrl(token, registryUrl, subscriptionId string) (string, error) { finalUrl := "https://portal.azure.com/#view/Microsoft_Azure_ContainerRegistries/TagMetadataBlade/registryId/subscriptions/%s/resourceGroups/%s/providers/Microsoft.ContainerRegistry/" registry := strings.Split(registryUrl, ".")[0] - fmt.Fprintf(os.Stderr, "aman %s %s %s\n", registry, registryUrl, subscriptionId) burl := "https://management.azure.com/subscriptions/" + subscriptionId + "/resources?$filter=resourceType%20eq%20'Microsoft.ContainerRegistry/registries'%20and%20name%20eq%20'" + registry + "'&api-version=2021-04-01&$select=id" @@ -424,8 +422,6 @@ func getPublicUrl(token, registryUrl, subscriptionId string) (string, error) { } defer res.Body.Close() - fmt.Fprintf(os.Stderr, "aman %d %s %s\n", res.StatusCode, strings.Split(subscriptionId, "b"), burl) - var response strct err = json.NewDecoder(res.Body).Decode(&response) if err != nil { diff --git a/pkg/artifact/artifact.go b/pkg/artifact/artifact.go index fd166fa..9f01286 100644 --- a/pkg/artifact/artifact.go +++ b/pkg/artifact/artifact.go @@ -52,7 +52,6 @@ func WritePluginArtifactFile(registryType RegistryTypeEnum, artifactFilePath, re RegistryUrl: registryUrl, Images: images, } - fmt.Fprintf(os.Stderr, "public Url is %s\n", registryUrl) dockerArtifact := DockerArtifact{ Kind: dockerArtifactV1, From 128a2d77c0c22a5b87bff3ee5210afb35035de70 Mon Sep 17 00:00:00 2001 From: Aman Singh Date: Wed, 2 Nov 2022 12:36:19 +0530 Subject: [PATCH 3/8] fixed minor changes --- cmd/kaniko-acr/main.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cmd/kaniko-acr/main.go b/cmd/kaniko-acr/main.go index c48a3b9..2112c7f 100644 --- a/cmd/kaniko-acr/main.go +++ b/cmd/kaniko-acr/main.go @@ -30,6 +30,7 @@ const ( certPathEnv string = "AZURE_CLIENT_CERTIFICATE_PATH" dockerConfigPath string = "/kaniko/.docker" defaultDigestFile string = "/kaniko/digest-file" + finalUrl string = "https://portal.azure.com/#view/Microsoft_Azure_ContainerRegistries/TagMetadataBlade/registryId/subscriptions/%s/resourceGroups/%s/providers/Microsoft.ContainerRegistry/" ) var ( @@ -257,7 +258,7 @@ func run(c *cli.Context) error { Artifact: kaniko.Artifact{ Tags: c.StringSlice("tags"), Repo: c.String("repo"), - Registry: publicUrl, + Registry: publicUrl, // this is public url on which the artifact can be seen ArtifactFile: c.String("artifact-file"), RegistryType: artifact.Docker, }, @@ -399,14 +400,17 @@ func setupACRCert(cert string) error { } func getPublicUrl(token, registryUrl, subscriptionId string) (string, error) { - finalUrl := "https://portal.azure.com/#view/Microsoft_Azure_ContainerRegistries/TagMetadataBlade/registryId/subscriptions/%s/resourceGroups/%s/providers/Microsoft.ContainerRegistry/" + // for backward compatibilty, if the subscription id is not defined, do not fail step. + if len(subscriptionId) == 0 { + return "", nil + } + registry := strings.Split(registryUrl, ".")[0] burl := "https://management.azure.com/subscriptions/" + subscriptionId + "/resources?$filter=resourceType%20eq%20'Microsoft.ContainerRegistry/registries'%20and%20name%20eq%20'" + registry + "'&api-version=2021-04-01&$select=id" method := "GET" - client := &http.Client{} req, err := http.NewRequest(method, burl, nil) @@ -427,8 +431,7 @@ func getPublicUrl(token, registryUrl, subscriptionId string) (string, error) { if err != nil { return "", errors.Wrap(err, "failed to send request for getting container registry setting") } - resourceGroup := response.Value[0].ID - return fmt.Sprintf(finalUrl, subscriptionId, resourceGroup), nil + return fmt.Sprintf(finalUrl, subscriptionId, response.Value[0].ID), nil } type strct struct { From 6ac1efad25ec51ef3a29aaad148f648bc637f12e Mon Sep 17 00:00:00 2001 From: Aman Singh Date: Wed, 2 Nov 2022 15:58:40 +0530 Subject: [PATCH 4/8] fixed url --- cmd/kaniko-acr/main.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/kaniko-acr/main.go b/cmd/kaniko-acr/main.go index 2112c7f..dc4c59c 100644 --- a/cmd/kaniko-acr/main.go +++ b/cmd/kaniko-acr/main.go @@ -30,7 +30,7 @@ const ( certPathEnv string = "AZURE_CLIENT_CERTIFICATE_PATH" dockerConfigPath string = "/kaniko/.docker" defaultDigestFile string = "/kaniko/digest-file" - finalUrl string = "https://portal.azure.com/#view/Microsoft_Azure_ContainerRegistries/TagMetadataBlade/registryId/subscriptions/%s/resourceGroups/%s/providers/Microsoft.ContainerRegistry/" + finalUrl string = "https://portal.azure.com/#view/Microsoft_Azure_ContainerRegistries/TagMetadataBlade/registryId/%s" ) var ( @@ -413,11 +413,11 @@ func getPublicUrl(token, registryUrl, subscriptionId string) (string, error) { method := "GET" client := &http.Client{} req, err := http.NewRequest(method, burl, nil) - if err != nil { fmt.Println(err) return "", errors.Wrap(err, "failed to create request for getting container registry setting") } + req.Header.Add("Authorization", "Bearer "+token) res, err := client.Do(req) if err != nil { @@ -431,7 +431,11 @@ func getPublicUrl(token, registryUrl, subscriptionId string) (string, error) { if err != nil { return "", errors.Wrap(err, "failed to send request for getting container registry setting") } - return fmt.Sprintf(finalUrl, subscriptionId, response.Value[0].ID), nil + return finalUrl + encodeParam(response.Value[0].ID), nil +} + +func encodeParam(s string) string { + return url.QueryEscape(s) } type strct struct { From 9c899979ff41c6fb44b631d327eee988cc2fef3d Mon Sep 17 00:00:00 2001 From: Aman Singh Date: Wed, 2 Nov 2022 16:09:57 +0530 Subject: [PATCH 5/8] fixed url --- cmd/kaniko-acr/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kaniko-acr/main.go b/cmd/kaniko-acr/main.go index dc4c59c..0a647b6 100644 --- a/cmd/kaniko-acr/main.go +++ b/cmd/kaniko-acr/main.go @@ -30,7 +30,7 @@ const ( certPathEnv string = "AZURE_CLIENT_CERTIFICATE_PATH" dockerConfigPath string = "/kaniko/.docker" defaultDigestFile string = "/kaniko/digest-file" - finalUrl string = "https://portal.azure.com/#view/Microsoft_Azure_ContainerRegistries/TagMetadataBlade/registryId/%s" + finalUrl string = "https://portal.azure.com/#view/Microsoft_Azure_ContainerRegistries/TagMetadataBlade/registryId/" ) var ( From 54f2fe097a4170f8ec5ed17c414c2dab2c590b17 Mon Sep 17 00:00:00 2001 From: Aman Singh Date: Wed, 2 Nov 2022 16:17:23 +0530 Subject: [PATCH 6/8] fixed url --- cmd/kaniko-acr/main.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/kaniko-acr/main.go b/cmd/kaniko-acr/main.go index 0a647b6..0073500 100644 --- a/cmd/kaniko-acr/main.go +++ b/cmd/kaniko-acr/main.go @@ -347,7 +347,9 @@ func getACRToken(subscriptionId, tenantId, clientId, clientSecret, cert, registr publicUrl, err := getPublicUrl(azToken.Token, registry, subscriptionId) if err != nil { - return "", "", errors.Wrap(err, "failed to fetch access token") + // execution should not fail because of this error. + fmt.Fprintf(os.Stderr, "failed to get public url with error: %s\n", err) + return "", "", nil } ACRToken, err := fetchACRToken(tenantId, azToken.Token, registry) From 7d751135b1851e38345db904a423a659efb3b3a7 Mon Sep 17 00:00:00 2001 From: Aman Singh Date: Wed, 2 Nov 2022 16:20:54 +0530 Subject: [PATCH 7/8] fixed url --- cmd/kaniko-acr/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/kaniko-acr/main.go b/cmd/kaniko-acr/main.go index 0073500..ff60c55 100644 --- a/cmd/kaniko-acr/main.go +++ b/cmd/kaniko-acr/main.go @@ -349,7 +349,6 @@ func getACRToken(subscriptionId, tenantId, clientId, clientSecret, cert, registr if err != nil { // execution should not fail because of this error. fmt.Fprintf(os.Stderr, "failed to get public url with error: %s\n", err) - return "", "", nil } ACRToken, err := fetchACRToken(tenantId, azToken.Token, registry) From d11c254840138ca10c8c357509218578914e8fbb Mon Sep 17 00:00:00 2001 From: Aman Singh Date: Wed, 2 Nov 2022 17:12:34 +0530 Subject: [PATCH 8/8] fixed test --- pkg/artifact/artifact.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/artifact/artifact.go b/pkg/artifact/artifact.go index 9f01286..3e4ecf9 100644 --- a/pkg/artifact/artifact.go +++ b/pkg/artifact/artifact.go @@ -30,7 +30,6 @@ type ( Data struct { RegistryType RegistryTypeEnum `json:"registryType"` RegistryUrl string `json:"registryUrl"` - PublicUrl string `json:"publicUrl"` Images []Image `json:"images"` } DockerArtifact struct {