Skip to content
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

Support for spatial_index in azurerm_cosmosdb_sql_container #11625

Merged
merged 11 commits into from
Jun 11, 2021
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions azurerm/internal/services/cosmos/common/indexing_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,30 @@ func expandAzureRmCosmosDBIndexingPolicyCompositeIndexes(input []interface{}) *[
return &indexes
}

func expandAzureRmCosmosDBIndexingPolicySpatialIndexes(input []interface{}) *[]documentdb.SpatialSpec {
if len(input) == 0 || input[0] == nil {
return nil
}
indexes := make([]documentdb.SpatialSpec, 0)
// no matter what spatial types are updated, all types will be set and returned from service
spatialTypes := []documentdb.SpatialType{
documentdb.SpatialTypeLineString,
documentdb.SpatialTypeMultiPolygon,
documentdb.SpatialTypePoint,
documentdb.SpatialTypePolygon,
}

for _, i := range input {
indexPair := i.(map[string]interface{})
indexes = append(indexes, documentdb.SpatialSpec{
Types: &spatialTypes,
Path: utils.String(indexPair["path"].(string)),
})
}
Comment on lines +75 to +88
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we expose this in schema instead of applying each spatial index to all types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The service will return all types no matter what types we try to update. But at least one type is necessary to be set, because missing types will lead to bad request.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure why that doesn't mean we shouldn't expose the four types for people to individually set?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It means, for example, we update types = ["String"], and the service returns ["String", "MultiPolygon", "Point", "Polygon"], shall we ignore the difference for the customers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It means that the service has set all types for you. The service team has confirmed that it's by design. I'd like to suggest not to expose types to the clients?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's by design by service. Here is the service's reply:
https://gist.github.com/yupwei68/6c3ecffe6aeec33d945210e70bb3513a#gistcomment-3735507

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my point is would someone ever want to set a different spatial index path for different types? it type1 is path1 and type2 is path2 ect or will there always only ever be one index path for all types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There will always be index paths for all types, according to the service team. The user can't specify the specific types for their paths.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so to be clear, there is a single path that is shared by all types regardless and its not possible to assign different paths to different types? i assume the portal only has a "path" property then and hides away types from the user? if this is all the case maybe it make sense to document htis in the property so users know the path is applied to all types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The portal has not supported this yet. I have updated the document and I have marked the types computed.


return &indexes
}

func ExpandAzureRmCosmosDbIndexingPolicy(d *pluginsdk.ResourceData) *documentdb.IndexingPolicy {
i := d.Get("indexing_policy").([]interface{})

Expand All @@ -85,6 +109,9 @@ func ExpandAzureRmCosmosDbIndexingPolicy(d *pluginsdk.ResourceData) *documentdb.
if v, ok := input["composite_index"].([]interface{}); ok {
policy.CompositeIndexes = expandAzureRmCosmosDBIndexingPolicyCompositeIndexes(v)
}

policy.SpatialIndexes = expandAzureRmCosmosDBIndexingPolicySpatialIndexes(input["spatial_index"].([]interface{}))

return policy
}

Expand Down Expand Up @@ -163,6 +190,41 @@ func flattenCosmosDBIndexingPolicyIncludedPaths(input *[]documentdb.IncludedPath
return includedPaths
}

func flattenCosmosDBIndexingPolicySpatialIndexes(input *[]documentdb.SpatialSpec) []interface{} {
if input == nil {
return []interface{}{}
}

indexes := make([]interface{}, 0)

for _, v := range *input {
var path string
if v.Path != nil {
path = *v.Path
}
indexes = append(indexes, map[string]interface{}{
"path": path,
"types": flattenCosmosDBIndexingPolicySpatialIndexesTypes(v.Types),
})
}

return indexes
}

func flattenCosmosDBIndexingPolicySpatialIndexesTypes(input *[]documentdb.SpatialType) []interface{} {
if input == nil {
return nil
}

types := make([]interface{}, 0)

for _, v := range *input {
types = append(types, string(v))
}

return types
}

