diff --git a/CHANGELOG.md b/CHANGELOG.md index da725cd..64b4634 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 2.5.2 + +BUG FIXES: + +- Fix drift detection not triggering Terraform plan changes: remote response drift now forces resource replacement via `ModifyPlan` instead of relying on computed `drift_marker` updates during `Read()`. Closes #133. +- Preserve stored `response` / `sensitive_response` in state when drift is detected during `Read()` until replacement completes. + ## 2.5.1 BUG FIXES: diff --git a/docs/resources/request.md b/docs/resources/request.md index 0fc6815..784e763 100644 --- a/docs/resources/request.md +++ b/docs/resources/request.md @@ -1,5 +1,4 @@ --- -# generated by https://github.com/hashicorp/terraform-plugin-docs page_title: "terracurl_request Resource - terracurl" subcategory: "" description: |- @@ -10,7 +9,9 @@ description: |- TerraCurl request resource +## Drift detection +When `skip_read` is `false` and `read_url`, `read_method`, and `read_response_codes` are configured, TerraCurl compares the stored response with the live read response during planning. If the remote state diverges (including an unexpected read HTTP status code), Terraform plans a **replace** (destroy then create) so configuration can be reconciled. Use `ignore_response_fields` to exclude volatile JSON fields from comparison. Drift remediation is replace-only; there is no in-place update of remote state. ## Schema @@ -68,9 +69,11 @@ TerraCurl request resource ### Read-Only - `destroy_request_url_string` (String) Destroy request URL includes parameters if request specified -- `drift_marker` (String) Marker to track state drift and trigger resource replacement +- `drift_marker` (String) Informational marker updated when remote drift is detected during read. Replacement is planned via ModifyPlan when drift is detected. - `id` (String) Example identifier - `request_url_string` (String) Request URL includes parameters if request specified - `response` (String) JSON response received from request. Empty when `response_sensitive` is `true`; use `sensitive_response` instead. - `sensitive_response` (String, Sensitive) JSON response received from request, marked as sensitive so it is not displayed in plan output. Populated only when `response_sensitive` is `true`. - `status_code` (String) Response status code received from request + + diff --git a/internal/provider/curl_resource.go b/internal/provider/curl_resource.go index 40fff52..5bf37a2 100644 --- a/internal/provider/curl_resource.go +++ b/internal/provider/curl_resource.go @@ -34,6 +34,7 @@ const ( // Ensure provider defined types fully satisfy framework interfaces. var _ resource.Resource = &CurlResource{} var _ resource.ResourceWithImportState = &CurlResource{} +var _ resource.ResourceWithModifyPlan = &CurlResource{} func NewCurlResource() resource.Resource { return &CurlResource{} @@ -405,7 +406,7 @@ func (r *CurlResource) Schema(ctx context.Context, req resource.SchemaRequest, r }, "drift_marker": schema.StringAttribute{ Computed: true, - MarkdownDescription: "Marker to track state drift and trigger resource replacement", + MarkdownDescription: "Informational marker updated when remote drift is detected during read. Replacement is planned via ModifyPlan when drift is detected.", PlanModifiers: []planmodifier.String{ stringplanmodifier.RequiresReplace(), }, @@ -612,31 +613,23 @@ func (r *CurlResource) Create(ctx context.Context, req resource.CreateRequest, r resp.Diagnostics.Append(diags...) } -func (r *CurlResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { - var data CurlResourceModel - - // Load prior state - resp.Diagnostics.Append(req.State.Get(ctx, &data)...) - if resp.Diagnostics.HasError() { - return - } +type readDriftResult struct { + Drifted bool + SanitizedResponse string + StatusCode int +} - // Skip read if configured - if data.SkipRead.ValueBool() { - tflog.Debug(ctx, "Skipping Read() as skip_read is true") - return - } - // ======= Validate Required Read Arguments ======= - if data.ReadUrl.IsNull() || data.ReadMethod.IsNull() || data.ReadResponseCodes.IsNull() { - resp.Diagnostics.AddError( - "Read Configuration Error", - "`read_url`, `read_method`, and `read_response_codes` are required when `skip_read` is false.", - ) - return +func ignoredResponseFields(data CurlResourceModel) []string { + var ignoredFields []string + for _, v := range data.IgnoreResponseFields.Elements() { + if strVal, ok := v.(types.String); ok { + ignoredFields = append(ignoredFields, strVal.ValueString()) + } } + return ignoredFields +} - // ======= Build TLS Client if `read_*` TLS Arguments Provided ======= - var client *http.Client +func (r *CurlResource) executeReadRequest(ctx context.Context, data CurlResourceModel) (statusCode int, body string, diags diag.Diagnostics) { useReadTls := !data.ReadCertFile.IsNull() || !data.ReadKeyFile.IsNull() || !data.ReadCaCertFile.IsNull() var readTlsConfig *TlsConfig @@ -654,14 +647,12 @@ func (r *CurlResource) Read(ctx context.Context, req resource.ReadRequest, resp tflog.Debug(ctx, "Using default HTTP client for Read() operation") } - var err error - client, err = r.providerMeta().NewHTTPClient(readTlsConfig) + client, err := r.providerMeta().NewHTTPClient(readTlsConfig) if err != nil { - resp.Diagnostics.AddError("Read Error", fmt.Sprintf("Failed to create HTTP client: %s", err)) + diags.AddError("Read Error", fmt.Sprintf("Failed to create HTTP client: %s", err)) return } - // ======= Build Read Request ======= var reqBody io.Reader = nil if !data.ReadRequestBody.IsNull() && !data.ReadRequestBody.IsUnknown() { reqBody = bytes.NewBuffer([]byte(data.ReadRequestBody.ValueString())) @@ -669,14 +660,12 @@ func (r *CurlResource) Read(ctx context.Context, req resource.ReadRequest, resp request, err := http.NewRequest(data.ReadMethod.ValueString(), data.ReadUrl.ValueString(), reqBody) if err != nil { - resp.Diagnostics.AddError("Read Error", fmt.Sprintf("Failed to create request: %s", err)) + diags.AddError("Read Error", fmt.Sprintf("Failed to create request: %s", err)) return } - // ======= Add Headers ======= applyRequestHeaders(request, data.ReadHeaders) - // ======= Add Query Parameters ======= if !data.ReadParameters.IsNull() && !data.ReadParameters.IsUnknown() { params := request.URL.Query() for k, v := range data.ReadParameters.Elements() { @@ -687,67 +676,154 @@ func (r *CurlResource) Read(ctx context.Context, req resource.ReadRequest, resp request.URL.RawQuery = params.Encode() } - // ======= Execute Request ======= tflog.Debug(ctx, fmt.Sprintf("Resource read API Call: \nURL: %s\nHeaders: %s\nMethod: %s\nRequest Body: %s\n", request.URL.String(), request.Header, request.Method, data.ReadRequestBody.ValueString())) httpResp, err := client.Do(request) if err != nil { - resp.Diagnostics.AddError("Read Error", fmt.Sprintf("Failed to call API: %s", err)) + diags.AddError("Read Error", fmt.Sprintf("Failed to call API: %s", err)) return } defer func(Body io.ReadCloser) { - err := Body.Close() - if err != nil { - return - } + _ = Body.Close() }(httpResp.Body) - // Read and store the response bodyBytes, _ := io.ReadAll(httpResp.Body) - newResponse := string(bodyBytes) + return httpResp.StatusCode, string(bodyBytes), diags +} - // ===== DRIFT DETECTION ===== +func checkReadDrift(data CurlResourceModel, statusCode int, liveResponse string) (drifted bool, sanitizedLive string, diags diag.Diagnostics) { + ignoredFields := ignoredResponseFields(data) - var ignoredFields []string - for _, v := range data.IgnoreResponseFields.Elements() { - if strVal, ok := v.(types.String); ok { - ignoredFields = append(ignoredFields, strVal.ValueString()) - } - } - - sanitizedResponse, err := sanitizeResponse(newResponse, ignoredFields) + var err error + sanitizedLive, err = sanitizeResponse(liveResponse, ignoredFields) if err != nil { - resp.Diagnostics.AddError("Sanitize Error", fmt.Sprintf("Failed to sanitize stored response: %s", err)) + diags.AddError("Sanitize Error", fmt.Sprintf("Failed to sanitize live response: %s", err)) return } - // Compare old and new sanitized responses. The prior response is stored in - // whichever attribute matches the current response_sensitive setting. priorResponse := priorResponseValue(data.ResponseSensitive, data.Response, data.SensitiveResponse) oldSanitized, err := sanitizeResponse(priorResponse, ignoredFields) if err != nil { - resp.Diagnostics.AddError("Sanitize Error", fmt.Sprintf("Failed to sanitize prior response: %s", err)) + diags.AddError("Sanitize Error", fmt.Sprintf("Failed to sanitize prior response: %s", err)) return } - // Drift detection - if !responseCodeChecker(data.ReadResponseCodes, httpResp.StatusCode) || (oldSanitized != "null" && oldSanitized != sanitizedResponse) { + if !responseCodeChecker(data.ReadResponseCodes, statusCode) { + return true, sanitizedLive, diags + } + + if oldSanitized != "null" && oldSanitized != sanitizedLive { + return true, sanitizedLive, diags + } + + return false, sanitizedLive, diags +} + +func (r *CurlResource) detectReadDrift(ctx context.Context, data CurlResourceModel) (readDriftResult, diag.Diagnostics) { + var result readDriftResult + var diags diag.Diagnostics + + if data.SkipRead.ValueBool() { + return result, diags + } + + if data.ReadUrl.IsNull() || data.ReadMethod.IsNull() || data.ReadResponseCodes.IsNull() { + diags.AddError( + "Read Configuration Error", + "`read_url`, `read_method`, and `read_response_codes` are required when `skip_read` is false.", + ) + return result, diags + } + + statusCode, body, execDiags := r.executeReadRequest(ctx, data) + diags.Append(execDiags...) + if diags.HasError() { + return result, diags + } + + result.StatusCode = statusCode + drifted, sanitizedLive, compareDiags := checkReadDrift(data, statusCode, body) + diags.Append(compareDiags...) + if diags.HasError() { + return result, diags + } + + result.Drifted = drifted + result.SanitizedResponse = sanitizedLive + return result, diags +} + +func (r *CurlResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + var data CurlResourceModel + + resp.Diagnostics.Append(req.State.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + + if data.SkipRead.ValueBool() { + tflog.Debug(ctx, "Skipping Read() as skip_read is true") + return + } + + result, diags := r.detectReadDrift(ctx, data) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + if result.Drifted { tflog.Warn(ctx, "Drift detected: Response has changed, marking for recreation.") data.DriftMarker = types.StringValue(time.Now().Format(time.RFC3339Nano)) } else { - // Set an initial drift marker if none exists if data.DriftMarker.IsNull() || data.DriftMarker.IsUnknown() { data.DriftMarker = types.StringValue("initial") } + setResourceResponseValues(&data, result.SanitizedResponse) } - // Store the new sanitized response in the appropriate attribute. - setResourceResponseValues(&data, sanitizedResponse) - resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) } +func (r *CurlResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { + if req.State.Raw.IsNull() || req.Plan.Raw.IsNull() { + return + } + + var data CurlResourceModel + resp.Diagnostics.Append(req.State.Get(ctx, &data)...) + if resp.Diagnostics.HasError() { + return + } + + if data.SkipRead.ValueBool() { + return + } + + result, diags := r.detectReadDrift(ctx, data) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + if result.Drifted { + var plan CurlResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if resp.Diagnostics.HasError() { + return + } + + plan.DriftMarker = types.StringValue(time.Now().Format(time.RFC3339Nano)) + resp.Diagnostics.Append(resp.Plan.Set(ctx, &plan)...) + resp.RequiresReplace.Append(path.Root("drift_marker")) + resp.Diagnostics.AddWarning( + "Remote Drift Detected", + "The read response does not match stored state. Terraform will replace this resource to reconcile configuration.", + ) + } +} + func (r *CurlResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { var data CurlResourceModel diff --git a/internal/provider/curl_resource_test.go b/internal/provider/curl_resource_test.go index 1600de3..d5e7f8b 100644 --- a/internal/provider/curl_resource_test.go +++ b/internal/provider/curl_resource_test.go @@ -493,7 +493,7 @@ func TestAccresourceCurlRead(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, - CheckDestroy: testMockEndpointCount("GET https://example.com/read", 1), + CheckDestroy: testMockEndpointRegister("GET https://example.com/read"), Steps: []resource.TestStep{ { Config: testAccresourceCurlRead(rName, RequestBody), @@ -1406,6 +1406,10 @@ func TestCurlResource_Read_ResponseSensitiveToggle(t *testing.T) { t.Error("Expected drift to be detected after response changed, but drift_marker remained 'initial'") } + if stateAfterRead2.Response.ValueString() != initialResponse { + t.Errorf("Expected stored response to remain unchanged on drift, got %s", stateAfterRead2.Response.ValueString()) + } + stateWithReverseToggle := CurlResourceModel{ Id: types.StringValue("test"), Name: types.StringValue("test"), @@ -1456,6 +1460,238 @@ func TestCurlResource_Read_ResponseSensitiveToggle(t *testing.T) { if stateAfterRead3.DriftMarker.ValueString() == "initial" { t.Error("Expected drift to be detected after reverse toggle, but drift_marker remained 'initial'") } + + if stateAfterRead3.SensitiveResponse.ValueString() != changedResponse { + t.Errorf("Expected stored sensitive response to remain unchanged on drift, got %s", stateAfterRead3.SensitiveResponse.ValueString()) + } +} + +func TestCheckReadDrift_NullPriorNoFalsePositive(t *testing.T) { + data := CurlResourceModel{ + ReadResponseCodes: types.ListValueMust(types.StringType, []attr.Value{types.StringValue("200")}), + Response: types.StringValue("null"), + ResponseSensitive: types.BoolValue(false), + IgnoreResponseFields: types.ListNull(types.StringType), + } + + drifted, _, diags := checkReadDrift(data, 200, "null") + if diags.HasError() { + t.Fatalf("unexpected error: %v", diags) + } + if drifted { + t.Error("expected no drift when prior sanitizes to null and live response is null") + } +} + +func TestCurlResource_ModifyPlan_ReadDrift(t *testing.T) { + t.Setenv("USE_DEFAULT_CLIENT_FOR_TESTS", "true") + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + storedResponse := `{"key":"value1"}` + remoteResponse := `{"key":"value2"}` + + httpmock.RegisterResponder( + "GET", + "https://example.com/read", + httpmock.NewStringResponder(200, remoteResponse), + ) + + ctx := context.Background() + r := &CurlResource{meta: DefaultProviderMeta()} + + schemaResp := &resource2.SchemaResponse{} + r.Schema(ctx, resource2.SchemaRequest{}, schemaResp) + + stateModel := CurlResourceModel{ + Id: types.StringValue("test"), + Name: types.StringValue("test"), + Url: types.StringValue("https://example.com/create"), + Method: types.StringValue("POST"), + SkipRead: types.BoolValue(false), + ReadUrl: types.StringValue("https://example.com/read"), + ReadMethod: types.StringValue("GET"), + ReadResponseCodes: types.ListValueMust(types.StringType, []attr.Value{types.StringValue("200")}), + ResponseCodes: types.ListValueMust(types.StringType, []attr.Value{types.StringValue("200")}), + Response: types.StringValue(storedResponse), + SensitiveResponse: types.StringValue(""), + ResponseSensitive: types.BoolValue(false), + DriftMarker: types.StringValue("initial"), + IgnoreResponseFields: types.ListNull(types.StringType), + DestroyResponseCodes: types.ListNull(types.StringType), + SkipDestroy: types.BoolValue(true), + Headers: types.MapNull(types.StringType), + RequestParameters: types.MapNull(types.StringType), + ReadHeaders: types.MapNull(types.StringType), + ReadParameters: types.MapNull(types.StringType), + DestroyHeaders: types.MapNull(types.StringType), + DestroyRequestParameters: types.MapNull(types.StringType), + } + + state := tfsdk.State{Schema: schemaResp.Schema} + if diags := state.Set(ctx, &stateModel); diags.HasError() { + t.Fatalf("failed to set state: %v", diags) + } + + plan := tfsdk.Plan{Schema: schemaResp.Schema} + if diags := plan.Set(ctx, &stateModel); diags.HasError() { + t.Fatalf("failed to set plan: %v", diags) + } + + config := tfsdk.Config{Schema: schemaResp.Schema} + + modifyResp := &resource2.ModifyPlanResponse{Plan: plan} + r.ModifyPlan(ctx, resource2.ModifyPlanRequest{ + State: state, + Plan: plan, + Config: config, + }, modifyResp) + + if modifyResp.Diagnostics.HasError() { + t.Fatalf("ModifyPlan failed: %v", modifyResp.Diagnostics) + } + + if len(modifyResp.RequiresReplace) == 0 { + t.Fatal("expected RequiresReplace when read drift is detected") + } +} + +func TestCurlResource_ModifyPlan_BadReadStatusCode(t *testing.T) { + t.Setenv("USE_DEFAULT_CLIENT_FOR_TESTS", "true") + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + httpmock.RegisterResponder( + "GET", + "https://example.com/read", + httpmock.NewStringResponder(500, `{"error":"failed"}`), + ) + + ctx := context.Background() + r := &CurlResource{meta: DefaultProviderMeta()} + + schemaResp := &resource2.SchemaResponse{} + r.Schema(ctx, resource2.SchemaRequest{}, schemaResp) + + stateModel := CurlResourceModel{ + Id: types.StringValue("test"), + Name: types.StringValue("test"), + Url: types.StringValue("https://example.com/create"), + Method: types.StringValue("POST"), + SkipRead: types.BoolValue(false), + ReadUrl: types.StringValue("https://example.com/read"), + ReadMethod: types.StringValue("GET"), + ReadResponseCodes: types.ListValueMust(types.StringType, []attr.Value{types.StringValue("200")}), + ResponseCodes: types.ListValueMust(types.StringType, []attr.Value{types.StringValue("200")}), + Response: types.StringValue(`{"key":"value1"}`), + SensitiveResponse: types.StringValue(""), + ResponseSensitive: types.BoolValue(false), + DriftMarker: types.StringValue("initial"), + IgnoreResponseFields: types.ListNull(types.StringType), + DestroyResponseCodes: types.ListNull(types.StringType), + SkipDestroy: types.BoolValue(true), + Headers: types.MapNull(types.StringType), + RequestParameters: types.MapNull(types.StringType), + ReadHeaders: types.MapNull(types.StringType), + ReadParameters: types.MapNull(types.StringType), + DestroyHeaders: types.MapNull(types.StringType), + DestroyRequestParameters: types.MapNull(types.StringType), + } + + state := tfsdk.State{Schema: schemaResp.Schema} + if diags := state.Set(ctx, &stateModel); diags.HasError() { + t.Fatalf("failed to set state: %v", diags) + } + + plan := tfsdk.Plan{Schema: schemaResp.Schema} + if diags := plan.Set(ctx, &stateModel); diags.HasError() { + t.Fatalf("failed to set plan: %v", diags) + } + + config := tfsdk.Config{Schema: schemaResp.Schema} + + modifyResp := &resource2.ModifyPlanResponse{Plan: plan} + r.ModifyPlan(ctx, resource2.ModifyPlanRequest{ + State: state, + Plan: plan, + Config: config, + }, modifyResp) + + if modifyResp.Diagnostics.HasError() { + t.Fatalf("ModifyPlan failed: %v", modifyResp.Diagnostics) + } + + if len(modifyResp.RequiresReplace) == 0 { + t.Fatal("expected RequiresReplace when read status code is unexpected") + } +} + +func TestAccCurlResourceDriftTriggersReplace(t *testing.T) { + t.Setenv("TF_ACC", "true") + t.Setenv("USE_DEFAULT_CLIENT_FOR_TESTS", "true") + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + httpmock.RegisterResponder( + "POST", + "https://example.com/create", + httpmock.NewStringResponder(200, `{"key":"value1"}`), + ) + httpmock.RegisterResponder( + "GET", + "https://example.com/read", + httpmock.NewStringResponder(200, `{"key":"value1"}`), + ) + + rName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + config := testAccCurlResourceDriftDetection(rName) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: config, + }, + { + PreConfig: func() { + httpmock.RegisterResponder( + "GET", + "https://example.com/read", + httpmock.NewStringResponder(200, `{"key":"value2"}`), + ) + }, + Config: config, + PlanOnly: true, + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func testAccCurlResourceDriftDetection(name string) string { + return fmt.Sprintf(` +resource "terracurl_request" "test" { + name = "%s" + url = "https://example.com/create" + method = "POST" + response_codes = ["200"] + + request_body = jsonencode({ + key = "value1" + }) + + skip_read = false + read_url = "https://example.com/read" + read_method = "GET" + read_response_codes = ["200"] + + skip_destroy = true +} +`, name) } // TestCurlResource_StateUpgrade_EmptyDestroyParameters tests handling of null destroy_parameters. diff --git a/templates/resources/request.md.tmpl b/templates/resources/request.md.tmpl new file mode 100644 index 0000000..d140785 --- /dev/null +++ b/templates/resources/request.md.tmpl @@ -0,0 +1,16 @@ +--- +page_title: "{{ .Name }} {{ .Type }} - {{ .ProviderName }}" +subcategory: "" +description: |- + TerraCurl request resource +--- + +# {{ .Name }} ({{ .Type }}) + +TerraCurl request resource + +## Drift detection + +When `skip_read` is `false` and `read_url`, `read_method`, and `read_response_codes` are configured, TerraCurl compares the stored response with the live read response during planning. If the remote state diverges (including an unexpected read HTTP status code), Terraform plans a **replace** (destroy then create) so configuration can be reconciled. Use `ignore_response_fields` to exclude volatile JSON fields from comparison. Drift remediation is replace-only; there is no in-place update of remote state. + +{{ .SchemaMarkdown }}