Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Improved detector validation #538

Merged
merged 4 commits into from
Oct 24, 2024
Merged

Feature: Improved detector validation #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb67550
Adding current state of detector improvements
MovieStoreGuy Oct 24, 2024
3a2903a
Fixing up coverage
MovieStoreGuy Oct 24, 2024
3fedecf
Improving overally quality
MovieStoreGuy Oct 24, 2024
89362b2
Update internal/common/notification_list.go
MovieStoreGuy Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion internal/definition/detector/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/signalfx/signalfx-go/detector"

Expand All @@ -24,6 +25,7 @@

func NewResource() *schema.Resource {
return &schema.Resource{
SchemaVersion: 1,
SchemaFunc: newSchema,
CreateContext: resourceCreate,
ReadContext: resourceRead,
Expand All @@ -35,6 +37,7 @@
StateUpgraders: []schema.StateUpgrader{
{Type: v0state().CoreConfigSchema().ImpliedType(), Upgrade: v0stateMigration, Version: 0},
},
CustomizeDiff: customdiff.If(resourceValidateCond, resourceValidateFunc),
}
}

Expand Down Expand Up @@ -76,9 +79,14 @@
)...,
)

data.SetId(resp.Id)

// Some fields are only set from calling a the read operation,
// so to keep the output consistent, this defers the remaining
// effort to read to get the data
return tfext.AppendDiagnostics(
issues,
tfext.AsErrorDiagnostics(encodeTerraform(resp, data))...,
resourceRead(ctx, data, meta)...,
)
}

Expand All @@ -99,6 +107,12 @@
issues = tfext.AppendDiagnostics(issues, tfext.AsWarnDiagnostics(fmt.Errorf("detector is over mts limit"))...)
}

issues = tfext.AppendDiagnostics(issues,
tfext.AsErrorDiagnostics(
data.Set("url", pmeta.LoadApplicationURL(ctx, meta, AppPath, dt.Id, "edit")),
)...,
)

return tfext.AppendDiagnostics(
issues,
tfext.AsErrorDiagnostics(encodeTerraform(dt, data))...,
Expand Down Expand Up @@ -158,3 +172,45 @@
err = common.HandleError(ctx, client.DeleteDetector(ctx, data.Id()), data)
return tfext.AsErrorDiagnostics(err)
}

func resourceValidateCond(ctx context.Context, diff *schema.ResourceDiff, _ any) (validate bool) {
tflog.Debug(ctx, "Checking if program text or rules needed to be updated")
if _, ok := diff.GetOkExists("rule"); ok {
validate = true
}
if _, ok := diff.GetOkExists("program_text"); ok {
validate = true
}
return validate

Check warning on line 184 in internal/definition/detector/resource.go

View check run for this annotation

Codecov / codecov/patch

internal/definition/detector/resource.go#L176-L184 

Added lines #L176 - L184 were not covered by tests
}

func resourceValidateFunc(ctx context.Context, diff *schema.ResourceDiff, meta any) error {
var rules []*detector.Rule
for _, v := range diff.Get("rule").(*schema.Set).List() {
data := v.(map[string]any)
rules = append(rules, &detector.Rule{
Description: data["description"].(string),
DetectLabel: data["detect_label"].(string),
Disabled: data["disabled"].(bool),
Severity: detector.Severity(data["severity"].(string)),
ParameterizedBody: data["parameterized_body"].(string),
ParameterizedSubject: data["parameterized_subject"].(string),
RunbookUrl: data["runbook_url"].(string),
Tip: data["tip"].(string),
})
}

Check warning on line 201 in internal/definition/detector/resource.go

View check run for this annotation

Codecov / codecov/patch

internal/definition/detector/resource.go#L187-L201 

Added lines #L187 - L201 were not covered by tests

client, err := pmeta.LoadClient(ctx, meta)
if err != nil {
return err
}

Check warning on line 206 in internal/definition/detector/resource.go

View check run for this annotation

Codecov / codecov/patch

internal/definition/detector/resource.go#L203-L206 

Added lines #L203 - L206 were not covered by tests

tflog.Debug(ctx, "Sending detector payload for validation", tfext.NewLogFields().JSON("content", rules))
return client.ValidateDetector(ctx, &detector.ValidateDetectorRequestModel{
Name: diff.Get("name").(string),
ProgramText: diff.Get("program_text").(string),
Rules: rules,
DetectorOrigin: diff.Get("detector_origin").(string),
ParentDetectorId: diff.Get("parent_detector_id").(string),
})

Check warning on line 215 in internal/definition/detector/resource.go

View check run for this annotation

Codecov / codecov/patch

internal/definition/detector/resource.go#L208-L215 

Added lines #L208 - L215 were not covered by tests
}
169 changes: 169 additions & 0 deletions internal/definition/detector/resource_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
// Copyright Splunk, Inc.
// SPDX-License-Identifier: MPL-2.0

