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

Support Custom Group Profile Schema #851

Merged
merged 16 commits into from
Dec 13, 2021

Conversation

ymylei
Copy link
Contributor

@ymylei ymylei commented Dec 9, 2021

PR is designed to:

  • Re-use the design/logic for the custom_profile_attributes from okta_user and implement in okta_group

@ymylei
Copy link
Contributor Author

ymylei commented Dec 9, 2021

@bogdanprodan-okta @monde QQ for you regarding this PR. I see the version of the SDK you're currently using is some custom version of the 2.9.2 SDK, however it's missing the GroupProfileMap from the official release (https://github.com/okta/okta-sdk-golang/releases/tag/v2.9.2).

Would y'all mind weighing in on how we should proceed here? I noticed that if I manually swapped to the normal release, there were type issues and other things raised, so it's deviated further than I expected.

@bogdanprodan-okta
Copy link
Contributor

@bogdanprodan-okta @monde QQ for you regarding this PR. I see the version of the SDK you're currently using is some custom version of the 2.9.2 SDK, however it's missing the GroupProfileMap from the official release (https://github.com/okta/okta-sdk-golang/releases/tag/v2.9.2).

Would y'all mind weighing in on how we should proceed here? I noticed that if I manually swapped to the normal release, there were type issues and other things raised, so it's deviated further than I expected.

@ymylei currently, a generator that is used to generate SDK's codebase is not very flexible, so I keep some manual changes in a separate branch. This is necessary for the correct behavior of the provider. I've created PR #852 that contains all the necessary fields.

@ymylei
Copy link
Contributor Author

ymylei commented Dec 9, 2021

@bogdanprodan-okta @monde QQ for you regarding this PR. I see the version of the SDK you're currently using is some custom version of the 2.9.2 SDK, however it's missing the GroupProfileMap from the official release (https://github.com/okta/okta-sdk-golang/releases/tag/v2.9.2).
Would y'all mind weighing in on how we should proceed here? I noticed that if I manually swapped to the normal release, there were type issues and other things raised, so it's deviated further than I expected.

@ymylei currently, a generator that is used to generate SDK's codebase is not very flexible, so I keep some manual changes in a separate branch. This is necessary for the correct behavior of the provider. I've created PR #852 that contains all the necessary fields.

Cheers! Much appreciated!

@ymylei ymylei marked this pull request as ready for review December 10, 2021 03:06
@ymylei
Copy link
Contributor Author

ymylei commented Dec 10, 2021

@bogdanprodan-okta @monde Pretty sure this is ready to go, notifying you since my tests were producing some errors but passing. Based on the messages I suspect it's just weird behavior from the okta_group_schema_property & associated API.

@ymylei ymylei changed the title [WIP] Support Custom Group Profile Schema Support Custom Group Profile Schema Dec 10, 2021
@ymylei
Copy link
Contributor Author

ymylei commented Dec 10, 2021

Updated test results:

➜  terraform-provider-okta git:(group-profile) ✗ TEST_FILTER="TestAccOktaGroup_customschema" make testacc
go mod tidy
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -run TestAccOktaGroup_customschema -timeout 120m
?       github.com/okta/terraform-provider-okta [no test files]
=== RUN   TestAccOktaGroup_customschema
2021/12/09 09:13:03 [ERROR] failed to apply changes after several retries
2021/12/09 09:13:03 [ERROR] failed to apply changes after several retries
2021/12/09 09:13:41 [ERROR] failed to apply changes after several retries
2021/12/09 09:13:41 [ERROR] failed to apply changes after several retries
--- PASS: TestAccOktaGroup_customschema (43.54s)
PASS
ok      github.com/okta/terraform-provider-okta/okta    43.788s
testing: warning: no tests to run
PASS
ok      github.com/okta/terraform-provider-okta/okta/internal/apimutex  (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/okta/terraform-provider-okta/okta/internal/transport (cached) [no tests to run]
?       github.com/okta/terraform-provider-okta/sdk     [no test files]

@bogdanprodanj
Copy link
Contributor

@ymylei I would suggest adding depends_on to the okta_group resource. It will enable creating custom attributes along with okta_group. Another option would be using index values themselves in the okta_group.

resource "okta_group" "test" {
  name        = "testAcc_replace_with_uuid"
  description = "testing, testing"
  custom_profile_attributes = jsonencode({
    format("%s", okta_group_schema_property.test1.index) = "testing1234",
  })
}

resource "okta_group_schema_property" "test1" {
  index       = "testSchema1_replace_with_uuid"
  title       = "TestSchema1_replace_with_uuid"
  type        = "string"
  description = "Test string schema"
  master      = "OKTA"
}

@ymylei
Copy link
Contributor Author

ymylei commented Dec 11, 2021

@bogdanprodan-okta Good shout! I used the index values and was able to streamline the test. Updated results here:

➜  terraform-provider-okta git:(group-profile) ✗ TEST_FILTER="TestAccOktaGroup_customschema" make testacc
go mod tidy
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -run TestAccOktaGroup_customschema -timeout 120m
?       github.com/okta/terraform-provider-okta [no test files]
=== RUN   TestAccOktaGroup_customschema
2021/12/11 01:08:36 [ERROR] failed to apply changes after several retries
2021/12/11 01:08:36 [ERROR] failed to apply changes after several retries
--- PASS: TestAccOktaGroup_customschema (29.14s)
PASS
ok      github.com/okta/terraform-provider-okta/okta    29.387s
testing: warning: no tests to run
PASS
ok      github.com/okta/terraform-provider-okta/okta/internal/apimutex  (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/okta/terraform-provider-okta/okta/internal/transport (cached) [no tests to run]
?       github.com/okta/terraform-provider-okta/sdk     [no test files]

@ymylei
Copy link
Contributor Author

ymylei commented Dec 11, 2021

Results of TestAccOktaGroup_crud to confirm no breaking changes to the resource behavior:

➜  terraform-provider-okta git:(group-profile) TEST_FILTER="TestAccOktaGroup_crud" make testacc
go mod tidy
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -run TestAccOktaGroup_crud -timeout 120m
?       github.com/okta/terraform-provider-okta [no test files]
=== RUN   TestAccOktaGroup_crud
--- PASS: TestAccOktaGroup_crud (29.58s)
PASS
ok      github.com/okta/terraform-provider-okta/okta    29.824s
testing: warning: no tests to run
PASS
ok      github.com/okta/terraform-provider-okta/okta/internal/apimutex  0.191s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/okta/terraform-provider-okta/okta/internal/transport 0.135s [no tests to run]
?       github.com/okta/terraform-provider-okta/sdk     [no test files]

@bogdanprodan-okta bogdanprodan-okta added the enhancement Asking for new behavior or feature label Dec 13, 2021
@bogdanprodan-okta bogdanprodan-okta merged commit 69c7dae into okta:master Dec 13, 2021
@ymylei ymylei deleted the group-profile branch December 14, 2021 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Asking for new behavior or feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants