-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve 4.0 TODOs Kusto #28530
base: main
Are you sure you want to change the base?
Resolve 4.0 TODOs Kusto #28530
Changes from 30 commits
9dc4b66
154d6f3
b2790d3
526fc6a
c173974
46ea78e
a916c7a
b998109
c7685b0
c28adb7
af70f5b
7959930
a0947ee
28d1212
1542311
6518a8c
e506264
e02c82c
f2ce1b6
a4e0cca
e6ff5a7
7e2f085
0b0ba06
86b38f8
b34e6f1
9fa73be
2d8615e
d8acb96
34b378a
09b2b79
66898e1
c3221c9
6c1245d
4b5487b
623a4d3
51f0dee
1385604
a64811d
2e88d79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,8 @@ import ( | |
"fmt" | ||
"testing" | ||
|
||
"github.com/hashicorp/terraform-provider-azurerm/internal/features" | ||
|
||
wyattfry marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"github.com/hashicorp/go-azure-sdk/resource-manager/kusto/2023-08-15/attacheddatabaseconfigurations" | ||
"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" | ||
"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check" | ||
|
@@ -32,6 +34,24 @@ func TestAccKustoAttachedDatabaseConfiguration_basic(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestAccKustoAttachedDatabaseConfiguration_clusterResourceId(t *testing.T) { | ||
if features.FivePointOh() { | ||
t.Skip() | ||
} | ||
data := acceptance.BuildTestData(t, "azurerm_kusto_attached_database_configuration", "test") | ||
r := KustoAttachedDatabaseConfigurationResource{} | ||
|
||
data.ResourceTest(t, r, []acceptance.TestStep{ | ||
{ | ||
Config: r.clusterResourceId(data), | ||
Check: acceptance.ComposeTestCheckFunc( | ||
check.That(data.ResourceName).ExistsInAzure(r), | ||
), | ||
}, | ||
data.ImportStep(), | ||
}) | ||
} | ||
|
||
func (KustoAttachedDatabaseConfigurationResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { | ||
id, err := attacheddatabaseconfigurations.ParseAttachedDatabaseConfigurationID(state.ID) | ||
if err != nil { | ||
|
@@ -104,7 +124,74 @@ resource "azurerm_kusto_attached_database_configuration" "test" { | |
resource_group_name = azurerm_resource_group.rg.name | ||
location = azurerm_resource_group.rg.location | ||
cluster_name = azurerm_kusto_cluster.cluster1.name | ||
cluster_resource_id = azurerm_kusto_cluster.cluster2.id | ||
cluster_id = azurerm_kusto_cluster.cluster2.id | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to ensure that we haven't introduced any regressions to how the deprecated property behaves now and should still validate that is works as expected until it's removed in the next major version, so we might want to add another test and test configuration just for the deprecated property. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Roger. Added a test |
||
database_name = azurerm_kusto_database.test.name | ||
|
||
sharing { | ||
external_tables_to_exclude = ["ExternalTable2"] | ||
external_tables_to_include = ["ExternalTable1"] | ||
materialized_views_to_exclude = ["MaterializedViewTable2"] | ||
materialized_views_to_include = ["MaterializedViewTable1"] | ||
tables_to_exclude = ["Table2"] | ||
tables_to_include = ["Table1"] | ||
} | ||
} | ||
`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString, data.RandomInteger, data.RandomInteger, data.RandomInteger) | ||
} | ||
|
||
func (KustoAttachedDatabaseConfigurationResource) clusterResourceId(data acceptance.TestData) string { | ||
return fmt.Sprintf(` | ||
provider "azurerm" { | ||
features {} | ||
} | ||
|
||
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 | ||
} | ||
|
||
resource "azurerm_kusto_attached_database_configuration" "test" { | ||
name = "acctestka-%d" | ||
resource_group_name = azurerm_resource_group.rg.name | ||
location = azurerm_resource_group.rg.location | ||
cluster_name = azurerm_kusto_cluster.cluster1.name | ||
cluster_resource_id = azurerm_kusto_cluster.cluster2.id ### <-- Testing this deprecated property | ||
database_name = azurerm_kusto_database.test.name | ||
|
||
sharing { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to what Steph said here, we'll have to remove
ForceNew
here as well for when people move fromcluster_resource_id
tocluster_id
ascluster_resource_id
will have to be removed which will causeForceNew
to triggerThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah i see, thanks!