Skip to content

Commit d1bda51

Browse files
committed
fix(proxy_record): symmetric domain normalisation, util test helpers
Two follow-ups on top of @JulienJBO's #74: * Domain normalisation was applied on the write side (BuildCreateRequest -> normalizeProxyRecordDomain) but not on plan, so a user config of `Example.COM.` would plan as `Example.COM.` while state ended up at the API's canonical form `example.com`. This trips the framework's "Provider produced inconsistent result after apply" check on first apply, and triggers a permanent RequiresReplace loop on every subsequent plan. Added a stringplanmodifier (normalizeProxyRecordDomainPlanModifier) that runs the existing normaliser on the planned value so plan and state stay symmetric. Covered with a table-driven unit test for the modifier itself, plus a bumped assertion in TestProxyRecordResourceNameAndSchema for the new modifier slot. * Local stringPtr / int64Ptr test helpers in both proxy_record test files replaced with util.StringPtr / util.Int64Ptr now that #83 has landed the centralised helpers; resource also uses util.PtrToInt64 in place of the inline nil-check for resp.CreatedBy. Also: drop the `2026-04-21` date from the design note observation about the retry endpoint — date-stamped probes age badly, so generalise to "at the time of writing".
1 parent 284fa50 commit d1bda51

5 files changed

Lines changed: 90 additions & 25 deletions

File tree

docs/notes/proxy-record-design.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ Recommended lifecycle behavior:
5656
- `Delete`: delete the record and rely on `404` handling for any later refresh.
5757

5858
`retry` should not be part of the declarative CRUD contract. It is an operational remediation endpoint to be used after the user fixes DNS, not a stable desired-state transition.
59-
The live API probe on `2026-04-21` also showed that `POST /retry/` returns `403 permission_denied` when called with a Personal API Key, so it cannot be a reliable part of provider CRUD with the current authentication model.
59+
At the time of writing, `POST /retry/` also returns `403 permission_denied` when called with a Personal API Key, which rules it out of provider CRUD with the current authentication model.
6060

6161
## Why Acceptance Is The Real Blocker
6262

