Skip to content

Commit

Permalink
Add Azure Workload Identity support (radius-project#7640)
Browse files Browse the repository at this point in the history
# Description

* Add Azure Workload Identity support to the Azure provider

Associated changes in DE:
radius-project/deployment-engine#372

Note: I expect the functional tests to fail because I am updating them
in this PR. I will run them manually to confirm that the changes work.

## Type of change

<!--

Please select **one** of the following options that describes your
change and delete the others. Clearly identifying the type of change you
are making will help us review your PR faster, and is used in authoring
release notes.

If you are making a bug fix or functionality change to Radius and do not
have an associated issue link please create one now.

-->

- This pull request adds or changes features of Radius and has an
approved issue (issue link required).

<!--

Please update the following to link the associated issue. This is
required for some kinds of changes (see above).

-->

Issue Reference: radius-project#7308

---------

Signed-off-by: willdavsmith <[email protected]>
  • Loading branch information
willdavsmith authored Jun 18, 2024
1 parent 86b4397 commit b748443
Show file tree
Hide file tree
Showing 73 changed files with 2,411 additions and 551 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/functional-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ jobs:
echo "*** Configuring Azure provider ***"
rad env update kind-radius --azure-subscription-id ${{ secrets.AZURE_SUBSCRIPTIONID_TESTS }} \
--azure-resource-group ${{ env.AZURE_TEST_RESOURCE_GROUP }}
rad credential register azure --client-id ${{ secrets.AZURE_SP_TESTS_APPID }} \
rad credential register azure sp --client-id ${{ secrets.AZURE_SP_TESTS_APPID }} \
--client-secret ${{ secrets.INTEGRATION_TEST_SP_PASSWORD }} \
--tenant-id ${{ secrets.AZURE_SP_TESTS_TENANTID }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/long-running-azure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ jobs:
echo "*** Configuring Azure provider ***"
rad env update ${{ env.RADIUS_TEST_ENVIRONMENT_NAME }} --azure-subscription-id ${{ secrets.AZURE_SUBSCRIPTIONID_TESTS }} \
--azure-resource-group ${{ env.AZURE_TEST_RESOURCE_GROUP }}
rad credential register azure --client-id ${{ secrets.AZURE_SP_TESTS_APPID }} \
rad credential register azure sp --client-id ${{ secrets.AZURE_SP_TESTS_APPID }} \
--client-secret ${{ secrets.INTEGRATION_TEST_SP_PASSWORD }} \
--tenant-id ${{ secrets.AZURE_SP_TESTS_TENANTID }}
Expand Down
3 changes: 3 additions & 0 deletions deploy/Chart/templates/de/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ spec:
control-plane: bicep-de
app.kubernetes.io/name: bicep-de
app.kubernetes.io/part-of: radius
{{- if eq .Values.global.azureWorkloadIdentity.enabled true }}
azure.workload.identity/use: "true"
{{- end }}
{{- if eq .Values.global.prometheus.enabled true }}
annotations:
prometheus.io/path: "/metrics"
Expand Down
3 changes: 3 additions & 0 deletions deploy/Chart/templates/rp/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ spec:
control-plane: applications-rp
app.kubernetes.io/name: applications-rp
app.kubernetes.io/part-of: radius
{{- if eq .Values.global.azureWorkloadIdentity.enabled true }}
azure.workload.identity/use: "true"
{{- end }}
{{- if eq .Values.global.prometheus.enabled true }}
annotations:
prometheus.io/path: "{{ .Values.global.prometheus.path }}"
Expand Down
3 changes: 3 additions & 0 deletions deploy/Chart/templates/ucp/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ spec:
control-plane: ucp
app.kubernetes.io/name: ucp
app.kubernetes.io/part-of: radius
{{- if eq .Values.global.azureWorkloadIdentity.enabled true }}
azure.workload.identity/use: "true"
{{- end }}
{{- if eq .Values.global.prometheus.enabled true }}
annotations:
prometheus.io/path: "{{ .Values.global.prometheus.path }}"
Expand Down
5 changes: 5 additions & 0 deletions deploy/Chart/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ global:
# url: "http://jaeger-collector.radius-monitoring.svc.cluster.local:9411/api/v2/spans"
#

# Configure global.azureWorkloadIdentity.enabled=true to enable Azure Workload Identity.
# Disabled by default.
azureWorkloadIdentity:
enabled: false

controller:
image: ghcr.io/radius-project/controller
# Default tag uses Chart AppVersion.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ The above steps will not configure the ability for Radius to talk with azure res
go run ./cmd/rad/main.go env create radius-rg --namespace default
go run ./cmd/rad/main.go env switch radius-rg
go run ./cmd/rad/main.go env update radius-rg --azure-subscription-id <subscriptionId> --azure-resource-group <resourcegroupName>
go run ./cmd/rad/main.go credential register azure --client-id <appId> --client-secret <pwd> --tenant-id <tenantId>
go run ./cmd/rad/main.go credential register azure sp --client-id <appId> --client-secret <pwd> --tenant-id <tenantId>
```
1. Run a deployment. Executing `go run \cmd\rad\main.go deploy <bicep>` will deploy your file to the cluster.
Expand Down
10 changes: 5 additions & 5 deletions pkg/azure/armauth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
const (
UCPCredentialAuth = "UCPCredential"
ServicePrincipalAuth = "ServicePrincipal"
WorkloadIdentityAuth = "WorkloadIdentity"
ManagedIdentityAuth = "ManagedIdentity"
CliAuth = "CLI"
)
Expand Down Expand Up @@ -67,6 +68,8 @@ func NewArmConfig(opt *Options) (*ArmConfig, error) {
func NewARMCredential(opt *Options) (azcore.TokenCredential, error) {
authMethod := GetAuthMethod()

// Use the Azure SDK for Go to create a credential based on the authentication method
// https://learn.microsoft.com/en-us/azure/aks/workload-identity-overview?tabs=go#azure-identity-client-libraries
switch authMethod {
case UCPCredentialAuth:
return azcred.NewUCPCredential(azcred.UCPCredentialOptions{
Expand All @@ -76,6 +79,8 @@ func NewARMCredential(opt *Options) (azcore.TokenCredential, error) {
return azidentity.NewEnvironmentCredential(nil)
case ManagedIdentityAuth:
return azidentity.NewManagedIdentityCredential(nil)
case WorkloadIdentityAuth:
return azidentity.NewDefaultAzureCredential(nil)
default:
return azidentity.NewAzureCLICredential(nil)
}
Expand All @@ -101,8 +106,3 @@ func GetAuthMethod() string {
return CliAuth
}
}

// IsServicePrincipalConfigured checks if ServicePrincipalAuth is the authentication method configured.
func IsServicePrincipalConfigured() (bool, error) {
return GetAuthMethod() == ServicePrincipalAuth, nil
}
77 changes: 66 additions & 11 deletions pkg/azure/credential/ucpcredentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ package credential
import (
"context"
"errors"
"fmt"
"sync"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
sdk_cred "github.com/radius-project/radius/pkg/ucp/credentials"
"github.com/radius-project/radius/pkg/ucp/datamodel"
"github.com/radius-project/radius/pkg/ucp/ucplog"

"go.uber.org/atomic"
Expand Down Expand Up @@ -87,8 +89,6 @@ func (c *UCPCredential) refreshExpiry() {
}

func (c *UCPCredential) refreshCredentials(ctx context.Context) error {
logger := ucplog.FromContextOrDiscard(ctx)

c.tokenCredMu.Lock()
defer c.tokenCredMu.Unlock()

Expand All @@ -102,18 +102,35 @@ func (c *UCPCredential) refreshCredentials(ctx context.Context) error {
return err
}

if s.ClientID == "" || s.ClientSecret == "" || s.TenantID == "" {
return errors.New("invalid azure service principal credential info")
switch s.Kind {
case datamodel.AzureServicePrincipalCredentialKind:
return refreshAzureServicePrincipalCredentials(ctx, c, s)
case datamodel.AzureWorkloadIdentityCredentialKind:
return refreshAzureWorkloadIdentityCredentials(ctx, c, s)
default:
return fmt.Errorf("unknown Azure credential kind, expected ServicePrincipal or WorkloadIdentity (got %s)", s.Kind)
}
}

func refreshAzureServicePrincipalCredentials(ctx context.Context, c *UCPCredential, s *sdk_cred.AzureCredential) error {
logger := ucplog.FromContextOrDiscard(ctx)

azureServicePrincipalCredential := s.ServicePrincipal
if azureServicePrincipalCredential.ClientID == "" || azureServicePrincipalCredential.ClientSecret == "" || azureServicePrincipalCredential.TenantID == "" {
return errors.New("Client ID, Tenant ID, or Client Secret can't be empty")
}

// Do not instantiate new client unless the secret is rotated.
if c.credential != nil && c.credential.ClientSecret == s.ClientSecret &&
c.credential.ClientID == s.ClientID && c.credential.TenantID == s.TenantID {
if c.credential != nil &&
c.credential.ServicePrincipal != nil &&
c.credential.ServicePrincipal.ClientSecret == azureServicePrincipalCredential.ClientSecret &&
c.credential.ServicePrincipal.ClientID == azureServicePrincipalCredential.ClientID &&
c.credential.ServicePrincipal.TenantID == azureServicePrincipalCredential.TenantID {
c.refreshExpiry()
return nil
}

logger.Info("Retreived Azure Credential - ClientID: " + s.ClientID)
logger.Info("Retrieved Azure Credential - ClientID: " + azureServicePrincipalCredential.ClientID)

// Rotate credentials by creating new ClientSecretCredential.
var opt *azidentity.ClientSecretCredentialOptions
Expand All @@ -123,7 +140,45 @@ func (c *UCPCredential) refreshCredentials(ctx context.Context) error {
}
}

azCred, err := azidentity.NewClientSecretCredential(s.TenantID, s.ClientID, s.ClientSecret, opt)
azCred, err := azidentity.NewClientSecretCredential(azureServicePrincipalCredential.TenantID, azureServicePrincipalCredential.ClientID, azureServicePrincipalCredential.ClientSecret, opt)
if err != nil {
return err
}

c.tokenCred = azCred
c.credential = s

c.refreshExpiry()
return nil
}

func refreshAzureWorkloadIdentityCredentials(ctx context.Context, c *UCPCredential, s *sdk_cred.AzureCredential) error {
logger := ucplog.FromContextOrDiscard(ctx)

azureWorkloadIdentityCredential := s.WorkloadIdentity
if azureWorkloadIdentityCredential.ClientID == "" || azureWorkloadIdentityCredential.TenantID == "" {
return errors.New("empty clientID or tenantID provided for Azure workload identity")
}

// Do not instantiate new client unless clientId and tenantId are changed.
if c.credential != nil &&
c.credential.WorkloadIdentity != nil &&
c.credential.WorkloadIdentity.ClientID == azureWorkloadIdentityCredential.ClientID &&
c.credential.WorkloadIdentity.TenantID == azureWorkloadIdentityCredential.TenantID {
c.refreshExpiry()
return nil
}

logger.Info("Retrieved Azure Credential - ClientID: " + azureWorkloadIdentityCredential.ClientID)

var opt *azidentity.DefaultAzureCredentialOptions
if c.options.ClientOptions != nil {
opt = &azidentity.DefaultAzureCredentialOptions{
ClientOptions: *c.options.ClientOptions,
}
}

azCred, err := azidentity.NewDefaultAzureCredential(opt)
if err != nil {
return err
}
Expand All @@ -135,15 +190,15 @@ func (c *UCPCredential) refreshCredentials(ctx context.Context) error {
return nil
}

// GetToken attempts to refresh the Azure service principal credential if it is expired and then returns an
// GetToken attempts to refresh the Azure credential if it is expired and then returns an
// access token if the credential is ready. This method is called automatically by Azure SDK clients.
func (c *UCPCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
logger := ucplog.FromContextOrDiscard(ctx)

if c.isExpired() {
err := c.refreshCredentials(ctx)
if err != nil {
logger.Error(err, "failed to refresh Azure service principal credential.")
logger.Error(err, "failed to refresh Azure credential.")
}
}

Expand All @@ -152,7 +207,7 @@ func (c *UCPCredential) GetToken(ctx context.Context, opts policy.TokenRequestOp
c.tokenCredMu.RUnlock()

if credentialAuth == nil {
return azcore.AccessToken{}, errors.New("azure service principal credential is not ready")
return azcore.AccessToken{}, errors.New("azure credential is not ready")
}

return credentialAuth.GetToken(ctx, opts)
Expand Down
88 changes: 77 additions & 11 deletions pkg/azure/credential/ucpcredentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,39 +38,105 @@ func (p *mockProvider) Fetch(ctx context.Context, planeName, name string) (*sdk_
return p.fakeCredential, nil
}

func newMockProvider() *mockProvider {
func newServicePrincipalMockProvider() *mockProvider {
return &mockProvider{
fakeCredential: &sdk_cred.AzureCredential{
ClientID: "fakeid",
TenantID: "fakeid",
ClientSecret: "fakeSecret",
Kind: sdk_cred.AzureServicePrincipalCredentialKind,
ServicePrincipal: &sdk_cred.AzureServicePrincipalCredential{
ClientID: "fakeid",
TenantID: "fakeid",
ClientSecret: "fakeSecret",
},
},
}
}

func TestNewUCPCredential(t *testing.T) {
func newWorkloadIdentityMockProvider() *mockProvider {
return &mockProvider{
fakeCredential: &sdk_cred.AzureCredential{
Kind: sdk_cred.AzureWorkloadIdentityCredentialKind,
WorkloadIdentity: &sdk_cred.AzureWorkloadIdentityCredential{
ClientID: "fakeid",
TenantID: "fakeid",
},
},
}
}

func Test_NewUCPCredential_AzureServicePrincipal(t *testing.T) {
_, err := NewUCPCredential(UCPCredentialOptions{})
require.Error(t, err)

c, err := NewUCPCredential(UCPCredentialOptions{Provider: newMockProvider()})
c, err := NewUCPCredential(UCPCredentialOptions{Provider: newServicePrincipalMockProvider()})
require.NoError(t, err)
require.Equal(t, DefaultExpireDuration, c.options.Duration)
require.True(t, c.isExpired())
}

func TestRefreshCredentials(t *testing.T) {
func Test_NewUCPCredential_WorkloadIdentity(t *testing.T) {
_, err := NewUCPCredential(UCPCredentialOptions{})
require.Error(t, err)

c, err := NewUCPCredential(UCPCredentialOptions{Provider: newWorkloadIdentityMockProvider()})
require.NoError(t, err)
require.Equal(t, DefaultExpireDuration, c.options.Duration)
require.True(t, c.isExpired())
}

func Test_RefreshCredentials_ServicePrincipal(t *testing.T) {
t.Run("invalid credential", func(t *testing.T) {
p := newServicePrincipalMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)
p.fakeCredential.ServicePrincipal.ClientID = ""

err = c.refreshCredentials(context.TODO())
require.Error(t, err)
})

t.Run("do not refresh credential", func(t *testing.T) {
p := newServicePrincipalMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)

err = c.refreshCredentials(context.TODO())
require.NoError(t, err)
require.False(t, c.isExpired())
})

t.Run("same credentials", func(t *testing.T) {
p := newServicePrincipalMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)

err = c.refreshCredentials(context.TODO())
require.NoError(t, err)

// reset next refresh time.
c.nextExpiry.Store(0)
require.True(t, c.isExpired())
old := c.tokenCred

err = c.refreshCredentials(context.TODO())
require.NoError(t, err)
require.False(t, c.isExpired())
require.Equal(t, old, c.tokenCred)
})
}

func Test_RefreshCredentials_WorkloadIdentity(t *testing.T) {
t.Run("invalid credential", func(t *testing.T) {
p := newMockProvider()
p := newWorkloadIdentityMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)
p.fakeCredential.ClientID = ""
p.fakeCredential.WorkloadIdentity.ClientID = ""

err = c.refreshCredentials(context.TODO())
require.Error(t, err)
})

t.Run("do not refresh credential", func(t *testing.T) {
p := newMockProvider()
p := newWorkloadIdentityMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)

Expand All @@ -80,7 +146,7 @@ func TestRefreshCredentials(t *testing.T) {
})

t.Run("same credentials", func(t *testing.T) {
p := newMockProvider()
p := newWorkloadIdentityMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)

Expand Down
Loading

0 comments on commit b748443

Please sign in to comment.