Skip to content
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

Use different summaries and details when reporting errors about non-null write-only attribute values #36274

Merged
merged 5 commits into from
Jan 8, 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
5 changes: 5 additions & 0 deletions .changes/unreleased/ENHANCEMENTS-20250108-104335.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: ENHANCEMENTS
body: Changed the text in errors returned from Terraform when a non-null value is found for a write-only attribute.
time: 2025-01-08T10:43:35.241109Z
custom:
Issue: "36274"
14 changes: 5 additions & 9 deletions internal/lang/ephemeral/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,20 @@
package ephemeral

import (
"fmt"

"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs/configschema"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
)

func ValidateWriteOnlyAttributes(newVal cty.Value, schema *configschema.Block, provider addrs.AbsProviderConfig, addr addrs.AbsResourceInstance) (diags tfdiags.Diagnostics) {
// ValidateWriteOnlyAttributes identifies all instances of write-only paths that contain non-null values
// and returns a diagnostic for each instance
func ValidateWriteOnlyAttributes(summary string, detail func(cty.Path) string, newVal cty.Value, schema *configschema.Block) (diags tfdiags.Diagnostics) {
if writeOnlyPaths := NonNullWriteOnlyPaths(newVal, schema, nil); len(writeOnlyPaths) != 0 {
for _, p := range writeOnlyPaths {
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Error,
"Write-only attribute set",
fmt.Sprintf(
"Provider %q returned a value for the write-only attribute \"%s%s\". Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.",
provider.String(), addr.String(), tfdiags.FormatCtyPath(p),
),
summary,
detail(p),
SarahFrench marked this conversation as resolved.
Show resolved Hide resolved
))
}
}
Expand Down
12 changes: 11 additions & 1 deletion internal/refactoring/cross_provider_move.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,17 @@ func (move *crossTypeMove) applyCrossTypeMove(stmt *MoveStatement, source, targe
}

// Providers are supposed to return null values for all write-only attributes
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(resp.TargetState, move.targetResourceSchema, move.targetProviderAddr, target)
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(
"Provider returned invalid value",
func(path cty.Path) string {
return fmt.Sprintf(
"The provider %q returned a value for the write-only attribute \"%s%s\" during an across type move operation to %s. Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.",
move.targetProviderAddr, target, tfdiags.FormatCtyPath(path), target,
)
},
resp.TargetState,
move.targetResourceSchema,
)
diags = diags.Append(writeOnlyDiags)

