Fixing incorrect deprecation notice

[deeikele]

Fixing a deprecation notice that shows one property to be deprecated in favor of another one, while its actually the other way around.

[mbfrahry]

Hey @deeikele, it looks like this isn't quite right. We've got it in our schema that public_network_access_enabled is deprecated in favor of public_access_behind_virtual_network_enabled because public_network_access_enabled doesn't quite work.

More information can be found here

[sean-yao]

Hey @deeikele, it looks like this isn't quite right. We've got it in our schema that public_network_access_enabled is deprecated in favor of public_access_behind_virtual_network_enabled because public_network_access_enabled doesn't quite work.

More information can be found here

Which team should own what to deprecate? I thought it's service owner team. In addition, we have a property called public_network_access which is of enum type {Enabled, Disabled, PartiallyEnabled}. Translating this to a boolean value type with a name called public_network_access_enabled is very confusing in terraform. We also have a deprecating property name called allow_public_access_when_behind_virtual_network, which will be consolidated into public_network_access property per guidance from Azure Networking team. This kind of name translation should be avoided. Otherwise, it's inconsistent and very confusing to users who also use CLI for reads or updates.

[deeikele]

@mbfrahry @sean-yao and I are owning Azure ML RP that Terraform AzureRM is using. We are deprecating allow_public_access_when_behind_virtual_network in favor of public_network_access. Across AzureRM Terraform has forced a boolean type on the public_network_access property which is expecting an Enum {{Enabled, Disabled, PartiallyEnabled}, naming them public_network_access_enabled. The last option is not yet implemented for all services like Azure ML. If public_network_access is not working today, this might be related. FYI @xuzhang3.

[tombuildsstuff]

@deeikele can you provide some background on PartiallyEnabled?

[sean-yao]

PartiallyEnabled == EnabledForSelectedNetworks

On Mon, May 2, 2022 at 21:01 Tom Harvey ***@***.***> wrote: @deeikele <https://github.com/deeikele> can you provide some background on PartiallyEnabled? — Reply to this email directly, view it on GitHub <#16624 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMF6GYW5ORFUMBU2N5ZJBH3VICQKRANCNFSM5U5JTE6A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

--

[stephybun]

FYI I seem to have copied the wrong issue link in the code comment, it should be this one Azure/azure-rest-api-specs#18601

[deeikele]

• In 2022-05-01 Machine Learning API allowPublicAccessBehindVNet has been deprecated over publicNetworkAccess
• In current Terraform implementation, public_network_access_enabled provides forward compatibility to this change but in the background uses allowPublicAccessBehindVNet. This is using 2021-07-01 API which does not expose publicNetworkAccess.
• By design allowPublicAccessBehindVNet is a property that only can be set when a machine learning workspaces has a private link endpoint. This is explaining why public_network_access_enabled in the Terraform implementation, only works for private link endpoint workspaces.
• With publicNetworkAccess, this property can be set regardless of being a private link endpoint workspace.

[deeikele]

One of the problems with the current Terraform API version, is that setting publicNetworkAccess forces replacement. This is an update property on the backend machine learning API, hence should never force replacement. This should be addressed through a separate item, but at least I recommend getting this doc change out to set right expectations on deprecation.

[stephybun]

The deprecation of this property was resolved in #18279 so closing this PR for now.

[github-actions]

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.