From c6d4c5f42040ed1aab8ebaa024d0ab72a9ad0171 Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Thu, 9 May 2024 12:29:07 +0930 Subject: [PATCH 1/4] Refactor 'id' schema for SLO resource --- docs/resources/kibana_slo.md | 6 +++++- generated/slo/.openapi-generator/VERSION | 2 +- generated/slo/api_slo.go | 12 +++++++++++ generated/slo/client.go | 8 +++++++- internal/clients/kibana/slo.go | 16 ++++++++++----- internal/clients/kibana/slo_test.go | 8 ++++++-- internal/kibana/slo.go | 26 ++++++++++++------------ internal/models/slo.go | 2 +- 8 files changed, 56 insertions(+), 24 deletions(-) diff --git a/docs/resources/kibana_slo.md b/docs/resources/kibana_slo.md index 41e543dd6..4dee6be5d 100644 --- a/docs/resources/kibana_slo.md +++ b/docs/resources/kibana_slo.md @@ -210,13 +210,17 @@ resource "elasticstack_kibana_slo" "custom_metric" { - `apm_latency_indicator` (Block List, Max: 1) (see [below for nested schema](#nestedblock--apm_latency_indicator)) - `group_by` (String) Optional group by field to use to generate an SLO per distinct value. - `histogram_custom_indicator` (Block List, Max: 1) (see [below for nested schema](#nestedblock--histogram_custom_indicator)) -- `id` (String) An ID (8 and 36 characters). If omitted, a UUIDv1 will be generated server-side. - `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. - `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. +### Read-Only + +- `id` (String) The ID of this resource. + ### Nested Schema for `objective` diff --git a/generated/slo/.openapi-generator/VERSION b/generated/slo/.openapi-generator/VERSION index f7667587c..73a86b197 100644 --- a/generated/slo/.openapi-generator/VERSION +++ b/generated/slo/.openapi-generator/VERSION @@ -1 +1 @@ -7.0.0-beta \ No newline at end of file +7.0.1 \ No newline at end of file diff --git a/generated/slo/api_slo.go b/generated/slo/api_slo.go index bd95b7880..d4340c8c7 100644 --- a/generated/slo/api_slo.go +++ b/generated/slo/api_slo.go @@ -1105,15 +1105,27 @@ func (a *SloAPIService) FindSlosOpExecute(r ApiFindSlosOpRequest) (*FindSloRespo } if r.page != nil { parameterAddToHeaderOrQuery(localVarQueryParams, "page", r.page, "") + } else { + var defaultValue int32 = 1 + r.page = &defaultValue } if r.perPage != nil { parameterAddToHeaderOrQuery(localVarQueryParams, "perPage", r.perPage, "") + } else { + var defaultValue int32 = 25 + r.perPage = &defaultValue } if r.sortBy != nil { parameterAddToHeaderOrQuery(localVarQueryParams, "sortBy", r.sortBy, "") + } else { + var defaultValue string = "status" + r.sortBy = &defaultValue } if r.sortDirection != nil { parameterAddToHeaderOrQuery(localVarQueryParams, "sortDirection", r.sortDirection, "") + } else { + var defaultValue string = "asc" + r.sortDirection = &defaultValue } // to determine the Content-Type header localVarHTTPContentTypes := []string{} diff --git a/generated/slo/client.go b/generated/slo/client.go index 1adb7840b..6a012a56f 100644 --- a/generated/slo/client.go +++ b/generated/slo/client.go @@ -440,6 +440,7 @@ func (c *APIClient) decode(v interface{}, b []byte, contentType string) (err err return } _, err = f.Seek(0, io.SeekStart) + err = os.Remove(f.Name()) return } if f, ok := v.(**os.File); ok { @@ -452,6 +453,7 @@ func (c *APIClient) decode(v interface{}, b []byte, contentType string) (err err return } _, err = (*f).Seek(0, io.SeekStart) + err = os.Remove((*f).Name()) return } if xmlCheck.MatchString(contentType) { @@ -528,7 +530,11 @@ func setBody(body interface{}, contentType string) (bodyBuf *bytes.Buffer, err e } else if jsonCheck.MatchString(contentType) { err = json.NewEncoder(bodyBuf).Encode(body) } else if xmlCheck.MatchString(contentType) { - err = xml.NewEncoder(bodyBuf).Encode(body) + var bs []byte + bs, err = xml.Marshal(body) + if err == nil { + bodyBuf.Write(bs) + } } if err != nil { diff --git a/internal/clients/kibana/slo.go b/internal/clients/kibana/slo.go index 518513745..4edc8434d 100644 --- a/internal/clients/kibana/slo.go +++ b/internal/clients/kibana/slo.go @@ -75,7 +75,7 @@ func UpdateSlo(ctx context.Context, apiClient *clients.ApiClient, s models.Slo) Tags: s.Tags, } - req := client.UpdateSloOp(ctxWithAuth, s.SpaceID, s.ID).KbnXsrf("true").UpdateSloRequest(reqModel) + req := client.UpdateSloOp(ctxWithAuth, s.SpaceID, s.SloID).KbnXsrf("true").UpdateSloRequest(reqModel) slo, res, err := req.Execute() if err != nil { @@ -83,7 +83,7 @@ func UpdateSlo(ctx context.Context, apiClient *clients.ApiClient, s models.Slo) } defer res.Body.Close() - if diags := utils.CheckHttpError(res, "unable to update slo with id "+s.ID); diags.HasError() { + if diags := utils.CheckHttpError(res, "unable to update slo with id "+s.SloID); diags.HasError() { return nil, diags } @@ -112,6 +112,12 @@ func CreateSlo(ctx context.Context, apiClient *clients.ApiClient, s models.Slo) GroupBy: s.GroupBy, Tags: s.Tags, } + + // Explicitly set SLO object id if provided, otherwise we'll use the autogenerated ID from the Kibana API response + if s.SloID != "" { + reqModel.Id = &s.SloID + } + req := client.CreateSloOp(ctxWithAuth, s.SpaceID).KbnXsrf("true").CreateSloRequest(reqModel) sloRes, res, err := req.Execute() if err != nil { @@ -119,11 +125,11 @@ func CreateSlo(ctx context.Context, apiClient *clients.ApiClient, s models.Slo) } defer res.Body.Close() - if diags := utils.CheckHttpError(res, "unable to create slo with id "+s.ID); diags.HasError() { + if diags := utils.CheckHttpError(res, "unable to create slo with id "+s.SloID); diags.HasError() { return nil, diags } - s.ID = sloRes.Id + s.SloID = sloRes.Id return &s, diag.Diagnostics{} } @@ -162,7 +168,7 @@ func sloResponseToModel(spaceID string, res *slo.SloResponse) *models.Slo { } return &models.Slo{ - ID: res.Id, + SloID: res.Id, SpaceID: spaceID, Name: res.Name, Description: res.Description, diff --git a/internal/clients/kibana/slo_test.go b/internal/clients/kibana/slo_test.go index 5076acb8e..26b7ac0f1 100644 --- a/internal/clients/kibana/slo_test.go +++ b/internal/clients/kibana/slo_test.go @@ -14,12 +14,14 @@ func Test_sloResponseToModel(t *testing.T) { tests := []struct { name string spaceId string + sloId string sloResponse *slo.SloResponse expectedModel *models.Slo }{ { name: "should return a model with the correct values", spaceId: "space-id", + sloId: "slo-id", sloResponse: &slo.SloResponse{ Id: "slo-id", Name: "slo-name", @@ -50,7 +52,7 @@ func Test_sloResponseToModel(t *testing.T) { UpdatedAt: "2023-08-11T00:05:36.567Z", }, expectedModel: &models.Slo{ - ID: "slo-id", + SloID: "slo-id", Name: "slo-name", Description: "slo-description", Indicator: slo.SloResponseIndicator{ @@ -81,6 +83,7 @@ func Test_sloResponseToModel(t *testing.T) { { name: "should return tags if available", spaceId: "space-id", + sloId: "slo-id", sloResponse: &slo.SloResponse{ Id: "slo-id", Name: "slo-name", @@ -112,7 +115,7 @@ func Test_sloResponseToModel(t *testing.T) { UpdatedAt: "2023-08-11T00:05:36.567Z", }, expectedModel: &models.Slo{ - ID: "slo-id", + SloID: "slo-id", Name: "slo-name", Description: "slo-description", Indicator: slo.SloResponseIndicator{ @@ -144,6 +147,7 @@ func Test_sloResponseToModel(t *testing.T) { { name: "nil response should return a nil model", spaceId: "space-id", + sloId: "slo-id", sloResponse: nil, expectedModel: nil, }, diff --git a/internal/kibana/slo.go b/internal/kibana/slo.go index 6f42e8f50..a5b75a2d2 100644 --- a/internal/kibana/slo.go +++ b/internal/kibana/slo.go @@ -21,12 +21,12 @@ func ResourceSlo() *schema.Resource { } sloSchema := map[string]*schema.Schema{ - "id": { + "slo_id": { Description: "An ID (8 and 36 characters). If omitted, a UUIDv1 will be generated server-side.", Type: schema.TypeString, Optional: true, - ForceNew: true, Computed: true, + ForceNew: true, }, "name": { Description: "The name of the SLO.", @@ -617,6 +617,12 @@ func getSloFromResourceData(d *schema.ResourceData) (models.Slo, diag.Diagnostic GroupBy: getOrNilString("group_by", d), } + // Explicitly set SLO object id if provided, otherwise we'll use the autogenerated ID from the Kibana API response + if sloID := getOrNilString("slo_id", d); sloID != nil && *sloID != "" { + slo.SloID = *sloID + fmt.Println("Setting slo.SloID to", *sloID) + } + if tags, ok := d.GetOk("tags"); ok { for _, t := range tags.([]interface{}) { slo.Tags = append(slo.Tags, t.(string)) @@ -643,8 +649,8 @@ func resourceSloCreate(ctx context.Context, d *schema.ResourceData, meta interfa return diags } - id := &clients.CompositeId{ClusterId: slo.SpaceID, ResourceId: res.ID} - d.SetId(id.String()) + compositeID := &clients.CompositeId{ClusterId: slo.SpaceID, ResourceId: res.SloID} + d.SetId(compositeID.String()) return resourceSloRead(ctx, d, meta) } @@ -660,20 +666,14 @@ func resourceSloUpdate(ctx context.Context, d *schema.ResourceData, meta interfa return diags } - compId, diags := clients.CompositeIdFromStr(d.Id()) - if diags.HasError() { - return diags - } - slo.ID = compId.ResourceId - res, diags := kibana.UpdateSlo(ctx, client, slo) if diags.HasError() { return diags } - id := &clients.CompositeId{ClusterId: slo.SpaceID, ResourceId: res.ID} - d.SetId(id.String()) + compositeID := &clients.CompositeId{ClusterId: slo.SpaceID, ResourceId: res.SloID} + d.SetId(compositeID.String()) return resourceSloRead(ctx, d, meta) } @@ -844,7 +844,7 @@ func resourceSloRead(ctx context.Context, d *schema.ResourceData, meta interface } } - if err := d.Set("id", s.ID); err != nil { + if err := d.Set("slo_id", s.SloID); err != nil { return diag.FromErr(err) } if err := d.Set("space_id", s.SpaceID); err != nil { diff --git a/internal/models/slo.go b/internal/models/slo.go index 9bad1a0ff..295c659bf 100644 --- a/internal/models/slo.go +++ b/internal/models/slo.go @@ -5,7 +5,7 @@ import ( ) type Slo struct { - ID string + SloID string Name string Description string Indicator slo.SloResponseIndicator From 737329904989e80471f6bd67ea95f4771f90d700 Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Thu, 9 May 2024 14:10:32 +0930 Subject: [PATCH 2/4] CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 204378a95..89e55a6d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Prevent a provider panic when an `elasticstack_elasticsearch_template` or `elasticstack_elasticsearch_component_template` includes an empty `template` (`template {}`) block. ([#598](https://github.com/elastic/terraform-provider-elasticstack/pull/598)) - Prevent `elasticstack_kibana_space` to attempt the space recreation if `initials` and `color` are not provided. ([#606](https://github.com/elastic/terraform-provider-elasticstack/pull/606)) - Prevent a provider panic in `elasticstack_kibana_data_view` when a `field_format` does not include a `pattern`. ([#619](https://github.com/elastic/terraform-provider-elasticstack/pull/619/files)) +- Fixed a bug where the `id` attribute for `elasticstack_kibana_slo` resources was ignored by renaming the attribute to `slo_id`. ([#622](https://github.com/elastic/terraform-provider-elasticstack/pull/622)) ### Added From c014d39c9d5991da8aba0852a6bc463abb9443e6 Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Thu, 9 May 2024 14:43:25 +0930 Subject: [PATCH 3/4] Add acc test and remove debugging print statement --- internal/kibana/slo.go | 1 - internal/kibana/slo_test.go | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/kibana/slo.go b/internal/kibana/slo.go index a5b75a2d2..727075e79 100644 --- a/internal/kibana/slo.go +++ b/internal/kibana/slo.go @@ -620,7 +620,6 @@ func getSloFromResourceData(d *schema.ResourceData) (models.Slo, diag.Diagnostic // Explicitly set SLO object id if provided, otherwise we'll use the autogenerated ID from the Kibana API response if sloID := getOrNilString("slo_id", d); sloID != nil && *sloID != "" { slo.SloID = *sloID - fmt.Println("Setting slo.SloID to", *sloID) } if tags, ok := d.GetOk("tags"); ok { diff --git a/internal/kibana/slo_test.go b/internal/kibana/slo_test.go index 57f8f1061..fd2efc89d 100644 --- a/internal/kibana/slo_test.go +++ b/internal/kibana/slo_test.go @@ -39,6 +39,7 @@ func TestAccResourceSlo(t *testing.T) { Config: getSLOConfig(sloName, "apm_latency_indicator", false, []string{}, ""), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "name", sloName), + resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "slo_id", "fully-sick-slo"), resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "description", "fully sick SLO"), resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "apm_latency_indicator.0.environment", "production"), resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "apm_latency_indicator.0.service", "my-service"), @@ -292,6 +293,7 @@ func getSLOConfig(name string, indicatorType string, settingsEnabled bool, tags resource "elasticstack_kibana_slo" "test_slo" { name = "%s" + slo_id = "fully-sick-slo" description = "fully sick SLO" %s From 5a3193b8ff69e82641aaa19a962e50cf23548fd6 Mon Sep 17 00:00:00 2001 From: Brad Deam <54515790+b-deam@users.noreply.github.com> Date: Thu, 9 May 2024 15:44:48 +0930 Subject: [PATCH 4/4] Update internal/kibana/slo_test.go Co-authored-by: Toby Brain --- internal/kibana/slo_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/kibana/slo_test.go b/internal/kibana/slo_test.go index fd2efc89d..80f84d458 100644 --- a/internal/kibana/slo_test.go +++ b/internal/kibana/slo_test.go @@ -293,7 +293,7 @@ func getSLOConfig(name string, indicatorType string, settingsEnabled bool, tags resource "elasticstack_kibana_slo" "test_slo" { name = "%s" - slo_id = "fully-sick-slo" + slo_id = "fully-sick-slo" description = "fully sick SLO" %s