Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

domainservice: refactoring domainservice resources to using go-azure-sdk #17595

Merged
merged 5 commits into from
Jul 22, 2022
Merged

domainservice: refactoring domainservice resources to using go-azure-sdk #17595

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
693b019
domainservice: refactoring domainservice resources to using go-azure-sdk
magodo Jul 12, 2022
f730a2a
Set the SDK normalized ID as DS data source ID
magodo Jul 20, 2022
1d32dcf
Merge remote-tracking branch 'upstream/main' into domainservice_refactor
magodo Jul 20, 2022
3220434
Merge remote-tracking branch 'upstream/main' into domainservice_refactor
magodo Jul 20, 2022
8dff91a
go mod tidy && vendor
magodo Jul 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 26 additions & 16 deletions internal/services/domainservices/active_directory_domain_service_data_source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ import (
"fmt"
"time"

"github.com/Azure/azure-sdk-for-go/services/domainservices/mgmt/2020-01-01/aad"
"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema"
"github.com/hashicorp/go-azure-helpers/resourcemanager/location"
"github.com/hashicorp/go-azure-helpers/resourcemanager/tags"
"github.com/hashicorp/go-azure-sdk/resource-manager/aad/2020-01-01/domainservices"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
"github.com/hashicorp/terraform-provider-azurerm/internal/tags"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
"github.com/hashicorp/terraform-provider-azurerm/internal/timeouts"
"github.com/hashicorp/terraform-provider-azurerm/utils"
)

