Add group target support for okta_group_role

[ymylei]

This adds additional schema to the okta_group_role resource and exposes the ability to assign a list of target groups to group admin role assignments which specifically support it.

Important: This relies upon / assumes #255 has been merged (it is appending schema to that resource created in the PR). You will find these commits in here temporarily until that is merged (or another similar resource is finished) in which case I will then rebase this and fix the test configurations based upon that outcome.

Combined with the PR mentioned above, this would replace #154.

[ymylei]

Docs on the API have me in a bit of a bind here: https://developer.okta.com/docs/reference/api/roles/#remove-a-group-target-from-a-group-administrator-role-given-to-a-group

Note: Don't remove the last Group target from a role assignment, as this causes an exception. If you need a role assignment that applies to all Groups, the API consumer should delete the USER_ADMIN role assignment and recreate it.

Because of that, this may probably be better off being an embedded object inside of the okta_group_role resource instead of a standalone resource, since it cannot support delete operations fully without also affecting resource.

[ymylei]

Acceptance test results (would like some additional confirmations here):

tom@DESKTOP-NNSTVLT:~/terraform-provider-okta$ make testacc TEST=./okta TESTARGS='-run=TestAccOktaGroupAdminRole_crud'
go mod tidy
go mod vendor
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./okta -v -run=TestAccOktaGroupAdminRole_crud  -timeout 120m
=== RUN   TestAccOktaGroupAdminRole_crud
--- PASS: TestAccOktaGroupAdminRole_crud (19.63s)
PASS
ok      github.com/oktadeveloper/terraform-provider-okta/okta   19.637s

[bogdanprodan-okta]

Hi, @ymylei! Thanks for the PR! Everything looks good, I just have to double-check everything and I think this might be included in the upcoming 3.8.0 release.

[jlaundry]

@ymylei I admire your perseverance with this project, and I hope Okta reward you accordingly 😊

If I could make one suggestion/request, have you considered separating each target to be it's own resource, instead of a list? (i.e., like I did in #154)?

While I appreciate that this PR faithfully implements the Okta API structure, it isn't consistent with the Terraform principle that resources should be a declarative representation of single component. The primary driver for this is that it makes it easier to implement modular config structure; we have cases were some group targets are not instantiated in a single Terraform repo, which will mean Terraform will remove those.

[ymylei]

@ymylei I admire your perseverance with this project, and I hope Okta reward you accordingly 😊

If I could make one suggestion/request, have you considered separating each target to be it's own resource, instead of a list? (i.e., like I did in #154)?

While I appreciate that this PR faithfully implements the Okta API structure, it isn't consistent with the Terraform principle that resources should be a declarative representation of single component. The primary driver for this is that it makes it easier to implement modular config structure; we have cases were some group targets are not instantiated in a single Terraform repo, which will mean Terraform will remove those.

Hey @jlaundry,

Appreciate the kind comment!

Per the comment I made here: #256 (comment) and the pertinent line from the Okta API docs:

Note: Don't remove the last Group target from a role assignment, as this causes an exception. If you need a role assignment that applies to all Groups, the API consumer should delete the USER_ADMIN role assignment and recreate it.

The standalone resource would be unable to fully support delete operations since a full delete would require also require affecting the parent role resource itself, therefore I do not consider it a good candidate to exist by itself.

[ymylei]

To add to the above, if you take this branch, go to the commit d6de84c and run the acceptance tests, you'll get the exception / api failure described above.

[bogdanprodan-okta]

Hi, @ymylei! I've merged your previous PR, so please fix the conflicts in this one! Thanks!

[ymylei]

@bogdanprodan-okta Just completed the rebase, updated acceptance test result below:

tom@DESKTOP-NNSTVLT:~/terraform-provider-okta$ make testacc TEST=./okta TESTARGS='-run=TestAccOktaGroupAdminRole_crud'
go mod tidy
go mod vendor
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./okta -v -run=TestAccOktaGroupAdminRole_crud  -timeout 120m
=== RUN   TestAccOktaGroupAdminRole_crud
--- PASS: TestAccOktaGroupAdminRole_crud (20.05s)
PASS
ok      github.com/oktadeveloper/terraform-provider-okta/okta   20.058s