add support for iam group resource #1350
Conversation
…> broke the tests)
…ication_ids work fine again
Codecov Report
@@ Coverage Diff @@
## master #1350 +/- ##
==========================================
- Coverage 69.34% 68.50% -0.84%
==========================================
Files 112 114 +2
Lines 17964 18183 +219
==========================================
Hits 12457 12457
- Misses 4424 4643 +219
Partials 1083 1083
Continue to review full report at Codecov.
|
…orm-provider-scaleway into diff_iam_group_resource
|
|
||
| data "scaleway_iam_group" "find_by_id_basic" { | ||
| group_id = scaleway_iam_group.main_ds_basic.id | ||
| organization_id = "08555df8-bb26-43bc-b749-1b98c5d02343" |
There was a problem hiding this comment.
Can the organization_id be deduced from the provider settings? it seems cumbersome to specify it again
There was a problem hiding this comment.
For now it is not possible for requests that use query arguments like ListGroups, so it affects the sweeper functions and the search by name in datasources. After discussing it with the team, the solution we found was to temporarily make the organization_id required in datasources (every iam datasource is concerned).
Apparently, it's related to this issue.
scaleway/resource_iam_group.go
Outdated
| if d.HasChange("name") { | ||
| req.Name = expandStringPtr(d.Get("name").(string)) | ||
| } else { | ||
| req.Name = &group.Name |
There was a problem hiding this comment.
Why should we have this else branch? Shouldn't we send only the thing we wish to modify?
There was a problem hiding this comment.
The Update request uses the PATCH method, not PUT so we have to send all information, even if we don't want to modify all of them. But indeed the way I wrote it could be improved so I've changed it a bit.
|
The problem with the order of IDs has been resolved by using a TypeSet instead of a TypeList for the |
Co-authored-by: Rémy Léone <[email protected]>
I managed to keep things the way they worked before the breaking changes of the IAM API, all my tests with users. applications and both at the same time work fine without these attributes being required, I think it's more user-friendly this way.