[Bug]: TopicSubscription.spec.forProvider.filterPolicyScope cannot be removed once .filterPolicy was set

[baburciu]

Is there an existing issue for this?

• I have searched the existing issues

Affected Resource(s)

sns.aws.upbound.io/v1beta1

Resource MRs required to reproduce the bug

apiVersion: sns.aws.upbound.io/v1beta1
kind: TopicSubscription
metadata:
  name: acme-corp-application-subscription-demo-a-b92579d7
spec:
  deletionPolicy: Delete
  forProvider:
    confirmationTimeoutInMinutes: 1
    endpoint: >-
      arn:aws:sqs:eu-west-1:111222333444:acme-corp-application-events
    filterPolicy: |
      {
        "event_version": ["1.0.0"],
        "event_type": ["TransactionStatusChanged"]
      }
    protocol: sqs
    rawMessageDelivery: true
    redrivePolicy: |
      {
        "deadLetterTargetArn": "arn:aws:sqs:eu-west-1:111222333444:acme-corp-application-events-dlq"
      }
    region: eu-west-1
    topicArn: >-
      arn:aws:sns:eu-west-1:111222333444:acme-corp-application-uploads
  initProvider: {}
  managementPolicies:
    - '*'
  providerConfigRef:
    name: provider-aws-upbound-sns

Steps to Reproduce

If this MR is first deployed, the MR .status.atProvider.filterPolicyScope field will be set to MessageAttributes value.
Then if a new MR generation is without .spec.forProvider.filterPolicy, the MR .status.conditions[].message will be

        observe failed: cannot compute the instance diff: failed to get
        *terraform.InstanceDiff: filter_policy is required when
        filter_policy_scope is set

and .reason: ReconcileError.

What happened?

We're building the MR through a mode: Pipeline Composition with function-go-templating step, that allows the self-service platform consumer inject .spec.forProvider.filterPolicy (and only that, the filterPolicyScope is added by the provider controller if filterPolicy is present, I think), but consumer may later decide to drop filterPolicy (updating some helm values, Composite Claim is deployed with Helm) and then the MR would not have .spec.forProvider.filterPolicy, but it would still have .spec.forProvider.filterPolicyScope , resulting in the mentioned error.

Crossplane provider controller adds TopicSubscription.spec.forProvider.filterPolicyScope as we can see in its .metadata.managedFields

    - apiVersion: sns.aws.upbound.io/v1beta1
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:annotations:
            f:crossplane.io/external-create-failed: {}
            f:crossplane.io/external-create-pending: {}
            f:crossplane.io/external-create-succeeded: {}
            f:crossplane.io/external-name: {}
          f:finalizers:
            .: {}
            v:"finalizer.managedresource.crossplane.io": {}
        f:spec:
          f:forProvider:
            f:confirmationTimeoutInMinutes: {}
            f:filterPolicyScope: {}  #<==
          f:initProvider: {}
      manager: provider
      operation: Update
      time: '2024-11-07T13:51:26Z

By the look of it, this is caused by Terraform provider underlying the Crossplane provider: https://github.com/hashicorp/terraform-provider-aws/blob/v5.58.0/internal/service/sns/topic_subscription.go#L571-L573.
And trying to template the Composition to "nullify" this with sending an empty string "" for either .spec.forProvider.filterPolicy, .spec.forProvider.filterPolicyScope, or even both at the same time, did not help, even though this code should have removed filterPolicyScope.

Relevant Error Output Snippet

observe failed: cannot compute the instance diff: failed to get *terraform.InstanceDiff: filter_policy is required when filter_policy_scope is set

Crossplane Version

1.18.0

Provider Version

1.14.0

Kubernetes Version

v1.28.13

Kubernetes Distribution

EKS v1.28.13-eks-a737599

Additional Info

The provider:

$ kubectl get pod -n crossplane-system provider-aws-upbound-sns-5836380c004f-6cd585b7c8-cq2sk -o yaml | yq .spec.containers[].image
xpkg.upbound.io/upbound/provider-aws-sns:v1.14.0

