Skip to content

Commit

Permalink
fix(resources/servicevcl): set LastActive after Update activates a ne…
Browse files Browse the repository at this point in the history
…w service version
  • Loading branch information
Integralist committed Nov 1, 2023
1 parent 96273bd commit 129b261
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 4 deletions.
5 changes: 4 additions & 1 deletion internal/provider/resources/servicevcl/process_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,16 @@ func (r *Resource) Update(ctx context.Context, req resource.UpdateRequest, resp

if nestedResourcesChanged && plan.Activate.ValueBool() {
clientReq := r.client.VersionAPI.ActivateServiceVersion(r.clientCtx, plan.ID.ValueString(), serviceVersion)
_, httpResp, err := clientReq.Execute()
clientResp, httpResp, err := clientReq.Execute()
if err != nil {
tflog.Trace(ctx, "Fastly VersionAPI.ActivateServiceVersion error", map[string]any{"http_resp": httpResp})
resp.Diagnostics.AddError(helpers.ErrorAPIClient, fmt.Sprintf("Unable to activate service version %d, got error: %s", 1, err))
return
}
defer httpResp.Body.Close()

// Only set LastActive if we successfully activate the service.
plan.LastActive = types.Int64Value(int64(clientResp.GetNumber()))
}

// NOTE: The service attributes (Name, Comment) are 'versionless'.
Expand Down
52 changes: 49 additions & 3 deletions internal/provider/tests/resources/service_vcl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func TestAccResourceServiceVCLImportServiceVersion(t *testing.T) {
})
}

// The following test validates two scenarios.
// The following test validates two state drift scenarios.
//
// The first is that when we have more than one service version, where the
// latter version is not 'active' (because the user has set `activate=false`),
Expand Down Expand Up @@ -381,7 +381,9 @@ func TestAccResourceServiceVCLStateDrift(t *testing.T) {
domain2Name: domain2Name,
})

// Update the first domain's comment + set `activate=false`.
// Update the first domain by adding a comment + set `activate=false`.
// This will result in a new service version that's inactive.
// We want Terraform to be tracking this version and not the active version.
configUpdate1 := fmt.Sprintf(`
resource "fastly_service_vcl" "test" {
activate = false
Expand All @@ -401,6 +403,8 @@ func TestAccResourceServiceVCLStateDrift(t *testing.T) {
`, serviceName, domain1Name, domain1CommentAdded, domain2Name)

// Update the first domain's comment.
// This will result in a new service version that's inactive.
// We want Terraform to be tracking this version and not the prior inactive version.
configUpdate2 := fmt.Sprintf(`
resource "fastly_service_vcl" "test" {
activate = false
Expand All @@ -419,6 +423,28 @@ func TestAccResourceServiceVCLStateDrift(t *testing.T) {
}
`, serviceName, domain1Name, domain1CommentUpdated, domain2Name)

// Update the first domain by adding a comment.
// This will result in a new service version that's active.
// We'll then cause a side-effect that re-activates version 1.
// We want Terraform to track version=1 and last_active=2.
configUpdate3 := fmt.Sprintf(`
resource "fastly_service_vcl" "test" {
activate = true
force_destroy = true
name = "%s"
domains = {
"example-1" = {
name = "%s"
comment = "%s"
},
"example-2" = {
name = "%s"
},
}
}
`, serviceName, domain1Name, domain1CommentAdded, domain2Name)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { provider.TestAccPreCheck(t) },
ProtoV6ProviderFactories: provider.TestAccProtoV6ProviderFactories,
Expand Down Expand Up @@ -493,7 +519,7 @@ func TestAccResourceServiceVCLStateDrift(t *testing.T) {
//
// NOTE: This test case validates the 'no active version' behaviour.
// Which is we'll track the latest service version, not the last active.
// i.e. If `activate=false` then `last_active` is never set. Because
// i.e. If `activate=false` then `last_active` is never set.
//
// Terraform's import test behaviour is to compare the imported state to
// the previous state, so as the last step test found `version` to be 2,
Expand All @@ -506,6 +532,26 @@ func TestAccResourceServiceVCLStateDrift(t *testing.T) {
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"activate", "domain", "force_destroy"},
},
// Delete resource by emptying the TF config
{
Config: `# can't use an empty string`,
},
// Create and Read testing
{
Config: configCreate1,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("fastly_service_vcl.test", "last_active", "1"),
resource.TestCheckResourceAttr("fastly_service_vcl.test", "version", "1"),
),
},
// Update and Read testing
{
Config: configUpdate3,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("fastly_service_vcl.test", "last_active", "2"),
resource.TestCheckResourceAttr("fastly_service_vcl.test", "version", "2"),
),
},
// Delete testing automatically occurs at the end of the TestCase.
},
})
Expand Down

0 comments on commit 129b261

Please sign in to comment.