Skip to content

Validate that mappings are a JSON object, not just valid json #719

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

Merged
merged 4 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## [Unreleased]
- Add support for Kibana synthetics http and tcp monitors ([#699](https://github.com/elastic/terraform-provider-elasticstack/pull/699))

- Improve validation for index settings and mappings ([#719](https://github.com/elastic/terraform-provider-elasticstack/pull/719))
- Add support for Kibana synthetics http and tcp monitors ([#699](https://github.com/elastic/terraform-provider-elasticstack/pull/699))
- Add `elasticstack_kibana_spaces` data source ([#682](https://github.com/elastic/terraform-provider-elasticstack/pull/682))

## [0.11.5] - 2024-08-12
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/elasticsearch_component_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ resource "elasticstack_elasticsearch_index_template" "my_template" {
Optional:

- `alias` (Block Set) Alias to add. (see [below for nested schema](#nestedblock--template--alias))
- `mappings` (String) Mapping for fields in the index.
- `mappings` (String) Mapping for fields in the index. Should be specified as a JSON object of field mappings. See the documentation (https://www.elastic.co/guide/en/elasticsearch/reference/current/explicit-mapping.html) for more details
- `settings` (String) Configuration options for the index. See, https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#index-modules-settings

<a id="nestedblock--template--alias"></a>
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/elasticsearch_index_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Optional:
Optional:

- `alias` (Block Set) Alias to add. (see [below for nested schema](#nestedblock--template--alias))
- `mappings` (String) Mapping for fields in the index.
- `mappings` (String) Mapping for fields in the index. Should be specified as a JSON object of field mappings. See the documentation (https://www.elastic.co/guide/en/elasticsearch/reference/current/explicit-mapping.html) for more details
- `settings` (String) Configuration options for the index. See, https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#index-modules-settings

<a id="nestedblock--template--alias"></a>
Expand Down
10 changes: 7 additions & 3 deletions internal/elasticsearch/index/component_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,22 @@ func ResourceComponentTemplate() *schema.Resource {
},
},
"mappings": {
Description: "Mapping for fields in the index.",
Description: "Mapping for fields in the index. Should be specified as a JSON object of field mappings. See the documentation (https://www.elastic.co/guide/en/elasticsearch/reference/current/explicit-mapping.html) for more details",
Type: schema.TypeString,
Optional: true,
DiffSuppressFunc: utils.DiffJsonSuppress,
ValidateFunc: validation.StringIsJSON,
ValidateFunc: validation.All(
validation.StringIsJSON, stringIsJSONObject,
),
},
"settings": {
Description: "Configuration options for the index. See, https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#index-modules-settings",
Type: schema.TypeString,
Optional: true,
DiffSuppressFunc: utils.DiffIndexSettingSuppress,
ValidateFunc: validation.StringIsJSON,
ValidateFunc: validation.All(
validation.StringIsJSON, stringIsJSONObject,
),
},
},
},
Expand Down
6 changes: 4 additions & 2 deletions internal/elasticsearch/index/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,8 +489,10 @@ If specified, this mapping can include: field names, [field data types](https://
Type: schema.TypeString,
Optional: true,
DiffSuppressFunc: utils.DiffJsonSuppress,
ValidateFunc: validation.StringIsJSON,
Default: "{}",
ValidateFunc: validation.All(
validation.StringIsJSON, stringIsJSONObject,
),
Default: "{}",
},
// Deprecated: individual setting field should be used instead
"settings": {
Expand Down
10 changes: 7 additions & 3 deletions internal/elasticsearch/index/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,18 +140,22 @@ func ResourceTemplate() *schema.Resource {
},
},
"mappings": {
Description: "Mapping for fields in the index.",
Description: "Mapping for fields in the index. Should be specified as a JSON object of field mappings. See the documentation (https://www.elastic.co/guide/en/elasticsearch/reference/current/explicit-mapping.html) for more details",
Type: schema.TypeString,
Optional: true,
DiffSuppressFunc: utils.DiffJsonSuppress,
ValidateFunc: validation.StringIsJSON,
ValidateFunc: validation.All(
validation.StringIsJSON, stringIsJSONObject,
),
},
"settings": {
Description: "Configuration options for the index. See, https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#index-modules-settings",
Type: schema.TypeString,
Optional: true,
DiffSuppressFunc: utils.DiffIndexSettingSuppress,
ValidateFunc: validation.StringIsJSON,
ValidateFunc: validation.All(
validation.StringIsJSON, stringIsJSONObject,
),
},
},
},
Expand Down
22 changes: 22 additions & 0 deletions internal/elasticsearch/index/validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package index

import (
"encoding/json"
"fmt"
)

func stringIsJSONObject(i interface{}, s string) (warnings []string, errors []error) {
iStr, ok := i.(string)
if !ok {
errors = append(errors, fmt.Errorf("expected type of %s to be string", s))
return warnings, errors
}

m := map[string]interface{}{}
if err := json.Unmarshal([]byte(iStr), &m); err != nil {
errors = append(errors, fmt.Errorf("expected %s to be a JSON object. Check the documentation for the expected format. %w", s, err))
return
}

return
}
51 changes: 51 additions & 0 deletions internal/elasticsearch/index/validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package index

import (
"testing"

"github.com/stretchr/testify/require"
)

func Test_stringIsJSONObject(t *testing.T) {
tests := []struct {
name string
fieldVal interface{}
expectedErrsToContain []string
}{
{
name: "should not return an error for a valid json object",
fieldVal: "{}",
},
{
name: "should not return an error for a null",
fieldVal: "null",
},

{
name: "should return an error if the field is not a string",
fieldVal: true,
expectedErrsToContain: []string{
"expected type of field-name to be string",
},
},
{
name: "should return an error if the field is valid json, but not an object",
fieldVal: "[]",
expectedErrsToContain: []string{
"expected field-name to be a JSON object. Check the documentation for the expected format.",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
warnings, errors := stringIsJSONObject(tt.fieldVal, "field-name")
require.Empty(t, warnings)

require.Equal(t, len(tt.expectedErrsToContain), len(errors))
for i, err := range errors {
require.ErrorContains(t, err, tt.expectedErrsToContain[i])
}
})
}
}