Skip to content

Add identity logging to ARM token exchange through IMDS #111

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.

Sign up for GitHub

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

Open
Copilot wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add identity logging to ARM token exchange through IMDS #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5bab338
Initial plan for issue
Copilot Jun 6, 2025
7b96176
Add identity logging to ARM token exchange through IMDS
Copilot Jun 6, 2025
c5c9886
Format code and finalize identity logging implementation
Copilot Jun 6, 2025
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
2 changes: 1 addition & 1 deletion internal/controller/acrpullbinding_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func NewV1beta1Reconciler(opts *V1beta1ReconcilerOpts) *AcrPullBindingReconciler
msiClientID, msiResourceID, acrServer := specOrDefault(opts, binding.Spec)
return base36sha224([]byte(msiClientID + msiResourceID + acrServer + binding.Spec.Scope))
},
CreatePullCredential: func(ctx context.Context, binding *msiacrpullv1beta1.AcrPullBinding, serviceAccount *corev1.ServiceAccount) (string, time.Time, error) {
CreatePullCredential: func(ctx context.Context, logger logr.Logger, binding *msiacrpullv1beta1.AcrPullBinding, serviceAccount *corev1.ServiceAccount) (string, time.Time, error) {
msiClientID, msiResourceID, acrServer := specOrDefault(opts, binding.Spec)
acrAccessToken, err := opts.Auth.AcquireACRAccessToken(ctx, msiResourceID, msiClientID, acrServer, binding.Spec.Scope)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/acrpullbinding_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,7 @@ func Test_ACRPullBindingController_reconcile(t *testing.T) {
referencingServiceAccounts: []corev1.ServiceAccount{
{
ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "delegate"},
ImagePullSecrets: []corev1.LocalObjectReference{{Name: "binding-msi-acrpull-secret"},{Name: "previous-binding-msi-acrpull-secret"},{Name: "extraneous"}},
ImagePullSecrets: []corev1.LocalObjectReference{{Name: "binding-msi-acrpull-secret"}, {Name: "previous-binding-msi-acrpull-secret"}, {Name: "extraneous"}},
},
},
pullSecrets: []corev1.Secret{{
Expand Down
6 changes: 3 additions & 3 deletions internal/controller/acrpullbinding_v1beta2_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type CoreOpts struct {
}

type ServiceAccountTokenMinter func(ctx context.Context, serviceAccountNamespace, serviceAccountName string) (*authenticationv1.TokenRequest, error)
type armTokenFetcher func(ctx context.Context, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error)
type armTokenFetcher func(ctx context.Context, logger logr.Logger, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error)
type armAcrTokenExchanger func(ctx context.Context, armToken azcore.AccessToken, spec msiacrpullv1beta2.AcrConfiguration) (azcore.AccessToken, error)

// V1beta2ReconcilerOpts configures the inputs for reconciling v1beta2 pull bindings
Expand Down Expand Up @@ -97,7 +97,7 @@ func NewV1beta2Reconciler(opts *V1beta2ReconcilerOpts) *PullBindingReconciler {
GetInputsHash: func(binding *msiacrpullv1beta2.AcrPullBinding) string {
return inputsHash(binding.Spec)
},
CreatePullCredential: func(ctx context.Context, binding *msiacrpullv1beta2.AcrPullBinding, serviceAccount *corev1.ServiceAccount) (string, time.Time, error) {
CreatePullCredential: func(ctx context.Context, logger logr.Logger, binding *msiacrpullv1beta2.AcrPullBinding, serviceAccount *corev1.ServiceAccount) (string, time.Time, error) {
var tenantId, clientId, token string
if binding.Spec.Auth.WorkloadIdentity != nil {
if binding.Spec.Auth.WorkloadIdentity.TenantID != "" {
Expand Down Expand Up @@ -126,7 +126,7 @@ func NewV1beta2Reconciler(opts *V1beta2ReconcilerOpts) *PullBindingReconciler {
token = response.Status.Token
}

armToken, err := opts.fetchArmToken(ctx, binding.Spec, tenantId, clientId, token)
armToken, err := opts.fetchArmToken(ctx, logger, binding.Spec, tenantId, clientId, token)
if err != nil {
return "", time.Time{}, fmt.Errorf("failed to retrieve ARM token: %v", err)
}
Expand Down
9 changes: 5 additions & 4 deletions internal/controller/acrpullbinding_v1beta2_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
msiacrpullv1beta2 "github.com/Azure/msi-acrpull/api/v1beta2"
"github.com/go-logr/logr"
"github.com/go-logr/logr/testr"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -1559,7 +1560,7 @@ func noopTokenStub() func(*testing.T, *msiacrpullv1beta2.AcrPullBinding, *corev1
return func(t *testing.T, binding *msiacrpullv1beta2.AcrPullBinding, serviceAccount *corev1.ServiceAccount) (ServiceAccountTokenMinter, armTokenFetcher, armAcrTokenExchanger) {
return func(ctx context.Context, serviceAccountNamespace, serviceAccountName string) (*authenticationv1.TokenRequest, error) {
return nil, errors.New("unexpected call to SA token request")
}, func(ctx context.Context, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
}, func(ctx context.Context, logger logr.Logger, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
return azcore.AccessToken{}, errors.New("unexpected call to ARM token request")
}, func(ctx context.Context, armToken azcore.AccessToken, spec msiacrpullv1beta2.AcrConfiguration) (azcore.AccessToken, error) {
return azcore.AccessToken{}, errors.New("unexpected call to ARM ACR token exchange")
Expand All @@ -1571,7 +1572,7 @@ func managedIdentityValidatingTokenStub(output azcore.AccessToken, outputError e
return func(t *testing.T, binding *msiacrpullv1beta2.AcrPullBinding, serviceAccount *corev1.ServiceAccount) (ServiceAccountTokenMinter, armTokenFetcher, armAcrTokenExchanger) {
return func(ctx context.Context, serviceAccountNamespace, serviceAccountName string) (*authenticationv1.TokenRequest, error) {
return nil, errors.New("unexpected call to SA token request for managed identity")
}, func(ctx context.Context, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
}, func(ctx context.Context, logger logr.Logger, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
assert.Empty(t, cmp.Diff(spec, binding.Spec), "arm token request binding spec mismatch")
assert.Empty(t, serviceAccount.Annotations["azure.workload.identity/tenant-id"], "arm token request unexpected tenant id")
assert.Empty(t, serviceAccount.Annotations["azure.workload.identity/client-id"], "arm token request unexpected client id")
Expand All @@ -1597,7 +1598,7 @@ func workloadIdentityValidatingTokenStub(output azcore.AccessToken, outputError
Token: "fake-sa-token",
},
}, nil
}, func(ctx context.Context, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
}, func(ctx context.Context, logger logr.Logger, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
assert.Empty(t, cmp.Diff(spec, binding.Spec), "arm token request binding spec mismatch")
assert.Equal(t, serviceAccount.Annotations["azure.workload.identity/tenant-id"], tenantId, "arm token request tenant id mismatch")
assert.Equal(t, serviceAccount.Annotations["azure.workload.identity/client-id"], clientId, "arm token request client id mismatch")
Expand All @@ -1623,7 +1624,7 @@ func workloadIdentityLiteralValidatingTokenStub(output azcore.AccessToken, outpu
Token: "fake-sa-token",
},
}, nil
}, func(ctx context.Context, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
}, func(ctx context.Context, logger logr.Logger, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
assert.Empty(t, cmp.Diff(spec, binding.Spec), "arm token request binding spec mismatch")
assert.Equal(t, spec.Auth.WorkloadIdentity.TenantID, tenantId, "arm token request tenant id mismatch")
assert.Equal(t, spec.Auth.WorkloadIdentity.ClientID, clientId, "arm token request client id mismatch")
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/generic_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type genericReconciler[O pullBinding] struct {
GetPullSecretName func(O) string
GetInputsHash func(O) string

CreatePullCredential func(context.Context, O, *corev1.ServiceAccount) (string, time.Time, error)
CreatePullCredential func(context.Context, logr.Logger, O, *corev1.ServiceAccount) (string, time.Time, error)

UpdateStatusError func(O, string) O

Expand Down Expand Up @@ -162,7 +162,7 @@ func (r *genericReconciler[O]) reconcile(ctx context.Context, logger logr.Logger
if pullSecretMissing || pullSecretNeedsRefresh || pullSecretInputsChanged {
logger.WithValues("pullSecretMissing", pullSecretMissing, "pullSecretNeedsRefresh", pullSecretNeedsRefresh, "pullSecretInputsChanged", pullSecretInputsChanged).Info("generating new pull credential")

dockerConfig, expiresOn, err := r.CreatePullCredential(ctx, acrBinding, serviceAccount)
dockerConfig, expiresOn, err := r.CreatePullCredential(ctx, logger, acrBinding, serviceAccount)
if err != nil {
logger.Info(err.Error())
return &action[O]{updatePullBindingStatus: r.UpdateStatusError(acrBinding, err.Error())}
Expand Down
8 changes: 7 additions & 1 deletion pkg/authorizer/token_retriever.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/go-logr/logr"

msiacrpullv1beta2 "github.com/Azure/msi-acrpull/api/v1beta2"
)
Expand All @@ -31,7 +32,7 @@ func AcquireARMToken(ctx context.Context, id azidentity.ManagedIDKind) (azcore.A
return cred.GetToken(ctx, policy.TokenRequestOptions{Scopes: []string{customARMResource}})
}

func ARMTokenForBinding(ctx context.Context, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
func ARMTokenForBinding(ctx context.Context, logger logr.Logger, spec msiacrpullv1beta2.AcrPullBindingSpec, tenantId, clientId, serviceAccountToken string) (azcore.AccessToken, error) {
env := environment(spec.ACR.Environment, spec.ACR.CloudConfig)

var credential azcore.TokenCredential
Expand All @@ -41,11 +42,16 @@ func ARMTokenForBinding(ctx context.Context, spec msiacrpullv1beta2.AcrPullBindi
var id azidentity.ManagedIDKind
if spec.Auth.ManagedIdentity.ClientID != "" {
id = azidentity.ClientID(spec.Auth.ManagedIdentity.ClientID)
logger.Info("Attempting token exchange through IMDS using managed identity", "clientID", spec.Auth.ManagedIdentity.ClientID)
} else if spec.Auth.ManagedIdentity.ResourceID != "" {
id = azidentity.ResourceID(spec.Auth.ManagedIdentity.ResourceID)
logger.Info("Attempting token exchange through IMDS using managed identity", "resourceID", spec.Auth.ManagedIdentity.ResourceID)
} else {
logger.Info("Attempting token exchange through IMDS using system-assigned managed identity")
}
credential, err = azidentity.NewManagedIdentityCredential(&azidentity.ManagedIdentityCredentialOptions{ID: id})
case spec.Auth.WorkloadIdentity != nil:
logger.Info("Attempting token exchange using workload identity", "tenantID", tenantId, "clientID", clientId)
// n.b. the built-in azidentity.WorkloadIdentityCredential assumes we're loading a service account token
// from a file in a Pod, where the Kubernetes API server is rotating it, etc. Unfortunately that is not
// our use-case here, and we certainly don't want to centralize every service account token we ever mint
Expand Down
Loading