[Bug]: Role.iam silently removes [] from spec.forProvider fields

mbbush commented 2 weeks ago

Is there an existing issue for this?

[X] I have searched the existing issues

Affected Resource(s)

iam.aws.upbound.io/v1beta1 - Role

Resource MRs required to reproduce the bug

apiVersion: iam.aws.upbound.io/v1beta1
kind: Role
metadata:
  name: role-with-inline-policy
spec:
  forProvider:
    assumeRolePolicy: |
      {
        "Version": "2012-10-17",
        "Statement": [
          {
            "Effect": "Allow",
            "Principal": {
              "Service": "eks.amazonaws.com"
            },
            "Action": "sts:AssumeRole"
          }
        ]
      }
    managedPolicyArns: []
    inlinePolicy:
      - name: "my_inline_policy"
        policy: |
          {
            "Version": "2012-10-17",
            "Statement": [
              {
                "Effect": "Allow",
                "Resource": "*",
                "Action": "ec2:Describe*"
              }
            ]
          }

Steps to Reproduce

Start running kubectl get -o yaml role.iam.aws.upbound.io --watch Apply the manifest to the cluster. I observe the same behavior regardless of whether it is server-side or client-side applied. Observe that while the very first version of the resource contains managedPolicyArns: [] as specified, every subsequent version has the key removed.

Here are the first few observed versions of the resource:

---
apiVersion: iam.aws.upbound.io/v1beta1
kind: Role
metadata:
  annotations:
    meta.upbound.io/example-id: iam/v1beta1/role
  creationTimestamp: "2024-06-12T23:37:16Z"
  generation: 1
  labels:
    testing.upbound.io/example-name: role
  name: role-with-inline-policy
  resourceVersion: "5128"
  uid: f6ff7514-ffd1-44be-ab15-e4b3790f2ca8
spec:
  deletionPolicy: Delete
  forProvider:
    assumeRolePolicy: |
      {
        "Version": "2012-10-17",
        "Statement": [
          {
            "Effect": "Allow",
            "Principal": {
              "Service": "eks.amazonaws.com"
            },
            "Action": "sts:AssumeRole"
          }
        ]
      }
    inlinePolicy:
    - name: my_inline_policy
      policy: |
        {
          "Version": "2012-10-17",
          "Statement": [
            {
              "Effect": "Allow",
              "Resource": "*",
              "Action": "ec2:Describe*"
            }
          ]
        }
    managedPolicyArns: []
  managementPolicies:
  - '*'
  providerConfigRef:
    name: default
---
apiVersion: iam.aws.upbound.io/v1beta1
kind: Role
metadata:
  annotations:
    meta.upbound.io/example-id: iam/v1beta1/role
  creationTimestamp: "2024-06-12T23:37:16Z"
  generation: 2
  labels:
    testing.upbound.io/example-name: role
  name: role-with-inline-policy
  resourceVersion: "5129"
  uid: f6ff7514-ffd1-44be-ab15-e4b3790f2ca8
spec:
  deletionPolicy: Delete
  forProvider:
    assumeRolePolicy: |
      {
        "Version": "2012-10-17",
        "Statement": [
          {
            "Effect": "Allow",
            "Principal": {
              "Service": "eks.amazonaws.com"
            },
            "Action": "sts:AssumeRole"
          }
        ]
      }
    inlinePolicy:
    - name: my_inline_policy
      policy: |
        {
          "Version": "2012-10-17",
          "Statement": [
            {
              "Effect": "Allow",
              "Resource": "*",
              "Action": "ec2:Describe*"
            }
          ]
        }
    tags:
      crossplane-kind: role.iam.aws.upbound.io
      crossplane-name: role-with-inline-policy
      crossplane-providerconfig: default
  initProvider: {}
  managementPolicies:
  - '*'
  providerConfigRef:
    name: default
---
apiVersion: iam.aws.upbound.io/v1beta1
kind: Role
metadata:
  annotations:
    crossplane.io/external-name: role-with-inline-policy
    meta.upbound.io/example-id: iam/v1beta1/role
  creationTimestamp: "2024-06-12T23:37:16Z"
  generation: 2
  labels:
    testing.upbound.io/example-name: role
  name: role-with-inline-policy
  resourceVersion: "5130"
  uid: f6ff7514-ffd1-44be-ab15-e4b3790f2ca8
spec:
  deletionPolicy: Delete
  forProvider:
    assumeRolePolicy: |
      {
        "Version": "2012-10-17",
        "Statement": [
          {
            "Effect": "Allow",
            "Principal": {
              "Service": "eks.amazonaws.com"
            },
            "Action": "sts:AssumeRole"
          }
        ]
      }
    inlinePolicy:
    - name: my_inline_policy
      policy: |
        {
          "Version": "2012-10-17",
          "Statement": [
            {
              "Effect": "Allow",
              "Resource": "*",
              "Action": "ec2:Describe*"
            }
          ]
        }
    tags:
      crossplane-kind: role.iam.aws.upbound.io
      crossplane-name: role-with-inline-policy
      crossplane-providerconfig: default
  initProvider: {}
  managementPolicies:
  - '*'
  providerConfigRef:
    name: default
---

What happened?

The managedPolicyArns field in the iam Role resource is a bit unusual. Its description says that setting an empty array will result in disassociating any policies attached out-of-band, while setting it to null will simply ignore attached policies.

I would like to use crossplane to enforce that additional policies are not attached to a certain IAM role, but it seems that the provider (or some other part of the reconciliation machinery) is removing the explicitly set [] value, so I can't actually get the managed resource to have the state it would need to enforce no additional policies.

Relevant Error Output Snippet

No response

Crossplane Version

1.14

Provider Version

1.6.0

Kubernetes Version

v1.30.0

Kubernetes Distribution

kind

Additional Info

No response

diranged commented 2 weeks ago

We're seeing the exact same behavior... thanks for opening this bug.

mbbush commented 2 weeks ago

The issue is caused by the omitempty tag in the type definition. This is generated code, and as far as I can tell upjet does not expose the ability to configure it, instead just always adding omitempty to every Field it creates. (caveat: I may be misreading the code and this might not actually be the section of upjet being invoked here)

I removed omitempty from the generated code and ran the provider locally, and this resolved the bug. The provider no longer removed the field from spec.forProvider, and it correctly enforced that externally-attached policies should be detached during the reconciliation loop. I did a little bit of testing of the more common use case of not specifying spec.forProvider.managedPolicyArns, and I didn't see any issues, although I wouldn't consider my tests exhaustive.

@ulucinar Do you remember why we're always setting omitempty? Conceptually, it seems unnecessary, and while it's harmless in most cases, it's causing a problem here where the empty state is meaningful.

crossplane-contrib / provider-upjet-aws