Skip to content

Commit

Permalink
fix comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sinbai committed Nov 27, 2024
1 parent e377146 commit d95a021
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,17 @@ func (r MsSqlManagedInstanceActiveDirectoryAdministratorResource) Delete() sdk.R

managedInstanceId := commonids.NewSqlManagedInstanceID(id.SubscriptionId, id.ResourceGroup, id.ManagedInstanceName)

err = aadAuthOnlyClient.DeleteThenPoll(ctx, managedInstanceId)
// 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 = aadAuthOnlyClient.CreateOrUpdateThenPoll(ctx, managedInstanceId, aadAuthOnlyParams)
if err != nil {
return fmt.Errorf("removing `azuread_authentication_only` for %s: %+v", managedInstanceId, err)
return fmt.Errorf("disabling `azuread_authentication_only` for %s: %+v", id, err)
}

err = client.DeleteThenPoll(ctx, managedInstanceId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"fmt"
"testing"
"time"

"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonids"
Expand All @@ -25,9 +26,6 @@ func TestAccMsSqlManagedInstanceActiveDirectoryAdministrator_basic(t *testing.T)
r := MsSqlManagedInstanceActiveDirectoryAdministratorResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.template(data),
},
{
Config: r.basic(data, true),
Check: acceptance.ComposeTestCheckFunc(
Expand All @@ -36,7 +34,8 @@ func TestAccMsSqlManagedInstanceActiveDirectoryAdministrator_basic(t *testing.T)
},
data.ImportStep("administrator_login_password"),
{
Config: r.basic(data, false),
PreConfig: func() { time.Sleep(5 * time.Minute) },
Config: r.basic(data, false),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
Expand Down Expand Up @@ -66,6 +65,18 @@ func (r MsSqlManagedInstanceActiveDirectoryAdministratorResource) Exists(ctx con

func (r MsSqlManagedInstanceActiveDirectoryAdministratorResource) template(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {
resource_group {
/* Due to the creation of unmanaged Microsoft.Network/networkIntentPolicies in this service,
prevent_deletion_if_contains_resources has been added here to allow the test resources to be
deleted until this can be properly investigated
*/
prevent_deletion_if_contains_resources = false
}
}
}
%[1]s
resource "azurerm_mssql_managed_instance" "test" {
Expand Down Expand Up @@ -95,13 +106,6 @@ resource "azurerm_mssql_managed_instance" "test" {
environment = "staging"
database = "test"
}
# changing azure_active_directory_administrator is ignored since the value of azure_active_directory_administrator is returned
# when applying a terraform refresh after step 2/5 of testcase TestAccMsSqlManagedInstanceActiveDirectoryAdministrator_basic
# this is expected since the azuread_authentication_only is set to true in step 2/5
lifecycle {
ignore_changes = [azure_active_directory_administrator]
}
}
data "azuread_client_config" "test" {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,11 @@ type AzureActiveDirectoryAdministrator struct {
TenantID string `tfschema:"tenant_id"`
}

var _ sdk.Resource = MsSqlManagedInstanceResource{}
var _ sdk.ResourceWithUpdate = MsSqlManagedInstanceResource{}
var _ sdk.ResourceWithCustomizeDiff = MsSqlManagedInstanceResource{}
var (
_ sdk.Resource = MsSqlManagedInstanceResource{}
_ sdk.ResourceWithUpdate = MsSqlManagedInstanceResource{}
_ sdk.ResourceWithCustomizeDiff = MsSqlManagedInstanceResource{}
)

type MsSqlManagedInstanceResource struct{}

Expand Down Expand Up @@ -615,9 +617,10 @@ func (r MsSqlManagedInstanceResource) Read() sdk.ResourceFunc {

model.AdministratorLogin = pointer.From(props.AdministratorLogin)

if props.Administrators != nil {
model.AzureActiveDirectoryAdministrator = flattenMsSqlManagedInstanceAdministrators(*props.Administrators)
}
// read from state since when `azuread_authentication_only` is enabled via resource `azurerm_mssql_managed_instance_active_directory_administrator`,
// the API returns the value of `AzureActiveDirectoryAdministrator` which causes diff.
model.AzureActiveDirectoryAdministrator = state.AzureActiveDirectoryAdministrator

model.Collation = pointer.From(props.Collation)
model.DnsZone = pointer.From(props.DnsZone)
model.Fqdn = pointer.From(props.FullyQualifiedDomainName)
Expand Down Expand Up @@ -837,13 +840,3 @@ func expandMsSqlManagedInstanceAdministrators(input []AzureActiveDirectoryAdmini

return pointer.To(adminProps)
}

func flattenMsSqlManagedInstanceAdministrators(admin managedinstances.ManagedInstanceExternalAdministrator) []AzureActiveDirectoryAdministrator {
results := make([]AzureActiveDirectoryAdministrator, 0)
return append(results, AzureActiveDirectoryAdministrator{
LoginUserName: pointer.From(admin.Login),
ObjectID: pointer.From(admin.Sid),
TenantID: pointer.From(admin.TenantId),
AzureADAuthenticationOnlyEnabled: pointer.From(admin.AzureADOnlyAuthentication),
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,18 @@ resource "azurerm_mssql_managed_instance" "test" {

func (r MsSqlManagedInstanceResource) withoutAadAdmin(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {
resource_group {
/* Due to the creation of unmanaged Microsoft.Network/networkIntentPolicies in this service,
prevent_deletion_if_contains_resources has been added here to allow the test resources to be
deleted until this can be properly investigated
*/
prevent_deletion_if_contains_resources = false
}
}
}
%[1]s
resource "azurerm_mssql_managed_instance" "test" {
Expand Down Expand Up @@ -962,6 +974,18 @@ func TestAccMsSqlManagedInstance_aadAdminUpdate(t *testing.T) {

func (r MsSqlManagedInstanceResource) aadAdmin(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {
resource_group {
/* Due to the creation of unmanaged Microsoft.Network/networkIntentPolicies in this service,
prevent_deletion_if_contains_resources has been added here to allow the test resources to be
deleted until this can be properly investigated
*/
prevent_deletion_if_contains_resources = false
}
}
}
%[1]s
provider "azuread" {}
Expand Down Expand Up @@ -1044,6 +1068,18 @@ resource "azuread_directory_role_member" "test" {

func (r MsSqlManagedInstanceResource) aadAdminWithAadAuthOnlyEnabled(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {
resource_group {
/* Due to the creation of unmanaged Microsoft.Network/networkIntentPolicies in this service,
prevent_deletion_if_contains_resources has been added here to allow the test resources to be
deleted until this can be properly investigated
*/
prevent_deletion_if_contains_resources = false
}
}
}
%[1]s
provider "azuread" {}
Expand Down Expand Up @@ -1101,6 +1137,18 @@ resource "azurerm_mssql_managed_instance" "test" {

func (r MsSqlManagedInstanceResource) setAadAdmin(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {
resource_group {
/* Due to the creation of unmanaged Microsoft.Network/networkIntentPolicies in this service,
prevent_deletion_if_contains_resources has been added here to allow the test resources to be
deleted until this can be properly investigated
*/
prevent_deletion_if_contains_resources = false
}
}
}
%[1]s
provider "azuread" {}
Expand Down Expand Up @@ -1165,6 +1213,18 @@ resource "azurerm_mssql_managed_instance" "test" {

func (r MsSqlManagedInstanceResource) aadAdminWithAadAuthOnlyEnabledUpdate(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {
resource_group {
/* Due to the creation of unmanaged Microsoft.Network/networkIntentPolicies in this service,
prevent_deletion_if_contains_resources has been added here to allow the test resources to be
deleted until this can be properly investigated
*/
prevent_deletion_if_contains_resources = false
}
}
}
%[1]s
provider "azuread" {}
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/mssql_managed_instance.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ resource "azurerm_route_table" "example" {
name = "routetable-mi"
location = azurerm_resource_group.example.location
resource_group_name = azurerm_resource_group.example.name
disable_bgp_route_propagation = false
bgp_route_propagation_enabled = true
depends_on = [
azurerm_subnet.example,
]
Expand Down

0 comments on commit d95a021

Please sign in to comment.