azurerm_kusto_attached_database_configuration - Add support for database_name_override, database_name_prefix, functions_to_exclude, and functions_to_include properties#31470
Conversation
… database configuration
azurerm_kusto_attached_database_configuration - support for database related propertiesazurerm_kusto_attached_database_configuration - Add support for database_name_override, database_name_prefix, functions_to_exclude, and functions_to_include properties
… database configuration
… database configuration
internal/services/kusto/kusto_attached_database_configuration_resource.go
Show resolved
Hide resolved
|
Thanks @liuwuliuyun, just one minor question, mostly LGTM |
magodo
left a comment
There was a problem hiding this comment.
Thanks for this PR!
I've taken a look through and left some comments inline, but this is mostly looking good to me 👍
website/docs/r/kusto_attached_database_configuration.html.markdown
Outdated
Show resolved
Hide resolved
internal/services/kusto/kusto_attached_database_configuration_resource.go
Outdated
Show resolved
Hide resolved
website/docs/r/kusto_attached_database_configuration.html.markdown
Outdated
Show resolved
Hide resolved
…down Co-authored-by: magodo <wztdyl@sina.com>
…down Co-authored-by: magodo <wztdyl@sina.com>
…hed database read function
…rraform-provider-azurerm into kusto-attached-db
| "database_name_override": { | ||
| Type: pluginsdk.TypeString, | ||
| Optional: true, | ||
| ValidateFunc: validation.StringIsNotEmpty, |
There was a problem hiding this comment.
My assumption is this will have similar limitations as the database_name field (minus the asterisk * as a valid option), validation should be improved. Please confirm and update.
As an example, there definitely are characters that are not supported:
│ Status: "Failed"
│ Code: "BadRequest_EntityNameIsNotValid"
│ Message: "[BadRequest] 'myspecialName@!@#%^^&': 'Name' is not valid for entities of kind 'DatabaseName'. Reason='DatabaseName: Name='myspecialName@!@#%^^&' does not comply with naming rules (contains invalid characters or format mismatch)'"
There was a problem hiding this comment.
Good point, will add validation for database name and prefix
| "database_name_prefix": { | ||
| Type: pluginsdk.TypeString, | ||
| Optional: true, | ||
| ValidateFunc: validation.StringIsNotEmpty, |
There was a problem hiding this comment.
Similar to my comment above, this needs better validation
│ Status: "Failed"
│ Code: "BadRequest_EntityNameIsNotValid"
│ Message: "[BadRequest] 'myspecialPrefix@!@#%^^&acctestkd2-0217': 'Name' is not valid for entities of kind 'DatabaseName'. Reason='DatabaseName: Name='myspecialPrefix@!@#%^^&acctestkd2-0217' does not comply with naming rules (contains invalid characters or format mismatch)'"
internal/services/kusto/kusto_attached_database_configuration_resource_test.go
Show resolved
Hide resolved
internal/services/kusto/kusto_attached_database_configuration_resource.go
Show resolved
Hide resolved
internal/services/kusto/kusto_attached_database_configuration_resource_test.go
Show resolved
Hide resolved
| features {} | ||
| features { | ||
| resource_group { | ||
| prevent_deletion_if_contains_resources = false |
There was a problem hiding this comment.
This process is a bit complex. Creating and deleting a Kusto cluster and its database can take over two hours. Even after the resource is deleted, it may still appear in the resource group list due to a cache consistency issue in ARM, which can cause the process to timeout repeatedly. If you want the acctest to run until completion, you’ll need to add this flag.
There was a problem hiding this comment.
I will try to remove this and run tests locally to see if it works.
| resource "azurerm_resource_group" "rg" { | ||
| name = "acctestRG-%d" | ||
| location = "%s" | ||
| } | ||
|
|
||
| resource "azurerm_kusto_cluster" "cluster1" { | ||
| name = "acctestkc1%s" | ||
| location = azurerm_resource_group.rg.location | ||
| resource_group_name = azurerm_resource_group.rg.name | ||
|
|
||
| sku { | ||
| name = "Dev(No SLA)_Standard_D11_v2" | ||
| capacity = 1 | ||
| } | ||
| } | ||
|
|
||
| resource "azurerm_kusto_cluster" "cluster2" { | ||
| name = "acctestkc2%s" | ||
| location = azurerm_resource_group.rg.location | ||
| resource_group_name = azurerm_resource_group.rg.name | ||
|
|
||
| sku { | ||
| name = "Dev(No SLA)_Standard_D11_v2" | ||
| capacity = 1 | ||
| } | ||
| } | ||
|
|
||
| resource "azurerm_kusto_database" "followed_database" { | ||
| name = "acctestkd-%d" | ||
| resource_group_name = azurerm_resource_group.rg.name | ||
| location = azurerm_resource_group.rg.location | ||
| cluster_name = azurerm_kusto_cluster.cluster1.name | ||
| } | ||
|
|
||
| resource "azurerm_kusto_database" "test" { | ||
| name = "acctestkd2-%d" | ||
| resource_group_name = azurerm_resource_group.rg.name | ||
| location = azurerm_resource_group.rg.location | ||
| cluster_name = azurerm_kusto_cluster.cluster2.name | ||
| } |
There was a problem hiding this comment.
Can you add a template() function that contains all of these dependencies so there's no need to copy-paste the same config
| provider "azurerm" { | ||
| features { | ||
| resource_group { | ||
| prevent_deletion_if_contains_resources = false |
There was a problem hiding this comment.
Same as above, why is this needed?
…ks in Kusto attached database configuration
…tabase_name is '*'
|
Latest testing results: |
|
Hi @sreallymatt , I've addressed all the review feedback. Here's a summary of the changes:
All tests pass locally. Could you please take another look when you get a chance? Thanks! |
sreallymatt
left a comment
There was a problem hiding this comment.
Thanks @liuwuliuyun - LGTM ✅
Community Note
Description
This PR adds support for the following new properties to the
azurerm_kusto_attached_database_configurationresource:Top-level properties:
database_name_override- Overrides the original database name. Relevant only when attaching to a specific database.database_name_prefix- Adds a prefix to the attached databases name. When following an entire cluster, that prefix would be added to all of the databases' original names from the leader cluster.Properties in the
sharingblock:functions_to_exclude- List of functions to exclude from the follower database.functions_to_include- List of functions to include in the follower database.These properties are already supported by the Azure API (2024-04-13) and the underlying SDK but were not exposed in the Terraform resource.
PR Checklist
For example: "
resource_name_here- description of change e.g. adding propertynew_property_name_here"Changes to existing Resource / Data Source
Testing
New Test Cases Added:
TestAccKustoAttachedDatabaseConfiguration_databaseNameOverride- Tests thedatabase_name_overridepropertyTestAccKustoAttachedDatabaseConfiguration_databaseNamePrefix- Tests thedatabase_name_prefixproperty withdatabase_name = "*"Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_kusto_attached_database_configuration- Add support fordatabase_name_override,database_name_prefix,functions_to_exclude, andfunctions_to_includeproperties [azurerm_kusto_attached_database_configuration- Add support fordatabase_name_override,database_name_prefix,functions_to_exclude, andfunctions_to_includeproperties #31470]This is a (please select all that apply):
Related Issue(s)
AI Assistance Disclosure
This PR was created with the assistance of GitHub Copilot for code generation, test cases, and documentation updates.
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
No changes to security controls in this pull request.