azurerm_video_indexer_account - support for the public_network_access property#29725
Conversation
sreallymatt
left a comment
There was a problem hiding this comment.
Hi @sinbai, could you rename public_network_access_enabled to public_network_access and make it of type String? I've left some more context inline, once that's done I'll give this a more thorough review, thanks!
|
|
||
| "identity": commonschema.SystemAssignedUserAssignedIdentityRequired(), | ||
|
|
||
| "public_network_access_enabled": { |
There was a problem hiding this comment.
While we've previously exposed the public network access enum as public_network_access_enabled (boolean), we're seeing more of these in the SDK with states other than Enabled and Disabled. Because of this, we'd prefer to standardize and expose these as public_network_access (string) moving forward to avoid potentially having to introduce a breaking change in the future.
There was a problem hiding this comment.
Hi @sreallymatt thanks for your time. I have fixed the comment, would you mind take another look?
1dc3064 to
e7a40f2
Compare
e7a40f2 to
b8a3aa0
Compare
b8a3aa0 to
a1a4e23
Compare
a1a4e23 to
da7395d
Compare
da7395d to
266f4f3
Compare
sreallymatt
left a comment
There was a problem hiding this comment.
Thanks for making those changes @sinbai, this looks good, I've just left one question inline.
| Type: pluginsdk.TypeString, | ||
| Optional: true, | ||
| Default: string(accounts.PublicNetworkAccessEnabled), | ||
| ValidateFunc: validation.StringInSlice(accounts.PossibleValuesForPublicNetworkAccess(), true), |
There was a problem hiding this comment.
Was there a specific reason for validating this insensitively? We'd prefer to use case-sensitive forms where possible
| ValidateFunc: validation.StringInSlice(accounts.PossibleValuesForPublicNetworkAccess(), true), | |
| ValidateFunc: validation.StringInSlice(accounts.PossibleValuesForPublicNetworkAccess(), false), |
Hi @sreallymatt thanks for pointing that out - It's been fixed. Would you mind take another look? |
sreallymatt
left a comment
There was a problem hiding this comment.
Thanks @sinbai, LGTM ✅
azurerm_video_indexer_account - support for the public_network_access_enabled propertyazurerm_video_indexer_account - support for the public_network_access property
|
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
Support for the
public_network_access_enabledproperty.Swagger:https://github.com/Azure/azure-rest-api-specs/blob/c9c3e8b9ec547d82c487f36f1126228f9eef0e79/specification/vi/resource-manager/Microsoft.VideoIndexer/stable/2025-04-01/vi.json#L1049C10-L1049C29
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_resource- support for thething1property [azurerm_video_indexer_account - public_network_access attribute is not available #29618]This is a (please select all that apply):
Related Issue(s)
Fixes #29618
Note
If this PR changes meaningfully during the course of review please update the title and description as required.