From 5a26c2435f5f8bfcfd0cbf53ff0ad280c27ca9af Mon Sep 17 00:00:00 2001 From: Mateusz Hawrus <48822818+nieomylnieja@users.noreply.github.com> Date: Fri, 28 Feb 2025 12:33:46 +0100 Subject: [PATCH] fix: Correct labels usage in Report object (#655) ## Motivation Report object was redefining `v1alpha.Labels` instead of using it. ## Summary Fixed some minor issues reported by _gopls_. ## Breaking Changes `report.Labels`, `report.LabelKey` and `report.LabelValue` have been removed, use `v1alpha.Labels` instead. --- internal/manifest/v1alpha/examples/report.go | 3 ++- .../v1alpha/examples/slo_composite.go | 2 +- manifest/v1alpha/report/report.go | 13 ++++------ .../v1alpha/report/system_health_review.go | 10 +++++--- .../report/validation_system_health_review.go | 6 +++-- manifest/v1alpha/report/validation_test.go | 25 ++++++++++--------- .../v1alpha/report/validation_time_frame.go | 2 +- tests/v1alpha_report_test.go | 21 ++++++++-------- tests/v1alpha_slo_test.go | 4 +-- 9 files changed, 46 insertions(+), 40 deletions(-) diff --git a/internal/manifest/v1alpha/examples/report.go b/internal/manifest/v1alpha/examples/report.go index 5dac6ca2c..2197aacf1 100644 --- a/internal/manifest/v1alpha/examples/report.go +++ b/internal/manifest/v1alpha/examples/report.go @@ -3,6 +3,7 @@ package v1alphaExamples import ( "time" + "github.com/nobl9/nobl9-go/manifest/v1alpha" "github.com/nobl9/nobl9-go/manifest/v1alpha/report" ) @@ -35,7 +36,7 @@ func Report() []Example { Columns: []report.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[report.LabelKey][]report.LabelValue{ + Labels: v1alpha.Labels{ "key1": {"value1"}, "key2": {"value2", "value3"}, }, diff --git a/internal/manifest/v1alpha/examples/slo_composite.go b/internal/manifest/v1alpha/examples/slo_composite.go index c9cda79f5..38edf3c04 100644 --- a/internal/manifest/v1alpha/examples/slo_composite.go +++ b/internal/manifest/v1alpha/examples/slo_composite.go @@ -28,7 +28,7 @@ func (s sloCompositeExample) GetSubVariant() string { func (s sloCompositeExample) GetYAMLComments() []string { return []string{ - fmt.Sprintf("Composite SLO"), + "Composite SLO", fmt.Sprintf("Budgeting method: %s", s.BudgetingMethod), fmt.Sprintf("Time window type: %s", s.TimeWindowType), } diff --git a/manifest/v1alpha/report/report.go b/manifest/v1alpha/report/report.go index efe3870bc..9be1972af 100644 --- a/manifest/v1alpha/report/report.go +++ b/manifest/v1alpha/report/report.go @@ -2,6 +2,7 @@ package report import ( "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha" ) //go:generate go run ../../../internal/cmd/objectimpl Report @@ -62,10 +63,10 @@ type CustomPeriod struct { } type Filters struct { - Projects []string `json:"projects,omitempty"` - Services Services `json:"services,omitempty"` - SLOs SLOs `json:"slos,omitempty"` - Labels Labels `json:"labels,omitempty"` + Projects []string `json:"projects,omitempty"` + Services Services `json:"services,omitempty"` + SLOs SLOs `json:"slos,omitempty"` + Labels v1alpha.Labels `json:"labels,omitempty"` } type Services []Service @@ -80,8 +81,4 @@ type SLO struct { Project string `json:"project" validate:"required"` } -type Labels map[LabelKey][]LabelValue -type LabelKey = string -type LabelValue = string - type ErrorBudgetStatusConfig struct{} diff --git a/manifest/v1alpha/report/system_health_review.go b/manifest/v1alpha/report/system_health_review.go index d17457bd3..7288308f3 100644 --- a/manifest/v1alpha/report/system_health_review.go +++ b/manifest/v1alpha/report/system_health_review.go @@ -1,6 +1,10 @@ package report -import "time" +import ( + "time" + + "github.com/nobl9/nobl9-go/manifest/v1alpha" +) type SystemHealthReviewConfig struct { TimeFrame SystemHealthReviewTimeFrame `json:"timeFrame" validate:"required"` @@ -19,8 +23,8 @@ type Thresholds struct { } type ColumnSpec struct { - DisplayName string `json:"displayName" validate:"required"` - Labels Labels `json:"labels" validate:"required"` + DisplayName string `json:"displayName" validate:"required"` + Labels v1alpha.Labels `json:"labels" validate:"required"` } type SnapshotTimeFrame struct { diff --git a/manifest/v1alpha/report/validation_system_health_review.go b/manifest/v1alpha/report/validation_system_health_review.go index 1cda762bb..c2972d50a 100644 --- a/manifest/v1alpha/report/validation_system_health_review.go +++ b/manifest/v1alpha/report/validation_system_health_review.go @@ -8,6 +8,8 @@ import ( "github.com/nobl9/govy/pkg/govy" "github.com/nobl9/govy/pkg/rules" + + "github.com/nobl9/nobl9-go/manifest/v1alpha" ) var systemHealthReviewValidation = govy.New[SystemHealthReviewConfig]( @@ -35,9 +37,9 @@ var columnValidation = govy.New[ColumnSpec]( WithName("displayName"). Required(). Rules(rules.StringMaxLength(63)), - govy.ForMap(func(c ColumnSpec) map[LabelKey][]LabelValue { return c.Labels }). + govy.ForMap(func(c ColumnSpec) v1alpha.Labels { return c.Labels }). WithName("labels"). - Rules(rules.MapMinLength[map[LabelKey][]LabelValue](1)), + Rules(rules.MapMinLength[v1alpha.Labels](1)), ) var timeFrameValidation = govy.New[SystemHealthReviewTimeFrame]( diff --git a/manifest/v1alpha/report/validation_test.go b/manifest/v1alpha/report/validation_test.go index e9d100f37..70ff571cf 100644 --- a/manifest/v1alpha/report/validation_test.go +++ b/manifest/v1alpha/report/validation_test.go @@ -13,6 +13,7 @@ import ( "github.com/nobl9/nobl9-go/internal/testutils" "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha" ) var validationMessageRegexp = regexp.MustCompile(strings.TrimSpace(` @@ -85,7 +86,7 @@ func TestValidate_Spec(t *testing.T) { Columns: []ColumnSpec{ { DisplayName: "Column 1", - Labels: map[LabelKey][]LabelValue{ + Labels: v1alpha.Labels{ "key1": {"value1"}, }, }, @@ -172,7 +173,7 @@ func TestValidate_Spec_Filters(t *testing.T) { }, }, Filters: &Filters{ - Labels: map[LabelKey][]LabelValue{ + Labels: v1alpha.Labels{ "key1": {"value1"}, }, }, @@ -619,7 +620,7 @@ func TestValidate_Spec_SLOHistory_TimeFrame(t *testing.T) { } func TestValidate_Spec_SystemHealthReview(t *testing.T) { - properLabel := map[LabelKey][]LabelValue{"key1": {"value1"}} + properLabel := v1alpha.Labels{"key1": {"value1"}} for name, test := range map[string]struct { ExpectedErrorsCount int ExpectedErrors []testutils.ExpectedError @@ -742,7 +743,7 @@ func TestValidate_Spec_SystemHealthReview(t *testing.T) { }, RowGroupBy: RowGroupByProject, Columns: []ColumnSpec{ - {DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{}}, + {DisplayName: "Column 1", Labels: v1alpha.Labels{}}, }, Thresholds: Thresholds{ RedLessThanOrEqual: ptr(0.0), @@ -902,7 +903,7 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) { Config: SystemHealthReviewConfig{ RowGroupBy: RowGroupByProject, Columns: []ColumnSpec{ - {DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}}, + {DisplayName: "Column 1", Labels: v1alpha.Labels{"key1": {"value1"}}}, }, Thresholds: Thresholds{ RedLessThanOrEqual: ptr(0.0), @@ -925,7 +926,7 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) { }, RowGroupBy: RowGroupByProject, Columns: []ColumnSpec{ - {DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}}, + {DisplayName: "Column 1", Labels: v1alpha.Labels{"key1": {"value1"}}}, }, Thresholds: Thresholds{ RedLessThanOrEqual: ptr(0.0), @@ -950,7 +951,7 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) { }, RowGroupBy: RowGroupByProject, Columns: []ColumnSpec{ - {DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}}, + {DisplayName: "Column 1", Labels: v1alpha.Labels{"key1": {"value1"}}}, }, Thresholds: Thresholds{ RedLessThanOrEqual: ptr(0.0), @@ -980,7 +981,7 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) { }, RowGroupBy: RowGroupByProject, Columns: []ColumnSpec{ - {DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}}, + {DisplayName: "Column 1", Labels: v1alpha.Labels{"key1": {"value1"}}}, }, Thresholds: Thresholds{ RedLessThanOrEqual: ptr(0.0), @@ -1010,7 +1011,7 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) { }, RowGroupBy: RowGroupByProject, Columns: []ColumnSpec{ - {DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}}, + {DisplayName: "Column 1", Labels: v1alpha.Labels{"key1": {"value1"}}}, }, Thresholds: Thresholds{ RedLessThanOrEqual: ptr(0.0), @@ -1037,7 +1038,7 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) { }, RowGroupBy: RowGroupByProject, Columns: []ColumnSpec{ - {DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}}, + {DisplayName: "Column 1", Labels: v1alpha.Labels{"key1": {"value1"}}}, }, Thresholds: Thresholds{ RedLessThanOrEqual: ptr(0.0), @@ -1068,7 +1069,7 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) { }, RowGroupBy: RowGroupByProject, Columns: []ColumnSpec{ - {DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}}, + {DisplayName: "Column 1", Labels: v1alpha.Labels{"key1": {"value1"}}}, }, Thresholds: Thresholds{ RedLessThanOrEqual: ptr(0.0), @@ -1094,7 +1095,7 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) { }, RowGroupBy: RowGroupByProject, Columns: []ColumnSpec{ - {DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}}, + {DisplayName: "Column 1", Labels: v1alpha.Labels{"key1": {"value1"}}}, }, Thresholds: Thresholds{ RedLessThanOrEqual: ptr(0.0), diff --git a/manifest/v1alpha/report/validation_time_frame.go b/manifest/v1alpha/report/validation_time_frame.go index 74e3b069a..bbdb97376 100644 --- a/manifest/v1alpha/report/validation_time_frame.go +++ b/manifest/v1alpha/report/validation_time_frame.go @@ -96,7 +96,7 @@ var calendarTimeFrameValidation = govy.New[CalendarTimeFrame]( }), govy.NewRule(func(t CalendarTimeFrame) error { if t.Count != nil && t.Unit != nil && - !(*t.Count == 1 && (*t.Unit == week || *t.Unit == month || *t.Unit == quarter || *t.Unit == year)) { + (*t.Count != 1 || *t.Unit != week && *t.Unit != month && *t.Unit != quarter && *t.Unit != year) { return errors.New("valid 'unit' and 'count' pairs are: 1 Week, 1 Month, 1 Quarter, 1 Year") } return nil diff --git a/tests/v1alpha_report_test.go b/tests/v1alpha_report_test.go index 3c43bc500..de8f31f90 100644 --- a/tests/v1alpha_report_test.go +++ b/tests/v1alpha_report_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "github.com/nobl9/nobl9-go/manifest" + "github.com/nobl9/nobl9-go/manifest/v1alpha" v1alphaReport "github.com/nobl9/nobl9-go/manifest/v1alpha/report" objectsV1 "github.com/nobl9/nobl9-go/sdk/endpoints/objects/v1" ) @@ -42,7 +43,7 @@ func Test_Objects_V1_V1alpha_Report(t *testing.T) { Columns: []v1alphaReport.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "team": {"grey"}, }, }, @@ -76,7 +77,7 @@ func Test_Objects_V1_V1alpha_Report(t *testing.T) { Columns: []v1alphaReport.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "team": {"grey"}, }, }, @@ -111,7 +112,7 @@ func Test_Objects_V1_V1alpha_Report(t *testing.T) { Columns: []v1alphaReport.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "team": {"grey"}, }, }, @@ -146,7 +147,7 @@ func Test_Objects_V1_V1alpha_Report(t *testing.T) { Columns: []v1alphaReport.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "team": {"grey"}, }, }, @@ -179,7 +180,7 @@ func Test_Objects_V1_V1alpha_Report(t *testing.T) { Columns: []v1alphaReport.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "team": {"grey"}, }, }, @@ -286,7 +287,7 @@ func Test_Objects_V1_V1alpha_ReportErrors(t *testing.T) { Columns: []v1alphaReport.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "team": {"grey"}, }, }, @@ -327,7 +328,7 @@ func Test_Objects_V1_V1alpha_ReportErrors(t *testing.T) { Columns: []v1alphaReport.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "team": {"grey"}, }, }, @@ -368,7 +369,7 @@ func Test_Objects_V1_V1alpha_ReportErrors(t *testing.T) { Columns: []v1alphaReport.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "team": {"grey"}, }, }, @@ -392,7 +393,7 @@ func Test_Objects_V1_V1alpha_ReportErrors(t *testing.T) { Shared: true, Filters: &v1alphaReport.Filters{ Projects: []string{project.GetName()}, - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "non-existing-label": {"non-existing-value"}, }, }, @@ -407,7 +408,7 @@ func Test_Objects_V1_V1alpha_ReportErrors(t *testing.T) { Columns: []v1alphaReport.ColumnSpec{ { DisplayName: "Column 1", - Labels: map[v1alphaReport.LabelKey][]v1alphaReport.LabelValue{ + Labels: v1alpha.Labels{ "team": {"grey"}, }, }, diff --git a/tests/v1alpha_slo_test.go b/tests/v1alpha_slo_test.go index c4ddd974a..2a16c0063 100644 --- a/tests/v1alpha_slo_test.go +++ b/tests/v1alpha_slo_test.go @@ -105,12 +105,12 @@ func Test_Objects_V1_V1alpha_SLO(t *testing.T) { slo.Spec.AlertPolicies = []string{alertPolicy.GetName()} if slo.Spec.HasCompositeObjectives() { - for componentIndex, component := range slo.Spec.Objectives[0].Composite.Components.Objectives { + for componentIndex, component := range slo.Spec.Objectives[0].Composite.Objectives { componentSlo := slos[len(slos)-1-componentIndex].(v1alphaSLO.SLO) component.Project = componentSlo.Metadata.Project component.SLO = componentSlo.Metadata.Name component.Objective = componentSlo.Spec.Objectives[0].Name - slo.Spec.Objectives[0].Composite.Components.Objectives[componentIndex] = component + slo.Spec.Objectives[0].Composite.Objectives[componentIndex] = component } } else { slo.Spec.AnomalyConfig.NoData.AlertMethods = []v1alphaSLO.AnomalyConfigAlertMethod{