fix: verify deletion before returning to prevent race conditions (#26)

* fix: verify deletion before returning to prevent race conditions

Pi-hole's API can return 204 (success) before a delete is fully
processed internally. When Terraform's ForceNew triggers a delete+create
sequence, this race condition causes "item already present" errors.

Changes:
- Add verification loop in Delete() to poll until record is confirmed gone
- Log warning when delete is called for non-existent record (stateless)
- Promote terraform-plugin-log from indirect to direct dependency for tflog

The verification loop polls up to 10 times (100ms intervals) to confirm
the record is no longer visible via the API before returning.

Fixes CI failures in TestAccLocalDNS on Pi-hole 2025.03.0.

* chore: fix lint and add local tooling

- Add make tools target to install golangci-lint locally to ./bin
- Update lint target to auto-install tool if missing
- Pin golangci-lint to v1.64.0 (matches CI)
- Suppress deprecated StopContext warning with nolint directive
- Regenerate docs

* chore: add success message to lint target

Author: poindexter12

Makefile

@@ -1,4 +1,11 @@
-.PHONY: help test testall lint docs docker-run release-snapshot
+.PHONY: help test testall lint docs docker-run release-snapshot tools
+
+# Tool versions
+GOLANGCI_LINT_VERSION ?= v1.64.0
+
+# Local bin directory for tools
+BIN_DIR := $(CURDIR)/bin
+export PATH := $(BIN_DIR):$(PATH)
 
 help: ## Show this help
 	@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}'
@@ -11,8 +18,15 @@ test: ## Run unit tests
 testall: ## Run all tests including acceptance tests (requires PIHOLE_URL and PIHOLE_PASSWORD)
 	TF_ACC=1 go test ./... -v $(TESTARGS) -timeout 120m
 
-lint: ## Run linter
-	golangci-lint run ./...
+tools: ## Install development tools locally to ./bin
+	@mkdir -p $(BIN_DIR)
+	GOBIN=$(BIN_DIR) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)
+
+lint: $(BIN_DIR)/golangci-lint ## Run linter
+	$(BIN_DIR)/golangci-lint run ./... && echo "✓ Lint passed"
+
+$(BIN_DIR)/golangci-lint:
+	@$(MAKE) tools
 
 docs: ## Generate Terraform documentation
 	go run github.com/hashicorp/terraform-plugin-docs/cmd/tfplugindocs@v0.19.4

docs/resources/dns_record.md

@@ -29,7 +29,7 @@ resource "pihole_dns_record" "record" {
 
 ### Optional
 
-- `force` (Boolean) Attempt to force record creation. Note: Pi-hole v6 API currently does not implement this for DNS endpoints, but it is included for forward compatibility with future Pi-hole versions.
+- `force` (Boolean) If true and the record already exists, delete it before creating the new record. Enables upsert/overwrite behavior.
 
 ### Read-Only
 

go.mod

@@ -4,6 +4,7 @@ go 1.22
 
 require (
 	github.com/hashicorp/go-retryablehttp v0.7.7
+	github.com/hashicorp/terraform-plugin-log v0.9.0
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.34.0
 )
 
@@ -30,7 +31,6 @@ require (
 	github.com/hashicorp/terraform-exec v0.21.0 // indirect
 	github.com/hashicorp/terraform-json v0.22.1 // indirect
 	github.com/hashicorp/terraform-plugin-go v0.23.0 // indirect
-	github.com/hashicorp/terraform-plugin-log v0.9.0 // indirect
 	github.com/hashicorp/terraform-registry-address v0.2.3 // indirect
 	github.com/hashicorp/terraform-svchost v0.1.1 // indirect
 	github.com/hashicorp/yamux v0.1.1 // indirect

internal/pihole/v6/cname.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/poindexter12/terraform-provider-pihole/internal/pihole"
 )
 
@@ -116,13 +117,17 @@ func (s *cnameService) Create(ctx context.Context, domain, target string, opts *
 }
 
 // Delete removes a CNAME record.
-// Returns nil if the record doesn't exist (idempotent delete).
+// Returns nil if the record doesn't exist (idempotent delete, logs warning).
+// Verifies the record is fully deleted before returning to prevent race conditions.
 func (s *cnameService) Delete(ctx context.Context, domain string) error {
 	// First get the record to find its target
 	record, err := s.Get(ctx, domain)
 	if err != nil {
-		// If record not found, delete is already done
+		// If record not found, delete is already done - warn but don't error
 		if err == pihole.ErrCNAMENotFound {
+			tflog.Warn(ctx, "CNAME record does not exist, nothing to delete", map[string]interface{}{
+				"domain": domain,
+			})
 			return nil
 		}
 		return err
@@ -141,7 +146,26 @@ func (s *cnameService) Delete(ctx context.Context, domain string) error {
 		return fmt.Errorf("unexpected status code: %d (expected 204)", resp.StatusCode)
 	}
 
-	return nil
+	// Verify the record is actually gone before returning.
+	// Pi-hole's API can return 204 before the delete is fully processed internally,
+	// causing race conditions when Create() is called immediately after.
+	const maxVerifyAttempts = 10
+	const verifyDelay = 100 * time.Millisecond
+	for attempt := 0; attempt < maxVerifyAttempts; attempt++ {
+		_, err := s.Get(ctx, domain)
+		if err == pihole.ErrCNAMENotFound {
+			// Record is confirmed deleted
+			return nil
+		}
+		// Record still visible, wait and retry
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-time.After(verifyDelay):
+		}
+	}
+
+	return fmt.Errorf("delete API succeeded but CNAME record %q still visible after %d verification attempts", domain, maxVerifyAttempts)
 }
 
 // parseCNAMEs converts "domain,target" strings to CNAMERecord structs

internal/pihole/v6/dns.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/poindexter12/terraform-provider-pihole/internal/pihole"
 )
 
