diff --git a/.github/workflows/cicd.yml b/.github/workflows/cicd.yml index bdc918b1..b57d2238 100644 --- a/.github/workflows/cicd.yml +++ b/.github/workflows/cicd.yml @@ -204,10 +204,14 @@ jobs: - name: Set env vars id: latest-versions run: | - echo "schemaversion=$( curl -sL https://api.github.com/repos/sdcio/schema-server/releases/latest | jq '.name' )" >> $GITHUB_OUTPUT - echo "dataversion=$( curl -sL https://api.github.com/repos/sdcio/data-server/releases/latest | jq '.name' )" >> $GITHUB_OUTPUT - echo "cacheversion=$( curl -sL https://api.github.com/repos/sdcio/cache/releases/latest | jq '.name' )" >> $GITHUB_OUTPUT - echo "certmanagerversion=$( curl -sL https://api.github.com/repos/cert-manager/cert-manager/releases/latest | jq '.name' )" >> $GITHUB_OUTPUT + set -euo pipefail + latest_release_name() { + curl -sfL "$1" | jq -er '.name' + } + echo "schemaversion=$(latest_release_name https://api.github.com/repos/sdcio/schema-server/releases/latest)" >> "$GITHUB_OUTPUT" + echo "dataversion=$(latest_release_name https://api.github.com/repos/sdcio/data-server/releases/latest)" >> "$GITHUB_OUTPUT" + echo "cacheversion=$(latest_release_name https://api.github.com/repos/sdcio/cache/releases/latest)" >> "$GITHUB_OUTPUT" + echo "certmanagerversion=$(latest_release_name https://api.github.com/repos/cert-manager/cert-manager/releases/latest)" >> "$GITHUB_OUTPUT" integration-tests: needs: [latest-versions, pr-release, resolve-paired-refs] @@ -220,3 +224,18 @@ jobs: certmanager_version: ${{ needs.latest-versions.outputs.certmanagerversion }} integration_tests_ref: ${{ needs.resolve-paired-refs.outputs.inttest_ref }} secrets: inherit + + integration-tests-cache-backend: + name: integration-tests (config-server cache backend) + needs: [latest-versions, pr-release, resolve-paired-refs] + uses: sdcio/integration-tests/.github/workflows/single.yml@main + with: + configserver_version: ${{ needs.resolve-paired-refs.outputs.configversion }} + dataserver_version: ${{ needs.pr-release.outputs.dataversion }} + schemaserver_version: ${{ needs.latest-versions.outputs.schemaversion }} + cache_version: ${{ needs.latest-versions.outputs.cacheversion }} + certmanager_version: ${{ needs.latest-versions.outputs.certmanagerversion }} + integration_tests_ref: ${{ needs.resolve-paired-refs.outputs.inttest_ref }} + cache_type: config-server + suites_to_run: 00-setup,01-crs,05-cache-backend + secrets: inherit diff --git a/Makefile b/Makefile index bbe483ff..2b196f30 100644 --- a/Makefile +++ b/Makefile @@ -65,6 +65,7 @@ mocks-gen: mocks-rm ## Generate mocks for all the defined interfaces. mockgen -package=mocktarget -source=pkg/datastore/target/target.go -destination=$(MOCKDIR)/mocktarget/target.go mockgen -package=mockTreeEntry -source=pkg/tree/api/entry.go -destination=$(MOCKDIR)/mocktreeentry/entry.go mockgen -package=mocksdcpbpath -source=pkg/tree/api/sdcpb_path.go -destination=$(MOCKDIR)/mocksdcpbpath/sdcpb_path.go + mockgen -package=mockconfigread -destination=$(MOCKDIR)/mockconfigread/client.go github.com/sdcio/sdc-protos/config_read ConfigReadServiceClient .PHONY: mocks-rm mocks-rm: ## remove generated mocks diff --git a/go.mod b/go.mod index 398d8bcc..c137260d 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,7 @@ require ( github.com/sdcio/cache v0.0.38 github.com/sdcio/logger v0.0.3 github.com/sdcio/schema-server v0.0.34 - github.com/sdcio/sdc-protos v0.0.56-0.20260923091638-fd8bd021dd18 + github.com/sdcio/sdc-protos v0.0.56-0.20260923101805-f894efbc2109 github.com/sdcio/yang-parser v0.0.12 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 diff --git a/go.sum b/go.sum index d735cb95..8a92d9e1 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-0.20260923091638-fd8bd021dd18 h1:zZcmOenZMl7SaIXacu4sUJmVwdia0Y8w6R1vU7AZCO0= -github.com/sdcio/sdc-protos v0.0.56-0.20260923091638-fd8bd021dd18/go.mod h1:hER0riYU3G55NnMQ0p0R9kIL/fzqq1zab4pOgUT3Yfg= +github.com/sdcio/sdc-protos v0.0.56-0.20260923101805-f894efbc2109 h1:LUWrCZAnPui2+vx2wdIYVEAHATqCv+/PVbK6QUmPhCY= +github.com/sdcio/sdc-protos v0.0.56-0.20260923101805-f894efbc2109/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/mocks/mockconfigread/client.go b/mocks/mockconfigread/client.go new file mode 100644 index 00000000..e2ddb909 --- /dev/null +++ b/mocks/mockconfigread/client.go @@ -0,0 +1,83 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/sdcio/sdc-protos/config_read (interfaces: ConfigReadServiceClient) +// +// Generated by this command: +// +// mockgen -package=mockconfigread -destination=./mocks/mockconfigread/client.go github.com/sdcio/sdc-protos/config_read ConfigReadServiceClient +// + +// Package mockconfigread is a generated GoMock package. +package mockconfigread + +import ( + context "context" + reflect "reflect" + + config_read "github.com/sdcio/sdc-protos/config_read" + gomock "go.uber.org/mock/gomock" + grpc "google.golang.org/grpc" +) + +// MockConfigReadServiceClient is a mock of ConfigReadServiceClient interface. +type MockConfigReadServiceClient struct { + ctrl *gomock.Controller + recorder *MockConfigReadServiceClientMockRecorder + isgomock struct{} +} + +// MockConfigReadServiceClientMockRecorder is the mock recorder for MockConfigReadServiceClient. +type MockConfigReadServiceClientMockRecorder struct { + mock *MockConfigReadServiceClient +} + +// NewMockConfigReadServiceClient creates a new mock instance. +func NewMockConfigReadServiceClient(ctrl *gomock.Controller) *MockConfigReadServiceClient { + mock := &MockConfigReadServiceClient{ctrl: ctrl} + mock.recorder = &MockConfigReadServiceClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockConfigReadServiceClient) EXPECT() *MockConfigReadServiceClientMockRecorder { + return m.recorder +} + +// Get mocks base method. +func (m *MockConfigReadServiceClient) Get(ctx context.Context, in *config_read.GetConfigRequest, opts ...grpc.CallOption) (*config_read.GetConfigResponse, error) { + m.ctrl.T.Helper() + varargs := []any{ctx, in} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Get", varargs...) + ret0, _ := ret[0].(*config_read.GetConfigResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockConfigReadServiceClientMockRecorder) Get(ctx, in any, opts ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{ctx, in}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockConfigReadServiceClient)(nil).Get), varargs...) +} + +// List mocks base method. +func (m *MockConfigReadServiceClient) List(ctx context.Context, in *config_read.ListConfigRequest, opts ...grpc.CallOption) (*config_read.ListConfigResponse, error) { + m.ctrl.T.Helper() + varargs := []any{ctx, in} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "List", varargs...) + ret0, _ := ret[0].(*config_read.ListConfigResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// List indicates an expected call of List. +func (mr *MockConfigReadServiceClientMockRecorder) List(ctx, in any, opts ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{ctx, in}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockConfigReadServiceClient)(nil).List), varargs...) +} diff --git a/pkg/cache/configserver.go b/pkg/cache/configserver.go new file mode 100644 index 00000000..592bd718 --- /dev/null +++ b/pkg/cache/configserver.go @@ -0,0 +1,264 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cache + +import ( + "context" + "errors" + "fmt" + "strings" + "sync" + + "github.com/sdcio/data-server/pkg/cache/configserver" + "github.com/sdcio/data-server/pkg/tree/importer" + treeproto "github.com/sdcio/data-server/pkg/tree/importer/proto" + "github.com/sdcio/sdc-protos/tree_persist" +) + +// ConfigServerCache is the Cache.Type: config-server Client: real Intents +// are read-only, served through a configserver.LocalConfigReader seam over +// the colocated config-server controller's own watch-synced store — this +// backend never persists its own copy of them, and write calls on them are +// unconditional no-ops, since config-server/kube-api is the sole writer. +// "running" is unaffected by backend choice (see the ADR): it is kept in its +// own independent in-memory, per-instance store, entirely separate from the +// seam. +type ConfigServerCache struct { + reader configserver.LocalConfigReader + + mu sync.RWMutex + running map[string]*tree_persist.Intent +} + +// NewConfigServerCache returns a Client backed by reader for real Intents. +// Every InstanceIntent* call derives its target namespace/name from the +// cacheInstanceName it's given (see target), so a single ConfigServerCache +// correctly serves datastores across multiple Kubernetes namespaces. +func NewConfigServerCache(reader configserver.LocalConfigReader) *ConfigServerCache { + return &ConfigServerCache{ + reader: reader, + running: map[string]*tree_persist.Intent{}, + } +} + +// NewConfigServerClient composes a reader-backed *ConfigServerCache with the +// generic noopIntentWriter into a full Client. ConfigServerCache alone never +// implements IntentWriter (config-server/kube-api is the sole writer of real +// Intents), so this is the one seam Server.createCacheClient's config-server +// case uses to assemble s.cacheClient. +func NewConfigServerClient(reader configserver.LocalConfigReader) Client { + return struct { + *ConfigServerCache + noopIntentWriter + }{ + ConfigServerCache: NewConfigServerCache(reader), + } +} + +// target derives the target namespace/name from cacheInstanceName, mirroring +// config-server's own . datastore-naming +// convention. It returns ErrMalformedDatastoreName when cacheInstanceName +// doesn't decode into a non-empty namespace and a non-empty name. +func (c *ConfigServerCache) target(cacheInstanceName string) (configserver.Target, error) { + return splitDatastoreName(cacheInstanceName) +} + +// lookupConfigName maps an owner/intent name onto the bare Config resource +// name ConfigReadService keys TargetSnapshot.Spec.Configs by. GetGVKNSN +// names (".") are stripped when the namespace matches the +// target; a bare name is passed through unchanged so existing Get callers +// keep working. +func lookupConfigName(target configserver.Target, intentName string) string { + prefix := target.Namespace + "." + if rest, ok := strings.CutPrefix(intentName, prefix); ok && rest != "" { + return rest + } + return intentName +} + +// InstanceCreate/InstanceDelete/InstanceClose/InstanceExists/InstancesList +// only manage the independent "running" store: real intents live in +// config-server, never locally, so there is nothing else to create/delete. + +// InstanceCreate/InstanceDelete error on an already-existing/missing +// instance, mirroring the error semantics of the underlying +// github.com/sdcio/cache Cache.InstanceCreate/InstanceDelete that LocalCache +// itself defers to for the same methods. +func (c *ConfigServerCache) InstanceCreate(ctx context.Context, cacheInstanceName string) error { + if _, err := c.target(cacheInstanceName); err != nil { + return err + } + c.mu.Lock() + defer c.mu.Unlock() + if _, exists := c.running[cacheInstanceName]; exists { + return fmt.Errorf("configserver cache: instance %q already exists", cacheInstanceName) + } + // Seed with an empty Intent rather than nil: InstanceRunningGet has + // nothing special to do for "not yet modified" beyond returning that + // empty value, same as a freshly created LocalCache instance's running + // store starts out with no data written to it yet. + c.running[cacheInstanceName] = &tree_persist.Intent{} + return nil +} + +func (c *ConfigServerCache) InstanceDelete(ctx context.Context, cacheInstanceName string) error { + c.mu.Lock() + defer c.mu.Unlock() + if _, exists := c.running[cacheInstanceName]; !exists { + return fmt.Errorf("configserver cache: instance %q does not exist", cacheInstanceName) + } + delete(c.running, cacheInstanceName) + return nil +} + +// InstanceClose removes the instance's running entry same as InstanceDelete: +// github.com/sdcio/cache's own Cache.InstanceClose (which LocalCache defers +// to) does the same — it closes the instance's store handle and then +// deletes it from the instance map, the same end state InstanceDelete +// reaches directly. There is no lighter-weight "close but keep" state to +// preserve here, since this backend's only local state for an instance is +// the running store itself. +func (c *ConfigServerCache) InstanceClose(ctx context.Context, cacheInstanceName string) error { + return c.InstanceDelete(ctx, cacheInstanceName) +} + +func (c *ConfigServerCache) InstanceExists(ctx context.Context, cacheInstanceName string) bool { + c.mu.RLock() + defer c.mu.RUnlock() + _, exists := c.running[cacheInstanceName] + return exists +} + +func (c *ConfigServerCache) InstancesList(ctx context.Context) []string { + c.mu.RLock() + defer c.mu.RUnlock() + result := make([]string, 0, len(c.running)) + for name := range c.running { + result = append(result, name) + } + return result +} + +// InstanceIntentsList calls List, mapped down to names only — client.List +// always returns full objects on the config-server side, so there is no +// cheaper name-only mode to prefer. +func (c *ConfigServerCache) InstanceIntentsList(ctx context.Context, cacheInstanceName string) ([]string, error) { + target, err := c.target(cacheInstanceName) + if err != nil { + return nil, err + } + docs, err := c.reader.List(ctx, target) + if err != nil { + return nil, err + } + names := make([]string, 0, len(docs)) + for _, d := range docs { + names = append(names, d.IntentName()) + } + return names, nil +} + +// InstanceIntentGet calls Get, wrapping the result in an +// importer.IntentAdapter per the ADR's field-mapping table. +func (c *ConfigServerCache) InstanceIntentGet(ctx context.Context, cacheName string, intentName string) (importer.IntentAdapter, error) { + target, err := c.target(cacheName) + if err != nil { + return nil, err + } + doc, err := c.reader.Get(ctx, target, lookupConfigName(target, intentName)) + if err != nil { + return nil, err + } + return configserver.NewImportAdapter(doc) +} + +// InstanceIntentExists calls Get and maps "not found" to (false, nil), +// matching the Client contract's existing meaning of that return; any other +// error propagates as (false, err). +func (c *ConfigServerCache) InstanceIntentExists(ctx context.Context, cacheName string, intentName string) (bool, error) { + target, err := c.target(cacheName) + if err != nil { + return false, err + } + _, err = c.reader.Get(ctx, target, lookupConfigName(target, intentName)) + if err != nil { + if errors.Is(err, configserver.ErrNotFound) { + return false, nil + } + return false, err + } + return true, nil +} + +// InstanceIntentGetAll calls List, then ranges over the results sending each +// into intentChan, closing intentChan/errChan when done or on ctx.Done() — +// mirroring LocalCache.InstanceIntentGetAll's existing shape. +// +// excludeIntentNames is accepted for interface compatibility but is always a +// no-op in practice: config-server has no "running" Config resource, so it +// can never appear in a List result to begin with. +func (c *ConfigServerCache) InstanceIntentGetAll(ctx context.Context, cacheName string, excludeIntentNames []string, intentChan chan<- importer.IntentAdapter, errChan chan<- error) { + defer close(intentChan) + defer close(errChan) + + target, err := c.target(cacheName) + if err != nil { + errChan <- err + return + } + + docs, err := c.reader.List(ctx, target) + if err != nil { + errChan <- err + return + } + + for _, d := range docs { + adapter, err := configserver.NewImportAdapter(d) + if err != nil { + errChan <- err + return + } + select { + case <-ctx.Done(): + return + case intentChan <- adapter: + } + } +} + +// InstanceRunningGet/InstanceRunningModify back "running" with its own +// independent in-memory, per-instance store — entirely separate from the +// seam, since "running" never touches config-server under any backend. + +func (c *ConfigServerCache) InstanceRunningGet(ctx context.Context, cacheName string) (importer.ImportConfigAdapter, error) { + c.mu.RLock() + defer c.mu.RUnlock() + intent, exists := c.running[cacheName] + if !exists { + return nil, fmt.Errorf("configserver cache: instance %q does not exist", cacheName) + } + return treeproto.NewProtoTreeImporter(intent), nil +} + +func (c *ConfigServerCache) InstanceRunningModify(ctx context.Context, cacheName string, intent *tree_persist.Intent) error { + c.mu.Lock() + defer c.mu.Unlock() + if _, exists := c.running[cacheName]; !exists { + return fmt.Errorf("configserver cache: instance %q does not exist", cacheName) + } + c.running[cacheName] = intent + return nil +} diff --git a/pkg/cache/configserver/fake.go b/pkg/cache/configserver/fake.go new file mode 100644 index 00000000..354dda80 --- /dev/null +++ b/pkg/cache/configserver/fake.go @@ -0,0 +1,91 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configserver + +import ( + "context" + "sort" + "sync" +) + +// FakeLocalConfigReader is an in-memory LocalConfigReader, seedable with test +// fixtures. It stands in for the real config-server local-read transport in +// unit tests, so the config-server-backed cache.Client can be built and fully +// tested well before that transport exists. +type FakeLocalConfigReader struct { + mu sync.RWMutex + docs map[Target]map[string]*Document +} + +// NewFakeLocalConfigReader returns an empty FakeLocalConfigReader. Use Seed +// to populate it with fixtures. +func NewFakeLocalConfigReader() *FakeLocalConfigReader { + return &FakeLocalConfigReader{ + docs: map[Target]map[string]*Document{}, + } +} + +// Seed adds/replaces Documents for target, keyed by Document.Name. +func (f *FakeLocalConfigReader) Seed(target Target, docs ...*Document) { + f.mu.Lock() + defer f.mu.Unlock() + + byName, ok := f.docs[target] + if !ok { + byName = make(map[string]*Document, len(docs)) + f.docs[target] = byName + } + for _, d := range docs { + byName[d.Name] = d + } +} + +// Reset removes every seeded Document for every target. +func (f *FakeLocalConfigReader) Reset() { + f.mu.Lock() + defer f.mu.Unlock() + f.docs = map[Target]map[string]*Document{} +} + +func (f *FakeLocalConfigReader) Get(ctx context.Context, target Target, name string) (*Document, error) { + f.mu.RLock() + defer f.mu.RUnlock() + + byName, ok := f.docs[target] + if !ok { + return nil, ErrNotFound + } + doc, ok := byName[name] + if !ok { + return nil, ErrNotFound + } + return doc, nil +} + +func (f *FakeLocalConfigReader) List(ctx context.Context, target Target) ([]*Document, error) { + f.mu.RLock() + defer f.mu.RUnlock() + + byName := f.docs[target] + result := make([]*Document, 0, len(byName)) + for _, doc := range byName { + result = append(result, doc) + } + // deterministic ordering for tests/callers that care. + sort.Slice(result, func(i, j int) bool { return result[i].Name < result[j].Name }) + return result, nil +} + +var _ LocalConfigReader = (*FakeLocalConfigReader)(nil) diff --git a/pkg/cache/configserver/fake_test.go b/pkg/cache/configserver/fake_test.go new file mode 100644 index 00000000..06a31d41 --- /dev/null +++ b/pkg/cache/configserver/fake_test.go @@ -0,0 +1,134 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configserver + +import ( + "context" + "errors" + "testing" + + "github.com/google/go-cmp/cmp" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" + "google.golang.org/protobuf/testing/protocmp" +) + +func TestFakeLocalConfigReader_GetNotFound(t *testing.T) { + f := NewFakeLocalConfigReader() + + _, err := f.Get(context.Background(), Target{Namespace: "ns1", Name: "target1"}, "missing") + if !errors.Is(err, ErrNotFound) { + t.Fatalf("Get() error = %v, want ErrNotFound", err) + } +} + +func TestFakeLocalConfigReader_GetNotFound_UnknownTarget(t *testing.T) { + f := NewFakeLocalConfigReader() + f.Seed(Target{Namespace: "ns1", Name: "target1"}, &Document{Name: "intent1"}) + + _, err := f.Get(context.Background(), Target{Namespace: "ns1", Name: "other-target"}, "intent1") + if !errors.Is(err, ErrNotFound) { + t.Fatalf("Get() error = %v, want ErrNotFound", err) + } +} + +func TestFakeLocalConfigReader_SeedAndGet(t *testing.T) { + f := NewFakeLocalConfigReader() + target := Target{Namespace: "ns1", Name: "target1"} + + want := &Document{ + Name: "intent1", + Priority: 10, + NonRevertive: true, + Orphan: true, + SensitivePaths: []*sdcpb.Path{ + {Elem: []*sdcpb.PathElem{{Name: "secret"}}}, + }, + Config: []*ConfigBlob{ + {Path: "/interface[name=eth0]/description", Value: []byte(`"uplink"`)}, + }, + } + f.Seed(target, want) + + got, err := f.Get(context.Background(), target, "intent1") + if err != nil { + t.Fatalf("Get() error = %v", err) + } + if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" { + t.Errorf("Get() mismatch (-want +got):\n%s", diff) + } +} + +func TestFakeLocalConfigReader_List(t *testing.T) { + f := NewFakeLocalConfigReader() + target := Target{Namespace: "ns1", Name: "target1"} + other := Target{Namespace: "ns1", Name: "other-target"} + + f.Seed(target, &Document{Name: "intent2"}, &Document{Name: "intent1"}) + f.Seed(other, &Document{Name: "unrelated"}) + + got, err := f.List(context.Background(), target) + if err != nil { + t.Fatalf("List() error = %v", err) + } + + var names []string + for _, d := range got { + names = append(names, d.Name) + } + if diff := cmp.Diff([]string{"intent1", "intent2"}, names); diff != "" { + t.Errorf("List() names mismatch (-want +got):\n%s", diff) + } +} + +func TestFakeLocalConfigReader_ListEmptyForUnknownTarget(t *testing.T) { + f := NewFakeLocalConfigReader() + + got, err := f.List(context.Background(), Target{Namespace: "ns1", Name: "target1"}) + if err != nil { + t.Fatalf("List() error = %v", err) + } + if len(got) != 0 { + t.Errorf("List() = %v, want empty", got) + } +} + +func TestFakeLocalConfigReader_SeedReplacesByName(t *testing.T) { + f := NewFakeLocalConfigReader() + target := Target{Namespace: "ns1", Name: "target1"} + + f.Seed(target, &Document{Name: "intent1", Priority: 1}) + f.Seed(target, &Document{Name: "intent1", Priority: 2}) + + got, err := f.Get(context.Background(), target, "intent1") + if err != nil { + t.Fatalf("Get() error = %v", err) + } + if got.Priority != 2 { + t.Errorf("Get().Priority = %d, want 2", got.Priority) + } +} + +func TestFakeLocalConfigReader_Reset(t *testing.T) { + f := NewFakeLocalConfigReader() + target := Target{Namespace: "ns1", Name: "target1"} + f.Seed(target, &Document{Name: "intent1"}) + + f.Reset() + + _, err := f.Get(context.Background(), target, "intent1") + if !errors.Is(err, ErrNotFound) { + t.Fatalf("Get() error = %v, want ErrNotFound after Reset", err) + } +} diff --git a/pkg/cache/configserver/grpc.go b/pkg/cache/configserver/grpc.go new file mode 100644 index 00000000..b4c21421 --- /dev/null +++ b/pkg/cache/configserver/grpc.go @@ -0,0 +1,115 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configserver + +import ( + "context" + + "github.com/sdcio/sdc-protos/config_read" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" +) + +// GRPCConfigReader implements LocalConfigReader over the real +// config_read.ConfigReadService, the localhost-bound gRPC surface ticket 07 +// serves from inside the colocated config-server controller. +// +// It takes a grpc.ClientConnInterface rather than dialing one itself, +// mirroring pkg/schema.NewRemoteClient: dial address/credentials are the +// caller's concern (Dial below is one convenience constructor for the +// localhost/insecure case this service is always deployed as), so nothing +// here is hardcoded. +type GRPCConfigReader struct { + client config_read.ConfigReadServiceClient +} + +// NewGRPCConfigReader returns a LocalConfigReader that calls the +// ConfigReadService over cc. +func NewGRPCConfigReader(cc grpc.ClientConnInterface) *GRPCConfigReader { + return &GRPCConfigReader{client: config_read.NewConfigReadServiceClient(cc)} +} + +// Dial establishes an insecure gRPC connection to address, the shape every +// caller needs for this service: it is always localhost-bound, colocated in +// the same pod as the controller that serves it (see the ADR's Scope note). +// address (and thus which port to reach) is entirely caller-supplied, never +// hardcoded here. +func Dial(address string, opts ...grpc.DialOption) (*grpc.ClientConn, error) { + dialOpts := opts + if len(dialOpts) == 0 { + dialOpts = []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())} + } + return grpc.NewClient(address, dialOpts...) +} + +// Get calls ConfigReadService.Get, mapping a NotFound gRPC status to +// ErrNotFound per the LocalConfigReader contract; any other error +// propagates unwrapped. +func (r *GRPCConfigReader) Get(ctx context.Context, target Target, name string) (*Document, error) { + rsp, err := r.client.Get(ctx, &config_read.GetConfigRequest{ + TargetNamespace: target.Namespace, + TargetName: target.Name, + Name: name, + }) + if err != nil { + if status.Code(err) == codes.NotFound { + return nil, ErrNotFound + } + return nil, err + } + return documentFromEntry(rsp.GetConfig()), nil +} + +// List calls ConfigReadService.List, mapping every returned ConfigEntry into +// a Document. +func (r *GRPCConfigReader) List(ctx context.Context, target Target) ([]*Document, error) { + rsp, err := r.client.List(ctx, &config_read.ListConfigRequest{ + TargetNamespace: target.Namespace, + TargetName: target.Name, + }) + if err != nil { + return nil, err + } + entries := rsp.GetConfig() + docs := make([]*Document, 0, len(entries)) + for _, e := range entries { + docs = append(docs, documentFromEntry(e)) + } + return docs, nil +} + +// documentFromEntry maps a config_read.ConfigEntry onto the seam's Document +// shape, field for field — no further translation happens here, that's +// NewImportAdapter's job. +func documentFromEntry(e *config_read.ConfigEntry) *Document { + blobs := e.GetConfig() + config := make([]*ConfigBlob, 0, len(blobs)) + for _, b := range blobs { + config = append(config, &ConfigBlob{Path: b.GetPath(), Value: b.GetValue()}) + } + return &Document{ + Name: e.GetName(), + Namespace: e.GetNamespace(), + Priority: e.GetPriority(), + NonRevertive: e.GetNonRevertive(), + Orphan: e.GetOrphan(), + SensitivePaths: e.GetSensitivePaths(), + Config: config, + } +} + +var _ LocalConfigReader = (*GRPCConfigReader)(nil) diff --git a/pkg/cache/configserver/grpc_test.go b/pkg/cache/configserver/grpc_test.go new file mode 100644 index 00000000..df023080 --- /dev/null +++ b/pkg/cache/configserver/grpc_test.go @@ -0,0 +1,187 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configserver + +import ( + "context" + "errors" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/sdcio/data-server/mocks/mockconfigread" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" + "go.uber.org/mock/gomock" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/testing/protocmp" + + "github.com/sdcio/sdc-protos/config_read" +) + +func TestGRPCConfigReader_Get(t *testing.T) { + ctrl := gomock.NewController(t) + client := mockconfigread.NewMockConfigReadServiceClient(ctrl) + target := Target{Namespace: "ns1", Name: "target1"} + + client.EXPECT().Get(gomock.Any(), &config_read.GetConfigRequest{ + TargetNamespace: "ns1", + TargetName: "target1", + Name: "intent1", + }).Return(&config_read.GetConfigResponse{ + Config: &config_read.ConfigEntry{ + Name: "intent1", + Namespace: "ns1", + Priority: 10, + NonRevertive: true, + Orphan: true, + SensitivePaths: []*sdcpb.Path{ + {Elem: []*sdcpb.PathElem{{Name: "secret"}}}, + }, + Config: []*config_read.ConfigBlob{ + {Path: "/interface[name=eth0]/description", Value: []byte(`"uplink"`)}, + }, + }, + }, nil) + + r := NewGRPCConfigReader(nil) + r.client = client + + want := &Document{ + Name: "intent1", + Namespace: "ns1", + Priority: 10, + NonRevertive: true, + Orphan: true, + SensitivePaths: []*sdcpb.Path{ + {Elem: []*sdcpb.PathElem{{Name: "secret"}}}, + }, + Config: []*ConfigBlob{ + {Path: "/interface[name=eth0]/description", Value: []byte(`"uplink"`)}, + }, + } + + got, err := r.Get(context.Background(), target, "intent1") + if err != nil { + t.Fatalf("Get() error = %v", err) + } + if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" { + t.Errorf("Get() mismatch (-want +got):\n%s", diff) + } +} + +func TestGRPCConfigReader_Get_NotFound(t *testing.T) { + ctrl := gomock.NewController(t) + client := mockconfigread.NewMockConfigReadServiceClient(ctrl) + target := Target{Namespace: "ns1", Name: "target1"} + + client.EXPECT().Get(gomock.Any(), gomock.Any()). + Return(nil, status.Error(codes.NotFound, "no such Config")) + + r := NewGRPCConfigReader(nil) + r.client = client + + _, err := r.Get(context.Background(), target, "missing") + if !errors.Is(err, ErrNotFound) { + t.Fatalf("Get() error = %v, want ErrNotFound", err) + } +} + +func TestGRPCConfigReader_Get_OtherError(t *testing.T) { + ctrl := gomock.NewController(t) + client := mockconfigread.NewMockConfigReadServiceClient(ctrl) + target := Target{Namespace: "ns1", Name: "target1"} + + wantErr := status.Error(codes.Unavailable, "controller unreachable") + client.EXPECT().Get(gomock.Any(), gomock.Any()).Return(nil, wantErr) + + r := NewGRPCConfigReader(nil) + r.client = client + + _, err := r.Get(context.Background(), target, "intent1") + if !errors.Is(err, wantErr) { + t.Fatalf("Get() error = %v, want %v", err, wantErr) + } + if errors.Is(err, ErrNotFound) { + t.Errorf("Get() error unexpectedly wrapped as ErrNotFound: %v", err) + } +} + +func TestGRPCConfigReader_List(t *testing.T) { + ctrl := gomock.NewController(t) + client := mockconfigread.NewMockConfigReadServiceClient(ctrl) + target := Target{Namespace: "ns1", Name: "target1"} + + client.EXPECT().List(gomock.Any(), &config_read.ListConfigRequest{ + TargetNamespace: "ns1", + TargetName: "target1", + }).Return(&config_read.ListConfigResponse{ + Config: []*config_read.ConfigEntry{ + {Name: "intent1", Namespace: "ns1", Priority: 1}, + {Name: "intent2", Namespace: "ns1", Priority: 2}, + }, + }, nil) + + r := NewGRPCConfigReader(nil) + r.client = client + + got, err := r.List(context.Background(), target) + if err != nil { + t.Fatalf("List() error = %v", err) + } + var names []string + for _, d := range got { + names = append(names, d.IntentName()) + } + if diff := cmp.Diff([]string{"ns1.intent1", "ns1.intent2"}, names); diff != "" { + t.Errorf("List() names mismatch (-want +got):\n%s", diff) + } +} + +func TestGRPCConfigReader_List_Empty(t *testing.T) { + ctrl := gomock.NewController(t) + client := mockconfigread.NewMockConfigReadServiceClient(ctrl) + target := Target{Namespace: "ns1", Name: "target1"} + + client.EXPECT().List(gomock.Any(), gomock.Any()). + Return(&config_read.ListConfigResponse{}, nil) + + r := NewGRPCConfigReader(nil) + r.client = client + + got, err := r.List(context.Background(), target) + if err != nil { + t.Fatalf("List() error = %v", err) + } + if len(got) != 0 { + t.Errorf("List() = %v, want empty", got) + } +} + +func TestGRPCConfigReader_List_Error(t *testing.T) { + ctrl := gomock.NewController(t) + client := mockconfigread.NewMockConfigReadServiceClient(ctrl) + target := Target{Namespace: "ns1", Name: "target1"} + + wantErr := status.Error(codes.Unavailable, "controller unreachable") + client.EXPECT().List(gomock.Any(), gomock.Any()).Return(nil, wantErr) + + r := NewGRPCConfigReader(nil) + r.client = client + + _, err := r.List(context.Background(), target) + if !errors.Is(err, wantErr) { + t.Fatalf("List() error = %v, want %v", err, wantErr) + } +} diff --git a/pkg/cache/configserver/importer.go b/pkg/cache/configserver/importer.go new file mode 100644 index 00000000..f591a6c0 --- /dev/null +++ b/pkg/cache/configserver/importer.go @@ -0,0 +1,60 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configserver + +import ( + "fmt" + + "github.com/sdcio/data-server/pkg/tree/importer" + jsonimporter "github.com/sdcio/data-server/pkg/tree/importer/json" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" +) + +// documentImporter adapts a Document — the local-read seam's representation +// of a config-server Config (joined with its SensitiveConfig) — into an +// importer.ImportConfigAdapter, per the ADR's field-mapping table. Traversal +// of the merged config payload is delegated to +// importer/json.JsonTreeImporter, which already knows how to walk the +// JSON_IETF-shaped map mergeConfigBlobs produces; only the two accessors +// JsonTreeImporter always stubs out for its own (synced-device-data) use +// case are overridden here, from Document fields the ADR maps them to. +type documentImporter struct { + *jsonimporter.JsonTreeImporter + orphan bool + sensitivePaths []*sdcpb.Path +} + +// NewImportAdapter builds the importer.IntentAdapter for doc. +func NewImportAdapter(doc *Document) (importer.IntentAdapter, error) { + root, err := mergeConfigBlobs(doc.Config) + if err != nil { + return nil, fmt.Errorf("configserver: building config for %q: %w", doc.Name, err) + } + return &documentImporter{ + JsonTreeImporter: jsonimporter.NewJsonTreeImporter(root, doc.IntentName(), doc.Priority, doc.NonRevertive), + orphan: doc.Orphan, + sensitivePaths: doc.SensitivePaths, + }, nil +} + +func (d *documentImporter) GetOrphan() bool { + return d.orphan +} + +func (d *documentImporter) GetSensitivePaths() []*sdcpb.Path { + return d.sensitivePaths +} + +var _ importer.IntentAdapter = (*documentImporter)(nil) diff --git a/pkg/cache/configserver/importer_test.go b/pkg/cache/configserver/importer_test.go new file mode 100644 index 00000000..51a3a3fd --- /dev/null +++ b/pkg/cache/configserver/importer_test.go @@ -0,0 +1,111 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configserver + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/sdcio/data-server/pkg/tree/importer" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" + "google.golang.org/protobuf/testing/protocmp" +) + +// TestNewImportAdapter_FieldMapping covers the ADR's field-mapping table end +// to end: every ImportConfigAdapter accessor must reflect the Document field +// it's mapped from. +func TestNewImportAdapter_FieldMapping(t *testing.T) { + sensitivePaths := []*sdcpb.Path{ + {Elem: []*sdcpb.PathElem{{Name: "secret"}}}, + } + doc := &Document{ + Name: "intent1", + Namespace: "ns1", + Priority: 10, + NonRevertive: true, + Orphan: true, + SensitivePaths: sensitivePaths, + Config: []*ConfigBlob{ + {Path: "/interface[name=eth0]/description", Value: []byte(`"uplink"`)}, + }, + } + + adapter, err := NewImportAdapter(doc) + if err != nil { + t.Fatalf("NewImportAdapter() error = %v", err) + } + + // ADR: GetName() is metadata.name/namespace via config.GetGVKNSN. + // A bare Config name (no ".") is what crashed config-server's + // DeviationWatcher: index parts[1] of SplitN(name, ".", 2). + const wantName = "ns1.intent1" + if got := adapter.GetName(); got != wantName { + t.Errorf("GetName() = %q, want %q (GetGVKNSN)", got, wantName) + } + if got := adapter.GetPriority(); got != doc.Priority { + t.Errorf("GetPriority() = %d, want %d", got, doc.Priority) + } + if got := adapter.GetNonRevertive(); got != doc.NonRevertive { + t.Errorf("GetNonRevertive() = %v, want %v", got, doc.NonRevertive) + } + if got := adapter.GetOrphan(); got != doc.Orphan { + t.Errorf("GetOrphan() = %v, want %v", got, doc.Orphan) + } + if diff := cmp.Diff(sensitivePaths, adapter.GetSensitivePaths(), protocmp.Transform()); diff != "" { + t.Errorf("GetSensitivePaths() mismatch (-want +got):\n%s", diff) + } + if adapter.GetDeletes() == nil { + t.Error("GetDeletes() = nil, want empty PathSet (config-server-owned intents never carry ExplicitDeletes)") + } + + elem := adapter.GetElement("interface") + if elem == nil { + t.Fatal("GetElement(\"interface\") = nil, want the merged config payload traversable") + } +} + +// TestNewImportAdapter_DefaultsWithoutSeeding verifies zero-valued Document +// fields (no orphan, no sensitive paths) round-trip as false/nil, per the +// ADR — these aren't errors, just the steady-state case for most intents. +func TestNewImportAdapter_DefaultsWithoutSeeding(t *testing.T) { + adapter, err := NewImportAdapter(&Document{Name: "intent1"}) + if err != nil { + t.Fatalf("NewImportAdapter() error = %v", err) + } + if adapter.GetOrphan() { + t.Error("GetOrphan() = true, want false") + } + if adapter.GetSensitivePaths() != nil { + t.Errorf("GetSensitivePaths() = %v, want nil", adapter.GetSensitivePaths()) + } +} + +func TestNewImportAdapter_InvalidConfigPropagatesError(t *testing.T) { + _, err := NewImportAdapter(&Document{ + Name: "intent1", + Config: []*ConfigBlob{{Path: "[invalid", Value: []byte(`1`)}}, + }) + if err == nil { + t.Fatal("NewImportAdapter() expected error for invalid config, got nil") + } +} + +func TestNewImportAdapter_ImplementsInterface(t *testing.T) { + adapter, err := NewImportAdapter(&Document{Name: "intent1"}) + if err != nil { + t.Fatalf("NewImportAdapter() error = %v", err) + } + var _ importer.ImportConfigAdapter = adapter +} diff --git a/pkg/cache/configserver/localread.go b/pkg/cache/configserver/localread.go new file mode 100644 index 00000000..7e90a01a --- /dev/null +++ b/pkg/cache/configserver/localread.go @@ -0,0 +1,94 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package configserver defines the seam between the config-server-backed +// cache.Client and the outside world it reads real Intents from. +// +// LocalConfigReader is exactly what that backend needs: get one Document by +// name, list every Document for a target. Nothing more. This decouples the +// backend (built behind this interface) from the real cross-repo transport +// (a unary gRPC service on config-server's colocated controller, defined in +// sdc-protos and wired up in later tickets) — the backend can be built and +// fully unit-tested against the fake in this package right now. +package configserver + +import ( + "context" + "errors" + + sdcpb "github.com/sdcio/sdc-protos/sdcpb" +) + +// ErrNotFound is returned by LocalConfigReader.Get when no Document exists +// for the given target/name pair. +var ErrNotFound = errors.New("configserver: document not found") + +// Target scopes a Get/List call to a single southbound target, mirroring the +// TargetNamespaceKey/TargetNameKey labels config-server's controller already +// indexes Config resources by (see the ADR's "Scope note"). +type Target struct { + Namespace string + Name string +} + +// ConfigBlob is a single raw path + JSON value entry from a Document's +// config payload — shaped so it parses cleanly into the same form +// importer/json.JsonTreeImporter already consumes. +type ConfigBlob struct { + Path string + // Value is the raw JSON encoding of the value at Path. + Value []byte +} + +// Document is the seam's representation of one config-server Config (joined +// with its SensitiveConfig, if any). It carries exactly the fields the ADR's +// field-mapping table needs to build an importer.ImportConfigAdapter: +// name, namespace, priority, non-revertive flag, orphan flag, sensitive +// paths, and the raw config payload. +type Document struct { + // Name is the Config resource's metadata.name — the lookup key + // ConfigReadService uses against TargetSnapshot.Spec.Configs. + Name string + // Namespace is the Config's Kubernetes namespace, populated from + // ConfigEntry.Namespace. Together with Name it forms the owner string + // config-server uses on TransactionSet (config.GetGVKNSN). + Namespace string + Priority int32 + NonRevertive bool + Orphan bool + SensitivePaths []*sdcpb.Path + Config []*ConfigBlob +} + +// IntentName is the owner name data-server and config-server share for a +// Config: ".", matching config.GetGVKNSN. When Namespace is +// empty the bare Name is returned, so incomplete fixtures stay usable. +func (d *Document) IntentName() string { + if d.Namespace == "" { + return d.Name + } + return d.Namespace + "." + d.Name +} + +// LocalConfigReader is the local-read seam a config-server-backed +// cache.Client depends on: get one Document by name, list every Document +// scoped to a target. +type LocalConfigReader interface { + // Get returns the Document named name for target. It returns ErrNotFound + // (wrapped or not) if no such Document exists. + Get(ctx context.Context, target Target, name string) (*Document, error) + // List returns every Document scoped to target. An empty/nil slice with a + // nil error means the target has no Documents. + List(ctx context.Context, target Target) ([]*Document, error) +} diff --git a/pkg/cache/configserver/localread_test.go b/pkg/cache/configserver/localread_test.go new file mode 100644 index 00000000..3b379460 --- /dev/null +++ b/pkg/cache/configserver/localread_test.go @@ -0,0 +1,40 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configserver + +import "testing" + +func TestDocument_IntentName(t *testing.T) { + cases := map[string]struct { + doc Document + want string + }{ + "gvk nsn": { + doc: Document{Namespace: "default", Name: "srl2-config"}, + want: "default.srl2-config", + }, + "bare name when namespace missing": { + doc: Document{Name: "srl2-config"}, + want: "srl2-config", + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + if got := tc.doc.IntentName(); got != tc.want { + t.Errorf("IntentName() = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/pkg/cache/configserver/merge.go b/pkg/cache/configserver/merge.go new file mode 100644 index 00000000..59146dc1 --- /dev/null +++ b/pkg/cache/configserver/merge.go @@ -0,0 +1,137 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configserver + +import ( + "encoding/json" + "fmt" + + sdcpb "github.com/sdcio/sdc-protos/sdcpb" +) + +// mergeConfigBlobs assembles a Document's flat path+value entries into a +// single nested, JSON_IETF-shaped map — the same shape +// importer/json.JsonTreeImporter already knows how to traverse, per the +// ADR's field-mapping table ("Value payload" row). List path elements (those +// carrying keys) become list entries (map[string]any, pre-seeded with their +// key leaves) inside a []any keyed by their element name, found-or-created +// by matching keys so repeated blobs under the same list entry converge on +// one map. +func mergeConfigBlobs(blobs []*ConfigBlob) (map[string]any, error) { + root := map[string]any{} + for _, b := range blobs { + p, err := sdcpb.ParsePath(b.Path) + if err != nil { + return nil, fmt.Errorf("parsing path %q: %w", b.Path, err) + } + + var val any + if len(b.Value) > 0 { + if err := json.Unmarshal(b.Value, &val); err != nil { + return nil, fmt.Errorf("unmarshalling value at %q: %w", b.Path, err) + } + } + + elems := p.GetElem() + if len(elems) == 0 { + if err := mergeRootBlob(root, val); err != nil { + return nil, fmt.Errorf("inserting %q: %w", b.Path, err) + } + continue + } + + if err := insertAtPath(root, elems, val); err != nil { + return nil, fmt.Errorf("inserting %q: %w", b.Path, err) + } + } + return root, nil +} + +// mergeRootBlob shallow-merges a config-server root blob (path "/") into root. +// An empty or absent value contributes nothing; the value must be a JSON object +// whose top-level keys are YANG children under the model root. +func mergeRootBlob(root map[string]any, val any) error { + if val == nil { + return nil + } + child, ok := val.(map[string]any) + if !ok { + return fmt.Errorf("root blob value must be a JSON object") + } + for k, v := range child { + root[k] = v + } + return nil +} + +// insertAtPath walks elems into node, creating intermediate containers/list +// entries as needed, and sets val at the final element. +func insertAtPath(node map[string]any, elems []*sdcpb.PathElem, val any) error { + if len(elems) == 0 { + return fmt.Errorf("empty path") + } + elem := elems[0] + rest := elems[1:] + + if len(elem.GetKey()) == 0 { + if len(rest) == 0 { + node[elem.Name] = val + return nil + } + child, ok := node[elem.Name].(map[string]any) + if !ok { + child = map[string]any{} + node[elem.Name] = child + } + return insertAtPath(child, rest, val) + } + + entry := findOrCreateListEntry(node, elem) + if len(rest) == 0 { + // A keyed element with nothing after it has only its own key leaves + // to contribute — already seeded by findOrCreateListEntry. + return nil + } + return insertAtPath(entry, rest, val) +} + +// findOrCreateListEntry returns the list entry under node[elem.Name] whose +// key leaves match elem's keys, creating (and appending) one pre-seeded with +// those key leaves if none matches yet. +func findOrCreateListEntry(node map[string]any, elem *sdcpb.PathElem) map[string]any { + list, _ := node[elem.Name].([]any) + for _, e := range list { + if entry, ok := e.(map[string]any); ok && listEntryMatchesKeys(entry, elem.GetKey()) { + return entry + } + } + + entry := map[string]any{} + for k, v := range elem.GetKey() { + entry[k] = v + } + node[elem.Name] = append(list, entry) + return entry +} + +func listEntryMatchesKeys(entry map[string]any, keys map[string]string) bool { + for k, v := range keys { + ev, ok := entry[k] + if !ok || fmt.Sprintf("%v", ev) != v { + return false + } + } + return true +} diff --git a/pkg/cache/configserver/merge_test.go b/pkg/cache/configserver/merge_test.go new file mode 100644 index 00000000..be0dea2f --- /dev/null +++ b/pkg/cache/configserver/merge_test.go @@ -0,0 +1,249 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configserver + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestMergeConfigBlobs_SimpleLeaf(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/description", Value: []byte(`"top level"`)}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + want := map[string]any{"description": "top level"} + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("mergeConfigBlobs() mismatch (-want +got):\n%s", diff) + } +} + +func TestMergeConfigBlobs_NestedContainer(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/system/config/hostname", Value: []byte(`"router1"`)}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + want := map[string]any{ + "system": map[string]any{ + "config": map[string]any{ + "hostname": "router1", + }, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("mergeConfigBlobs() mismatch (-want +got):\n%s", diff) + } +} + +func TestMergeConfigBlobs_ListEntry_KeySeeded(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/interface[name=eth0]/config/mtu", Value: []byte(`9000`)}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + want := map[string]any{ + "interface": []any{ + map[string]any{ + "name": "eth0", + "config": map[string]any{ + "mtu": float64(9000), + }, + }, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("mergeConfigBlobs() mismatch (-want +got):\n%s", diff) + } +} + +func TestMergeConfigBlobs_MultipleBlobsConvergeOnSameListEntry(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/interface[name=eth0]/config/mtu", Value: []byte(`9000`)}, + {Path: "/interface[name=eth0]/config/description", Value: []byte(`"uplink"`)}, + {Path: "/interface[name=eth1]/config/mtu", Value: []byte(`1500`)}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + want := map[string]any{ + "interface": []any{ + map[string]any{ + "name": "eth0", + "config": map[string]any{ + "mtu": float64(9000), + "description": "uplink", + }, + }, + map[string]any{ + "name": "eth1", + "config": map[string]any{ + "mtu": float64(1500), + }, + }, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("mergeConfigBlobs() mismatch (-want +got):\n%s", diff) + } +} + +func TestMergeConfigBlobs_MultiKeyList(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/neighbor[local-as=65001][peer-as=65002]/description", Value: []byte(`"peer"`)}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + want := map[string]any{ + "neighbor": []any{ + map[string]any{ + "local-as": "65001", + "peer-as": "65002", + "description": "peer", + }, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("mergeConfigBlobs() mismatch (-want +got):\n%s", diff) + } +} + +func TestMergeConfigBlobs_NoValue(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/enabled"}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + if got["enabled"] != nil { + t.Errorf("got[\"enabled\"] = %v, want nil", got["enabled"]) + } +} + +func TestMergeConfigBlobs_Empty(t *testing.T) { + got, err := mergeConfigBlobs(nil) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + if len(got) != 0 { + t.Errorf("mergeConfigBlobs(nil) = %v, want empty", got) + } +} + +func TestMergeConfigBlobs_InvalidPath(t *testing.T) { + _, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "[invalid", Value: []byte(`1`)}, + }) + if err == nil { + t.Fatal("mergeConfigBlobs() expected error for invalid path, got nil") + } +} + +func TestMergeConfigBlobs_InvalidJSON(t *testing.T) { + _, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/mtu", Value: []byte(`not-json`)}, + }) + if err == nil { + t.Fatal("mergeConfigBlobs() expected error for invalid JSON, got nil") + } +} + +func TestMergeConfigBlobs_RootBlob(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/", Value: []byte(`{"interface":[{"name":"ethernet-1/1","admin-state":"enable"}],"network-instance":[{"name":"vrf1","type":"ip-vrf"}]}`)}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + want := map[string]any{ + "interface": []any{ + map[string]any{ + "name": "ethernet-1/1", + "admin-state": "enable", + }, + }, + "network-instance": []any{ + map[string]any{ + "name": "vrf1", + "type": "ip-vrf", + }, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("mergeConfigBlobs() mismatch (-want +got):\n%s", diff) + } +} + +func TestMergeConfigBlobs_RootBlobEmpty(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/"}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + if len(got) != 0 { + t.Errorf("mergeConfigBlobs() = %v, want empty", got) + } +} + +func TestMergeConfigBlobs_RootBlobEmptyObject(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/", Value: []byte(`{}`)}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + if len(got) != 0 { + t.Errorf("mergeConfigBlobs() = %v, want empty", got) + } +} + +func TestMergeConfigBlobs_RootBlobScalarRejected(t *testing.T) { + _, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/", Value: []byte(`"hello"`)}, + }) + if err == nil { + t.Fatal("mergeConfigBlobs() expected error for scalar root blob, got nil") + } +} + +func TestMergeConfigBlobs_RootBlobThenGranular(t *testing.T) { + got, err := mergeConfigBlobs([]*ConfigBlob{ + {Path: "/", Value: []byte(`{"interface":[{"name":"eth0","config":{"mtu":1500}}]}`)}, + {Path: "/interface[name=eth0]/config/mtu", Value: []byte(`9000`)}, + }) + if err != nil { + t.Fatalf("mergeConfigBlobs() error = %v", err) + } + want := map[string]any{ + "interface": []any{ + map[string]any{ + "name": "eth0", + "config": map[string]any{ + "mtu": float64(9000), + }, + }, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("mergeConfigBlobs() mismatch (-want +got):\n%s", diff) + } +} diff --git a/pkg/cache/configserver_split.go b/pkg/cache/configserver_split.go new file mode 100644 index 00000000..07236999 --- /dev/null +++ b/pkg/cache/configserver_split.go @@ -0,0 +1,42 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cache + +import ( + "errors" + "strings" + + "github.com/sdcio/data-server/pkg/cache/configserver" +) + +// ErrMalformedDatastoreName is returned by splitDatastoreName when a +// datastore name doesn't decode into a non-empty namespace and a non-empty +// name. +var ErrMalformedDatastoreName = errors.New("configserver cache: malformed datastore name") + +// splitDatastoreName decodes a datastore name (e.g. "prod.srl1") into a +// configserver.Target, mirroring config-server's own encoding +// (storebackend.Key.String(), "."). It splits +// on the first '.' only, since a Kubernetes namespace is always a DNS-1123 +// label (no dots) while a target's bare name may itself legally contain +// dots. It returns ErrMalformedDatastoreName whenever there's no dot at all, +// or when either resulting segment is empty. +func splitDatastoreName(cacheInstanceName string) (configserver.Target, error) { + namespace, name, found := strings.Cut(cacheInstanceName, ".") + if !found || namespace == "" || name == "" { + return configserver.Target{}, ErrMalformedDatastoreName + } + return configserver.Target{Namespace: namespace, Name: name}, nil +} diff --git a/pkg/cache/configserver_split_test.go b/pkg/cache/configserver_split_test.go new file mode 100644 index 00000000..394f038f --- /dev/null +++ b/pkg/cache/configserver_split_test.go @@ -0,0 +1,75 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cache + +import ( + "errors" + "testing" + + "github.com/sdcio/data-server/pkg/cache/configserver" +) + +func TestSplitDatastoreName(t *testing.T) { + tests := []struct { + name string + in string + want configserver.Target + wantErr bool + }{ + { + name: "dotted", + in: "prod.srl1", + want: configserver.Target{Namespace: "prod", Name: "srl1"}, + }, + { + name: "dotted with extra dots in name", + in: "prod.rack1.srl1", + want: configserver.Target{Namespace: "prod", Name: "rack1.srl1"}, + }, + { + name: "no dot", + in: "srl1", + wantErr: true, + }, + { + name: "empty namespace", + in: ".srl1", + wantErr: true, + }, + { + name: "empty name", + in: "prod.", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := splitDatastoreName(tt.in) + if tt.wantErr { + if !errors.Is(err, ErrMalformedDatastoreName) { + t.Fatalf("splitDatastoreName(%q) error = %v, want ErrMalformedDatastoreName", tt.in, err) + } + return + } + if err != nil { + t.Fatalf("splitDatastoreName(%q) unexpected error = %v", tt.in, err) + } + if got != tt.want { + t.Errorf("splitDatastoreName(%q) = %+v, want %+v", tt.in, got, tt.want) + } + }) + } +} diff --git a/pkg/cache/configserver_test.go b/pkg/cache/configserver_test.go new file mode 100644 index 00000000..0fdadd02 --- /dev/null +++ b/pkg/cache/configserver_test.go @@ -0,0 +1,429 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cache + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/sdcio/data-server/pkg/cache/configserver" + "github.com/sdcio/data-server/pkg/tree/importer" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" + "github.com/sdcio/sdc-protos/tree_persist" +) + +const ( + testNamespace = "ns1" + testTarget = "target1" + testCacheName = testNamespace + "." + testTarget +) + +func newTestConfigServerCache(t *testing.T) (*ConfigServerCache, *configserver.FakeLocalConfigReader) { + t.Helper() + reader := configserver.NewFakeLocalConfigReader() + return NewConfigServerCache(reader), reader +} + +// TestConfigServerCache_InstanceIntentGet_FieldMapping verifies +// InstanceIntentGet calls the seam's Get and wraps the result per the ADR's +// field-mapping table, covering it end to end against the fake. +func TestConfigServerCache_InstanceIntentGet_FieldMapping(t *testing.T) { + ctx := context.Background() + c, reader := newTestConfigServerCache(t) + + sensitivePaths := []*sdcpb.Path{{Elem: []*sdcpb.PathElem{{Name: "secret"}}}} + reader.Seed(configserver.Target{Namespace: testNamespace, Name: testTarget}, &configserver.Document{ + Name: "intent1", + Namespace: testNamespace, + Priority: 10, + NonRevertive: true, + Orphan: true, + SensitivePaths: sensitivePaths, + Config: []*configserver.ConfigBlob{ + {Path: "/interface[name=eth0]/config/mtu", Value: []byte(`9000`)}, + }, + }) + + adapter, err := c.InstanceIntentGet(ctx, testCacheName, "intent1") + if err != nil { + t.Fatalf("InstanceIntentGet() error = %v", err) + } + if got := adapter.GetName(); got != testNamespace+".intent1" { + t.Errorf("GetName() = %q, want %q", got, testNamespace+".intent1") + } + if got := adapter.GetPriority(); got != 10 { + t.Errorf("GetPriority() = %d, want 10", got) + } + if !adapter.GetNonRevertive() { + t.Error("GetNonRevertive() = false, want true") + } + if !adapter.GetOrphan() { + t.Error("GetOrphan() = false, want true") + } +} + +func TestConfigServerCache_InstanceIntentGet_NotFound(t *testing.T) { + ctx := context.Background() + c, _ := newTestConfigServerCache(t) + + _, err := c.InstanceIntentGet(ctx, testCacheName, "missing") + if !errors.Is(err, configserver.ErrNotFound) { + t.Fatalf("InstanceIntentGet() error = %v, want ErrNotFound", err) + } +} + +func TestConfigServerCache_InstanceIntentGet_GVKNSNName(t *testing.T) { + ctx := context.Background() + c, reader := newTestConfigServerCache(t) + reader.Seed(configserver.Target{Namespace: testNamespace, Name: testTarget}, &configserver.Document{ + Name: "intent1", + Namespace: testNamespace, + }) + + adapter, err := c.InstanceIntentGet(ctx, testCacheName, testNamespace+".intent1") + if err != nil { + t.Fatalf("InstanceIntentGet() error = %v", err) + } + if got := adapter.GetName(); got != testNamespace+".intent1" { + t.Errorf("GetName() = %q, want %q", got, testNamespace+".intent1") + } +} + +func TestConfigServerCache_InstanceIntentsList(t *testing.T) { + ctx := context.Background() + c, reader := newTestConfigServerCache(t) + target := configserver.Target{Namespace: testNamespace, Name: testTarget} + reader.Seed(target, + &configserver.Document{Name: "intent2", Namespace: testNamespace}, + &configserver.Document{Name: "intent1", Namespace: testNamespace}, + ) + + got, err := c.InstanceIntentsList(ctx, testCacheName) + if err != nil { + t.Fatalf("InstanceIntentsList() error = %v", err) + } + if diff := cmp.Diff([]string{testNamespace + ".intent1", testNamespace + ".intent2"}, got); diff != "" { + t.Errorf("InstanceIntentsList() mismatch (-want +got):\n%s", diff) + } +} + +func TestConfigServerCache_InstanceIntentGetAll(t *testing.T) { + ctx := context.Background() + c, reader := newTestConfigServerCache(t) + target := configserver.Target{Namespace: testNamespace, Name: testTarget} + reader.Seed(target, + &configserver.Document{Name: "intent1", Namespace: testNamespace, Priority: 1}, + &configserver.Document{Name: "intent2", Namespace: testNamespace, Priority: 2}, + ) + + intentChan := make(chan importer.IntentAdapter) + errChan := make(chan error, 1) + go c.InstanceIntentGetAll(ctx, testCacheName, nil, intentChan, errChan) + + var names []string + for adapter := range intentChan { + names = append(names, adapter.GetName()) + } + if err := <-errChan; err != nil { + t.Fatalf("InstanceIntentGetAll() error = %v", err) + } + if diff := cmp.Diff([]string{testNamespace + ".intent1", testNamespace + ".intent2"}, names); diff != "" { + t.Errorf("InstanceIntentGetAll() names mismatch (-want +got):\n%s", diff) + } +} + +func TestConfigServerCache_InstanceIntentGetAll_NoDocuments(t *testing.T) { + ctx := context.Background() + c, _ := newTestConfigServerCache(t) + + intentChan := make(chan importer.IntentAdapter) + errChan := make(chan error, 1) + go c.InstanceIntentGetAll(ctx, testCacheName, nil, intentChan, errChan) + + var names []string + for adapter := range intentChan { + names = append(names, adapter.GetName()) + } + if err := <-errChan; err != nil { + t.Fatalf("InstanceIntentGetAll() error = %v", err) + } + if len(names) != 0 { + t.Errorf("InstanceIntentGetAll() names = %v, want empty", names) + } +} + +// TestConfigServerCache_InstanceIntentGetAll_ContextCancelled verifies +// InstanceIntentGetAll stops sending and returns once ctx is cancelled, +// per the ADR ("closing intentChan/errChan when done or on ctx.Done()"), +// rather than blocking forever on an unbuffered, unread channel. +func TestConfigServerCache_InstanceIntentGetAll_ContextCancelled(t *testing.T) { + c, reader := newTestConfigServerCache(t) + target := configserver.Target{Namespace: testNamespace, Name: testTarget} + reader.Seed(target, + &configserver.Document{Name: "intent1"}, + &configserver.Document{Name: "intent2"}, + ) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + intentChan := make(chan importer.IntentAdapter) + errChan := make(chan error, 1) + done := make(chan struct{}) + go func() { + c.InstanceIntentGetAll(ctx, testCacheName, nil, intentChan, errChan) + close(done) + }() + + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("InstanceIntentGetAll did not return after ctx cancellation") + } + + if _, open := <-intentChan; open { + t.Error("intentChan should be closed after ctx cancellation") + } + if _, open := <-errChan; open { + t.Error("errChan should be closed after ctx cancellation") + } +} + +func TestConfigServerCache_InstanceIntentExists(t *testing.T) { + ctx := context.Background() + c, reader := newTestConfigServerCache(t) + target := configserver.Target{Namespace: testNamespace, Name: testTarget} + reader.Seed(target, &configserver.Document{Name: "intent1"}) + + exists, err := c.InstanceIntentExists(ctx, testCacheName, "intent1") + if err != nil { + t.Fatalf("InstanceIntentExists() error = %v", err) + } + if !exists { + t.Error("InstanceIntentExists() = false, want true") + } + + exists, err = c.InstanceIntentExists(ctx, testCacheName, "missing") + if err != nil { + t.Fatalf("InstanceIntentExists() error = %v, want nil (not-found maps to false, nil)", err) + } + if exists { + t.Error("InstanceIntentExists() = true, want false") + } +} + +// TestConfigServerCache_WritesAreNoOps verifies the full Client built by +// NewConfigServerClient never errors, never panics, and never reaches the +// seam on IntentWriter calls (the fake has no Modify/Delete methods at +// all — only Get/List — so any attempt to use it that way wouldn't compile +// in the first place). Real-Intent writes are composed in via +// noopIntentWriter, not implemented directly on ConfigServerCache. +func TestConfigServerCache_WritesAreNoOps(t *testing.T) { + ctx := context.Background() + reader := configserver.NewFakeLocalConfigReader() + c := NewConfigServerClient(reader) + + if err := c.InstanceIntentModify(ctx, testCacheName, &tree_persist.Intent{IntentName: "intent1"}); err != nil { + t.Errorf("InstanceIntentModify() error = %v, want nil", err) + } + if err := c.InstanceIntentDelete(ctx, testCacheName, "intent1", false); err != nil { + t.Errorf("InstanceIntentDelete() error = %v, want nil", err) + } + + // A write must not make the intent appear/disappear from the seam's + // perspective — this backend never touches it either way. + if _, err := c.InstanceIntentGet(ctx, testCacheName, "intent1"); !errors.Is(err, configserver.ErrNotFound) { + t.Errorf("InstanceIntentGet() error = %v, want ErrNotFound (write must be a no-op)", err) + } +} + +// TestConfigServerCache_MalformedDatastoreName_ReadCallers verifies +// InstanceIntentsList/InstanceIntentGet/InstanceIntentExists all surface +// ErrMalformedDatastoreName as a normal returned error when the +// cacheInstanceName they're called with doesn't decode into a +// namespace/name pair, rather than building a lookup with an empty +// namespace or sending the whole compound name as the bare target name. +func TestConfigServerCache_MalformedDatastoreName_ReadCallers(t *testing.T) { + ctx := context.Background() + c, _ := newTestConfigServerCache(t) + const malformed = "no-dot-here" + + if _, err := c.InstanceIntentsList(ctx, malformed); !errors.Is(err, ErrMalformedDatastoreName) { + t.Errorf("InstanceIntentsList() error = %v, want ErrMalformedDatastoreName", err) + } + if _, err := c.InstanceIntentGet(ctx, malformed, "intent1"); !errors.Is(err, ErrMalformedDatastoreName) { + t.Errorf("InstanceIntentGet() error = %v, want ErrMalformedDatastoreName", err) + } + if _, err := c.InstanceIntentExists(ctx, malformed, "intent1"); !errors.Is(err, ErrMalformedDatastoreName) { + t.Errorf("InstanceIntentExists() error = %v, want ErrMalformedDatastoreName", err) + } +} + +// TestConfigServerCache_InstanceIntentGetAll_MalformedDatastoreName verifies +// InstanceIntentGetAll sends ErrMalformedDatastoreName on its existing +// errChan, the same channel real reader.List errors already use, and closes +// both channels. +func TestConfigServerCache_InstanceIntentGetAll_MalformedDatastoreName(t *testing.T) { + c, _ := newTestConfigServerCache(t) + const malformed = "no-dot-here" + + intentChan := make(chan importer.IntentAdapter) + errChan := make(chan error, 1) + go c.InstanceIntentGetAll(context.Background(), malformed, nil, intentChan, errChan) + + if _, open := <-intentChan; open { + t.Error("intentChan should be closed without ever sending on a malformed name") + } + if err := <-errChan; !errors.Is(err, ErrMalformedDatastoreName) { + t.Errorf("errChan error = %v, want ErrMalformedDatastoreName", err) + } +} + +// TestConfigServerCache_InstanceCreate_MalformedDatastoreName verifies +// InstanceCreate rejects a malformed datastore name at creation time, +// before it ever mutates the running map — the instance must not be left +// half-created. +func TestConfigServerCache_InstanceCreate_MalformedDatastoreName(t *testing.T) { + ctx := context.Background() + c, _ := newTestConfigServerCache(t) + const malformed = "no-dot-here" + + if err := c.InstanceCreate(ctx, malformed); !errors.Is(err, ErrMalformedDatastoreName) { + t.Fatalf("InstanceCreate() error = %v, want ErrMalformedDatastoreName", err) + } + if c.InstanceExists(ctx, malformed) { + t.Error("InstanceExists() = true after InstanceCreate rejected a malformed name") + } +} + +func TestConfigServerCache_InstanceLifecycle(t *testing.T) { + ctx := context.Background() + c, _ := newTestConfigServerCache(t) + + if c.InstanceExists(ctx, testCacheName) { + t.Error("InstanceExists() = true before InstanceCreate") + } + if err := c.InstanceCreate(ctx, testCacheName); err != nil { + t.Fatalf("InstanceCreate() error = %v", err) + } + if !c.InstanceExists(ctx, testCacheName) { + t.Error("InstanceExists() = false after InstanceCreate") + } + if err := c.InstanceCreate(ctx, testCacheName); err == nil { + t.Error("InstanceCreate() second call: want error, got nil") + } + if diff := cmp.Diff([]string{testCacheName}, c.InstancesList(ctx)); diff != "" { + t.Errorf("InstancesList() mismatch (-want +got):\n%s", diff) + } + if err := c.InstanceDelete(ctx, testCacheName); err != nil { + t.Fatalf("InstanceDelete() error = %v", err) + } + if c.InstanceExists(ctx, testCacheName) { + t.Error("InstanceExists() = true after InstanceDelete") + } + if err := c.InstanceDelete(ctx, testCacheName); err == nil { + t.Error("InstanceDelete() on missing instance: want error, got nil") + } +} + +// TestConfigServerCache_RunningIndependentOfSeam verifies InstanceRunningGet +// / InstanceRunningModify work purely off the in-memory store, entirely +// independent of the LocalConfigReader seam (which never sees "running" at +// all under this backend). +func TestConfigServerCache_RunningIndependentOfSeam(t *testing.T) { + ctx := context.Background() + c, reader := newTestConfigServerCache(t) + + if err := c.InstanceCreate(ctx, testCacheName); err != nil { + t.Fatalf("InstanceCreate() error = %v", err) + } + + want := &tree_persist.Intent{IntentName: "running", Priority: 1} + if err := c.InstanceRunningModify(ctx, testCacheName, want); err != nil { + t.Fatalf("InstanceRunningModify() error = %v", err) + } + + got, err := c.InstanceRunningGet(ctx, testCacheName) + if err != nil { + t.Fatalf("InstanceRunningGet() error = %v", err) + } + if gotName := got.GetName(); gotName != want.GetIntentName() { + t.Errorf("InstanceRunningGet().GetName() = %q, want %q", gotName, want.GetIntentName()) + } + if gotPriority := got.GetPriority(); gotPriority != want.GetPriority() { + t.Errorf("InstanceRunningGet().GetPriority() = %d, want %d", gotPriority, want.GetPriority()) + } + + // The seam was never seeded with anything and never asked for + // "running" — List/Get must still be untouched (fake has no docs at + // all for this target). + docs, err := reader.List(ctx, configserver.Target{Namespace: testNamespace, Name: testTarget}) + if err != nil { + t.Fatalf("reader.List() error = %v", err) + } + if len(docs) != 0 { + t.Errorf("reader.List() = %v, want empty (running never touches the seam)", docs) + } +} + +// TestConfigServerCache_InstanceRunningGet_NotYetModified verifies +// InstanceRunningGet on a freshly created instance returns an empty +// (zero-value) adapter rather than an error — InstanceCreate seeds +// "running" with an empty Intent, same as a freshly created LocalCache +// instance's running store starts out with no data written to it yet. +func TestConfigServerCache_InstanceRunningGet_NotYetModified(t *testing.T) { + ctx := context.Background() + c, _ := newTestConfigServerCache(t) + + if err := c.InstanceCreate(ctx, testCacheName); err != nil { + t.Fatalf("InstanceCreate() error = %v", err) + } + + got, err := c.InstanceRunningGet(ctx, testCacheName) + if err != nil { + t.Fatalf("InstanceRunningGet() error = %v", err) + } + if gotName := got.GetName(); gotName != "" { + t.Errorf("InstanceRunningGet().GetName() = %q, want empty", gotName) + } +} + +func TestConfigServerCache_InstanceRunningGet_UnknownInstance(t *testing.T) { + ctx := context.Background() + c, _ := newTestConfigServerCache(t) + + _, err := c.InstanceRunningGet(ctx, "unknown") + if err == nil { + t.Fatal("InstanceRunningGet() on unknown instance: want error, got nil") + } +} + +// TestConfigServerCache_ImplementsReaderCapabilities verifies +// *ConfigServerCache satisfies IntentReader, RunningStore, and +// InstanceLifecycle directly. It deliberately does not assert IntentWriter +// here — there is no method left on the type to test for no-op behavior; +// that behavior lives in noopIntentWriter, composed in at +// Server.createCacheClient instead. +func TestConfigServerCache_ImplementsReaderCapabilities(t *testing.T) { + c := NewConfigServerCache(configserver.NewFakeLocalConfigReader()) + var ( + _ IntentReader = c + _ RunningStore = c + _ InstanceLifecycle = c + ) +} diff --git a/pkg/config/datastore.go b/pkg/config/datastore.go index 22dda61b..72b67e35 100644 --- a/pkg/config/datastore.go +++ b/pkg/config/datastore.go @@ -156,7 +156,10 @@ type CacheConfig struct { // Local cache attr StoreType string `yaml:"store-type,omitempty" json:"store-type,omitempty"` Dir string `yaml:"dir,omitempty" json:"dir,omitempty"` - // Remote cache attr + // Remote cache attr; also doubles as the config-server cache attr: + // address of the colocated config-server controller's local + // ConfigReadService (see the ADR's "New cache.Client implementation" + // section). Address string `yaml:"address,omitempty" json:"address,omitempty"` } @@ -279,7 +282,12 @@ func (c *CacheConfig) validateSetDefaults() error { c.Dir = defaultCacheDir } case cacheTypeConfigServer: - // Known type; CacheConfig has no defaults for it. + if c.Address == "" { + return errors.New("missing config-server cache address") + } + if _, _, err := net.SplitHostPort(c.Address); err != nil { + return err + } default: return fmt.Errorf("unknown cache type: %q", c.Type) } diff --git a/pkg/config/datastore_test.go b/pkg/config/datastore_test.go index baf01c1c..7f0d97ac 100644 --- a/pkg/config/datastore_test.go +++ b/pkg/config/datastore_test.go @@ -138,14 +138,27 @@ func TestCacheConfigValidateSetDefaults(t *testing.T) { wantErr: true, }, { - name: "config-server type survives unchanged", - in: &CacheConfig{Type: "config-server"}, + name: "config-server type requires an address", + in: &CacheConfig{Type: "config-server"}, + wantErr: true, + }, + { + name: "config-server type rejects an invalid address", + in: &CacheConfig{Type: "config-server", Address: "not-a-valid-address"}, + wantErr: true, + }, + { + name: "config-server type keeps explicit connection settings", + in: &CacheConfig{Type: "config-server", Address: "localhost:50051"}, check: func(t *testing.T, c *CacheConfig) { if c.Type != "config-server" { t.Errorf("Type = %q, want %q", c.Type, "config-server") } - if c.StoreType != "" || c.Dir != "" || c.Address != "" { - t.Errorf("expected no defaults to be applied for config-server type, got %+v", c) + if c.Address != "localhost:50051" { + t.Errorf("Address = %q, want %q", c.Address, "localhost:50051") + } + if c.StoreType != "" || c.Dir != "" { + t.Errorf("expected no local-cache defaults to be applied for config-server type, got %+v", c) } }, }, diff --git a/pkg/server/cache.go b/pkg/server/cache.go index 538e7e16..91a89e0b 100644 --- a/pkg/server/cache.go +++ b/pkg/server/cache.go @@ -22,6 +22,7 @@ import ( cconfig "github.com/sdcio/cache/pkg/config" "github.com/sdcio/data-server/pkg/cache" + "github.com/sdcio/data-server/pkg/cache/configserver" logf "github.com/sdcio/logger" ) @@ -42,6 +43,13 @@ START: time.Sleep(time.Second) goto START } + case "config-server": + err = s.createConfigServerCacheClient(ctx) + if err != nil { + log.Error(err, "failed to initialize a config-server cache client") + time.Sleep(time.Second) + goto START + } } } @@ -56,3 +64,21 @@ func (s *Server) createLocalCacheClient(ctx context.Context) error { log.Info("local cache created") return err } + +// createConfigServerCacheClient wires the colocated config-server +// controller's local ConfigReadService (ticket 07) in as the real-intent +// backend, per the ADR's "New cache.Client implementation" section: +// real Intents are read through configserver.GRPCConfigReader, "running" +// stays independent in-memory state owned by cache.ConfigServerCache itself. +func (s *Server) createConfigServerCacheClient(ctx context.Context) error { + log := logf.FromContext(ctx) + log.Info("initializing config-server cache client", "address", s.config.Cache.Address) + cc, err := configserver.Dial(s.config.Cache.Address) + if err != nil { + return err + } + reader := configserver.NewGRPCConfigReader(cc) + s.cacheClient = cache.NewConfigServerClient(reader) + log.Info("config-server cache client created") + return nil +} diff --git a/pkg/server/cache_test.go b/pkg/server/cache_test.go new file mode 100644 index 00000000..e5365ffc --- /dev/null +++ b/pkg/server/cache_test.go @@ -0,0 +1,73 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "context" + "testing" + + "github.com/sdcio/data-server/pkg/cache" + "github.com/sdcio/data-server/pkg/config" +) + +func TestCreateConfigServerCacheClient(t *testing.T) { + s := &Server{ + config: &config.Config{ + Cache: &config.CacheConfig{ + Type: "config-server", + Address: "localhost:50051", + }, + }, + } + + err := s.createConfigServerCacheClient(context.Background()) + if err != nil { + t.Fatalf("createConfigServerCacheClient() error = %v", err) + } + + if s.cacheClient == nil { + t.Fatal("cacheClient = nil, want a config-server-backed cache.Client") + } + if _, ok := s.cacheClient.(cache.IntentReader); !ok { + t.Fatalf("cacheClient = %T, does not implement cache.IntentReader", s.cacheClient) + } +} + +// TestCreateConfigServerCacheClient_WritesAreNoOps verifies the config-server +// case composes a Client whose IntentModify/IntentDelete behavior is the +// generic no-op (noopIntentWriter) — reachable without ever going through +// the read seam (configserver.LocalConfigReader), since a real backend would +// need network/reader wiring to answer at all. +func TestCreateConfigServerCacheClient_WritesAreNoOps(t *testing.T) { + s := &Server{ + config: &config.Config{ + Cache: &config.CacheConfig{ + Type: "config-server", + Address: "localhost:50051", + }, + }, + } + + if err := s.createConfigServerCacheClient(context.Background()); err != nil { + t.Fatalf("createConfigServerCacheClient() error = %v", err) + } + + if err := s.cacheClient.InstanceIntentModify(context.Background(), "ns1.target1", nil); err != nil { + t.Errorf("InstanceIntentModify() error = %v, want nil", err) + } + if err := s.cacheClient.InstanceIntentDelete(context.Background(), "ns1.target1", "intent1", false); err != nil { + t.Errorf("InstanceIntentDelete() error = %v, want nil", err) + } +}