-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add azuread_access_package_catalog_role
and azuread_access_package_catalog_role_assignment
#1033
Conversation
044cec3
to
34dc70f
Compare
34dc70f
to
481ca8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @alexwilcox9, thanks for working on this! This is looking great, I have some suggestions below but since they're minor I'm going to go ahead and commit them, then this should be good to merge. Thanks!
internal/services/identitygovernance/access_package_catalog_role_assignment_resource.go
Outdated
Show resolved
Hide resolved
internal/services/identitygovernance/access_package_catalog_role_assignment_resource.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to add test coverage for different principal types, mainly so that we can monitor that the API and provider are both interacting properly in all the supported cases. I'll add this shortly.
"display_name": { | ||
Description: "The display name of the catalog role", | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Computed: true, | ||
ExactlyOneOf: []string{"display_name", "object_id"}, | ||
}, | ||
|
||
"template_id": { | ||
Description: "The object ID of the template associated with the catalog role", | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
|
||
"description": { | ||
Description: "The description of the catalog role", | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
|
||
"object_id": { | ||
Description: "The object ID of the catalog role", | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Computed: true, | ||
ExactlyOneOf: []string{"display_name", "object_id"}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally we try to put required attributes first, then optional ones, and computed attributes last. Will update and push this shortly.
internal/services/identitygovernance/access_package_catalog_role_assignment_resource_test.go
Show resolved
Hide resolved
This functionality has been released in v2.38.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
Hello @alexwilcox9, I open a new issue here #1091 |
Adds support for the data source
azuread_access_package_catalog_role
and the resourceazuread_access_package_catalog_role_assignment
This PR depends on #903, once that is merged I'll rebase this branch to main.