From 9d964277957dd0efbb26dd5a57c23a6bbde4ab21 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Tue, 29 Jul 2025 15:53:56 -0700 Subject: [PATCH 1/2] Adds validation for slo_id --- docs/resources/kibana_slo.md | 2 +- internal/kibana/slo.go | 7 +- internal/kibana/slo_test.go | 133 +++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 2 deletions(-) diff --git a/docs/resources/kibana_slo.md b/docs/resources/kibana_slo.md index c5759e012..0ce7e5788 100644 --- a/docs/resources/kibana_slo.md +++ b/docs/resources/kibana_slo.md @@ -248,7 +248,7 @@ resource "elasticstack_kibana_slo" "timeslice_metric" { - `kql_custom_indicator` (Block List, Max: 1) (see [below for nested schema](#nestedblock--kql_custom_indicator)) - `metric_custom_indicator` (Block List, Max: 1) (see [below for nested schema](#nestedblock--metric_custom_indicator)) - `settings` (Block List, Max: 1) The default settings should be sufficient for most users, but if needed, these properties can be overwritten. (see [below for nested schema](#nestedblock--settings)) -- `slo_id` (String) An ID (8 and 36 characters). If omitted, a UUIDv1 will be generated server-side. +- `slo_id` (String) An ID (8 to 48 characters) that contains only letters, numbers, hyphens, and underscores. If omitted, a UUIDv1 will be generated server-side. - `space_id` (String) An identifier for the space. If space_id is not provided, the default space is used. - `tags` (List of String) The tags for the SLO. - `timeslice_metric_indicator` (Block List, Max: 1) Defines a timeslice metric indicator for SLO. (see [below for nested schema](#nestedblock--timeslice_metric_indicator)) diff --git a/internal/kibana/slo.go b/internal/kibana/slo.go index c09b82838..85ba3e27e 100644 --- a/internal/kibana/slo.go +++ b/internal/kibana/slo.go @@ -3,6 +3,7 @@ package kibana import ( "context" "fmt" + "regexp" "github.com/elastic/terraform-provider-elasticstack/generated/slo" "github.com/elastic/terraform-provider-elasticstack/internal/clients" @@ -81,11 +82,15 @@ func getSchema() map[string]*schema.Schema { return map[string]*schema.Schema{ "slo_id": { - Description: "An ID (8 and 36 characters). If omitted, a UUIDv1 will be generated server-side.", + Description: "An ID (8 to 48 characters) that contains only letters, numbers, hyphens, and underscores. If omitted, a UUIDv1 will be generated server-side.", Type: schema.TypeString, Optional: true, Computed: true, ForceNew: true, + ValidateFunc: validation.All( + validation.StringLenBetween(8, 48), + validation.StringMatch(regexp.MustCompile(`^[a-zA-Z0-9_-]+$`), "must contain only letters, numbers, hyphens, and underscores"), + ), }, "name": { Description: "The name of the SLO.", diff --git a/internal/kibana/slo_test.go b/internal/kibana/slo_test.go index ed41a5c7b..94ab9b762 100644 --- a/internal/kibana/slo_test.go +++ b/internal/kibana/slo_test.go @@ -611,6 +611,83 @@ func TestAccResourceSloErrors(t *testing.T) { }) } +func TestAccResourceSloValidation(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ProtoV6ProviderFactories: acctest.Providers, + Steps: []resource.TestStep{ + { + Config: getSLOConfigWithInvalidSloId("short", "sh", "apm_latency_indicator"), + ExpectError: regexp.MustCompile(`expected length of slo_id to be in the range \(8 - 48\)`), + }, + { + Config: getSLOConfigWithInvalidSloId("toolongid", "this-id-is-way-too-long-and-exceeds-the-48-character-limit-for-slo-ids", "apm_latency_indicator"), + ExpectError: regexp.MustCompile(`expected length of slo_id to be in the range \(8 - 48\)`), + }, + { + Config: getSLOConfigWithInvalidSloId("invalidchars", "invalid@id$", "apm_latency_indicator"), + ExpectError: regexp.MustCompile(`must contain only letters, numbers, hyphens, and underscores`), + }, + }, + }) +} + +func TestSloIdValidation(t *testing.T) { + resource := kibanaresource.ResourceSlo() + sloIdSchema := resource.Schema["slo_id"] + + // Test valid slo_id values + validIds := []string{ + "valid_id", // 8 chars with underscore + "valid-id", // 8 chars with hyphen + "validId123", // 11 chars with mixed case and numbers + "a1234567", // exactly 8 chars + "this-is-a-very-long-but-valid-slo-id-12345678", // exactly 48 chars + } + + for _, id := range validIds { + warnings, errors := sloIdSchema.ValidateFunc(id, "slo_id") + if len(errors) > 0 { + t.Errorf("Expected valid ID %q to pass validation, but got errors: %v", id, errors) + } + if len(warnings) > 0 { + t.Errorf("Expected valid ID %q to have no warnings, but got: %v", id, warnings) + } + } + + // Test invalid slo_id values + invalidTests := []struct { + id string + expectedErr string + }{ + {"short", "expected length of slo_id to be in the range (8 - 48)"}, + {"1234567", "expected length of slo_id to be in the range (8 - 48)"}, // 7 chars + {"this-is-a-very-long-slo-id-that-exceeds-the-48-character-limit-for-sure", "expected length of slo_id to be in the range (8 - 48)"}, // > 48 chars + {"invalid@id", "must contain only letters, numbers, hyphens, and underscores"}, + {"invalid$id", "must contain only letters, numbers, hyphens, and underscores"}, + {"invalid id", "must contain only letters, numbers, hyphens, and underscores"}, // space + {"invalid.id", "must contain only letters, numbers, hyphens, and underscores"}, // period + } + + for _, test := range invalidTests { + _, errors := sloIdSchema.ValidateFunc(test.id, "slo_id") + if len(errors) == 0 { + t.Errorf("Expected invalid ID %q to fail validation", test.id) + } else { + found := false + for _, err := range errors { + if strings.Contains(err.Error(), test.expectedErr) { + found = true + break + } + } + if !found { + t.Errorf("Expected error for ID %q to contain %q, but got: %v", test.id, test.expectedErr, errors) + } + } + } +} + func checkResourceSloDestroy(s *terraform.State) error { client, err := clients.NewAcceptanceTestingClient() if err != nil { @@ -856,3 +933,59 @@ func getSLOConfig(vars sloVars) string { return config } + +func getSLOConfigWithInvalidSloId(name, sloId, indicatorType string) string { + configTemplate := ` + provider "elasticstack" { + elasticsearch {} + kibana {} + } + + resource "elasticstack_elasticsearch_index" "my_index" { + name = "my-index-%s" + deletion_protection = false + } + + resource "elasticstack_kibana_slo" "test_slo" { + name = "%s" + slo_id = "%s" + description = "fully sick SLO" + + %s + + time_window { + duration = "7d" + type = "rolling" + } + + budgeting_method = "timeslices" + + objective { + target = 0.999 + timeslice_target = 0.95 + timeslice_window = "5m" + } + + depends_on = [elasticstack_elasticsearch_index.my_index] + + } + ` + + var indicator string + switch indicatorType { + case "apm_latency_indicator": + indicator = fmt.Sprintf(` + apm_latency_indicator { + environment = "production" + service = "my-service" + transaction_type = "request" + transaction_name = "GET /sup/dawg" + index = "my-index-%s" + threshold = 500 + } + `, name) + } + + config := fmt.Sprintf(configTemplate, name, name, sloId, indicator) + return config +} From c555bbc7da412160f1e6c41c2dcb65894a85b3c7 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Tue, 29 Jul 2025 17:19:45 -0700 Subject: [PATCH 2/2] Updates CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index feaedb045..c5e418e54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## [Unreleased] +- Add `slo_id` validation to `elasticstack_kibana_slo` ([#1221](https://github.com/elastic/terraform-provider-elasticstack/pull/1221)) - Add `ignore_missing_component_templates` to `elasticstack_elasticsearch_index_template` ([#1206](https://github.com/elastic/terraform-provider-elasticstack/pull/1206)) - Prevent provider panic when a script exists in state, but not in Elasticsearch ([#1218](https://github.com/elastic/terraform-provider-elasticstack/pull/1218))