Skip to content

Commit 6f2ec51

Browse files
Copilottobio
Copilot
and
tobio
authored
Fix null value handling in Kibana connector config causing inconsistent apply state (#1524)
* Initial plan * Add null value removal to SanitizedValue function and unit tests Co-authored-by: tobio <444668+tobio@users.noreply.github.com> * Fix regex escaping consistency in acceptance test Co-authored-by: tobio <444668+tobio@users.noreply.github.com> * Refactor removeNulls to use explicit continue statements Co-authored-by: tobio <444668+tobio@users.noreply.github.com> * Fix tests * Add CHANGELOG entry for null value handling fix Co-authored-by: tobio <444668+tobio@users.noreply.github.com> * Update CHANGELOG entry to include PR number for consistency Co-authored-by: tobio <444668+tobio@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tobio <444668+tobio@users.noreply.github.com> Co-authored-by: Toby Brain <toby.brain@elastic.co>
1 parent 6591ad4 commit 6f2ec51

File tree

6 files changed

+133
-1
lines changed

6 files changed

+133
-1
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ alias = [
4040

4141
### Changes
4242

43+
- Fix `elasticstack_kibana_action_connector` failing with "inconsistent result after apply" when config contains null values ([#1524](https://github.com/elastic/terraform-provider-elasticstack/pull/1524))
4344
- Add `host_name_format` to `elasticstack_fleet_agent_policy` to configure host name format (hostname or FQDN) ([#1312](https://github.com/elastic/terraform-provider-elasticstack/pull/1312))
4445
- Create `elasticstack_kibana_prebuilt_rule` resource ([#1296](https://github.com/elastic/terraform-provider-elasticstack/pull/1296))
4546
- Add `required_versions` to `elasticstack_fleet_agent_policy` ([#1436](https://github.com/elastic/terraform-provider-elasticstack/pull/1436))

‎internal/kibana/connectors/acc_test.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func TestAccResourceKibanaConnectorCasesWebhook(t *testing.T) {
9191
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"getIncidentResponseExternalTitleKey\":\"title\"`)),
9292
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"getIncidentUrl\":\"https://www\.elastic\.co/\"`)),
9393
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"updateIncidentJson\":\"{}\"`)),
94-
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"updateIncidentUrl\":\"https://www.elastic\.co/\"`)),
94+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"updateIncidentUrl\":\"https://www\.elastic\.co/\"`)),
9595
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"viewIncidentUrl\":\"https://www\.elastic\.co/\"`)),
9696

9797
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "secrets", regexp.MustCompile(`\"user\":\"user1\"`)),
@@ -119,6 +119,26 @@ func TestAccResourceKibanaConnectorCasesWebhook(t *testing.T) {
119119
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "secrets", regexp.MustCompile(`\"password\":\"password2\"`)),
120120
),
121121
},
122+
{
123+
SkipFunc: versionutils.CheckIfVersionIsUnsupported(tc.minVersion),
124+
ConfigDirectory: acctest.NamedTestCaseDirectory("null_headers"),
125+
ConfigVariables: vars,
126+
Check: resource.ComposeTestCheckFunc(
127+
testCommonAttributes(connectorName, ".cases-webhook"),
128+
129+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"createIncidentJson\":\"{}\"`)),
130+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"createIncidentResponseKey\":\"key\"`)),
131+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"createIncidentUrl\":\"https://www\.elastic\.co/\"`)),
132+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"getIncidentResponseExternalTitleKey\":\"title\"`)),
133+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"getIncidentUrl\":\"https://www\.elastic\.co/\"`)),
134+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"updateIncidentJson\":\"{}\"`)),
135+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"updateIncidentUrl\":\"https://www\.elastic\.co/\"`)),
136+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"viewIncidentUrl\":\"https://www\.elastic\.co/\"`)),
137+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "config", regexp.MustCompile(`\"headers\":null`)),
138+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "secrets", regexp.MustCompile(`\"user\":\"user1\"`)),
139+
resource.TestMatchResourceAttr("elasticstack_kibana_action_connector.test", "secrets", regexp.MustCompile(`\"password\":\"password1\"`)),
140+
),
141+
},
122142
},
123143
})
124144
})

‎internal/kibana/connectors/config_value.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func (v ConfigValue) SanitizedValue() (string, diag.Diagnostics) {
6767
}
6868

6969
delete(unsanitizedMap, connectorTypeIDKey)
70+
removeNulls(unsanitizedMap)
7071
sanitizedValue, err := json.Marshal(unsanitizedMap)
7172
if err != nil {
7273
diags.AddError("Failed to marshal sanitized config value", err.Error())
@@ -76,6 +77,21 @@ func (v ConfigValue) SanitizedValue() (string, diag.Diagnostics) {
7677
return string(sanitizedValue), diags
7778
}
7879

80+
// removeNulls recursively removes all null values from the map
81+
func removeNulls(m map[string]interface{}) {
82+
for key, value := range m {
83+
if value == nil {
84+
delete(m, key)
85+
continue
86+
}
87+
88+
if nestedMap, ok := value.(map[string]interface{}); ok {
89+
removeNulls(nestedMap)
90+
continue
91+
}
92+
}
93+
}
94+
7995
// StringSemanticEquals returns true if the given config object value is semantically equal to the current config object value.
8096
// The comparison will ignore any default values present in one value, but unset in the other.
8197
func (v ConfigValue) StringSemanticEquals(ctx context.Context, newValuable basetypes.StringValuable) (bool, diag.Diagnostics) {

‎internal/kibana/connectors/config_value_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,47 @@ func TestConfigValue_SanitizedValue(t *testing.T) {
125125
expectError: true,
126126
errorContains: "Failed to unmarshal config value",
127127
},
128+
{
129+
name: "JSON with null values gets sanitized - top level",
130+
configValue: ConfigValue{
131+
Normalized: jsontypes.NewNormalizedValue(`{"key": "value", "nullField": null, "another": "field"}`),
132+
},
133+
expectedResult: `{"another":"field","key":"value"}`,
134+
expectError: false,
135+
},
136+
{
137+
name: "JSON with null values gets sanitized - nested",
138+
configValue: ConfigValue{
139+
Normalized: jsontypes.NewNormalizedValue(`{"key": "value", "nested": {"field": "value", "nullField": null}}`),
140+
},
141+
expectedResult: `{"key":"value","nested":{"field":"value"}}`,
142+
expectError: false,
143+
},
144+
{
145+
name: "JSON with null values gets sanitized - mixed",
146+
configValue: ConfigValue{
147+
Normalized: jsontypes.NewNormalizedValue(`{"key": "value", "nullTop": null, "nested": {"field": "value", "nullNested": null}, "another": null}`),
148+
},
149+
expectedResult: `{"key":"value","nested":{"field":"value"}}`,
150+
expectError: false,
151+
},
152+
{
153+
name: "JSON with only null values results in empty object",
154+
configValue: ConfigValue{
155+
Normalized: jsontypes.NewNormalizedValue(`{"nullField1": null, "nullField2": null}`),
156+
},
157+
expectedResult: `{}`,
158+
expectError: false,
159+
},
160+
{
161+
name: "JSON with null and connector type ID gets both sanitized",
162+
configValue: ConfigValue{
163+
Normalized: jsontypes.NewNormalizedValue(`{"key": "value", "nullField": null, "__tf_provider_connector_type_id": "test-connector"}`),
164+
connectorTypeID: "test-connector",
165+
},
166+
expectedResult: `{"key":"value"}`,
167+
expectError: false,
168+
},
128169
}
129170

130171
for _, tt := range tests {

‎internal/kibana/connectors/testdata/TestAccResourceKibanaConnectorCasesWebhook/with_empty_connector_id/null_headers/connector.tf‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
variable "connector_name" {
2+
description = "The connector name"
3+
type = string
4+
}
5+
6+
resource "elasticstack_kibana_action_connector" "test" {
7+
name = var.connector_name
8+
config = jsonencode({
9+
createIncidentJson = "{}"
10+
createIncidentResponseKey = "key"
11+
createIncidentUrl = "https://www.elastic.co/"
12+
getIncidentResponseExternalTitleKey = "title"
13+
getIncidentUrl = "https://www.elastic.co/"
14+
headers = null
15+
updateIncidentJson = "{}"
16+
updateIncidentUrl = "https://www.elastic.co/"
17+
viewIncidentUrl = "https://www.elastic.co/"
18+
})
19+
secrets = jsonencode({
20+
user = "user1"
21+
password = "password1"
22+
})
23+
connector_type_id = ".cases-webhook"
24+
}

‎internal/kibana/connectors/testdata/TestAccResourceKibanaConnectorCasesWebhook/with_predefined_connector_id/null_headers/connector.tf‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
variable "connector_name" {
2+
description = "The connector name"
3+
type = string
4+
}
5+
6+
variable "connector_id" {
7+
description = "Connector ID"
8+
type = string
9+
}
10+
11+
resource "elasticstack_kibana_action_connector" "test" {
12+
name = var.connector_name
13+
connector_id = var.connector_id
14+
config = jsonencode({
15+
createIncidentJson = "{}"
16+
createIncidentResponseKey = "key"
17+
createIncidentUrl = "https://www.elastic.co/"
18+
getIncidentResponseExternalTitleKey = "title"
19+
getIncidentUrl = "https://www.elastic.co/"
20+
headers = null
21+
updateIncidentJson = "{}"
22+
updateIncidentUrl = "https://www.elastic.co/"
23+
viewIncidentUrl = "https://www.elastic.co/"
24+
})
25+
secrets = jsonencode({
26+
user = "user1"
27+
password = "password1"
28+
})
29+
connector_type_id = ".cases-webhook"
30+
}

0 commit comments

Comments
 (0)