Skip to content

Validate identity to match identity schema #36904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/terraform/context_apply2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2224,6 +2224,7 @@ import {
Required: true,
},
},
Nesting: configschema.NestingSingle,
},
},
})
Expand Down
28 changes: 27 additions & 1 deletion internal/terraform/context_apply_identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ func TestContext2Apply_identity(t *testing.T) {
prevRunState *states.State
requiresReplace []cty.Path
plannedIdentity cty.Value
appliedIdentity cty.Value

expectedIdentity cty.Value
expectedIdentity cty.Value
expectDiagnostics tfdiags.Diagnostics
}{
"create": {
plannedIdentity: cty.ObjectVal(map[string]cty.Value{
Expand All @@ -33,6 +35,17 @@ func TestContext2Apply_identity(t *testing.T) {
"id": cty.StringVal("foo"),
}),
},
"create - invalid applied identity schema": {
plannedIdentity: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("foo"),
}),
appliedIdentity: cty.ObjectVal(map[string]cty.Value{
"id": cty.BoolVal(false),
}),
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(tfdiags.Error, "Provider produced an identity that doesn't match the schema", "Provider \"registry.terraform.io/hashicorp/test\" returned an identity for test_resource.test that doesn't match the identity schema: .id: string required, but received bool. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker."),
},
},

"update": {
prevRunState: states.BuildState(func(s *states.SyncState) {
Expand Down Expand Up @@ -165,10 +178,23 @@ func TestContext2Apply_identity(t *testing.T) {
}
}

if !tc.appliedIdentity.IsNull() {
p.ApplyResourceChangeFn = func(req providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse {
resp := providers.ApplyResourceChangeResponse{}
resp.NewState = req.PlannedState
resp.NewIdentity = tc.appliedIdentity
return resp
}
}

plan, diags := ctx.Plan(m, tc.prevRunState, &PlanOpts{Mode: tc.mode})
tfdiags.AssertNoDiagnostics(t, diags)

state, diags := ctx.Apply(plan, m, nil)
if tc.expectDiagnostics.HasErrors() {
tfdiags.AssertDiagnosticsMatch(t, diags, tc.expectDiagnostics)
return
}
tfdiags.AssertNoDiagnostics(t, diags)

if !tc.expectedIdentity.IsNull() {
Expand Down
16 changes: 16 additions & 0 deletions internal/terraform/context_plan_identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,22 @@ func TestContext2Plan_resource_identity_plan(t *testing.T) {
"id": cty.StringVal("foo"),
}),
},
"create - invalid planned identity schema": {
plannedIdentity: cty.ObjectVal(map[string]cty.Value{
"id": cty.BoolVal(false),
}),
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(tfdiags.Error, "Provider produced an identity that doesn't match the schema", "Provider \"registry.terraform.io/hashicorp/test\" returned an identity for test_resource.test that doesn't match the identity schema: .id: string required, but received bool. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker."),
},
},
"create - null planned identity schema": {
plannedIdentity: cty.ObjectVal(map[string]cty.Value{
"id": cty.NullVal(cty.String),
}),
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(tfdiags.Error, "Provider produced an identity that doesn't match the schema", "Provider \"registry.terraform.io/hashicorp/test\" returned an identity for test_resource.test that doesn't match the identity schema: attributes \"id\" are required and must not be null. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker."),
},
},
"update": {
prevRunState: states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
Expand Down
71 changes: 65 additions & 6 deletions internal/terraform/node_resource_abstract_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,17 @@ func (n *NodeAbstractResourceInstance) planDestroy(ctx EvalContext, currentState
return noop, deferred, nil
}

_, providerSchema, err := getProvider(ctx, n.ResolvedProvider)
if err != nil {
return plan, deferred, diags.Append(err)
}
schema := providerSchema.SchemaForResourceAddr(n.Addr.Resource.Resource)
if schema.Body == nil {
// Should be caught during validation, so we don't bother with a pretty error here
diags = diags.Append(fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Resource.Type))
return nil, deferred, diags
}

