Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON Patch remove operations should consider and predict the item location on the list #2075

Open
wellsiau-aws opened this issue Oct 24, 2024 · 5 comments

Comments

@wellsiau-aws
Copy link
Collaborator

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
  • The resources and data sources in this provider are generated from the CloudFormation schema, so they can only support the actions that the underlying schema supports. For this reason submitted bugs should be limited to defects in the generation and runtime code of the provider. Customizing behavior of the resource, or noting a gap in behavior are not valid bugs and should be submitted as enhancements to AWS via the CloudFormation Open Coverage Roadmap.

Description

AWSCC uses JSON patch to create delta changes between old and new state.

In the event when the new state will remove certain items for Attribute list, then it will produce multiple patch document such as:

## in this example, the patch will remove Tags on index 0, 4 and 6
[
  {"op":"remove","path":"/Tags/0"},
  {"op":"remove","path":"/Tags/4"},
  {"op":"remove","path":"/Tags/6"}
]

In the AWS Cloud Control API (CCAPI), the patch operations is applied sequentially in the order that they appear in the patch document. Ref

As such, the patch document sent by the AWSCC provider to the CCAPI should be aware of relative positions of each item after subsequent patch operations.

Now imagine a situation where we have resource as follow

Resource before changes

resource "awscc_s3_bucket" "example" {
  bucket_name = "exmaple-bucket-202410231857"
  tags = [
    {
    key   = "Name"
    value = "My bucket"
    },
    {
      key   = "Environment"
      value = "Dev"
    },
    {
      key   = "Modified By"
      value = "AWSCC"
    },
    {
      key   = "Department"
      value = "Engineering"
    },
    {
      key   = "Project"
      value = "wellsiau"
    },
    {
      key   = "Owner"
      value = "wellsiau"
    },
    {
      key   = "Managed By"
      value = "Terraform"
    }
  ]
}

Resource after changes

And let's assume we will remove / comment the first 3 tags

resource "awscc_s3_bucket" "example" {
  bucket_name = "exmaple-bucket-202410231857"
  tags = [
    # {
    # key   = "Name"
    # value = "My bucket"
    # },
    # {
    #   key   = "Environment"
    #   value = "Dev"
    # },
    # {
    #   key   = "Modified By"
    #   value = "AWSCC"
    # },
    {
      key   = "Department"
      value = "Engineering"
    },
    {
      key   = "Project"
      value = "wellsiau"
    },
    {
      key   = "Owner"
      value = "wellsiau"
    },
    {
      key   = "Managed By"
      value = "Terraform"
    }
  ]
}

State file before changes

And assume the current state file as follows (redacted for brevity). Noticed that the order of the tags are a bit different from the HCL.

      "instances": [
        {
          "schema_version": 1,
          "attributes": {
            "bucket_name": "exmaple-bucket-202410231857",
            . . .
            "tags": [
              {
                "key": "Modified By",
                "value": "AWSCC"
              },
              {
                "key": "Project",
                "value": "wellsiau"
              },
              {
                "key": "Department",
                "value": "Engineering"
              },
              {
                "key": "Owner",
                "value": "wellsiau"
              },
              {
                "key": "Environment",
                "value": "Dev"
              },
              {
                "key": "Managed By",
                "value": "Terraform"
              },
              {
                "key": "Name",
                "value": "My bucket"
              }
            ],
            "versioning_configuration": null,
            "website_configuration": null,
            "website_url": "http://exmaple-bucket-202410231857.s3-website-us-east-1.amazonaws.com"
          },
          "sensitive_attributes": []
        }
      ]
    }
  ],

Order of patch operations

Using the earlier example of the patch order:

[
  {"op":"remove","path":"/Tags/0"},
  {"op":"remove","path":"/Tags/4"},
  {"op":"remove","path":"/Tags/6"}
]
  1. After the first patch operation completed ( {"op":"remove","path":"/Tags/0"} ) there are only 5 Tags remaining in the list.
  2. After the second patch operations completed ( {"op":"remove","path":"/Tags/4"} ), there are only 4 tags remaining in the list.
  3. The third patch operations will fail due to index out of bounds ( {"op":"remove","path":"/Tags/6"})

