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

Add sso_team_id to the tfe_team resource and data source #457

Merged
merged 3 commits into from
Apr 19, 2022

Conversation

JarrettSpiker
Copy link
Contributor

@JarrettSpiker JarrettSpiker commented Mar 29, 2022

Description

Add sso_team_id to the tfe_team resource and data source

Testing plan

  1. Create a team using the tfe provider
  2. Add, change, and remove an SSO team ID for that team, through the provider
  3. Read the SSO Team ID from a data source referencing said team

Output from acceptance tests

❯ TF_ACC=1 go test github.com/hashicorp/terraform-provider-tfe/tfe -run TestAccTFETeam_ -run TFETeamDataSource_ -v
=== RUN   TestAccTFETeamDataSource_basic
--- PASS: TestAccTFETeamDataSource_basic (8.00s)
=== RUN   TestAccTFETeamDataSource_ssoTeamId
--- PASS: TestAccTFETeamDataSource_ssoTeamId (7.99s)
PASS
ok      github.com/hashicorp/terraform-provider-tfe/tfe 16.222s

❯ TF_ACC=1 go test github.com/hashicorp/terraform-provider-tfe/tfe -run TestAccTFETeam_ -v                        
=== RUN   TestAccTFETeam_basic
--- PASS: TestAccTFETeam_basic (8.29s)
=== RUN   TestAccTFETeam_full
--- PASS: TestAccTFETeam_full (7.87s)
=== RUN   TestAccTFETeam_full_update
--- PASS: TestAccTFETeam_full_update (17.70s)
=== RUN   TestAccTFETeam_import
--- PASS: TestAccTFETeam_import (7.74s)
PASS
ok      github.com/hashicorp/terraform-provider-tfe/tfe 
...

@JarrettSpiker JarrettSpiker marked this pull request as draft March 29, 2022 15:30
@JarrettSpiker JarrettSpiker marked this pull request as ready for review March 29, 2022 19:26
@JarrettSpiker JarrettSpiker changed the title Draft: Add sso_team_id to the tfe_team resource and data source Add sso_team_id to the tfe_team resource and data source Mar 29, 2022
@JarrettSpiker JarrettSpiker requested a review from a team March 29, 2022 19:39
if v, ok := d.GetOk("sso_team_id"); ok {
options.SSOTeamID = tfe.String(v.(string))
} else {
options.SSOTeamID = tfe.String("")
Copy link

@mpminardi mpminardi Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the else case you mentioned that can be removed once this PR is through right? (This is more just out of my own curiosity / to reload context and not at all blocking).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yah, that is right

@@ -41,6 +41,7 @@ The following arguments are supported:
* `organization` - (Required) Name of the organization.
* `visibility` - (Optional) The visibility of the team ("secret" or "organization"). Defaults to "secret".
* `organization_access` - (Optional) Settings for the team's [organization access](https://www.terraform.io/docs/cloud/users-teams-organizations/permissions.html#organization-level-permissions).
* `sso_team_id` - (Optional) Unique Identifier to control team membership via SAML. Defaults to `null`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we link to the docs for the organization_access attribute above. Should we be doing the same for the SSO team ID docs here and in the datasource docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yah, good idea, updated

Copy link

@mpminardi mpminardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Would probably be good to get someone from the provider team to give the go-ahead here as well though

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