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

* Validate that mappings are a JSON object, not just valid json * Docs * Changelog * Update internal/elasticsearch/index/validation_test.go Co-authored-by: Boris Ilyushonak <[email protected]> --------- Co-authored-by: Boris Ilyushonak <[email protected]>
elastic · Aug 21, 2024 · 890e65d · 890e65d
1 parent 1920946
commit 890e65d
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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

diff --git a/docs/resources/elasticsearch_component_template.md b/docs/resources/elasticsearch_component_template.md
@@ -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>

diff --git a/docs/resources/elasticsearch_index_template.md b/docs/resources/elasticsearch_index_template.md
@@ -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>

diff --git a/internal/elasticsearch/index/component_template.go b/internal/elasticsearch/index/component_template.go
@@ -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,
+						),
 					},
 				},
 			},

diff --git a/internal/elasticsearch/index/index.go b/internal/elasticsearch/index/index.go
@@ -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": {

diff --git a/internal/elasticsearch/index/template.go b/internal/elasticsearch/index/template.go
@@ -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,
+						),
 					},
 				},
 			},

diff --git a/internal/elasticsearch/index/validation.go b/internal/elasticsearch/index/validation.go
@@ -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
+}
diff --git a/internal/elasticsearch/index/validation_test.go b/internal/elasticsearch/index/validation_test.go
@@ -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])
+			}
+		})
+	}
+}