Skip to content

Commit

Permalink
Prevent EnsureProviderValid from mutating
Browse files Browse the repository at this point in the history
This function has caused repeated test failures with concurrent map mutations, the most
recent of which is
https://github.com/pulumi/pulumi-terraform-bridge/actions/runs/12772071950/job/35600764628?pr=2832. This
commit converts the function to return a copy instead of mutating.
  • Loading branch information
iwahbe committed Jan 15, 2025
1 parent 5ed2021 commit a913512
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 48 deletions.
43 changes: 37 additions & 6 deletions pkg/internal/tests/pulcheck/pulcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,39 @@ func resourceNeedsUpdate(res *schema.Resource) bool {
return false
}

func copyMap[K comparable, V any](m map[K]V, cp func(V) V) map[K]V {
dst := make(map[K]V, len(m))
for k, v := range m {
dst[k] = cp(v)
}
return dst
}

// WithValidProvider returns a copy of tfp, with all required fields filled with default
// values.
//
// This is an experimental API.
func EnsureProviderValid(t T, tfp *schema.Provider) {
func WithValidProvider(t T, tfp *schema.Provider) *schema.Provider {
if tfp == nil {
return nil
}

// Copy tfp as deep as we will mutate.
{
dst := *tfp // memcopy
dst.ResourcesMap = copyMap(tfp.ResourcesMap, func(v *schema.Resource) *schema.Resource {
cp := *v // memcopy
cp.Schema = copyMap(cp.Schema, func(s *schema.Schema) *schema.Schema {
cp := *s
return &cp
})
return &cp
})
tfp = &dst
}

// Now ensure that tfp is valid

for _, r := range tfp.ResourcesMap {
if r.Schema["id"] == nil {
r.Schema["id"] = &schema.Schema{
Expand Down Expand Up @@ -108,6 +139,8 @@ func EnsureProviderValid(t T, tfp *schema.Provider) {
}
}
require.NoError(t, tfp.InternalValidate())

return tfp
}

func ProviderServerFromInfo(
Expand Down Expand Up @@ -206,7 +239,7 @@ func BridgedProvider(t T, providerName string, tfp *schema.Provider, opts ...Bri
opt(&options)
}

EnsureProviderValid(t, tfp)
tfp = WithValidProvider(t, tfp)

// If the PULUMI_ACCURATE_BRIDGE_PREVIEWS environment variable is set, use it to enable
// accurate bridge previews.
Expand All @@ -230,10 +263,8 @@ func BridgedProvider(t T, providerName string, tfp *schema.Provider, opts ...Bri
EnableAccurateBridgePreview: accurateBridgePreviews,
Config: options.configInfo,
}
makeToken := func(module, name string) (string, error) {
return tokens.MakeStandard(providerName)(module, name)
}
provider.MustComputeTokens(tokens.SingleModule(providerName, "index", makeToken))
provider.MustComputeTokens(tokens.SingleModule(providerName,
"index", tokens.MakeStandard(providerName)))

return provider
}
Expand Down
83 changes: 42 additions & 41 deletions pkg/tests/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,54 +16,57 @@ import (
"gopkg.in/yaml.v3"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/internal/tests/pulcheck"
"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info"
)

func TestFullyComputedNestedAttribute(t *testing.T) {
t.Parallel()
resMap := map[string]*schema.Resource{
"prov_test": {
Schema: map[string]*schema.Schema{
"attached_disks": {
Type: schema.TypeList,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"name": {
Optional: true,
Type: schema.TypeString,
},
"key256": {
Computed: true,
Type: schema.TypeString,

bridgedProvider := func(importVal any) info.Provider {
return pulcheck.BridgedProvider(t, "prov", &schema.Provider{
ResourcesMap: map[string]*schema.Resource{
"prov_test": {
Schema: map[string]*schema.Schema{
"attached_disks": {
Type: schema.TypeList,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"name": {
Optional: true,
Type: schema.TypeString,
},
"key256": {
Computed: true,
Type: schema.TypeString,
},
},
},
},
"top_level_computed": {
Type: schema.TypeString,
Computed: true,
},
},
},
"top_level_computed": {
Type: schema.TypeString,
Computed: true,
},
},
},
}

importer := func(val any) func(context.Context, *schema.ResourceData, interface{}) ([]*schema.ResourceData, error) {
return func(ctx context.Context, rd *schema.ResourceData, i interface{}) ([]*schema.ResourceData, error) {
elMap := map[string]any{
"name": "disk1",
"key256": val,
}
err := rd.Set("attached_disks", []map[string]any{elMap})
require.NoError(t, err)
Importer: &schema.ResourceImporter{
StateContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) ([]*schema.ResourceData, error) {
elMap := map[string]any{
"name": "disk1",
"key256": importVal,
}
err := rd.Set("attached_disks", []map[string]any{elMap})
require.NoError(t, err)

err = rd.Set("top_level_computed", "computed_val")
require.NoError(t, err)
err = rd.Set("top_level_computed", "computed_val")
require.NoError(t, err)

return []*schema.ResourceData{rd}, nil
}
return []*schema.ResourceData{rd}, nil
},
},
},
},
})
}
tfp := &schema.Provider{ResourcesMap: resMap}
bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp)

program := `
name: test
Expand All @@ -83,9 +86,7 @@ runtime: yaml
},
} {
t.Run(tc.name, func(t *testing.T) {
resMap["prov_test"].Importer = &schema.ResourceImporter{
StateContext: importer(tc.importVal),
}
bridgedProvider := bridgedProvider(tc.importVal)

pt := pulcheck.PulCheck(t, bridgedProvider, program)

Expand Down
2 changes: 1 addition & 1 deletion pkg/tests/tfcheck/tfcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func NewTfDriver(t pulcheck.T, dir, providerName string, prov any) *TFDriver {
}

func newTfDriverSDK(t pulcheck.T, dir, providerName string, prov *schema.Provider) *TFDriver {
pulcheck.EnsureProviderValid(t, prov)
prov = pulcheck.WithValidProvider(t, prov)
v6server, err := tf5to6server.UpgradeServer(context.Background(),
func() tfprotov5.ProviderServer { return prov.GRPCProvider() })
require.NoError(t, err)
Expand Down

0 comments on commit a913512

Please sign in to comment.