IAM: permissions groups #521
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Stoakes
reviewed
Jan 2, 2024
| @@ -28,7 +28,6 @@ resource "ovh_iam_policy" "manager" { | |||
| "account:apiovh:me/certificates/get", | |||
| "account:apiovh:me/tag/get", | |||
| "account:apiovh:services/get", | |||
| "account:apiovh:*", | |||
There was a problem hiding this comment.
I found it was a good thing to have at least one example with a wildcard.
That illustrated that this field could take several values.
Thus I would keep at least a mention of wildcard (and thus either change the policy name or add a second one in example)
Stoakes
reviewed
Jan 2, 2024
| @@ -42,6 +41,7 @@ resource "ovh_iam_policy" "manager" { | |||
| * `allow` - List of actions allowed on resources by identities | |||
| * `except` - List of overrides of action that must not be allowed even if they are caught by allow. Only makes sens if allow contains wildcards. | |||
| * `deny` - List of actions that will be denied no matter what policy exists. | |||
| * `permissions_group` - Set of permissions group that apply to the policy. | |||
There was a problem hiding this comment.
Suggested change
| * `permissions_group` - Set of permissions group that apply to the policy. | |
| * `permissions_group` - Set of permissions included in the policy. (They are overriden by allow, except & deny fields) |
I would also detail how the permission group behaves compared to the allow, except & deny fields in the policy (suggestion between parenthesis above)
Stoakes
reviewed
Jan 2, 2024
| * `description` - Group description. | ||
| * `allow` - Set of actions allowed by the permissions group. | ||
| * `except` - Set of actions that will be subtracted from the `allow` list. | ||
| * `deny` - Set of actions that will be denied no matter what permissions group exists. |
There was a problem hiding this comment.
I find the wording surprising. Especially because a permission group doesn't reference a permission group
What about
Suggested change
| * `deny` - Set of actions that will be denied no matter what permissions group exists. | |
| * `deny` - Set of actions that will be denied no matter what permissions are defined in the permission group or policies using it. |
Stoakes
reviewed
Jan 2, 2024
| * `owner` - Owner of the permissions group. | ||
| * `created_at` - Creation date of this group. | ||
| * `updated_at` - Date of the last update of this group. | ||
| * `read_only` - Indicates that the permissions group is a default one. |
There was a problem hiding this comment.
Suggested change
| * `read_only` - Indicates that the permissions group is a default one. | |
| * `read_only` - Indicates that this is a default permission group, managed by OVHcloud. |
amstuta
requested changes
Jan 10, 2024
fehrnah
force-pushed
the
dev/nfournie/perm_group
branch
from
3b827b3
to
4ed9942
Compare
amstuta
approved these changes
Jan 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add support for the now available
permissions_groupsfor the IAM.This means supporting them in the policies and adding them as datasource and resource.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
make testacc TESTARGS="-run TestAccIamPerm.*"Checklist:
go mod vendorif I added or modifygo.modfile