docs/resources/proxy_record.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ output "analytics_proxy_target_cname" {
3030

3131
### Required
3232

33-
- `domain` (String) The custom domain to provision in PostHog.
33+
- `domain` (String) The custom domain to provision in PostHog. Configured values are normalised to lowercase with no trailing dot, so `Example.COM.` and `example.com` plan as the same value.
3434

3535
### Optional
3636

internal/httpclient/proxy_record_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net/http/httptest"
88
"testing"
99

10+
"github.com/posthog/terraform-provider/internal/util"
1011
"github.com/stretchr/testify/assert"
1112
"github.com/stretchr/testify/require"
1213
)
@@ -29,7 +30,7 @@ func TestListProxyRecords(t *testing.T) {
2930
assert.Equal(t, testProxyRecordsCollectionPath(), r.URL.Path)
3031

3132
resp := ProxyRecordListResponse{
32-
MaxProxyRecords: int64Ptr(10),
33+
MaxProxyRecords: util.Int64Ptr(10),
3334
Results: []ProxyRecord{
3435
{ID: testProxyRecordID, Domain: "a.example.com", TargetCNAME: "cname-1.proxyhog.com.", Status: "waiting"},
3536
{ID: "proxy-2", Domain: "b.example.com", TargetCNAME: "cname-2.proxyhog.com.", Status: "valid"},
@@ -124,10 +125,6 @@ func writeJSONResponse(t *testing.T, w http.ResponseWriter, body any) {
124125
require.NoError(t, json.NewEncoder(w).Encode(body))
125126
}
126127

127-
func int64Ptr(v int64) *int64 {
128-
return &v
129-
}
130-
131128
func testProxyRecordsCollectionPath() string {
132129
return apiOrganizationsPrefix + testOrganizationID + proxyRecordsPathSuffix
133130
}

internal/resource/proxy_record.go

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/hashicorp/terraform-plugin-framework/types"
1515
"github.com/posthog/terraform-provider/internal/httpclient"
1616
"github.com/posthog/terraform-provider/internal/resource/core"
17+
"github.com/posthog/terraform-provider/internal/util"
1718
)
1819

1920
func NewProxyRecord() resource.Resource {
@@ -49,9 +50,10 @@ func (o ProxyRecordOps) Schema() schema.Schema {
4950
"organization_id": core.OrganizationIDSchemaAttribute(),
5051
"domain": schema.StringAttribute{
5152
Required: true,
52-
MarkdownDescription: "The custom domain to provision in PostHog.",
53+
MarkdownDescription: "The custom domain to provision in PostHog. Configured values are normalised to lowercase with no trailing dot, so `Example.COM.` and `example.com` plan as the same value.",
5354
PlanModifiers: []planmodifier.String{
5455
stringplanmodifier.RequiresReplace(),
56+
normalizeProxyRecordDomainPlanModifier{},
5557
},
5658
},
5759
"target_cname": proxyRecordComputedStringAttribute("The PostHog-managed CNAME target that your DNS record must point to."),
@@ -90,11 +92,7 @@ func (o ProxyRecordOps) MapResponseToModel(_ context.Context, resp httpclient.Pr
9092
model.Message = core.PtrToStringNullIfEmptyTrimmed(resp.Message)
9193
model.CreatedAt = core.PtrToStringNullIfEmptyTrimmed(resp.CreatedAt)
9294
model.UpdatedAt = core.PtrToStringNullIfEmptyTrimmed(resp.UpdatedAt)
93-
if resp.CreatedBy == nil {
94-
model.CreatedBy = types.Int64Null()
95-
} else {
96-
model.CreatedBy = types.Int64Value(*resp.CreatedBy)
97-
}
95+
model.CreatedBy = util.PtrToInt64(resp.CreatedBy)
9896

9997
return diags
10098
}
@@ -119,6 +117,33 @@ func normalizeProxyRecordDomain(raw string) string {
119117
return strings.ToLower(strings.TrimSuffix(strings.TrimSpace(raw), "."))
120118
}
121119

120+
// normalizeProxyRecordDomainPlanModifier rewrites the planned `domain` value
121+
// through normalizeProxyRecordDomain before plan finalises. Without it, a config
122+
// of `Example.COM.` would plan as `Example.COM.` while state holds the API's
123+
// canonical form (`example.com`), which trips Terraform's "Provider produced
124+
// inconsistent result after apply" check on first apply and triggers a
125+
// permanent RequiresReplace loop on every subsequent plan.
126+
type normalizeProxyRecordDomainPlanModifier struct{}
127+
128+
func (normalizeProxyRecordDomainPlanModifier) Description(_ context.Context) string {
129+
return "Normalises the domain to lowercase with no trailing dot."
130+
}
131+
132+
func (m normalizeProxyRecordDomainPlanModifier) MarkdownDescription(ctx context.Context) string {
133+
return m.Description(ctx)
134+
}
135+
136+
func (normalizeProxyRecordDomainPlanModifier) PlanModifyString(_ context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) {
137+
if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() {
138+
return
139+
}
140+
normalized := normalizeProxyRecordDomain(req.ConfigValue.ValueString())
141+
if normalized == req.PlanValue.ValueString() {
142+
return
143+
}
144+
resp.PlanValue = types.StringValue(normalized)
145+
}
146+
122147
func proxyRecordComputedStringAttribute(description string) schema.StringAttribute {
123148
return schema.StringAttribute{
124149
Computed: true,

internal/resource/proxy_record_test.go

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ import (
88
"testing"
99

1010
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
11+
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
1112
"github.com/hashicorp/terraform-plugin-framework/types"
1213
"github.com/posthog/terraform-provider/internal/httpclient"
1314
"github.com/posthog/terraform-provider/internal/resource/core"
15+
"github.com/posthog/terraform-provider/internal/util"
1416
"github.com/stretchr/testify/assert"
1517
"github.com/stretchr/testify/require"
1618
)
@@ -56,6 +58,55 @@ func TestNormalizeProxyRecordDomain(t *testing.T) {
5658
}
5759
}
5860

61+
func TestProxyRecordDomainPlanModifier(t *testing.T) {
62+
tests := map[string]struct {
63+
config types.String
64+
planBefore types.String
65+
expectedAfter types.String
66+
expectModified bool
67+
modifierExpected string
68+
}{
69+
"normalises mixed-case config": {
70+
config: types.StringValue("Proxy.Example.COM"),
71+
planBefore: types.StringValue("Proxy.Example.COM"),
72+
expectedAfter: types.StringValue(testNormalizedProxyRecordDomain),
73+
expectModified: true,
74+
},
75+
"strips trailing dot": {
76+
config: types.StringValue(testNormalizedProxyRecordDomain + "."),
77+
planBefore: types.StringValue(testNormalizedProxyRecordDomain + "."),
78+
expectedAfter: types.StringValue(testNormalizedProxyRecordDomain),
79+
expectModified: true,
80+
},
81+
"already canonical config is left alone": {
82+
config: types.StringValue(testNormalizedProxyRecordDomain),
83+
planBefore: types.StringValue(testNormalizedProxyRecordDomain),
84+
expectedAfter: types.StringValue(testNormalizedProxyRecordDomain),
85+
expectModified: false,
86+
},
87+
"null config is a no-op": {
88+
config: types.StringNull(),
89+
planBefore: types.StringNull(),
90+
expectedAfter: types.StringNull(),
91+
expectModified: false,
92+
},
93+
}
94+
95+
for name, tc := range tests {
96+
t.Run(name, func(t *testing.T) {
97+
req := planmodifier.StringRequest{
98+
ConfigValue: tc.config,
99+
PlanValue: tc.planBefore,
100+
}
101+
resp := planmodifier.StringResponse{PlanValue: tc.planBefore}
102+
103+
normalizeProxyRecordDomainPlanModifier{}.PlanModifyString(context.Background(), req, &resp)
104+
105+
assert.Equal(t, tc.expectedAfter, resp.PlanValue)
106+
})
107+
}
108+
}
109+
59110
func TestProxyRecordBuildCreateRequest(t *testing.T) {
60111
ops := ProxyRecordOps{}
61112
req, diags := ops.BuildCreateRequest(context.Background(), ProxyRecordTFModel{
@@ -84,7 +135,7 @@ func TestProxyRecordResourceNameAndSchema(t *testing.T) {
84135
domainAttr, ok := s.Attributes["domain"].(schema.StringAttribute)
85136
require.True(t, ok, "domain must be a string attribute")
86137
assert.True(t, domainAttr.Required)
87-
require.Len(t, domainAttr.PlanModifiers, 1)
138+
require.Len(t, domainAttr.PlanModifiers, 2, "domain should carry RequiresReplace and the normalize plan modifier")
88139

89140
targetAttr, ok := s.Attributes["target_cname"].(schema.StringAttribute)
90141
require.True(t, ok, "target_cname must be a string attribute")
@@ -102,9 +153,9 @@ func TestProxyRecordMapResponseToModel(t *testing.T) {
102153
Domain: testNormalizedProxyRecordDomain,
103154
TargetCNAME: testProxyRecordTargetCNAME,
104155
Status: testProxyRecordStatus,
105-
CreatedAt: stringPtr(testProxyRecordCreatedAt),
106-
UpdatedAt: stringPtr(testProxyRecordUpdatedAt),
107-
CreatedBy: int64Ptr(testProxyRecordCreatorID),
156+
CreatedAt: util.StringPtr(testProxyRecordCreatedAt),
157+
UpdatedAt: util.StringPtr(testProxyRecordUpdatedAt),
158+
CreatedBy: util.Int64Ptr(testProxyRecordCreatorID),
108159
}
109160

110161
var model ProxyRecordTFModel
@@ -204,14 +255,6 @@ func TestProxyRecordUpdateReturnsError(t *testing.T) {
204255
assert.Contains(t, err.Error(), "do not support updates")
205256
}
206257

207-
func stringPtr(v string) *string {
208-
return &v
209-
}
210-
211-
func int64Ptr(v int64) *int64 {
212-
return &v
213-
}
214-
215258
func testProxyRecordCollectionPath() string {
216259
return testProxyRecordAPIPathPrefix + testProxyRecordOrganizationID + testProxyRecordAPIPathSuffix
217260
}

0 commit comments

Comments
 (0)