Skip to content

Commit bcd99f0

Browse files
austinvalleaustinvalle
committed
add immutability validation to plan and apply
1 parent 5c1ab2d commit bcd99f0

File tree

4 files changed

+498
-77
lines changed

4 files changed

+498
-77
lines changed

‎internal/fwserver/server_applyresourcechange_test.go‎

Lines changed: 211 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,73 @@ func TestServerApplyResourceChange(t *testing.T) {
524524
Private: testEmptyPrivate,
525525
},
526526
},
527+
"create-response-newidentity-changes": {
528+
server: &fwserver.Server{
529+
Provider: &testprovider.Provider{},
530+
},
531+
request: &fwserver.ApplyResourceChangeRequest{
532+
Config: &tfsdk.Config{
533+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
534+
"test_computed": tftypes.NewValue(tftypes.String, nil),
535+
"test_required": tftypes.NewValue(tftypes.String, "test-config-value"),
536+
}),
537+
Schema: testSchema,
538+
},
539+
PlannedState: &tfsdk.Plan{
540+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
541+
"test_computed": tftypes.NewValue(tftypes.String, nil),
542+
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
543+
}),
544+
Schema: testSchema,
545+
},
546+
PlannedIdentity: &tfsdk.ResourceIdentity{
547+
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
548+
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
549+
}),
550+
Schema: testIdentitySchema,
551+
},
552+
IdentitySchema: testIdentitySchema,
553+
PriorState: testEmptyState,
554+
ResourceSchema: testSchema,
555+
Resource: &testprovider.ResourceWithIdentity{
556+
Resource: &testprovider.Resource{
557+
CreateMethod: func(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
558+
resp.Diagnostics.Append(resp.Identity.Set(ctx, testIdentitySchemaData{
559+
TestID: types.StringValue("new-id-123"),
560+
})...)
561+
562+
// Prevent missing resource state error diagnostic
563+
var data testSchemaData
564+
565+
resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
566+
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
567+
},
568+
DeleteMethod: func(_ context.Context, _ resource.DeleteRequest, resp *resource.DeleteResponse) {
569+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Delete")
570+
},
571+
UpdateMethod: func(_ context.Context, _ resource.UpdateRequest, resp *resource.UpdateResponse) {
572+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Create, Got: Update")
573+
},
574+
},
575+
},
576+
},
577+
expectedResponse: &fwserver.ApplyResourceChangeResponse{
578+
NewIdentity: &tfsdk.ResourceIdentity{
579+
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
580+
"test_id": tftypes.NewValue(tftypes.String, "new-id-123"),
581+
}),
582+
Schema: testIdentitySchema,
583+
},
584+
NewState: &tfsdk.State{
585+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
586+
"test_computed": tftypes.NewValue(tftypes.String, nil),
587+
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
588+
}),
589+
Schema: testSchema,
590+
},
591+
Private: testEmptyPrivate,
592+
},
593+
},
527594
"create-response-newstate-null": {
528595
server: &fwserver.Server{
529596
Provider: &testprovider.Provider{},
@@ -1591,7 +1658,7 @@ func TestServerApplyResourceChange(t *testing.T) {
15911658
Private: testEmptyPrivate,
15921659
},
15931660
},
1594-
"update-response-newidentity": {
1661+
"update-response-newidentity-null-plannedidentity": {
15951662
server: &fwserver.Server{
15961663
Provider: &testprovider.Provider{},
15971664
},
@@ -1652,6 +1719,149 @@ func TestServerApplyResourceChange(t *testing.T) {
16521719
Private: testEmptyPrivate,
16531720
},
16541721
},
1722+
"update-response-newidentity-with-plannedidentity": {
1723+
server: &fwserver.Server{
1724+
Provider: &testprovider.Provider{},
1725+
},
1726+
request: &fwserver.ApplyResourceChangeRequest{
1727+
Config: &tfsdk.Config{
1728+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1729+
"test_computed": tftypes.NewValue(tftypes.String, nil),
1730+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
1731+
}),
1732+
Schema: testSchema,
1733+
},
1734+
PlannedState: &tfsdk.Plan{
1735+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1736+
"test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
1737+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
1738+
}),
1739+
Schema: testSchema,
1740+
},
1741+
PlannedIdentity: &tfsdk.ResourceIdentity{
1742+
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
1743+
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
1744+
}),
1745+
Schema: testIdentitySchema,
1746+
},
1747+
PriorState: &tfsdk.State{
1748+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1749+
"test_computed": tftypes.NewValue(tftypes.String, nil),
1750+
"test_required": tftypes.NewValue(tftypes.String, "test-old-value"),
1751+
}),
1752+
Schema: testSchema,
1753+
},
1754+
IdentitySchema: testIdentitySchema,
1755+
ResourceSchema: testSchema,
1756+
Resource: &testprovider.ResourceWithIdentity{
1757+
Resource: &testprovider.Resource{
1758+
CreateMethod: func(_ context.Context, _ resource.CreateRequest, resp *resource.CreateResponse) {
1759+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Create")
1760+
},
1761+
DeleteMethod: func(_ context.Context, _ resource.DeleteRequest, resp *resource.DeleteResponse) {
1762+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Delete")
1763+
},
1764+
UpdateMethod: func(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
1765+
resp.Diagnostics.Append(resp.Identity.Set(ctx, testIdentitySchemaData{
1766+
TestID: types.StringValue("id-123"),
1767+
})...)
1768+
},
1769+
},
1770+
},
1771+
},
1772+
expectedResponse: &fwserver.ApplyResourceChangeResponse{
1773+
NewIdentity: &tfsdk.ResourceIdentity{
1774+
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
1775+
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
1776+
}),
1777+
Schema: testIdentitySchema,
1778+
},
1779+
NewState: &tfsdk.State{
1780+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1781+
"test_computed": tftypes.NewValue(tftypes.String, nil),
1782+
"test_required": tftypes.NewValue(tftypes.String, "test-old-value"),
1783+
}),
1784+
Schema: testSchema,
1785+
},
1786+
Private: testEmptyPrivate,
1787+
},
1788+
},
1789+
"update-invalid-response-newidentity-changes": {
1790+
server: &fwserver.Server{
1791+
Provider: &testprovider.Provider{},
1792+
},
1793+
request: &fwserver.ApplyResourceChangeRequest{
1794+
Config: &tfsdk.Config{
1795+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1796+
"test_computed": tftypes.NewValue(tftypes.String, nil),
1797+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
1798+
}),
1799+
Schema: testSchema,
1800+
},
1801+
PlannedState: &tfsdk.Plan{
1802+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1803+
"test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
1804+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
1805+
}),
1806+
Schema: testSchema,
1807+
},
1808+
PlannedIdentity: &tfsdk.ResourceIdentity{
1809+
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
1810+
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
1811+
}),
1812+
Schema: testIdentitySchema,
1813+
},
1814+
PriorState: &tfsdk.State{
1815+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1816+
"test_computed": tftypes.NewValue(tftypes.String, nil),
1817+
"test_required": tftypes.NewValue(tftypes.String, "test-old-value"),
1818+
}),
1819+
Schema: testSchema,
1820+
},
1821+
IdentitySchema: testIdentitySchema,
1822+
ResourceSchema: testSchema,
1823+
Resource: &testprovider.ResourceWithIdentity{
1824+
Resource: &testprovider.Resource{
1825+
CreateMethod: func(_ context.Context, _ resource.CreateRequest, resp *resource.CreateResponse) {
1826+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Create")
1827+
},
1828+
DeleteMethod: func(_ context.Context, _ resource.DeleteRequest, resp *resource.DeleteResponse) {
1829+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Delete")
1830+
},
1831+
UpdateMethod: func(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
1832+
resp.Diagnostics.Append(resp.Identity.Set(ctx, testIdentitySchemaData{
1833+
TestID: types.StringValue("new-id-123"),
1834+
})...)
1835+
},
1836+
},
1837+
},
1838+
},
1839+
expectedResponse: &fwserver.ApplyResourceChangeResponse{
1840+
Diagnostics: diag.Diagnostics{
1841+
diag.NewErrorDiagnostic(
1842+
"Unexpected Identity Change",
1843+
"During the apply operation, the Terraform Provider unexpectedly returned a different identity then the previously stored one.\n\n"+
1844+
"This is always a problem with the provider and should be reported to the provider developer.\n\n"+
1845+
"Planned Identity: tftypes.Object[\"test_id\":tftypes.String]<\"test_id\":tftypes.String<\"id-123\">>\n"+
1846+
"New Identity: tftypes.Object[\"test_id\":tftypes.String]<\"test_id\":tftypes.String<\"new-id-123\">>",
1847+
),
1848+
},
1849+
NewIdentity: &tfsdk.ResourceIdentity{
1850+
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
1851+
"test_id": tftypes.NewValue(tftypes.String, "new-id-123"),
1852+
}),
1853+
Schema: testIdentitySchema,
1854+
},
1855+
NewState: &tfsdk.State{
1856+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1857+
"test_computed": tftypes.NewValue(tftypes.String, nil),
1858+
"test_required": tftypes.NewValue(tftypes.String, "test-old-value"),
1859+
}),
1860+
Schema: testSchema,
1861+
},
1862+
Private: testEmptyPrivate,
1863+
},
1864+
},
16551865
"update-response-newstate-null": {
16561866
server: &fwserver.Server{
16571867
Provider: &testprovider.Provider{},

‎internal/fwserver/server_planresourcechange.go‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -368,14 +368,29 @@ func (s *Server) PlanResourceChange(ctx context.Context, req *PlanResourceChange
368368
}
369369
}
370370

371-
if resp.PlannedIdentity != nil && req.IdentitySchema == nil {
372-
resp.Diagnostics.AddError(
373-
"Unexpected Plan Response",
374-
"An unexpected error was encountered when creating the plan response. New identity data was returned by the provider planning operation, but the resource does not indicate identity support.\n\n"+
375-
"This is always a problem with the provider and should be reported to the provider developer.",
376-
)
371+
if resp.PlannedIdentity != nil {
372+
if req.IdentitySchema == nil {
373+
resp.Diagnostics.AddError(
374+
"Unexpected Plan Response",
375+
"An unexpected error was encountered when creating the plan response. New identity data was returned by the provider planning operation, but the resource does not indicate identity support.\n\n"+
376+
"This is always a problem with the provider and should be reported to the provider developer.",
377+
)
377378

378-
return
379+
return
380+
}
381+
382+
// If we're updating or deleting and we already have an identity stored, validate that the planned identity isn't changing
383+
if !req.PriorState.Raw.IsNull() && !req.PriorIdentity.Raw.IsNull() && !req.PriorIdentity.Raw.Equal(resp.PlannedIdentity.Raw) {
384+
resp.Diagnostics.AddError(
385+
"Unexpected Identity Change",
386+
"During the planning operation, the Terraform Provider unexpectedly returned a different identity then the previously stored one.\n\n"+
387+
"This is always a problem with the provider and should be reported to the provider developer.\n\n"+
388+
fmt.Sprintf("Prior Identity: %s\n", req.PriorIdentity.Raw.String())+
389+
fmt.Sprintf("Planned Identity: %s", resp.PlannedIdentity.Raw.String()),
390+
)
391+
392+
return
393+
}
379394
}
380395

381396
// Ensure deterministic RequiresReplace by sorting and deduplicating

0 commit comments

Comments
 (0)