Conversation
alexrashed
left a comment
There was a problem hiding this comment.
The change is looking good, but it feels like this should be the default in order to have a working configuration of LocalStack. With the currently suggested change, I would have to explicitly configure these roles to fix ECS tasks in LocalStack, right?
Maybe we should just add them? Or set them as default with the possibility of removing them?
|
@alexrashed I agree, however at the same time it's harder to remove something than adding rules. But perhaps time boxed it might worth to look into it @simonrw. I can live with either one. |
|
Perhaps something like: (values.toml) role:
defaultRules: true
extraRules: []and we default to these rules in the PR. Then the user can opt out of whatever we add with (inspired by Rust's handling of "features": https://doc.rust-lang.org/cargo/reference/features.html#the-default-feature and the "default" feature) |
|
I would actually just add it to the roles without having it configurable. If it gets requested to be configurable (i.e. the possibility to remove roles while risking to break LocalStack), then we can extract the whole content of the roles (including the already existing ones) to the default value of a newly introduced variable. |
Motivation
When running ECS tasks, the service role needs more permissions than what is specified in the template. Specifically, in addition to the existing rules we need:
Changes