azurerm_dev_center_project_pool - support new property for single_sign_on_enabled#30440
Conversation
sreallymatt
left a comment
There was a problem hiding this comment.
Thanks @neil-yechenwei, couple minor comments, could you add this to the data source as well to keep the resource and data source in sync?
|
|
||
| "single_sign_on_enabled": { | ||
| Type: pluginsdk.TypeBool, | ||
| Optional: true, |
There was a problem hiding this comment.
Would it make sense to give this a default of false given that's what's happening in the code? (i.e. Disabled by default)
Functionally it doesn't make a difference, but makes it easier to tell what the default is by glancing at the schema
| if model.SingleSignOnEnabled { | ||
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusEnabled) | ||
| } else { | ||
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusDisabled) | ||
| } |
There was a problem hiding this comment.
For consistency, usually we simply set the default then conditionally update it rather than including an else statement
| if model.SingleSignOnEnabled { | |
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusEnabled) | |
| } else { | |
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusDisabled) | |
| } | |
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusDisabled) | |
| if model.SingleSignOnEnabled { | |
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusEnabled) | |
| } |
| if model.SingleSignOnEnabled { | ||
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusEnabled) | ||
| } else { | ||
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusDisabled) | ||
| } |
There was a problem hiding this comment.
Same as above
| if model.SingleSignOnEnabled { | |
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusEnabled) | |
| } else { | |
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusDisabled) | |
| } | |
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusDisabled) | |
| if model.SingleSignOnEnabled { | |
| parameters.Properties.SingleSignOnStatus = pointer.To(pools.SingleSignOnStatusEnabled) | |
| } |
|
@sreallymatt , thanks for the comments. I updated PR. Please take another look. |
sreallymatt
left a comment
There was a problem hiding this comment.
Thanks @neil-yechenwei LGTM ✅
|
Test results: |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Community Note
Description
This PR is to support new property SingleSignOnStatus.
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Note: The failed test cases also failed on Teamcity Daily Run due to quota issue so that they are not related with this PR.

Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_dev_center_project_pool- support new property forsingle_sign_on_enabledThis is a (please select all that apply):
Related Issue(s)
Fixes #30410
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.