Skip to content

Commit 3499bd6

Browse files
committed
fix: restore session cleanup and implement force delete-before-create
Session cleanup: - Restore cleanupOnStop goroutine that was accidentally removed in ProviderMeta refactor - Sessions are now properly logged out when terraform exits - Prevents "API seats exceeded" errors from stale session accumulation Force flag: - Implement actual upsert behavior for DNS records - When force=true and record exists, delete it before creating - Pi-hole v6 API doesn't support force query param, so implement client-side - Update description to reflect working functionality Fixes session exhaustion and "item already present" errors during terraform apply.
1 parent 8817128 commit 3499bd6

3 files changed

Lines changed: 38 additions & 5 deletions

File tree

internal/pihole/v6/dns.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,26 @@ func (s *dnsService) Get(ctx context.Context, domain string) (*pihole.DNSRecord,
6565
}
6666

6767
// Create adds a new DNS record.
68+
// If opts.Force is true and the record already exists, it will be deleted first.
6869
// Includes retry logic for transient errors that can occur during
6970
// ForceNew operations when Pi-hole hasn't fully processed a prior delete.
7071
func (s *dnsService) Create(ctx context.Context, domain, ip string, opts *pihole.CreateOptions) (*pihole.DNSRecord, error) {
71-
path := fmt.Sprintf("%s/%s", dnsHostsPath, url.PathEscape(ip+" "+domain))
72-
73-
// Append force parameter if requested
72+
// If force is requested and record exists, delete it first
7473
if opts != nil && opts.Force {
75-
path += "?force=true"
74+
existing, err := s.Get(ctx, domain)
75+
if err == nil && existing != nil {
76+
// Record exists - delete it first
77+
if delErr := s.Delete(ctx, domain); delErr != nil {
78+
return nil, fmt.Errorf("force delete failed: %w", delErr)
79+
}
80+
// Brief pause to let Pi-hole process the delete
81+
time.Sleep(100 * time.Millisecond)
82+
}
83+
// If err != nil (not found), that's fine - proceed with create
7684
}
7785

86+
path := fmt.Sprintf("%s/%s", dnsHostsPath, url.PathEscape(ip+" "+domain))
87+
7888
const maxRetries = 5
7989
var lastErr error
8090
for attempt := 0; attempt < maxRetries; attempt++ {

internal/provider/provider.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"context"
55
"fmt"
66
"os"
7+
"time"
78

89
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
910
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
11+
"github.com/poindexter12/terraform-provider-pihole/internal/pihole"
1012
"github.com/poindexter12/terraform-provider-pihole/internal/version"
1113
)
1214

@@ -76,7 +78,28 @@ func configure(version string, provider *schema.Provider) func(ctx context.Conte
7678
return nil, diag.FromErr(fmt.Errorf("failed to instantiate client: %w", err))
7779
}
7880

81+
// Only register cleanup for sessions we created ourselves.
82+
// Don't logout sessions passed in via __PIHOLE_SESSION_ID as those
83+
// are managed externally (e.g., for testing or session pooling).
84+
if externalSessionID == "" {
85+
if stopCtx, ok := schema.StopContext(ctx); ok {
86+
go cleanupOnStop(stopCtx, piholeClient)
87+
}
88+
}
89+
7990
// Return ProviderMeta which wraps the client and provides coordination
8091
return &ProviderMeta{Client: piholeClient}, nil
8192
}
8293
}
94+
95+
// cleanupOnStop waits for the stop context to be cancelled
96+
// and then logs out the Pi-hole session to free up the session slot.
97+
func cleanupOnStop(stopCtx context.Context, client pihole.Client) {
98+
<-stopCtx.Done()
99+
100+
// Use a fresh context for logout since stopCtx is cancelled
101+
logoutCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
102+
defer cancel()
103+
104+
_ = client.Logout(logoutCtx) // Best effort - ignore errors
105+
}

internal/provider/resource_dns_record.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func resourceDNSRecord() *schema.Resource {
3535
ValidateDiagFunc: validateIPAddress(),
3636
},
3737
"force": {
38-
Description: "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.",
38+
Description: "If true and the record already exists, delete it before creating the new record. Enables upsert/overwrite behavior.",
3939
Type: schema.TypeBool,
4040
Optional: true,
4141
Default: false,

0 commit comments

Comments
 (0)