azurerm_ai_services - mark as deprecated for 5.0#31809
azurerm_ai_services - mark as deprecated for 5.0#31809ZiChuanBuXiu wants to merge 10 commits intohashicorp:mainfrom
azurerm_ai_services - mark as deprecated for 5.0#31809Conversation
magodo
left a comment
There was a problem hiding this comment.
Thanks for this PR!
I have left some additional mostly minor comments that once addressed this should be good to merge 👍
gerrytan
left a comment
There was a problem hiding this comment.
Thanks @ZiChuanBuXiu , I left some question and feedbacks. Please also trigger another run of acctest if you push a commit. You can run only relevant test using the tctest tool.
| | (not present) | `kind` | **Required**. Set to `"AIServices"` to match the behaviour of `azurerm_ai_services`. | | ||
| | `local_authentication_enabled` | `local_auth_enabled` | **Renamed**. Both default to `true`. | | ||
| | `public_network_access` | `public_network_access_enabled` | **Changed type**. String (`"Enabled"` / `"Disabled"`) → Boolean (`true` / `false`). Defaults to `true`. | | ||
| | (not present) | `project_management_enabled` |**Required**. Set to `true` to match the behaviour of `azurerm_ai_services`. | |
There was a problem hiding this comment.
project_management_enabled is not a required field in cognitive_account. Also in ai_services we never set any value. Are you saying the API default when it's nil is equal to true?
There was a problem hiding this comment.
It's not a "REQUIRED" field but it's required to be set to true during migration, otherwise it will force a new resource to be created based on this instruction.
I may change the wording to Mandatory to make it less confusing with the "Required" field terminology?
There was a problem hiding this comment.
To add some more context, project_management_enabled flag is optional and false by default as it's not applicable to other kinds. Users should be setting the project_management_enabled to true when kind is set to AIServices. Without it, users can't create projects within the account which defeats the purpose.
There was a problem hiding this comment.
Ah got it thanks!
Also noted we already got a CustomizeDiff check for this in cognitive_account:
internal/services/cognitive/cognitive_account_resource.go
if d.Get("project_management_enabled").(bool) {
if kind != "AIServices" {
return errors.New("`project_management_enabled` can only be set to `true` when `kind` is set to `AIServices`")
}| | `local_authentication_enabled` | `local_auth_enabled` | **Renamed**. Both default to `true`. | | ||
| | `public_network_access` | `public_network_access_enabled` | **Changed type**. String (`"Enabled"` / `"Disabled"`) → Boolean (`true` / `false`). Defaults to `true`. | | ||
| | (not present) | `project_management_enabled` |**Required**. Set to `true` to match the behaviour of `azurerm_ai_services`. | | ||
| | `customer_managed_key.managed_hsm_key_id` | (not present) | `customer_managed_key.managed_hsm_key_id` is not present in `azurerm_cognitive_account`. Try update to use `customer_managed_key.key_vault_key_id` before migration. | |
There was a problem hiding this comment.
This needs grammatical improvement
| | `customer_managed_key.managed_hsm_key_id` | (not present) | `customer_managed_key.managed_hsm_key_id` is not present in `azurerm_cognitive_account`. Try update to use `customer_managed_key.key_vault_key_id` before migration. | | |
| | `customer_managed_key.managed_hsm_key_id` | (not present) | Use `customer_managed_key.key_vault_key_id` property, it can accept both regular and HSM key id. | |
|
|
||
| ### `azurerm_ai_services` | ||
|
|
||
| * This deprecated resource has been retired and has been removed from the Azure Provider. |
There was a problem hiding this comment.
Please use the wording format for superseded removal
| * This deprecated resource has been retired and has been removed from the Azure Provider. | |
| * This deprecated resource has been superseded by `azurerm_cognitive_account` and has been removed from the Azure Provider. |
|
|
||
| Manages an AI Services Account. | ||
|
|
||
| ~> **Note:** The `azurerm_ai_services` resource has been deprecated and will be removed in v5.0 of the AzureRM Provider. Please use [`azurerm_cognitive_account`](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/cognitive_account) resource instead. |
There was a problem hiding this comment.
Do not hardcode full URI, use relative link instead
| ~> **Note:** The `azurerm_ai_services` resource has been deprecated and will be removed in v5.0 of the AzureRM Provider. Please use [`azurerm_cognitive_account`](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/cognitive_account) resource instead. | |
| ~> **Note:** The `azurerm_ai_services` resource has been deprecated and will be removed in v5.0 of the AzureRM Provider. Please use [`azurerm_cognitive_account`](cognitive_account.html.markdown) resource instead. |
gerrytan
left a comment
There was a problem hiding this comment.
I have triggered a fresh set of acctests:
- https://hashicorp.teamcity.com/viewQueued.html?itemId=624587
- https://hashicorp.teamcity.com/viewQueued.html?itemId=624586
And have pinged @magodo to run the GH workflow.
Assuming all the result is good, this PR is good to go. Nice work @ZiChuanBuXiu !
| | (not present) | `kind` | **Required**. Set to `"AIServices"` to match the behaviour of `azurerm_ai_services`. | | ||
| | `local_authentication_enabled` | `local_auth_enabled` | **Renamed**. Both default to `true`. | | ||
| | `public_network_access` | `public_network_access_enabled` | **Changed type**. String (`"Enabled"` / `"Disabled"`) → Boolean (`true` / `false`). Defaults to `true`. | | ||
| | (not present) | `project_management_enabled` |**Required**. Set to `true` to match the behaviour of `azurerm_ai_services`. | |
There was a problem hiding this comment.
Ah got it thanks!
Also noted we already got a CustomizeDiff check for this in cognitive_account:
internal/services/cognitive/cognitive_account_resource.go
if d.Get("project_management_enabled").(bool) {
if kind != "AIServices" {
return errors.New("`project_management_enabled` can only be set to `true` when `kind` is set to `AIServices`")
}
Community Note
Description
Mark
azurerm_ai_servicesas deprecated for 5.0 as it's replaced byazurerm_cognitive_accountazurerm_cognitive_accountasDeprecatedInFavourOfResourceforazurerm_ai_servicesazurerm_ai_servicesacceptance testsazurerm_ai_servicesai_services.html.markdownto indicate deprecation5.0-upgrade-guide.html.markdownPR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Acceptance test for Cognitive AI Services
Acceptance test for AI Foundry
Verification test for deprecation warning
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_ai_services- Deprecate for 5.0 in favour ofazurerm_cognitive_account[GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #0000
AI Assistance Disclosure
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.