func dataSourceActiveDirectoryDomainService() *pluginsdk.Resource {
Expand Down Expand Up @@ -167,7 +167,7 @@ func dataSourceActiveDirectoryDomainService() *pluginsdk.Resource {
Computed: true,
},

"tags": tags.SchemaDataSource(),
"tags": commonschema.Tags(),

"tenant_id": {
Type: pluginsdk.TypeString,
Expand Down Expand Up @@ -220,32 +220,42 @@ func dataSourceActiveDirectoryDomainServiceReplicaSetSchema() map[string]*plugin

func dataSourceActiveDirectoryDomainServiceRead(d *pluginsdk.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).DomainServices.DomainServicesClient
subscrptionId := meta.(*clients.Client).Account.SubscriptionId
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

name := d.Get("name").(string)
resourceGroup := d.Get("resource_group_name").(string)

resp, err := client.Get(ctx, resourceGroup, name)
idsdk := domainservices.NewDomainServiceID(subscrptionId, resourceGroup, name)

resp, err := client.Get(ctx, idsdk)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
if response.WasNotFound(resp.HttpResponse) {
return nil
}
return err
}

if resp.ID == nil {
model := resp.Model
if model == nil {
return fmt.Errorf("reading Domain Service: model was returned nil")
}

if model.Id == nil {
return fmt.Errorf("reading Domain Service: ID was returned nil")
}
d.SetId(*resp.ID)
d.SetId(*model.Id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason we're using this ID rather than the Resource ID above? they aren't guaranteed to be the same casing-wise?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tombuildsstuff I also noticed this, whilst I don't know whether changing this to the resource ID will introduce a breaking change. The intent of this PR is to switch the SDK verbatim.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should move also move towards using our generated IDs over the model ids in this swap too. Let's check to see if the ids are different too. If they are, we need to write a migration on top of it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbfrahry I've made the change.


d.Set("name", name)
d.Set("resource_group_name", resourceGroup)
d.Set("location", location.NormalizeNilable(model.Location))
if err := tags.FlattenAndSet(d, model.Tags); err != nil {
return err
}

d.Set("location", location.NormalizeNilable(resp.Location))

if props := resp.DomainServiceProperties; props != nil {
d.Set("deployment_id", props.DeploymentID)
if props := model.Properties; props != nil {
d.Set("deployment_id", props.DeploymentId)

domainConfigType := ""
if v := props.DomainConfigurationType; v != nil {
Expand All @@ -256,14 +266,14 @@ func dataSourceActiveDirectoryDomainServiceRead(d *pluginsdk.ResourceData, meta
d.Set("domain_name", props.DomainName)

d.Set("filtered_sync_enabled", false)
if props.FilteredSync == aad.FilteredSyncEnabled {
if props.FilteredSync != nil && *props.FilteredSync == domainservices.FilteredSyncEnabled {
d.Set("filtered_sync_enabled", true)
}

d.Set("resource_id", resp.ID)
d.Set("resource_id", model.Id)
d.Set("sku", props.Sku)
d.Set("sync_owner", props.SyncOwner)
d.Set("tenant_id", props.TenantID)
d.Set("tenant_id", props.TenantId)
d.Set("version", props.Version)

if err := d.Set("notifications", flattenDomainServiceNotifications(props.NotificationSettings)); err != nil {
Expand All @@ -284,5 +294,5 @@ func dataSourceActiveDirectoryDomainServiceRead(d *pluginsdk.ResourceData, meta
}
}

return tags.FlattenAndSet(d, resp.Tags)
return nil
}
127 changes: 73 additions & 54 deletions internal/services/domainservices/active_directory_domain_service_replica_set_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import (
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/domainservices/mgmt/2020-01-01/aad"
"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/location"
"github.com/hashicorp/go-azure-sdk/resource-manager/aad/2020-01-01/domainservices"
"github.com/hashicorp/terraform-provider-azurerm/helpers/azure"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
Expand Down Expand Up @@ -88,82 +89,90 @@ func resourceActiveDirectoryDomainServiceReplicaSetCreate(d *pluginsdk.ResourceD
return fmt.Errorf("parsing ID for Domain Service Replica Set")
}

idsdk := domainservices.NewDomainServiceID(domainServiceId.SubscriptionId, domainServiceId.ResourceGroup, domainServiceId.Name)

locks.ByName(domainServiceId.Name, DomainServiceResourceName)
defer locks.UnlockByName(domainServiceId.Name, DomainServiceResourceName)

domainService, err := client.Get(ctx, domainServiceId.ResourceGroup, domainServiceId.Name)
domainService, err := client.Get(ctx, idsdk)
if err != nil {
if utils.ResponseWasNotFound(domainService.Response) {
if response.WasNotFound(domainService.HttpResponse) {
return fmt.Errorf("could not find %s: %s", domainServiceId, err)
}
return fmt.Errorf("reading %s: %s", domainServiceId, err)
}

if domainService.DomainServiceProperties.ReplicaSets == nil || len(*domainService.DomainServiceProperties.ReplicaSets) == 0 {
return fmt.Errorf("reading %s: returned with missing replica set information, expected at least 1 replica set: %s", domainServiceId, err)
model := domainService.Model
if model == nil {
return fmt.Errorf("reading %s: returned with null model", domainServiceId)
}

if model.Properties == nil || model.Properties.ReplicaSets == nil || len(*model.Properties.ReplicaSets) == 0 {
return fmt.Errorf("reading %s: returned with missing replica set information, expected at least 1 replica set", domainServiceId)
}

subnetId := d.Get("subnet_id").(string)
replicaSets := *domainService.DomainServiceProperties.ReplicaSets
replicaSets := *model.Properties.ReplicaSets

for _, r := range replicaSets {
if r.ReplicaSetID == nil {
if r.ReplicaSetId == nil {
return fmt.Errorf("reading %s: a replica set was returned with a missing ReplicaSetID", domainServiceId)
}
if r.SubnetID == nil {
if r.SubnetId == nil {
return fmt.Errorf("reading %s: a replica set was returned with a missing SubnetID", domainServiceId)
}

// We assume that two replica sets cannot coexist in the same subnet
if strings.EqualFold(subnetId, *r.SubnetID) {
if strings.EqualFold(subnetId, *r.SubnetId) {
// Generate an ID here since we only know it once we know the ReplicaSetID
id := parse.NewDomainServiceReplicaSetID(domainServiceId.SubscriptionId, domainServiceId.ResourceGroup, domainServiceId.Name, *r.ReplicaSetID)
id := parse.NewDomainServiceReplicaSetID(domainServiceId.SubscriptionId, domainServiceId.ResourceGroup, domainServiceId.Name, *r.ReplicaSetId)
return tf.ImportAsExistsError("azurerm_active_directory_domain_service_replica_set", id.ID())
}
}

loc := location.Normalize(d.Get("location").(string))
replicaSets = append(replicaSets, aad.ReplicaSet{
replicaSets = append(replicaSets, domainservices.ReplicaSet{
Location: utils.String(loc),
SubnetID: utils.String(subnetId),
SubnetId: utils.String(subnetId),
})

domainService.DomainServiceProperties.ReplicaSets = &replicaSets
model.Properties.ReplicaSets = &replicaSets

future, err := client.CreateOrUpdate(ctx, domainServiceId.ResourceGroup, domainServiceId.Name, domainService)
if err != nil {
if err := client.CreateOrUpdateThenPoll(ctx, idsdk, *model); err != nil {
return fmt.Errorf("creating/updating Replica Sets for %s: %+v", domainServiceId, err)
}
if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return fmt.Errorf("waiting for Replica Sets for %s: %+v", domainServiceId, err)
}

// We need to retrieve the domain service again to find out the new replica set ID
domainService, err = client.Get(ctx, domainServiceId.ResourceGroup, domainServiceId.Name)
domainService, err = client.Get(ctx, idsdk)
if err != nil {
if utils.ResponseWasNotFound(domainService.Response) {
if response.WasNotFound(domainService.HttpResponse) {
return fmt.Errorf("could not find %s: %s", domainServiceId, err)
}
return fmt.Errorf("reading %s: %s", domainServiceId, err)
}

if domainService.DomainServiceProperties.ReplicaSets == nil || len(*domainService.DomainServiceProperties.ReplicaSets) == 0 {
return fmt.Errorf("reading %s: returned with missing replica set information, expected at least 1 replica set: %s", domainServiceId, err)
model = domainService.Model
if model == nil {
return fmt.Errorf("reading %s: returned with null model", domainServiceId)
}

if model.Properties == nil || model.Properties.ReplicaSets == nil || len(*model.Properties.ReplicaSets) == 0 {
return fmt.Errorf("reading %s: returned with missing replica set information, expected at least 1 replica set", domainServiceId)
}

var id parse.DomainServiceReplicaSetId
// Assuming that two replica sets cannot coexist in the same subnet, we identify our new replica set by its SubnetID
for _, r := range *domainService.DomainServiceProperties.ReplicaSets {
if r.ReplicaSetID == nil {
for _, r := range *model.Properties.ReplicaSets {
if r.ReplicaSetId == nil {
return fmt.Errorf("reading %s: a replica set was returned with a missing ReplicaSetID", domainServiceId)
}
if r.SubnetID == nil {
if r.SubnetId == nil {
return fmt.Errorf("reading %s: a replica set was returned with a missing SubnetID", domainServiceId)
}

if strings.EqualFold(subnetId, *r.SubnetID) {
if strings.EqualFold(subnetId, *r.SubnetId) {
// We found it!
id = parse.NewDomainServiceReplicaSetID(domainServiceId.SubscriptionId, domainServiceId.ResourceGroup, domainServiceId.Name, *r.ReplicaSetID)
id = parse.NewDomainServiceReplicaSetID(domainServiceId.SubscriptionId, domainServiceId.ResourceGroup, domainServiceId.Name, *r.ReplicaSetId)
}
}

Expand Down Expand Up @@ -201,17 +210,24 @@ func resourceActiveDirectoryDomainServiceReplicaSetRead(d *pluginsdk.ResourceDat
return err
}

domainService, err := client.Get(ctx, id.ResourceGroup, id.DomainServiceName)
idsdk := domainservices.NewDomainServiceID(id.SubscriptionId, id.ResourceGroup, id.DomainServiceName)

domainService, err := client.Get(ctx, idsdk)
if err != nil {
if utils.ResponseWasNotFound(domainService.Response) {
if response.WasNotFound(domainService.HttpResponse) {
d.SetId("")
return nil
}
return err
}

if domainService.DomainServiceProperties.ReplicaSets == nil || len(*domainService.DomainServiceProperties.ReplicaSets) == 0 {
return fmt.Errorf("reading %s: domain service returned with missing replica set information, expected at least 1 replica set: %s", id, err)
model := domainService.Model
if model == nil {
return fmt.Errorf("reading %s: returned with null model", id)
}

if model.Properties == nil || model.Properties.ReplicaSets == nil || len(*model.Properties.ReplicaSets) == 0 {
return fmt.Errorf("reading %s: returned with missing replica set information, expected at least 1 replica set", id)
}

var (
Expand All @@ -222,29 +238,29 @@ func resourceActiveDirectoryDomainServiceReplicaSetRead(d *pluginsdk.ResourceDat
subnetId string
)

replicaSets := *domainService.DomainServiceProperties.ReplicaSets
replicaSets := *model.Properties.ReplicaSets

for _, r := range replicaSets {
if r.ReplicaSetID == nil {
if r.ReplicaSetId == nil {
return fmt.Errorf("reading %s: a replica set was returned with a missing ReplicaSetID", id)
}

// ReplicaSetName in the ID struct is really the replica set ID
if *r.ReplicaSetID == id.ReplicaSetName {
if r.DomainControllerIPAddress != nil {
domainControllerIpAddresses = *r.DomainControllerIPAddress
if *r.ReplicaSetId == id.ReplicaSetName {
if r.DomainControllerIpAddress != nil {
domainControllerIpAddresses = *r.DomainControllerIpAddress
}
if r.ExternalAccessIPAddress != nil {
externalAccessIpAddress = *r.ExternalAccessIPAddress
if r.ExternalAccessIpAddress != nil {
externalAccessIpAddress = *r.ExternalAccessIpAddress
}
if r.Location != nil {
loc = location.NormalizeNilable(r.Location)
}
if r.ServiceStatus != nil {
serviceStatus = *r.ServiceStatus
}
if r.SubnetID != nil {
subnetId = *r.SubnetID
if r.SubnetId != nil {
subnetId = *r.SubnetId
}
}
}
Expand All @@ -268,27 +284,34 @@ func resourceActiveDirectoryDomainServiceReplicaSetDelete(d *pluginsdk.ResourceD
return err
}

domainService, err := client.Get(ctx, id.ResourceGroup, id.DomainServiceName)
idsdk := domainservices.NewDomainServiceID(id.SubscriptionId, id.ResourceGroup, id.DomainServiceName)

domainService, err := client.Get(ctx, idsdk)
if err != nil {
if utils.ResponseWasNotFound(domainService.Response) {
if response.WasNotFound(domainService.HttpResponse) {
return fmt.Errorf("deleting %s: domain service was not found: %s", id, err)
}
return err
}

if domainService.DomainServiceProperties.ReplicaSets == nil || len(*domainService.DomainServiceProperties.ReplicaSets) == 0 {
return fmt.Errorf("deleting %s: domain service returned with missing replica set information, expected at least 1 replica set: %s", id, err)
model := domainService.Model
if model == nil {
return fmt.Errorf("reading %s: returned with null model", id)
}

replicaSets := *domainService.DomainServiceProperties.ReplicaSets
if model.Properties == nil || model.Properties.ReplicaSets == nil || len(*model.Properties.ReplicaSets) == 0 {
return fmt.Errorf("reading %s: returned with missing replica set information, expected at least 1 replica set", id)
}

newReplicaSets := make([]aad.ReplicaSet, 0)
replicaSets := *model.Properties.ReplicaSets

newReplicaSets := make([]domainservices.ReplicaSet, 0)
for _, r := range replicaSets {
if r.ReplicaSetID == nil {
if r.ReplicaSetId == nil {
return fmt.Errorf("deleting %s: a replica set was returned with a missing ReplicaSetID", id)
}

if *r.ReplicaSetID == id.ReplicaSetName {
if *r.ReplicaSetId == id.ReplicaSetName {
continue
}

Expand All @@ -299,15 +322,11 @@ func resourceActiveDirectoryDomainServiceReplicaSetDelete(d *pluginsdk.ResourceD
return fmt.Errorf("deleting %s: could not determine which replica set to remove", id)
}

domainService.DomainServiceProperties.ReplicaSets = &newReplicaSets
model.Properties.ReplicaSets = &newReplicaSets

future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.DomainServiceName, domainService)
if err != nil {
if err := client.CreateOrUpdateThenPoll(ctx, idsdk, *model); err != nil {
return fmt.Errorf("deleting %s: %+v", id, err)
}
if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return fmt.Errorf("waiting for deletion of %s: %+v", id, err)
}

// Wait for all replica sets to become available with two domain controllers each before proceeding
// Generate a partial DomainServiceId since we don't need to know the initial replica set ID here
Expand Down
Loading