Closed cgroschupp closed 1 year ago
Hi @cgroschupp thanks for the contribution. There are several different changes bundled in this, which don't correspond to the error you posted in the description. Can you explain why the different changes are being made here?
@phillbaker I also added support for updating the resource. Should I split these changes to a second pr/commit?
Yes, can you please split the changes to a second PR? We may need to update tests as well.
On Mon, Feb 6, 2023 at 3:27 AM Christian Groschupp @.***> wrote:
@phillbaker https://github.com/phillbaker I also added support for updating the resource. Should I split these changes to a second pr/commit?
— Reply to this email directly, view it on GitHub https://github.com/phillbaker/terraform-provider-elasticsearch/pull/335#issuecomment-1418692689, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAXCKMNTP5AHYWB5VY5K7DWWCYXTANCNFSM6AAAAAAUPIEGLA . You are receiving this because you were mentioned.Message ID: @.*** com>
My test setup:
export ES_KIBANA_IMAGE=docker.elastic.co/kibana/kibana:7.17.4
export XPACK_IMAGE="docker.elastic.co/elasticsearch/elasticsearch:7.17.4"
podman-compose up -d kibana
ELASTICSEARCH_URL=http://admin:admin@localhost:9230 TF_ACC=1 go test ./... -run ^TestAccElasticsearchKibanaAlert$ -v
resource_elasticsearch_kibana_alert_test.go:75: Step 3/3 error: After applying this test step and performing a `terraform refresh`, the plan was not empty.
stdout
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# elasticsearch_kibana_alert.test_action_json will be updated in-place
~ resource "elasticsearch_kibana_alert" "test_action_json" {
id = "a4f1c390-a86e-11ed-8c66-1bd69e550f42"
name = "terraform-alert"
- notify_when = "onActiveAlert" -> null
tags = []
# (3 unchanged attributes hidden)
- actions {
- action_type_id = ".index" -> null
- group = "threshold met" -> null
- id = "9ff361f0-a86e-11ed-8c66-1bd69e550f42" -> null
- params = {} -> null
- params_json = jsonencode(
{
- level = "info"
- message = <<-EOT
alert '{{alertName}}' is active for group '{{context.group}}':
- Value: {{context.value}}
- Conditions Met: {{context.conditions}} over {{params.timeWindowSize}}{{params.timeWindowUnit}}
- Timestamp: {{context.date}}
EOT
}
) -> null
}
+ actions {
+ action_type_id = ".index"
+ group = "threshold met"
+ id = "9ff361f0-a86e-11ed-8c66-1bd69e550f42"
+ params_json = jsonencode(
{
+ level = "info"
+ message = <<-EOT
alert '{{alertName}}' is active for group '{{context.group}}':
- Value: {{context.value}}
- Conditions Met: {{context.conditions}} over {{params.timeWindowSize}}{{params.timeWindowUnit}}
- Timestamp: {{context.date}}
EOT
}
)
}
# (2 unchanged blocks hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccElasticsearchKibanaAlert (12.54s)
@phillbaker After a refresh I get a not empty plan. I have no idea why this happens, do you have any advice?
@phillbaker i found a solution, can you please rerun the tests.
@phillbaker I have corrected the format, now terrafmt runs through.
@phillbaker can you please retrigger the build.
@phillbaker second steps forward one step back. sorry for that.
@cgroschupp thanks for the contribution, it's passing CI! I'll be able to give it a final review tomorrow.
@phillbaker is there anything I can do to improve it?
Also - can you add some context, what does this allow that just using the params map did not allow?
Also - can you add some context, what does this allow that just using the params map did not allow?
See the PR description. The email alert action needs a `to' field, but this field must be an array. This was not possible with the params field of type map[string]string.
The `to' field must be an array.