@@ -126,13 +127,17 @@ func (s *dnsService) Create(ctx context.Context, domain, ip string, opts *pihole
 }
 
 // Delete removes a DNS record.
-// Returns nil if the record doesn't exist (idempotent delete).
+// Returns nil if the record doesn't exist (idempotent delete, logs warning).
+// Verifies the record is fully deleted before returning to prevent race conditions.
 func (s *dnsService) Delete(ctx context.Context, domain string) error {
 	// First get the record to find its IP
 	record, err := s.Get(ctx, domain)
 	if err != nil {
-		// If record not found, delete is already done
+		// If record not found, delete is already done - warn but don't error
 		if err == pihole.ErrDNSNotFound {
+			tflog.Warn(ctx, "DNS record does not exist, nothing to delete", map[string]interface{}{
+				"domain": domain,
+			})
 			return nil
 		}
 		return err
@@ -151,7 +156,26 @@ func (s *dnsService) Delete(ctx context.Context, domain string) error {
 		return fmt.Errorf("unexpected status code: %d (expected 204)", resp.StatusCode)
 	}
 
-	return nil
+	// Verify the record is actually gone before returning.
+	// Pi-hole's API can return 204 before the delete is fully processed internally,
+	// causing race conditions when Create() is called immediately after.
+	const maxVerifyAttempts = 10
+	const verifyDelay = 100 * time.Millisecond
+	for attempt := 0; attempt < maxVerifyAttempts; attempt++ {
+		_, err := s.Get(ctx, domain)
+		if err == pihole.ErrDNSNotFound {
+			// Record is confirmed deleted
+			return nil
+		}
+		// Record still visible, wait and retry
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-time.After(verifyDelay):
+		}
+	}
+
+	return fmt.Errorf("delete API succeeded but record %q still visible after %d verification attempts", domain, maxVerifyAttempts)
 }
 
 // parseDNSHosts converts "IP domain" strings to DNSRecord structs

internal/provider/provider.go

@@ -82,7 +82,10 @@ func configure(version string, provider *schema.Provider) func(ctx context.Conte
 		// Don't logout sessions passed in via __PIHOLE_SESSION_ID as those
 		// are managed externally (e.g., for testing or session pooling).
 		if externalSessionID == "" {
-			if stopCtx, ok := schema.StopContext(ctx); ok {
+			// StopContext is deprecated but still functional in SDK v2.
+			// The replacement (context-aware CRUD) doesn't provide stop signals.
+			// TODO: Remove when migrating to terraform-plugin-framework.
+			if stopCtx, ok := schema.StopContext(ctx); ok { //nolint:staticcheck
 				go cleanupOnStop(stopCtx, piholeClient)
 			}
 		}