From 48e03db758c0efded0166819904e85f67174ad81 Mon Sep 17 00:00:00 2001 From: Daniel Corbett <38925638+daniel-corbett@users.noreply.github.com> Date: Thu, 2 May 2024 09:58:43 -0400 Subject: [PATCH] BUG/MEDIUM: edge: avoid setting state until success (#221) It was recently reported by @sloh-fastly that if a `sigsci_edge_deployment` was created without a `sigsci_site` it appeared to have been created within Terraform state. This caused some weird inconsistencies and could lead to the below error: ``` Error: Provider produced inconsistent result after apply When applying changes to sigsci_site.acme_nginx_waf["stg"], provider "provider[\"registry.terraform.io/signalsciences/sigsci\"]" produced an unexpected new value: Root resource was present, but now absent. This is a bug in the provider, which should be reported in the provider's own issue tracker. ``` Shu provided a thorough outline to reproduce, unfortunately I was not able to reproduce. However, I was able to identify what I believe to be happening. It seemed that even though the `sigsci_edge_deployment` was failing it was being populated into the terraform.tfstate file with a "tainted" status attached. It was found that the provider was prematurely calling `d.SetId()` without first checking if an error had occurred. This was causing Terraform to believe that it at least partially succeeded warranting it to be in a tainted state. This commit moves `d.SetId()` to only be called if the calls to `CreateOrUpdateEdgeDeployment()` and `CreateOrUpdateEdgeDeploymentService` were sucessful. This fixes the underlying issue and avoids populating the Terraform state with tainted versions of `sigsci_edge_deployment` and `sigsci_edge_deployment_service` if an error occurred. --- provider/resource_edge_deployment.go | 8 +++++++- provider/resource_edge_deployment_service.go | 12 +++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/provider/resource_edge_deployment.go b/provider/resource_edge_deployment.go index 891204d..cd69eba 100644 --- a/provider/resource_edge_deployment.go +++ b/provider/resource_edge_deployment.go @@ -24,9 +24,15 @@ func resourceEdgeDeployment() *schema.Resource { func createOrUpdateEdgeDeployment(d *schema.ResourceData, m interface{}) error { pm := m.(providerMetadata) + err := pm.Client.CreateOrUpdateEdgeDeployment(pm.Corp, d.Get("site_short_name").(string)) + + if err != nil { + return err + } + d.SetId(d.Get("site_short_name").(string)) - return pm.Client.CreateOrUpdateEdgeDeployment(pm.Corp, d.Get("site_short_name").(string)) + return nil } func readEdgeDeployment(d *schema.ResourceData, m interface{}) error { diff --git a/provider/resource_edge_deployment_service.go b/provider/resource_edge_deployment_service.go index 909afd6..5195051 100644 --- a/provider/resource_edge_deployment_service.go +++ b/provider/resource_edge_deployment_service.go @@ -45,13 +45,19 @@ func resourceEdgeDeploymentService() *schema.Resource { func createOrUpdateEdgeDeploymentService(d *schema.ResourceData, m interface{}) error { pm := m.(providerMetadata) - d.SetId(d.Get("fastly_sid").(string)) - activateVersion := d.Get("activate_version").(bool) - return pm.Client.CreateOrUpdateEdgeDeploymentService(pm.Corp, d.Get("site_short_name").(string), d.Get("fastly_sid").(string), sigsci.CreateOrUpdateEdgeDeploymentServiceBody{ + err := pm.Client.CreateOrUpdateEdgeDeploymentService(pm.Corp, d.Get("site_short_name").(string), d.Get("fastly_sid").(string), sigsci.CreateOrUpdateEdgeDeploymentServiceBody{ ActivateVersion: &activateVersion, PercentEnabled: d.Get("percent_enabled").(int), }) + + if err != nil { + return err + } + + d.SetId(d.Get("fastly_sid").(string)) + + return nil } func readEdgeDeploymentService(d *schema.ResourceData, m interface{}) error {