Skip to content

Commit

Permalink
Fixup JSON Patch -- writeOnlyProperties can only be updated using 'add'.
Browse files Browse the repository at this point in the history
  • Loading branch information
ewbankkit committed May 16, 2024
1 parent 20266d8 commit 424d70a
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 19 deletions.
47 changes: 29 additions & 18 deletions internal/generic/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/json"
"fmt"
"log"
"strings"
"time"

Expand Down Expand Up @@ -111,21 +112,18 @@ func resourceIsImmutableType(v bool) ResourceOptionsFunc {
// the previous calls' values.
func resourceWithWriteOnlyPropertyPaths(v []string) ResourceOptionsFunc {
return func(o *genericResource) error {
writeOnlyAttributePaths := make([]*path.Path, 0)

for _, writeOnlyPropertyPath := range v {
writeOnlyAttributePath, err := o.propertyPathToAttributePath(writeOnlyPropertyPath)
writeOnlyAttributePath, patchPath, err := o.propertyPathToAttributePath(writeOnlyPropertyPath)

if err != nil {
// return fmt.Errorf("creating write-only attribute path (%s): %w", writeOnlyPropertyPath, err)
continue
}

writeOnlyAttributePaths = append(writeOnlyAttributePaths, writeOnlyAttributePath)
o.writeOnlyAttributePaths = append(o.writeOnlyAttributePaths, writeOnlyAttributePath)
o.writeOnlyPatchPaths = append(o.writeOnlyPatchPaths, patchPath)
}

o.writeOnlyAttributePaths = writeOnlyAttributePaths

return nil
}
}
Expand Down Expand Up @@ -311,6 +309,7 @@ type genericResource struct {
cfToTfNameMap map[string]string // Map of CloudFormation property name to Terraform attribute name
isImmutableType bool // Resources cannot be updated and must be recreated
writeOnlyAttributePaths []*path.Path // Paths to any write-only attributes
writeOnlyPatchPaths []string // Paths to any write-only properties
createTimeout time.Duration // Maximum wait time for resource creation
updateTimeout time.Duration // Maximum wait time for resource update
deleteTimeout time.Duration // Maximum wait time for resource deletion
Expand Down Expand Up @@ -539,7 +538,7 @@ func (r *genericResource) Update(ctx context.Context, request resource.UpdateReq
return
}

patchDocument, err := patchDocument(currentDesiredState, plannedDesiredState)
patchDocument, err := r.patchDocument(currentDesiredState, plannedDesiredState)

if err != nil {
response.Diagnostics.AddError(
Expand Down Expand Up @@ -745,14 +744,26 @@ func (r *genericResource) bootstrapContext(ctx context.Context) context.Context
}

// patchDocument returns a JSON Patch document describing the difference between `old` and `new`.
func patchDocument(old, new string) (string, error) {
patch, err := jsonpatch.CreatePatch([]byte(old), []byte(new))
func (r *genericResource) patchDocument(old, new string) (string, error) {
patches, err := jsonpatch.CreatePatch([]byte(old), []byte(new))

if err != nil {
return "", err
}

b, err := json.Marshal(patch)
// writeOnlyProperties can only be updated using 'add'.
for i, patch := range patches {
log.Printf("[INFO] patch.Path: %s, patch.Operation: %s", patch.Path, patch.Operation)
for _, writeOnlyPatchPath := range r.writeOnlyPatchPaths {
log.Printf("[INFO] writeOnlyPatchPath: %s", writeOnlyPatchPath)
if path, op := patch.Path, patch.Operation; (path == writeOnlyPatchPath || strings.HasPrefix(path, writeOnlyPatchPath+"/")) && op == "replace" {
log.Printf("[INFO] Fixup: %s", path)
patches[i].Operation = "add"
}
}
}

b, err := json.Marshal(patches)

if err != nil {
return "", err
Expand All @@ -761,37 +772,37 @@ func patchDocument(old, new string) (string, error) {
return string(b), nil
}

// propertyPathToAttributePath returns the AttributePath for the specified JSON Pointer property path.
func (r *genericResource) propertyPathToAttributePath(propertyPath string) (*path.Path, error) {
// propertyPathToAttributePath returns the AttributePath and JSON Patch path for the specified JSON Pointer property path.
func (r *genericResource) propertyPathToAttributePath(propertyPath string) (*path.Path, string, error) {
segments := strings.Split(propertyPath, "/")

if got, expected := len(segments), 3; got < expected {
return nil, fmt.Errorf("expected at least %d property path segments, got: %d", expected, got)
return nil, "", fmt.Errorf("expected at least %d property path segments, got: %d", expected, got)
}

if got, expected := segments[0], ""; got != expected {
return nil, fmt.Errorf("expected %q for the initial property path segment, got: %q", expected, got)
return nil, "", fmt.Errorf("expected %q for the initial property path segment, got: %q", expected, got)
}

if got, expected := segments[1], "properties"; got != expected {
return nil, fmt.Errorf("expected %q for the second property path segment, got: %q", expected, got)
return nil, "", fmt.Errorf("expected %q for the second property path segment, got: %q", expected, got)
}

attributePath := path.Empty()

for _, segment := range segments[2:] {
switch segment {
case "", "*":
return nil, fmt.Errorf("invalid property path segment: %q", segment)
return nil, "", fmt.Errorf("invalid property path segment: %q", segment)

default:
attributeName, ok := r.cfToTfNameMap[segment]
if !ok {
return nil, fmt.Errorf("attribute name mapping not found: %s", segment)
return nil, "", fmt.Errorf("attribute name mapping not found: %s", segment)
}
attributePath = attributePath.AtName(attributeName)
}
}

return &attributePath, nil
return &attributePath, strings.TrimPrefix(propertyPath, "/properties"), nil
}
2 changes: 1 addition & 1 deletion internal/generic/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestPropertyPathToAttributePath(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.TestName, func(t *testing.T) {
attributePath, err := rt.propertyPathToAttributePath(testCase.PropertyPath)
attributePath, _, err := rt.propertyPathToAttributePath(testCase.PropertyPath)

if err == nil && testCase.ExpectedError {
t.Fatalf("expected error from propertyPathToAttributePath")
Expand Down

0 comments on commit 424d70a

Please sign in to comment.