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

Validate write-only attributes returned from providers during the UpgradeResourceState RPC #36305

Merged
merged 8 commits into from
Jan 14, 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 internal/providers/testing/provider_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,11 @@ func (p *MockProvider) ValidateEphemeralResourceConfig(r providers.ValidateEphem
return resp
}

// UpgradeResourceState mocks out the response from the provider during an UpgradeResourceState RPC
// The default logic will return the resource's state unchanged, unless other logic is defined on the mock (e.g. UpgradeResourceStateFn)
//
// When using this mock you may need to provide custom logic if the plugin-framework alters values in state,
// e.g. when handling write-only attributes.
func (p *MockProvider) UpgradeResourceState(r providers.UpgradeResourceStateRequest) (resp providers.UpgradeResourceStateResponse) {
p.Lock()
defer p.Unlock()
Expand Down
10 changes: 10 additions & 0 deletions internal/terraform/context_apply_ephemeral_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,16 @@ resource "ephem_write_only" "wo" {
},
},
}
// Below we force the write_only attribute's returned state to be Null, mimicking what the plugin-framework would
// return during an UpgradeResourceState RPC
ephem.UpgradeResourceStateFn = func(ursr providers.UpgradeResourceStateRequest) providers.UpgradeResourceStateResponse {
return providers.UpgradeResourceStateResponse{
UpgradedState: cty.ObjectVal(map[string]cty.Value{
"normal": cty.StringVal("normal"),
"write_only": cty.NullVal(cty.String),
}),
}
}

ctx := testContext2(t, &ContextOpts{
Providers: map[addrs.Provider]providers.Factory{
Expand Down
60 changes: 60 additions & 0 deletions internal/terraform/context_plan2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6068,3 +6068,63 @@ data "test_data_source" "foo" {
_, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)))
assertNoErrors(t, diags)
}

func TestContext2Plan_upgradeState_WriteOnlyAttribute(t *testing.T) {
m := testModuleInline(t, map[string]string{
"main.tf": `
resource "aws_instance" "foo" {
wo_attr = "value"
}
`,
})

p := testProvider("aws")
p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&providerSchema{
ResourceTypes: map[string]*configschema.Block{
"aws_instance": {
Attributes: map[string]*configschema.Attribute{
"wo_attr": {
Type: cty.String,
Optional: true,
WriteOnly: true,
},
},
},
},
})
p.UpgradeResourceStateFn = func(r providers.UpgradeResourceStateRequest) (resp providers.UpgradeResourceStateResponse) {
return providers.UpgradeResourceStateResponse{
UpgradedState: cty.ObjectVal(map[string]cty.Value{
"wo_attr": cty.StringVal("not-empty"),
}),
}
}

ctx := testContext2(t, &ContextOpts{
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
},
})

priorState := states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
mustResourceInstanceAddr("aws_instance.foo"),
&states.ResourceInstanceObjectSrc{
// The UpgradeResourceStateFn above does not care about specific prior
// state but it must not be empty for the function to be actually called
AttrsJSON: []byte(`{"wo_attr":null}`),
Status: states.ObjectReady,
}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
)
})

// Plan should invoke state upgrade logic and trigger validation, given the mocks above
_, diags := ctx.Plan(m, priorState, DefaultPlanOpts)
if !diags.HasErrors() {
t.Fatalf("expected errors but got none")
}

if got, want := diags.Err().Error(), "Invalid resource state upgrade"; !strings.Contains(got, want) {
t.Errorf("unexpected error message\ngot: %s\nwant substring: %s", got, want)
}
}
19 changes: 19 additions & 0 deletions internal/terraform/upgrade_resource_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs/configschema"
"github.com/hashicorp/terraform/internal/lang/ephemeral"
"github.com/hashicorp/terraform/internal/providers"
"github.com/hashicorp/terraform/internal/states"
"github.com/hashicorp/terraform/internal/tfdiags"
Expand Down Expand Up @@ -121,6 +122,24 @@ func upgradeResourceState(addr addrs.AbsResourceInstance, provider providers.Int
return nil, diags
}

// Check for any write-only attributes that have non-null values
writeOnlyDiags := ephemeral.ValidateWriteOnlyAttributes(
"Invalid resource state upgrade",
func(path cty.Path) string {
return fmt.Sprintf(
"While attempting to upgrade state of resource %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.",
addr, providerType, addr, tfdiags.FormatCtyPath(path),
)
},
newValue,
currentSchema,
)
diags = diags.Append(writeOnlyDiags)

if writeOnlyDiags.HasErrors() {
return nil, diags
}

new, err := src.CompleteUpgrade(newValue, currentSchema.ImpliedType(), uint64(currentVersion))
if err != nil {
// We already checked for type conformance above, so getting into this
Expand Down
Loading