package detector_test

import (
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

"github.com/splunk-terraform/terraform-provider-signalfx/internal/definition/detector"
"github.com/splunk-terraform/terraform-provider-signalfx/internal/definition/team"
"github.com/splunk-terraform/terraform-provider-signalfx/internal/tftest"
)

func TestAcceptance(t *testing.T) {
for _, tc := range []struct {
name string
steps []resource.TestStep
}{
{
name: "minimal detector",
steps: []resource.TestStep{
{
Config: tftest.LoadConfig("testdata/minimal.tf"),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("signalfx_detector.minimal", "name", "my minimal detector"),
resource.TestCheckResourceAttr("signalfx_detector.minimal", "program_text", "detect(when(const(1) > 1)).publish('HCF')\n"),
),
ExpectNonEmptyPlan: false,
},
},
},
{
name: "advanced detector",
steps: []resource.TestStep{
{
Config: tftest.LoadConfig("testdata/advanced_00.tf"),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "name", "example detector"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "description", "A detector made from terraform"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "timezone", "Europe/Paris"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.#", "2"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.0", "tag-1"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.1", "tag-2"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "teams.#", "1"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "max_delay", "30"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "min_delay", "15"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "program_text", "signal = data('app.delay').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\ndetect(when(signal > 60, '30m')).publish('Processing old messages 30m')\n"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.#", "2"),

// Rule #1
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.description", "maximum > 60 for 5m"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.detect_label", "Processing old messages 5m"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.disabled", "false"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.notifications.#", "1"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.notifications.0", "Email,[email protected]"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.parameterized_body", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.parameterized_subject", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.runbook_url", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.severity", "Warning"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.tip", ""),

// Rule #2
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.description", "maximum > 60 for 30m"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.detect_label", "Processing old messages 30m"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.disabled", "false"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.#", "1"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.0", "Email,[email protected]"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.parameterized_body", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.parameterized_subject", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.runbook_url", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.severity", "Critical"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.tip", ""),
),
ExpectNonEmptyPlan: false,
Destroy: false,
},
{
Config: tftest.LoadConfig("testdata/advanced_01.tf"),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "name", "max average delay UPDATED"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "description", "your application is slowER"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "timezone", "Europe/Paris"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.#", "3"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.0", "tag-1"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.1", "tag-2"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.2", "tag-3"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "teams.#", "0"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "max_delay", "60"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "min_delay", "30"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector",
"time_range", "3600"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "program_text", "signal = data('app.delay2').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\ndetect(when(signal > 60, '30m')).publish('Processing old messages 30m')\n"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "show_data_markers", "true"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "show_event_lines", "true"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "disable_sampling", "true"),

resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.%", "2"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.Processing old messages 30m", "1000"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "label_resolutions.Processing old messages 5m", "1000"),

resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.#", "2"),

// Rule #1
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.description", "NEW maximum > 60 for 5m"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.notifications.0", "Email,[email protected]"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.parameterized_body", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.parameterized_subject", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.severity", "Warning"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.runbook_url", "https://www.example.com"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.1.tip", "reboot it"),

// Rule #2
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.description", "NEW maximum > 60 for 30m"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.detect_label", "Processing old messages 30m"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.disabled", "false"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.#", "1"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.0", "Email,[email protected]"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.parameterized_body", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.parameterized_subject", ""),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.runbook_url", "https://www.example.com"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.severity", "Critical"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.tip", ""),
),
},
{
Config: tftest.LoadConfig("testdata/advanced_02.tf"),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "name", "max average delay UPDATED"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "description", "your application is slowER"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "timezone", "Europe/Paris"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "tags.#", "0"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "teams.#", "0"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "max_delay", "60"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "min_delay", "30"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector",
"time_range", "3600"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "program_text", "signal = data('app.delay2').max().publish('app delay')\ndetect(when(signal > 60, '5m')).publish('Processing old messages 5m')\n"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "show_data_markers", "true"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "show_event_lines", "true"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "disable_sampling", "true"),

resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.#", "1"),

