From e2c5e58a70786eb642117c607ab37bc61934fc2a Mon Sep 17 00:00:00 2001 From: Markus Vahlenkamp Date: Fri, 6 Nov 2026 14:48:26 +0100 Subject: [PATCH 1/8] feat(api): add pkg/api domain types, error sentinels, IsRetryable and ComputeCreateDSHash Signed-off-by: Markus Vahlenkamp --- pkg/api/datastore.go | 124 +++++++++++++++++++++++++++++++++++++++++ pkg/api/deviation.go | 39 +++++++++++++ pkg/api/doc.go | 6 ++ pkg/api/errors.go | 36 ++++++++++++ pkg/api/errors_test.go | 56 +++++++++++++++++++ pkg/api/hash.go | 108 +++++++++++++++++++++++++++++++++++ pkg/api/hash_test.go | 115 ++++++++++++++++++++++++++++++++++++++ pkg/api/schema.go | 28 ++++++++++ pkg/api/transaction.go | 73 ++++++++++++++++++++++++ 9 files changed, 585 insertions(+) create mode 100644 pkg/api/datastore.go create mode 100644 pkg/api/deviation.go create mode 100644 pkg/api/doc.go create mode 100644 pkg/api/errors.go create mode 100644 pkg/api/errors_test.go create mode 100644 pkg/api/hash.go create mode 100644 pkg/api/hash_test.go create mode 100644 pkg/api/schema.go create mode 100644 pkg/api/transaction.go diff --git a/pkg/api/datastore.go b/pkg/api/datastore.go new file mode 100644 index 00000000..46c6bd0d --- /dev/null +++ b/pkg/api/datastore.go @@ -0,0 +1,124 @@ +package api + +// TargetType names the southbound protocol of a datastore target. +type TargetType string + +const ( + TargetTypeGNMI TargetType = "gnmi" + TargetTypeNetconf TargetType = "netconf" +) + +// ConnectionState reports the southbound connection state of a datastore. +type ConnectionState int + +const ( + ConnectionUnknown ConnectionState = iota + ConnectionConnected + ConnectionNotConnected +) + +// Credentials authenticate against a target. +type Credentials struct { + Username string + Password string + Token string +} + +// TLS configures the client side of the target connection. +type TLS struct { + CA string + Cert string + Key string + SkipVerify bool +} + +// CommitCandidate selects which NETCONF datastore is committed. +type CommitCandidate int + +const ( + CommitCandidateCandidate CommitCandidate = iota + CommitCandidateRunning +) + +// GNMIOptions are the gNMI-specific protocol options. +type GNMIOptions struct { + Encoding string + TargetName string +} + +// NetconfOptions are the NETCONF-specific protocol options. +type NetconfOptions struct { + IncludeNs bool + OperationWithNs bool + UseOperationRemove bool + CommitCandidate CommitCandidate +} + +// ProtocolOptions is a union: at most one of GNMI and Netconf is set. +type ProtocolOptions struct { + GNMI *GNMIOptions + Netconf *NetconfOptions +} + +// Target describes how to reach a device. +type Target struct { + Type TargetType + Address string + Port uint32 + Credentials *Credentials + TLS *TLS + ProtocolOptions ProtocolOptions +} + +// SchemaRef identifies a schema by vendor and version. +type SchemaRef struct { + Vendor string + Version string +} + +// SyncMode selects how a sync strategy collects device state. +type SyncMode int + +const ( + SyncModeOnChange SyncMode = iota + SyncModeSample + SyncModeOnce + SyncModeGet +) + +// SyncConfig is one sync strategy. Target, when set, overrides the datastore +// target for this strategy. +type SyncConfig struct { + Name string + Target *Target + Paths []string + Mode SyncMode + Interval uint64 +} + +// Sync is the datastore sync strategy. +type Sync struct { + Validate bool + Buffer int64 + WriteWorkers int64 + Configs []SyncConfig +} + +// DatastoreSpec is the desired state used to create a datastore. +type DatastoreSpec struct { + Name string + Target Target + Schema SchemaRef + Sync Sync +} + +// DatastoreInfo is the observed state of a datastore. +type DatastoreInfo struct { + Name string + ConnectionState ConnectionState + // StatusDetails explains a not-connected state when available. + StatusDetails string + Intents []string + // Spec is the effective spec the datastore runs with. + Spec DatastoreSpec +} diff --git a/pkg/api/deviation.go b/pkg/api/deviation.go new file mode 100644 index 00000000..478aa211 --- /dev/null +++ b/pkg/api/deviation.go @@ -0,0 +1,39 @@ +package api + +// DeviationKind classifies a DeviationEvent. +type DeviationKind int + +const ( + DeviationStart DeviationKind = iota + DeviationUpdate + DeviationEnd + DeviationClear +) + +// DeviationReason explains why a deviation exists. +type DeviationReason int + +const ( + DeviationReasonUnknown DeviationReason = iota + DeviationReasonUnhandled + DeviationReasonNotApplied + DeviationReasonOverruled + DeviationReasonIntentExists +) + +// DeviationEntry is one deviating path with its expected and current values. +type DeviationEntry struct { + Path string + Expected string + Current string +} + +// DeviationEvent is emitted by a deviation watch. Intent, Reason and Entries +// are set only where the Kind carries them. +type DeviationEvent struct { + Datastore string + Kind DeviationKind + Intent string + Reason DeviationReason + Entries []DeviationEntry +} diff --git a/pkg/api/doc.go b/pkg/api/doc.go new file mode 100644 index 00000000..cf954f86 --- /dev/null +++ b/pkg/api/doc.go @@ -0,0 +1,6 @@ +// Package api holds the domain types, error sentinels and helpers shared by the +// data-server core and its consumers. It is types-only: it imports neither gRPC +// nor any data-server internals. The only sdcpb types it uses are TypedValue and +// Path, the value and address types of the domain; no other sdcpb type may +// appear here. +package api diff --git a/pkg/api/errors.go b/pkg/api/errors.go new file mode 100644 index 00000000..318cf7eb --- /dev/null +++ b/pkg/api/errors.go @@ -0,0 +1,36 @@ +package api + +import ( + "context" + "errors" +) + +// Sentinels are matched with errors.Is; implementations wrap them with context. +var ( + // ErrDatastoreLocked: another action holds the datastore; retry later. + ErrDatastoreLocked = errors.New("datastore is locked, other action is ongoing") + // ErrNotConnected: the southbound target is unreachable; retry later. + ErrNotConnected = errors.New("not connected") + // ErrResourceExhausted: a resource limit was hit; retry later. + ErrResourceExhausted = errors.New("resource exhausted") + // ErrUnknownDatastore: the named datastore does not exist. + ErrUnknownDatastore = errors.New("unknown datastore") + // ErrInvalidArgument: the request is malformed. + ErrInvalidArgument = errors.New("invalid argument") +) + +// IsRetryable reports whether err is transient, so the same request may +// succeed later. Untyped errors are not retryable. +func IsRetryable(err error) bool { + switch { + case err == nil: + return false + case errors.Is(err, ErrDatastoreLocked), + errors.Is(err, ErrNotConnected), + errors.Is(err, ErrResourceExhausted), + errors.Is(err, context.DeadlineExceeded): + return true + default: + return false + } +} diff --git a/pkg/api/errors_test.go b/pkg/api/errors_test.go new file mode 100644 index 00000000..eb9c35d1 --- /dev/null +++ b/pkg/api/errors_test.go @@ -0,0 +1,56 @@ +package api + +import ( + "context" + "errors" + "fmt" + "testing" +) + +func TestIsRetryable(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + {"nil", nil, false}, + {"locked", ErrDatastoreLocked, true}, + {"not connected", ErrNotConnected, true}, + {"resource exhausted", ErrResourceExhausted, true}, + {"deadline", context.DeadlineExceeded, true}, + {"wrapped locked", fmt.Errorf("ds1: %w", ErrDatastoreLocked), true}, + {"joined not connected", errors.Join(errors.New("x"), fmt.Errorf("a: %w", ErrNotConnected)), true}, + {"unknown datastore", ErrUnknownDatastore, false}, + {"invalid argument", ErrInvalidArgument, false}, + {"canceled", context.Canceled, false}, + {"untyped", errors.New("boom"), false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsRetryable(tt.err); got != tt.want { + t.Fatalf("IsRetryable(%v) = %v, want %v", tt.err, got, tt.want) + } + }) + } +} + +func TestSentinelsAreDistinct(t *testing.T) { + all := []error{ErrDatastoreLocked, ErrNotConnected, ErrResourceExhausted, ErrUnknownDatastore, ErrInvalidArgument} + for i, a := range all { + for j, b := range all { + if (i == j) != errors.Is(a, b) { + t.Fatalf("errors.Is(%v, %v) = %v", a, b, errors.Is(a, b)) + } + } + } +} + +func TestErrorsIsThroughWrapping(t *testing.T) { + err := fmt.Errorf("outer: %w", fmt.Errorf("inner: %w", ErrUnknownDatastore)) + if !errors.Is(err, ErrUnknownDatastore) { + t.Fatal("wrapped sentinel not matched") + } + if errors.Is(err, ErrInvalidArgument) { + t.Fatal("unrelated sentinel matched") + } +} diff --git a/pkg/api/hash.go b/pkg/api/hash.go new file mode 100644 index 00000000..322d799d --- /dev/null +++ b/pkg/api/hash.go @@ -0,0 +1,108 @@ +package api + +import ( + "crypto/sha256" + "encoding/binary" + "encoding/hex" + "hash" +) + +// ComputeCreateDSHash returns a stable hex digest of the spec. Equal specs hash +// equal and any field change alters the digest, so callers recreate a datastore +// when it differs. extra values (for example resource versions of referenced +// objects) are mixed in, in order. +func ComputeCreateDSHash(spec DatastoreSpec, extra ...string) string { + w := hashWriter{h: sha256.New()} + w.str(spec.Name) + w.target(&spec.Target) + w.str(spec.Schema.Vendor) + w.str(spec.Schema.Version) + w.bool(spec.Sync.Validate) + w.i64(spec.Sync.Buffer) + w.i64(spec.Sync.WriteWorkers) + w.i64(int64(len(spec.Sync.Configs))) + for i := range spec.Sync.Configs { + c := &spec.Sync.Configs[i] + w.str(c.Name) + if c.Target == nil { + w.bool(false) + } else { + w.bool(true) + w.target(c.Target) + } + w.i64(int64(len(c.Paths))) + for _, p := range c.Paths { + w.str(p) + } + w.i64(int64(c.Mode)) + w.i64(int64(c.Interval)) + } + w.i64(int64(len(extra))) + for _, e := range extra { + w.str(e) + } + return hex.EncodeToString(w.h.Sum(nil)) +} + +// hashWriter writes length-prefixed fields so adjacent values cannot collide. +type hashWriter struct { + h hash.Hash +} + +func (w hashWriter) i64(v int64) { + var b [8]byte + binary.BigEndian.PutUint64(b[:], uint64(v)) + w.h.Write(b[:]) +} + +func (w hashWriter) str(s string) { + w.i64(int64(len(s))) + w.h.Write([]byte(s)) +} + +func (w hashWriter) bool(v bool) { + if v { + w.h.Write([]byte{1}) + return + } + w.h.Write([]byte{0}) +} + +func (w hashWriter) target(t *Target) { + w.str(string(t.Type)) + w.str(t.Address) + w.i64(int64(t.Port)) + if t.Credentials == nil { + w.bool(false) + } else { + w.bool(true) + w.str(t.Credentials.Username) + w.str(t.Credentials.Password) + w.str(t.Credentials.Token) + } + if t.TLS == nil { + w.bool(false) + } else { + w.bool(true) + w.str(t.TLS.CA) + w.str(t.TLS.Cert) + w.str(t.TLS.Key) + w.bool(t.TLS.SkipVerify) + } + if g := t.ProtocolOptions.GNMI; g == nil { + w.bool(false) + } else { + w.bool(true) + w.str(g.Encoding) + w.str(g.TargetName) + } + if n := t.ProtocolOptions.Netconf; n == nil { + w.bool(false) + } else { + w.bool(true) + w.bool(n.IncludeNs) + w.bool(n.OperationWithNs) + w.bool(n.UseOperationRemove) + w.i64(int64(n.CommitCandidate)) + } +} diff --git a/pkg/api/hash_test.go b/pkg/api/hash_test.go new file mode 100644 index 00000000..592f0d60 --- /dev/null +++ b/pkg/api/hash_test.go @@ -0,0 +1,115 @@ +package api + +import ( + "reflect" + "testing" +) + +func fullSpec() DatastoreSpec { + tgt := func() Target { + return Target{ + Type: TargetTypeNetconf, + Address: "10.0.0.1", + Port: 830, + Credentials: &Credentials{Username: "u", Password: "p", Token: "t"}, + TLS: &TLS{CA: "ca", Cert: "cert", Key: "key", SkipVerify: true}, + ProtocolOptions: ProtocolOptions{ + GNMI: &GNMIOptions{Encoding: "JSON_IETF", TargetName: "tn"}, + Netconf: &NetconfOptions{IncludeNs: true, OperationWithNs: true, UseOperationRemove: true, CommitCandidate: CommitCandidateRunning}, + }, + } + } + syncTgt := tgt() + return DatastoreSpec{ + Name: "ds1", + Target: tgt(), + Schema: SchemaRef{Vendor: "nokia", Version: "24.3"}, + Sync: Sync{ + Validate: true, Buffer: 5, WriteWorkers: 3, + Configs: []SyncConfig{{Name: "c", Target: &syncTgt, Paths: []string{"/a", "/b"}, Mode: SyncModeSample, Interval: 10}}, + }, + } +} + +func TestHashStable(t *testing.T) { + a, b := ComputeCreateDSHash(fullSpec()), ComputeCreateDSHash(fullSpec()) + if a != b { + t.Fatalf("hash differs for equal specs: %s vs %s", a, b) + } + if len(a) != 64 { + t.Fatalf("unexpected digest length %d", len(a)) + } +} + +// Golden value pins the canonical form: changing it forces every datastore to be recreated. +func TestHashGolden(t *testing.T) { + const want = "0532c42716855523d20c478edb017f08cf480320a7f7c3c060db8d2f8200f082" + if got := ComputeCreateDSHash(fullSpec()); got != want { + t.Fatalf("hash = %s, want %s", got, want) + } +} + +// Every leaf field of the spec must influence the hash, so a change recreates the datastore. +func TestHashChangesOnEveryField(t *testing.T) { + base := ComputeCreateDSHash(fullSpec()) + s := fullSpec() + var walk func(path string, v reflect.Value) + walk = func(path string, v reflect.Value) { + switch v.Kind() { + case reflect.Pointer: + walk(path, v.Elem()) + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + walk(path+"."+v.Type().Field(i).Name, v.Field(i)) + } + case reflect.Slice: + if v.Len() > 0 { + walk(path+"[0]", v.Index(0)) + } + default: + orig := reflect.New(v.Type()).Elem() + orig.Set(v) + switch v.Kind() { + case reflect.String: + v.SetString(v.String() + "x") + case reflect.Bool: + v.SetBool(!v.Bool()) + case reflect.Int, reflect.Int32, reflect.Int64: + v.SetInt(v.Int() + 1) + case reflect.Uint32, reflect.Uint64: + v.SetUint(v.Uint() + 1) + default: + t.Fatalf("%s: unhandled kind %s", path, v.Kind()) + } + if ComputeCreateDSHash(s) == base { + t.Errorf("hash ignores %s", path) + } + v.Set(orig) + } + } + walk("Spec", reflect.ValueOf(&s).Elem()) +} + +func TestHashNilVersusEmpty(t *testing.T) { + a := fullSpec() + b := fullSpec() + a.Target.Credentials = nil + b.Target.Credentials = &Credentials{} + if ComputeCreateDSHash(a) == ComputeCreateDSHash(b) { + t.Fatal("nil and empty credentials hash equal") + } +} + +func TestHashExtra(t *testing.T) { + s := fullSpec() + base := ComputeCreateDSHash(s) + if ComputeCreateDSHash(s, "1") == base { + t.Fatal("extra ignored") + } + if ComputeCreateDSHash(s, "1") != ComputeCreateDSHash(s, "1") { + t.Fatal("extra unstable") + } + if ComputeCreateDSHash(s, "a", "b") == ComputeCreateDSHash(s, "ab") { + t.Fatal("extra boundaries collide") + } +} diff --git a/pkg/api/schema.go b/pkg/api/schema.go new file mode 100644 index 00000000..a9621722 --- /dev/null +++ b/pkg/api/schema.go @@ -0,0 +1,28 @@ +package api + +// SchemaState is the lifecycle state of a schema. +type SchemaState int + +const ( + SchemaReady SchemaState = iota + SchemaReloading + SchemaInitializing + SchemaFailed +) + +// SchemaInfo is a schema with its state and source locations. +type SchemaInfo struct { + Schema SchemaRef + State SchemaState + Files []string + Directories []string + Excludes []string +} + +// CreateSchemaSpec is the input for creating a schema. +type CreateSchemaSpec struct { + Schema SchemaRef + Files []string + Directories []string + Excludes []string +} diff --git a/pkg/api/transaction.go b/pkg/api/transaction.go new file mode 100644 index 00000000..de350d2d --- /dev/null +++ b/pkg/api/transaction.go @@ -0,0 +1,73 @@ +package api + +import "time" + +// Origin identifies the requester of an operation for logging and audit. It is +// optional; the gRPC adapter fills it from the peer and in-process callers set +// their own name. +type Origin string + +// TxRef addresses a transaction within a datastore. +type TxRef struct { + Datastore string + ID string +} + +// SetOptions tune a transaction set. +type SetOptions struct { + // DryRun validates without applying. + DryRun bool + // Timeout auto-cancels an unconfirmed transaction; zero means the default. + Timeout time.Duration + Origin Origin +} + +// IntentSpec is the body of an Intent: exactly one of Update and Delete is set. +type IntentSpec struct { + Update *Update + Delete *Delete +} + +// Intent is a named unit of desired configuration within a transaction. +type Intent struct { + Name string + Spec IntentSpec +} + +// Update creates or replaces an intent. +type Update struct { + Priority int32 + // Content is the intent body, JSON encoded. + Content []byte + // NonRevertive leaves device-side changes in place instead of reverting them. + NonRevertive bool + // PreviouslyApplied marks a non-revertive intent that was applied before. + PreviouslyApplied bool +} + +// Delete removes an intent. +type Delete struct { + // IgnoreNoExist suppresses the error for an intent that does not exist. + IgnoreNoExist bool + // Orphan removes the intent from the intended store but keeps device config. + Orphan bool +} + +// Format selects the encoding of intent and blame output. +type Format int + +const ( + FormatJSON Format = iota + FormatJSONIETF + FormatXML + FormatProto + FormatXPath +) + +// IntentRequest identifies an intent to read back. +type IntentRequest struct { + Datastore string + Intent string + Format Format + Origin Origin +} From 9c9bb495692eb914fbfcf0d61c4a3043536e13d1 Mon Sep 17 00:00:00 2001 From: Markus Vahlenkamp Date: Fri, 6 Nov 2026 14:54:39 +0100 Subject: [PATCH 2/8] refactor(api): add DeleteSchemaSpec, document DeviationKind, dedupe hash presence checks Signed-off-by: Markus Vahlenkamp --- pkg/api/deviation.go | 12 ++++++++---- pkg/api/hash.go | 31 +++++++++++-------------------- pkg/api/schema.go | 5 +++++ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/api/deviation.go b/pkg/api/deviation.go index 478aa211..6579d9d9 100644 --- a/pkg/api/deviation.go +++ b/pkg/api/deviation.go @@ -1,6 +1,10 @@ package api -// DeviationKind classifies a DeviationEvent. +import sdcpb "github.com/sdcio/sdc-protos/sdcpb" + +// DeviationKind classifies a DeviationEvent: Start opens a deviation stream +// for a datastore, Update reports a changed deviation, End closes the stream +// normally and Clear drops all deviations previously reported for the intent. type DeviationKind int const ( @@ -23,9 +27,9 @@ const ( // DeviationEntry is one deviating path with its expected and current values. type DeviationEntry struct { - Path string - Expected string - Current string + Path *sdcpb.Path + Expected *sdcpb.TypedValue + Current *sdcpb.TypedValue } // DeviationEvent is emitted by a deviation watch. Intent, Reason and Entries diff --git a/pkg/api/hash.go b/pkg/api/hash.go index 322d799d..70564423 100644 --- a/pkg/api/hash.go +++ b/pkg/api/hash.go @@ -24,10 +24,7 @@ func ComputeCreateDSHash(spec DatastoreSpec, extra ...string) string { for i := range spec.Sync.Configs { c := &spec.Sync.Configs[i] w.str(c.Name) - if c.Target == nil { - w.bool(false) - } else { - w.bool(true) + if w.present(c.Target != nil) { w.target(c.Target) } w.i64(int64(len(c.Paths))) @@ -68,38 +65,32 @@ func (w hashWriter) bool(v bool) { w.h.Write([]byte{0}) } +// present writes a presence marker and returns ok, so nil and zero values hash apart. +func (w hashWriter) present(ok bool) bool { + w.bool(ok) + return ok +} + func (w hashWriter) target(t *Target) { w.str(string(t.Type)) w.str(t.Address) w.i64(int64(t.Port)) - if t.Credentials == nil { - w.bool(false) - } else { - w.bool(true) + if w.present(t.Credentials != nil) { w.str(t.Credentials.Username) w.str(t.Credentials.Password) w.str(t.Credentials.Token) } - if t.TLS == nil { - w.bool(false) - } else { - w.bool(true) + if w.present(t.TLS != nil) { w.str(t.TLS.CA) w.str(t.TLS.Cert) w.str(t.TLS.Key) w.bool(t.TLS.SkipVerify) } - if g := t.ProtocolOptions.GNMI; g == nil { - w.bool(false) - } else { - w.bool(true) + if g := t.ProtocolOptions.GNMI; w.present(g != nil) { w.str(g.Encoding) w.str(g.TargetName) } - if n := t.ProtocolOptions.Netconf; n == nil { - w.bool(false) - } else { - w.bool(true) + if n := t.ProtocolOptions.Netconf; w.present(n != nil) { w.bool(n.IncludeNs) w.bool(n.OperationWithNs) w.bool(n.UseOperationRemove) diff --git a/pkg/api/schema.go b/pkg/api/schema.go index a9621722..75f5dc71 100644 --- a/pkg/api/schema.go +++ b/pkg/api/schema.go @@ -26,3 +26,8 @@ type CreateSchemaSpec struct { Directories []string Excludes []string } + +// DeleteSchemaSpec identifies the schema to delete. +type DeleteSchemaSpec struct { + Schema SchemaRef +} From 5333e81b5e66d1762b6d1c2110b84684e07bc252 Mon Sep 17 00:00:00 2001 From: Markus Vahlenkamp Date: Mon, 9 Nov 2026 10:54:33 +0100 Subject: [PATCH 3/8] test(api): compare extra-hash stability against a stored value Signed-off-by: Markus Vahlenkamp --- pkg/api/hash_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/api/hash_test.go b/pkg/api/hash_test.go index 592f0d60..464fd3a6 100644 --- a/pkg/api/hash_test.go +++ b/pkg/api/hash_test.go @@ -103,10 +103,11 @@ func TestHashNilVersusEmpty(t *testing.T) { func TestHashExtra(t *testing.T) { s := fullSpec() base := ComputeCreateDSHash(s) - if ComputeCreateDSHash(s, "1") == base { + withExtra := ComputeCreateDSHash(s, "1") + if withExtra == base { t.Fatal("extra ignored") } - if ComputeCreateDSHash(s, "1") != ComputeCreateDSHash(s, "1") { + if withExtra != ComputeCreateDSHash(s, "1") { t.Fatal("extra unstable") } if ComputeCreateDSHash(s, "a", "b") == ComputeCreateDSHash(s, "ab") { From 3c262168d8b1d15d3446c19ae0c06eb2d202a6f9 Mon Sep 17 00:00:00 2001 From: Markus Vahlenkamp Date: Mon, 9 Nov 2026 16:39:17 +0100 Subject: [PATCH 4/8] refactor(api): drop unused sync fields and hash, type gNMI encoding Remove Sync.Validate, Buffer and WriteWorkers: nothing read them, and validation runs per transaction on the tree. Drop the unused ComputeCreateDSHash. Add api.GNMIEncoding; unknown or numeric encodings are rejected and a gNMI sync without an encoding inherits the target's. go.mod temporarily replaces sdc-protos with the commit that reserves the removed Sync fields. Signed-off-by: Markus Vahlenkamp --- collocated.yaml | 2 - data-server.yaml | 2 - go.mod | 2 + go.sum | 4 +- lab/distributed/data-server.yaml | 5 -- pkg/api/datastore.go | 33 +++++++-- pkg/api/datastore_test.go | 16 +++++ pkg/api/hash.go | 99 ------------------------- pkg/api/hash_test.go | 116 ------------------------------ pkg/config/datastore.go | 48 ++++++++++--- pkg/config/datastore_sync_test.go | 38 ++++++++++ pkg/config/defaults.go | 2 - pkg/server/datastore.go | 5 +- remote_cache.yaml | 2 - 14 files changed, 124 insertions(+), 250 deletions(-) create mode 100644 pkg/api/datastore_test.go delete mode 100644 pkg/api/hash.go delete mode 100644 pkg/api/hash_test.go create mode 100644 pkg/config/datastore_sync_test.go diff --git a/collocated.yaml b/collocated.yaml index cee26488..e6352915 100644 --- a/collocated.yaml +++ b/collocated.yaml @@ -55,7 +55,6 @@ grpc-server: # username: admin # password: admin # sync: - # validate: true # config: # - name: config # protocol: netconf @@ -81,7 +80,6 @@ grpc-server: # tls: # skip-verify: true # sync: -# validate: true # gnmi: # - path: # - / diff --git a/data-server.yaml b/data-server.yaml index 7658482f..41985371 100644 --- a/data-server.yaml +++ b/data-server.yaml @@ -55,7 +55,6 @@ datastores: # this specifies MAIN datastores # tls: # skip-verify: true # sync: -# validate: true # gnmi: # - path: # - / @@ -81,7 +80,6 @@ datastores: # this specifies MAIN datastores # tls: # skip-verify: true # sync: -# validate: true # gnmi: # - path: # - / diff --git a/go.mod b/go.mod index 34bea1db..94500868 100644 --- a/go.mod +++ b/go.mod @@ -115,3 +115,5 @@ require ( sigs.k8s.io/structured-merge-diff/v4 v4.4.3 // indirect sigs.k8s.io/yaml v1.4.0 // indirect ) + +replace github.com/sdcio/sdc-protos => github.com/sdcio/sdc-protos v0.0.57-0.20261109153201-334b5937c4ae diff --git a/go.sum b/go.sum index fe7afb4f..e3bcce4c 100644 --- a/go.sum +++ b/go.sum @@ -191,8 +191,8 @@ github.com/sdcio/logger v0.0.3 h1:IFUbObObGry+S8lHGwOQKKRxJSuOphgRU/hxVhOdMOM= github.com/sdcio/logger v0.0.3/go.mod h1:yWaOxK/G6vszjg8tKZiMqiEjlZouHsjFME4zSk+SAEA= github.com/sdcio/schema-server v0.0.34 h1:NNDOkvtUMONtBA7cVvN96F+FWGD/Do6HNqfchy9B8eI= github.com/sdcio/schema-server v0.0.34/go.mod h1:6t8HLXpqUqEJmE5yNZh29u/KZw0jlOICdNWns7zE4GE= -github.com/sdcio/sdc-protos v0.0.56 h1:8tEbo53/2KAd/5mruqiKhNsDljKcWI6nhCfriYE2SvE= -github.com/sdcio/sdc-protos v0.0.56/go.mod h1:hER0riYU3G55NnMQ0p0R9kIL/fzqq1zab4pOgUT3Yfg= +github.com/sdcio/sdc-protos v0.0.57-0.20261109153201-334b5937c4ae h1:6vIatz59rXaWl5HVrJWrvOxfBCLjhJBnKeOEc5ischE= +github.com/sdcio/sdc-protos v0.0.57-0.20261109153201-334b5937c4ae/go.mod h1:hER0riYU3G55NnMQ0p0R9kIL/fzqq1zab4pOgUT3Yfg= github.com/sdcio/yang-parser v0.0.12 h1:RSSeqfAOIsJx5Lno5u4/ezyOmQYUduQ22rBfU/mtpJ4= github.com/sdcio/yang-parser v0.0.12/go.mod h1:CBqn3Miq85qmFVGHxHXHLluXkaIOsTzV06IM4DW6+D4= github.com/sirikothe/gotextfsm v1.0.1-0.20200816110946-6aa2cfd355e4 h1:FHUL2HofYJuslFOQdy/JjjP36zxqIpd/dcoiwLMIs7k= diff --git a/lab/distributed/data-server.yaml b/lab/distributed/data-server.yaml index e0ae20c4..3f748232 100644 --- a/lab/distributed/data-server.yaml +++ b/lab/distributed/data-server.yaml @@ -39,7 +39,6 @@ datastores: # this creates MAIN datastores tls: skip-verify: true sync: - validate: false gnmi: - name: config paths: @@ -67,7 +66,6 @@ datastores: # this creates MAIN datastores tls: skip-verify: true sync: - validate: false gnmi: - name: config paths: @@ -94,7 +92,6 @@ datastores: # this creates MAIN datastores tls: skip-verify: true sync: - validate: false gnmi: - name: config paths: @@ -121,7 +118,6 @@ datastores: # this creates MAIN datastores password: admin # include-ns: true sync: - validate: false gnmi: - name: config paths: @@ -149,7 +145,6 @@ datastores: # this creates MAIN datastores password: admin # include-ns: true sync: - validate: false gnmi: - name: config paths: diff --git a/pkg/api/datastore.go b/pkg/api/datastore.go index 46c6bd0d..a0599b06 100644 --- a/pkg/api/datastore.go +++ b/pkg/api/datastore.go @@ -1,5 +1,10 @@ package api +import ( + "fmt" + "strings" +) + // TargetType names the southbound protocol of a datastore target. type TargetType string @@ -40,9 +45,30 @@ const ( CommitCandidateRunning ) +// GNMIEncoding names a gNMI value encoding, spelled as in the gNMI spec. +type GNMIEncoding string + +const ( + GNMIEncodingJSON GNMIEncoding = "JSON" + GNMIEncodingBytes GNMIEncoding = "BYTES" + GNMIEncodingProto GNMIEncoding = "PROTO" + GNMIEncodingASCII GNMIEncoding = "ASCII" + GNMIEncodingJSONIETF GNMIEncoding = "JSON_IETF" +) + +// ParseGNMIEncoding resolves a case-insensitive encoding name. Unknown names, +// including numeric values, are rejected. +func ParseGNMIEncoding(s string) (GNMIEncoding, error) { + switch e := GNMIEncoding(strings.ToUpper(s)); e { + case GNMIEncodingJSON, GNMIEncodingBytes, GNMIEncodingProto, GNMIEncodingASCII, GNMIEncodingJSONIETF: + return e, nil + } + return "", fmt.Errorf("unknown gnmi encoding %q", s) +} + // GNMIOptions are the gNMI-specific protocol options. type GNMIOptions struct { - Encoding string + Encoding GNMIEncoding TargetName string } @@ -98,10 +124,7 @@ type SyncConfig struct { // Sync is the datastore sync strategy. type Sync struct { - Validate bool - Buffer int64 - WriteWorkers int64 - Configs []SyncConfig + Configs []SyncConfig } // DatastoreSpec is the desired state used to create a datastore. diff --git a/pkg/api/datastore_test.go b/pkg/api/datastore_test.go new file mode 100644 index 00000000..59173746 --- /dev/null +++ b/pkg/api/datastore_test.go @@ -0,0 +1,16 @@ +package api + +import "testing" + +func TestParseGNMIEncoding(t *testing.T) { + for in, want := range map[string]GNMIEncoding{"json_ietf": GNMIEncodingJSONIETF, "JSON": GNMIEncodingJSON, "Proto": GNMIEncodingProto} { + if got, err := ParseGNMIEncoding(in); err != nil || got != want { + t.Errorf("%q: got %q, %v", in, got, err) + } + } + for _, in := range []string{"", "jsno", "4"} { + if _, err := ParseGNMIEncoding(in); err == nil { + t.Errorf("%q accepted", in) + } + } +} diff --git a/pkg/api/hash.go b/pkg/api/hash.go deleted file mode 100644 index 70564423..00000000 --- a/pkg/api/hash.go +++ /dev/null @@ -1,99 +0,0 @@ -package api - -import ( - "crypto/sha256" - "encoding/binary" - "encoding/hex" - "hash" -) - -// ComputeCreateDSHash returns a stable hex digest of the spec. Equal specs hash -// equal and any field change alters the digest, so callers recreate a datastore -// when it differs. extra values (for example resource versions of referenced -// objects) are mixed in, in order. -func ComputeCreateDSHash(spec DatastoreSpec, extra ...string) string { - w := hashWriter{h: sha256.New()} - w.str(spec.Name) - w.target(&spec.Target) - w.str(spec.Schema.Vendor) - w.str(spec.Schema.Version) - w.bool(spec.Sync.Validate) - w.i64(spec.Sync.Buffer) - w.i64(spec.Sync.WriteWorkers) - w.i64(int64(len(spec.Sync.Configs))) - for i := range spec.Sync.Configs { - c := &spec.Sync.Configs[i] - w.str(c.Name) - if w.present(c.Target != nil) { - w.target(c.Target) - } - w.i64(int64(len(c.Paths))) - for _, p := range c.Paths { - w.str(p) - } - w.i64(int64(c.Mode)) - w.i64(int64(c.Interval)) - } - w.i64(int64(len(extra))) - for _, e := range extra { - w.str(e) - } - return hex.EncodeToString(w.h.Sum(nil)) -} - -// hashWriter writes length-prefixed fields so adjacent values cannot collide. -type hashWriter struct { - h hash.Hash -} - -func (w hashWriter) i64(v int64) { - var b [8]byte - binary.BigEndian.PutUint64(b[:], uint64(v)) - w.h.Write(b[:]) -} - -func (w hashWriter) str(s string) { - w.i64(int64(len(s))) - w.h.Write([]byte(s)) -} - -func (w hashWriter) bool(v bool) { - if v { - w.h.Write([]byte{1}) - return - } - w.h.Write([]byte{0}) -} - -// present writes a presence marker and returns ok, so nil and zero values hash apart. -func (w hashWriter) present(ok bool) bool { - w.bool(ok) - return ok -} - -func (w hashWriter) target(t *Target) { - w.str(string(t.Type)) - w.str(t.Address) - w.i64(int64(t.Port)) - if w.present(t.Credentials != nil) { - w.str(t.Credentials.Username) - w.str(t.Credentials.Password) - w.str(t.Credentials.Token) - } - if w.present(t.TLS != nil) { - w.str(t.TLS.CA) - w.str(t.TLS.Cert) - w.str(t.TLS.Key) - w.bool(t.TLS.SkipVerify) - } - if g := t.ProtocolOptions.GNMI; w.present(g != nil) { - w.str(g.Encoding) - w.str(g.TargetName) - } - if n := t.ProtocolOptions.Netconf; w.present(n != nil) { - w.bool(n.IncludeNs) - w.bool(n.OperationWithNs) - w.bool(n.UseOperationRemove) - w.i64(int64(n.CommitCandidate)) - } -} diff --git a/pkg/api/hash_test.go b/pkg/api/hash_test.go deleted file mode 100644 index 464fd3a6..00000000 --- a/pkg/api/hash_test.go +++ /dev/null @@ -1,116 +0,0 @@ -package api - -import ( - "reflect" - "testing" -) - -func fullSpec() DatastoreSpec { - tgt := func() Target { - return Target{ - Type: TargetTypeNetconf, - Address: "10.0.0.1", - Port: 830, - Credentials: &Credentials{Username: "u", Password: "p", Token: "t"}, - TLS: &TLS{CA: "ca", Cert: "cert", Key: "key", SkipVerify: true}, - ProtocolOptions: ProtocolOptions{ - GNMI: &GNMIOptions{Encoding: "JSON_IETF", TargetName: "tn"}, - Netconf: &NetconfOptions{IncludeNs: true, OperationWithNs: true, UseOperationRemove: true, CommitCandidate: CommitCandidateRunning}, - }, - } - } - syncTgt := tgt() - return DatastoreSpec{ - Name: "ds1", - Target: tgt(), - Schema: SchemaRef{Vendor: "nokia", Version: "24.3"}, - Sync: Sync{ - Validate: true, Buffer: 5, WriteWorkers: 3, - Configs: []SyncConfig{{Name: "c", Target: &syncTgt, Paths: []string{"/a", "/b"}, Mode: SyncModeSample, Interval: 10}}, - }, - } -} - -func TestHashStable(t *testing.T) { - a, b := ComputeCreateDSHash(fullSpec()), ComputeCreateDSHash(fullSpec()) - if a != b { - t.Fatalf("hash differs for equal specs: %s vs %s", a, b) - } - if len(a) != 64 { - t.Fatalf("unexpected digest length %d", len(a)) - } -} - -// Golden value pins the canonical form: changing it forces every datastore to be recreated. -func TestHashGolden(t *testing.T) { - const want = "0532c42716855523d20c478edb017f08cf480320a7f7c3c060db8d2f8200f082" - if got := ComputeCreateDSHash(fullSpec()); got != want { - t.Fatalf("hash = %s, want %s", got, want) - } -} - -// Every leaf field of the spec must influence the hash, so a change recreates the datastore. -func TestHashChangesOnEveryField(t *testing.T) { - base := ComputeCreateDSHash(fullSpec()) - s := fullSpec() - var walk func(path string, v reflect.Value) - walk = func(path string, v reflect.Value) { - switch v.Kind() { - case reflect.Pointer: - walk(path, v.Elem()) - case reflect.Struct: - for i := 0; i < v.NumField(); i++ { - walk(path+"."+v.Type().Field(i).Name, v.Field(i)) - } - case reflect.Slice: - if v.Len() > 0 { - walk(path+"[0]", v.Index(0)) - } - default: - orig := reflect.New(v.Type()).Elem() - orig.Set(v) - switch v.Kind() { - case reflect.String: - v.SetString(v.String() + "x") - case reflect.Bool: - v.SetBool(!v.Bool()) - case reflect.Int, reflect.Int32, reflect.Int64: - v.SetInt(v.Int() + 1) - case reflect.Uint32, reflect.Uint64: - v.SetUint(v.Uint() + 1) - default: - t.Fatalf("%s: unhandled kind %s", path, v.Kind()) - } - if ComputeCreateDSHash(s) == base { - t.Errorf("hash ignores %s", path) - } - v.Set(orig) - } - } - walk("Spec", reflect.ValueOf(&s).Elem()) -} - -func TestHashNilVersusEmpty(t *testing.T) { - a := fullSpec() - b := fullSpec() - a.Target.Credentials = nil - b.Target.Credentials = &Credentials{} - if ComputeCreateDSHash(a) == ComputeCreateDSHash(b) { - t.Fatal("nil and empty credentials hash equal") - } -} - -func TestHashExtra(t *testing.T) { - s := fullSpec() - base := ComputeCreateDSHash(s) - withExtra := ComputeCreateDSHash(s, "1") - if withExtra == base { - t.Fatal("extra ignored") - } - if withExtra != ComputeCreateDSHash(s, "1") { - t.Fatal("extra unstable") - } - if ComputeCreateDSHash(s, "a", "b") == ComputeCreateDSHash(s, "ab") { - t.Fatal("extra boundaries collide") - } -} diff --git a/pkg/config/datastore.go b/pkg/config/datastore.go index 90ab054b..5ec1c9b7 100644 --- a/pkg/config/datastore.go +++ b/pkg/config/datastore.go @@ -20,8 +20,10 @@ import ( "fmt" "net" "os" + "strings" "time" + "github.com/sdcio/data-server/pkg/api" "github.com/sdcio/data-server/pkg/utils" ) @@ -129,10 +131,7 @@ func (c Creds) MarshalJSON() ([]byte, error) { } type Sync struct { - Validate bool `yaml:"validate,omitempty" json:"validate,omitempty"` - Buffer int64 `yaml:"buffer,omitempty" json:"buffer,omitempty"` - WriteWorkers int64 `yaml:"write-workers,omitempty" json:"write-workers,omitempty"` - Config []*SyncProtocol `yaml:"config,omitempty" json:"config,omitempty"` + Config []*SyncProtocol `yaml:"config,omitempty" json:"config,omitempty"` } type SyncProtocol struct { @@ -163,6 +162,9 @@ func (ds *DatastoreConfig) ValidateSetDefaults() error { if err = ds.Sync.validateSetDefaults(); err != nil { return err } + if err = ds.Sync.resolveGnmiEncodings(ds.SBI); err != nil { + return err + } } if ds.Validation == nil { @@ -198,6 +200,11 @@ func (s *SBI) validateSetDefaults() error { if s.GnmiOptions.Encoding == "" { return errors.New("no encoding defined") } + enc, err := api.ParseGNMIEncoding(s.GnmiOptions.Encoding) + if err != nil { + return err + } + s.GnmiOptions.Encoding = string(enc) default: return fmt.Errorf("unknown sbi type: %q", s.Type) } @@ -225,13 +232,6 @@ func (s *Sync) validateSetDefaults() error { if s == nil || len(s.Config) == 0 { return nil } - if s.Buffer <= 0 { - s.Buffer = defaultBufferSize - } - if s.WriteWorkers <= 0 { - s.WriteWorkers = defaultWriteWorkers - } - var errs []error for _, sp := range s.Config { err := sp.validateSetDefaults() @@ -245,6 +245,32 @@ func (s *Sync) validateSetDefaults() error { return nil } +// resolveGnmiEncodings validates the encoding of every gNMI sync. A sync +// without its own encoding inherits the datastore's gNMI target encoding. +// Noop targets ignore syncs, so their encodings are left as given. +func (s *Sync) resolveGnmiEncodings(sbi *SBI) error { + if sbi != nil && sbi.Type == sbiNOOP { + return nil + } + for _, sp := range s.Config { + if !strings.EqualFold(sp.Protocol, sbiGNMI) { + continue + } + if sp.Encoding == "" && sbi != nil && sbi.GnmiOptions != nil { + sp.Encoding = sbi.GnmiOptions.Encoding + } + if sp.Encoding == "" { + return fmt.Errorf("sync %q: no encoding defined", sp.Name) + } + enc, err := api.ParseGNMIEncoding(sp.Encoding) + if err != nil { + return fmt.Errorf("sync %q: %w", sp.Name, err) + } + sp.Encoding = string(enc) + } + return nil +} + func (s *SyncProtocol) validateSetDefaults() error { if s.Interval <= 0 { s.Interval = defaultSyncInterval diff --git a/pkg/config/datastore_sync_test.go b/pkg/config/datastore_sync_test.go new file mode 100644 index 00000000..c723f405 --- /dev/null +++ b/pkg/config/datastore_sync_test.go @@ -0,0 +1,38 @@ +package config + +import "testing" + +func gnmiSBI(enc string) *SBI { + return &SBI{Type: sbiGNMI, Address: "a", Port: 1, GnmiOptions: &SBIGnmiOptions{Encoding: enc}} +} + +func TestSyncEncodingInheritsFromTarget(t *testing.T) { + ds := &DatastoreConfig{ + SBI: gnmiSBI("json_ietf"), + Sync: &Sync{Config: []*SyncProtocol{{Name: "a", Protocol: "gnmi"}, {Name: "b", Protocol: "gnmi", Encoding: "proto"}}}, + } + if err := ds.ValidateSetDefaults(); err != nil { + t.Fatal(err) + } + if got := ds.Sync.Config[0].Encoding; got != "JSON_IETF" { + t.Errorf("inherited encoding = %q", got) + } + if got := ds.Sync.Config[1].Encoding; got != "PROTO" { + t.Errorf("explicit encoding = %q", got) + } +} + +func TestSyncEncodingErrors(t *testing.T) { + tests := map[string]*DatastoreConfig{ + "unknown target encoding": {SBI: gnmiSBI("jsno")}, + "numeric sync encoding": {SBI: gnmiSBI("JSON"), Sync: &Sync{Config: []*SyncProtocol{{Name: "a", Protocol: "gnmi", Encoding: "4"}}}}, + "nothing to inherit": {SBI: &SBI{Type: sbiNETCONF, Address: "a", Port: 1, NetconfOptions: &SBINetconfOptions{}}, Sync: &Sync{Config: []*SyncProtocol{{Name: "a", Protocol: "gnmi"}}}}, + } + for name, ds := range tests { + t.Run(name, func(t *testing.T) { + if err := ds.ValidateSetDefaults(); err == nil { + t.Fatal("expected error") + } + }) + } +} diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 83605c02..20f2c21e 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -27,10 +27,8 @@ const ( defaultNCPort = 830 defaultCacheType = "local" defaultRemoteCacheAddress = "localhost:50100" - defaultBufferSize = 1000 defaultStoreType = "badgerdb" defaultCacheDir = "./cached/caches" - defaultWriteWorkers = 16 defaultTimeout = 30 * time.Second defaultSyncInterval = 30 * time.Second diff --git a/pkg/server/datastore.go b/pkg/server/datastore.go index 9d39628c..113b0590 100644 --- a/pkg/server/datastore.go +++ b/pkg/server/datastore.go @@ -166,10 +166,7 @@ func (s *Server) CreateDataStore(ctx context.Context, req *sdcpb.CreateDataStore } if req.GetSync() != nil { dsConfig.Sync = &config.Sync{ - Validate: req.GetSync().GetValidate(), - Buffer: req.GetSync().GetBuffer(), - WriteWorkers: req.GetSync().GetWriteWorkers(), - Config: make([]*config.SyncProtocol, 0, len(req.GetSync().GetConfig())), + Config: make([]*config.SyncProtocol, 0, len(req.GetSync().GetConfig())), } for _, pSync := range req.GetSync().GetConfig() { gnSyncConfig := &config.SyncProtocol{ diff --git a/remote_cache.yaml b/remote_cache.yaml index c7a2dc7c..3bc42d60 100644 --- a/remote_cache.yaml +++ b/remote_cache.yaml @@ -55,7 +55,6 @@ datastores: # this specifies MAIN datastores # tls: # skip-verify: true # sync: -# validate: true # gnmi: # - path: # - / @@ -81,7 +80,6 @@ datastores: # this specifies MAIN datastores # tls: # skip-verify: true # sync: -# validate: true # gnmi: # - path: # - / From 2d2161f40a3689650b50aa2cd13cb9f4d2c608db Mon Sep 17 00:00:00 2001 From: Markus Vahlenkamp Date: Mon, 9 Nov 2026 16:54:04 +0100 Subject: [PATCH 5/8] feat(api): restore ComputeCreateDSHash for recreate-on-change config-server hashes the resolved DatastoreSpec plus referenced object versions to decide when to recreate a datastore. Restore it without the removed Sync fields and with the typed gNMI encoding, and document why the hash is defined over DatastoreSpec. Signed-off-by: Markus Vahlenkamp --- pkg/api/hash.go | 103 ++++++++++++++++++++++++++++++++++++++ pkg/api/hash_test.go | 115 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 218 insertions(+) create mode 100644 pkg/api/hash.go create mode 100644 pkg/api/hash_test.go diff --git a/pkg/api/hash.go b/pkg/api/hash.go new file mode 100644 index 00000000..24025cfc --- /dev/null +++ b/pkg/api/hash.go @@ -0,0 +1,103 @@ +package api + +import ( + "crypto/sha256" + "encoding/binary" + "encoding/hex" + "hash" +) + +// ComputeCreateDSHash returns a stable hex digest of the spec. Equal specs hash +// equal and any field change alters the digest, so callers recreate a datastore +// when it differs. extra values (for example resource versions of referenced +// objects) are mixed in, in order. +// +// The hash is over the resolved DatastoreSpec, not a caller's own API type, so +// it changes exactly when the datastore the server would build changes. Callers +// that resolve references (secrets, TLS material) pass their versions as extra. +// It lives next to DatastoreSpec so a new field cannot be added without +// hashing it; TestHashChangesOnEveryField enforces this. Changing the encoding +// changes every digest and recreates all datastores. +func ComputeCreateDSHash(spec DatastoreSpec, extra ...string) string { + w := hashWriter{h: sha256.New()} + w.str(spec.Name) + w.target(&spec.Target) + w.str(spec.Schema.Vendor) + w.str(spec.Schema.Version) + w.i64(int64(len(spec.Sync.Configs))) + for i := range spec.Sync.Configs { + c := &spec.Sync.Configs[i] + w.str(c.Name) + if w.present(c.Target != nil) { + w.target(c.Target) + } + w.i64(int64(len(c.Paths))) + for _, p := range c.Paths { + w.str(p) + } + w.i64(int64(c.Mode)) + w.i64(int64(c.Interval)) + } + w.i64(int64(len(extra))) + for _, e := range extra { + w.str(e) + } + return hex.EncodeToString(w.h.Sum(nil)) +} + +// hashWriter writes length-prefixed fields so adjacent values cannot collide. +type hashWriter struct { + h hash.Hash +} + +func (w hashWriter) i64(v int64) { + var b [8]byte + binary.BigEndian.PutUint64(b[:], uint64(v)) + w.h.Write(b[:]) +} + +func (w hashWriter) str(s string) { + w.i64(int64(len(s))) + w.h.Write([]byte(s)) +} + +func (w hashWriter) bool(v bool) { + if v { + w.h.Write([]byte{1}) + return + } + w.h.Write([]byte{0}) +} + +// present writes a presence marker and returns ok, so nil and zero values hash apart. +func (w hashWriter) present(ok bool) bool { + w.bool(ok) + return ok +} + +func (w hashWriter) target(t *Target) { + w.str(string(t.Type)) + w.str(t.Address) + w.i64(int64(t.Port)) + if w.present(t.Credentials != nil) { + w.str(t.Credentials.Username) + w.str(t.Credentials.Password) + w.str(t.Credentials.Token) + } + if w.present(t.TLS != nil) { + w.str(t.TLS.CA) + w.str(t.TLS.Cert) + w.str(t.TLS.Key) + w.bool(t.TLS.SkipVerify) + } + if g := t.ProtocolOptions.GNMI; w.present(g != nil) { + w.str(string(g.Encoding)) + w.str(g.TargetName) + } + if n := t.ProtocolOptions.Netconf; w.present(n != nil) { + w.bool(n.IncludeNs) + w.bool(n.OperationWithNs) + w.bool(n.UseOperationRemove) + w.i64(int64(n.CommitCandidate)) + } +} diff --git a/pkg/api/hash_test.go b/pkg/api/hash_test.go new file mode 100644 index 00000000..84b5f0c7 --- /dev/null +++ b/pkg/api/hash_test.go @@ -0,0 +1,115 @@ +package api + +import ( + "reflect" + "testing" +) + +func fullSpec() DatastoreSpec { + tgt := func() Target { + return Target{ + Type: TargetTypeNetconf, + Address: "10.0.0.1", + Port: 830, + Credentials: &Credentials{Username: "u", Password: "p", Token: "t"}, + TLS: &TLS{CA: "ca", Cert: "cert", Key: "key", SkipVerify: true}, + ProtocolOptions: ProtocolOptions{ + GNMI: &GNMIOptions{Encoding: GNMIEncodingJSONIETF, TargetName: "tn"}, + Netconf: &NetconfOptions{IncludeNs: true, OperationWithNs: true, UseOperationRemove: true, CommitCandidate: CommitCandidateRunning}, + }, + } + } + syncTgt := tgt() + return DatastoreSpec{ + Name: "ds1", + Target: tgt(), + Schema: SchemaRef{Vendor: "nokia", Version: "24.3"}, + Sync: Sync{ + Configs: []SyncConfig{{Name: "c", Target: &syncTgt, Paths: []string{"/a", "/b"}, Mode: SyncModeSample, Interval: 10}}, + }, + } +} + +func TestHashStable(t *testing.T) { + a, b := ComputeCreateDSHash(fullSpec()), ComputeCreateDSHash(fullSpec()) + if a != b { + t.Fatalf("hash differs for equal specs: %s vs %s", a, b) + } + if len(a) != 64 { + t.Fatalf("unexpected digest length %d", len(a)) + } +} + +// Golden value pins the canonical form: changing it forces every datastore to be recreated. +func TestHashGolden(t *testing.T) { + const want = "1c70111729aba574666d8c49f61cc5a7708f6e13d7005e9064b235847194abb7" + if got := ComputeCreateDSHash(fullSpec()); got != want { + t.Fatalf("hash = %s, want %s", got, want) + } +} + +// Every leaf field of the spec must influence the hash, so a change recreates the datastore. +func TestHashChangesOnEveryField(t *testing.T) { + base := ComputeCreateDSHash(fullSpec()) + s := fullSpec() + var walk func(path string, v reflect.Value) + walk = func(path string, v reflect.Value) { + switch v.Kind() { + case reflect.Pointer: + walk(path, v.Elem()) + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + walk(path+"."+v.Type().Field(i).Name, v.Field(i)) + } + case reflect.Slice: + if v.Len() > 0 { + walk(path+"[0]", v.Index(0)) + } + default: + orig := reflect.New(v.Type()).Elem() + orig.Set(v) + switch v.Kind() { + case reflect.String: + v.SetString(v.String() + "x") + case reflect.Bool: + v.SetBool(!v.Bool()) + case reflect.Int, reflect.Int32, reflect.Int64: + v.SetInt(v.Int() + 1) + case reflect.Uint32, reflect.Uint64: + v.SetUint(v.Uint() + 1) + default: + t.Fatalf("%s: unhandled kind %s", path, v.Kind()) + } + if ComputeCreateDSHash(s) == base { + t.Errorf("hash ignores %s", path) + } + v.Set(orig) + } + } + walk("Spec", reflect.ValueOf(&s).Elem()) +} + +func TestHashNilVersusEmpty(t *testing.T) { + a := fullSpec() + b := fullSpec() + a.Target.Credentials = nil + b.Target.Credentials = &Credentials{} + if ComputeCreateDSHash(a) == ComputeCreateDSHash(b) { + t.Fatal("nil and empty credentials hash equal") + } +} + +func TestHashExtra(t *testing.T) { + s := fullSpec() + base := ComputeCreateDSHash(s) + withExtra := ComputeCreateDSHash(s, "1") + if withExtra == base { + t.Fatal("extra ignored") + } + if withExtra != ComputeCreateDSHash(s, "1") { + t.Fatal("extra unstable") + } + if ComputeCreateDSHash(s, "a", "b") == ComputeCreateDSHash(s, "ab") { + t.Fatal("extra boundaries collide") + } +} From 3157f29d52bcbb48871eb8343c1dcee9cc690b07 Mon Sep 17 00:00:00 2001 From: Markus Vahlenkamp Date: Wed, 11 Nov 2026 12:16:27 +0100 Subject: [PATCH 6/8] refactor(api): flatten DeviationEvent to one path per event Replace the Entries slice with Path, Expected and Current to match the per-deviation WatchDeviationResponse, drop DeviationEntry and document DeviationReasonIntentExists as an intent marker rather than a reason. Signed-off-by: Markus Vahlenkamp --- pkg/api/deviation.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/pkg/api/deviation.go b/pkg/api/deviation.go index 6579d9d9..4d691b1b 100644 --- a/pkg/api/deviation.go +++ b/pkg/api/deviation.go @@ -22,22 +22,20 @@ const ( DeviationReasonUnhandled DeviationReasonNotApplied DeviationReasonOverruled + // DeviationReasonIntentExists is not a deviation reason: it marks that the + // named intent exists, and is sent without path or values. DeviationReasonIntentExists ) -// DeviationEntry is one deviating path with its expected and current values. -type DeviationEntry struct { - Path *sdcpb.Path - Expected *sdcpb.TypedValue - Current *sdcpb.TypedValue -} - -// DeviationEvent is emitted by a deviation watch. Intent, Reason and Entries -// are set only where the Kind carries them. +// DeviationEvent is emitted by a deviation watch, one per deviating path. +// Intent, Reason, Path, Expected and Current are set only where the Kind +// carries them. type DeviationEvent struct { Datastore string Kind DeviationKind Intent string Reason DeviationReason - Entries []DeviationEntry + Path *sdcpb.Path + Expected *sdcpb.TypedValue + Current *sdcpb.TypedValue } From 6275100ade4e086f7c83515efcf827b50660bf01 Mon Sep 17 00:00:00 2001 From: Markus Vahlenkamp Date: Wed, 11 Nov 2026 13:32:05 +0100 Subject: [PATCH 7/8] refactor(api): move ComputeCreateDSHash to config-server Only config-server compares datastore hashes to decide when to recreate a datastore, and its extra inputs are Kubernetes resource versions. The data server never computes or compares a hash, so it does not belong in pkg/api. Signed-off-by: Markus Vahlenkamp --- pkg/api/hash.go | 103 -------------------------------------- pkg/api/hash_test.go | 115 ------------------------------------------- 2 files changed, 218 deletions(-) delete mode 100644 pkg/api/hash.go delete mode 100644 pkg/api/hash_test.go diff --git a/pkg/api/hash.go b/pkg/api/hash.go deleted file mode 100644 index 24025cfc..00000000 --- a/pkg/api/hash.go +++ /dev/null @@ -1,103 +0,0 @@ -package api - -import ( - "crypto/sha256" - "encoding/binary" - "encoding/hex" - "hash" -) - -// ComputeCreateDSHash returns a stable hex digest of the spec. Equal specs hash -// equal and any field change alters the digest, so callers recreate a datastore -// when it differs. extra values (for example resource versions of referenced -// objects) are mixed in, in order. -// -// The hash is over the resolved DatastoreSpec, not a caller's own API type, so -// it changes exactly when the datastore the server would build changes. Callers -// that resolve references (secrets, TLS material) pass their versions as extra. -// It lives next to DatastoreSpec so a new field cannot be added without -// hashing it; TestHashChangesOnEveryField enforces this. Changing the encoding -// changes every digest and recreates all datastores. -func ComputeCreateDSHash(spec DatastoreSpec, extra ...string) string { - w := hashWriter{h: sha256.New()} - w.str(spec.Name) - w.target(&spec.Target) - w.str(spec.Schema.Vendor) - w.str(spec.Schema.Version) - w.i64(int64(len(spec.Sync.Configs))) - for i := range spec.Sync.Configs { - c := &spec.Sync.Configs[i] - w.str(c.Name) - if w.present(c.Target != nil) { - w.target(c.Target) - } - w.i64(int64(len(c.Paths))) - for _, p := range c.Paths { - w.str(p) - } - w.i64(int64(c.Mode)) - w.i64(int64(c.Interval)) - } - w.i64(int64(len(extra))) - for _, e := range extra { - w.str(e) - } - return hex.EncodeToString(w.h.Sum(nil)) -} - -// hashWriter writes length-prefixed fields so adjacent values cannot collide. -type hashWriter struct { - h hash.Hash -} - -func (w hashWriter) i64(v int64) { - var b [8]byte - binary.BigEndian.PutUint64(b[:], uint64(v)) - w.h.Write(b[:]) -} - -func (w hashWriter) str(s string) { - w.i64(int64(len(s))) - w.h.Write([]byte(s)) -} - -func (w hashWriter) bool(v bool) { - if v { - w.h.Write([]byte{1}) - return - } - w.h.Write([]byte{0}) -} - -// present writes a presence marker and returns ok, so nil and zero values hash apart. -func (w hashWriter) present(ok bool) bool { - w.bool(ok) - return ok -} - -func (w hashWriter) target(t *Target) { - w.str(string(t.Type)) - w.str(t.Address) - w.i64(int64(t.Port)) - if w.present(t.Credentials != nil) { - w.str(t.Credentials.Username) - w.str(t.Credentials.Password) - w.str(t.Credentials.Token) - } - if w.present(t.TLS != nil) { - w.str(t.TLS.CA) - w.str(t.TLS.Cert) - w.str(t.TLS.Key) - w.bool(t.TLS.SkipVerify) - } - if g := t.ProtocolOptions.GNMI; w.present(g != nil) { - w.str(string(g.Encoding)) - w.str(g.TargetName) - } - if n := t.ProtocolOptions.Netconf; w.present(n != nil) { - w.bool(n.IncludeNs) - w.bool(n.OperationWithNs) - w.bool(n.UseOperationRemove) - w.i64(int64(n.CommitCandidate)) - } -} diff --git a/pkg/api/hash_test.go b/pkg/api/hash_test.go deleted file mode 100644 index 84b5f0c7..00000000 --- a/pkg/api/hash_test.go +++ /dev/null @@ -1,115 +0,0 @@ -package api - -import ( - "reflect" - "testing" -) - -func fullSpec() DatastoreSpec { - tgt := func() Target { - return Target{ - Type: TargetTypeNetconf, - Address: "10.0.0.1", - Port: 830, - Credentials: &Credentials{Username: "u", Password: "p", Token: "t"}, - TLS: &TLS{CA: "ca", Cert: "cert", Key: "key", SkipVerify: true}, - ProtocolOptions: ProtocolOptions{ - GNMI: &GNMIOptions{Encoding: GNMIEncodingJSONIETF, TargetName: "tn"}, - Netconf: &NetconfOptions{IncludeNs: true, OperationWithNs: true, UseOperationRemove: true, CommitCandidate: CommitCandidateRunning}, - }, - } - } - syncTgt := tgt() - return DatastoreSpec{ - Name: "ds1", - Target: tgt(), - Schema: SchemaRef{Vendor: "nokia", Version: "24.3"}, - Sync: Sync{ - Configs: []SyncConfig{{Name: "c", Target: &syncTgt, Paths: []string{"/a", "/b"}, Mode: SyncModeSample, Interval: 10}}, - }, - } -} - -func TestHashStable(t *testing.T) { - a, b := ComputeCreateDSHash(fullSpec()), ComputeCreateDSHash(fullSpec()) - if a != b { - t.Fatalf("hash differs for equal specs: %s vs %s", a, b) - } - if len(a) != 64 { - t.Fatalf("unexpected digest length %d", len(a)) - } -} - -// Golden value pins the canonical form: changing it forces every datastore to be recreated. -func TestHashGolden(t *testing.T) { - const want = "1c70111729aba574666d8c49f61cc5a7708f6e13d7005e9064b235847194abb7" - if got := ComputeCreateDSHash(fullSpec()); got != want { - t.Fatalf("hash = %s, want %s", got, want) - } -} - -// Every leaf field of the spec must influence the hash, so a change recreates the datastore. -func TestHashChangesOnEveryField(t *testing.T) { - base := ComputeCreateDSHash(fullSpec()) - s := fullSpec() - var walk func(path string, v reflect.Value) - walk = func(path string, v reflect.Value) { - switch v.Kind() { - case reflect.Pointer: - walk(path, v.Elem()) - case reflect.Struct: - for i := 0; i < v.NumField(); i++ { - walk(path+"."+v.Type().Field(i).Name, v.Field(i)) - } - case reflect.Slice: - if v.Len() > 0 { - walk(path+"[0]", v.Index(0)) - } - default: - orig := reflect.New(v.Type()).Elem() - orig.Set(v) - switch v.Kind() { - case reflect.String: - v.SetString(v.String() + "x") - case reflect.Bool: - v.SetBool(!v.Bool()) - case reflect.Int, reflect.Int32, reflect.Int64: - v.SetInt(v.Int() + 1) - case reflect.Uint32, reflect.Uint64: - v.SetUint(v.Uint() + 1) - default: - t.Fatalf("%s: unhandled kind %s", path, v.Kind()) - } - if ComputeCreateDSHash(s) == base { - t.Errorf("hash ignores %s", path) - } - v.Set(orig) - } - } - walk("Spec", reflect.ValueOf(&s).Elem()) -} - -func TestHashNilVersusEmpty(t *testing.T) { - a := fullSpec() - b := fullSpec() - a.Target.Credentials = nil - b.Target.Credentials = &Credentials{} - if ComputeCreateDSHash(a) == ComputeCreateDSHash(b) { - t.Fatal("nil and empty credentials hash equal") - } -} - -func TestHashExtra(t *testing.T) { - s := fullSpec() - base := ComputeCreateDSHash(s) - withExtra := ComputeCreateDSHash(s, "1") - if withExtra == base { - t.Fatal("extra ignored") - } - if withExtra != ComputeCreateDSHash(s, "1") { - t.Fatal("extra unstable") - } - if ComputeCreateDSHash(s, "a", "b") == ComputeCreateDSHash(s, "ab") { - t.Fatal("extra boundaries collide") - } -} From efb48a518c0ee2450d8fba1b337c99ba7b50ed7f Mon Sep 17 00:00:00 2001 From: Markus Vahlenkamp Date: Wed, 11 Nov 2026 13:48:48 +0100 Subject: [PATCH 8/8] feat(api): add GNMIEncodingConfig for the vendor CONFIG encoding CONFIG is sent as gNMI encoding 45. The typed encoding rejects numeric values, so name it explicitly and resolve it in the gNMI encoding parsers. Signed-off-by: Markus Vahlenkamp --- pkg/api/datastore.go | 4 +++- pkg/api/datastore_test.go | 2 +- pkg/api/doc.go | 6 +++--- pkg/api/transaction.go | 11 ++++++++--- pkg/datastore/target/gnmi/utils/utils.go | 9 +++++++++ pkg/datastore/target/gnmi/utils/utils_test.go | 15 +++++++++++++++ 6 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 pkg/datastore/target/gnmi/utils/utils_test.go diff --git a/pkg/api/datastore.go b/pkg/api/datastore.go index a0599b06..84cbf682 100644 --- a/pkg/api/datastore.go +++ b/pkg/api/datastore.go @@ -54,13 +54,15 @@ const ( GNMIEncodingProto GNMIEncoding = "PROTO" GNMIEncodingASCII GNMIEncoding = "ASCII" GNMIEncodingJSONIETF GNMIEncoding = "JSON_IETF" + // GNMIEncodingConfig is the vendor-specific CONFIG encoding, sent as gNMI encoding 45. + GNMIEncodingConfig GNMIEncoding = "CONFIG" ) // ParseGNMIEncoding resolves a case-insensitive encoding name. Unknown names, // including numeric values, are rejected. func ParseGNMIEncoding(s string) (GNMIEncoding, error) { switch e := GNMIEncoding(strings.ToUpper(s)); e { - case GNMIEncodingJSON, GNMIEncodingBytes, GNMIEncodingProto, GNMIEncodingASCII, GNMIEncodingJSONIETF: + case GNMIEncodingJSON, GNMIEncodingBytes, GNMIEncodingProto, GNMIEncodingASCII, GNMIEncodingJSONIETF, GNMIEncodingConfig: return e, nil } return "", fmt.Errorf("unknown gnmi encoding %q", s) diff --git a/pkg/api/datastore_test.go b/pkg/api/datastore_test.go index 59173746..3e0a9f4b 100644 --- a/pkg/api/datastore_test.go +++ b/pkg/api/datastore_test.go @@ -3,7 +3,7 @@ package api import "testing" func TestParseGNMIEncoding(t *testing.T) { - for in, want := range map[string]GNMIEncoding{"json_ietf": GNMIEncodingJSONIETF, "JSON": GNMIEncodingJSON, "Proto": GNMIEncodingProto} { + for in, want := range map[string]GNMIEncoding{"json_ietf": GNMIEncodingJSONIETF, "JSON": GNMIEncodingJSON, "Proto": GNMIEncodingProto, "config": GNMIEncodingConfig} { if got, err := ParseGNMIEncoding(in); err != nil || got != want { t.Errorf("%q: got %q, %v", in, got, err) } diff --git a/pkg/api/doc.go b/pkg/api/doc.go index cf954f86..6eafe856 100644 --- a/pkg/api/doc.go +++ b/pkg/api/doc.go @@ -1,6 +1,6 @@ // Package api holds the domain types, error sentinels and helpers shared by the // data-server core and its consumers. It is types-only: it imports neither gRPC -// nor any data-server internals. The only sdcpb types it uses are TypedValue and -// Path, the value and address types of the domain; no other sdcpb type may -// appear here. +// nor any data-server internals. The only sdcpb types it uses are TypedValue, +// Path and Update, the value, address and path-value types of the domain; no +// other sdcpb type may appear here. package api diff --git a/pkg/api/transaction.go b/pkg/api/transaction.go index de350d2d..ed21eddc 100644 --- a/pkg/api/transaction.go +++ b/pkg/api/transaction.go @@ -1,6 +1,10 @@ package api -import "time" +import ( + "time" + + sdcpb "github.com/sdcio/sdc-protos/sdcpb" +) // Origin identifies the requester of an operation for logging and audit. It is // optional; the gRPC adapter fills it from the peer and in-process callers set @@ -37,8 +41,9 @@ type Intent struct { // Update creates or replaces an intent. type Update struct { Priority int32 - // Content is the intent body, JSON encoded. - Content []byte + // Updates are the intent body. A JSON value expands below its path into + // typed leaf values; other value types are taken as given. + Updates []*sdcpb.Update // NonRevertive leaves device-side changes in place instead of reverting them. NonRevertive bool // PreviouslyApplied marks a non-revertive intent that was applied before. diff --git a/pkg/datastore/target/gnmi/utils/utils.go b/pkg/datastore/target/gnmi/utils/utils.go index 2d1b4129..aefa23a7 100644 --- a/pkg/datastore/target/gnmi/utils/utils.go +++ b/pkg/datastore/target/gnmi/utils/utils.go @@ -37,7 +37,13 @@ func SdcpbEncodingToGNMIENcoding(x sdcpb.Encoding) (gnmi.Encoding, error) { return 9999, fmt.Errorf("unable to convert sdcpb encoding %s to gnmi encoding", x) } +// configEncoding is the vendor-specific gNMI encoding value behind the CONFIG name. +const configEncoding = 45 + func ParseGnmiEncoding(e string) int { + if strings.EqualFold(e, "CONFIG") { + return configEncoding + } enc, ok := gnmi.Encoding_value[strings.ToUpper(e)] if ok { return int(enc) @@ -50,6 +56,9 @@ func ParseGnmiEncoding(e string) int { } func ParseSdcpbEncoding(e string) int { + if strings.EqualFold(e, "CONFIG") { + return configEncoding + } enc, ok := sdcpb.Encoding_value[strings.ToUpper(e)] if ok { return int(enc) diff --git a/pkg/datastore/target/gnmi/utils/utils_test.go b/pkg/datastore/target/gnmi/utils/utils_test.go new file mode 100644 index 00000000..9dcce0a8 --- /dev/null +++ b/pkg/datastore/target/gnmi/utils/utils_test.go @@ -0,0 +1,15 @@ +package utils + +import "testing" + +func TestParseEncodingConfig(t *testing.T) { + if got := ParseGnmiEncoding("config"); got != 45 { + t.Errorf("ParseGnmiEncoding(config) = %d, want 45", got) + } + if got := ParseSdcpbEncoding("CONFIG"); got != 45 { + t.Errorf("ParseSdcpbEncoding(CONFIG) = %d, want 45", got) + } + if got := ParseGnmiEncoding("45"); got != 45 { + t.Errorf("ParseGnmiEncoding(45) = %d, want 45", got) + } +}