Skip to content

Commit

Permalink
update code
Browse files Browse the repository at this point in the history
  • Loading branch information
@sinbai
sinbai committed Dec 18, 2024
1 parent f446b44 commit fdac89b
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 18 deletions.
40 changes: 27 additions & 13 deletions internal/services/mssqlmanagedinstance/mssql_managed_instance_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ type AzureActiveDirectoryAdministrator struct {
ObjectID string `tfschema:"object_id"`
AzureADAuthenticationOnlyEnabled bool `tfschema:"azuread_authentication_only_enabled"`
TenantID string `tfschema:"tenant_id"`
PrincipalType string `tfschema:"principal_type"`
}

var (
Expand Down Expand Up @@ -200,6 +201,12 @@ func (r MsSqlManagedInstanceResource) Arguments() map[string]*pluginsdk.Schema {
ValidateFunc: validation.IsUUID,
},

"principal_type": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice(managedinstances.PossibleValuesForPrincipalType(), false),
},

"azuread_authentication_only_enabled": {
Type: pluginsdk.TypeBool,
Optional: true,
Expand Down Expand Up @@ -406,7 +413,8 @@ func (r MsSqlManagedInstanceResource) Create() sdk.ResourceFunc {
TimezoneId: pointer.To(model.TimezoneId),
VCores: pointer.To(model.VCores),
ZoneRedundant: pointer.To(model.ZoneRedundantEnabled),
Administrators: expandMsSqlManagedInstanceExternalAdministrators(model.AzureActiveDirectoryAdministrator),
// `Administrators` is only valid when specified during creation`
Administrators: expandMsSqlManagedInstanceExternalAdministrators(model.AzureActiveDirectoryAdministrator),
},
Tags: pointer.To(model.Tags),
}
Expand Down Expand Up @@ -450,17 +458,17 @@ func (r MsSqlManagedInstanceResource) Update() sdk.ResourceFunc {
return err
}

metadata.Logger.Infof("Decoding state for %s", id)
metadata.Logger.Infof("Decoding state for %s", *id)
var state MsSqlManagedInstanceModel
if err := metadata.Decode(&state); err != nil {
return err
}

metadata.Logger.Infof("Updating %s", id)
metadata.Logger.Infof("Updating %s", *id)

sku, err := r.expandSkuName(state.SkuName)
if err != nil {
return fmt.Errorf("expanding `sku_name` for SQL Managed Instance Server %q: %v", id.ID(), err)
return fmt.Errorf("expanding `sku_name` for SQL Managed Instance Server %q: %v", *id, err)
}

properties := managedinstances.ManagedInstance{
Expand Down Expand Up @@ -522,9 +530,16 @@ func (r MsSqlManagedInstanceResource) Update() sdk.ResourceFunc {
}

if aadAdminExists {
if err := azureADAuthenticationOnlyClient.DeleteThenPoll(ctx, *id); err != nil {
log.Printf("[INFO] Deletion of AAD Authentication Only failed for %s: %+v", *id, err)
return fmt.Errorf("disabling AAD Authentication Only for %s: %+v", *id, err)
// Before deleting an AAD admin, it is necessary to disable `AzureADOnlyAuthentication` first, as deleting an AAD admin when `AzureADOnlyAuthentication` feature is enabled is not supported.
// Use `CreateOrUpdateThenPoll` instead of `DeleteThenPoll`, because the actual deletion behavior of the API is not to really delete the record, but to update `AzureADOnlyAuthentication` to false. Therefore, using `DeleteThenPoll` will cause pull till done to never end until it times out.
aadAuthOnlyParams := managedinstanceazureadonlyauthentications.ManagedInstanceAzureADOnlyAuthentication{
Properties: &managedinstanceazureadonlyauthentications.ManagedInstanceAzureADOnlyAuthProperties{
AzureADOnlyAuthentication: false,
},
}
err = azureADAuthenticationOnlyClient.CreateOrUpdateThenPoll(ctx, *id, aadAuthOnlyParams)
if err != nil {
return fmt.Errorf("disabling `azuread_authentication_only` for %s: %+v", *id, err)
}

if err := adminClient.DeleteThenPoll(ctx, *id); err != nil {
Expand All @@ -551,7 +566,6 @@ func (r MsSqlManagedInstanceResource) Update() sdk.ResourceFunc {
return fmt.Errorf("setting `azuread_authentication_only_enabled` for %s: %+v", *id, err)
}
}
properties.Properties.Administrators = expandMsSqlManagedInstanceExternalAdministrators(state.AzureActiveDirectoryAdministrator)
}

metadata.Logger.Infof("Updating %s", *id)
Expand Down Expand Up @@ -805,17 +819,17 @@ func expandMsSqlManagedInstanceExternalAdministrators(input []AzureActiveDirecto

admin := input[0]
adminParams := managedinstances.ManagedInstanceExternalAdministrator{
AdministratorType: pointer.To(managedinstances.AdministratorTypeActiveDirectory),
Login: pointer.To(admin.LoginUserName),
Sid: pointer.To(admin.ObjectID),
AdministratorType: pointer.To(managedinstances.AdministratorTypeActiveDirectory),
PrincipalType: pointer.To(managedinstances.PrincipalType(admin.PrincipalType)),
Login: pointer.To(admin.LoginUserName),
Sid: pointer.To(admin.ObjectID),
AzureADOnlyAuthentication: pointer.To(admin.AzureADAuthenticationOnlyEnabled),
}

if admin.TenantID != "" {
adminParams.TenantId = pointer.To(admin.TenantID)
}

adminParams.AzureADOnlyAuthentication = pointer.To(admin.AzureADAuthenticationOnlyEnabled)

return &adminParams
}

Expand Down
49 changes: 44 additions & 5 deletions internal/services/mssqlmanagedinstance/mssql_managed_instance_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,16 @@ func TestAccMsSqlManagedInstance_aadAdmin(t *testing.T) {
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep("administrator_login_password"),
data.ImportStep(
"administrator_login_password",
"azure_active_directory_administrator.#",
"azure_active_directory_administrator.0.%",
"azure_active_directory_administrator.0.azuread_authentication_only_enabled",
"azure_active_directory_administrator.0.login_username",
"azure_active_directory_administrator.0.object_id",
"azure_active_directory_administrator.0.principal_type",
"azure_active_directory_administrator.0.tenant_id",
),
})
}