// Rule #1
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.description", "NEW maximum > 60 for 5m"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.notifications.0", "Email,[email protected]"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.severity", "Warning"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.runbook_url", "https://www.example.com"),
resource.TestCheckResourceAttr("signalfx_detector.my_detector", "rule.0.tip", "reboot it"),
),
ExpectNonEmptyPlan: true,
},
},
},
} {
t.Run(tc.name, func(t *testing.T) {
tftest.NewAcceptanceHandler(
tftest.WithAcceptanceResources(map[string]*schema.Resource{
detector.ResourceName: detector.NewResource(),
team.ResourceName: team.NewResource(),
}),
).
Test(t, tc.steps)
})
}
}
48 changes: 38 additions & 10 deletions internal/definition/detector/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,34 @@ func TestResourceCreate(t *testing.T) {
DetectorOrigin: req.DetectorOrigin,
})
},
"GET /v2/detector/id-01": func(w http.ResponseWriter, r *http.Request) {
_, _ = io.Copy(io.Discard, r.Body)
_ = r.Body.Close()

_ = json.NewEncoder(w).Encode(&detector.Detector{
Id: "id-01",
Name: "test detector",
Description: "An example detector response",
AuthorizedWriters: &detector.AuthorizedWriters{},
TimeZone: "Australia/Sydney",
MaxDelay: common.AsPointer[int32](1000),
MinDelay: common.AsPointer[int32](1000),
ProgramText: `detect(when(data('*').count() < 1)).publish('no data')`,
OverMTSLimit: false,
Rules: []*detector.Rule{
{
DetectLabel: "no data",
Notifications: []*notification.Notification{
{Type: "Team", Value: &notification.TeamNotification{Type: "Team", Team: "awesome-team"}},
},
},
},
Tags: []string{"tag-01"},
Teams: []string{"team-01"},
DetectorOrigin: "Standard",
VisualizationOptions: &detector.Visualization{},
})
},
}),
Encoder: encodeTerraform,
Decoder: decodeTerraform,
Expand All @@ -97,8 +125,8 @@ func TestResourceCreate(t *testing.T) {
Description: "An example detector response",
AuthorizedWriters: &detector.AuthorizedWriters{},
TimeZone: "Australia/Sydney",
MaxDelay: common.AsPointer[int32](100),
MinDelay: common.AsPointer[int32](100),
MaxDelay: common.AsPointer[int32](1300),
MinDelay: common.AsPointer[int32](1400),
ProgramText: `detect(when(data('*').count() < 1)).publish('no data')`,
OverMTSLimit: false,
Rules: []*detector.Rule{
Expand All @@ -120,8 +148,8 @@ func TestResourceCreate(t *testing.T) {
Description: "An example detector response",
AuthorizedWriters: &detector.AuthorizedWriters{},
TimeZone: "Australia/Sydney",
MaxDelay: common.AsPointer[int32](100000000),
MinDelay: common.AsPointer[int32](100000000),
MaxDelay: common.AsPointer[int32](1000),
MinDelay: common.AsPointer[int32](1000),
ProgramText: `detect(when(data('*').count() < 1)).publish('no data')`,
OverMTSLimit: false,
Rules: []*detector.Rule{
Expand Down Expand Up @@ -195,8 +223,8 @@ func TestResourceRead(t *testing.T) {
Name: "test detector",
Description: "An example detector response",
TimeZone: "Australia/Sydney",
MaxDelay: common.AsPointer[int32](100),
MinDelay: common.AsPointer[int32](100),
MaxDelay: common.AsPointer[int32](1000),
MinDelay: common.AsPointer[int32](1000),
ProgramText: `detect(when(data('*').count() < 1)).publish('no data')`,
Rules: []*detector.Rule{
{
Expand All @@ -221,8 +249,8 @@ func TestResourceRead(t *testing.T) {
Description: "An example detector response",
AuthorizedWriters: &detector.AuthorizedWriters{},
TimeZone: "Australia/Sydney",
MaxDelay: common.AsPointer[int32](100000),
MinDelay: common.AsPointer[int32](100000),
MaxDelay: common.AsPointer[int32](1000),
MinDelay: common.AsPointer[int32](1000),
ProgramText: `detect(when(data('*').count() < 1)).publish('no data')`,
Rules: []*detector.Rule{
{
Expand Down Expand Up @@ -280,8 +308,8 @@ func TestResourceRead(t *testing.T) {
Description: "An example detector response",
AuthorizedWriters: &detector.AuthorizedWriters{},
TimeZone: "Australia/Sydney",
MaxDelay: common.AsPointer[int32](100000000),
MinDelay: common.AsPointer[int32](100000000),
MaxDelay: common.AsPointer[int32](100),
MinDelay: common.AsPointer[int32](100),
ProgramText: `detect(when(data('*').count() < 1)).publish('no data')`,
OverMTSLimit: true,
Rules: []*detector.Rule{
Expand Down
Loading
Loading