Skip to content

[RFC-0010] Add provider audience to cache key and decouple packages #928

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.

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

Merged
merged 1 commit into from
May 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 13 additions & 39 deletions auth/gcp/gke_metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,51 +17,25 @@ limitations under the License.
package gcp_test

import (
"context"
"errors"
"fmt"
"net"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

. "github.com/onsi/gomega"
)

func startGKEMetadataServer(t *testing.T) {
t.Helper()
g := NewWithT(t)

lis, err := net.Listen("tcp", ":0")
g.Expect(err).NotTo(HaveOccurred())

gkeMetadataServer := &http.Server{
Addr: lis.Addr().String(),
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/computeMetadata/v1/project/project-id":
fmt.Fprintf(w, "%s", "project-id")
case "/computeMetadata/v1/instance/attributes/cluster-location":
fmt.Fprintf(w, "%s", "cluster-location")
case "/computeMetadata/v1/instance/attributes/cluster-name":
fmt.Fprintf(w, "%s", "cluster-name")
}
}),
}

go func() {
err := gkeMetadataServer.Serve(lis)
if err != nil && !errors.Is(err, http.ErrServerClosed) {
g.Expect(err).NotTo(HaveOccurred())
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/computeMetadata/v1/project/project-id":
fmt.Fprintf(w, "%s", "project-id")
case "/computeMetadata/v1/instance/attributes/cluster-location":
fmt.Fprintf(w, "%s", "cluster-location")
case "/computeMetadata/v1/instance/attributes/cluster-name":
fmt.Fprintf(w, "%s", "cluster-name")
}
}()

t.Cleanup(func() {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
err := gkeMetadataServer.Shutdown(ctx)
g.Expect(err).NotTo(HaveOccurred())
})

t.Setenv("GCE_METADATA_HOST", lis.Addr().String())
}))
t.Cleanup(srv.Close)
t.Setenv("GCE_METADATA_HOST", strings.TrimPrefix(srv.URL, "http://"))
}
6 changes: 3 additions & 3 deletions auth/gcp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ func getServiceAccountEmail(serviceAccount corev1.ServiceAccount) (string, error
return email, nil
}

const workloadIdentityProviderPattern = `^https://iam\.googleapis\.com/projects/\d{1,30}/locations/global/workloadIdentityPools/[^/]{1,100}/providers/[^/]{1,100}$`
const workloadIdentityProviderPattern = `^projects/\d{1,30}/locations/global/workloadIdentityPools/[^/]{1,100}/providers/[^/]{1,100}$`

var workloadIdentityProviderRegex = regexp.MustCompile(workloadIdentityProviderPattern)