func FlattenAzureRmCosmosDbIndexingPolicy(indexingPolicy *documentdb.IndexingPolicy) []interface{} {
results := make([]interface{}, 0)
if indexingPolicy == nil {
Expand All @@ -174,6 +236,7 @@ func FlattenAzureRmCosmosDbIndexingPolicy(indexingPolicy *documentdb.IndexingPol
result["included_path"] = flattenCosmosDBIndexingPolicyIncludedPaths(indexingPolicy.IncludedPaths)
result["excluded_path"] = flattenCosmosDBIndexingPolicyExcludedPaths(indexingPolicy.ExcludedPaths)
result["composite_index"] = flattenCosmosDBIndexingPolicyCompositeIndexes(indexingPolicy.CompositeIndexes)
result["spatial_index"] = flattenCosmosDBIndexingPolicySpatialIndexes(indexingPolicy.SpatialIndexes)

results = append(results, result)
return results
Expand Down
22 changes: 22 additions & 0 deletions azurerm/internal/services/cosmos/common/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,28 @@ func CosmosDbIndexingPolicySchema() *pluginsdk.Schema {
},
},
},

"spatial_index": {
Type: pluginsdk.TypeList,
Optional: true,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"path": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
},

"types": {
Type: pluginsdk.TypeSet,
Computed: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
},
},
},
},
},
},
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,21 @@ func TestAccCosmosDbSqlContainer_indexing_policy(t *testing.T) {
check.That(data.ResourceName).ExistsInAzure(r),
),
},
{

Config: r.indexing_policy_update_spatialIndex(data, "/includedPath02/*", "/excludedPath02/?"),
Check: acceptance.ComposeAggregateTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
{

Config: r.basic(data),
Check: acceptance.ComposeAggregateTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}
Expand Down Expand Up @@ -450,6 +465,66 @@ resource "azurerm_cosmosdb_sql_container" "test" {
`, CosmosSqlDatabaseResource{}.basic(data), data.RandomInteger, includedPath, excludedPath)
}

func (CosmosSqlContainerResource) indexing_policy_update_spatialIndex(data acceptance.TestData, includedPath, excludedPath string) string {
return fmt.Sprintf(`
%[1]s

resource "azurerm_cosmosdb_sql_container" "test" {
name = "acctest-CSQLC-%[2]d"
resource_group_name = azurerm_cosmosdb_account.test.resource_group_name
account_name = azurerm_cosmosdb_account.test.name
database_name = azurerm_cosmosdb_sql_database.test.name
partition_key_path = "/definition/id"

indexing_policy {
indexing_mode = "Consistent"

included_path {
path = "/*"
}

included_path {
path = "%s"
}

excluded_path {
path = "%s"
}

composite_index {
index {
path = "/path1"
order = "Ascending"
}
index {
path = "/path2"
order = "Descending"
}
}

composite_index {
index {
path = "/path3"
order = "Ascending"
}
index {
path = "/path4"
order = "Descending"
}
}

spatial_index {
path = "/path/*"
}

spatial_index {
path = "/test/to/all/?"
}
}
}
`, CosmosSqlDatabaseResource{}.basic(data), data.RandomInteger, includedPath, excludedPath)
}

func (CosmosSqlContainerResource) partition_key_version(data acceptance.TestData, version int) string {
return fmt.Sprintf(`
%[1]s
Expand Down
23 changes: 23 additions & 0 deletions website/docs/r/cosmosdb_sql_container.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ A `unique_key` block supports the following:

* `paths` - (Required) A list of paths to use for this unique key.

---
An `indexing_policy` block supports the following:

* `indexing_mode` - (Optional) Indicates the indexing mode. Possible values include: `Consistent` and `None`. Defaults to `Consistent`.
Expand All @@ -97,18 +98,34 @@ An `indexing_policy` block supports the following:

* `composite_index` - (Optional) One or more `composite_index` blocks as defined below.

* `spatial_index` - (Optional) One or more `spatial_index` blocks as defined below.

---

A `spatial_index` block supports the following:

* `path` - (Required) Path for which the indexing behaviour applies to. According to the service design, all spatial types including `LineString`, `MultiPolygon`, `Point`, and `Polygon` will be applied to the path.

---

An `included_path` block supports the following:

* `path` - Path for which the indexing behaviour applies to.

---

An `excluded_path` block supports the following:

* `path` - Path that is excluded from indexing.

---

A `composite_index` block supports the following:

* `index` - One or more `index` blocks as defined below.

---

An `index` block supports the following:

* `path` - Path for which the indexing behaviour applies to.
Expand All @@ -131,6 +148,12 @@ The following attributes are exported:

* `id` - The ID of the CosmosDB SQL Container.

---

A `spatial_index` block exports the following:

* `types` - A set of spatial types of the path.

## Timeouts

The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/docs/configuration/resources.html#timeouts) for certain actions:
Expand Down