// If we are in a context where we forget instead of destroying, we can
// just return the forget change without consulting the provider.
if ctx.Forget() {
Expand Down Expand Up @@ -453,7 +464,7 @@ func (n *NodeAbstractResourceInstance) planDestroy(ctx EvalContext, currentState
if !resp.PlannedIdentity.IsNull() {
// Destroying is an operation where we allow identity changes.
diags = diags.Append(n.validateIdentityKnown(resp.PlannedIdentity))
diags = diags.Append(n.validateIdentity(resp.PlannedIdentity))
diags = diags.Append(n.validateIdentity(resp.PlannedIdentity, providerSchema.Provider.Identity))
}

// We may not have a config for all destroys, but we want to reference
Expand Down Expand Up @@ -659,7 +670,7 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state

if !resp.Identity.IsNull() {
diags = diags.Append(n.validateIdentityKnown(resp.Identity))
diags = diags.Append(n.validateIdentity(resp.Identity))
diags = diags.Append(n.validateIdentity(resp.Identity, schema.Identity))
diags = diags.Append(n.validateIdentityDidNotChange(state, resp.Identity))
}
if resp.Deferred != nil {
Expand Down Expand Up @@ -1117,7 +1128,7 @@ func (n *NodeAbstractResourceInstance) plan(
}
}

diags = diags.Append(n.validateIdentity(plannedIdentity))
diags = diags.Append(n.validateIdentity(plannedIdentity, schema.Identity))
}
if diags.HasErrors() {
return nil, nil, deferred, keyData, diags
Expand Down Expand Up @@ -1179,7 +1190,7 @@ func (n *NodeAbstractResourceInstance) plan(

if !resp.PlannedIdentity.IsNull() {
// On replace the identity is allowed to change and be unknown.
diags = diags.Append(n.validateIdentity(resp.PlannedIdentity))
diags = diags.Append(n.validateIdentity(resp.PlannedIdentity, schema.Identity))
}
}
// We need to tread carefully here, since if there are any warnings
Expand Down Expand Up @@ -2636,7 +2647,7 @@ func (n *NodeAbstractResourceInstance) apply(

if !resp.NewIdentity.IsNull() {
diags = diags.Append(n.validateIdentityKnown(resp.NewIdentity))
diags = diags.Append(n.validateIdentity(resp.NewIdentity))
diags = diags.Append(n.validateIdentity(resp.NewIdentity, schema.Identity))
if !change.Action.IsReplace() {
diags = diags.Append(n.validateIdentityDidNotChange(state, resp.NewIdentity))
}
Expand Down Expand Up @@ -2920,7 +2931,7 @@ func (n *NodeAbstractResourceInstance) validateIdentityDidNotChange(state *state
return diags
}

func (n *NodeAbstractResourceInstance) validateIdentity(newIdentity cty.Value) (diags tfdiags.Diagnostics) {
func (n *NodeAbstractResourceInstance) validateIdentity(newIdentity cty.Value, identitySchema *configschema.Object) (diags tfdiags.Diagnostics) {
if _, marks := newIdentity.UnmarkDeep(); len(marks) > 0 {
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Error,
Expand All @@ -2932,6 +2943,54 @@ func (n *NodeAbstractResourceInstance) validateIdentity(newIdentity cty.Value) (
))
}

// The identity schema is always a single object, so we can check the
// nesting type here.
if identitySchema.Nesting != configschema.NestingSingle {
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Error,
"Provider produced invalid identity",
fmt.Sprintf(
"Provider %q returned an identity with a nesting type of %s for %s. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.",
n.ResolvedProvider.Provider, identitySchema.Nesting, n.Addr,
),
))
}

newType := newIdentity.Type()
currentType := identitySchema.ImpliedType()
if errs := newType.TestConformance(currentType); len(errs) > 0 {
for _, err := range errs {
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Error,
"Provider produced an identity that doesn't match the schema",
fmt.Sprintf(
"Provider %q returned an identity for %s that doesn't match the identity schema: %s. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.",
n.ResolvedProvider.Provider, n.Addr, tfdiags.FormatError(err),
),
))
}
return diags
}

// Check for required attributes
names := make([]string, 0, len(identitySchema.Attributes))
for name, attrS := range identitySchema.Attributes {
if attrS.Required && newIdentity.GetAttr(name).IsNull() {
names = append(names, name)
}
}
if len(names) > 0 {
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Error,
"Provider produced an identity that doesn't match the schema",
fmt.Sprintf(
"Provider %q returned an identity for %s that doesn't match the identity schema: attributes %q are required and must not be null. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.",
n.ResolvedProvider.Provider, n.Addr,
strings.Join(names, ", "),
),
))
}

return diags
}

Expand Down
Loading