Skip to content
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

New Resource: User Group Memberships #416

Merged
merged 11 commits into from
Apr 14, 2021

Conversation

ymylei
Copy link
Contributor

@ymylei ymylei commented Apr 8, 2021

In an effort to correct some of the mistakes posed by the use of the resource okta_group_membership (#252), I'm creating/proposing this new okta_user_group_memberships resource.

This is basically a clone of okta_group_membership, however the read functionality and handling is modified so the groups are handled in bulk on a per user basis. This allows for significant reduction in API calls during the READ phase (and create phase) when doing the validation of memberships of groups. This is because the standard API endpoints for users will allow you to get all groups, so it's better due to the API limits imposed to compute as much as we can IMO.

@ymylei
Copy link
Contributor Author

ymylei commented Apr 8, 2021

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

@ymylei
Copy link
Contributor Author

ymylei commented Apr 8, 2021

tom@DESKTOP-NNSTVLT:~/terraform-provider-okta$ make testacc
go mod tidy
go mod vendor
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v   -timeout 120m
?       github.com/okta/terraform-provider-okta [no test files]
=== RUN   TestAccOktaDataSourceAppOauth_read
--- PASS: TestAccOktaDataSourceAppOauth_read (11.13s)
=== RUN   TestAccOktaDataSourceAppMetadataSaml_read
--- PASS: TestAccOktaDataSourceAppMetadataSaml_read (7.71s)
=== RUN   TestAccOktaDataSourceAppSaml_read
--- PASS: TestAccOktaDataSourceAppSaml_read (12.07s)
=== RUN   TestAccOktaDataSourceApp_read
--- PASS: TestAccOktaDataSourceApp_read (41.03s)
=== RUN   TestAccOktaDataSourceAuthServerPolicy_read
--- PASS: TestAccOktaDataSourceAuthServerPolicy_read (11.24s)
=== RUN   TestAccOktaDataSourceAuthServerScopes
--- PASS: TestAccOktaDataSourceAuthServerScopes (5.67s)
=== RUN   TestAccOktaDataSourceAuthServer_read
--- PASS: TestAccOktaDataSourceAuthServer_read (10.59s)
=== RUN   TestAccOktaDataSourceDefaultPolicy_readPasswordPolicy
--- PASS: TestAccOktaDataSourceDefaultPolicy_readPasswordPolicy (4.81s)
=== RUN   TestAccOktaDataSourceDefaultPolicy_readIdpPolicy
--- PASS: TestAccOktaDataSourceDefaultPolicy_readIdpPolicy (4.85s)
=== RUN   TestAccOktaDataSourceEveryoneGroup_read
--- PASS: TestAccOktaDataSourceEveryoneGroup_read (4.82s)
=== RUN   TestAccOktaDataSourceGroup_read
--- PASS: TestAccOktaDataSourceGroup_read (12.63s)
=== RUN   TestAccOktaDataSourceGroups_read
    TestAccOktaDataSourceGroups_read: data_source_okta_groups_test.go:15: Step 2/2 error: Check failed: Check 2/2 error: data.okta_groups.test: Attribute 'groups.#' expected "2", got "11"
--- FAIL: TestAccOktaDataSourceGroups_read (6.70s)
=== RUN   TestAccOktaDataSourceIdpMetadataSaml_read
--- PASS: TestAccOktaDataSourceIdpMetadataSaml_read (9.03s)
=== RUN   TestAccOktaDataSourceIdpOidc_read
--- PASS: TestAccOktaDataSourceIdpOidc_read (11.78s)
=== RUN   TestAccOktaDataSourceIdpSaml_read
--- PASS: TestAccOktaDataSourceIdpSaml_read (18.07s)
=== RUN   TestAccOktaDataSourceIdpSocial_read
--- PASS: TestAccOktaDataSourceIdpSocial_read (11.14s)
=== RUN   TestAccOktaDataSourcePolicy_read
--- PASS: TestAccOktaDataSourcePolicy_read (4.83s)
=== RUN   TestAccOktaDataSourceUser_read
--- PASS: TestAccOktaDataSourceUser_read (13.44s)
=== RUN   TestAccOktaDataSourceUserType_read
--- PASS: TestAccOktaDataSourceUserType_read (8.47s)
=== RUN   TestAccOktaDataSourceUsers_read
--- PASS: TestAccOktaDataSourceUsers_read (9.50s)
=== RUN   TestProvider
--- PASS: TestProvider (0.01s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccAdminRoleTargets
--- PASS: TestAccAdminRoleTargets (12.70s)
=== RUN   TestAccAppAutoLoginApplication_crud
--- PASS: TestAccAppAutoLoginApplication_crud (10.89s)
=== RUN   TestAccAppBasicAuthApplication_crud
--- PASS: TestAccAppBasicAuthApplication_crud (12.45s)
=== RUN   TestAccAppBookmarkApplication_crud
--- PASS: TestAccAppBookmarkApplication_crud (13.29s)
=== RUN   TestAccAppGroupAssignment_crud
--- PASS: TestAccAppGroupAssignment_crud (46.50s)
=== RUN   TestAccAppGroupAssignment_retain
--- PASS: TestAccAppGroupAssignment_retain (10.17s)
=== RUN   TestAccAppGroupAssignments_crud
--- PASS: TestAccAppGroupAssignments_crud (16.50s)
=== RUN   TestAccAppOAuthApplication_apiScope
--- PASS: TestAccAppOAuthApplication_apiScope (41.04s)
=== RUN   TestAccAppOAuthApplication_redirectCrud
--- PASS: TestAccAppOAuthApplication_redirectCrud (11.26s)
=== RUN   TestAccAppOauth_basic
--- PASS: TestAccAppOauth_basic (12.13s)
=== RUN   TestAccAppOauth_serviceNative
--- PASS: TestAccAppOauth_serviceNative (11.22s)
=== RUN   TestAccAppOauth_federationBroker
    TestAccAppOauth_federationBroker: resource_okta_app_oauth_test.go:119: This is an 'Early Access Feature' and needs to be enabled by Okta, skipping this test as it fails when this feature is not available
--- SKIP: TestAccAppOauth_federationBroker (0.00s)
=== RUN   TestAccAppOauth_badGrantTypes
--- PASS: TestAccAppOauth_badGrantTypes (2.22s)
=== RUN   TestAccAppOauth_customProfileAttributes
--- PASS: TestAccAppOauth_customProfileAttributes (15.01s)
=== RUN   TestAccAppOauth_customClientID
--- PASS: TestAccAppOauth_customClientID (10.58s)
=== RUN   TestAccAppOauth_customClientIDError
--- PASS: TestAccAppOauth_customClientIDError (1.44s)
=== RUN   TestAccAppOauth_serviceWithJWKS
--- PASS: TestAccAppOauth_serviceWithJWKS (6.19s)
=== RUN   TestAccAppSaml_conditionalRequire
--- PASS: TestAccAppSaml_conditionalRequire (2.21s)
=== RUN   TestAccAppSaml_invalidURL
--- PASS: TestAccAppSaml_invalidURL (1.44s)
=== RUN   TestAccAppSaml_crud
--- PASS: TestAccAppSaml_crud (19.63s)
=== RUN   TestAccAppSaml_userGroups
--- PASS: TestAccAppSaml_userGroups (15.35s)
=== RUN   TestAccAppSecurePasswordStoreApplication_credsSchemes
--- PASS: TestAccAppSecurePasswordStoreApplication_credsSchemes (10.51s)
=== RUN   TestAccAppSwaApplication_preconfig
--- PASS: TestAccAppSwaApplication_preconfig (11.21s)
=== RUN   TestAccAppSwaApplication_crud
--- PASS: TestAccAppSwaApplication_crud (10.16s)
=== RUN   TestAccAppThreeFieldApplication_crud
--- PASS: TestAccAppThreeFieldApplication_crud (9.80s)
=== RUN   TestAccAppUserBaseSchema_change
--- PASS: TestAccAppUserBaseSchema_change (10.33s)
=== RUN   TestAccAppUserSchemas_crud
--- PASS: TestAccAppUserSchemas_crud (11.68s)
=== RUN   TestAccOktaAppUser_crud
--- PASS: TestAccOktaAppUser_crud (20.83s)
=== RUN   TestAccOktaAppUser_retain
--- PASS: TestAccOktaAppUser_retain (10.90s)
=== RUN   TestAccOktaAuthServerClaimDefault
--- PASS: TestAccOktaAuthServerClaimDefault (5.79s)
=== RUN   TestAccOktaAuthServerClaim_create
--- PASS: TestAccOktaAuthServerClaim_create (9.62s)
=== RUN   TestAccOktaAuthServerClaim_groupType
--- PASS: TestAccOktaAuthServerClaim_groupType (5.47s)
=== RUN   TestAccOktaAuthServerPolicyRule_create
--- PASS: TestAccOktaAuthServerPolicyRule_create (11.21s)
=== RUN   TestAccOktaAuthServerPolicy_crud
--- PASS: TestAccOktaAuthServerPolicy_crud (9.81s)
=== RUN   TestAccOktaAuthServerScope_crud
--- PASS: TestAccOktaAuthServerScope_crud (9.79s)
=== RUN   TestAccOktaAuthServer_crud
--- PASS: TestAccOktaAuthServer_crud (9.47s)
=== RUN   TestAccOktaAuthServer_fullStack
--- PASS: TestAccOktaAuthServer_fullStack (12.93s)
=== RUN   TestAccOktaAuthServer_gh299
--- PASS: TestAccOktaAuthServer_gh299 (5.87s)
=== RUN   TestAccOktaAuthServerDefault_crud
--- PASS: TestAccOktaAuthServerDefault_crud (13.20s)
=== RUN   TestAccOktaEventHook_crud
--- PASS: TestAccOktaEventHook_crud (12.67s)
=== RUN   TestAccOktaGroupMembership_crud
--- PASS: TestAccOktaGroupMembership_crud (14.65s)
=== RUN   TestAccOktaGroupAdminRole_crud
    TestAccOktaGroupAdminRole_crud: resource_okta_group_role_test.go:21: Step 3/4 error: Error running apply: exit status 1

        Error: failed to remove group target from admin role assignment JBCUYUC7IRCVGS27IFCE2SKO of group 00g2u7febzGsLuG664x7: The API returned an error: Not found: Resource not found: 00g2u7dotqhVIvw044x7 (UserGroup)

          on terraform_plugin_test.tf line 22, in resource "okta_group_role" "test":
          22: resource "okta_group_role" "test" {


--- FAIL: TestAccOktaGroupAdminRole_crud (45.22s)
=== RUN   TestAccOktaGroupAdminRoles_crud
--- PASS: TestAccOktaGroupAdminRoles_crud (11.59s)
=== RUN   TestAccOktaGroupRule_crud
--- PASS: TestAccOktaGroupRule_crud (18.67s)
=== RUN   TestAccOktaGroupRule_invalidHandle
--- PASS: TestAccOktaGroupRule_invalidHandle (15.92s)
=== RUN   TestAccOktaGroups_crud
--- PASS: TestAccOktaGroups_crud (15.72s)
=== RUN   TestAccOktaIdpOidc_crud
--- PASS: TestAccOktaIdpOidc_crud (10.80s)
=== RUN   TestAccOktaIdpSaml_crud
--- PASS: TestAccOktaIdpSaml_crud (12.78s)
=== RUN   TestAccOktaIdpSocial_crud
--- PASS: TestAccOktaIdpSocial_crud (10.92s)
=== RUN   TestAccOktaInlineHook_crud
--- PASS: TestAccOktaInlineHook_crud (20.86s)
=== RUN   TestAccOktaNetworkZone_crud
--- PASS: TestAccOktaNetworkZone_crud (8.55s)
=== RUN   TestAccOktaDefaultMFAPolicy
--- PASS: TestAccOktaDefaultMFAPolicy (12.03s)
=== RUN   TestAccOktaMfaPolicy_crud
    TestAccOktaMfaPolicy_crud: resource_okta_policy_mfa_test.go:24: Step 2/2 error: Error running apply: exit status 1

        Error: failed to activate factor: The API returned an error: Api validation failed: orgAuthFactor. Causes: errorSummary: Factor not supported by the provider.

          on terraform_plugin_test.tf line 42, in resource "okta_factor" "hotp":
          42: resource "okta_factor" "hotp" {


--- FAIL: TestAccOktaMfaPolicy_crud (7.65s)
=== RUN   TestAccOktaDefaultPasswordPolicy
--- PASS: TestAccOktaDefaultPasswordPolicy (12.04s)
=== RUN   TestAccOktaPolicyPassword_crud
    TestAccOktaPolicyPassword_crud: resource_okta_policy_password_test.go:45: Step 2/2 error: Error running apply: exit status 1

        Error: failed to update password policy: The API returned an error: Api validation failed: settings.recovery.factors.okta_call.status. Causes: errorSummary: settings.recovery.factors.okta_call.status: The recovery.factors.okta_call.status property cannot be set to ACTIVE unless the PHONE_CALL_FACTOR feature is enabled.

          on terraform_plugin_test.tf line 5, in resource "okta_policy_password" "test":
           5: resource "okta_policy_password" "test" {


--- FAIL: TestAccOktaPolicyPassword_crud (7.12s)
=== RUN   TestAccOktaPolicyRuleIdpDiscovery_crud
--- PASS: TestAccOktaPolicyRuleIdpDiscovery_crud (29.51s)
=== RUN   TestAccOktaMfaPolicyRule_crud
--- PASS: TestAccOktaMfaPolicyRule_crud (9.97s)
=== RUN   TestAccOktaPolicyRulePassword_crud
--- PASS: TestAccOktaPolicyRulePassword_crud (9.96s)
=== RUN   TestAccOktaPolicyRulePassword_priorityError
--- PASS: TestAccOktaPolicyRulePassword_priorityError (3.37s)
=== RUN   TestAccOktaPolicyRulePassword_priority
--- PASS: TestAccOktaPolicyRulePassword_priority (5.83s)
=== RUN   TestAccOktaPolicyRuleSignon_defaultErrors
--- PASS: TestAccOktaPolicyRuleSignon_defaultErrors (2.20s)
=== RUN   TestAccOktaPolicyRuleSignon_crud
--- PASS: TestAccOktaPolicyRuleSignon_crud (15.67s)
=== RUN   TestAccOktaPolicySignOn_defaultError
--- PASS: TestAccOktaPolicySignOn_defaultError (2.20s)
=== RUN   TestAccOktaPolicySignOn_crud
--- PASS: TestAccOktaPolicySignOn_crud (14.85s)
=== RUN   TestAccOktaProfileMapping_crud
    TestAccOktaProfileMapping_crud: resource_okta_profile_mapping_test.go:20: Step 1/3 error: Error running apply: exit status 1

        Error: failed to find profile mapping source: The API returned an error: You do not have permission to access the feature you are requesting

          on terraform_plugin_test.tf line 68, in data "okta_user_profile_mapping_source" "user":
          68: data "okta_user_profile_mapping_source" "user" {


--- FAIL: TestAccOktaProfileMapping_crud (4.24s)
=== RUN   TestAccOktaEmailTemplate_crud
    TestAccOktaEmailTemplate_crud: resource_okta_template_email_test.go:18: Step 1/1 error: Error running apply: exit status 1

        Error: failed to create email template: The API returned an error: You do not have permission to perform the requested action

          on terraform_plugin_test.tf line 1, in resource "okta_template_email" "test":
           1: resource "okta_template_email" "test" {


--- FAIL: TestAccOktaEmailTemplate_crud (2.33s)
=== RUN   TestAccOktaSmsTemplate_crud
--- PASS: TestAccOktaSmsTemplate_crud (8.34s)
=== RUN   TestAccOktaTrustedOrigin_crud
--- PASS: TestAccOktaTrustedOrigin_crud (9.32s)
=== RUN   TestAccOktaUserBaseSchema_crud
--- PASS: TestAccOktaUserBaseSchema_crud (17.46s)
=== RUN   TestAccOktaUserBaseSchemaLogin_crud
--- PASS: TestAccOktaUserBaseSchemaLogin_crud (10.16s)
=== RUN   TestAccOktaUserGroupMemberships_crud
--- PASS: TestAccOktaUserGroupMemberships_crud (14.41s)
=== RUN   TestAccOktaUserSchema_crud
--- PASS: TestAccOktaUserSchema_crud (19.31s)
=== RUN   TestAccOktaUserSchema_arrayString
--- PASS: TestAccOktaUserSchema_arrayString (15.80s)
=== RUN   TestAccOktaUser_customProfileAttributes
--- PASS: TestAccOktaUser_customProfileAttributes (25.84s)
=== RUN   TestAccOktaUser_groupMembership
--- PASS: TestAccOktaUser_groupMembership (14.15s)
=== RUN   TestAccOktaUser_invalidCustomProfileAttribute
--- PASS: TestAccOktaUser_invalidCustomProfileAttribute (2.33s)
=== RUN   TestAccOktaUser_updateAllAttributes
--- PASS: TestAccOktaUser_updateAllAttributes (26.63s)
=== RUN   TestAccOktaUser_updateCredentials
--- PASS: TestAccOktaUser_updateCredentials (10.34s)
=== RUN   TestAccOktaUser_statusDeprovisioned
--- PASS: TestAccOktaUser_statusDeprovisioned (9.87s)
=== RUN   TestAccOktaUser_updateDeprovisioned
--- PASS: TestAccOktaUser_updateDeprovisioned (7.20s)
=== RUN   TestAccOktaUser_validRole
--- PASS: TestAccOktaUser_validRole (3.49s)
=== RUN   TestAccOktaUserType_crud
--- PASS: TestAccOktaUserType_crud (9.59s)
=== RUN   TestContainsAll
--- PASS: TestContainsAll (0.00s)
=== RUN   TestContainsOne
--- PASS: TestContainsOne (0.00s)
=== RUN   TestContains
--- PASS: TestContains (0.00s)
=== RUN   TestBuildSchema
--- PASS: TestBuildSchema (0.00s)
FAIL
FAIL    github.com/okta/terraform-provider-okta/okta    1221.890s
?       github.com/okta/terraform-provider-okta/sdk     [no test files]
FAIL
make: *** [GNUmakefile:37: testacc] Error 1

@bogdanprodan-okta
Copy link
Contributor

Hi, @ymylei! Thanks for submitting this PR. Do you think it's worth deprecating the okta_group_membership resource? We are planning to release 4.x version and remove all the deprecated resources.

@ymylei
Copy link
Contributor Author

ymylei commented Apr 9, 2021

Hi, @ymylei! Thanks for submitting this PR. Do you think it's worth deprecating the okta_group_membership resource? We are planning to release 4.x version and remove all the deprecated resources.

I don't think it would be a good idea yet, as long as this resource is able to be used at scale decently, and then we create a "reversed" version where the group is the primary key, at that point I think it would be safe to deprecate since we'd have a way to replace the relationships efficiently for either design choice.

But we should probably let this and the other "bed in" for a bit and iron out any bugs that the normal terraform testing / small scale usage will find difficult to uncover.

@ymylei
Copy link
Contributor Author

ymylei commented Apr 9, 2021

Updated acceptance test after my fiddling with the resource checks:

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

@bogdanprodan-okta bogdanprodan-okta merged commit 40f4b6f into okta:master Apr 14, 2021
@ymylei ymylei deleted the user_group_memberships branch April 15, 2021 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants