Skip to content

Commit

Permalink
Replace Azure Service Principal auth with Azure Workload Identity aut…
Browse files Browse the repository at this point in the history
…h in functional tests (#7787)

# Description

* Switch Azure SP auth to WI in functional tests
* Fix issue with Azure WI + application WI in applications-rp
* Fix issue with Azure WI in terraform
* Adding `workflow-dispatch` triggers

Example successful workflow:
https://github.com/radius-project/radius/actions/runs/10311808492

## 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 fixes a bug in Radius and has an approved issue
(issue link required).
- This pull request adds or changes features of Radius and has an
approved issue (issue link required).
- This pull request is a minor refactor, code cleanup, test improvement,
or other maintenance task and doesn't change the functionality of Radius
(issue link optional).

<!--

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

-->

Fixes: #7715

---------

Signed-off-by: willdavsmith <[email protected]>
Signed-off-by: Will Smith <[email protected]>
  • Loading branch information
willdavsmith authored Aug 13, 2024
1 parent d01ecf9 commit 9ed50f4
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 44 deletions.
20 changes: 13 additions & 7 deletions .github/workflows/functional-test-cloud.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ env:
# Azure Keyvault CSI driver chart version
AZURE_KEYVAULT_CSI_DRIVER_VER: "1.4.2"
# Azure workload identity webhook chart version
AZURE_WORKLOAD_IDENTITY_WEBHOOK_VER: "1.1.0"
AZURE_WORKLOAD_IDENTITY_WEBHOOK_VER: "1.3.0"
# Container registry for storing container images
CONTAINER_REGISTRY: ghcr.io/radius-project/dev
# Container registry for storing Bicep recipe artifacts
Expand Down Expand Up @@ -97,7 +97,7 @@ jobs:
build:
name: Build Radius for test
runs-on: ubuntu-latest
if: github.event_name == 'repository_dispatch' || (github.event_name == 'schedule' && github.repository == 'radius-project/radius') || github.event_name == 'workflow_run'
if: github.event_name == 'repository_dispatch' || (github.event_name == 'schedule' && github.repository == 'radius-project/radius') || github.event_name == 'workflow_run' || github.event_name == 'workflow_dispatch'
env:
DE_IMAGE: "ghcr.io/radius-project/deployment-engine"
DE_TAG: "latest"
Expand All @@ -119,7 +119,13 @@ jobs:
private_key: ${{ secrets.FUNCTIONAL_TEST_APP_PRIVATE_KEY }}

- name: Set up checkout target (scheduled)
if: github.event_name == 'schedule' || github.event_name == 'repository_dispatch'
if: github.event_name == 'schedule'
run: |
echo "CHECKOUT_REPO=${{ github.repository }}" >> $GITHUB_ENV
echo "CHECKOUT_REF=refs/heads/main" >> $GITHUB_ENV
- name: Set up checkout target (repository_dispatch)
if: github.event_name == 'repository_dispatch'
run: |
echo "CHECKOUT_REPO=${{ github.repository }}" >> $GITHUB_ENV
echo "CHECKOUT_REF=refs/heads/main" >> $GITHUB_ENV
Expand Down Expand Up @@ -412,7 +418,7 @@ jobs:
tests:
name: Run ${{ matrix.name }} functional tests
needs: [build]
if: github.event_name == 'repository_dispatch' || (github.event_name == 'schedule' && github.repository == 'radius-project/radius') || github.event_name == 'workflow_run'
if: github.event_name == 'repository_dispatch' || (github.event_name == 'schedule' && github.repository == 'radius-project/radius') || github.event_name == 'workflow_run' || github.event_name == 'workflow_dispatch'
strategy:
fail-fast: true
matrix:
Expand Down Expand Up @@ -616,7 +622,7 @@ jobs:
echo "*** Installing Radius to Kubernetes ***"
rad install kubernetes \
--chart ${{ env.RADIUS_CHART_LOCATION }} \
--set rp.image=${{ env.CONTAINER_REGISTRY }}/applications-rp,rp.tag=${{ env.REL_VERSION }},controller.image=${{ env.CONTAINER_REGISTRY }}/controller,controller.tag=${{ env.REL_VERSION }},ucp.image=${{ env.CONTAINER_REGISTRY }}/ucpd,ucp.tag=${{ env.REL_VERSION }},de.image=${{ env.DE_IMAGE }},de.tag=${{ env.DE_TAG }}
--set rp.image=${{ env.CONTAINER_REGISTRY }}/applications-rp,rp.tag=${{ env.REL_VERSION }},controller.image=${{ env.CONTAINER_REGISTRY }}/controller,controller.tag=${{ env.REL_VERSION }},ucp.image=${{ env.CONTAINER_REGISTRY }}/ucpd,ucp.tag=${{ env.REL_VERSION }},de.image=${{ env.DE_IMAGE }},de.tag=${{ env.DE_TAG }},global.azureWorkloadIdentity.enabled=true
echo "*** Create workspace, group and environment for test ***"
rad workspace create kubernetes
Expand All @@ -630,8 +636,8 @@ 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 sp --client-id ${{ secrets.AZURE_SP_TESTS_APPID }} \
--client-secret ${{ secrets.INTEGRATION_TEST_SP_PASSWORD }} \
rad credential register azure wi \
--client-id ${{ secrets.AZURE_SP_TESTS_APPID }} \
--tenant-id ${{ secrets.AZURE_SP_TESTS_TENANTID }}
echo "*** Configuring AWS provider ***"
Expand Down
18 changes: 15 additions & 3 deletions pkg/azure/credential/ucpcredentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ type UCPCredentialOptions struct {

// ClientOptions is the options for azure client.
ClientOptions *azcore.ClientOptions

// TokenFilePath is the path to the azure token file (for use with Azure workload identity)
TokenFilePath string
}

// UCPCredential authenticates service principal using UCP credential APIs.
Expand Down Expand Up @@ -171,14 +174,23 @@ func refreshAzureWorkloadIdentityCredentials(ctx context.Context, c *UCPCredenti

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

var opt *azidentity.DefaultAzureCredentialOptions
var opt *azidentity.WorkloadIdentityCredentialOptions
if c.options.ClientOptions != nil {
opt = &azidentity.DefaultAzureCredentialOptions{
opt = &azidentity.WorkloadIdentityCredentialOptions{
ClientID: azureWorkloadIdentityCredential.ClientID,
TenantID: azureWorkloadIdentityCredential.TenantID,
TokenFilePath: c.options.TokenFilePath,
ClientOptions: *c.options.ClientOptions,
}
} else {
opt = &azidentity.WorkloadIdentityCredentialOptions{
TokenFilePath: c.options.TokenFilePath,
ClientID: azureWorkloadIdentityCredential.ClientID,
TenantID: azureWorkloadIdentityCredential.TenantID,
}
}

azCred, err := azidentity.NewDefaultAzureCredential(opt)
azCred, err := azidentity.NewWorkloadIdentityCredential(opt)
if err != nil {
return err
}
Expand Down
24 changes: 12 additions & 12 deletions pkg/azure/credential/ucpcredentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ func newServicePrincipalMockProvider() *mockProvider {
fakeCredential: &sdk_cred.AzureCredential{
Kind: sdk_cred.AzureServicePrincipalCredentialKind,
ServicePrincipal: &sdk_cred.AzureServicePrincipalCredential{
ClientID: "fakeid",
TenantID: "fakeid",
ClientID: "fakeClientID",
TenantID: "fakeTenantID",
ClientSecret: "fakeSecret",
},
},
Expand All @@ -56,8 +56,8 @@ func newWorkloadIdentityMockProvider() *mockProvider {
fakeCredential: &sdk_cred.AzureCredential{
Kind: sdk_cred.AzureWorkloadIdentityCredentialKind,
WorkloadIdentity: &sdk_cred.AzureWorkloadIdentityCredential{
ClientID: "fakeid",
TenantID: "fakeid",
ClientID: "fakeClientID",
TenantID: "fakeTenantID",
},
},
}
Expand All @@ -84,7 +84,7 @@ func Test_NewUCPCredential_WorkloadIdentity(t *testing.T) {
}

func Test_RefreshCredentials_ServicePrincipal(t *testing.T) {
t.Run("invalid credential", func(t *testing.T) {
t.Run("invalid service principal credential", func(t *testing.T) {
p := newServicePrincipalMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)
Expand All @@ -94,7 +94,7 @@ func Test_RefreshCredentials_ServicePrincipal(t *testing.T) {
require.Error(t, err)
})

t.Run("do not refresh credential", func(t *testing.T) {
t.Run("do not refresh service principal credential", func(t *testing.T) {
p := newServicePrincipalMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)
Expand All @@ -104,7 +104,7 @@ func Test_RefreshCredentials_ServicePrincipal(t *testing.T) {
require.False(t, c.isExpired())
})

t.Run("same credentials", func(t *testing.T) {
t.Run("same service principal credentials", func(t *testing.T) {
p := newServicePrincipalMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)
Expand All @@ -125,7 +125,7 @@ func Test_RefreshCredentials_ServicePrincipal(t *testing.T) {
}

func Test_RefreshCredentials_WorkloadIdentity(t *testing.T) {
t.Run("invalid credential", func(t *testing.T) {
t.Run("invalid workload identity credential", func(t *testing.T) {
p := newWorkloadIdentityMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
require.NoError(t, err)
Expand All @@ -135,19 +135,19 @@ func Test_RefreshCredentials_WorkloadIdentity(t *testing.T) {
require.Error(t, err)
})

t.Run("do not refresh credential", func(t *testing.T) {
t.Run("do not refresh workload identity credential", func(t *testing.T) {
p := newWorkloadIdentityMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p, TokenFilePath: "/var/run/secrets/azure/tokens/azure-identity-token"})
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) {
t.Run("same workload identity credentials", func(t *testing.T) {
p := newWorkloadIdentityMockProvider()
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p})
c, err := NewUCPCredential(UCPCredentialOptions{Provider: p, TokenFilePath: "/var/run/secrets/azure/tokens/azure-identity-token"})
require.NoError(t, err)

err = c.refreshCredentials(context.TODO())
Expand Down
4 changes: 2 additions & 2 deletions pkg/corerp/handlers/azure_userassigned_managedidentity.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (handler *azureUserAssignedManagedIdentityHandler) Put(ctx context.Context,

identity, err := msiClient.CreateOrUpdate(ctx, rgName, identityName, armmsi.Identity{Location: &resourceLocation}, nil)
if err != nil {
return properties, fmt.Errorf("failed to create user assigned managed identity: %w", err)
return properties, fmt.Errorf("failed to create user-assigned managed identity: %w", err)
}

properties[UserAssignedIdentityIDKey] = to.String(identity.ID)
Expand All @@ -111,7 +111,7 @@ func (handler *azureUserAssignedManagedIdentityHandler) Put(ctx context.Context,
}

options.Resource.ID = id
logger.Info("Created managed identity for KeyVault access", logging.LogFieldLocalID, rpv1.LocalIDUserAssignedManagedIdentity)
logger.Info("Created user-assigned managed identity", logging.LogFieldLocalID, rpv1.LocalIDUserAssignedManagedIdentity)

return properties, nil
}
Expand Down
30 changes: 19 additions & 11 deletions pkg/recipes/terraform/config/providers/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,20 @@ import (
const (
AzureProviderName = "azurerm"

azureFeaturesParam = "features"
azureSubIDParam = "subscription_id"
azureClientIDParam = "client_id"
azureClientSecretParam = "client_secret"
azureTenantIDParam = "tenant_id"
azureUseAKSWorkloadIdentityParam = "use_aks_workload_identity"
azureUseCLIParam = "use_cli"
azureFeaturesParam = "features"
azureSubIDParam = "subscription_id"
azureClientIDParam = "client_id"
azureClientSecretParam = "client_secret"
azureTenantIDParam = "tenant_id"
azureUseOIDCParam = "use_oidc"
azureUseCLIParam = "use_cli"
azureOIDCTokenFilePathParam = "oidc_token_file_path"

// The Azure AD Workload Identity Mutating Admission Webhook projects a signed service account token to
// this well known path.
// https://azure.github.io/azure-workload-identity/docs/installation/mutating-admission-webhook.html
// https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs#argument-reference
azureOIDCTokenFilePath = "/var/run/secrets/azure/tokens/azure-identity-token"
)

var _ Provider = (*azureProvider)(nil)
Expand Down Expand Up @@ -175,13 +182,14 @@ func (p *azureProvider) generateProviderConfigMap(configMap map[string]any, cred
if credentials.WorkloadIdentity != nil &&
credentials.WorkloadIdentity.ClientID != "" &&
credentials.WorkloadIdentity.TenantID != "" {

// Use OIDC for Workload Identity
// https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/guides/service_principal_oidc
configMap[azureClientIDParam] = credentials.WorkloadIdentity.ClientID
configMap[azureTenantIDParam] = credentials.WorkloadIdentity.TenantID

// Use AKS Workload Identity for Azure provider
// https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/guides/aks_workload_identity#configuring-with-environment-variables
configMap[azureUseAKSWorkloadIdentityParam] = true
configMap[azureUseCLIParam] = false
configMap[azureUseOIDCParam] = true
configMap[azureOIDCTokenFilePathParam] = azureOIDCTokenFilePath
}
}

Expand Down
16 changes: 10 additions & 6 deletions pkg/recipes/terraform/config/providers/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,13 @@ func TestAzureProvider_generateProviderConfigMap(t *testing.T) {
subscription: testSubscription,
credentials: testAzureWorkloadIdentityCredential,
expectedConfig: map[string]any{
azureFeaturesParam: map[string]any{},
azureSubIDParam: testSubscription,
azureTenantIDParam: testAzureWorkloadIdentityCredential.WorkloadIdentity.TenantID,
azureClientIDParam: testAzureWorkloadIdentityCredential.WorkloadIdentity.ClientID,
azureUseAKSWorkloadIdentityParam: true,
azureUseCLIParam: false,
azureFeaturesParam: map[string]any{},
azureSubIDParam: testSubscription,
azureTenantIDParam: testAzureWorkloadIdentityCredential.WorkloadIdentity.TenantID,
azureClientIDParam: testAzureWorkloadIdentityCredential.WorkloadIdentity.ClientID,
azureUseCLIParam: false,
azureUseOIDCParam: true,
azureOIDCTokenFilePathParam: "/var/run/secrets/azure/tokens/azure-identity-token",
},
},
{
Expand Down Expand Up @@ -369,6 +370,9 @@ func TestAzureProvider_generateProviderConfigMap(t *testing.T) {
require.Equal(t, tt.expectedConfig[azureClientIDParam], config[azureClientIDParam])
require.Equal(t, tt.expectedConfig[azureClientSecretParam], config[azureClientSecretParam])
require.Equal(t, tt.expectedConfig[azureTenantIDParam], config[azureTenantIDParam])
require.Equal(t, tt.expectedConfig[azureUseOIDCParam], config[azureUseOIDCParam])
require.Equal(t, tt.expectedConfig[azureUseCLIParam], config[azureUseCLIParam])
require.Equal(t, tt.expectedConfig[azureOIDCTokenFilePathParam], config[azureOIDCTokenFilePathParam])
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"github.com/radius-project/radius/test/validation"
)

// Test_Storage tests if a container can be created and then deleted by the magpiego with the workload identity.
// Test_Storage tests if a container on an Azure Storage Account can be created and then deleted by the magpiego with the workload identity.
func Test_Storage(t *testing.T) {
template := "testdata/corerp-resources-container-workload.bicep"
name := "corerp-resources-container-workload"
Expand Down
2 changes: 1 addition & 1 deletion test/magpiego/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/radius-project/radius/test/magpiego

go 1.22
go 1.22.0

require (
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ terraform {
required_providers {
azurerm = {
source = "hashicorp/azurerm"
version = "~> 3.0.0"
version = "~> 3.114.0"
}
}
}
Expand Down

0 comments on commit 9ed50f4

Please sign in to comment.