From 210092f3d4fa2eb3254fd1b3c78874f4c32f39ee Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 22 Mar 2024 20:40:36 +0000 Subject: [PATCH] Don't send writeOnly properties if they're also createOnly (#1448) Fixes #1435 Ideally we'd have an integration test around this case to avoid future regression but currently the VPC-based test is very unstable, so has been disabled, but included for documentation of the issues. - Deletion of the `aws-native:ec2:IpamPoolCidr` regularly fails with `"GeneralServiceException": Error occurred during operation 'The CIDR has one or more allocations.'` even after the VPC is deleted for serveral minutes. - Sometimes Update of the `aws-native:ec2:IpamPool` fails with `"NotStabilized": IpamPoolCidrFailureReason(Message=The CIDR has one or more allocations.` - The refresh of the `aws-native:ec2:IpamPool` fails due to a diff in the `provisionedCidrs` property. - It's also not ideal that we have to create the IPAM manually to avoid the test failing if run in parallel as only 1 IPAM is allowed to be created in each region. Extracted the patch generation code instead and wrote a unit around it. Also replicated the fix into the untyped ExtensionResource implementation and tested using the same test suite. --- examples/examples_py_test.go | 21 +++ examples/vpc-py/step1/.gitignore | 2 + examples/vpc-py/step1/Pulumi.yaml | 3 + examples/vpc-py/step1/__main__.py | 26 +++ examples/vpc-py/step1/requirements.txt | 2 + examples/vpc-py/step2/__main__.py | 25 +++ .../pulumi-resource-aws-native/schema.json | 7 + provider/pkg/naming/convert.go | 10 +- provider/pkg/provider/provider.go | 14 +- provider/pkg/resources/extension_resource.go | 150 +++++++----------- provider/pkg/resources/patching.go | 85 ++++++++++ provider/pkg/resources/patching_test.go | 114 +++++++++++++ sdk/dotnet/ExtensionResource.cs | 13 ++ sdk/go/aws/extensionResource.go | 6 + sdk/nodejs/extensionResource.ts | 6 + .../pulumi_aws_native/extension_resource.py | 23 +++ 16 files changed, 402 insertions(+), 105 deletions(-) create mode 100644 examples/vpc-py/step1/.gitignore create mode 100644 examples/vpc-py/step1/Pulumi.yaml create mode 100644 examples/vpc-py/step1/__main__.py create mode 100644 examples/vpc-py/step1/requirements.txt create mode 100644 examples/vpc-py/step2/__main__.py create mode 100644 provider/pkg/resources/patching.go create mode 100644 provider/pkg/resources/patching_test.go diff --git a/examples/examples_py_test.go b/examples/examples_py_test.go index a82de81cb0..725d93570f 100644 --- a/examples/examples_py_test.go +++ b/examples/examples_py_test.go @@ -75,6 +75,27 @@ func TestDefaultTagsPython(t *testing.T) { integration.ProgramTest(t, &test) } +func TestVpcPython(t *testing.T) { + // This test is not stable for several reasons so we can't run it in CI. + // Deletion of the aws-native:ec2:IpamPoolCidr regularly fails with "GeneralServiceException: Error occurred during operation 'The CIDR has one or more allocations.'" even after the VPC is deleted. + // Sometimes Update of the aws-native:ec2:IpamPool fails with "NotStabilized: IpamPoolCidrFailureReason(Message=The CIDR has one or more allocations." + // The refresh of the aws-native:ec2:IpamPool fails due to a diff in the provisionedCidrs property. + // It's also not ideal that we have to create the IPAM manually to avoid the test failing if run in parallel. + t.SkipNow() + test := getPythonBaseOptions(t). + With(integration.ProgramTestOptions{ + Dir: filepath.Join(getCwd(t), "vpc-py", "step1"), + EditDirs: []integration.EditDir{ + { + Dir: filepath.Join(getCwd(t), "vpc-py", "step2"), + Additive: true, + }, + }, + }) + + integration.ProgramTest(t, &test) +} + func getPythonBaseOptions(t *testing.T) integration.ProgramTestOptions { base := getBaseOptions(t) basePy := base.With(integration.ProgramTestOptions{ diff --git a/examples/vpc-py/step1/.gitignore b/examples/vpc-py/step1/.gitignore new file mode 100644 index 0000000000..a3807e5bdb --- /dev/null +++ b/examples/vpc-py/step1/.gitignore @@ -0,0 +1,2 @@ +*.pyc +venv/ diff --git a/examples/vpc-py/step1/Pulumi.yaml b/examples/vpc-py/step1/Pulumi.yaml new file mode 100644 index 0000000000..e635e5b383 --- /dev/null +++ b/examples/vpc-py/step1/Pulumi.yaml @@ -0,0 +1,3 @@ +name: simple-py +runtime: python +description: Test updating a VPC which has properties which are both create-only and write-only diff --git a/examples/vpc-py/step1/__main__.py b/examples/vpc-py/step1/__main__.py new file mode 100644 index 0000000000..acfc5f79ba --- /dev/null +++ b/examples/vpc-py/step1/__main__.py @@ -0,0 +1,26 @@ +# Copyright 2021, Pulumi Corporation. All rights reserved. + +import pulumi +from pulumi_aws_native import ec2 +from pulumi_command import local + +aws_native_config = pulumi.Config("aws-native") + +# Only 1 IPAM can exist per region, so this must be created manually as a pre-requisite and is therefore looked up here. +ipamId = local.run( + command="aws ec2 describe-ipams --query 'Ipams[0].IpamId' --output text").stdout + +ipam = ec2.get_ipam(ipam_id=ipamId) + +ipamPool = ec2.IpamPool("ipamPool", address_family="ipv4", + ipam_scope_id=ipam.private_default_scope_id, + locale=aws_native_config.require("region")) + +ipamPoolCidr = ec2.IpamPoolCidr( + "ipamPoolCidr", ipam_pool_id=ipamPool.id, cidr="10.0.0.0/16") + +vpc = ec2.Vpc("vpc", + # enable_dns_hostnames=True, + ipv4_ipam_pool_id=ipamPool.id, + ipv4_netmask_length=20, + opts=pulumi.ResourceOptions(depends_on=[ipamPoolCidr])) diff --git a/examples/vpc-py/step1/requirements.txt b/examples/vpc-py/step1/requirements.txt new file mode 100644 index 0000000000..c9e3713076 --- /dev/null +++ b/examples/vpc-py/step1/requirements.txt @@ -0,0 +1,2 @@ +pulumi>=3.5.1,<4.0.0 +pulumi-command>=0.9.2,<2.0.0 diff --git a/examples/vpc-py/step2/__main__.py b/examples/vpc-py/step2/__main__.py new file mode 100644 index 0000000000..f9861dc611 --- /dev/null +++ b/examples/vpc-py/step2/__main__.py @@ -0,0 +1,25 @@ +# Copyright 2021, Pulumi Corporation. All rights reserved. + +import pulumi +from pulumi_aws_native import ec2 +from pulumi_command import local + +aws_native_config = pulumi.Config("aws-native") + +ipamId = local.run( + command="aws ec2 describe-ipams --query 'Ipams[0].IpamId' --output text").stdout + +ipam = ec2.get_ipam(ipam_id=ipamId) + +ipamPool = ec2.IpamPool("ipamPool", address_family="ipv4", + ipam_scope_id=ipam.private_default_scope_id, + locale=aws_native_config.require("region")) + +ipamPoolCidr = ec2.IpamPoolCidr( + "ipamPoolCidr", ipam_pool_id=ipamPool.id, cidr="10.0.0.0/16") + +vpc = ec2.Vpc("vpc", + enable_dns_hostnames=True, + ipv4_ipam_pool_id=ipamPool.id, + ipv4_netmask_length=20, + opts=pulumi.ResourceOptions(depends_on=[ipamPoolCidr])) diff --git a/provider/cmd/pulumi-resource-aws-native/schema.json b/provider/cmd/pulumi-resource-aws-native/schema.json index 7182aa5888..27c1c998b5 100644 --- a/provider/cmd/pulumi-resource-aws-native/schema.json +++ b/provider/cmd/pulumi-resource-aws-native/schema.json @@ -173204,6 +173204,13 @@ "outputs" ], "inputProperties": { + "createOnly": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Property names as defined by `createOnlyProperties` in the CloudFormation schema. Create-only properties can't be set during updates, so will not be included in patches even if they are also marked as write-only, and will cause an error if attempted to be updated. Therefore any property here should also be included in the `replaceOnChanges` resource option too.\nIn the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`)." + }, "properties": { "type": "object", "additionalProperties": { diff --git a/provider/pkg/naming/convert.go b/provider/pkg/naming/convert.go index dd757643f5..06d022c060 100644 --- a/provider/pkg/naming/convert.go +++ b/provider/pkg/naming/convert.go @@ -5,6 +5,7 @@ package naming import ( "fmt" "reflect" + "slices" "strings" "github.com/mattbaird/jsonpatch" @@ -179,7 +180,14 @@ func (c *sdkToCfnConverter) sdkObjectValueToCfn(typeName string, spec metadata.C func (c *sdkToCfnConverter) diffToPatch(diff *resource.ObjectDiff) ([]jsonpatch.JsonPatchOperation, error) { var ops []jsonpatch.JsonPatchOperation - for sdkName, prop := range c.spec.Inputs { + // Sort keys to ensure deterministic ordering of patch operations. + sortedKeys := make([]string, 0, len(c.spec.Inputs)) + for sdkName := range c.spec.Inputs { + sortedKeys = append(sortedKeys, string(sdkName)) + } + slices.Sort(sortedKeys) + for _, sdkName := range sortedKeys { + prop := c.spec.Inputs[sdkName] cfnName := ToCfnName(sdkName, c.spec.IrreversibleNames) key := resource.PropertyKey(sdkName) if v, ok := diff.Updates[key]; ok { diff --git a/provider/pkg/provider/provider.go b/provider/pkg/provider/provider.go index 5f545ff8cd..9182e157bc 100644 --- a/provider/pkg/provider/provider.go +++ b/provider/pkg/provider/provider.go @@ -1074,19 +1074,7 @@ func (p *cfnProvider) Update(ctx context.Context, req *pulumirpc.UpdateRequest) return nil, errors.Errorf("Resource type %s not found", resourceToken) } - diff := oldInputs.Diff(newInputs) - - // Write-only properties can't even be read internally within the CloudControl service so they must be included in - // patch requests as adds to ensure the updated model validates. - for _, writeOnlyPropName := range spec.WriteOnly { - propKey := resource.PropertyKey(writeOnlyPropName) - if _, ok := diff.Sames[propKey]; ok { - delete(diff.Sames, propKey) - diff.Adds[propKey] = newInputs[propKey] - } - } - - ops, err := naming.DiffToPatch(&spec, p.resourceMap.Types, diff) + ops, err := resources.CalcPatch(oldInputs, newInputs, spec, p.resourceMap.Types) if err != nil { return nil, err } diff --git a/provider/pkg/resources/extension_resource.go b/provider/pkg/resources/extension_resource.go index e6788a4c08..49645fddcb 100644 --- a/provider/pkg/resources/extension_resource.go +++ b/provider/pkg/resources/extension_resource.go @@ -6,18 +6,17 @@ import ( "context" "fmt" - "github.com/mattbaird/jsonpatch" "github.com/pulumi/pulumi-aws-native/provider/pkg/client" "github.com/pulumi/pulumi-aws-native/provider/pkg/default_tags" "github.com/pulumi/pulumi-go-provider/resourcex" pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema" "github.com/pulumi/pulumi/sdk/v3/go/common/resource" - "github.com/wI2L/jsondiff" ) type ExtensionResourceInputs struct { Type string Properties map[string]any + CreateOnly []string WriteOnly []string TagsProperty string TagsStyle default_tags.TagsStyle @@ -163,41 +162,9 @@ func (r *extensionResource) Update(ctx context.Context, urn resource.URN, id str return nil, fmt.Errorf("changing the type of an extension resource is not supported") } - jsonDiffPatch, err := jsondiff.Compare(typedOldInputs.Properties, typedInputs.Properties) + jsonPatch, err := CalculateUntypedPatch(typedOldInputs, typedInputs) if err != nil { - return nil, fmt.Errorf("failed to compare properties: %w", err) - } - - // Write-only properties can't even be read internally within the CloudControl service so they must be included in - // patch requests as adds to ensure the updated model validates. - for _, writeOnlyPropName := range typedInputs.WriteOnly { - newValue, ok := typedInputs.Properties[writeOnlyPropName] - if !ok { - continue - } - hasPatch := false - for _, op := range jsonDiffPatch { - if op.Path == writeOnlyPropName { - hasPatch = true - break - } - } - if !hasPatch { - jsonDiffPatch = append(jsonDiffPatch, jsondiff.Operation{ - Type: "add", - Path: writeOnlyPropName, - Value: newValue, - }) - } - } - - jsonPatch := make([]jsonpatch.JsonPatchOperation, 0, len(jsonDiffPatch)) - for _, op := range jsonDiffPatch { - jsonPatch = append(jsonPatch, jsonpatch.JsonPatchOperation{ - Operation: op.Type, - Path: op.Path, - Value: op.Value, - }) + return nil, fmt.Errorf("failed to calculate patch: %w", err) } resourceState, err := r.client.Update(ctx, typedInputs.Type, id, jsonPatch) @@ -223,72 +190,73 @@ func (r *extensionResource) Delete(ctx context.Context, urn resource.URN, id str return nil } -func extensionResourceInputProperties() map[string]pschema.PropertySpec { - return map[string]pschema.PropertySpec{ - "type": { - Description: "CloudFormation type name. This has three parts, each separated by two colons. For AWS resources this starts with `AWS::` e.g. `AWS::Logs::LogGroup`. Third party resources should use a namespace prefix e.g. `MyCompany::MyService::MyResource`.", - TypeSpec: pschema.TypeSpec{ - Type: "string", - }, - ReplaceOnChanges: true, - }, - "properties": { - Description: "Property bag containing the properties for the resource. These should be defined using the casing expected by the CloudControl API as these values are sent exact as provided.", - TypeSpec: pschema.TypeSpec{ - Type: "object", - AdditionalProperties: &pschema.TypeSpec{ - Ref: "pulumi.json#/Any", +func ExtensionResourceSpec() pschema.ResourceSpec { + return pschema.ResourceSpec{ + ObjectTypeSpec: pschema.ObjectTypeSpec{ + Description: "A special resource that enables deploying CloudFormation Extensions (third-party resources). An extension has to be pre-registered in your AWS account in order to use this resource.", + Properties: map[string]pschema.PropertySpec{ + "outputs": { + Description: "Dictionary of the extension resource attributes.", + TypeSpec: pschema.TypeSpec{ + Type: "object", + AdditionalProperties: &pschema.TypeSpec{ + Ref: "pulumi.json#/Any", + }, + }, }, }, + Required: []string{"outputs"}, }, - "writeOnly": { - Description: "Property names as defined by `writeOnlyProperties` in the CloudFormation schema. Write-only properties are not returned during read operations and have to be included in all update operations as CloudControl itself can't read their previous values.\nIn the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`).", - TypeSpec: pschema.TypeSpec{ - Type: "array", - Items: &pschema.TypeSpec{ + InputProperties: map[string]pschema.PropertySpec{ + "type": { + Description: "CloudFormation type name. This has three parts, each separated by two colons. For AWS resources this starts with `AWS::` e.g. `AWS::Logs::LogGroup`. Third party resources should use a namespace prefix e.g. `MyCompany::MyService::MyResource`.", + TypeSpec: pschema.TypeSpec{ Type: "string", }, + ReplaceOnChanges: true, }, - }, - "tagsProperty": { - Description: "Optional name of the property containing the tags. Defaults to \"Tags\" if the `tagsStyle` is set to either \"stringMap\" or \"keyValueArray\". This is used to apply default tags to the resource and can be ignored if not using default tags.", - TypeSpec: pschema.TypeSpec{ - Type: "string", + "properties": { + Description: "Property bag containing the properties for the resource. These should be defined using the casing expected by the CloudControl API as these values are sent exact as provided.", + TypeSpec: pschema.TypeSpec{ + Type: "object", + AdditionalProperties: &pschema.TypeSpec{ + Ref: "pulumi.json#/Any", + }, + }, }, - }, - "tagsStyle": { - Description: "Optional style of tags this resource uses. Valid values are \"stringMap\", \"keyValueArray\" or \"none\". Defaults to `keyValueArray` if `tagsProperty` is set. This is used to apply default tags to the resource and can be ignored if not using default tags.", - TypeSpec: pschema.TypeSpec{ - Type: "string", + "createOnly": { + Description: "Property names as defined by `createOnlyProperties` in the CloudFormation schema. Create-only properties can't be set during updates, so will not be included in patches even if they are also marked as write-only, and will cause an error if attempted to be updated. Therefore any property here should also be included in the `replaceOnChanges` resource option too.\nIn the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`).", + TypeSpec: pschema.TypeSpec{ + Type: "array", + Items: &pschema.TypeSpec{ + Type: "string", + }, + }, }, - }, - } -} - -func ExtensionResourceSpec() pschema.ResourceSpec { - return pschema.ResourceSpec{ - ObjectTypeSpec: pschema.ObjectTypeSpec{ - Description: "A special resource that enables deploying CloudFormation Extensions (third-party resources). An extension has to be pre-registered in your AWS account in order to use this resource.", - Properties: extensionResourceOutputProperties(), - Required: []string{"outputs"}, - }, - InputProperties: extensionResourceInputProperties(), - RequiredInputs: []string{"type", "properties"}, - } -} - -func extensionResourceOutputProperties() map[string]pschema.PropertySpec { - properties := map[string]pschema.PropertySpec{} - properties["outputs"] = pschema.PropertySpec{ - Description: "Dictionary of the extension resource attributes.", - TypeSpec: pschema.TypeSpec{ - Type: "object", - AdditionalProperties: &pschema.TypeSpec{ - Ref: "pulumi.json#/Any", + "writeOnly": { + Description: "Property names as defined by `writeOnlyProperties` in the CloudFormation schema. Write-only properties are not returned during read operations and have to be included in all update operations as CloudControl itself can't read their previous values.\nIn the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`).", + TypeSpec: pschema.TypeSpec{ + Type: "array", + Items: &pschema.TypeSpec{ + Type: "string", + }, + }, + }, + "tagsProperty": { + Description: "Optional name of the property containing the tags. Defaults to \"Tags\" if the `tagsStyle` is set to either \"stringMap\" or \"keyValueArray\". This is used to apply default tags to the resource and can be ignored if not using default tags.", + TypeSpec: pschema.TypeSpec{ + Type: "string", + }, + }, + "tagsStyle": { + Description: "Optional style of tags this resource uses. Valid values are \"stringMap\", \"keyValueArray\" or \"none\". Defaults to `keyValueArray` if `tagsProperty` is set. This is used to apply default tags to the resource and can be ignored if not using default tags.", + TypeSpec: pschema.TypeSpec{ + Type: "string", + }, }, }, + RequiredInputs: []string{"type", "properties"}, } - return properties } func (r *ExtensionResourceInputs) toOutputs(resourceState map[string]interface{}) map[string]any { diff --git a/provider/pkg/resources/patching.go b/provider/pkg/resources/patching.go new file mode 100644 index 0000000000..4649bf6b74 --- /dev/null +++ b/provider/pkg/resources/patching.go @@ -0,0 +1,85 @@ +package resources + +import ( + "fmt" + "slices" + "strings" + + "github.com/mattbaird/jsonpatch" + "github.com/pulumi/pulumi-aws-native/provider/pkg/metadata" + "github.com/pulumi/pulumi-aws-native/provider/pkg/naming" + "github.com/pulumi/pulumi/pkg/v3/codegen" + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" + "github.com/wI2L/jsondiff" +) + +func CalcPatch(oldInputs resource.PropertyMap, newInputs resource.PropertyMap, spec metadata.CloudAPIResource, types map[string]metadata.CloudAPIType) ([]jsonpatch.JsonPatchOperation, error) { + diff := oldInputs.Diff(newInputs) + + // Write-only properties can't even be read internally within the CloudControl service so they must be included in + // patch requests as adds to ensure the updated model validates. + // If a property is both write-only and create-only, we should not include it in the patch request + // because create-only properties can't be updated and even doing an add of the same value is rejected. + createOnlyProps := codegen.NewStringSet(spec.CreateOnly...) + writeOnlyProps := codegen.NewStringSet(spec.WriteOnly...) + mustSendProps := writeOnlyProps.Subtract(createOnlyProps) + for _, writeOnlyPropName := range mustSendProps.SortedValues() { + propKey := resource.PropertyKey(writeOnlyPropName) + if _, ok := diff.Sames[propKey]; ok { + delete(diff.Sames, propKey) + diff.Adds[propKey] = newInputs[propKey] + } + } + + return naming.DiffToPatch(&spec, types, diff) +} + +func CalculateUntypedPatch(typedOldInputs ExtensionResourceInputs, typedInputs ExtensionResourceInputs) ([]jsonpatch.JsonPatchOperation, error) { + jsonDiffPatch, err := jsondiff.Compare(typedOldInputs.Properties, typedInputs.Properties) + if err != nil { + return nil, fmt.Errorf("failed to compare properties: %w", err) + } + + // Write-only properties can't even be read internally within the CloudControl service so they must be included in + // patch requests as adds to ensure the updated model validates. + // If a property is both write-only and create-only, we should not include it in the patch request + // because create-only properties can't be updated and even doing an add of the same value is rejected. + createOnlyProps := codegen.NewStringSet(typedInputs.CreateOnly...) + for _, writeOnlyPropName := range typedInputs.WriteOnly { + if createOnlyProps.Has(writeOnlyPropName) { + continue + } + newValue, ok := typedInputs.Properties[writeOnlyPropName] + if !ok { + continue + } + propPath := "/" + writeOnlyPropName + hasPatch := false + for _, op := range jsonDiffPatch { + if op.Path == propPath { + hasPatch = true + break + } + } + if !hasPatch { + jsonDiffPatch = append(jsonDiffPatch, jsondiff.Operation{ + Type: "add", + Path: propPath, + Value: newValue, + }) + } + } + + jsonPatch := make([]jsonpatch.JsonPatchOperation, 0, len(jsonDiffPatch)) + for _, op := range jsonDiffPatch { + jsonPatch = append(jsonPatch, jsonpatch.JsonPatchOperation{ + Operation: op.Type, + Path: op.Path, + Value: op.Value, + }) + } + slices.SortStableFunc(jsonPatch, func(a, b jsonpatch.JsonPatchOperation) int { + return strings.Compare(a.Path, b.Path) + }) + return jsonPatch, nil +} diff --git a/provider/pkg/resources/patching_test.go b/provider/pkg/resources/patching_test.go new file mode 100644 index 0000000000..a5f5bcdd47 --- /dev/null +++ b/provider/pkg/resources/patching_test.go @@ -0,0 +1,114 @@ +package resources + +import ( + "testing" + + "github.com/mattbaird/jsonpatch" + "github.com/pulumi/pulumi-aws-native/provider/pkg/metadata" + "github.com/pulumi/pulumi-aws-native/provider/pkg/naming" + "github.com/pulumi/pulumi/pkg/v3/codegen/schema" + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" + "github.com/pulumi/pulumi/sdk/v3/go/common/slice" + "github.com/stretchr/testify/assert" +) + +func TestCalcPatch(t *testing.T) { + type args struct { + oldInputs resource.PropertyMap + newInputs resource.PropertyMap + spec metadata.CloudAPIResource + types map[string]metadata.CloudAPIType + } + type implementation func(t *testing.T, args args) []jsonpatch.JsonPatchOperation + typed := func(t *testing.T, args args) []jsonpatch.JsonPatchOperation { + t.Helper() + patch, err := CalcPatch(args.oldInputs, args.newInputs, args.spec, args.types) + assert.NoError(t, err) + return patch + } + untyped := func(t *testing.T, args args) []jsonpatch.JsonPatchOperation { + t.Helper() + // Convert all keys to upper camel case to match the behavior of the SdkToCfn function. + keysToUpperCamel := func(s string) (string, bool) { + return naming.ToUpperCamel(s), true + } + patch, err := CalculateUntypedPatch(ExtensionResourceInputs{ + Properties: args.oldInputs.MapRepl(keysToUpperCamel, nil), + }, ExtensionResourceInputs{ + Properties: args.newInputs.MapRepl(keysToUpperCamel, nil), + CreateOnly: slice.Map(args.spec.CreateOnly, naming.ToUpperCamel), + WriteOnly: slice.Map(args.spec.WriteOnly, naming.ToUpperCamel), + }) + assert.NoError(t, err) + return patch + } + + for name, run := range map[string]implementation{ + "Typed": typed, + "Untyped": untyped, + } { + t.Run(name, func(t *testing.T) { + t.Run("no diff", func(t *testing.T) { + patch := run(t, args{ + oldInputs: resource.PropertyMap{ + "prop1": resource.NewStringProperty("value1"), + }, + newInputs: resource.PropertyMap{ + "prop1": resource.NewStringProperty("value1"), + }, + spec: metadata.CloudAPIResource{ + Inputs: map[string]schema.PropertySpec{ + "prop1": {TypeSpec: schema.TypeSpec{Type: "string"}}, + }, + }}) + assert.Empty(t, patch) + }) + t.Run("always sends write-only properties", func(t *testing.T) { + expected := []jsonpatch.JsonPatchOperation{ + {Operation: "add", Path: "/Prop1", Value: "1"}, + {Operation: "replace", Path: "/Prop2", Value: "2b"}, + } + patch := run(t, args{ + oldInputs: resource.PropertyMap{ + "prop1": resource.NewStringProperty("1"), + "prop2": resource.NewStringProperty("2a"), + }, + newInputs: resource.PropertyMap{ + "prop1": resource.NewStringProperty("1"), + "prop2": resource.NewStringProperty("2b"), + }, + spec: metadata.CloudAPIResource{ + Inputs: map[string]schema.PropertySpec{ + "prop1": {TypeSpec: schema.TypeSpec{Type: "string"}}, + "prop2": {TypeSpec: schema.TypeSpec{Type: "string"}}, + }, + WriteOnly: []string{"prop1"}, + }}) + assert.Equal(t, expected, patch) + }) + t.Run("don't send write-only, create-only properties", func(t *testing.T) { + expected := []jsonpatch.JsonPatchOperation{ + {Operation: "replace", Path: "/Prop2", Value: "2b"}, + } + patch := run(t, args{ + oldInputs: resource.PropertyMap{ + "prop1": resource.NewStringProperty("1"), + "prop2": resource.NewStringProperty("2a"), + }, + newInputs: resource.PropertyMap{ + "prop1": resource.NewStringProperty("1"), + "prop2": resource.NewStringProperty("2b"), + }, + spec: metadata.CloudAPIResource{ + Inputs: map[string]schema.PropertySpec{ + "prop1": {TypeSpec: schema.TypeSpec{Type: "string"}}, + "prop2": {TypeSpec: schema.TypeSpec{Type: "string"}}, + }, + WriteOnly: []string{"prop1"}, + CreateOnly: []string{"prop1"}, + }}) + assert.Equal(t, expected, patch) + }) + }) + } +} diff --git a/sdk/dotnet/ExtensionResource.cs b/sdk/dotnet/ExtensionResource.cs index 88e49919b1..b49fcd6599 100644 --- a/sdk/dotnet/ExtensionResource.cs +++ b/sdk/dotnet/ExtensionResource.cs @@ -66,6 +66,19 @@ public static ExtensionResource Get(string name, Input id, CustomResourc public sealed class ExtensionResourceArgs : global::Pulumi.ResourceArgs { + [Input("createOnly")] + private InputList? _createOnly; + + /// + /// Property names as defined by `createOnlyProperties` in the CloudFormation schema. Create-only properties can't be set during updates, so will not be included in patches even if they are also marked as write-only, and will cause an error if attempted to be updated. Therefore any property here should also be included in the `replaceOnChanges` resource option too. + /// In the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`). + /// + public InputList CreateOnly + { + get => _createOnly ?? (_createOnly = new InputList()); + set => _createOnly = value; + } + [Input("properties", required: true)] private InputMap? _properties; diff --git a/sdk/go/aws/extensionResource.go b/sdk/go/aws/extensionResource.go index cd0f3369f0..279df8857d 100644 --- a/sdk/go/aws/extensionResource.go +++ b/sdk/go/aws/extensionResource.go @@ -66,6 +66,9 @@ func (ExtensionResourceState) ElementType() reflect.Type { } type extensionResourceArgs struct { + // Property names as defined by `createOnlyProperties` in the CloudFormation schema. Create-only properties can't be set during updates, so will not be included in patches even if they are also marked as write-only, and will cause an error if attempted to be updated. Therefore any property here should also be included in the `replaceOnChanges` resource option too. + // In the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`). + CreateOnly []string `pulumi:"createOnly"` // Property bag containing the properties for the resource. These should be defined using the casing expected by the CloudControl API as these values are sent exact as provided. Properties map[string]interface{} `pulumi:"properties"` // Optional name of the property containing the tags. Defaults to "Tags" if the `tagsStyle` is set to either "stringMap" or "keyValueArray". This is used to apply default tags to the resource and can be ignored if not using default tags. @@ -81,6 +84,9 @@ type extensionResourceArgs struct { // The set of arguments for constructing a ExtensionResource resource. type ExtensionResourceArgs struct { + // Property names as defined by `createOnlyProperties` in the CloudFormation schema. Create-only properties can't be set during updates, so will not be included in patches even if they are also marked as write-only, and will cause an error if attempted to be updated. Therefore any property here should also be included in the `replaceOnChanges` resource option too. + // In the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`). + CreateOnly pulumi.StringArrayInput // Property bag containing the properties for the resource. These should be defined using the casing expected by the CloudControl API as these values are sent exact as provided. Properties pulumi.MapInput // Optional name of the property containing the tags. Defaults to "Tags" if the `tagsStyle` is set to either "stringMap" or "keyValueArray". This is used to apply default tags to the resource and can be ignored if not using default tags. diff --git a/sdk/nodejs/extensionResource.ts b/sdk/nodejs/extensionResource.ts index 293467417d..ee0b67f40c 100644 --- a/sdk/nodejs/extensionResource.ts +++ b/sdk/nodejs/extensionResource.ts @@ -56,6 +56,7 @@ export class ExtensionResource extends pulumi.CustomResource { if ((!args || args.type === undefined) && !opts.urn) { throw new Error("Missing required property 'type'"); } + resourceInputs["createOnly"] = args ? args.createOnly : undefined; resourceInputs["properties"] = args ? args.properties : undefined; resourceInputs["tagsProperty"] = args ? args.tagsProperty : undefined; resourceInputs["tagsStyle"] = args ? args.tagsStyle : undefined; @@ -74,6 +75,11 @@ export class ExtensionResource extends pulumi.CustomResource { * The set of arguments for constructing a ExtensionResource resource. */ export interface ExtensionResourceArgs { + /** + * Property names as defined by `createOnlyProperties` in the CloudFormation schema. Create-only properties can't be set during updates, so will not be included in patches even if they are also marked as write-only, and will cause an error if attempted to be updated. Therefore any property here should also be included in the `replaceOnChanges` resource option too. + * In the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`). + */ + createOnly?: pulumi.Input[]>; /** * Property bag containing the properties for the resource. These should be defined using the casing expected by the CloudControl API as these values are sent exact as provided. */ diff --git a/sdk/python/pulumi_aws_native/extension_resource.py b/sdk/python/pulumi_aws_native/extension_resource.py index 901c80ab94..761aa717c6 100644 --- a/sdk/python/pulumi_aws_native/extension_resource.py +++ b/sdk/python/pulumi_aws_native/extension_resource.py @@ -16,6 +16,7 @@ class ExtensionResourceArgs: def __init__(__self__, *, properties: pulumi.Input[Mapping[str, Any]], type: pulumi.Input[str], + create_only: Optional[pulumi.Input[Sequence[pulumi.Input[str]]]] = None, tags_property: Optional[pulumi.Input[str]] = None, tags_style: Optional[pulumi.Input[str]] = None, write_only: Optional[pulumi.Input[Sequence[pulumi.Input[str]]]] = None): @@ -23,6 +24,8 @@ def __init__(__self__, *, The set of arguments for constructing a ExtensionResource resource. :param pulumi.Input[Mapping[str, Any]] properties: Property bag containing the properties for the resource. These should be defined using the casing expected by the CloudControl API as these values are sent exact as provided. :param pulumi.Input[str] type: CloudFormation type name. This has three parts, each separated by two colons. For AWS resources this starts with `AWS::` e.g. `AWS::Logs::LogGroup`. Third party resources should use a namespace prefix e.g. `MyCompany::MyService::MyResource`. + :param pulumi.Input[Sequence[pulumi.Input[str]]] create_only: Property names as defined by `createOnlyProperties` in the CloudFormation schema. Create-only properties can't be set during updates, so will not be included in patches even if they are also marked as write-only, and will cause an error if attempted to be updated. Therefore any property here should also be included in the `replaceOnChanges` resource option too. + In the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`). :param pulumi.Input[str] tags_property: Optional name of the property containing the tags. Defaults to "Tags" if the `tagsStyle` is set to either "stringMap" or "keyValueArray". This is used to apply default tags to the resource and can be ignored if not using default tags. :param pulumi.Input[str] tags_style: Optional style of tags this resource uses. Valid values are "stringMap", "keyValueArray" or "none". Defaults to `keyValueArray` if `tagsProperty` is set. This is used to apply default tags to the resource and can be ignored if not using default tags. :param pulumi.Input[Sequence[pulumi.Input[str]]] write_only: Property names as defined by `writeOnlyProperties` in the CloudFormation schema. Write-only properties are not returned during read operations and have to be included in all update operations as CloudControl itself can't read their previous values. @@ -30,6 +33,8 @@ def __init__(__self__, *, """ pulumi.set(__self__, "properties", properties) pulumi.set(__self__, "type", type) + if create_only is not None: + pulumi.set(__self__, "create_only", create_only) if tags_property is not None: pulumi.set(__self__, "tags_property", tags_property) if tags_style is not None: @@ -61,6 +66,19 @@ def type(self) -> pulumi.Input[str]: def type(self, value: pulumi.Input[str]): pulumi.set(self, "type", value) + @property + @pulumi.getter(name="createOnly") + def create_only(self) -> Optional[pulumi.Input[Sequence[pulumi.Input[str]]]]: + """ + Property names as defined by `createOnlyProperties` in the CloudFormation schema. Create-only properties can't be set during updates, so will not be included in patches even if they are also marked as write-only, and will cause an error if attempted to be updated. Therefore any property here should also be included in the `replaceOnChanges` resource option too. + In the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`). + """ + return pulumi.get(self, "create_only") + + @create_only.setter + def create_only(self, value: Optional[pulumi.Input[Sequence[pulumi.Input[str]]]]): + pulumi.set(self, "create_only", value) + @property @pulumi.getter(name="tagsProperty") def tags_property(self) -> Optional[pulumi.Input[str]]: @@ -104,6 +122,7 @@ class ExtensionResource(pulumi.CustomResource): def __init__(__self__, resource_name: str, opts: Optional[pulumi.ResourceOptions] = None, + create_only: Optional[pulumi.Input[Sequence[pulumi.Input[str]]]] = None, properties: Optional[pulumi.Input[Mapping[str, Any]]] = None, tags_property: Optional[pulumi.Input[str]] = None, tags_style: Optional[pulumi.Input[str]] = None, @@ -115,6 +134,8 @@ def __init__(__self__, :param str resource_name: The name of the resource. :param pulumi.ResourceOptions opts: Options for the resource. + :param pulumi.Input[Sequence[pulumi.Input[str]]] create_only: Property names as defined by `createOnlyProperties` in the CloudFormation schema. Create-only properties can't be set during updates, so will not be included in patches even if they are also marked as write-only, and will cause an error if attempted to be updated. Therefore any property here should also be included in the `replaceOnChanges` resource option too. + In the CloudFormation schema these are fully qualified property paths (e.g. `/properties/AccessToken`) whereas here we only include the top-level property name (e.g. `AccessToken`). :param pulumi.Input[Mapping[str, Any]] properties: Property bag containing the properties for the resource. These should be defined using the casing expected by the CloudControl API as these values are sent exact as provided. :param pulumi.Input[str] tags_property: Optional name of the property containing the tags. Defaults to "Tags" if the `tagsStyle` is set to either "stringMap" or "keyValueArray". This is used to apply default tags to the resource and can be ignored if not using default tags. :param pulumi.Input[str] tags_style: Optional style of tags this resource uses. Valid values are "stringMap", "keyValueArray" or "none". Defaults to `keyValueArray` if `tagsProperty` is set. This is used to apply default tags to the resource and can be ignored if not using default tags. @@ -146,6 +167,7 @@ def __init__(__self__, resource_name: str, *args, **kwargs): def _internal_init(__self__, resource_name: str, opts: Optional[pulumi.ResourceOptions] = None, + create_only: Optional[pulumi.Input[Sequence[pulumi.Input[str]]]] = None, properties: Optional[pulumi.Input[Mapping[str, Any]]] = None, tags_property: Optional[pulumi.Input[str]] = None, tags_style: Optional[pulumi.Input[str]] = None, @@ -160,6 +182,7 @@ def _internal_init(__self__, raise TypeError('__props__ is only valid when passed in combination with a valid opts.id to get an existing resource') __props__ = ExtensionResourceArgs.__new__(ExtensionResourceArgs) + __props__.__dict__["create_only"] = create_only if properties is None and not opts.urn: raise TypeError("Missing required property 'properties'") __props__.__dict__["properties"] = properties