Comments (18)
Frankly, I think that sort of case should simply be handled by invoking the module twice (for the buckets with different permission sets). If we try to handle every possible case through variables, the interface ends up overly complicated IMO.
from terraform-google-cloud-storage.
It could, it complicates things a bit. One solution would be this:
roles = {
'roles/storage.objectCreator' = ["bucket-one", "bucket-two"],
'roles/storage.legacyObjectReader' = ["bucket-two"]
}
role_members = {
'roles/storage.objectCreator' = [
"user:[email protected]", "group:[email protected]"
],
'roles/storage.legacyObjectReader' = [
"user:[email protected]", "group:[email protected]"
]
}
It's a bit repetitive but much easier than dealing with 20 variables (one boolean per role, plus one containing the role members). Roles could maybe be swapped, with buckets as keys and bucket roles as values, and at that point we could probably have bucket => { role => members}. At which point 20 variables start to make sense again, probably. I need a bit more time to think it through, but definitely welcome comments. :)
from terraform-google-cloud-storage.
@morgante absolutely, I want to push this idea to the limit and then dial it down to find a balance between complexity vs using more instances of the module.
from terraform-google-cloud-storage.
This may benefit from renaming of some variables (e.g. admin
should probably be reserved for storage.admin
, we could use object_admin
for what admin
is now. And the bucket_admins|creators|viewers
might be confusing with storage.legacyBucket(Reader|Writer|Owner)
if they were to be added
from terraform-google-cloud-storage.
Hi @ludoo I was wondering if a PR was actively in the works and what will be your approach? Thanks
from terraform-google-cloud-storage.
Hey! I'm sorry I could not get around to doing it yet, I hope to have it by week(end)'s end.
from terraform-google-cloud-storage.
No worries at all, I was just curious :) Grazie mille @ludoo !!
from terraform-google-cloud-storage.
So, I found half an hour to mull this over, and I'm thinking it's probably a good time to change and simplify the interface. What I have in mind is having only two variables for roles that will be applied to all buckets: roles
and role_members
. This would look something like this in a tfvars
file:
roles = ['roles/storage.objectCreator', 'roles/storage.legacyObjectReader']
role_members = {
'roles/storage.objectCreator' = [
"user:[email protected]", "group:[email protected]"
],
'roles/storage.legacyObjectReader' = [
"user:[email protected]", "group:[email protected]"
]
}
The drawback is one needs to know the role names, the big advantages are a) hyper flexible, and b) it would use for_each
so allow removing/adding roles and members without impacting all resources.
I did not think about the rest of the resources, but thinking about bucket-wide roles, would this suit your use case?
PS - we need a list of plain role names so as to avoid using dynamic values in for_each
, hence the duplication of role names in both variables.
from terraform-google-cloud-storage.
I like the idea of simplifying the interface, however if I understand correctly, the ability to define different bindings per bucket will be eliminated?
Random example: Say I create buckets A
, B
. I would like team_1
as storage.admin
of A
but object.viewer
of B
I would also like that team_2
be object.creator
of bucket B
only and no rights on bucket A
.
Is this kind of thing still possible with the suggested refactoring?
from terraform-google-cloud-storage.
Given it a bit more thought, and my preference would be for:
- merging the three variables that apply to all buckets (
admins
,creators
,viewers
) into two newroles
androle_members
variables that handle any role like described in my comment above; this will allow setting any role uniformly to all buckets, including currently unsupported roles - adding support for the two per-bucket missing roles (
storage.admin
andstorage.hmacKeyAdmin
) so that we cover all non-legacy roles at the individual bucket level
This IMHO would make the module more flexible by a) supporting any role applied to all buckets, and b) covering all non-legacy roles at the individual bucket level. The additional complexity in adding the two new per-bucket roles is balanced by simplifying the all-bucket roles variables.
The number of resources will increase slightly, and as they will be incompatible with previous versions it will also be a good time to migrate from count
to for_each
, thus providing better resiliency to changes.
And as Morgante wrote above, the right pattern for cases not directly covered (eg where different legacy roles are needed for specific buckets) is to have more module invocations for the different buckets or groups of buckets.
@naseemkullah would this cover your case?
from terraform-google-cloud-storage.
@averbuks can you also share your opinion on the proposed changes?
from terraform-google-cloud-storage.
This proposed refactor will allow as you mentioned
a) supporting any role applied to all buckets, and b) covering all non-legacy roles at the individual bucket level
Is it possible to go as for as b) supporting any role applied at the individual bucket level or would that be too complex?
If that is not possible, what do you think about using the same method for both all buckets and per bucket permissions settings, so for all buckets something like:
For all buckets:
all_buckets_object_admins
all_buckets_object_creators
all_buckets_object_viewers
all_buckets_admins
all_buckets_hmac_key_admins
and for per bucket:
bucket_scoped_object_admins
bucket_scoped_object_creators
bucket_scoped_object_viewers
bucket_scoped_admins
bucket_scoped_hmac_key_admins
I think it is a bit confusing to have very different ways of specifying roles on all buckets vs per bucket level.
But yes this would cover my use case, because i realize that storage.admin
can be used in favor of mixing storage.objectCreator
with storage.legacyBucketReader
!
from terraform-google-cloud-storage.
FWIW I am also fine with removing the bucket_scoped permissions as Morgante mentioned and just invocating the module multiple times, in my case i would probably define just 1 bucket per incovation, but the module is still useful in this case.
from terraform-google-cloud-storage.
Can we consider switching to the simplified interface where the same permissions are applied to all buckets?
It's rather unclear to me why you would want/need buckets which have entirely different permissions to be created using the same module invocation. In such cases, it seems like the benefit is almost 0 while increasing complexity for users who only want to (a) create one bucket or (b) apply the same roles across all buckets.
from terraform-google-cloud-storage.
If you mean by that that we will be able to have our own module versions in Fabric with support for multiple resources and roles, I'll be glad to modify this. Otherwise, I don't see how this could happen.
from terraform-google-cloud-storage.
I'm just trying to understand why interface A is so much worse than interface B for you. They both accomplish the same outcome and A seems much much more intuitive (based on actual user feedback):
Interface A:
module "my-foo-bucket" {
source = "../modules/simple_bucket"
names = ["foo-bucket"]
project = "foo-project"
location = "us-central1"
iam_members = [
{ role = "roles/storage.objectAdmin", member = "user:[email protected]" }
]
}
module "my-bar-bucket" {
source = "../modules/simple_bucket"
names = ["bar-bucket"]
project = "foo-project"
location = "us-central1"
iam_members = [
{ role = "roles/storage.objectAdmin", member = "user:[email protected]" }
]
}
Interface B:
module "my-simple-buckets" {
source = "../modules/simple_bucket"
names = ["foo-bucket", "bar-bucket"]
project = "foo-project"
location = "us-central1"
iam_members = [
"foo-bucket" = { role = "roles/storage.objectAdmin", member = "user:[email protected]" }
"bar-bucket" = { role = "roles/storage.objectAdmin", member = "user:[email protected]" }
]
}
Anyways, what if we moved the multi-bucket, multi-role assignment into a submodule? Just trying to find a solution which is intuitive for everyone.
from terraform-google-cloud-storage.
Hi @morgante, that discussion absolutely makes sense from my point of view since the module is supporting something which is not commonly used, but some fabric examples require per_bucket_admins
functionality. Like for ex. where we create a SA and bucket per environment and giving the SAs with permissions to the buckets respectively https://github.com/terraform-google-modules/cloud-foundation-fabric/blob/master/foundations/environments/main.tf#L61-L73. It also answers the question above.
Or imagine someone creates a list of buckets for storing sensitive data for different teams, it that case they would want to have different buckets for diff teams without a need to call the module len(teams) times.
That problem will be simplified a lot once terraform has for_each
functionality for modules.
from terraform-google-cloud-storage.
@averbuks Thanks for providing an example link, that makes the use case much clearer for me. I still think it's complex, but clearly necessary for some users.
Given this, I think we should move this complexity into a submodule while providing a simpler interface (for a single bucket) through the root module.
from terraform-google-cloud-storage.
Related Issues (20)
- Add support for public_access_prevention
- Provisioning fails when creating buckets with labels: "Provider produced inconsistent final plan" HOT 4
- Allow configure the autoclass argument HOT 2
- Include a matches_prefix variable in this module HOT 2
- Specifying `bucket_admins` has no effect HOT 3
- Fix documentation for encryption_key_names variable HOT 1
- Doesn't work with apple silicone HOT 1
- Set versioning for all buckets in module HOT 1
- bucket_lifecycle_rules cannot be used without specifying lifecycle_rules
- The "logging" input parameter is not taken into account HOT 1
- On module "buckets", using action type "SetStorageClass" on the "lifecycle_rules" is not working HOT 11
- Dependency requirement conflict when using this module along GKE module HOT 2
- Allow CORS and website configurations only for one bucket HOT 9
- matches_prefix on gcs lifecycle management not working with terraform HOT 1
- Support for configuring soft-deletes in GCS buckets HOT 1
- bucket destroying after adding default value in var.tf HOT 2
- Out of date gcp provider (Support version 5+)
- Bucket names are not validated HOT 2
- soft_delete_policy default value is incorrect HOT 1
- Module "simple_bucket" issue
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from terraform-google-cloud-storage.