Example error logs:

awscc_s3_bucket.example: Modifying... [id=exmaple-bucket-202410231857]
╷
│ Error: AWS SDK Go Service Operation Unsuccessful
│ 
│   with awscc_s3_bucket.example,
│   on main.tf line 1, in resource "awscc_s3_bucket" "example":
│    1: resource "awscc_s3_bucket" "example" {
│ 
│ Calling Cloud Control API service UpdateResource operation returned: operation error CloudControl: UpdateResource, https response error StatusCode: 400, RequestID:
│ 87fdad5d-54d2-4890-bc39-8772b8b7d696, api error ValidationException: index Out of bound, index is greater than 4

New or Affected Resource(s)

  • All resources

References

  • #0000
@wellsiau-aws
Copy link
Collaborator Author

The patchDocument uses JSON Marshal which I thought might tried to order the list.

b, err := json.Marshal(patch)

https://pkg.go.dev/encoding/json#Marshal

Map values encode as JSON objects. The map's key type must either be a string, an integer type, or implement encoding.TextMarshaler. The map keys are sorted and used as JSON object keys by applying the following rules, subject to the UTF-8 coercion described for string values above:

@wellsiau-aws
Copy link
Collaborator Author

However even the raw patches are also sorted

patch, err := jsonpatch.CreatePatch([]byte(old), []byte(new))

"Raw patch: [
  {Operation:remove Path:/Tags/1 Value:<nil>} 
  {Operation:remove Path:/Tags/5 Value:<nil>} 
  {Operation:remove Path:/Tags/6 Value:<nil>}
]"

@wellsiau-aws
Copy link
Collaborator Author

Related to mattbaird/jsonpatch#30

@ottosanchez
Copy link

ottosanchez commented Oct 25, 2024

From the patch remove examples above and from testing, it seems that AWSCC lexically sorts the remove operations in the patch document by index. The sorting order needs to be reversed so that AWSCC API can apply the remove operations properly, and according to the RFC6902 standard.

AWSCC organizes operations in the patch document by remove and add operations (in that order) to add/replace/remove elements in the resource array.

Example:
Original resource configuration:

resource "awscc_s3_bucket" "test_bucket" {
  bucket_name = "<test-bucket>"
  tags = [
    {
      key   = "Name"
      value = "My bucket"
    },
    {
      key   = "Environment"
      value = "Dev"
    },
    {
      key   = "Modified By"
      value = "AWSCC"
    },
    {
      key   = "Department"
      value = "Engineering"
    },
    {
      key   = "Project"
      value = "ottsan"
    },
    {
      key   = "Owner"
      value = "ottsan"
    },
  ]

Desired resource configuration:

resource "awscc_s3_bucket" "test_bucket" {
  bucket_name = "<test-bucket>"

  tags = [
    { 
      key   = "Name"
      value = "My bucket"
    },
    {
      key   = "Environment"
      value = "Dev1" # updated
    },
    {
      key   = "Modified By"
      value = "AWSCC1" # updated
    },
    # removed
    # {
    #   key   = "Department"
    #   value = "Engineering"
    # },
    # {
    #   key   = "Project"
    #   value = "ottsan"
    # },
    # {
    #   key   = "Owner"
    #   value = "ottsan"
    # },
    { # added
      key   = "Cost Center"
      value = "12345"
    },
    { # added
      key   = "Data Classification"
      value = "Confidential"
    },
  ]

}

AWSCC patch document (remove, then add ops, lexically sorted by tag index):

"PatchDocument": "[{\"op\":\"remove\",\"path\":\"/Tags/0\"},{\"op\":\"remove\",\"path\":\"/Tags/1\"},{\"op\":\"remove\",\"path\":\"/Tags/2\"},{\"op\":\"remove\",\"path\":\"/Tags/3\"},{\"op\":\"remove\",\"path\":\"/Tags/4\"},{\"op\":\"add\",\"path\":\"/Tags/1\",\"value\":{\"Key\":\"Environment\",\"Value\":\"Dev1\"}},{\"op\":\"add\",\"path\":\"/Tags/2\",\"value\":{\"Key\":\"Modified By\",\"Value\":\"AWSCC1\"}},{\"op\":\"add\",\"path\":\"/Tags/3\",\"value\":{\"Key\":\"Cost Center\",\"Value\":\"12345\"}},{\"op\":\"add\",\"path\":\"/Tags/4\",\"value\":{\"Key\":\"Data Classification\",\"Value\":\"Confidential\"}}]"
Operation result:

│ Error: AWS SDK Go Service Operation Unsuccessful
│ 
│   with awscc_s3_bucket.test_bucket,
│   on main.tf line 40, in resource "awscc_s3_bucket" "test_bucket":
│   40: resource "awscc_s3_bucket" "test_bucket" {
│ 
│ Calling Cloud Control API service UpdateResource operation returned:
│ operation error CloudControl: UpdateResource, https response error
│ StatusCode: 400, RequestID: 3173f9b9-cb4f-4e48-9378-aabbaabbaabb, api error
│ ValidationException: index Out of bound, index is greater than 3

Reverse sorting the patch remove operations and updating the resource manually through the cli yields the desired configuration in the Tags array:

"PatchDocument": "[{\"op\":\"remove\",\"path\":\"/Tags/4\"},{\"op\":\"remove\",\"path\":\"/Tags/3\"},{\"op\":\"remove\",\"path\":\"/Tags/2\"},{\"op\":\"remove\",\"path\":\"/Tags/1\"},{\"op\":\"remove\",\"path\":\"/Tags/0\"},{\"op\":\"add\",\"path\":\"/Tags/1\",\"value\":{\"Key\":\"Environment\",\"Value\":\"Dev1\"}},{\"op\":\"add\",\"path\":\"/Tags/2\",\"value\":{\"Key\":\"Modified By\",\"Value\":\"AWSCC1\"}},{\"op\":\"add\",\"path\":\"/Tags/3\",\"value\":{\"Key\":\"Cost Center\",\"Value\":\"12345\"}},{\"op\":\"add\",\"path\":\"/Tags/4\",\"value\":{\"Key\":\"Data Classification\",\"Value\":\"Confidential\"}}]"

aws cloudcontrol update-resource --cli-input-json file://patchdoc-sorted-1.json --region --output json
{
  "ProgressEvent": {
    "TypeName": "AWS::S3::Bucket",
    "Identifier": "<test-bucket>",
    "RequestToken": "c758772c-14e6-4ae6-ab61-aabbaabbaabb",
    "Operation": "UPDATE",
    "OperationStatus": "IN_PROGRESS",
    "EventTime": "2024-10-25T06:12:55.225000-05:00",
    "ResourceModel": "{\"PublicAccessBlockConfiguration\":{\"RestrictPublicBuckets\":true,\"BlockPublicPolicy\":true,\"BlockPublicAcls\":true,\"IgnorePublicAcls\":true},\"BucketName\":\"<test-bucket>\",\"OwnershipControls\":{\"Rules\":[{\"ObjectOwnership\":\"BucketOwnerEnforced\"}]},\"BucketEncryption\":{\"ServerSideEncryptionConfiguration\":[{\"BucketKeyEnabled\":false,\"ServerSideEncryptionByDefault\":{\"SSEAlgorithm\":\"AES256\"}}]},\"Tags\":[{\"Value\":\"My bucket\",\"Key\":\"Name\"},{\"Value\":\"Dev1\",\"Key\":\"Environment\"},{\"Value\":\"AWSCC1\",\"Key\":\"Modified By\"},{\"Value\":\"12345\",\"Key\":\"Cost Center\"},{\"Value\":\"Confidential\",\"Key\":\"Data Classification\"}]}"
  }
}
aws cloudcontrol get-resource-request-status \
    --request-token c758772c-14e6-4ae6-ab61-aabbaabbaabb --output json
{
  "ProgressEvent": {
    "TypeName": "AWS::S3::Bucket",
    "Identifier": "<test-bucket>",
    "RequestToken": "c758772c-14e6-4ae6-ab61-aabbaabbaabb",
    "Operation": "UPDATE",
    "OperationStatus": "SUCCESS",
    "EventTime": "2024-10-25T06:13:15.763000-05:00"
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants