Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
danielrbradley committed Mar 22, 2024
1 parent db38e34 commit ec8345a
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 10 deletions.
9 changes: 5 additions & 4 deletions provider/pkg/naming/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ package naming
import (
"fmt"
"reflect"
"slices"
"strings"

"github.com/mattbaird/jsonpatch"
"github.com/pulumi/pulumi-aws-native/provider/pkg/metadata"
"github.com/pulumi/pulumi-go-provider/resourcex"
"github.com/pulumi/pulumi/pkg/v3/codegen"
pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
)
Expand Down Expand Up @@ -180,11 +180,12 @@ func (c *sdkToCfnConverter) sdkObjectValueToCfn(typeName string, spec metadata.C

func (c *sdkToCfnConverter) diffToPatch(diff *resource.ObjectDiff) ([]jsonpatch.JsonPatchOperation, error) {
var ops []jsonpatch.JsonPatchOperation
inputKeys := codegen.NewStringSet()
sortedKeys := make([]string, 0, len(c.spec.Inputs))
for sdkName := range c.spec.Inputs {
inputKeys.Add(string(sdkName))
sortedKeys = append(sortedKeys, string(sdkName))
}
for _, sdkName := range inputKeys.SortedValues() {
slices.Sort(sortedKeys)
for _, sdkName := range sortedKeys {
prop := c.spec.Inputs[sdkName]
cfnName := ToCfnName(sdkName, c.spec.IrreversibleNames)
key := resource.PropertyKey(sdkName)
Expand Down
3 changes: 0 additions & 3 deletions provider/pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -1074,9 +1074,6 @@ func (p *cfnProvider) Update(ctx context.Context, req *pulumirpc.UpdateRequest)
return nil, errors.Errorf("Resource type %s not found", resourceToken)
}

// 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.
ops, err := resources.CalcPatch(oldInputs, newInputs, spec, p.resourceMap.Types)
if err != nil {
return nil, err
Expand Down
2 changes: 0 additions & 2 deletions provider/pkg/resources/extension_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,6 @@ 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")
}

// 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.
jsonPatch, err := CalculateUntypedPatch(typedOldInputs, typedInputs)
if err != nil {
return nil, fmt.Errorf("failed to calculate patch: %w", err)
Expand Down
7 changes: 6 additions & 1 deletion provider/pkg/resources/patching.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ func CalcPatch(oldInputs resource.PropertyMap, newInputs resource.PropertyMap, s

// 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.
// 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)
Expand All @@ -39,6 +40,10 @@ func CalculateUntypedPatch(typedOldInputs ExtensionResourceInputs, typedInputs E
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) {
Expand Down

0 comments on commit ec8345a

Please sign in to comment.