Merge `BucketPolicyMembership` into `BucketPolicy`?

[negz]

What happened?

It seems like BucketPolicy and BucketPolicyMembership actually both manipulate the same underlying API object, per https://cloud.google.com/storage/docs/json_api/v1/buckets/setIamPolicy. This doesn't seem idiomatic - I would expect there to be a single BucketPolicy type that could be updated to add and remove members.

How can we reproduce it?

What environment did it happen in?

Crossplane version:

[negz]

@turkenh I forget whether this was discussed when you implemented https://github.com/crossplane/provider-gcp/pull/291. Was there a reason to split this into two different managed resources?

[turkenh]

TL;DR: It was because BucketPolicy resource could remove google managed policies breaking basic access to the bucket from console or cli.

---

@negz I don't remember any public discussion that I could refer to. However, I remember the reason why I ended up doing so. My primary use case for the resources introduced with #291 was to provision required infrastructure to run a production-grade Vault on GKE with proper configuration.

After creating a bucket and a service account for Vault storage, I need to add two members to the policy of the bucket which corresponds to the following commands:

gsutil iam ch \
  ${SERVICE_ACCOUNT}@${PROJECT_ID}.iam.gserviceaccount.com:objectAdmin \
  gs://${GCS_BUCKET_NAME}

gsutil iam ch \
  ${SERVICE_ACCOUNT}@${PROJECT_ID}.iam.gserviceaccount.com:legacyBucketReader \
  gs://${GCS_BUCKET_NAME}

My first attempt was to achieve this with a single BucketPolicy object, however, when I created a managed resource with the above two members, I could no longer access the bucket from the GKE web console since it overrides some existing google managed policies which was allowing me to do so. I believe, it was the same as in this stackoverflow post. Later I noticed that, terraform also has separate resources for policy and member and followed the same approach.

[negz]

Yeah, I started using BucketPolicyMember in earnest last night after raising this issue and I can see why we've modelled it this way. I feel a little uncomfortable about potentially having N managed resources all managing the state of one API endpoint, but it does lead to a much better UX. 😬