feat: PC-13893 Deprecate usage of objective's value field for Composi…

…te SLOs (#295)

## Motivation

Currently, we have `value` field required for all objectives in an SLO.
We currently allow to not pass that field in YAML at all but it causes
it to default to 0 anyway and is returned `value: 0`. This is
inconsistent because GET yaml is different than APPLY YAML in such case.

Composite SLOs always have exactly one objective, so as such `value`
doesn’t matter for them at all as long as it doesn't change.

Moreover, the existence of value is documented and used in examples
which is confusing to new adopters of Composite SLOs because it is
required, changing it will restart budget, but it doesn’t do anything.

We want to encourage and allow not setting `value` field for Composite
SLOs while maintaining backward compatibility with users who perhaps
already explicitly set it.

## Summary

`value` field of SLO's `spec.objective` is now optional.

`value` is removed from all examples of Composite SLOs.

Nobl9 SDK is temporarily redirected unreleased version containing
unreleased changes required for this to work.

## Related Changes

nobl9/nobl9-go#551
nobl9/nobl9-go#549
nobl9/n9#15406
#312
#295

## Testing

- Describe how to check introduced code changes manually.
- Take care of test coverage on unit, integration or end-to-end levels.

## Release Notes

Usage of `value` field in schema for `objective` (part of SLO resource)
becomes deprecated for objectives using `composite` section.
* New Composite SLOs should not set `value` field.
* New Composite SLOs will still accept `value: 0` for backward
compatibility with older versions of Nobl9 SDK and Nobl9 Terrafrom
Provider.
* If `value` was previously set to `0` for Composite SLO then it should
be omitted going forward.
* If `value` was previously set in Composite SLO to a number other than
`0` then it can no longer be updated but still will be accepted for
backward compatibility.

The usage of `value` for SLOs using ratio or threshold SLIs does not
change.

Makefile

@@ -9,7 +9,7 @@ BIN_DIR=./bin
 BINARY=$(BIN_DIR)/terraform-provider-$(NAME)
 VERSION=0.32.1
 BUILD_FLAGS="-X github.com/nobl9/terraform-provider-nobl9/nobl9.Version=$(VERSION)"
-OS_ARCH?=linux_amd64
+OS_ARCH?=darwin_arm64
 
 # renovate datasource=github-releases depName=securego/gosec
 GOSEC_VERSION := v2.21.4

docs/resources/slo.md

@@ -164,7 +164,6 @@ resource "nobl9_slo" "composite_slo" {
     display_name = "OK"
     name         = "tf-objective-1"
     target       = 0.8
-    value        = 1
     composite {
       max_delay = "45m"
       components {
@@ -227,7 +226,6 @@ resource "nobl9_slo" "composite_slo" {
 Required:
 
 - `target` (Number) The numeric target for your objective.
-- `value` (Number) For threshold metrics, the threshold value. For ratio metrics, for legacy reasons, this must be a unique value per objective.
 
 Optional:
 
@@ -239,6 +237,7 @@ Optional:
 - `primary` (Boolean) Is objective marked as primary.
 - `raw_metric` (Block Set) Raw data is used to compare objective values. (see [below for nested schema](#nestedblock--objective--raw_metric))
 - `time_slice_target` (Number) Designated value for slice.
+- `value` (Number) Required for threshold and ratio metrics. Optional for composite SLOs. For thresholdmetrics, the threshold value. For ratio metrics, for legacy reasons, this must be a unique valueper objective. For composite SLOs it should be omitted. If, for composite SLO, it was setpreviously to a non zero value then it should remain set to that value.
 
 <a id="nestedblock--objective--composite"></a>
 ### Nested Schema for `objective.composite`

examples/resources/nobl9_slo/resource.tf

@@ -144,7 +144,6 @@ resource "nobl9_slo" "composite_slo" {
     display_name = "OK"
     name         = "tf-objective-1"
     target       = 0.8
-    value        = 1
     composite {
       max_delay = "45m"
       components {

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/hashicorp/go-cty v1.4.1-0.20200723130312-85980079f637
 	github.com/hashicorp/terraform-plugin-docs v0.19.4
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.34.0
-	github.com/nobl9/nobl9-go v0.90.1
+	github.com/nobl9/nobl9-go v0.92.0
 	github.com/stretchr/testify v1.9.0
 	github.com/teambition/rrule-go v1.8.2
 )

go.sum

@@ -180,8 +180,8 @@ github.com/nobl9/go-yaml v1.0.1 h1:Aj1kSaYdRQTKlvS6ihvXzQJhCpoHhtf9nfA95zqWH4Q=
 github.com/nobl9/go-yaml v1.0.1/go.mod h1:t7vCO8ctYdBweZxU5lUgxzAw31+ZcqJYeqRtrv+5RHI=
 github.com/nobl9/govy v0.9.1 h1:ht+mgbDB40QEZEmHpzG2xqey+PbfzCwgj3+vdJuHAVQ=
 github.com/nobl9/govy v0.9.1/go.mod h1:O+xSiKwZ6gs/orRvH5qLkfkgyT7CkuXprRIq3C5uNXQ=
-github.com/nobl9/nobl9-go v0.90.1 h1:OX/0jVrklISXk4Q9FiMcX/SaopLxwXuFNFgvPPsc1fY=
-github.com/nobl9/nobl9-go v0.90.1/go.mod h1:SWvmmfSGCairnWDCdRLusRB0zROxF+7XiA9aho2f7rA=
+github.com/nobl9/nobl9-go v0.92.0 h1:pw2PiBR60V7tpojN4QGEeJzuNMZx1h8kwOp/dmxz49Q=
+github.com/nobl9/nobl9-go v0.92.0/go.mod h1:SWvmmfSGCairnWDCdRLusRB0zROxF+7XiA9aho2f7rA=
 github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA=
 github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU=
 github.com/pjbgf/sha1cd v0.3.0 h1:4D5XXmUUBUl/xQ6IjCkEAbqXskkq/4O7LmGn0AqMDs4=

nobl9/resource_slo.go

@@ -149,9 +149,12 @@ func resourceObjective() *schema.Resource {
 				Description: "Designated value for slice.",
 			},
 			"value": {
-				Type:        schema.TypeFloat,
-				Required:    true,
-				Description: "For threshold metrics, the threshold value. For ratio metrics, for legacy reasons, this must be a unique value per objective.",
+				Type:     schema.TypeFloat,
+				Optional: true,
+				Description: "Required for threshold and ratio metrics. Optional for composite SLOs. For threshold" +
+					"metrics, the threshold value. For ratio metrics, for legacy reasons, this must be a unique value" +
+					"per objective. For composite SLOs it should be omitted. If, for composite SLO, it was set" +
+					"previously to a non zero value then it should remain set to that value.",
 			},
 			"name": {
 				Type:        schema.TypeString,
@@ -362,8 +365,7 @@ func resourceSLOApply(
 	resultSlo := manifest.SetDefaultProject([]manifest.Object{slo}, config.Project)
 
 	if err := retry.RetryContext(ctx, d.Timeout(schema.TimeoutCreate)-time.Minute, func() *retry.RetryError {
-		err := client.Objects().V1().Apply(ctx, resultSlo)
-		if err != nil {
+		if err := client.Objects().V1().Apply(ctx, resultSlo); err != nil {
 			if errors.Is(err, errConcurrencyIssue) {
 				return retry.RetryableError(err)
 			}
@@ -595,11 +597,11 @@ func marshalIndicator(d *schema.ResourceData) *v1alphaSLO.Indicator {
 }
 
 func marshalObjectives(d *schema.ResourceData) ([]v1alphaSLO.Objective, diag.Diagnostics) {
-	objectivesSchema := d.Get("objective").(*schema.Set).List()
+	objectivesSchemaSet := d.Get("objective").(*schema.Set)
+	objectivesSchema := objectivesSchemaSet.List()
 	objectives := make([]v1alphaSLO.Objective, len(objectivesSchema))
 	for i, o := range objectivesSchema {
 		objective := o.(map[string]interface{})
-		value := objective["value"].(float64)
 		target := objective["target"].(float64)
 		timeSliceTarget := objective["time_slice_target"].(float64)
 		var timeSliceTargetPtr *float64
@@ -612,11 +614,16 @@ func marshalObjectives(d *schema.ResourceData) ([]v1alphaSLO.Objective, diag.Dia
 		if err != nil {
 			return nil, diag.FromErr(err)
 		}
-
+		var valuePtr *float64
+		value := objective["value"].(float64)
+		valuePtr = &value
+		if value == 0 && compositeSpec != nil {
+			valuePtr = nil
+		}
 		objectives[i] = v1alphaSLO.Objective{
 			ObjectiveBase: v1alphaSLO.ObjectiveBase{
 				DisplayName: objective["display_name"].(string),
-				Value:       &value,
+				Value:       valuePtr,
 				Name:        objective["name"].(string),
 			},
 			BudgetTarget:    &target,
@@ -861,7 +868,9 @@ func unmarshalObjectives(d *schema.ResourceData, spec v1alphaSLO.Spec) error {
 		objectiveTF["name"] = objective.Name
 		objectiveTF["display_name"] = objective.DisplayName
 		objectiveTF["op"] = objective.Operator
-		objectiveTF["value"] = objective.Value
+		if objective.Value != nil && (*objective.Value != 0 || objective.Composite == nil) {
+			objectiveTF["value"] = objective.Value
+		}
 		objectiveTF["target"] = objective.BudgetTarget
 		objectiveTF["time_slice_target"] = objective.TimeSliceTarget
 		objectiveTF["primary"] = objective.Primary
@@ -1841,6 +1850,7 @@ func unmarshalHoneycombMetric(metric interface{}) map[string]interface{} {
 		return nil
 	}
 	res := make(map[string]interface{})
+	// nolint: staticcheck
 	res["calculation"] = hMetric.Calculation
 	res["attribute"] = hMetric.Attribute
 	return res

nobl9/resource_slo_test.go

@@ -30,6 +30,7 @@ func TestAcc_Nobl9SLO(t *testing.T) {
 		{"test-composite-time-slices-deprecated", testCompositeSLOTimeSlicesDeprecated},
 		{"test-composite-occurrences", testCompositeSLOOccurrences},
 		{"test-composite-time-slices", testCompositeSLOTimeSlices},
+		{"test-composite-with-value", testCompositeSLOValueZeroBackwardCompatibility},
 		{"test-datadog", testDatadogSLO},
 		{"test-dynatrace", testDynatraceSLO},
 		{"test-grafanaloki", testGrafanaLokiSLO},
@@ -1023,7 +1024,6 @@ resource "nobl9_slo" ":name" {
    display_name = "obj1"
    name         = "tf-objective-1"
    target       = 0.7
-   value        = 0
    composite {
      max_delay = "45m"
      components {
@@ -1085,7 +1085,7 @@ resource "nobl9_slo" ":name" {
    display_name = "obj1"
    name         = "tf-objective-1"
    target       = 0.7
-   value        = 0
+   value        = 1
    time_slice_target = 0.7
    composite {
      max_delay = "45m"
@@ -1126,6 +1126,79 @@ resource "nobl9_slo" ":name" {
 	return config
 }
 
+func testCompositeSLOValueZeroBackwardCompatibility(name string) string {
+	var serviceName = name + "-tf-service"
+	var agentName = name + "-tf-agent"
+	var sloDependencyName = name + "-dependency-slo-1"
+	config :=
+		testService(serviceName) +
+			testPrometheusAgent(agentName) +
+			testCompositeDependencySLO(serviceName, agentName, sloDependencyName) + `
+resource "nobl9_slo" ":name" {
+ name         = ":name"
+ display_name = ":name"
+ project      = ":project"
+ service      = nobl9_service.:serviceName.name
+
+
+ depends_on = [nobl9_slo.:sloDependencyName]
+
+ label {
+  key = "team"
+  values = ["green","sapphire"]
+ }
+
+ label {
+  key = "env"
+  values = ["dev", "staging", "prod"]
+ }
+
+ budgeting_method = "Occurrences"
+
+ objective {
+   display_name = "obj1"
+   name         = "tf-objective-1"
+   target       = 0.7
+   value        = 0
+   composite {
+     max_delay = "45m"
+     components {
+       objectives {
+         composite_objective {
+           project      = ":project"
+           slo          = ":sloDependencyName"
+           objective    = "objective-1"
+           weight       = 0.8
+           when_delayed = "CountAsGood"
+         }
+		 composite_objective {
+           project      = ":project"
+           slo          = ":sloDependencyName"
+           objective    = "objective-2"
+           weight       = 1.0
+           when_delayed = "CountAsBad"
+         }
+       }
+     }
+   }
+ }
+
+ time_window {
+   count      = 10
+   is_rolling = true
+   unit       = "Minute"
+ }
+}
+`
+	config = strings.ReplaceAll(config, ":name", name)
+	config = strings.ReplaceAll(config, ":serviceName", serviceName)
+	config = strings.ReplaceAll(config, ":agentName", agentName)
+	config = strings.ReplaceAll(config, ":project", testProject)
+	config = strings.ReplaceAll(config, ":sloDependencyName", sloDependencyName)
+
+	return config
+}
+
 func testDatadogSLO(name string) string {
 	var serviceName = name + "-tf-service"
 	var agentName = name + "-tf-agent"