func getWorkloadIdentityProvider(serviceAccount corev1.ServiceAccount) (string, error) {
func getWorkloadIdentityProviderAudience(serviceAccount corev1.ServiceAccount) (string, error) {
const key = "gcp.auth.fluxcd.io/workload-identity-provider"
wip := serviceAccount.Annotations[key]
if wip == "" {
Expand All @@ -54,5 +54,5 @@ func getWorkloadIdentityProvider(serviceAccount corev1.ServiceAccount) (string,
return "", fmt.Errorf("invalid %s annotation: '%s'. must match %s",
key, wip, workloadIdentityProviderPattern)
}
return wip, nil
return fmt.Sprintf("https://iam.googleapis.com/%s", wip), nil
}
4 changes: 2 additions & 2 deletions auth/gcp/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (p Provider) NewControllerToken(ctx context.Context, opts ...auth.Option) (
func (Provider) GetAudience(ctx context.Context, serviceAccount corev1.ServiceAccount) (string, error) {
// Check if a workload identity provider is specified in the service account.
// If so, the current cluster is not GKE and the audience is the provider itself.
audience, err := getWorkloadIdentityProvider(serviceAccount)
audience, err := getWorkloadIdentityProviderAudience(serviceAccount)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -100,7 +100,7 @@ func (p Provider) NewTokenForServiceAccount(ctx context.Context, oidcToken strin

// Check if a workload identity provider is specified in the service account.
// If so, the current cluster is not GKE and the audience is the provider itself.
audience, err := getWorkloadIdentityProvider(serviceAccount)
audience, err := getWorkloadIdentityProviderAudience(serviceAccount)
if err != nil {
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions auth/gcp/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestProvider_NewTokenForServiceAccount(t *testing.T) {
UniverseDomain: "googleapis.com",
},
annotations: map[string]string{
"gcp.auth.fluxcd.io/workload-identity-provider": "https://iam.googleapis.com/projects/1234567890/locations/global/workloadIdentityPools/test-pool/providers/test-provider",
"gcp.auth.fluxcd.io/workload-identity-provider": "projects/1234567890/locations/global/workloadIdentityPools/test-pool/providers/test-provider",
},
},
{
Expand All @@ -128,7 +128,7 @@ func TestProvider_NewTokenForServiceAccount(t *testing.T) {
},
annotations: map[string]string{
"iam.gke.io/gcp-service-account": "[email protected]",
"gcp.auth.fluxcd.io/workload-identity-provider": "https://iam.googleapis.com/projects/1234567890/locations/global/workloadIdentityPools/test-pool/providers/test-provider",
"gcp.auth.fluxcd.io/workload-identity-provider": "projects/1234567890/locations/global/workloadIdentityPools/test-pool/providers/test-provider",
},
},
{
Expand All @@ -143,7 +143,7 @@ func TestProvider_NewTokenForServiceAccount(t *testing.T) {
annotations: map[string]string{
"gcp.auth.fluxcd.io/workload-identity-provider": "foobar",
},
err: `invalid gcp.auth.fluxcd.io/workload-identity-provider annotation: 'foobar'. must match ^https://iam\.googleapis\.com/projects/\d{1,30}/locations/global/workloadIdentityPools/[^/]{1,100}/providers/[^/]{1,100}$`,
err: `invalid gcp.auth.fluxcd.io/workload-identity-provider annotation: 'foobar'. must match ^projects/\d{1,30}/locations/global/workloadIdentityPools/[^/]{1,100}/providers/[^/]{1,100}$`,
},
} {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -195,7 +195,7 @@ func TestProvider_GetAudience(t *testing.T) {
{
name: "federation",
annotations: map[string]string{
"gcp.auth.fluxcd.io/workload-identity-provider": "https://iam.googleapis.com/projects/1234567890/locations/global/workloadIdentityPools/test-pool/providers/test-provider",
"gcp.auth.fluxcd.io/workload-identity-provider": "projects/1234567890/locations/global/workloadIdentityPools/test-pool/providers/test-provider",
},
expected: "https://iam.googleapis.com/projects/1234567890/locations/global/workloadIdentityPools/test-pool/providers/test-provider",
},
Expand Down
9 changes: 6 additions & 3 deletions auth/get_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func GetToken(ctx context.Context, provider Provider, opts ...Option) (Token, er
}

// Update access token fetcher for a service account if specified.
var providerAudience string
var providerIdentity string
var serviceAccountP *corev1.ServiceAccount
if o.ServiceAccount != nil {
Expand All @@ -63,7 +64,7 @@ func GetToken(ctx context.Context, provider Provider, opts ...Option) (Token, er

// Get provider audience.
var err error
providerAudience, err := provider.GetAudience(ctx, serviceAccount)
providerAudience, err = provider.GetAudience(ctx, serviceAccount)
if err != nil {
return nil, fmt.Errorf("failed to get provider audience: %w", err)
}
Expand Down Expand Up @@ -131,7 +132,8 @@ func GetToken(ctx context.Context, provider Provider, opts ...Option) (Token, er
}

// Build cache key.
cacheKey := buildCacheKey(provider, providerIdentity, artifactRepositoryCacheKey, serviceAccountP, opts...)
cacheKey := buildCacheKey(provider, providerAudience, providerIdentity,
artifactRepositoryCacheKey, serviceAccountP, opts...)

// Get involved object details.
kind := o.InvolvedObject.Kind
Expand Down Expand Up @@ -163,7 +165,7 @@ func newServiceAccountToken(ctx context.Context, client client.Client,
return tokenReq.Status.Token, nil
}

func buildCacheKey(provider Provider, providerIdentity, artifactRepositoryKey string,
func buildCacheKey(provider Provider, providerAudience, providerIdentity, artifactRepositoryKey string,
serviceAccount *corev1.ServiceAccount, opts ...Option) string {

var o Options
Expand All @@ -174,6 +176,7 @@ func buildCacheKey(provider Provider, providerIdentity, artifactRepositoryKey st
keyParts = append(keyParts, fmt.Sprintf("provider=%s", provider.GetName()))

if serviceAccount != nil {
keyParts = append(keyParts, fmt.Sprintf("providerAudience=%s", providerAudience))
keyParts = append(keyParts, fmt.Sprintf("providerIdentity=%s", providerIdentity))
keyParts = append(keyParts, fmt.Sprintf("serviceAccountName=%s", serviceAccount.Name))
keyParts = append(keyParts, fmt.Sprintf("serviceAccountNamespace=%s", serviceAccount.Namespace))
Expand Down
3 changes: 2 additions & 1 deletion auth/get_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ func TestGetToken(t *testing.T) {
name: "all the options are taken into account in the cache key",
provider: &mockProvider{
returnName: "mock-provider",
returnAudience: "mock-audience",
returnIdentity: "mock-identity",
returnRegistryInput: "artifact-cache-key",
paramServiceAccount: *defaultServiceAccount,
Expand All @@ -325,7 +326,7 @@ func TestGetToken(t *testing.T) {
tokenCache, err := cache.NewTokenCache(1)
g.Expect(err).NotTo(HaveOccurred())

const key = "da48da328aa46181e677d76c835b7ca32b5fbf64da01577463d42a2720708ecb"
const key = "3e8e270134e99fda1a01d7dca77f29448eb4c7f6cc026137b85a1bcd96b276fa"
token := &mockToken{token: "cached-token"}
cachedToken, ok, err := tokenCache.GetOrSet(ctx, key, func(ctx context.Context) (cache.Token, error) {
return token, nil
Expand Down
5 changes: 1 addition & 4 deletions auth/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ module github.com/fluxcd/pkg/auth

go 1.24.0

replace (
github.com/fluxcd/pkg/cache => ../cache
github.com/fluxcd/pkg/ssh => ../ssh
)
replace github.com/fluxcd/pkg/cache => ../cache

require (
cloud.google.com/go/compute/metadata v0.6.0
Expand Down
12 changes: 7 additions & 5 deletions auth/utils/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

// authutils contains utility functions that import both the core
// auth package and the provider packages i.e. functions that
// cannot be placed in the core package because they would cause
// a cyclic dependency (the provider packages also import the
// core package).
// authutils contains small utility functions without much logic
// wrapping the major APIs of the core auth package for ease of use
// in the controllers. These functions also import the provider
// packages to wrap switch-case choice of provider implementations.
// Because of that, these functions cannot be placed in the core
// package as they would cause a cyclic dependency given that the
// provider packages also import the core package.
package authutils
51 changes: 51 additions & 0 deletions auth/utils/git.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
Copyright 2025 The Flux authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package authutils

import (
"context"
"errors"

"github.com/fluxcd/pkg/auth"
"github.com/fluxcd/pkg/auth/azure"
)

// GitCredentials contains authentication data needed in order to access a Git
// repository.
type GitCredentials struct {
BearerToken string
Username string
Password string
}

// GetGitCredentials looks up by the implemented providers that support Git
// and returns the credentials for the provider.
func GetGitCredentials(ctx context.Context, providerName string, opts ...auth.Option) (*GitCredentials, error) {
switch providerName {
case azure.ProviderName:
opts = append(opts, auth.WithScopes(azure.ScopeDevOps))
token, err := auth.GetToken(ctx, azure.Provider{}, opts...)
if err != nil {
return nil, err
}
return &GitCredentials{
BearerToken: token.(*azure.Token).Token,
}, nil
default:
return nil, errors.New("unsupported provider")
}
}
92 changes: 0 additions & 92 deletions git/credentials.go

This file was deleted.

Loading
Loading