Skip to content

Commit 5d4dd4a

Browse files
fix(slos): rely on StripSLOServerFields to clear dash0.com/id
Upgrade to dash0-api-client-go v1.18.2, which clears dash0.com/id in StripSLOServerFields (dash0hq/dash0-api-client-go#24). This removes the local ClearSLOID workarounds that stood in for the missing behaviour. Also adds the SLO case to marshalForDiff. SLO was the only asset type absent from the type switch, so neither side of a diff was normalized and re-applying an unchanged document rendered a spurious hunk from the server-bumped dash0.com/updated-at and dash0.com/version instead of reporting "no changes". slos update -f now falls back to dash0.com/origin when no id is present, matching notification channels and teams. SLO ids are server-assigned, so a hand-authored document can only pin an origin — the flow the command's own help advertises was previously impossible.
1 parent ef437df commit 5d4dd4a

6 files changed

Lines changed: 81 additions & 19 deletions

File tree

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ go 1.25.1
44

55
require (
66
github.com/cli/browser v1.3.0
7-
github.com/dash0hq/dash0-api-client-go v1.18.1
7+
github.com/dash0hq/dash0-api-client-go v1.18.2
88
github.com/google/uuid v1.6.0
99
github.com/muesli/termenv v0.16.0
1010
github.com/pmezard/go-difflib v1.0.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XL
1111
github.com/cli/browser v1.3.0 h1:LejqCrpWr+1pRqmEPDGnTZOjsMe7sehifLynZJuqJpo=
1212
github.com/cli/browser v1.3.0/go.mod h1:HH8s+fOAxjhQoBUAsKuPCbqUuxZDhQ2/aD+SzsEfBTk=
1313
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
14-
github.com/dash0hq/dash0-api-client-go v1.18.1 h1:NhIt93aEj6C0YmVIWODESDbUbUTVwJ0JEFmC7qi54kw=
15-
github.com/dash0hq/dash0-api-client-go v1.18.1/go.mod h1:KNGbcgETrEN4m2QPGAorZ9FybTpVg/oi20UEoWGCGlE=
14+
github.com/dash0hq/dash0-api-client-go v1.18.2 h1:o+txjfJifmRHxIlXweZOdXJz6AzYfD4ivLFA+rjoHw8=
15+
github.com/dash0hq/dash0-api-client-go v1.18.2/go.mod h1:KNGbcgETrEN4m2QPGAorZ9FybTpVg/oi20UEoWGCGlE=
1616
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
1717
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
1818
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

internal/asset/diff_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,60 @@ func TestPrintDiff_SLO_ServerBumpedMetadataIsNotAChange(t *testing.T) {
355355
assert.NotContains(t, buf.String(), "dash0.com/version")
356356
}
357357

358+
// A hand-authored SLO document can only pin dash0.com/origin — ids are
359+
// server-assigned — so on `slos update -f <origin-only file>` the `before`
360+
// fetched from the API carries an id that the `after` never will. The id is
361+
// server-managed metadata, so its absence from the file must not render as a
362+
// deletion. Regression test for a phantom "-dash0.com/id / +labels: {}" diff.
363+
func TestPrintDiff_SLO_ServerAssignedIDIsNotAChange(t *testing.T) {
364+
dashcolor.NoColor = true
365+
defer func() { dashcolor.NoColor = false }()
366+
367+
// before: fetched by origin, so the server filled in id/version/dataset.
368+
beforeJSON := `{
369+
"apiVersion": "openslo.com/v1",
370+
"kind": "SLO",
371+
"metadata": {
372+
"name": "checkout-availability",
373+
"labels": {
374+
"dash0.com/id": "slo_01kyn68gwrf22afc4vc9xjdtg5",
375+
"dash0.com/version": "3",
376+
"dash0.com/dataset": "default",
377+
"dash0.com/origin": "manual-test-checkout-api"
378+
},
379+
"annotations": {
380+
"dash0.com/display-name": "Checkout API availability",
381+
"dash0.com/created-at": "2026-01-15T10:00:00Z"
382+
}
383+
},
384+
"spec": {"service": "checkout", "budgetingMethod": "Occurrences"}
385+
}`
386+
// after: the user's file — origin only, no id, no server metadata. It does
387+
// carry the client-settable display-name annotation, as a real authored
388+
// document does; that keeps the comparison about the id alone.
389+
afterJSON := `{
390+
"apiVersion": "openslo.com/v1",
391+
"kind": "SLO",
392+
"metadata": {
393+
"name": "checkout-availability",
394+
"labels": {"dash0.com/origin": "manual-test-checkout-api"},
395+
"annotations": {"dash0.com/display-name": "Checkout API availability"}
396+
},
397+
"spec": {"service": "checkout", "budgetingMethod": "Occurrences"}
398+
}`
399+
400+
var before, after dash0api.SloDefinition
401+
require.NoError(t, json.Unmarshal([]byte(beforeJSON), &before))
402+
require.NoError(t, json.Unmarshal([]byte(afterJSON), &after))
403+
404+
var buf bytes.Buffer
405+
require.NoError(t, PrintDiff(&buf, "SLO", "checkout-availability", &before, &after))
406+
407+
assert.Contains(t, buf.String(), `SLO "checkout-availability": no changes`)
408+
assert.NotContains(t, buf.String(), "dash0.com/id", "the server-assigned id must not appear as a change")
409+
assert.NotContains(t, buf.String(), "labels: {}", "stripping every label must not render an empty-map diff")
410+
}
411+
358412
// TestPrintDiff_SLO_RealSpecChangeStillDiffs is the counterpart guard: stripping
359413
// server-managed metadata must not swallow a genuine change to the document.
360414
func TestPrintDiff_SLO_RealSpecChangeStillDiffs(t *testing.T) {

internal/asset/slo.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ import (
2727
// - Otherwise, POST is used and the server assigns both id and origin.
2828
//
2929
// PUT is create-or-replace, so upserting on either key is idempotent across
30-
// repeated applies. The origin and id labels are captured before
31-
// StripSLOServerFields runs because that helper clears dash0.com/origin.
30+
// repeated applies. Both labels are captured before StripSLOServerFields runs,
31+
// because that helper clears dash0.com/origin and dash0.com/id.
3232
func ImportSLO(ctx context.Context, apiClient dash0api.Client, slo *dash0api.SloDefinition, dataset *string) (ImportResult, error) {
33-
// Capture identifiers before stripping — StripSLOServerFields clears the
34-
// dash0.com/origin label, so origin- and id-based routing must observe the
35-
// input first.
33+
// Capture identifiers before stripping — StripSLOServerFields clears both
34+
// the dash0.com/origin and dash0.com/id labels, so origin- and id-based
35+
// routing must observe the input first.
3636
origin := ""
3737
if slo.Metadata.Labels != nil && slo.Metadata.Labels.Dash0Comorigin != nil {
3838
origin = *slo.Metadata.Labels.Dash0Comorigin
@@ -74,12 +74,9 @@ func ImportSLO(ctx context.Context, apiClient dash0api.Client, slo *dash0api.Slo
7474
if upsertKey != "" {
7575
result, err = apiClient.UpdateSLO(ctx, upsertKey, slo, dataset)
7676
} else {
77-
// StripSLOServerFields does not touch dash0.com/id, so on the
78-
// preflight-404 fallback the body would still carry the id that belongs
79-
// to the *source* organization. The server assigns SLO ids on create, so
80-
// send the document without one rather than asking it to ignore a
81-
// foreign identifier.
82-
dash0api.ClearSLOID(slo)
77+
// No explicit ClearSLOID here: StripSLOServerFields above already
78+
// removed dash0.com/id, so the cross-environment POST fallback cannot
79+
// carry an identifier belonging to the source organization.
8380
result, err = apiClient.CreateSLO(ctx, slo, dataset)
8481
}
8582
if err != nil {

internal/slos/integration_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -766,9 +766,10 @@ func TestCreateSLOFromFile_UpsertByID_FallsBackToPOSTWhenNotFound(t *testing.T)
766766
assertPOSTPath(t, server.Requests(), apiPathSLOs, "expected POST fallback after GET 404")
767767

768768
// The POST body must not carry the source organization's id. SLO ids are
769-
// assigned by the server on create, and StripSLOServerFields does not clear
770-
// dash0.com/id (only version, origin, dataset, source, and timestamps), so
771-
// ImportSLO calls ClearSLOID explicitly on this path.
769+
// assigned by the server on create, and StripSLOServerFields clears
770+
// dash0.com/id along with version, origin, dataset, source, and the
771+
// timestamps — so the cross-environment fallback cannot leak a foreign
772+
// identifier. This assertion pins that guarantee at the wire level.
772773
post := findRecordedRequest(server.Requests(), http.MethodPost, apiPathSLOs)
773774
require.NotNil(t, post, "expected a recorded POST to %s", apiPathSLOs)
774775
var body struct {

internal/slos/update.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,25 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags)
4949

5050
var id string
5151
fileID := dash0api.GetSLOID(&slo)
52+
// Fall back to dash0.com/origin when the document carries no id. SLO ids are
53+
// server-assigned (`slo_<ulid>`), so a hand-authored document can only ever
54+
// pin an origin — without this fallback the `update -f <file>` form this
55+
// command advertises is impossible for any SLO that was not first exported.
56+
// GET/PUT accept an origin-or-id path segment, so routing on origin works.
57+
// There is no GetSLOOrigin helper in the API client, so the label is read
58+
// directly (as ImportSLO and apply's parseDocumentHeader do).
59+
if fileID == "" && slo.Metadata.Labels != nil && slo.Metadata.Labels.Dash0Comorigin != nil {
60+
fileID = *slo.Metadata.Labels.Dash0Comorigin
61+
}
5262
if len(args) == 1 {
5363
id = args[0]
5464
if fileID != "" && fileID != id {
55-
return fmt.Errorf("the ID argument %q does not match the ID in the file %q", id, fileID)
65+
return fmt.Errorf("the ID argument %q does not match the ID or origin in the file %q", id, fileID)
5666
}
5767
} else {
5868
id = fileID
5969
if id == "" {
60-
return fmt.Errorf("no SLO ID provided as argument, and the file does not contain an ID")
70+
return fmt.Errorf("no SLO ID provided as argument, and the file contains neither an ID nor a dash0.com/origin label")
6171
}
6272
}
6373

0 commit comments

Comments
 (0)