Skip to content

Commit

Permalink
Use different summaries and details when reporting errors about non-n…
Browse files Browse the repository at this point in the history
…ull write-only attribute values (#36274)

* Use different summaries when reporting errors about non-null write-only attribute values

* Refactor `ValidateWriteOnlyAttributes` to use custom details for diagnostics

* Update affected unit tests

* Add change file
  • Loading branch information
SarahFrench authored Jan 8, 2025
1 parent 4074d35 commit d5efc81
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 23 deletions.
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),
))
}
}
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.
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

0 comments on commit d5efc81

Please sign in to comment.