diff --git a/cmd/controlplane/controller.go b/cmd/controlplane/controller.go index c21305322f..84ced205c3 100644 --- a/cmd/controlplane/controller.go +++ b/cmd/controlplane/controller.go @@ -38,7 +38,7 @@ import ( "github.com/akuity/kargo/pkg/types" versionpkg "github.com/akuity/kargo/pkg/x/version" - _ "github.com/akuity/kargo/pkg/credentials/acr" + _ "github.com/akuity/kargo/pkg/credentials/azure" _ "github.com/akuity/kargo/pkg/credentials/basic" _ "github.com/akuity/kargo/pkg/credentials/ecr" _ "github.com/akuity/kargo/pkg/credentials/gar" diff --git a/docs/docs/40-operator-guide/40-security/50-ambient-credentials.md b/docs/docs/40-operator-guide/40-security/50-ambient-credentials.md index 04ad3e22d2..8d0d6b7c0a 100644 --- a/docs/docs/40-operator-guide/40-security/50-ambient-credentials.md +++ b/docs/docs/40-operator-guide/40-security/50-ambient-credentials.md @@ -175,15 +175,16 @@ Tokens Kargo obtains for accessing any specific GAR repository on behalf of any specific Kargo Project are valid for 60 minutes and cached until shortly before they expire. A controller restart clears the cache. -## Azure Container Registry (ACR) +## Azure Container Registry (ACR) and Azure DevOps (ADO) -Kargo can be configured to authenticate to ACR repositories using +Kargo can be configured to authenticate to ACR and ADO repositories using [Azure Workload Identity](https://learn.microsoft.com/en-us/azure/aks/workload-identity-overview). If Kargo locates no `Secret` resources matching a repository URL and is deployed within an AKS cluster with workload identity enabled, it will attempt to use it -to authenticate. Leveraging this eliminates the need to store ACR credentials in -a `Secret` resource. Workload Identity can be enabled when creating a +to authenticate. Leveraging this eliminates the need to store ACR and/or ADO +credentials in a `Secret` resource. Workload Identity can be enabled when +creating a [new cluster](https://learn.microsoft.com/en-us/azure/aks/workload-identity-deploy-cluster#create-an-aks-cluster) or can be added to an [existing cluster](https://learn.microsoft.com/en-us/azure/aks/workload-identity-deploy-cluster#update-an-existing-aks-cluster). @@ -226,19 +227,29 @@ To access container images or Helm charts hosted in ACR, the managed identity [must be granted the `AcrPull` role](https://learn.microsoft.com/en-us/azure/container-registry/container-registry-authentication-managed-identity?tabs=azure-cli#grant-identity-access-to-the-container-registry) on the registry or on individual repositories within it. +To access git repositories hosted in Azure DevOps, the managed identity (or +service principal) must also be added to your Azure DevOps organization with +`Contribute` permissions for authorized repositories. Refer to the +[Azure DevOps documentation](https://learn.microsoft.com/en-us/azure/devops/integrate/get-started/authentication/service-principal-managed-identity?view=azure-devops) +for more information on configuring access to Azure DevOps via Entra identities. + :::danger Before continuing, be certain of the following: -* You have created a **User-Assigned Managed Identity**. - - ⚠️ This is different from an App Registration! +In general: * You have created a **Federated Identity Credential** that associates the managed identity with the Kubernetes `ServiceAccount` used by the Kargo controller. (In a typical installation of Kargo, this is the `kargo-controller` `ServiceAccount` in the `kargo` namespace.) +Additionally, for accessing artifacts hosted in ACR: + +* You have created a **User-Assigned Managed Identity**. + + ⚠️ This is different from an App Registration! + * The managed identity has been granted the **`AcrPull` role** on your ACR registry or specific repositories within it. diff --git a/pkg/credentials/acr/workload_identity.go b/pkg/credentials/azure/workload_identity.go similarity index 58% rename from pkg/credentials/acr/workload_identity.go rename to pkg/credentials/azure/workload_identity.go index 611f84ce17..daa38a0c74 100644 --- a/pkg/credentials/acr/workload_identity.go +++ b/pkg/credentials/azure/workload_identity.go @@ -1,9 +1,10 @@ -package acr +package azure import ( "context" "fmt" "regexp" + "strings" "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore" @@ -17,15 +18,17 @@ import ( ) const ( - // cacheTTLMinutes is how long we cache ACR tokens before refreshing them. - // Set to 2.5 hours to ensure we refresh before the 3-hour token expiry. + // cacheTTLMinutes is how long we cache tokens before refreshing them. + // Set to 2.5 hours to ensure we refresh before the 3-hour ACR token expiry. cacheTTLMinutes = 150 // cleanupIntervalMinutes is how often the cache cleanup runs cleanupIntervalMinutes = 30 - // acrTokenUsername is the fixed username used for ACR token authentication - acrTokenUsername = "00000000-0000-0000-0000-000000000000" // acrScope is the Azure AD scope required for ACR authentication acrScope = "https://containerregistry.azure.net/.default" + // adoScope is the Azure AD scope for Azure DevOps + adoScope = "499b84ac-1321-427f-aa17-267ca6975798/.default" + // azTokenUsername is the fixed username used for token authentication + azTokenUsername = "00000000-0000-0000-0000-000000000000" ) // acrURLRegex matches Azure Container Registry URLs. @@ -44,14 +47,14 @@ func init() { } // WorkloadIdentityProvider implements credentials.Provider for Azure Container -// Registry Workload Identity. +// Registry and Azure DevOps via Azure Workload Identity. type WorkloadIdentityProvider struct { - // tokenCache is an in-memory cache of ACR registry access tokens keyed by - // registry name. + // tokenCache is an in-memory cache of access tokens keyed by ACR registry + // name or (static) ADO scope. tokenCache *cache.Cache credential azcore.TokenCredential - getAccessTokenFn func(ctx context.Context, registryName string) (string, error) + getAccessTokenFn func(ctx context.Context, credentialsType credentials.Type, registryName string) (string, time.Duration, error) } // NewWorkloadIdentityProvider returns a new WorkloadIdentityProvider if Azure @@ -86,43 +89,56 @@ func (p *WorkloadIdentityProvider) Supports( _ context.Context, req credentials.Request, ) (bool, error) { - if req.Type != credentials.TypeImage && req.Type != credentials.TypeHelm { + switch req.Type { + case credentials.TypeImage, credentials.TypeHelm: + // Check if this is an ACR URL + return acrURLRegex.MatchString(req.RepoURL), nil + case credentials.TypeGit: + return strings.HasPrefix(req.RepoURL, "http://") || strings.HasPrefix(req.RepoURL, "https://"), nil + default: return false, nil } - // Check if this is an ACR URL - return acrURLRegex.MatchString(req.RepoURL), nil } func (p *WorkloadIdentityProvider) GetCredentials( ctx context.Context, req credentials.Request, ) (*credentials.Credentials, error) { - // Extract the registry name from the ACR URL - matches := acrURLRegex.FindStringSubmatch(req.RepoURL) - if len(matches) != 2 { // This doesn't look like an ACR URL - return nil, nil + var cacheKey string + switch req.Type { + case credentials.TypeImage, credentials.TypeHelm: + // Extract the registry name from the ACR URL + matches := acrURLRegex.FindStringSubmatch(req.RepoURL) + if len(matches) != 2 { // This doesn't look like an ACR URL + return nil, nil + } + cacheKey = matches[1] + case credentials.TypeGit: + // Use the Azure DevOps scope as the cache key + cacheKey = adoScope + default: + return nil, fmt.Errorf("invalid credentials type: %s", req.Type) } - registryName := matches[1] logger := logging.LoggerFromContext(ctx).WithValues( - "provider", "acrWorkloadIdentity", + "provider", "azureWorkloadIdentity", "repoURL", req.RepoURL, ) // Check the cache for the token - if entry, exists := p.tokenCache.Get(registryName); exists { + if entry, exists := p.tokenCache.Get(cacheKey); exists { logger.Debug("access token cache hit") return &credentials.Credentials{ - Username: acrTokenUsername, + Username: azTokenUsername, Password: entry.(string), // nolint: forcetypeassert }, nil } logger.Debug("access token cache miss") // Cache miss, get a new token - accessToken, err := p.getAccessTokenFn(ctx, registryName) + accessToken, ttl, err := p.getAccessTokenFn(ctx, req.Type, cacheKey) if err != nil { - return nil, fmt.Errorf("error getting ACR access token: %w", err) + return nil, fmt.Errorf("error getting access token: %w", err) } // If we didn't get a token, we'll treat this as no credentials found @@ -131,22 +147,42 @@ func (p *WorkloadIdentityProvider) GetCredentials( } logger.Debug("obtained new access token") - // Cache the token using the default TTL. The ACR refresh token exchange API - // does not expose token expiry, so a dynamic TTL is not possible here. + // Use the token TTL if available, otherwise fall back to the default. + // In general, Azure AD exposes token expiry, but the ACR refresh token + // exchange API does not. + if ttl == 0 { + ttl = cache.DefaultExpiration + } logger.Debug( "caching access token", - "ttl", cache.DefaultExpiration, + "ttl", ttl, ) - p.tokenCache.Set(registryName, accessToken, cache.DefaultExpiration) + p.tokenCache.Set(cacheKey, accessToken, ttl) return &credentials.Credentials{ - Username: acrTokenUsername, + Username: azTokenUsername, Password: accessToken, }, nil } -// getAccessToken returns an ACR refresh token using Azure workload identity. func (p *WorkloadIdentityProvider) getAccessToken( + ctx context.Context, + credentialsType credentials.Type, + registryName string, +) (string, time.Duration, error) { + switch credentialsType { + case credentials.TypeImage, credentials.TypeHelm: + token, err := p.getAcrAccessToken(ctx, registryName) + return token, 0, err + case credentials.TypeGit: + return p.getAdoAccessToken(ctx) + default: + return "", 0, fmt.Errorf("invalid credentials type: %s", credentialsType) + } +} + +// getAcrAccessToken returns an ACR refresh token using Azure workload identity. +func (p *WorkloadIdentityProvider) getAcrAccessToken( ctx context.Context, registryName string, ) (string, error) { @@ -190,3 +226,23 @@ func (p *WorkloadIdentityProvider) getAccessToken( return *refreshTokenResp.RefreshToken, nil } + +// getAdoAccessToken returns an ADO access token using Azure workload identity. +func (p *WorkloadIdentityProvider) getAdoAccessToken(ctx context.Context) (string, time.Duration, error) { + // Get Azure AD access token with the standard ADO scope + token, err := p.credential.GetToken(ctx, policy.TokenRequestOptions{ + Scopes: []string{adoScope}, + }) + if err != nil { + return "", 0, fmt.Errorf("failed to get Azure AD access token for ADO: %w", err) + } + // Return the time until the explicit refresh-on if provided, otherwise fall + // back to the expires-on minus 5 minutes (to be safe). + duration := time.Duration(0) + if !token.RefreshOn.IsZero() { + duration = time.Until(token.RefreshOn) + } else if !token.ExpiresOn.IsZero() { + duration = time.Until(token.ExpiresOn) - time.Minute*5 + } + return token.Token, duration, nil +} diff --git a/pkg/credentials/acr/workload_identity_test.go b/pkg/credentials/azure/workload_identity_test.go similarity index 80% rename from pkg/credentials/acr/workload_identity_test.go rename to pkg/credentials/azure/workload_identity_test.go index 198c394147..2b47a3a572 100644 --- a/pkg/credentials/acr/workload_identity_test.go +++ b/pkg/credentials/azure/workload_identity_test.go @@ -1,4 +1,4 @@ -package acr +package azure import ( "context" @@ -64,15 +64,21 @@ func TestWorkloadIdentityProvider_Supports(t *testing.T) { expected: true, }, { - name: "helm HTTP/S repo URLs not supported", - credType: credentials.TypeHelm, + name: "git credential type supported", + credType: credentials.TypeGit, repoURL: testHTTPSRepoURL, - expected: false, + expected: true, }, { - name: "git credential type not supported", + name: "git non-HTTP/S repo URLs not supported", credType: credentials.TypeGit, - repoURL: testOCIRepoURL, + repoURL: "ssh://repo", + expected: false, + }, + { + name: "helm HTTP/S repo URLs not supported", + credType: credentials.TypeHelm, + repoURL: testHTTPSRepoURL, expected: false, }, { @@ -117,8 +123,8 @@ func TestWorkloadIdentityProvider_GetCredentials(t *testing.T) { { name: "not supported", provider: &WorkloadIdentityProvider{}, - credType: credentials.TypeGit, - repoURL: "git://repo", + credType: credentials.TypeHelm, + repoURL: "https://repo", assertions: func( t *testing.T, _ *cache.Cache, @@ -160,15 +166,15 @@ func TestWorkloadIdentityProvider_GetCredentials(t *testing.T) { ) { assert.NoError(t, err) assert.NotNil(t, creds) - assert.Equal(t, acrTokenUsername, creds.Username) + assert.Equal(t, azTokenUsername, creds.Username) assert.Equal(t, testToken, creds.Password) }, }, { name: "cache miss, successful token fetch", provider: &WorkloadIdentityProvider{ - getAccessTokenFn: func(_ context.Context, _ string) (string, error) { - return testToken, nil + getAccessTokenFn: func(_ context.Context, _ credentials.Type, _ string) (string, time.Duration, error) { + return testToken, 0, nil }, }, credType: credentials.TypeImage, @@ -181,7 +187,7 @@ func TestWorkloadIdentityProvider_GetCredentials(t *testing.T) { ) { assert.NoError(t, err) assert.NotNil(t, creds) - assert.Equal(t, acrTokenUsername, creds.Username) + assert.Equal(t, azTokenUsername, creds.Username) assert.Equal(t, testToken, creds.Password) // Verify the token was cached @@ -190,11 +196,41 @@ func TestWorkloadIdentityProvider_GetCredentials(t *testing.T) { assert.Equal(t, testToken, cachedToken) }, }, + { + name: "cache miss, successful token fetch with expiry", + provider: &WorkloadIdentityProvider{ + getAccessTokenFn: func(_ context.Context, _ credentials.Type, _ string) (string, time.Duration, error) { + return testToken, 5 * time.Minute, nil + }, + }, + credType: credentials.TypeGit, + repoURL: testRepoURL, + assertions: func( + t *testing.T, + c *cache.Cache, + creds *credentials.Credentials, + err error, + ) { + assert.NoError(t, err) + assert.NotNil(t, creds) + assert.Equal(t, azTokenUsername, creds.Username) + assert.Equal(t, testToken, creds.Password) + + // Verify the token was cached with a TTL based on the token's actual + // expiry + items := c.Items() + item, found := items[adoScope] + assert.True(t, found) + expectedTTL := 5 * time.Minute + actualTTL := time.Until(time.Unix(0, item.Expiration)) + assert.InDelta(t, expectedTTL.Seconds(), actualTTL.Seconds(), 5) + }, + }, { name: "error in getAccessToken", provider: &WorkloadIdentityProvider{ - getAccessTokenFn: func(_ context.Context, _ string) (string, error) { - return "", errors.New("access token error") + getAccessTokenFn: func(_ context.Context, _ credentials.Type, _ string) (string, time.Duration, error) { + return "", 0, errors.New("access token error") }, }, credType: credentials.TypeImage, @@ -205,15 +241,15 @@ func TestWorkloadIdentityProvider_GetCredentials(t *testing.T) { creds *credentials.Credentials, err error, ) { - assert.ErrorContains(t, err, "error getting ACR access token") + assert.ErrorContains(t, err, "error getting access token") assert.Nil(t, creds) }, }, { name: "empty token from getAccessToken", provider: &WorkloadIdentityProvider{ - getAccessTokenFn: func(_ context.Context, _ string) (string, error) { - return "", nil + getAccessTokenFn: func(_ context.Context, _ credentials.Type, _ string) (string, time.Duration, error) { + return "", 0, nil }, }, credType: credentials.TypeImage,