Skip to content

Commit

Permalink
PluginFramework encoders now parse "" as nil numbers (#2921)
Browse files Browse the repository at this point in the history 
This is a workaround until #1667 can guarantee that RawState is parsed
correctly and passed to migration machinery.

Fixes #2903

In theory there should be no harm to be a little tolerant when parsing
incorrectly typed data, but there may be other use cases that do not
agree that "" should be interpreted as nil.

For the moment this workaround fixes a popular issue in `pulumi-aws`:

    pulumi/pulumi-aws#5222

---------

Co-authored-by: corymhall <[email protected]>
  • Loading branch information
@t0yv0 @corymhall
t0yv0 and corymhall authored Feb 28, 2025
1 parent 0fc4e0e commit 8864b73
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 11 deletions.
9 changes: 8 additions & 1 deletion pkg/convert/number.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,16 @@ func (*numberEncoder) fromPropertyValue(p resource.PropertyValue) (tftypes.Value
if p.IsNull() {
return tftypes.NewValue(tftypes.Number, nil), nil
}
// TODO[pulumi/pulumi-terraform-bridge#1667] workaround a problem in
// https://github.com/pulumi/pulumi-aws/issues/5222 where SDKv2 TypeNullableInt gets parsed
// into a schema that now expects a number. This case interprets an empty string value as a
// nil number when parsing numbers.
if p.IsString() && p.StringValue() == "" {
return tftypes.NewValue(tftypes.Number, nil), nil
}
if !p.IsNumber() {
return tftypes.NewValue(tftypes.Number, nil),
fmt.Errorf("Expected a Number")
fmt.Errorf("Expected a Number, got %#v %v", p, p.IsString())
}
return tftypes.NewValue(tftypes.Number, p.NumberValue()), nil
}
Expand Down
30 changes: 30 additions & 0 deletions pkg/convert/number_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2016-2025, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package convert

import (
"testing"

"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/stretchr/testify/require"
)

func Test_numberEncoder_emptyStringToNull(t *testing.T) {
t.Parallel()
n := newNumberEncoder()
v, err := n.fromPropertyValue(resource.NewStringProperty(""))
require.NoError(t, err)
require.True(t, v.IsKnown())
}
27 changes: 17 additions & 10 deletions pkg/pf/internal/providerbuilder/build_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ type NewResourceArgs struct {
Name string
ResourceSchema schema.Schema

CreateFunc func(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse)
ReadFunc func(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse)
UpdateFunc func(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse)
DeleteFunc func(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse)
ImportStateFunc func(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse)
CreateFunc func(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse)
ReadFunc func(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse)
UpdateFunc func(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse)
DeleteFunc func(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse)
ImportStateFunc func(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse)
UpgradeStateFunc func(context.Context) map[int64]resource.StateUpgrader
}

// NewResource creates a new Resource with the given parameters, filling reasonable defaults.
Expand Down Expand Up @@ -67,6 +68,11 @@ func NewResource(args NewResourceArgs) Resource {
resp.State = tfsdk.State(req.Plan)
}
}
if args.UpgradeStateFunc == nil {
args.UpgradeStateFunc = func(ctx context.Context) map[int64]resource.StateUpgrader {
return nil
}
}

return Resource(args)
}
Expand All @@ -76,11 +82,12 @@ type Resource struct {
Name string
ResourceSchema schema.Schema

CreateFunc func(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse)
ReadFunc func(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse)
UpdateFunc func(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse)
DeleteFunc func(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse)
ImportStateFunc func(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse)
CreateFunc func(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse)
ReadFunc func(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse)
UpdateFunc func(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse)
DeleteFunc func(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse)
ImportStateFunc func(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse)
UpgradeStateFunc func(context.Context) map[int64]resource.StateUpgrader
}

