From 661f084ffa8b6e5117ce1743c0262ed5c507fbcb Mon Sep 17 00:00:00 2001 From: 3u13r Date: Fri, 26 May 2023 11:45:03 +0200 Subject: [PATCH] cli: use uami for in-cluter authentication (#1820) --- cli/internal/cloudcmd/iam.go | 8 ++--- cli/internal/cloudcmd/iam_test.go | 16 ++++------ cli/internal/cmd/iamcreate.go | 4 --- cli/internal/cmd/iamcreate_test.go | 10 ++---- cli/internal/cmd/init.go | 18 +++++++---- cli/internal/iamid/id.go | 8 ++--- cli/internal/terraform/terraform.go | 32 ++++--------------- .../terraform/terraform/iam/azure/main.tf | 28 ---------------- .../terraform/terraform/iam/azure/outputs.tf | 9 ------ cli/internal/terraform/terraform_test.go | 18 ++++------- docs/docs/reference/config-migration.md | 15 +++++++-- internal/config/config.go | 12 +++++-- internal/config/config_test.go | 4 +-- 13 files changed, 65 insertions(+), 117 deletions(-) diff --git a/cli/internal/cloudcmd/iam.go b/cli/internal/cloudcmd/iam.go index cf3a9afde..c24212257 100644 --- a/cli/internal/cloudcmd/iam.go +++ b/cli/internal/cloudcmd/iam.go @@ -201,11 +201,9 @@ func (c *IAMCreator) createAzure(ctx context.Context, cl terraformClient, opts * return iamid.File{ CloudProvider: cloudprovider.Azure, AzureOutput: iamid.AzureFile{ - ApplicationID: iamOutput.Azure.ApplicationID, - ApplicationClientSecretValue: iamOutput.Azure.ApplicationClientSecretValue, - SubscriptionID: iamOutput.Azure.SubscriptionID, - TenantID: iamOutput.Azure.TenantID, - UAMIID: iamOutput.Azure.UAMIID, + SubscriptionID: iamOutput.Azure.SubscriptionID, + TenantID: iamOutput.Azure.TenantID, + UAMIID: iamOutput.Azure.UAMIID, }, }, nil } diff --git a/cli/internal/cloudcmd/iam_test.go b/cli/internal/cloudcmd/iam_test.go index 0a5e37113..1ff6ea9a5 100644 --- a/cli/internal/cloudcmd/iam_test.go +++ b/cli/internal/cloudcmd/iam_test.go @@ -50,21 +50,17 @@ func TestIAMCreator(t *testing.T) { } validAzureIAMOutput := terraform.IAMOutput{ Azure: terraform.AzureIAMOutput{ - SubscriptionID: "test_subscription_id", - TenantID: "test_tenant_id", - ApplicationID: "test_application_id", - ApplicationClientSecretValue: "test_application_client_secret_value", - UAMIID: "test_uami_id", + SubscriptionID: "test_subscription_id", + TenantID: "test_tenant_id", + UAMIID: "test_uami_id", }, } validAzureIAMIDFile := iamid.File{ CloudProvider: cloudprovider.Azure, AzureOutput: iamid.AzureFile{ - SubscriptionID: "test_subscription_id", - TenantID: "test_tenant_id", - ApplicationID: "test_application_id", - ApplicationClientSecretValue: "test_application_client_secret_value", - UAMIID: "test_uami_id", + SubscriptionID: "test_subscription_id", + TenantID: "test_tenant_id", + UAMIID: "test_uami_id", }, } diff --git a/cli/internal/cmd/iamcreate.go b/cli/internal/cmd/iamcreate.go index fe3d1b04c..85ea6e5a6 100644 --- a/cli/internal/cmd/iamcreate.go +++ b/cli/internal/cmd/iamcreate.go @@ -486,8 +486,6 @@ func (c *azureIAMCreator) printOutputValues(cmd *cobra.Command, flags iamFlags, cmd.Printf("location:\t\t%s\n", flags.azure.region) cmd.Printf("resourceGroup:\t\t%s\n", flags.azure.resourceGroup) cmd.Printf("userAssignedIdentity:\t%s\n", iamFile.AzureOutput.UAMIID) - cmd.Printf("appClientID:\t\t%s\n", iamFile.AzureOutput.ApplicationID) - cmd.Printf("clientSecretValue:\t%s\n\n", iamFile.AzureOutput.ApplicationClientSecretValue) } func (c *azureIAMCreator) writeOutputValuesToConfig(conf *config.Config, flags iamFlags, iamFile iamid.File) { @@ -496,8 +494,6 @@ func (c *azureIAMCreator) writeOutputValuesToConfig(conf *config.Config, flags i conf.Provider.Azure.Location = flags.azure.region conf.Provider.Azure.ResourceGroup = flags.azure.resourceGroup conf.Provider.Azure.UserAssignedIdentity = iamFile.AzureOutput.UAMIID - conf.Provider.Azure.AppClientID = iamFile.AzureOutput.ApplicationID - conf.Provider.Azure.ClientSecretValue = iamFile.AzureOutput.ApplicationClientSecretValue } func (c *azureIAMCreator) parseAndWriteIDFile(_ iamid.File, _ file.Handler) error { diff --git a/cli/internal/cmd/iamcreate_test.go b/cli/internal/cmd/iamcreate_test.go index 429952b20..5e6c64d8b 100644 --- a/cli/internal/cmd/iamcreate_test.go +++ b/cli/internal/cmd/iamcreate_test.go @@ -358,11 +358,9 @@ func TestIAMCreateAzure(t *testing.T) { validIAMIDFile := iamid.File{ CloudProvider: cloudprovider.Azure, AzureOutput: iamid.AzureFile{ - SubscriptionID: "test_subscription_id", - TenantID: "test_tenant_id", - ApplicationID: "test_application_id", - ApplicationClientSecretValue: "test_application_client_secret_value", - UAMIID: "test_uami_id", + SubscriptionID: "test_subscription_id", + TenantID: "test_tenant_id", + UAMIID: "test_uami_id", }, } @@ -611,8 +609,6 @@ func TestIAMCreateAzure(t *testing.T) { require.NoError(readErr) assert.Equal(tc.creator.id.AzureOutput.SubscriptionID, readConfig.Provider.Azure.SubscriptionID) assert.Equal(tc.creator.id.AzureOutput.TenantID, readConfig.Provider.Azure.TenantID) - assert.Equal(tc.creator.id.AzureOutput.ApplicationID, readConfig.Provider.Azure.AppClientID) - assert.Equal(tc.creator.id.AzureOutput.ApplicationClientSecretValue, readConfig.Provider.Azure.ClientSecretValue) assert.Equal(tc.creator.id.AzureOutput.UAMIID, readConfig.Provider.Azure.UserAssignedIdentity) assert.Equal(tc.regionFlag, readConfig.Provider.Azure.Location) assert.Equal(tc.resourceGroupFlag, readConfig.Provider.Azure.ResourceGroup) diff --git a/cli/internal/cmd/init.go b/cli/internal/cmd/init.go index 161aa9da6..2867ac5a3 100644 --- a/cli/internal/cmd/init.go +++ b/cli/internal/cmd/init.go @@ -449,13 +449,19 @@ func (i *initCmd) getMarshaledServiceAccountURI(provider cloudprovider.Provider, return "", nil // AWS does not need a service account URI case cloudprovider.Azure: i.log.Debugf("Handling case for Azure") + + // TODO(3u13r): Remove this fallback and enforce assigned managed identity after the v2.8.0 but before the v2.9.0 release. + authMethod := azureshared.AuthMethodUserAssignedIdentity + if config.Provider.Azure.AppClientID != "" { + authMethod = azureshared.AuthMethodServicePrincipal + } + creds := azureshared.ApplicationCredentials{ - TenantID: config.Provider.Azure.TenantID, - AppClientID: config.Provider.Azure.AppClientID, - ClientSecretValue: config.Provider.Azure.ClientSecretValue, - Location: config.Provider.Azure.Location, - // TODO(malt3): Switch preferred auth method to uami as planned by AB#2961 - PreferredAuthMethod: azureshared.AuthMethodServicePrincipal, + TenantID: config.Provider.Azure.TenantID, + AppClientID: config.Provider.Azure.AppClientID, + ClientSecretValue: config.Provider.Azure.ClientSecretValue, + Location: config.Provider.Azure.Location, + PreferredAuthMethod: authMethod, UamiResourceID: config.Provider.Azure.UserAssignedIdentity, } return creds.ToCloudServiceAccountURI(), nil diff --git a/cli/internal/iamid/id.go b/cli/internal/iamid/id.go index 0a9af666e..d5f5a274d 100644 --- a/cli/internal/iamid/id.go +++ b/cli/internal/iamid/id.go @@ -31,11 +31,9 @@ type GCPFile struct { // AzureFile contains the output information of a Microsoft Azure IAM configuration. type AzureFile struct { - SubscriptionID string `json:"subscriptionID,omitempty"` - TenantID string `json:"tenantID,omitempty"` - ApplicationID string `json:"applicationID,omitempty"` - UAMIID string `json:"uamiID,omitempty"` - ApplicationClientSecretValue string `json:"applicationClientSecretValue,omitempty"` + SubscriptionID string `json:"subscriptionID,omitempty"` + TenantID string `json:"tenantID,omitempty"` + UAMIID string `json:"uamiID,omitempty"` } // AWSFile contains the output information of an AWS IAM configuration. diff --git a/cli/internal/terraform/terraform.go b/cli/internal/terraform/terraform.go index 75f1081d5..691d9e5a3 100644 --- a/cli/internal/terraform/terraform.go +++ b/cli/internal/terraform/terraform.go @@ -185,11 +185,9 @@ type GCPIAMOutput struct { // AzureIAMOutput contains the output information of the Terraform IAM operation on Microsoft Azure. type AzureIAMOutput struct { - SubscriptionID string - TenantID string - ApplicationID string - UAMIID string - ApplicationClientSecretValue string + SubscriptionID string + TenantID string + UAMIID string } // AWSIAMOutput contains the output information of the Terraform IAM operation on GCP. @@ -249,14 +247,6 @@ func (c *Client) CreateIAMConfig(ctx context.Context, provider cloudprovider.Pro if !ok { return IAMOutput{}, errors.New("invalid type in tenant id output: not a string") } - applicationIDRaw, ok := tfState.Values.Outputs["application_id"] - if !ok { - return IAMOutput{}, errors.New("no application id output found") - } - applicationIDOutput, ok := applicationIDRaw.Value.(string) - if !ok { - return IAMOutput{}, errors.New("invalid type in application id output: not a string") - } uamiIDRaw, ok := tfState.Values.Outputs["uami_id"] if !ok { return IAMOutput{}, errors.New("no UAMI id output found") @@ -265,21 +255,11 @@ func (c *Client) CreateIAMConfig(ctx context.Context, provider cloudprovider.Pro if !ok { return IAMOutput{}, errors.New("invalid type in UAMI id output: not a string") } - appClientSecretRaw, ok := tfState.Values.Outputs["application_client_secret_value"] - if !ok { - return IAMOutput{}, errors.New("no application client secret value output found") - } - appClientSecretOutput, ok := appClientSecretRaw.Value.(string) - if !ok { - return IAMOutput{}, errors.New("invalid type in application client secret valueoutput: not a string") - } return IAMOutput{ Azure: AzureIAMOutput{ - SubscriptionID: subscriptionIDOutput, - TenantID: tenantIDOutput, - ApplicationID: applicationIDOutput, - UAMIID: uamiIDOutput, - ApplicationClientSecretValue: appClientSecretOutput, + SubscriptionID: subscriptionIDOutput, + TenantID: tenantIDOutput, + UAMIID: uamiIDOutput, }, }, nil case cloudprovider.AWS: diff --git a/cli/internal/terraform/terraform/iam/azure/main.tf b/cli/internal/terraform/terraform/iam/azure/main.tf index aa28bf298..361466c86 100644 --- a/cli/internal/terraform/terraform/iam/azure/main.tf +++ b/cli/internal/terraform/terraform/iam/azure/main.tf @@ -68,31 +68,3 @@ resource "azurerm_role_assignment" "uami_owner_role" { role_definition_name = "Owner" principal_id = azurerm_user_assigned_identity.identity_uami.principal_id } - -# the app registration, application secrets -# and role assignments below will be removed in the future -# TODO(malt3): remove app registration as planned by AB#2961 - -# Create application registration -resource "azuread_application" "base_application" { - display_name = "${var.resource_group_name}-application" - owners = [data.azuread_client_config.current.object_id] -} - -resource "azuread_service_principal" "application_principal" { - application_id = azuread_application.base_application.application_id - app_role_assignment_required = false - owners = [data.azuread_client_config.current.object_id] -} - -# Set identity as base resource group owner -resource "azurerm_role_assignment" "app_registration_owner_role" { - scope = azurerm_resource_group.base_resource_group.id - role_definition_name = "Owner" - principal_id = azuread_service_principal.application_principal.object_id -} - -# Create application secret (password) -resource "azuread_application_password" "base_application_secret" { - application_object_id = azuread_application.base_application.object_id -} diff --git a/cli/internal/terraform/terraform/iam/azure/outputs.tf b/cli/internal/terraform/terraform/iam/azure/outputs.tf index cb269a755..7e13fabb0 100644 --- a/cli/internal/terraform/terraform/iam/azure/outputs.tf +++ b/cli/internal/terraform/terraform/iam/azure/outputs.tf @@ -6,15 +6,6 @@ output "tenant_id" { value = data.azurerm_subscription.current.tenant_id } -output "application_id" { - value = azuread_application.base_application.application_id -} - output "uami_id" { value = azurerm_user_assigned_identity.identity_uami.id } - -output "application_client_secret_value" { - value = azuread_application_password.base_application_secret.value - sensitive = true -} diff --git a/cli/internal/terraform/terraform_test.go b/cli/internal/terraform/terraform_test.go index 6f1156360..dcdc558a7 100644 --- a/cli/internal/terraform/terraform_test.go +++ b/cli/internal/terraform/terraform_test.go @@ -116,9 +116,8 @@ func TestPrepareIAM(t *testing.T) { ServiceAccountID: "const-test-case", } azureVars := &AzureIAMVariables{ - Region: "westus", - ServicePrincipal: "constell-test-sp", - ResourceGroup: "constell-test-rg", + Region: "westus", + ResourceGroup: "constell-test-rg", } awsVars := &AWSIAMVariables{ Region: "eu-east-2a", @@ -480,9 +479,8 @@ func TestCreateIAM(t *testing.T) { ServiceAccountID: "const-test-case", } azureVars := &AzureIAMVariables{ - Region: "westus", - ServicePrincipal: "constell-test-sp", - ResourceGroup: "constell-test-rg", + Region: "westus", + ResourceGroup: "constell-test-rg", } awsVars := &AWSIAMVariables{ Region: "eu-east-2a", @@ -581,11 +579,9 @@ func TestCreateIAM(t *testing.T) { tf: &stubTerraform{showState: newTestState()}, fs: afero.NewMemMapFs(), want: IAMOutput{Azure: AzureIAMOutput{ - SubscriptionID: "test_subscription_id", - TenantID: "test_tenant_id", - ApplicationID: "test_application_id", - ApplicationClientSecretValue: "test_application_client_secret_value", - UAMIID: "test_uami_id", + SubscriptionID: "test_subscription_id", + TenantID: "test_tenant_id", + UAMIID: "test_uami_id", }}, }, "azure init fails": { diff --git a/docs/docs/reference/config-migration.md b/docs/docs/reference/config-migration.md index 5153067e0..a5cceb208 100644 --- a/docs/docs/reference/config-migration.md +++ b/docs/docs/reference/config-migration.md @@ -1,8 +1,19 @@ -# Configuration migrations +# Migrations -This document describes breaking changes in the configuration file format between Constellation releases. +This document describes breaking changes and migrations between Constellation releases. Use [`constellation config migrate`](./cli.md#constellation-config-migrate) to automatically update an old config file to a new format. +## Migrating from Azure's service principal authentication to managed identity authentication + +- The `provider.azure.appClientID` and `provider.azure.appClientSecret` fields are no longer required and should be removed. +- To keep using an existing UAMI add the `Owner` permission with the scope of your `resourceGroup`. +- Otherwise, simply [create new Constellation IAM credentials](../workflows/config.md#creating-iam-credentials) and use the created UAMI. +- To migrate the authentication for an existing Constellation on Azure to an UAMI with the necessary permissions: + 1. Remove the `aadClientId` and `aadClientSecret` from the azureconfig secret. + 2. Set `useManagedIdentityExtension` to `true` and use the `userAssignedIdentity` from the Constellation config for the value of `userAssignedIdentityID`. + 3. Restart the CSI driver, cloud controller manager, cluster autoscaler, and Constellation operator pods. + + ## Migrating from CLI versions before 2.8 - The `measurements` field for each cloud service provider was replaced with a global `attestation` field. diff --git a/internal/config/config.go b/internal/config/config.go index 5863f6d95..feb8807bc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -143,10 +143,10 @@ type AzureConfig struct { UserAssignedIdentity string `yaml:"userAssignedIdentity" validate:"required"` // description: | // Application client ID of the Active Directory app registration. - AppClientID string `yaml:"appClientID" validate:"uuid"` + AppClientID string `yaml:"appClientID,omitempty" validate:"omitempty,uuid"` // description: | // Client secret value of the Active Directory app registration credentials. Alternatively leave empty and pass value via CONSTELL_AZURE_CLIENT_SECRET_VALUE environment variable. - ClientSecretValue string `yaml:"clientSecretValue" validate:"required"` + ClientSecretValue string `yaml:"clientSecretValue,omitempty" validate:"omitempty"` // description: | // VM instance type to use for Constellation nodes. InstanceType string `yaml:"instanceType" validate:"azure_instance_type"` @@ -419,6 +419,14 @@ func NewWithClient(fileHandler file.Handler, name string, client fetcher.HTTPCli c.MicroserviceVersion = Default().MicroserviceVersion } + // TODO(3u13r): Remove this deprecation warning and enforce assigned managed identity after the v2.8.0 but before the v2.9.0 release. + if c.Provider.Azure != nil && + (c.Provider.Azure.AppClientID != "" || c.Provider.Azure.ClientSecretValue != "") { + // Deprecation warning for old auth method + fmt.Fprintf(os.Stderr, "WARNING: Using a service principal for authentication is deprecated and will be removed in an upcoming version.") + fmt.Fprintf(os.Stderr, " Migrate to using a user assigned managed identity by following the migration guide: https://docs.edgeless.systems/constellation/reference/migration.") + } + return c, c.Validate(force) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d5f811da4..5cddc7fa4 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -277,8 +277,8 @@ func TestNewWithDefaultOptions(t *testing.T) { } func TestValidate(t *testing.T) { - const defaultErrCount = 34 // expect this number of error messages by default because user-specific values are not set and multiple providers are defined by default - const azErrCount = 9 + const defaultErrCount = 32 // expect this number of error messages by default because user-specific values are not set and multiple providers are defined by default + const azErrCount = 7 const gcpErrCount = 6 // TODO(AB#3132,3u13r): refactor config validation tests