MR error when trying to reset filterPolicyScope from Composition:

                [[- if $spec.filterPolicy ]]
                filterPolicy:
                [[ $spec.filterPolicy | toYaml | indent 6 ]]
                  [[- if $spec.filterPolicyScope ]]
                filterPolicyScope: [[ $spec.filterPolicyScope ]]
                  [[- end ]]
                [[- else ]]
                filterPolicyScope: ""
                [[- end ]]

we still get the error

spec:
  deletionPolicy: Delete
  forProvider:
    confirmationTimeoutInMinutes: 1
    endpoint: >-
      arn:aws:sqs:eu-west-1:111222333444:acme-corp-application-events
    filterPolicyScope: ''    # <===
    protocol: sqs
    rawMessageDelivery: true
    redrivePolicy: |
      {
        "deadLetterTargetArn": "arn:aws:sqs:eu-west-1:111222333444:acme-corp-application-events-dlq"
      }
    region: eu-west-1
    topicArn: >-
      arn:aws:sns:eu-west-1:111222333444:acme-corp-application-uploads
  initProvider: {}
  managementPolicies:
    - '*'
  providerConfigRef:
    name: provider-aws-upbound-sns
status:
  atProvider:
    arn: >-
      arn:aws:sns:eu-west-1:111222333444:acme-corp-application-uploads:fbaf5a9a-11be-47f4-9411-05f31dbc79d8
    confirmationTimeoutInMinutes: 1
    confirmationWasAuthenticated: true
    deliveryPolicy: ''
    endpoint: >-
      arn:aws:sqs:eu-west-1:111222333444:acme-corp-application-events
    endpointAutoConfirms: false
    filterPolicy: '{"event_version":["1.0.0"],"event_type":["TransactionStatusChanged"]}'
    filterPolicyScope: MessageAttributes
    id: >-
      arn:aws:sns:eu-west-1:111222333444:acme-corp-application-uploads:fbaf5a9a-11be-47f4-9411-05f31dbc79d8
    ownerId: '027159581111'
    pendingConfirmation: false
    protocol: sqs
    rawMessageDelivery: true
    redrivePolicy: |
      {
        "deadLetterTargetArn": "arn:aws:sqs:eu-west-1:111222333444:acme-corp-application-events-dlq"
      }
    replayPolicy: ''
    subscriptionRoleArn: ''
    topicArn: >-
      arn:aws:sns:eu-west-1:111222333444:acme-corp-application-uploads
  conditions:
    - lastTransitionTime: '2024-11-08T15:03:56Z'
      reason: Available
      status: 'True'
      type: Ready
    - lastTransitionTime: '2024-11-08T15:10:12Z'
      message: >-
        observe failed: cannot compute the instance diff: failed to get
        *terraform.InstanceDiff: filter_policy is required when
        filter_policy_scope is set
      reason: ReconcileError
      status: 'False'
      type: Synced
    - lastTransitionTime: '2024-11-08T15:03:56Z'
      reason: Success
      status: 'True'
      type: LastAsyncOperation

PRs https://github.com/hashicorp/terraform-provider-aws/pull/28004/files & https://github.com/hashicorp/terraform-provider-aws/pull/28253/files cover filter_policy_scope feature in Terraform provider and I think the issue may be with https://github.com/hashicorp/terraform-provider-aws/blob/v5.58.0/internal/service/sns/topic_subscription.go#L546C1-L549C59

    func resourceTopicSubscriptionCustomizeDiff(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error {
	hasPolicy := diff.Get("filter_policy").(string) != ""
	hasScope := !diff.GetRawConfig().GetAttr("filter_policy_scope").IsNull()
	hadScope := diff.Get("filter_policy_scope").(string) != "

I see this diff type is from Go package https://github.com/hashicorp/terraform-plugin-sdk/blob/v2.34.0/helper/schema/resource_diff.go#L104-L146 but I don't get why the hasPolicy and hasScope are determined with different methods. Somehow the conditional branch for diff.Clear("filter_policy_scope") is skipped.

Would this be a bug in the Terraform provider and should I report it there?
Thanks a lot!