func (r *Resource) Metadata(ctx context.Context, req resource.MetadataRequest, re *resource.MetadataResponse) {
Expand Down
137 changes: 137 additions & 0 deletions pkg/pf/tests/schema_and_program_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,27 @@ package tfbridgetests
import (
"context"
"encoding/json"
"fmt"
"os"
"path/filepath"
"strconv"
"testing"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
pschema "github.com/hashicorp/terraform-plugin-framework/provider/schema"
"github.com/hashicorp/terraform-plugin-framework/resource"
rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
"github.com/pulumi/pulumi/sdk/v3/go/auto/optpreview"
"github.com/pulumi/pulumi/sdk/v3/go/common/apitype"
presource "github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -413,3 +419,134 @@ outputs:

pt.Preview(t, optpreview.Diff(), optpreview.ExpectNoChanges())
}

type lifecycleRuleFilterModel struct {
ObjectSizeGreaterThan types.Int64 `tfsdk:"object_size_greater_than"`
}
type lifecycleRuleFilterModelV0 struct {
ObjectSizeGreaterThan types.String `tfsdk:"object_size_greater_than"`
}

func stringToInt64Legacy(_ context.Context, s types.String, diags *diag.Diagnostics) types.Int64 {
if s.ValueString() == "" {
return types.Int64Null()
}

v, err := strconv.ParseInt(s.ValueString(), 10, 64)
if err != nil {
diags.AddError(
"Conversion Error",
fmt.Sprintf("When upgrading state, failed to read a string as an integer value.\n"+
"Value: %q\nError: %s",
s.ValueString(),
err.Error(),
),
)
return types.Int64Unknown()
}
return types.Int64Value(v)
}

func TestStateUpgrade(t *testing.T) {
t.Parallel()
provBuilder := providerbuilder.NewProvider(
providerbuilder.NewProviderArgs{
AllResources: []providerbuilder.Resource{
providerbuilder.NewResource(providerbuilder.NewResourceArgs{
UpgradeStateFunc: func(ctx context.Context) map[int64]resource.StateUpgrader {
return map[int64]resource.StateUpgrader{
0: {
StateUpgrader: func(ctx context.Context, usr1 resource.UpgradeStateRequest, usr2 *resource.UpgradeStateResponse) {
var usr1V0 lifecycleRuleFilterModelV0
usr2.Diagnostics.Append(usr1.State.Get(ctx, &usr1V0)...)
usr2.Diagnostics.Append(usr2.State.Set(ctx, lifecycleRuleFilterModel{
ObjectSizeGreaterThan: stringToInt64Legacy(ctx, usr1V0.ObjectSizeGreaterThan, &usr2.Diagnostics),
})...)
},
PriorSchema: &rschema.Schema{
Version: 0,
Attributes: map[string]rschema.Attribute{
"object_size_greater_than": rschema.StringAttribute{
Optional: true,
},
},
},
},
}
},
ResourceSchema: rschema.Schema{
Attributes: map[string]rschema.Attribute{
"object_size_greater_than": rschema.Int64Attribute{
Optional: true,
Computed: true, // Because of Legacy value handling
PlanModifiers: []planmodifier.Int64{
int64planmodifier.UseStateForUnknown(),
},
},
},
},
}),
},
})

prov := provBuilder.ToProviderInfo()

program := `
name: test
runtime: yaml
resources:
mainRes:
type: testprovider:index:Test`

pt, err := pulcheck.PulCheck(t, prov, program)
require.NoError(t, err)
pt.ImportStack(t, apitype.UntypedDeployment{
Version: 3,
Deployment: []byte(`{
"manifest": {
"time": "2025-02-20T14:09:00.155613-05:00",
"magic": "0cfd49ecb2b79ab5c815533dd5e24026f84295ad05c68df3861ea07ea846919a",
"version": "v3.150.0"
},
"resources": [
{
"urn": "urn:pulumi:test::test::pulumi:pulumi:Stack::test-test",
"custom": false,
"type": "pulumi:pulumi:Stack",
"created": "2025-02-20T19:09:00.146543Z",
"modified": "2025-02-20T19:09:00.146543Z"
},
{
"urn": "urn:pulumi:test::test::pulumi:providers:testprovider::default",
"custom": true,
"id": "127dc091-46cd-41f1-a3a9-ccdeca036b02",
"type": "pulumi:providers:testprovider",
"created": "2025-02-20T19:09:00.151776Z",
"modified": "2025-02-20T19:09:00.151776Z"
},
{
"urn": "urn:pulumi:test::test::testprovider:index/test:Test::mainRes",
"custom": true,
"id": "test-id",
"type": "testprovider:index/test:Test",
"inputs": {
},
"outputs": {
"id": "test-id",
"objectSizeGreaterThan": ""
},
"parent": "urn:pulumi:test::test::pulumi:pulumi:Stack::test-test",
"provider": "urn:pulumi:test::test::pulumi:providers:testprovider::default::127dc091-46cd-41f1-a3a9-ccdeca036b02",
"propertyDependencies": {
"objectSizeGreaterThan": []
},
"created": "2025-02-20T19:09:00.154504Z",
"modified": "2025-02-20T19:09:00.154504Z"
}
],
"metadata": {}
}`),
})

pt.Up(t)
}

0 comments on commit 8864b73

Please sign in to comment.