From 7aa8ceb22ce7c843fc2f410e8aaec39c55c2d7c5 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Mon, 14 Mar 2022 13:53:34 +0100 Subject: [PATCH 1/6] r/eventgrid_event_subscription: refactoring to use an ID Parser --- .../eventgrid_event_subscription_resource.go | 86 ++++++++----------- .../eventgrid/parse/event_subscription.go | 25 ++++++ 2 files changed, 61 insertions(+), 50 deletions(-) diff --git a/internal/services/eventgrid/eventgrid_event_subscription_resource.go b/internal/services/eventgrid/eventgrid_event_subscription_resource.go index cc691a027aed..2c8b6a91a39a 100644 --- a/internal/services/eventgrid/eventgrid_event_subscription_resource.go +++ b/internal/services/eventgrid/eventgrid_event_subscription_resource.go @@ -162,19 +162,17 @@ func resourceEventGridEventSubscriptionCreateUpdate(d *pluginsdk.ResourceData, m ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() - name := d.Get("name").(string) - scope := d.Get("scope").(string) - + id := parse.NewEventSubscriptionID(d.Get("scope").(string), d.Get("name").(string)) if d.IsNewResource() { - existing, err := client.Get(ctx, scope, name) + existing, err := client.Get(ctx, id.Scope, id.Name) if err != nil { if !utils.ResponseWasNotFound(existing.Response) { - return fmt.Errorf("checking for presence of existing EventGrid Event Subscription %q (Scope %q): %s", name, scope, err) + return fmt.Errorf("checking for presence of existing %s: %+v", id, err) } } if !utils.ResponseWasNotFound(existing.Response) { - return tf.ImportAsExistsError("azurerm_eventgrid_event_subscription", *existing.ID) + return tf.ImportAsExistsError("azurerm_eventgrid_event_subscription", id.ID()) } } @@ -185,12 +183,12 @@ func resourceEventGridEventSubscriptionCreateUpdate(d *pluginsdk.ResourceData, m filter, err := expandEventGridEventSubscriptionFilter(d) if err != nil { - return fmt.Errorf("expanding filters for EventGrid Event Subscription %q (Scope %q): %+v", name, scope, err) + return fmt.Errorf("expanding filters for %s: %+v", id, err) } expirationTime, err := expandEventGridExpirationTime(d) if err != nil { - return fmt.Errorf("creating/updating EventGrid Event Subscription %q (Scope %q): %s", name, scope, err) + return fmt.Errorf("expanding `expiration_time` for %s: %+v", id, err) } deadLetterDestination := expandEventGridEventSubscriptionStorageBlobDeadLetterDestination(d) @@ -240,27 +238,15 @@ func resourceEventGridEventSubscriptionCreateUpdate(d *pluginsdk.ResourceData, m EventSubscriptionProperties: &eventSubscriptionProperties, } - log.Printf("[INFO] preparing arguments for AzureRM EventGrid Event Subscription creation with Properties: %+v.", eventSubscription) - - future, err := client.CreateOrUpdate(ctx, scope, name, eventSubscription) + future, err := client.CreateOrUpdate(ctx, id.Scope, id.Name, eventSubscription) if err != nil { - return fmt.Errorf("creating/updating EventGrid Event Subscription %q (Scope %q): %s", name, scope, err) + return fmt.Errorf("creating/updating %s: %+v", id, err) } - if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("waiting for EventGrid Event Subscription %q (Scope %q) to become available: %s", name, scope, err) - } - - read, err := client.Get(ctx, scope, name) - if err != nil { - return fmt.Errorf("retrieving EventGrid Event Subscription %q (Scope %q): %s", name, scope, err) + return fmt.Errorf("waiting for %s: %+v", id, err) } - if read.ID == nil { - return fmt.Errorf("Cannot read EventGrid Event Subscription %s (Scope %s) ID", name, scope) - } - - d.SetId(*read.ID) + d.SetId(id.ID()) return resourceEventGridEventSubscriptionRead(d, meta) } @@ -277,15 +263,15 @@ func resourceEventGridEventSubscriptionRead(d *pluginsdk.ResourceData, meta inte resp, err := client.Get(ctx, id.Scope, id.Name) if err != nil { if utils.ResponseWasNotFound(resp.Response) { - log.Printf("[WARN] EventGrid Event Subscription '%s' was not found (resource group '%s')", id.Name, id.Scope) + log.Printf("[WARN] %s was not found - removing from state", *id) d.SetId("") return nil } - return fmt.Errorf("making Read request on EventGrid Event Subscription '%s': %+v", id.Name, err) + return fmt.Errorf("retrieving %s: %+v", *id, err) } - d.Set("name", resp.Name) + d.Set("name", id.Name) d.Set("scope", id.Scope) if props := resp.EventSubscriptionProperties; props != nil { @@ -302,82 +288,82 @@ func resourceEventGridEventSubscriptionRead(d *pluginsdk.ResourceData, meta inte deliveryIdentityFlattened = flattenEventGridEventSubscriptionIdentity(deliveryIdentity.Identity) } if err := d.Set("delivery_identity", deliveryIdentityFlattened); err != nil { - return fmt.Errorf("setting `delivery_identity` for EventGrid Event Subscription %q (Scope %q): %s", id.Name, id.Scope, err) + return fmt.Errorf("setting `delivery_identity` for %s: %+v", *id, err) } if azureFunctionEndpoint, ok := destination.AsAzureFunctionEventSubscriptionDestination(); ok { if err := d.Set("azure_function_endpoint", flattenEventGridEventSubscriptionAzureFunctionEndpoint(azureFunctionEndpoint)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid Event Subscription %q (Scope %q): %s", "azure_function_endpoint", id.Name, id.Scope, err) + return fmt.Errorf("setting `azure_function_endpoint` for %s: %+v", *id, err) } if azureFunctionEndpoint.DeliveryAttributeMappings != nil { if err := d.Set("delivery_property", flattenDeliveryProperties(d, azureFunctionEndpoint.DeliveryAttributeMappings)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid delivery properties %q (Scope %q): %s", "azure_function_endpoint", id.Name, id.Scope, err) + return fmt.Errorf("setting `delivery_property` for %s: %+v", *id, err) } } } if v, ok := destination.AsEventHubEventSubscriptionDestination(); ok { if err := d.Set("eventhub_endpoint_id", v.ResourceID); err != nil { - return fmt.Errorf("setting `%q` for EventGrid Event Subscription %q (Scope %q): %s", "eventhub_endpoint_id", id.Name, id.Scope, err) + return fmt.Errorf("setting `eventhub_endpoint_id` for %s: %+v", *id, err) } if err := d.Set("eventhub_endpoint", flattenEventGridEventSubscriptionEventhubEndpoint(v)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid Event Subscription %q (Scope %q): %s", "eventhub_endpoint", id.Name, id.Scope, err) + return fmt.Errorf("setting `eventhub_endpoint` for %s: %+v", *id, err) } if v.DeliveryAttributeMappings != nil { if err := d.Set("delivery_property", flattenDeliveryProperties(d, v.DeliveryAttributeMappings)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid delivery properties %q (Scope %q): %s", "eventhub_endpoint", id.Name, id.Scope, err) + return fmt.Errorf("setting `delivery_property` for %s: %+v", *id, err) } } } if v, ok := destination.AsHybridConnectionEventSubscriptionDestination(); ok { if err := d.Set("hybrid_connection_endpoint_id", v.ResourceID); err != nil { - return fmt.Errorf("setting `%q` for EventGrid Event Subscription %q (Scope %q): %s", "hybrid_connection_endpoint_id", id.Name, id.Scope, err) + return fmt.Errorf("setting `hybrid_connection_endpoint_id` for %s: %+v", *id, err) } if err := d.Set("hybrid_connection_endpoint", flattenEventGridEventSubscriptionHybridConnectionEndpoint(v)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid Event Subscription %q (Scope %q): %s", "hybrid_connection_endpoint", id.Name, id.Scope, err) + return fmt.Errorf("setting `hybrid_connection_endpoint` for %s: %+v", *id, err) } if v.DeliveryAttributeMappings != nil { if err := d.Set("delivery_property", flattenDeliveryProperties(d, v.DeliveryAttributeMappings)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid delivery properties %q (Scope %q): %s", "hybrid_connection_endpoint", id.Name, id.Scope, err) + return fmt.Errorf("setting `delivery_property` for %s: %+v", *id, err) } } } if serviceBusQueueEndpoint, ok := destination.AsServiceBusQueueEventSubscriptionDestination(); ok { if err := d.Set("service_bus_queue_endpoint_id", serviceBusQueueEndpoint.ResourceID); err != nil { - return fmt.Errorf("setting `%q` for EventGrid Event Subscription %q (Scope %q): %s", "service_bus_queue_endpoint_id", id.Name, id.Scope, err) + return fmt.Errorf("setting `service_bus_queue_endpoint_id` for %s: %+v", *id, err) } } if serviceBusTopicEndpoint, ok := destination.AsServiceBusTopicEventSubscriptionDestination(); ok { if err := d.Set("service_bus_topic_endpoint_id", serviceBusTopicEndpoint.ResourceID); err != nil { - return fmt.Errorf("setting `%q` for EventGrid Event Subscription %q (Scope %q): %s", "service_bus_topic_endpoint_id", id.Name, id.Scope, err) + return fmt.Errorf("setting `service_bus_topic_endpoint_id` for %s: %+v", *id, err) } if serviceBusTopicEndpoint.DeliveryAttributeMappings != nil { if err := d.Set("delivery_property", flattenDeliveryProperties(d, serviceBusTopicEndpoint.DeliveryAttributeMappings)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid delivery properties %q (Scope %q): %s", "service_bus_topic_endpoint_id", id.Name, id.Scope, err) + return fmt.Errorf("setting `delivery_property` for %s: %+v", *id, err) } } } if v, ok := destination.AsStorageQueueEventSubscriptionDestination(); ok { if err := d.Set("storage_queue_endpoint", flattenEventGridEventSubscriptionStorageQueueEndpoint(v)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid Event Subscription %q (Scope %q): %s", "storage_queue_endpoint", id.Name, id.Scope, err) + return fmt.Errorf("setting `storage_queue_endpoint` for %s: %+v", *id, err) } } if v, ok := destination.AsWebHookEventSubscriptionDestination(); ok { fullURL, err := client.GetFullURL(ctx, id.Scope, id.Name) if err != nil { - return fmt.Errorf("making Read request on EventGrid Event Subscription full URL '%s': %+v", id.Name, err) + return fmt.Errorf("retrieving Full WebHook URL for %s: %+v", *id, err) } if err := d.Set("webhook_endpoint", flattenEventGridEventSubscriptionWebhookEndpoint(v, &fullURL)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid Event Subscription %q (Scope %q): %s", "webhook_endpoint", id.Name, id.Scope, err) + return fmt.Errorf("setting `webhook_endpoint` for %s: %+v", *id, err) } if v.DeliveryAttributeMappings != nil { if err := d.Set("delivery_property", flattenDeliveryProperties(d, v.DeliveryAttributeMappings)); err != nil { - return fmt.Errorf("setting `%q` for EventGrid delivery properties %q (Scope %q): %s", "webhook_endpoint", id.Name, id.Scope, err) + return fmt.Errorf("setting `delivery_property` for %s: %+v", *id, err) } } } @@ -389,13 +375,13 @@ func resourceEventGridEventSubscriptionRead(d *pluginsdk.ResourceData, meta inte deadLetterIdentityFlattened = flattenEventGridEventSubscriptionIdentity(deadLetterIdentity.Identity) } if err := d.Set("dead_letter_identity", deadLetterIdentityFlattened); err != nil { - return fmt.Errorf("setting `dead_letter_identity` for EventGrid Event Subscription %q (Scope %q): %s", id.Name, id.Scope, err) + return fmt.Errorf("setting `dead_letter_identity` for %s: %+v", *id, err) } if deadLetterDestination != nil { if storageBlobDeadLetterDestination, ok := deadLetterDestination.AsStorageBlobDeadLetterDestination(); ok { if err := d.Set("storage_blob_dead_letter_destination", flattenEventGridEventSubscriptionStorageBlobDeadLetterDestination(storageBlobDeadLetterDestination)); err != nil { - return fmt.Errorf("Error setting `storage_blob_dead_letter_destination` for EventGrid Event Subscription %q (Scope %q): %s", id.Name, id.Scope, err) + return fmt.Errorf("Error setting `storage_blob_dead_letter_destination` for %s: %+v", *id, err) } } } @@ -404,29 +390,29 @@ func resourceEventGridEventSubscriptionRead(d *pluginsdk.ResourceData, meta inte d.Set("included_event_types", filter.IncludedEventTypes) d.Set("advanced_filtering_on_arrays_enabled", filter.EnableAdvancedFilteringOnArrays) if err := d.Set("subject_filter", flattenEventGridEventSubscriptionSubjectFilter(filter)); err != nil { - return fmt.Errorf("setting `subject_filter` for EventGrid Event Subscription %q (Scope %q): %s", id.Name, id.Scope, err) + return fmt.Errorf("setting `subject_filter` for %s: %+v", *id, err) } if err := d.Set("advanced_filter", flattenEventGridEventSubscriptionAdvancedFilter(filter)); err != nil { - return fmt.Errorf("setting `advanced_filter` for EventGrid Event Subscription %q (Scope %q): %s", id.Name, id.Scope, err) + return fmt.Errorf("setting `advanced_filter` for %s: %+v", *id, err) } } if props.DeadLetterDestination != nil { if storageBlobDeadLetterDestination, ok := props.DeadLetterDestination.AsStorageBlobDeadLetterDestination(); ok { if err := d.Set("storage_blob_dead_letter_destination", flattenEventGridEventSubscriptionStorageBlobDeadLetterDestination(storageBlobDeadLetterDestination)); err != nil { - return fmt.Errorf("setting `storage_blob_dead_letter_destination` for EventGrid Event Subscription %q (Scope %q): %s", id.Name, id.Scope, err) + return fmt.Errorf("setting `storage_blob_dead_letter_destination` for %s: %+v", *id, err) } } } if retryPolicy := props.RetryPolicy; retryPolicy != nil { if err := d.Set("retry_policy", flattenEventGridEventSubscriptionRetryPolicy(retryPolicy)); err != nil { - return fmt.Errorf("setting `retry_policy` for EventGrid Event Subscription %q (Scope %q): %s", id.Name, id.Scope, err) + return fmt.Errorf("setting `retry_policy` for %s: %+v", *id, err) } } if err := d.Set("labels", props.Labels); err != nil { - return fmt.Errorf("setting `labels` for EventGrid Event Subscription %q (Scope %q): %s", id.Name, id.Scope, err) + return fmt.Errorf("setting `labels` for %s: %+v", *id, err) } } diff --git a/internal/services/eventgrid/parse/event_subscription.go b/internal/services/eventgrid/parse/event_subscription.go index bfdc0764e210..0325cda6adb3 100644 --- a/internal/services/eventgrid/parse/event_subscription.go +++ b/internal/services/eventgrid/parse/event_subscription.go @@ -4,14 +4,39 @@ import ( "fmt" "strings" + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" + "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" ) +var _ resourceids.Id = EventSubscriptionId{} + type EventSubscriptionId struct { Scope string Name string } +func NewEventSubscriptionID(scope string, name string) EventSubscriptionId { + return EventSubscriptionId{ + Scope: scope, + Name: name, + } +} + +func (id EventSubscriptionId) ID() string { + fmtStr := "%s/providers/Microsoft.EventGrid/eventSubscriptions/%s" + return fmt.Sprintf(fmtStr, id.Scope, id.Name) +} + +func (id EventSubscriptionId) String() string { + segments := []string{ + fmt.Sprintf("Name %q", id.Name), + fmt.Sprintf("Scope %q", id.Scope), + } + segmentsStr := strings.Join(segments, " / ") + return fmt.Sprintf("%s: (%s)", "Event Subscription", segmentsStr) +} + func EventSubscriptionID(input string) (*EventSubscriptionId, error) { _, err := azure.ParseAzureResourceID(input) if err != nil { From b3c90d468145a5bb17cc775ecf9fb78da20b9eed Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Tue, 15 Mar 2022 10:18:11 +0100 Subject: [PATCH 2/6] refactor: lighthouse assignment now uses an id parser --- .../lighthouse_assignment_resource.go | 34 +++++-------------- .../lighthouse/parse/lighthouse_assignment.go | 25 ++++++++++++++ 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/internal/services/lighthouse/lighthouse_assignment_resource.go b/internal/services/lighthouse/lighthouse_assignment_resource.go index 846c2879e911..92aff54830df 100644 --- a/internal/services/lighthouse/lighthouse_assignment_resource.go +++ b/internal/services/lighthouse/lighthouse_assignment_resource.go @@ -9,7 +9,6 @@ import ( "github.com/Azure/azure-sdk-for-go/services/managedservices/mgmt/2019-06-01/managedservices" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/go-uuid" - "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/internal/services/lighthouse/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/services/lighthouse/validate" @@ -77,40 +76,25 @@ func resourceLighthouseAssignmentCreate(d *pluginsdk.ResourceData, meta interfac lighthouseAssignmentName = uuid } - scope := d.Get("scope").(string) - - existing, err := client.Get(ctx, scope, lighthouseAssignmentName, utils.Bool(false)) + id := parse.NewLighthouseAssignmentID(d.Get("scope").(string), lighthouseAssignmentName) + existing, err := client.Get(ctx, id.Scope, id.Name, utils.Bool(false)) if err != nil { if !utils.ResponseWasNotFound(existing.Response) { - return fmt.Errorf("checking for presence of existing Lighthouse Assignment %q (Scope %q): %+v", lighthouseAssignmentName, scope, err) + return fmt.Errorf("checking for presence of existing %s: %+v", id, err) } } - if existing.ID != nil && *existing.ID != "" { - return tf.ImportAsExistsError("azurerm_lighthouse_assignment", *existing.ID) - } - parameters := managedservices.RegistrationAssignment{ Properties: &managedservices.RegistrationAssignmentProperties{ RegistrationDefinitionID: utils.String(d.Get("lighthouse_definition_id").(string)), }, } - if _, err := client.CreateOrUpdate(ctx, scope, lighthouseAssignmentName, parameters); err != nil { - return fmt.Errorf("creating Lighthouse Assignment %q (Scope %q): %+v", lighthouseAssignmentName, scope, err) + if _, err := client.CreateOrUpdate(ctx, id.Scope, id.Name, parameters); err != nil { + return fmt.Errorf("creating %s: %+v", id, err) } - read, err := client.Get(ctx, scope, lighthouseAssignmentName, utils.Bool(false)) - if err != nil { - return fmt.Errorf("retrieving Lighthouse Assessment %q (Scope %q): %+v", lighthouseAssignmentName, scope, err) - } - - if read.ID == nil || *read.ID == "" { - return fmt.Errorf("ID was nil or empty for Lighthouse Assignment %q ID (scope %q) ID", lighthouseAssignmentName, scope) - } - - d.SetId(*read.ID) - + d.SetId(id.ID()) return resourceLighthouseAssignmentRead(d, meta) } @@ -127,15 +111,15 @@ func resourceLighthouseAssignmentRead(d *pluginsdk.ResourceData, meta interface{ resp, err := client.Get(ctx, id.Scope, id.Name, utils.Bool(false)) if err != nil { if utils.ResponseWasNotFound(resp.Response) { - log.Printf("[WARN] Lighthouse Assignment %q was not found (Scope %q)", id.Name, id.Scope) + log.Printf("[WARN] %s was not found - removing from state", *id) d.SetId("") return nil } - return fmt.Errorf("making Read request on Lighthouse Assignment %q (Scope %q): %+v", id.Name, id.Scope, err) + return fmt.Errorf("retrieving %s: %+v", *id, err) } - d.Set("name", resp.Name) + d.Set("name", id.Name) d.Set("scope", id.Scope) if props := resp.Properties; props != nil { diff --git a/internal/services/lighthouse/parse/lighthouse_assignment.go b/internal/services/lighthouse/parse/lighthouse_assignment.go index 2f3b395a9430..b76df2c5e0ce 100644 --- a/internal/services/lighthouse/parse/lighthouse_assignment.go +++ b/internal/services/lighthouse/parse/lighthouse_assignment.go @@ -3,13 +3,38 @@ package parse import ( "fmt" "strings" + + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" ) +var _ resourceids.Id = LighthouseAssignmentId{} + type LighthouseAssignmentId struct { Scope string Name string } +func NewLighthouseAssignmentID(scope, name string) LighthouseAssignmentId { + return LighthouseAssignmentId{ + Scope: scope, + Name: name, + } +} + +func (id LighthouseAssignmentId) ID() string { + fmtStr := "%s/providers/Microsoft.ManagedServices/registrationAssignments/%s" + return fmt.Sprintf(fmtStr, id.Scope, id.Name) +} + +func (id LighthouseAssignmentId) String() string { + segments := []string{ + fmt.Sprintf("Name %q", id.Name), + fmt.Sprintf("Scope %q", id.Scope), + } + segmentsStr := strings.Join(segments, " / ") + return fmt.Sprintf("%s: (%s)", "Lighthouse Assignment", segmentsStr) +} + func LighthouseAssignmentID(id string) (*LighthouseAssignmentId, error) { segments := strings.Split(id, "/providers/Microsoft.ManagedServices/registrationAssignments/") if len(segments) != 2 { From ebfd5ab0afb550c553ce413eef1275780b9c3f09 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Tue, 15 Mar 2022 10:18:30 +0100 Subject: [PATCH 3/6] d/private_endpoint_connection: switching to use an id parser --- ...private_endpoint_connection_data_source.go | 40 ++++++++----------- .../network/private_endpoint_resource.go | 2 +- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/internal/services/network/private_endpoint_connection_data_source.go b/internal/services/network/private_endpoint_connection_data_source.go index 36fd421011ab..661e30b73e0a 100644 --- a/internal/services/network/private_endpoint_connection_data_source.go +++ b/internal/services/network/private_endpoint_connection_data_source.go @@ -81,28 +81,24 @@ func dataSourcePrivateEndpointConnection() *pluginsdk.Resource { func dataSourcePrivateEndpointConnectionRead(d *pluginsdk.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Network.PrivateEndpointClient + subscriptionId := meta.(*clients.Client).Account.SubscriptionId nicsClient := meta.(*clients.Client).Network.InterfacesClient 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, "") + id := parse.NewPrivateEndpointID(subscriptionId, d.Get("resource_group_name").(string), d.Get("name").(string)) + resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") if err != nil { if utils.ResponseWasNotFound(resp.Response) { - return fmt.Errorf("Private Endpoint %q was not found in Resource Group %q", name, resourceGroup) + return fmt.Errorf("%s was not found", id) } - return fmt.Errorf("reading Private Endpoint %q (Resource Group %q): %+v", name, resourceGroup, err) - } - if resp.ID == nil || *resp.ID == "" { - return fmt.Errorf("API returns a nil/empty id on Private Endpoint %q (Resource Group %q): %+v", name, resourceGroup, err) + return fmt.Errorf("retrieving %s: %+v", id, err) } - d.SetId(*resp.ID) - d.Set("name", resp.Name) - d.Set("resource_group_name", resourceGroup) + d.SetId(id.ID()) + d.Set("name", id.Name) + d.Set("resource_group_name", id.ResourceGroup) d.Set("location", location.NormalizeNilable(resp.Location)) if props := resp.PrivateEndpointProperties; props != nil { @@ -117,7 +113,7 @@ func dataSourcePrivateEndpointConnectionRead(d *pluginsdk.ResourceData, meta int } } - if err := d.Set("network_interface", getNetworkInterface(networkInterfaceId)); err != nil { + if err := d.Set("network_interface", flattenNetworkInterface(networkInterfaceId)); err != nil { return fmt.Errorf("setting `network_interface`: %+v", err) } @@ -129,20 +125,18 @@ func dataSourcePrivateEndpointConnectionRead(d *pluginsdk.ResourceData, meta int return nil } -func getNetworkInterface(networkInterfaceId string) interface{} { - results := make([]interface{}, 0) - +func flattenNetworkInterface(networkInterfaceId string) interface{} { id, err := parse.NetworkInterfaceID(networkInterfaceId) if err != nil { - return results + return []interface{}{} } - elem := map[string]string{} - - elem["id"] = id.ID() - elem["name"] = id.Name - - return append(results, elem) + return []interface{}{ + map[string]interface{}{ + "id": id.ID(), + "name": id.Name, + }, + } } func getPrivateIpAddress(ctx context.Context, client *network.InterfacesClient, networkInterfaceId string) string { diff --git a/internal/services/network/private_endpoint_resource.go b/internal/services/network/private_endpoint_resource.go index 6153800c41de..a242deb0c15a 100644 --- a/internal/services/network/private_endpoint_resource.go +++ b/internal/services/network/private_endpoint_resource.go @@ -486,7 +486,7 @@ func resourcePrivateEndpointRead(d *pluginsdk.ResourceData, meta interface{}) er } } - networkInterface := getNetworkInterface(networkInterfaceId) + networkInterface := flattenNetworkInterface(networkInterfaceId) if err := d.Set("network_interface", networkInterface); err != nil { return fmt.Errorf("setting `network_interface`: %+v", err) } From 771a69939c99f86f32830da766b3a57c3f19911e Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Tue, 15 Mar 2022 10:24:57 +0100 Subject: [PATCH 4/6] r/lighthouse_definition: refactoring to use an id parser --- .../lighthouse_definition_resource.go | 41 ++++++------------- .../lighthouse/parse/lighthouse_definition.go | 25 +++++++++++ 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/internal/services/lighthouse/lighthouse_definition_resource.go b/internal/services/lighthouse/lighthouse_definition_resource.go index b304bc8a4196..0b3ee4ea61a5 100644 --- a/internal/services/lighthouse/lighthouse_definition_resource.go +++ b/internal/services/lighthouse/lighthouse_definition_resource.go @@ -159,23 +159,17 @@ func resourceLighthouseDefinitionCreateUpdate(d *pluginsdk.ResourceData, meta in lighthouseDefinitionID = uuid } - subscriptionID := meta.(*clients.Client).Account.SubscriptionId - if subscriptionID == "" { - return fmt.Errorf("reading Subscription for Lighthouse Definition %q", lighthouseDefinitionID) - } - - scope := d.Get("scope").(string) - + id := parse.NewLighthouseDefinitionID(d.Get("scope").(string), lighthouseDefinitionID) if d.IsNewResource() { - existing, err := client.Get(ctx, scope, lighthouseDefinitionID) + existing, err := client.Get(ctx, id.Scope, id.LighthouseDefinitionID) if err != nil { if !utils.ResponseWasNotFound(existing.Response) { - return fmt.Errorf("checking for presence of existing Lighthouse Definition %q (Scope %q): %+v", lighthouseDefinitionID, scope, err) + return fmt.Errorf("checking for presence of existing %s: %+v", id, err) } } - if existing.ID != nil && *existing.ID != "" { - return tf.ImportAsExistsError("azurerm_lighthouse_definition", *existing.ID) + if !utils.ResponseWasNotFound(existing.Response) { + return tf.ImportAsExistsError("azurerm_lighthouse_definition", id.ID()) } } authorizations, err := expandLighthouseDefinitionAuthorization(d.Get("authorization").(*pluginsdk.Set).List()) @@ -192,21 +186,12 @@ func resourceLighthouseDefinitionCreateUpdate(d *pluginsdk.ResourceData, meta in }, } - if _, err := client.CreateOrUpdate(ctx, lighthouseDefinitionID, scope, parameters); err != nil { - return fmt.Errorf("Creating/Updating Lighthouse Definition %q (Scope %q): %+v", lighthouseDefinitionID, scope, err) - } - - read, err := client.Get(ctx, scope, lighthouseDefinitionID) - if err != nil { - return err + // NOTE: this API call uses DefinitionId then Scope - check in the future + if _, err := client.CreateOrUpdate(ctx, id.LighthouseDefinitionID, id.Scope, parameters); err != nil { + return fmt.Errorf("creating/updating %s: %+v", id, err) } - if read.ID == nil { - return fmt.Errorf("Cannot read Lighthouse Definition %q ID (scope %q) ID", lighthouseDefinitionID, scope) - } - - d.SetId(*read.ID) - + d.SetId(id.ID()) return resourceLighthouseDefinitionRead(d, meta) } @@ -223,15 +208,15 @@ func resourceLighthouseDefinitionRead(d *pluginsdk.ResourceData, meta interface{ resp, err := client.Get(ctx, id.Scope, id.LighthouseDefinitionID) if err != nil { if utils.ResponseWasNotFound(resp.Response) { - log.Printf("[WARN] Lighthouse Definition %q was not found (Scope %q)", id.LighthouseDefinitionID, id.Scope) + log.Printf("[WARN] %s was not found - removing from state", *id) d.SetId("") return nil } - return fmt.Errorf("making Read request on Lighthouse Definition %q (Scope %q): %+v", id.LighthouseDefinitionID, id.Scope, err) + return fmt.Errorf("retrieving %s: %+v", *id, err) } - d.Set("lighthouse_definition_id", resp.Name) + d.Set("lighthouse_definition_id", id.LighthouseDefinitionID) d.Set("scope", id.Scope) if err := d.Set("plan", flattenLighthouseDefinitionPlan(resp.Plan)); err != nil { @@ -261,7 +246,7 @@ func resourceLighthouseDefinitionDelete(d *pluginsdk.ResourceData, meta interfac } if _, err = client.Delete(ctx, id.LighthouseDefinitionID, id.Scope); err != nil { - return fmt.Errorf("deleting Lighthouse Definition %q at Scope %q: %+v", id.LighthouseDefinitionID, id.Scope, err) + return fmt.Errorf("deleting %s: %+v", *id, err) } return nil diff --git a/internal/services/lighthouse/parse/lighthouse_definition.go b/internal/services/lighthouse/parse/lighthouse_definition.go index 68086e8ac964..89ddd9aeb00e 100644 --- a/internal/services/lighthouse/parse/lighthouse_definition.go +++ b/internal/services/lighthouse/parse/lighthouse_definition.go @@ -3,13 +3,38 @@ package parse import ( "fmt" "strings" + + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" ) +var _ resourceids.Id = LighthouseDefinitionId{} + type LighthouseDefinitionId struct { Scope string LighthouseDefinitionID string } +func NewLighthouseDefinitionID(scope, lighthouseDefinitionId string) LighthouseDefinitionId { + return LighthouseDefinitionId{ + Scope: scope, + LighthouseDefinitionID: lighthouseDefinitionId, + } +} + +func (id LighthouseDefinitionId) ID() string { + fmtStr := "%s/providers/Microsoft.ManagedServices/registrationDefinitions/%s" + return fmt.Sprintf(fmtStr, id.Scope, id.LighthouseDefinitionID) +} + +func (id LighthouseDefinitionId) String() string { + segments := []string{ + fmt.Sprintf("Lighthouse Definition ID %q", id.LighthouseDefinitionID), + fmt.Sprintf("Scope %q", id.Scope), + } + segmentsStr := strings.Join(segments, " / ") + return fmt.Sprintf("%s: (%s)", "Lighthouse Assignment", segmentsStr) +} + func LighthouseDefinitionID(id string) (*LighthouseDefinitionId, error) { segments := strings.Split(id, "/providers/Microsoft.ManagedServices/registrationDefinitions/") From f97b2b15be4255b232d8d3e8b6ca39493048bc07 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Tue, 15 Mar 2022 11:41:09 +0100 Subject: [PATCH 5/6] loganalytics: removing unused code --- .../suppress/log_analytics_cluster.go | 27 ------ .../suppress/log_analytics_cluster_test.go | 87 ------------------- 2 files changed, 114 deletions(-) delete mode 100644 internal/services/loganalytics/suppress/log_analytics_cluster.go delete mode 100644 internal/services/loganalytics/suppress/log_analytics_cluster_test.go diff --git a/internal/services/loganalytics/suppress/log_analytics_cluster.go b/internal/services/loganalytics/suppress/log_analytics_cluster.go deleted file mode 100644 index edde20144d6f..000000000000 --- a/internal/services/loganalytics/suppress/log_analytics_cluster.go +++ /dev/null @@ -1,27 +0,0 @@ -package suppress - -import ( - "fmt" - "net" - "net/url" - - "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" -) - -func LogAnalyticsClusterUrl(_, old, new string, _ *pluginsdk.ResourceData) bool { - u, err := url.ParseRequestURI(old) - if err != nil || u.Host == "" { - return false - } - - host, _, err := net.SplitHostPort(u.Host) - if err != nil { - host = u.Host - } - - if new == fmt.Sprintf("%s://%s/", u.Scheme, host) { - return true - } - - return false -} diff --git a/internal/services/loganalytics/suppress/log_analytics_cluster_test.go b/internal/services/loganalytics/suppress/log_analytics_cluster_test.go deleted file mode 100644 index 9d160b7f3767..000000000000 --- a/internal/services/loganalytics/suppress/log_analytics_cluster_test.go +++ /dev/null @@ -1,87 +0,0 @@ -package suppress - -import "testing" - -func TestCaseClusterUrl(t *testing.T) { - cases := []struct { - Name string - ClusterURL string - KeyVaultURL string - Suppress bool - }{ - { - Name: "empty URL", - ClusterURL: "", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: false, - }, - { - Name: "URL with port and wrong scheme", - ClusterURL: "http://flynns.arcade.com:443", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: false, - }, - { - Name: "invalid URL scheme", - ClusterURL: "https//flynns.arcade.com", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: false, - }, - { - Name: "invalid URL character", - ClusterURL: "https://flynns^arcade.com/", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: false, - }, - { - Name: "invalid URL missing scheme", - ClusterURL: "//flynns.arcade.com/", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: false, - }, - { - Name: "URL with wrong scheme no port", - ClusterURL: "http://flynns.arcade.com", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: false, - }, - { - Name: "same URL different case", - ClusterURL: "https://Flynns.Arcade.com/", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: false, - }, - { - Name: "full URL with username@host/path?query#fragment", - ClusterURL: "https://Creator4983@flynns.arcade.com/ENCOM?games=MatrixBlaster#MCP", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: true, - }, - { - Name: "full URL with username:password@host/path?query#fragment", - ClusterURL: "https://Creator4983:7898@flynns.arcade.com/ENCOM?games=SpaceParanoids&developer=KevinFlynn#MCP", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: true, - }, - { - Name: "URL missing path separator", - ClusterURL: "https://flynns.arcade.com", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: true, - }, - { - Name: "same URL", - ClusterURL: "https://flynns.arcade.com/", - KeyVaultURL: "https://flynns.arcade.com/", - Suppress: true, - }, - } - - for _, tc := range cases { - t.Run(tc.Name, func(t *testing.T) { - if LogAnalyticsClusterUrl("test", tc.ClusterURL, tc.KeyVaultURL, nil) != tc.Suppress { - t.Fatalf("Expected LogAnalyticsClusterUrl to return %t for '%q' == '%q'", tc.Suppress, tc.ClusterURL, tc.KeyVaultURL) - } - }) - } -} From b715ff20fd6ba9b065b26e5604047aff7e5c7bdc Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Tue, 15 Mar 2022 11:41:29 +0100 Subject: [PATCH 6/6] r/log_analytics_saved_search: updating the casing of the resource ids --- .../log_analytics_saved_search_resource.go | 51 ++++------ .../migration/saved_search_v0_to_v1.go | 96 +++++++++++++++++++ 2 files changed, 114 insertions(+), 33 deletions(-) create mode 100644 internal/services/loganalytics/migration/saved_search_v0_to_v1.go diff --git a/internal/services/loganalytics/log_analytics_saved_search_resource.go b/internal/services/loganalytics/log_analytics_saved_search_resource.go index 45f4e17c519e..53de455e0507 100644 --- a/internal/services/loganalytics/log_analytics_saved_search_resource.go +++ b/internal/services/loganalytics/log_analytics_saved_search_resource.go @@ -2,11 +2,12 @@ package loganalytics import ( "fmt" - "log" "regexp" "strings" "time" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/loganalytics/migration" + "github.com/Azure/azure-sdk-for-go/services/operationalinsights/mgmt/2020-08-01/operationalinsights" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" @@ -14,7 +15,6 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/internal/services/loganalytics/validate" "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/suppress" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" "github.com/hashicorp/terraform-provider-azurerm/internal/timeouts" "github.com/hashicorp/terraform-provider-azurerm/utils" @@ -31,6 +31,11 @@ func resourceLogAnalyticsSavedSearch() *pluginsdk.Resource { return err }), + SchemaVersion: 1, + StateUpgraders: pluginsdk.StateUpgrades(map[int]pluginsdk.StateUpgrade{ + 0: migration.SavedSearchV0ToV1{}, + }), + Timeouts: &pluginsdk.ResourceTimeout{ Create: pluginsdk.DefaultTimeout(30 * time.Minute), Read: pluginsdk.DefaultTimeout(5 * time.Minute), @@ -44,16 +49,12 @@ func resourceLogAnalyticsSavedSearch() *pluginsdk.Resource { Required: true, ForceNew: true, ValidateFunc: validate.LogAnalyticsWorkspaceID, - // https://github.com/Azure/azure-rest-api-specs/issues/9330 - DiffSuppressFunc: suppress.CaseDifference, }, "name": { Type: pluginsdk.TypeString, Required: true, ForceNew: true, - // https://github.com/Azure/azure-rest-api-specs/issues/9330 - DiffSuppressFunc: suppress.CaseDifference, }, "category": { @@ -106,20 +107,18 @@ func resourceLogAnalyticsSavedSearchCreate(d *pluginsdk.ResourceData, meta inter client := meta.(*clients.Client).LogAnalytics.SavedSearchesClient ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() - log.Printf("[INFO] preparing arguments for AzureRM Log Analytics Saved Search creation.") - name := d.Get("name").(string) - workspaceID := d.Get("log_analytics_workspace_id").(string) - id, err := parse.LogAnalyticsWorkspaceID(workspaceID) + workspaceId, err := parse.LogAnalyticsWorkspaceID(d.Get("log_analytics_workspace_id").(string)) if err != nil { return err } + id := parse.NewLogAnalyticsSavedSearchID(workspaceId.SubscriptionId, workspaceId.ResourceGroup, workspaceId.WorkspaceName, d.Get("name").(string)) if d.IsNewResource() { - existing, err := client.Get(ctx, id.ResourceGroup, id.WorkspaceName, name) + existing, err := client.Get(ctx, id.ResourceGroup, id.WorkspaceName, id.SavedSearcheName) if err != nil { if !utils.ResponseWasNotFound(existing.Response) { - return fmt.Errorf("checking for presence of existing Log Analytics Saved Search %q (WorkSpace %q / Resource Group %q): %s", name, id.WorkspaceName, id.ResourceGroup, err) + return fmt.Errorf("checking for presence of existing %s: %+v", id, err) } } @@ -149,35 +148,22 @@ func resourceLogAnalyticsSavedSearchCreate(d *pluginsdk.ResourceData, meta inter parameters.SavedSearchProperties.FunctionParameters = utils.String(strings.Join(result, ", ")) } - if _, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.WorkspaceName, name, parameters); err != nil { - return fmt.Errorf("creating Saved Search %q (Log Analytics Workspace %q / Resource Group %q): %+v", name, id.WorkspaceName, id.ResourceGroup, err) + if _, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.WorkspaceName, id.SavedSearcheName, parameters); err != nil { + return fmt.Errorf("creating %s: %+v", id, err) } - read, err := client.Get(ctx, id.ResourceGroup, id.WorkspaceName, name) - if err != nil { - return fmt.Errorf("retrieving Saved Search %q (Log Analytics Workspace %q / Resource Group %q): %+v", name, id.WorkspaceName, id.ResourceGroup, err) - } - - if read.ID == nil { - return fmt.Errorf("cannot read Log Analytics Saved Search %q (WorkSpace %q / Resource Group %q): %s", name, id.WorkspaceName, id.ResourceGroup, err) - } - - d.SetId(*read.ID) - + d.SetId(id.ID()) return resourceLogAnalyticsSavedSearchRead(d, meta) } func resourceLogAnalyticsSavedSearchRead(d *pluginsdk.ResourceData, meta interface{}) error { client := meta.(*clients.Client).LogAnalytics.SavedSearchesClient - subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - // FIXME: @favoretti: API returns ID without a leading slash - id, err := parse.LogAnalyticsSavedSearchID(fmt.Sprintf("/%s", strings.TrimPrefix(d.Id(), "/"))) + id, err := parse.LogAnalyticsSavedSearchID(d.Id()) if err != nil { return err } - workspaceId := parse.NewLogAnalyticsWorkspaceID(subscriptionId, id.ResourceGroup, id.WorkspaceName).ID() resp, err := client.Get(ctx, id.ResourceGroup, id.WorkspaceName, id.SavedSearcheName) if err != nil { @@ -189,7 +175,7 @@ func resourceLogAnalyticsSavedSearchRead(d *pluginsdk.ResourceData, meta interfa } d.Set("name", id.SavedSearcheName) - d.Set("log_analytics_workspace_id", workspaceId) + d.Set("log_analytics_workspace_id", parse.NewLogAnalyticsWorkspaceID(id.SubscriptionId, id.ResourceGroup, id.WorkspaceName).ID()) if props := resp.SavedSearchProperties; props != nil { d.Set("display_name", props.DisplayName) @@ -215,14 +201,13 @@ func resourceLogAnalyticsSavedSearchDelete(d *pluginsdk.ResourceData, meta inter client := meta.(*clients.Client).LogAnalytics.SavedSearchesClient ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - // FIXME: @favoretti: API returns ID without a leading slash - id, err := parse.LogAnalyticsSavedSearchID(fmt.Sprintf("/%s", strings.TrimPrefix(d.Id(), "/"))) + id, err := parse.LogAnalyticsSavedSearchID(d.Id()) if err != nil { return err } if _, err = client.Delete(ctx, id.ResourceGroup, id.WorkspaceName, id.SavedSearcheName); err != nil { - return fmt.Errorf("deleting Saved Search %q (Log Analytics Workspace %q / Resource Group %q): %s", id.WorkspaceName, id.WorkspaceName, id.ResourceGroup, err) + return fmt.Errorf("deleting %s: %+v", *id, err) } return nil diff --git a/internal/services/loganalytics/migration/saved_search_v0_to_v1.go b/internal/services/loganalytics/migration/saved_search_v0_to_v1.go new file mode 100644 index 000000000000..2e5cfc85b73b --- /dev/null +++ b/internal/services/loganalytics/migration/saved_search_v0_to_v1.go @@ -0,0 +1,96 @@ +package migration + +import ( + "context" + "fmt" + "log" + "strings" + + "github.com/hashicorp/terraform-provider-azurerm/internal/services/loganalytics/parse" + + "github.com/hashicorp/terraform-provider-azurerm/internal/services/loganalytics/validate" + "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" + "github.com/hashicorp/terraform-provider-azurerm/internal/tf/suppress" +) + +var _ pluginsdk.StateUpgrade = SavedSearchV0ToV1{} + +type SavedSearchV0ToV1 struct{} + +func (SavedSearchV0ToV1) Schema() map[string]*pluginsdk.Schema { + return map[string]*pluginsdk.Schema{ + "log_analytics_workspace_id": { + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validate.LogAnalyticsWorkspaceID, + DiffSuppressFunc: suppress.CaseDifference, + }, + + "name": { + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + DiffSuppressFunc: suppress.CaseDifference, + }, + + "category": { + Type: pluginsdk.TypeString, + ForceNew: true, + Required: true, + }, + + "display_name": { + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + }, + + "query": { + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + }, + + "function_alias": { + Type: pluginsdk.TypeString, + Optional: true, + ForceNew: true, + }, + + "function_parameters": { + Type: pluginsdk.TypeSet, + Optional: true, + ForceNew: true, + Elem: &pluginsdk.Schema{ + Type: pluginsdk.TypeString, + }, + }, + + "tags": { + Type: pluginsdk.TypeMap, + Optional: true, + ForceNew: true, + Elem: &pluginsdk.Schema{ + Type: pluginsdk.TypeString, + }, + }, + } +} + +func (SavedSearchV0ToV1) UpgradeFunc() pluginsdk.StateUpgraderFunc { + return func(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + oldId := rawState["id"].(string) + + id, err := parse.LogAnalyticsSavedSearchID(fmt.Sprintf("/%s", strings.TrimPrefix(oldId, "/"))) + if err != nil { + return rawState, err + } + + newId := id.ID() + log.Printf("[DEBUG] Updating ID from %q to %q..", oldId, newId) + rawState["id"] = newId + + return rawState, nil + } +}