Expand All @@ -918,7 +927,16 @@ func TestAccMsSqlManagedInstance_aadAdminWithAadOnly(t *testing.T) {
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep("administrator_login_password"),
data.ImportStep(
"administrator_login_password",
"azure_active_directory_administrator.#",
"azure_active_directory_administrator.0.%",
"azure_active_directory_administrator.0.azuread_authentication_only_enabled",
"azure_active_directory_administrator.0.login_username",
"azure_active_directory_administrator.0.object_id",
"azure_active_directory_administrator.0.principal_type",
"azure_active_directory_administrator.0.tenant_id",
),
})
}

Expand Down Expand Up @@ -947,7 +965,16 @@ func TestAccMsSqlManagedInstance_aadAdminUpdate(t *testing.T) {
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep("administrator_login_password"),
data.ImportStep(
"administrator_login_password",
"azure_active_directory_administrator.#",
"azure_active_directory_administrator.0.%",
"azure_active_directory_administrator.0.azuread_authentication_only_enabled",
"azure_active_directory_administrator.0.login_username",
"azure_active_directory_administrator.0.object_id",
"azure_active_directory_administrator.0.principal_type",
"azure_active_directory_administrator.0.tenant_id",
),
{
Config: r.withoutAadAdmin(data),
Check: acceptance.ComposeTestCheckFunc(
Expand All @@ -968,7 +995,16 @@ func TestAccMsSqlManagedInstance_aadAdminUpdate(t *testing.T) {
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep("administrator_login_password"),
data.ImportStep(
"administrator_login_password",
"azure_active_directory_administrator.#",
"azure_active_directory_administrator.0.%",
"azure_active_directory_administrator.0.azuread_authentication_only_enabled",
"azure_active_directory_administrator.0.login_username",
"azure_active_directory_administrator.0.object_id",
"azure_active_directory_administrator.0.principal_type",
"azure_active_directory_administrator.0.tenant_id",
),
})
}

Expand Down Expand Up @@ -1023,6 +1059,7 @@ resource "azurerm_mssql_managed_instance" "test" {
azure_active_directory_administrator {
login_username = azuread_user.test.user_principal_name
object_id = azuread_user.test.object_id
principal_type = "User"
tenant_id = data.azurerm_client_config.test.tenant_id
}
Expand Down Expand Up @@ -1114,6 +1151,7 @@ resource "azurerm_mssql_managed_instance" "test" {
azure_active_directory_administrator {
login_username = azuread_user.test.user_principal_name
object_id = azuread_user.test.object_id
principal_type = "User"
tenant_id = data.azurerm_client_config.test.tenant_id
azuread_authentication_only_enabled = true
}
Expand Down Expand Up @@ -1195,7 +1233,7 @@ resource "azurerm_mssql_managed_instance" "test" {
azure_active_directory_administrator {
login_username = azuread_user.test.user_principal_name
object_id = azuread_user.test.object_id
tenant_id = data.azurerm_client_config.test.tenant_id
principal_type = "User"
}
tags = {
Expand Down Expand Up @@ -1272,6 +1310,7 @@ resource "azurerm_mssql_managed_instance" "test" {
login_username = azuread_user.test.user_principal_name
object_id = azuread_user.test.object_id
tenant_id = data.azurerm_client_config.test.tenant_id
principal_type = "User"
azuread_authentication_only_enabled = true
}
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/mssql_managed_instance.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ An `azure_active_directory_administrator` block supports the following:

* `object_id` - (Required) The object id of the Azure AD Administrator of this SQL Managed Instance.

* `principal_type` - (Required) The principal type of the Azure AD Administrator of this SQL Managed Instance. Possible values are `Application`, `Group`, `User`.

* `azuread_authentication_only_enabled` - (Optional) Specifies whether only Azure AD authentication can be used to log in to this SQL Managed Instance. When `true`, the `administrator_login` and `administrator_login_password` properties can be omitted. Defaults to `false`.

* `tenant_id` - (Optional) The tenant id of the Azure AD Administrator of this SQL Managed Instance. Should be specified if the Azure AD Administrator is homed in a different tenant to the SQL Managed Instance.
Expand Down

0 comments on commit fdac89b

Please sign in to comment.