IAM: better support for managed policies #621
Comments
rix0rrr
pushed a commit
that referenced
this issue
Aug 27, 2018
A number methods were allowing 'any's in places where they easily lead to passing the wrong object. - `role.attachManagedPolicy` - Various methods on `PolicyStatement`. By restrictinig the types to what we actually expect (or `string`s) these mistakes will be harder to make. Fixes #622, doesn't completely resolve but helps with #621.
rix0rrr
added a commit
that referenced
this issue
Aug 27, 2018
A number methods were allowing `any`s in places where they easily led
to passing the wrong object.
- `role.attachManagedPolicy()`, expected an `Arn` but it was possible to
pass it a `Policy` object by mistake.
- Various methods on `PolicyStatement`. Notably: `addResource()` which
is supposed to take an `Arn` but it was too easy to pass it a
resource object by mistake.
By restrictinig the types to what we actually expect these mistakes will
be harder to make (at the expense of slightly more code when passing raw
strings).
Most likely breakage consumers will see:
- `addResource("*")`, replace with `addAllResources()`.
- `addResource("some-arn")`, replace with `new Arn("some-arn")`.
- `new ServicePrincipal(new FnConcat(...))`, replace with
`new ServicePrincipal(new FnConcat(...).toString())`.
Fixes #622, doesn't completely resolve but helps with #621.
|
This can be closed for now; modeling of ManagedPolicies can be deferred to another ticket. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
role.attachManagedPolicy() has a poor signature. It accepts
any, but in fact it should accept onlyArns (until we have modeled ManagedPolicies)We had a user try to put a
PolicyStatementin there, which led to the following error:The text was updated successfully, but these errors were encountered: