From fff26b1cfd7d434ca0cdc441759df831e1c8e8ad Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 18 May 2022 12:17:28 +0200 Subject: [PATCH 1/2] applicationinsights: working around the breaking upstream api change Fixes #16805 --- .../application_insights_webtests_resource.go | 8 ++- .../azuresdkhacks/webtests.go | 66 +++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 internal/services/applicationinsights/azuresdkhacks/webtests.go diff --git a/internal/services/applicationinsights/application_insights_webtests_resource.go b/internal/services/applicationinsights/application_insights_webtests_resource.go index a923438e9ad8..232dd6419fe5 100644 --- a/internal/services/applicationinsights/application_insights_webtests_resource.go +++ b/internal/services/applicationinsights/application_insights_webtests_resource.go @@ -7,6 +7,8 @@ import ( "strings" "time" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/applicationinsights/azuresdkhacks" + "github.com/Azure/azure-sdk-for-go/services/appinsights/mgmt/2020-02-02/insights" "github.com/hashicorp/go-azure-helpers/resourcemanager/location" "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" @@ -135,7 +137,7 @@ func resourceApplicationInsightsWebTests() *pluginsdk.Resource { } func resourceApplicationInsightsWebTestsCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) error { - client := meta.(*clients.Client).AppInsights.WebTestsClient + client := azuresdkhacks.NewWebTestsClient(meta.(*clients.Client).AppInsights.WebTestsClient) ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() @@ -149,7 +151,7 @@ func resourceApplicationInsightsWebTestsCreateUpdate(d *pluginsdk.ResourceData, id := parse.NewWebTestID(appInsightsId.SubscriptionId, d.Get("resource_group_name").(string), d.Get("name").(string)) if d.IsNewResource() { - existing, err := client.Get(ctx, id.ResourceGroup, id.Name) + existing, err := client.Get(ctx, id) if err != nil { if !utils.ResponseWasNotFound(existing.Response) { return fmt.Errorf("checking for presence of existing Application Insights %s: %+v", id, err) @@ -197,7 +199,7 @@ func resourceApplicationInsightsWebTestsCreateUpdate(d *pluginsdk.ResourceData, Tags: tags.Expand(t), } - _, err = client.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, webTest) + _, err = client.CreateOrUpdate(ctx, id, webTest) if err != nil { return fmt.Errorf("creating/updating Application Insights %s: %+v", id, err) } diff --git a/internal/services/applicationinsights/azuresdkhacks/webtests.go b/internal/services/applicationinsights/azuresdkhacks/webtests.go new file mode 100644 index 000000000000..44040555d9a3 --- /dev/null +++ b/internal/services/applicationinsights/azuresdkhacks/webtests.go @@ -0,0 +1,66 @@ +package azuresdkhacks + +import ( + "context" + "net/http" + + "github.com/Azure/azure-sdk-for-go/services/appinsights/mgmt/2020-02-02/insights" + "github.com/Azure/go-autorest/autorest" + "github.com/Azure/go-autorest/autorest/azure" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/applicationinsights/parse" +) + +type WebTestsClient struct { + client *insights.WebTestsClient +} + +func NewWebTestsClient(client *insights.WebTestsClient) WebTestsClient { + return WebTestsClient{ + client: client, + } +} + +// CreateOrUpdate is a workaround to handle that the Azure API can return either a 200 / 201 +// rather than the 200 documented in the Swagger - this is a workaround until the upstream PR is merged +// TF issue: https://github.com/hashicorp/terraform-provider-azurerm/issues/16805 +// Swagger PR: https://github.com/Azure/azure-rest-api-specs/pull/19104 +func (c WebTestsClient) CreateOrUpdate(ctx context.Context, id parse.WebTestId, webTestDefinition insights.WebTest) (result insights.WebTest, err error) { + req, err := c.client.CreateOrUpdatePreparer(ctx, id.ResourceGroup, id.Name, webTestDefinition) + if err != nil { + err = autorest.NewErrorWithError(err, "insights.WebTestsClient", "CreateOrUpdate", nil, "Failure preparing request") + return + } + + resp, err := c.client.CreateOrUpdateSender(req) + if err != nil { + result.Response = autorest.Response{Response: resp} + err = autorest.NewErrorWithError(err, "insights.WebTestsClient", "CreateOrUpdate", resp, "Failure sending request") + return + } + + result, err = c.createOrUpdateResponder(resp) + if err != nil { + err = autorest.NewErrorWithError(err, "insights.WebTestsClient", "CreateOrUpdate", resp, "Failure responding to request") + return + } + + return +} + +// createOrUpdateResponder is a workaround to handle that the Azure API can return either a 200 / 201 +// rather than the 200 documented in the Swagger - this is a workaround until the upstream PR is merged +// TF issue: https://github.com/hashicorp/terraform-provider-azurerm/issues/16805 +// Swagger PR: https://github.com/Azure/azure-rest-api-specs/pull/19104 +func (c WebTestsClient) createOrUpdateResponder(resp *http.Response) (result insights.WebTest, err error) { + err = autorest.Respond( + resp, + azure.WithErrorUnlessStatusCode(http.StatusOK, http.StatusCreated), + autorest.ByUnmarshallingJSON(&result), + autorest.ByClosing()) + result.Response = autorest.Response{Response: resp} + return +} + +func (c WebTestsClient) Get(ctx context.Context, id parse.WebTestId) (result insights.WebTest, err error) { + return c.client.Get(ctx, id.ResourceGroup, id.Name) +} From c49b7ab2adb5eb9dd4d81bd3b9dfe148529ffd2f Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 18 May 2022 12:33:50 +0200 Subject: [PATCH 2/2] refactor: switching to use the workaround client for webtests everywhere This switches to using the Resource ID rather than the individual segments, since we'll be refactoring this soon anyway we may as well wrap this now. --- .../application_insights_webtests_resource.go | 8 +++----- .../application_insights_webtests_resource_test.go | 2 +- .../applicationinsights/azuresdkhacks/webtests.go | 8 ++++++-- internal/services/applicationinsights/client/client.go | 6 ++++-- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/internal/services/applicationinsights/application_insights_webtests_resource.go b/internal/services/applicationinsights/application_insights_webtests_resource.go index 232dd6419fe5..7f2f29dc639d 100644 --- a/internal/services/applicationinsights/application_insights_webtests_resource.go +++ b/internal/services/applicationinsights/application_insights_webtests_resource.go @@ -7,8 +7,6 @@ import ( "strings" "time" - "github.com/hashicorp/terraform-provider-azurerm/internal/services/applicationinsights/azuresdkhacks" - "github.com/Azure/azure-sdk-for-go/services/appinsights/mgmt/2020-02-02/insights" "github.com/hashicorp/go-azure-helpers/resourcemanager/location" "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" @@ -137,7 +135,7 @@ func resourceApplicationInsightsWebTests() *pluginsdk.Resource { } func resourceApplicationInsightsWebTestsCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) error { - client := azuresdkhacks.NewWebTestsClient(meta.(*clients.Client).AppInsights.WebTestsClient) + client := meta.(*clients.Client).AppInsights.WebTestsClient ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() @@ -221,7 +219,7 @@ func resourceApplicationInsightsWebTestsRead(d *pluginsdk.ResourceData, meta int log.Printf("[DEBUG] Reading AzureRM Application Insights %q", *id) - resp, err := client.Get(ctx, id.ResourceGroup, id.Name) + resp, err := client.Get(ctx, *id) if err != nil { if utils.ResponseWasNotFound(resp.Response) { log.Printf("[DEBUG] Application Insights %s was not found - removing from state!", *id) @@ -282,7 +280,7 @@ func resourceApplicationInsightsWebTestsDelete(d *pluginsdk.ResourceData, meta i log.Printf("[DEBUG] Deleting AzureRM Application Insights %s", *id) - resp, err := client.Delete(ctx, id.ResourceGroup, id.Name) + resp, err := client.Delete(ctx, *id) if err != nil { if resp.StatusCode == http.StatusNotFound { return nil diff --git a/internal/services/applicationinsights/application_insights_webtests_resource_test.go b/internal/services/applicationinsights/application_insights_webtests_resource_test.go index 699bd9192095..a781c6879740 100644 --- a/internal/services/applicationinsights/application_insights_webtests_resource_test.go +++ b/internal/services/applicationinsights/application_insights_webtests_resource_test.go @@ -101,7 +101,7 @@ func (t AppInsightsWebTestsResource) Exists(ctx context.Context, clients *client return nil, err } - resp, err := clients.AppInsights.WebTestsClient.Get(ctx, id.ResourceGroup, id.Name) + resp, err := clients.AppInsights.WebTestsClient.Get(ctx, *id) if err != nil { return nil, fmt.Errorf("retrieving Application Insights '%q' (resource group: '%q') does not exist", id.ResourceGroup, id.Name) } diff --git a/internal/services/applicationinsights/azuresdkhacks/webtests.go b/internal/services/applicationinsights/azuresdkhacks/webtests.go index 44040555d9a3..76ae70e05907 100644 --- a/internal/services/applicationinsights/azuresdkhacks/webtests.go +++ b/internal/services/applicationinsights/azuresdkhacks/webtests.go @@ -14,9 +14,9 @@ type WebTestsClient struct { client *insights.WebTestsClient } -func NewWebTestsClient(client *insights.WebTestsClient) WebTestsClient { +func NewWebTestsClient(client insights.WebTestsClient) WebTestsClient { return WebTestsClient{ - client: client, + client: &client, } } @@ -61,6 +61,10 @@ func (c WebTestsClient) createOrUpdateResponder(resp *http.Response) (result ins return } +func (c WebTestsClient) Delete(ctx context.Context, id parse.WebTestId) (result autorest.Response, err error) { + return c.client.Delete(ctx, id.ResourceGroup, id.Name) +} + func (c WebTestsClient) Get(ctx context.Context, id parse.WebTestId) (result insights.WebTest, err error) { return c.client.Get(ctx, id.ResourceGroup, id.Name) } diff --git a/internal/services/applicationinsights/client/client.go b/internal/services/applicationinsights/client/client.go index 451701be7337..4907c213e532 100644 --- a/internal/services/applicationinsights/client/client.go +++ b/internal/services/applicationinsights/client/client.go @@ -3,13 +3,14 @@ package client import ( "github.com/Azure/azure-sdk-for-go/services/appinsights/mgmt/2020-02-02/insights" "github.com/hashicorp/terraform-provider-azurerm/internal/common" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/applicationinsights/azuresdkhacks" ) type Client struct { AnalyticsItemsClient *insights.AnalyticsItemsClient APIKeysClient *insights.APIKeysClient ComponentsClient *insights.ComponentsClient - WebTestsClient *insights.WebTestsClient + WebTestsClient *azuresdkhacks.WebTestsClient BillingClient *insights.ComponentCurrentBillingFeaturesClient SmartDetectionRuleClient *insights.ProactiveDetectionConfigurationsClient } @@ -26,6 +27,7 @@ func NewClient(o *common.ClientOptions) *Client { webTestsClient := insights.NewWebTestsClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId) o.ConfigureClient(&webTestsClient.Client, o.ResourceManagerAuthorizer) + webTestsWorkaroundClient := azuresdkhacks.NewWebTestsClient(webTestsClient) billingClient := insights.NewComponentCurrentBillingFeaturesClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId) o.ConfigureClient(&billingClient.Client, o.ResourceManagerAuthorizer) @@ -37,7 +39,7 @@ func NewClient(o *common.ClientOptions) *Client { AnalyticsItemsClient: &analyticsItemsClient, APIKeysClient: &apiKeysClient, ComponentsClient: &componentsClient, - WebTestsClient: &webTestsClient, + WebTestsClient: &webTestsWorkaroundClient, BillingClient: &billingClient, SmartDetectionRuleClient: &smartDetectionRuleClient, }