Closed dancoates closed 9 months ago
preview
on datasets/production@illusional you can see above in the pulumi preview diff which projects this will alter the bucket role for. My only concern is whether any of those are using any of the other permissions that get removed by this role change, particularly things like storage.buckets.get
and storage.buckets.getIamPolicy
which are fairly innocuous and could potentially be used somewhere. I'm not sure the best way to check if these are being used though
Thanks for putting this together @dancoates! I wonder if we should add another BucketMembership type to our abstraction that recovers some of those permissions, like list, get, getIamPolicy.
I think this is actually fine, I don't think anyone (or service account) actively looks through the iam policies, nor do we want them to be able to?
I think this permission will be applied in an awkward way, potentially by removing the permission, and then adding it, which could break our permissions entirely during the deploy. I guess there are a few options, we wait till there's basically no analysis happening, or we try to do this in two stages (add an extra, then delete). What do you think?
All modified and coverable lines are covered by tests :white_check_mark:
:exclamation: No coverage uploaded for pull request base (
main@49c99d1
). Click here to learn what that means.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@illusional This has got a bit more involved than I'd like but I think it is important to have the resource key explicitly defined per bucket role so that when we remove the storage.admin
role we can ensure that the new StorageObjectAndBucketMutator
role maintains its same resource key and is not deleted and recreated.
I also didn't end up allowing specifying multiple bucket membership types as I'm not sure if there's much to be gained by stacking memberships - I think each role is a superset of the previous, so allowing selecting multiple wouldn't have much effect. I'm inferring that from the role names though, I don't think I can see the policies on the roles.
148
previously a
BucketMembership.MUTATE
bucket membership would assign the storage.admin role which has elevated privileges including allowing deletion of the bucket. Changing tostorage.objectAdmin
reduces these privileges to only allow manipulation of the bucket contents but not the bucket itself.The roles differ by
storage.admin
having the following wildcard entries:whereas
storage.objectAdmin
has:and no
storage.buckets
permissions. All other permissions are the same between the roles.So in effect this change removes the following permissions from the bucket mutate membership: