S3 with Bucket Policy will stuck with reason "Creating" state

[kelvinwijaya]

What happened?

This issue is related to : #1758 (comment)

How can we reproduce it?

Create S3 with Bucket Policy, the state will stuck with reason "Creating" state. Please refer to existing issue in #1758

         status:
           awsConditions:
        -  - reason: Available
        -    status: "True"
        +  - lastTransitionTime: "2023-06-12T06:51:35Z"
        +    reason: Creating
        +    status: "False"
             type: Ready

What environment did it happen in?

Crossplane version: 1.10.1
Provider-aws version: master build (last commit: 8282408)
Kubernetes version: v1.24.13-eks-0a21954

This is my sample Bucket manifest created from composition

apiVersion: s3.aws.crossplane.io/v1beta1
kind: Bucket
metadata:
  annotations:
    crossplane.io/external-create-pending: "2023-06-12T06:51:33Z"
    crossplane.io/external-create-succeeded: "2023-06-12T06:51:35Z"
    crossplane.io/external-name: test-bucket-xxxx
  creationTimestamp: "2023-06-12T06:51:33Z"
  finalizers:
  - finalizer.managedresource.crossplane.io
  generateName: test-bucket-mfqjc-
  generation: 2
  labels:
    crossplane.io/claim-name: test-bucket
    crossplane.io/claim-namespace: my-namespace
    crossplane.io/composite: test-bucket-mfqjc
  name: test-bucket-mfqjc-hnx2w
  ownerReferences:
  - apiVersion: crd.aws.acme.domain/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: S3Bucket
    name: test-bucket-mfqjc
    uid: b743c14a-2276-4744-b639-e16b9f31c99b
  resourceVersion: "930007057"
  uid: 9cf10dcd-f145-4c5e-8be1-9f95c23bf681
spec:
  deletionPolicy: Delete
  forProvider:
    corsConfiguration:
      corsRules:
      - allowedHeaders:
        - '*'
        allowedMethods:
        - PUT
        - POST
        - DELETE
        allowedOrigins:
        - '*'
      - allowedMethods:
        - GET
        allowedOrigins:
        - '*'
    lifecycleConfiguration:
      rules:
      - expiration:
          expiredObjectDeleteMarker: true
        noncurrentVersionExpiration:
          noncurrentDays: 1
        status: Enabled
    locationConstraint: us-east-1
    objectLockEnabledForBucket: false
    objectOwnership: BucketOwnerEnforced
    paymentConfiguration:
      payer: BucketOwner
    policy:
      statements:
      - action:
        - s3:PutObject
        condition:
        - conditions:
          - key: s3:x-amz-server-side-encryption
            listValue:
            - AES256
            - aws:kms
          operatorKey: StringNotEquals
        effect: Deny
        principal:
          allowAnon: true
        resource:
        - arn:aws:s3:::test-bucket-xxxx/*
        sid: DenyIncorrectEncryptionHeader
      - action:
        - s3:PutObject
        condition:
        - conditions:
          - booleanValue: true
            key: s3:x-amz-server-side-encryption
          operatorKey: Bool
        effect: Deny
        principal:
          allowAnon: true
        resource:
        - arn:aws:s3:::test-bucket-xxxx/*
        sid: DenyUnEncryptedObjectUploads
      - action:
        - s3:GetBucketLocation
        - s3:GetBucketVersioning
        - s3:GetLifecycleConfiguration
        - s3:GetObject
        - s3:GetObjectAcl
        - s3:GetObjectVersion
        - s3:GetObjectTagging
        - s3:GetObjectRetention
        - s3:PutObject
        - s3:PutObjectAcl
        - s3:DeleteObject
        - s3:ListBucket
        - s3:ListBucketVersions
        condition:
        - conditions:
          - key: aws:PrincipalAccount
            stringValue: "123456789012"
          operatorKey: StringEquals
        effect: Allow
        principal:
          allowAnon: true
        resource:
        - arn:aws:s3:::test-bucket-xxxx
        - arn:aws:s3:::test-bucket-xxxx/*
        sid: AllowTenantReadWrite
      - action:
        - s3:*
        condition:
        - conditions:
          - booleanValue: false
            key: aws:SecureTransport
          operatorKey: Bool
        effect: Deny
        principal:
          allowAnon: true
        resource:
        - arn:aws:s3:::test-bucket-xxxx
        - arn:aws:s3:::test-bucket-xxxx/*
        sid: AllowSSLRequestsOnly
      version: "2012-10-17"
    publicAccessBlockConfiguration:
      blockPublicAcls: true
      blockPublicPolicy: true
      ignorePublicAcls: true
      restrictPublicBuckets: true
    serverSideEncryptionConfiguration:
      rules:
      - applyServerSideEncryptionByDefault:
          sseAlgorithm: AES256
    tagging:
      tagSet:
      - key: version
        value: ""
    versioningConfiguration:
      status: Enabled
  providerConfigRef:
    name: aws-provider
  writeConnectionSecretToRef:
    name: test-bucket-mfqjc
    namespace: crossplane-system
status:
  atProvider:
    arn: arn:aws:s3:::test-bucket-xxxx
  conditions:
  - lastTransitionTime: "2023-06-12T06:51:35Z"
    reason: Creating
    status: "False"
    type: Ready
  - lastTransitionTime: "2023-06-12T06:51:35Z"
    reason: ReconcileSuccess
    status: "True"
    type: Synced

[MisterMX]

I tested it with the example above and it seems like the issue is caused by AWS converting bools in conditions to strings:

policy.Policy{
  	Version: "2012-10-17",
  	ID:      "",
  	Statements: policy.StatementList{
  		{SID: "DenyIncorrectEncryptionHeader", Effect: "Deny", Principal: &{AllowAnon: true}, Action: {"s3:PutObject"}, ...},
  		{
  			... // 6 identical fields
  			Resource:    {"arn:aws:s3:::test-bucket-mfqjc-hnx2w-asdas/*"},
  			NotResource: nil,
- 			Condition:   policy.ConditionMap{"Null": {"s3:x-amz-server-side-encryption": bool(true)}},
+ 			Condition:   policy.ConditionMap{"Null": {"s3:x-amz-server-side-encryption": string("true")}},
  		},
  		{SID: "AllowTenantReadWrite", Effect: "Allow", Principal: &{AllowAnon: true}, Action: {"s3:GetBucketLocation", "s3:GetBucketVersioning", "s3:GetLifecycleConfiguration", "s3:GetObject", ...}, ...},
  		{
  			... // 6 identical fields
  			Resource:    {"arn:aws:s3:::test-bucket-mfqjc-hnx2w-asdas", "arn:aws:s3:::test-bucket-mfqjc-hnx2w-asdas/*"},
  			NotResource: nil,
- 			Condition:   policy.ConditionMap{"Bool": {"aws:SecureTransport": bool(false)}},
+ 			Condition:   policy.ConditionMap{"Bool": {"aws:SecureTransport": string("false")}},
  		},
  	},
  }

[kelvinwijaya]

Thanks @MisterMX , i have tested with provider-aws v0.41.0

In my Bucket composition, i have to convert the conditions to making use of stringValue instead to make it works, not sure if this is the intention

(old implementation)
condition: 
  - operatorKey: Bool
    conditions:
      - key: aws:SecureTransport
        boolValue: false 
(new implementation)
condition:                  
  - operatorKey: Bool
    conditions:
      - key: aws:SecureTransport
        stringValue: "false"