Skip to content

Commit

Permalink
Fixup now renames "id" fields when they are user settable
Browse files Browse the repository at this point in the history 
`fixup` already renames fields called `"urn"`, since it is always invalid for a provider
resource to have a property called `"urn"`, since Pulumi will override it.

`fixup` now renames fields called `"id"` *only* when the `"id"` field is user
settable (Optional || Required), since Pulumi does not respect user set Inputs.

When we expect to have an ID property available (because the user or the provider will set
it), we use that as the source of our computed ID. If not, we still fallback to using
"missing ID" as the computed ID.

Fixes pulumi/pulumi-terraform-provider#29
  • Loading branch information
@iwahbe
iwahbe committed Sep 4, 2024
1 parent 9edfabf commit cf59a25
Show file tree
Hide file tree
Showing 5 changed files with 1,266 additions and 45 deletions.
108 changes: 79 additions & 29 deletions dynamic/internal/fixup/properties.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ import (
"errors"
"fmt"
"reflect"
"strings"

"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info"
Expand Down Expand Up @@ -100,54 +102,102 @@ func fixPropertyConflict(p *info.Provider) error {
return fmt.Errorf("must set p.Name")
}
return walkResources(p, func(r tfbridge.Resource) error {
var retError error
var retError []error
r.TF.Schema().Range(func(key string, value shim.Schema) bool {
if fix := badPropertyName(p.Name, key); fix != nil {
if r.Schema.Fields == nil {
r.Schema.Fields = map[string]*info.Schema{}
}

s, ok := r.Schema.Fields[key]
if !ok {
s = &info.Schema{}
}

if s.Name == "" {
var err error
s.Name, err = fix(r.TF)
if err != nil {
retError = fmt.Errorf("%q: %w", key, err)
}
}

if !ok {
r.Schema.Fields[key] = s
err := fix(r)
if err != nil {
retError = append(retError, fmt.Errorf("%s: %w", key, err))
}
}

return true
})
return retError
return errors.Join(retError...)
})
}

func badPropertyName(providerName, key string) func(shim.Resource) (string, error) {
func getField(i *map[string]*info.Schema, name string) *info.Schema {
contract.Assertf(i != nil, "i cannot be nil")
if *i == nil {
*i = map[string]*info.Schema{}
}
v, ok := (*i)[name]
if !ok {
v = new(info.Schema)
(*i)[name] = v
}
return v
}

type fixupProperty = func(tfbridge.Resource) error

func badPropertyName(providerName, key string) fixupProperty {
switch key {
case "urn":
return newName(providerName)
return fixURN(providerName)
case "id":
return fixID(providerName)
default:
return nil
}
}

func newName(providerName string) func(shim.Resource) (string, error) {
return func(r shim.Resource) (string, error) {
s := r.Schema()
func fixID(providerName string) fixupProperty {
return func(r tfbridge.Resource) error {
tfSchema := r.TF.Schema()
proposedIDFieldName := strings.ReplaceAll(providerName, "-", "_") + "_id"
tfIDProperty, ok := tfSchema.GetOk("id")
if !ok {
// could not find an ID
return nil
}

if _, ok := tfSchema.GetOk(proposedIDFieldName); ok {
return fmt.Errorf("no available new name, tried %q", proposedIDFieldName)
}

// If either id.Optional or id.Required are set, then the provider allows
// (or requires) the user to set "id" as an input. Pulumi does not allow
// that, so we alias "id".
if !tfIDProperty.Optional() && !tfIDProperty.Required() {
return nil
}

if f := getField(&r.Schema.Fields, "id"); f.Name == "" {
newIDField := tfbridge.TerraformToPulumiNameV2(proposedIDFieldName, tfSchema, r.Schema.Fields)
f.Name = newIDField

// We are altering the original ID because it is valid for the
// user to set it. As long as it will be present as an output, we
// should still be able to use it as the actual ID.
//
// We expect newIDField to be present in the output space when:
//
// 1. The user *must* set it.
// 2. The user *may* set it, but if they don't then the provider will.
if tfIDProperty.Required() || (tfIDProperty.Optional() && tfIDProperty.Computed()) {
r.Schema.ComputeID = tfbridge.DelegateIDField(resource.PropertyKey(newIDField), providerName,
"https://github.com/pulumi/pulumi-terraform-provider")
}
}

return nil
}
}

func fixURN(providerName string) fixupProperty {
return func(r tfbridge.Resource) error {
s := r.TF.Schema()
v := providerName + "_urn"
if _, ok := s.GetOk(v); !ok {
return tfbridge.TerraformToPulumiNameV2(v, s, nil), nil
if _, ok := s.GetOk(v); ok {
return fmt.Errorf("no available new name, tried %q", v)
}
return "", fmt.Errorf("no available new name, tried %q", v)
if f := getField(&r.Schema.Fields, "urn"); f.Name == "" {
f.Name = tfbridge.TerraformToPulumiNameV2(v, s, r.Schema.Fields)
}
return nil

}
}

Expand Down
157 changes: 150 additions & 7 deletions dynamic/internal/fixup/properties_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,26 +51,169 @@ func TestFixMissingID(t *testing.T) {
assert.NotNil(t, p.Resources["test_res"].ComputeID)
}

func TestFixURNProperty(t *testing.T) {
func TestFixPropertyConflicts(t *testing.T) {
t.Parallel()

simpleID := (&schema.Schema{
Type: shim.TypeString,
Computed: true,
}).Shim()

tests := []struct {
name string

schema schema.SchemaMap
info map[string]*info.Schema

expected map[string]*info.Schema
expectComputeIDSet bool
}{
{
name: "fix urn property name",
schema: schema.SchemaMap{
"urn": (&schema.Schema{
Type: shim.TypeString,
}).Shim(),
"id": simpleID,
},
expected: map[string]*info.Schema{
"urn": {Name: "testUrn"},
},
},
{
name: "ignore overridden urn property name",
schema: schema.SchemaMap{
"urn": (&schema.Schema{
Type: shim.TypeString,
}).Shim(),
"id": simpleID,
},
info: map[string]*info.Schema{
"urn": {Name: "overridden"},
},
expected: map[string]*info.Schema{
"urn": {Name: "overridden"},
},
},
{
name: "fix ID property name (computed + optional)",
schema: schema.SchemaMap{
"id": (&schema.Schema{
Type: shim.TypeString,
Optional: true,
Computed: true,
}).Shim(),
},
expected: map[string]*info.Schema{
"id": {Name: "testId"},
},
expectComputeIDSet: true,
},
{
name: "fix ID property name (required)",
schema: schema.SchemaMap{
"id": (&schema.Schema{
Type: shim.TypeString,
Required: true,
}).Shim(),
},
expected: map[string]*info.Schema{
"id": {Name: "testId"},
},
expectComputeIDSet: true,
},

{
name: "ignore output ID property name (computed)",
schema: schema.SchemaMap{
"id": (&schema.Schema{
Type: shim.TypeString,
Computed: true,
}).Shim(),
},
expected: nil,
},
{
name: "ignore overridden ID property name",
schema: schema.SchemaMap{
"id": (&schema.Schema{
Type: shim.TypeString,
Computed: true,
Optional: true,
}).Shim(),
},
info: map[string]*info.Schema{
"id": {Name: "overridden"},
},
expected: map[string]*info.Schema{
"id": {Name: "overridden"},
},
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
p := info.Provider{
Name: "test",
P: (&schema.Provider{
ResourcesMap: schema.ResourceMap{
"test_res": (&schema.Resource{
Schema: tt.schema,
}).Shim(),
},
}).Shim(),
Resources: map[string]*info.Resource{
"test_res": {
Fields: tt.info,
},
},
}
err := fixup.Default(&p)
require.NoError(t, err)

r := p.Resources["test_res"]
assert.Equal(t, tt.expected, r.Fields)

if tt.expectComputeIDSet {
assert.NotNil(t, r.ComputeID, "expected .ComputeID to be set")
} else {
assert.Nil(t, r.ComputeID, "expected .ComputeID to not be set")
}
})
}
}

// TestFixIDKebabCaseProvider validates that providers with kebab-case names that have ID
// fields that need fixups still result in good (camelCase) property names for Pulumi
// schemas.
func TestFixIDKebabCaseProvider(t *testing.T) {
t.Parallel()

p := info.Provider{
Name: "test",
Name: "test-provider",
P: (&schema.Provider{
ResourcesMap: schema.ResourceMap{
"test_res": (&schema.Resource{
"test-provider_res": (&schema.Resource{
Schema: schema.SchemaMap{
"urn": (&schema.Schema{
Type: shim.TypeString,
"id": (&schema.Schema{
Type: shim.TypeString,
Required: true,
}).Shim(),
},
}).Shim(),
},
}).Shim(),
}

err := fixup.Default(&p)
require.NoError(t, err)
assert.Equal(t, &info.Schema{Name: "testUrn"}, p.Resources["test_res"].Fields["urn"])

r := p.Resources["test-provider_res"]
assert.Equal(t, map[string]*info.Schema{
"id": {Name: "testProviderId"},
}, r.Fields)
assert.NotNil(t, r.ComputeID)
}

func TestFixProviderResourceName(t *testing.T) {
Expand Down
18 changes: 16 additions & 2 deletions dynamic/testdata/TestSchemaGeneration/Azure/alz-0.11.1.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,10 @@
"resources": {
"alz:index/policyRoleAssignments:PolicyRoleAssignments": {
"properties": {
"alzId": {
"type": "string",
"description": "The id of the management group, forming the last part of the resource ID.\n"
},
"assignments": {
"type": "object",
"additionalProperties": {
Expand All @@ -568,9 +572,14 @@
}
},
"required": [
"assignments"
"assignments",
"alzId"
],
"inputProperties": {
"alzId": {
"type": "string",
"description": "The id of the management group, forming the last part of the resource ID.\n"
},
"assignments": {
"type": "object",
"additionalProperties": {
Expand All @@ -579,11 +588,16 @@
}
},
"requiredInputs": [
"assignments"
"assignments",
"alzId"
],
"stateInputs": {
"description": "Input properties used for looking up and filtering PolicyRoleAssignments resources.\n",
"properties": {
"alzId": {
"type": "string",
"description": "The id of the management group, forming the last part of the resource ID.\n"
},
"assignments": {
"type": "object",
"additionalProperties": {
Expand Down
Loading

0 comments on commit cf59a25

Please sign in to comment.