if writeOnlyDiags.HasErrors() {
Expand Down
12 changes: 6 additions & 6 deletions internal/terraform/context_apply_ephemeral_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -804,8 +804,8 @@ resource "ephem_write_only" "wo" {

expectedDiags = append(expectedDiags, tfdiags.Sourceless(
tfdiags.Error,
"Write-only attribute set",
`Provider "provider[\"registry.terraform.io/hashicorp/ephem\"]" returned a value for the write-only attribute "ephem_write_only.wo.write_only". Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.`,
"Provider produced invalid object",
`Provider "provider[\"registry.terraform.io/hashicorp/ephem\"]" returned a value for the write-only attribute "ephem_write_only.wo.write_only" after apply. Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.`,
))

assertDiagnosticsMatch(t, diags, expectedDiags)
Expand Down Expand Up @@ -876,8 +876,8 @@ resource "ephem_write_only" "wo" {

expectedDiags = append(expectedDiags, tfdiags.Sourceless(
tfdiags.Error,
"Write-only attribute set",
`Provider "provider[\"registry.terraform.io/hashicorp/ephem\"]" returned a value for the write-only attribute "ephem_write_only.wo.write_only". Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.`,
"Provider produced invalid plan",
`Provider "provider[\"registry.terraform.io/hashicorp/ephem\"]" returned a value for the write-only attribute "ephem_write_only.wo.write_only" during planning. Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.`,
))

assertDiagnosticsMatch(t, diags, expectedDiags)
Expand Down Expand Up @@ -963,8 +963,8 @@ resource "ephem_write_only" "wo" {

expectedDiags = append(expectedDiags, tfdiags.Sourceless(
tfdiags.Error,
"Write-only attribute set",
`Provider "provider[\"registry.terraform.io/hashicorp/ephem\"]" returned a value for the write-only attribute "ephem_write_only.wo.write_only". Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.`,
"Provider produced invalid object",
`Provider "provider[\"registry.terraform.io/hashicorp/ephem\"]" returned a value for the write-only attribute "ephem_write_only.wo.write_only" during refresh. Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.`,
))

assertDiagnosticsMatch(t, diags, expectedDiags)
Expand Down
48 changes: 44 additions & 4 deletions internal/terraform/node_resource_abstract_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,17 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state
}

// Providers are supposed to return null values for all write-only attributes
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(resp.NewState, schema, n.ResolvedProvider, n.Addr)
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(
"Provider produced invalid object",
func(path cty.Path) string {
return fmt.Sprintf(
"Provider %q returned a value for the write-only attribute \"%s%s\" during refresh. Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.",
n.ResolvedProvider, n.Addr, tfdiags.FormatCtyPath(path),
)
},
resp.NewState,
schema,
)
diags = diags.Append(writeOnlyDiags)

if writeOnlyDiags.HasErrors() {
Expand Down Expand Up @@ -960,7 +970,17 @@ func (n *NodeAbstractResourceInstance) plan(
}

// Providers are supposed to return null values for all write-only attributes
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(plannedNewVal, schema, n.ResolvedProvider, n.Addr)
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(
"Provider produced invalid plan",
func(path cty.Path) string {
return fmt.Sprintf(
"Provider %q returned a value for the write-only attribute \"%s%s\" during planning. Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.",
n.ResolvedProvider, n.Addr, tfdiags.FormatCtyPath(path),
)
},
plannedNewVal,
schema,
)
diags = diags.Append(writeOnlyDiags)

if writeOnlyDiags.HasErrors() {
Expand Down Expand Up @@ -1148,7 +1168,17 @@ func (n *NodeAbstractResourceInstance) plan(
}

// Providers are supposed to return null values for all write-only attributes
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(plannedNewVal, schema, n.ResolvedProvider, n.Addr)
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(
"Provider produced invalid plan",
func(path cty.Path) string {
return fmt.Sprintf(
"Provider %q returned a value for the write-only attribute \"%s%s\" during planning. Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.",
n.ResolvedProvider, n.Addr, tfdiags.FormatCtyPath(path),
)
},
plannedNewVal,
schema,
)
diags = diags.Append(writeOnlyDiags)

if writeOnlyDiags.HasErrors() {
Expand Down Expand Up @@ -2590,7 +2620,17 @@ func (n *NodeAbstractResourceInstance) apply(
}

// Providers are supposed to return null values for all write-only attributes
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(newVal, schema, n.ResolvedProvider, n.Addr)
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(
"Provider produced invalid object",
func(path cty.Path) string {
return fmt.Sprintf(
"Provider %q returned a value for the write-only attribute \"%s%s\" after apply. Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.",
n.ResolvedProvider, n.Addr, tfdiags.FormatCtyPath(path),
)
},
newVal,
schema,
)
diags = diags.Append(writeOnlyDiags)

if writeOnlyDiags.HasErrors() {
Expand Down
13 changes: 12 additions & 1 deletion internal/terraform/node_resource_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/hashicorp/terraform/internal/providers"
"github.com/hashicorp/terraform/internal/states"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
)

type graphNodeImportState struct {
Expand Down Expand Up @@ -115,7 +116,17 @@ func (n *graphNodeImportState) Execute(ctx EvalContext, op walkOperation) (diags
// Providers are supposed to return null values for all write-only attributes
var writeOnlyDiags tfdiags.Diagnostics
for _, imported := range resp.ImportedResources {
writeOnlyDiags = ephemeral.ValidateWriteOnlyAttributes(imported.State, schema, n.ResolvedProvider, n.Addr)
writeOnlyDiags = ephemeral.ValidateWriteOnlyAttributes(
"Import returned a non-null value for a write-only attribute",
func(path cty.Path) string {
return fmt.Sprintf(
"Provider %q returned a value for the write-only attribute \"%s%s\" during import. Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.",
n.ResolvedProvider, n.Addr, tfdiags.FormatCtyPath(path),
)
},
imported.State,
schema,
)
diags = diags.Append(writeOnlyDiags)
}

Expand Down
14 changes: 12 additions & 2 deletions internal/terraform/node_resource_plan_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
}

// We skip the read and further validation since we make up the state
// of the imported resource anyways.
// of the imported resource anyways.
SarahFrench marked this conversation as resolved.
Show resolved Hide resolved
return state.AsInstanceObject(), deferred, diags
}

Expand All @@ -726,7 +726,17 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
}

// Providers are supposed to return null values for all write-only attributes
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(imported[0].State, schema, n.ResolvedProvider, n.Addr)
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(
"Import returned a non-null value for a write-only attribute",
func(path cty.Path) string {
return fmt.Sprintf(
"While attempting to import with ID %s, the provider %q returned a value for the write-only attribute \"%s%s\". Write-only attributes cannot be read back from the provider. This is a bug in the provider, which should be reported in the provider's own issue tracker.",
importId, n.ResolvedProvider, n.Addr, tfdiags.FormatCtyPath(path),
)
},
imported[0].State,
schema,
)
diags = diags.Append(writeOnlyDiags)

if writeOnlyDiags.HasErrors() {
Expand Down
Loading