diff --git a/go.mod b/go.mod index 82891a09..3b4db526 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,14 @@ go 1.25.0 replace github.com/openconfig/goyang v1.6.0 => github.com/sdcio/goyang v1.6.2-2 +// Pinned to open PRs; replace with tagged releases once merged: +// - sdcio/goyang PR#4: fix(ApplyDeviate): replace Exts on deviate replace, not append +// - sdcio/sdc-protos PR#123: feat(sensitive): schema.Path + LeafSchema.sensitive flag +replace ( + github.com/sdcio/goyang v1.6.2-2 => github.com/sdcio/goyang v1.6.2-2.0.20260608121857-4668a077cf72 + github.com/sdcio/sdc-protos v0.0.54 => github.com/sdcio/sdc-protos v0.0.55-0.20260610090020-aeb8edf494c4 +) + require ( github.com/AlekSi/pointer v1.2.0 github.com/beevik/etree v1.6.0 diff --git a/go.sum b/go.sum index 4f8192a6..ad2c13a0 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.54 h1:1EbtU9ZbbJHFPOFGi5aW8Th79cuY9i+AJaP0ABVx8hw= -github.com/sdcio/sdc-protos v0.0.54/go.mod h1:YMLHbey0/aL1qtLW8csSYVPafsgnnn7aY54HkV5dbyQ= +github.com/sdcio/sdc-protos v0.0.55-0.20260610090020-aeb8edf494c4 h1:M1C1wzt2ni0fssvPEaMNPdHNKqTzBtT8VTT3/EzbrCE= +github.com/sdcio/sdc-protos v0.0.55-0.20260610090020-aeb8edf494c4/go.mod h1:NsvzvHnTonLcwQ/WNzxzBCauQmqxpuviaW0wh7Lkrts= 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/mocktreeentry/entry.go b/mocks/mocktreeentry/entry.go index c3637e2b..25ef91db 100644 --- a/mocks/mocktreeentry/entry.go +++ b/mocks/mocktreeentry/entry.go @@ -43,48 +43,19 @@ func (m *MockEntry) EXPECT() *MockEntryMockRecorder { return m.recorder } -// AddChild mocks base method. -func (m *MockEntry) AddChild(arg0 context.Context, arg1 api.Entry) error { +// AddOrGetChild mocks base method. +func (m *MockEntry) AddOrGetChild(arg0 context.Context, arg1 api.Entry) (api.Entry, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AddChild", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 -} - -// AddChild indicates an expected call of AddChild. -func (mr *MockEntryMockRecorder) AddChild(arg0, arg1 any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddChild", reflect.TypeOf((*MockEntry)(nil).AddChild), arg0, arg1) -} - -// AddUpdateRecursive mocks base method. -func (m *MockEntry) AddUpdateRecursive(ctx context.Context, relativePath *sdcpb.Path, u *types.Update, flags *types.UpdateInsertFlags) (api.Entry, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AddUpdateRecursive", ctx, relativePath, u, flags) - ret0, _ := ret[0].(api.Entry) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// AddUpdateRecursive indicates an expected call of AddUpdateRecursive. -func (mr *MockEntryMockRecorder) AddUpdateRecursive(ctx, relativePath, u, flags any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddUpdateRecursive", reflect.TypeOf((*MockEntry)(nil).AddUpdateRecursive), ctx, relativePath, u, flags) -} - -// AddUpdateRecursiveInternal mocks base method. -func (m *MockEntry) AddUpdateRecursiveInternal(ctx context.Context, path *sdcpb.Path, idx int, u *types.Update, flags *types.UpdateInsertFlags) (api.Entry, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AddUpdateRecursiveInternal", ctx, path, idx, u, flags) + ret := m.ctrl.Call(m, "AddOrGetChild", arg0, arg1) ret0, _ := ret[0].(api.Entry) ret1, _ := ret[1].(error) return ret0, ret1 } -// AddUpdateRecursiveInternal indicates an expected call of AddUpdateRecursiveInternal. -func (mr *MockEntryMockRecorder) AddUpdateRecursiveInternal(ctx, path, idx, u, flags any) *gomock.Call { +// AddOrGetChild indicates an expected call of AddOrGetChild. +func (mr *MockEntryMockRecorder) AddOrGetChild(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddUpdateRecursiveInternal", reflect.TypeOf((*MockEntry)(nil).AddUpdateRecursiveInternal), ctx, path, idx, u, flags) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddOrGetChild", reflect.TypeOf((*MockEntry)(nil).AddOrGetChild), arg0, arg1) } // CanDelete mocks base method. @@ -115,6 +86,20 @@ func (mr *MockEntryMockRecorder) CanDeleteBranch(keepDefault any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CanDeleteBranch", reflect.TypeOf((*MockEntry)(nil).CanDeleteBranch), keepDefault) } +// ChoicesResolvers mocks base method. +func (m *MockEntry) ChoicesResolvers() api.ChoiceResolvers { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ChoicesResolvers") + ret0, _ := ret[0].(api.ChoiceResolvers) + return ret0 +} + +// ChoicesResolvers indicates an expected call of ChoicesResolvers. +func (mr *MockEntryMockRecorder) ChoicesResolvers() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ChoicesResolvers", reflect.TypeOf((*MockEntry)(nil).ChoicesResolvers)) +} + // DeepCopy mocks base method. func (m *MockEntry) DeepCopy(tc api.TreeContext, parent api.Entry) (api.Entry, error) { m.ctrl.T.Helper() @@ -130,20 +115,6 @@ func (mr *MockEntryMockRecorder) DeepCopy(tc, parent any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeepCopy", reflect.TypeOf((*MockEntry)(nil).DeepCopy), tc, parent) } -// DeleteBranch mocks base method. -func (m *MockEntry) DeleteBranch(ctx context.Context, path *sdcpb.Path, owner string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteBranch", ctx, path, owner) - ret0, _ := ret[0].(error) - return ret0 -} - -// DeleteBranch indicates an expected call of DeleteBranch. -func (mr *MockEntryMockRecorder) DeleteBranch(ctx, path, owner any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteBranch", reflect.TypeOf((*MockEntry)(nil).DeleteBranch), ctx, path, owner) -} - // DeleteCanDeleteChilds mocks base method. func (m *MockEntry) DeleteCanDeleteChilds(keepDefault bool) { m.ctrl.T.Helper() @@ -156,21 +127,6 @@ func (mr *MockEntryMockRecorder) DeleteCanDeleteChilds(keepDefault any) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteCanDeleteChilds", reflect.TypeOf((*MockEntry)(nil).DeleteCanDeleteChilds), keepDefault) } -// FilterChilds mocks base method. -func (m *MockEntry) FilterChilds(keys map[string]string) ([]api.Entry, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "FilterChilds", keys) - ret0, _ := ret[0].([]api.Entry) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// FilterChilds indicates an expected call of FilterChilds. -func (mr *MockEntryMockRecorder) FilterChilds(keys any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FilterChilds", reflect.TypeOf((*MockEntry)(nil).FilterChilds), keys) -} - // FinishInsertionPhase mocks base method. func (m *MockEntry) FinishInsertionPhase(ctx context.Context) error { m.ctrl.T.Helper() @@ -185,21 +141,6 @@ func (mr *MockEntryMockRecorder) FinishInsertionPhase(ctx any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FinishInsertionPhase", reflect.TypeOf((*MockEntry)(nil).FinishInsertionPhase), ctx) } -// GetChild mocks base method. -func (m *MockEntry) GetChild(name string) (api.Entry, bool) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetChild", name) - ret0, _ := ret[0].(api.Entry) - ret1, _ := ret[1].(bool) - return ret0, ret1 -} - -// GetChild indicates an expected call of GetChild. -func (mr *MockEntryMockRecorder) GetChild(name any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChild", reflect.TypeOf((*MockEntry)(nil).GetChild), name) -} - // GetChildMap mocks base method. func (m *MockEntry) GetChildMap() *api.ChildMap { m.ctrl.T.Helper() @@ -228,64 +169,6 @@ func (mr *MockEntryMockRecorder) GetChilds(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChilds", reflect.TypeOf((*MockEntry)(nil).GetChilds), arg0) } -// GetDeletes mocks base method. -func (m *MockEntry) GetDeletes(entries types.DeleteEntriesList, aggregatePaths bool) (types.DeleteEntriesList, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetDeletes", entries, aggregatePaths) - ret0, _ := ret[0].(types.DeleteEntriesList) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetDeletes indicates an expected call of GetDeletes. -func (mr *MockEntryMockRecorder) GetDeletes(entries, aggregatePaths any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDeletes", reflect.TypeOf((*MockEntry)(nil).GetDeletes), entries, aggregatePaths) -} - -// GetHighestPrecedence mocks base method. -func (m *MockEntry) GetHighestPrecedence(result api.LeafVariantSlice, onlyNewOrUpdated, includeDefaults, includeExplicitDelete bool) api.LeafVariantSlice { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetHighestPrecedence", result, onlyNewOrUpdated, includeDefaults, includeExplicitDelete) - ret0, _ := ret[0].(api.LeafVariantSlice) - return ret0 -} - -// GetHighestPrecedence indicates an expected call of GetHighestPrecedence. -func (mr *MockEntryMockRecorder) GetHighestPrecedence(result, onlyNewOrUpdated, includeDefaults, includeExplicitDelete any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetHighestPrecedence", reflect.TypeOf((*MockEntry)(nil).GetHighestPrecedence), result, onlyNewOrUpdated, includeDefaults, includeExplicitDelete) -} - -// GetHighestPrecedenceLeafValue mocks base method. -func (m *MockEntry) GetHighestPrecedenceLeafValue(arg0 context.Context) (*api.LeafEntry, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetHighestPrecedenceLeafValue", arg0) - ret0, _ := ret[0].(*api.LeafEntry) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetHighestPrecedenceLeafValue indicates an expected call of GetHighestPrecedenceLeafValue. -func (mr *MockEntryMockRecorder) GetHighestPrecedenceLeafValue(arg0 any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetHighestPrecedenceLeafValue", reflect.TypeOf((*MockEntry)(nil).GetHighestPrecedenceLeafValue), arg0) -} - -// GetHighestPrecedenceValueOfBranch mocks base method. -func (m *MockEntry) GetHighestPrecedenceValueOfBranch(filter api.HighestPrecedenceFilter) int32 { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetHighestPrecedenceValueOfBranch", filter) - ret0, _ := ret[0].(int32) - return ret0 -} - -// GetHighestPrecedenceValueOfBranch indicates an expected call of GetHighestPrecedenceValueOfBranch. -func (mr *MockEntryMockRecorder) GetHighestPrecedenceValueOfBranch(filter any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetHighestPrecedenceValueOfBranch", reflect.TypeOf((*MockEntry)(nil).GetHighestPrecedenceValueOfBranch), filter) -} - // GetLeafVariants mocks base method. func (m *MockEntry) GetLeafVariants() *api.LeafVariants { m.ctrl.T.Helper() @@ -314,36 +197,6 @@ func (mr *MockEntryMockRecorder) GetLevel() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLevel", reflect.TypeOf((*MockEntry)(nil).GetLevel)) } -// GetListChilds mocks base method. -func (m *MockEntry) GetListChilds() ([]api.Entry, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetListChilds") - ret0, _ := ret[0].([]api.Entry) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetListChilds indicates an expected call of GetListChilds. -func (mr *MockEntryMockRecorder) GetListChilds() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetListChilds", reflect.TypeOf((*MockEntry)(nil).GetListChilds)) -} - -// GetOrCreateChilds mocks base method. -func (m *MockEntry) GetOrCreateChilds(ctx context.Context, path *sdcpb.Path) (api.Entry, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetOrCreateChilds", ctx, path) - ret0, _ := ret[0].(api.Entry) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetOrCreateChilds indicates an expected call of GetOrCreateChilds. -func (mr *MockEntryMockRecorder) GetOrCreateChilds(ctx, path any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrCreateChilds", reflect.TypeOf((*MockEntry)(nil).GetOrCreateChilds), ctx, path) -} - // GetParent mocks base method. func (m *MockEntry) GetParent() api.Entry { m.ctrl.T.Helper() @@ -358,20 +211,6 @@ func (mr *MockEntryMockRecorder) GetParent() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetParent", reflect.TypeOf((*MockEntry)(nil).GetParent)) } -// GetRootBasedEntryChain mocks base method. -func (m *MockEntry) GetRootBasedEntryChain() []api.Entry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetRootBasedEntryChain") - ret0, _ := ret[0].([]api.Entry) - return ret0 -} - -// GetRootBasedEntryChain indicates an expected call of GetRootBasedEntryChain. -func (mr *MockEntryMockRecorder) GetRootBasedEntryChain() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRootBasedEntryChain", reflect.TypeOf((*MockEntry)(nil).GetRootBasedEntryChain)) -} - // GetSchema mocks base method. func (m *MockEntry) GetSchema() *sdcpb.SchemaElem { m.ctrl.T.Helper() @@ -386,20 +225,6 @@ func (mr *MockEntryMockRecorder) GetSchema() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSchema", reflect.TypeOf((*MockEntry)(nil).GetSchema)) } -// GetSchemaKeys mocks base method. -func (m *MockEntry) GetSchemaKeys() []string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSchemaKeys") - ret0, _ := ret[0].([]string) - return ret0 -} - -// GetSchemaKeys indicates an expected call of GetSchemaKeys. -func (mr *MockEntryMockRecorder) GetSchemaKeys() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSchemaKeys", reflect.TypeOf((*MockEntry)(nil).GetSchemaKeys)) -} - // GetTreeContext mocks base method. func (m *MockEntry) GetTreeContext() api.TreeContext { m.ctrl.T.Helper() @@ -414,20 +239,6 @@ func (mr *MockEntryMockRecorder) GetTreeContext() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTreeContext", reflect.TypeOf((*MockEntry)(nil).GetTreeContext)) } -// HoldsLeafvariants mocks base method. -func (m *MockEntry) HoldsLeafvariants() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "HoldsLeafvariants") - ret0, _ := ret[0].(bool) - return ret0 -} - -// HoldsLeafvariants indicates an expected call of HoldsLeafvariants. -func (mr *MockEntryMockRecorder) HoldsLeafvariants() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HoldsLeafvariants", reflect.TypeOf((*MockEntry)(nil).HoldsLeafvariants)) -} - // IsRoot mocks base method. func (m *MockEntry) IsRoot() bool { m.ctrl.T.Helper() @@ -442,21 +253,6 @@ func (mr *MockEntryMockRecorder) IsRoot() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsRoot", reflect.TypeOf((*MockEntry)(nil).IsRoot)) } -// NavigateSdcpbPath mocks base method. -func (m *MockEntry) NavigateSdcpbPath(ctx context.Context, path *sdcpb.Path) (api.Entry, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "NavigateSdcpbPath", ctx, path) - ret0, _ := ret[0].(api.Entry) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// NavigateSdcpbPath indicates an expected call of NavigateSdcpbPath. -func (mr *MockEntryMockRecorder) NavigateSdcpbPath(ctx, path any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NavigateSdcpbPath", reflect.TypeOf((*MockEntry)(nil).NavigateSdcpbPath), ctx, path) -} - // PathName mocks base method. func (m *MockEntry) PathName() string { m.ctrl.T.Helper() @@ -526,243 +322,3 @@ func (mr *MockEntryMockRecorder) StringIndent(result any) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StringIndent", reflect.TypeOf((*MockEntry)(nil).StringIndent), result) } - -// MockLeafVariantEntry is a mock of LeafVariantEntry interface. -type MockLeafVariantEntry struct { - ctrl *gomock.Controller - recorder *MockLeafVariantEntryMockRecorder - isgomock struct{} -} - -// MockLeafVariantEntryMockRecorder is the mock recorder for MockLeafVariantEntry. -type MockLeafVariantEntryMockRecorder struct { - mock *MockLeafVariantEntry -} - -// NewMockLeafVariantEntry creates a new mock instance. -func NewMockLeafVariantEntry(ctrl *gomock.Controller) *MockLeafVariantEntry { - mock := &MockLeafVariantEntry{ctrl: ctrl} - mock.recorder = &MockLeafVariantEntryMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockLeafVariantEntry) EXPECT() *MockLeafVariantEntryMockRecorder { - return m.recorder -} - -// GetEntry mocks base method. -func (m *MockLeafVariantEntry) GetEntry() api.Entry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetEntry") - ret0, _ := ret[0].(api.Entry) - return ret0 -} - -// GetEntry indicates an expected call of GetEntry. -func (mr *MockLeafVariantEntryMockRecorder) GetEntry() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEntry", reflect.TypeOf((*MockLeafVariantEntry)(nil).GetEntry)) -} - -// MarkDelete mocks base method. -func (m *MockLeafVariantEntry) MarkDelete(onlyIntended bool) *api.LeafEntry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "MarkDelete", onlyIntended) - ret0, _ := ret[0].(*api.LeafEntry) - return ret0 -} - -// MarkDelete indicates an expected call of MarkDelete. -func (mr *MockLeafVariantEntryMockRecorder) MarkDelete(onlyIntended any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkDelete", reflect.TypeOf((*MockLeafVariantEntry)(nil).MarkDelete), onlyIntended) -} - -// String mocks base method. -func (m *MockLeafVariantEntry) String() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "String") - ret0, _ := ret[0].(string) - return ret0 -} - -// String indicates an expected call of String. -func (mr *MockLeafVariantEntryMockRecorder) String() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "String", reflect.TypeOf((*MockLeafVariantEntry)(nil).String)) -} - -// MockLeafVariantEntries is a mock of LeafVariantEntries interface. -type MockLeafVariantEntries struct { - ctrl *gomock.Controller - recorder *MockLeafVariantEntriesMockRecorder - isgomock struct{} -} - -// MockLeafVariantEntriesMockRecorder is the mock recorder for MockLeafVariantEntries. -type MockLeafVariantEntriesMockRecorder struct { - mock *MockLeafVariantEntries -} - -// NewMockLeafVariantEntries creates a new mock instance. -func NewMockLeafVariantEntries(ctrl *gomock.Controller) *MockLeafVariantEntries { - mock := &MockLeafVariantEntries{ctrl: ctrl} - mock.recorder = &MockLeafVariantEntriesMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockLeafVariantEntries) EXPECT() *MockLeafVariantEntriesMockRecorder { - return m.recorder -} - -// Add mocks base method. -func (m *MockLeafVariantEntries) Add(l *api.LeafEntry) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "Add", l) -} - -// Add indicates an expected call of Add. -func (mr *MockLeafVariantEntriesMockRecorder) Add(l any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockLeafVariantEntries)(nil).Add), l) -} - -// AddExplicitDeleteEntry mocks base method. -func (m *MockLeafVariantEntries) AddExplicitDeleteEntry(owner string, priority int32) *api.LeafEntry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AddExplicitDeleteEntry", owner, priority) - ret0, _ := ret[0].(*api.LeafEntry) - return ret0 -} - -// AddExplicitDeleteEntry indicates an expected call of AddExplicitDeleteEntry. -func (mr *MockLeafVariantEntriesMockRecorder) AddExplicitDeleteEntry(owner, priority any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddExplicitDeleteEntry", reflect.TypeOf((*MockLeafVariantEntries)(nil).AddExplicitDeleteEntry), owner, priority) -} - -// AddWithStats mocks base method. -func (m *MockLeafVariantEntries) AddWithStats(l *api.LeafEntry, stats *types.ImportStats) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "AddWithStats", l, stats) -} - -// AddWithStats indicates an expected call of AddWithStats. -func (mr *MockLeafVariantEntriesMockRecorder) AddWithStats(l, stats any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddWithStats", reflect.TypeOf((*MockLeafVariantEntries)(nil).AddWithStats), l, stats) -} - -// DeleteByOwner mocks base method. -func (m *MockLeafVariantEntries) DeleteByOwner(owner string) *api.LeafEntry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteByOwner", owner) - ret0, _ := ret[0].(*api.LeafEntry) - return ret0 -} - -// DeleteByOwner indicates an expected call of DeleteByOwner. -func (mr *MockLeafVariantEntriesMockRecorder) DeleteByOwner(owner any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteByOwner", reflect.TypeOf((*MockLeafVariantEntries)(nil).DeleteByOwner), owner) -} - -// GetByOwner mocks base method. -func (m *MockLeafVariantEntries) GetByOwner(owner string) *api.LeafEntry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetByOwner", owner) - ret0, _ := ret[0].(*api.LeafEntry) - return ret0 -} - -// GetByOwner indicates an expected call of GetByOwner. -func (mr *MockLeafVariantEntriesMockRecorder) GetByOwner(owner any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetByOwner", reflect.TypeOf((*MockLeafVariantEntries)(nil).GetByOwner), owner) -} - -// GetHighestPrecedence mocks base method. -func (m *MockLeafVariantEntries) GetHighestPrecedence(onlyNewOrUpdated, includeDefaults, includeExplicitDeletes bool) *api.LeafEntry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetHighestPrecedence", onlyNewOrUpdated, includeDefaults, includeExplicitDeletes) - ret0, _ := ret[0].(*api.LeafEntry) - return ret0 -} - -// GetHighestPrecedence indicates an expected call of GetHighestPrecedence. -func (mr *MockLeafVariantEntriesMockRecorder) GetHighestPrecedence(onlyNewOrUpdated, includeDefaults, includeExplicitDeletes any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetHighestPrecedence", reflect.TypeOf((*MockLeafVariantEntries)(nil).GetHighestPrecedence), onlyNewOrUpdated, includeDefaults, includeExplicitDeletes) -} - -// GetRunning mocks base method. -func (m *MockLeafVariantEntries) GetRunning() *api.LeafEntry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetRunning") - ret0, _ := ret[0].(*api.LeafEntry) - return ret0 -} - -// GetRunning indicates an expected call of GetRunning. -func (mr *MockLeafVariantEntriesMockRecorder) GetRunning() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRunning", reflect.TypeOf((*MockLeafVariantEntries)(nil).GetRunning)) -} - -// Length mocks base method. -func (m *MockLeafVariantEntries) Length() int { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Length") - ret0, _ := ret[0].(int) - return ret0 -} - -// Length indicates an expected call of Length. -func (mr *MockLeafVariantEntriesMockRecorder) Length() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Length", reflect.TypeOf((*MockLeafVariantEntries)(nil).Length)) -} - -// MarkOwnerForDeletion mocks base method. -func (m *MockLeafVariantEntries) MarkOwnerForDeletion(owner string, onlyIntended bool) *api.LeafEntry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "MarkOwnerForDeletion", owner, onlyIntended) - ret0, _ := ret[0].(*api.LeafEntry) - return ret0 -} - -// MarkOwnerForDeletion indicates an expected call of MarkOwnerForDeletion. -func (mr *MockLeafVariantEntriesMockRecorder) MarkOwnerForDeletion(owner, onlyIntended any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkOwnerForDeletion", reflect.TypeOf((*MockLeafVariantEntries)(nil).MarkOwnerForDeletion), owner, onlyIntended) -} - -// RemoveDeletedByOwner mocks base method. -func (m *MockLeafVariantEntries) RemoveDeletedByOwner(owner string) *api.LeafEntry { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RemoveDeletedByOwner", owner) - ret0, _ := ret[0].(*api.LeafEntry) - return ret0 -} - -// RemoveDeletedByOwner indicates an expected call of RemoveDeletedByOwner. -func (mr *MockLeafVariantEntriesMockRecorder) RemoveDeletedByOwner(owner any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveDeletedByOwner", reflect.TypeOf((*MockLeafVariantEntries)(nil).RemoveDeletedByOwner), owner) -} - -// ResetFlags mocks base method. -func (m *MockLeafVariantEntries) ResetFlags(deleteFlag, newFlag, updatedFlag bool) int { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ResetFlags", deleteFlag, newFlag, updatedFlag) - ret0, _ := ret[0].(int) - return ret0 -} - -// ResetFlags indicates an expected call of ResetFlags. -func (mr *MockLeafVariantEntriesMockRecorder) ResetFlags(deleteFlag, newFlag, updatedFlag any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResetFlags", reflect.TypeOf((*MockLeafVariantEntries)(nil).ResetFlags), deleteFlag, newFlag, updatedFlag) -} diff --git a/pkg/datastore/datastore_rpc.go b/pkg/datastore/datastore_rpc.go index b3337060..024147dd 100644 --- a/pkg/datastore/datastore_rpc.go +++ b/pkg/datastore/datastore_rpc.go @@ -17,6 +17,7 @@ package datastore import ( "context" "errors" + "fmt" "runtime" "sync" "time" @@ -35,7 +36,10 @@ import ( "github.com/sdcio/data-server/pkg/pool" "github.com/sdcio/data-server/pkg/schema" "github.com/sdcio/data-server/pkg/tree" + "github.com/sdcio/data-server/pkg/tree/ops" "github.com/sdcio/data-server/pkg/tree/processors" + treetypes "github.com/sdcio/data-server/pkg/tree/types" + tree_persist "github.com/sdcio/sdc-protos/tree_persist" ) type Datastore struct { @@ -69,6 +73,10 @@ type Datastore struct { syncTree *tree.RootEntry syncTreeMutex *sync.RWMutex + // sensitivePathIndex is the in-memory reverse index of sensitive paths. + // It is populated at startup and updated on every intent write or delete. + sensitivePathIndex *treetypes.SensitivePathIndex + taskPool *pool.SharedTaskPool } @@ -104,17 +112,18 @@ func New(ctx context.Context, c *config.DatastoreConfig, sc schema.Client, cc ca ccb := cache.NewCacheClientBound(c.Name, cc) ds := &Datastore{ - config: c, - schemaClient: scb, - ctx: ctx, - cfn: cancel, - cacheClient: ccb, - m: &sync.RWMutex{}, - dmutex: &sync.Mutex{}, - deviationClients: make(map[sdcpb.DataServer_WatchDeviationsServer]string), - syncTree: syncTreeRoot, - syncTreeMutex: &sync.RWMutex{}, - taskPool: pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)), + config: c, + schemaClient: scb, + ctx: ctx, + cfn: cancel, + cacheClient: ccb, + m: &sync.RWMutex{}, + dmutex: &sync.Mutex{}, + deviationClients: make(map[sdcpb.DataServer_WatchDeviationsServer]string), + syncTree: syncTreeRoot, + syncTreeMutex: &sync.RWMutex{}, + sensitivePathIndex: treetypes.NewSensitivePathIndex(), + taskPool: pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)), } ds.transactionManager = types.NewTransactionManager(NewDatastoreRollbackAdapter(ds)) @@ -122,6 +131,12 @@ func New(ctx context.Context, c *config.DatastoreConfig, sc schema.Client, cc ca // this is a blocking call ds.initCache(ctx) + // populate the sensitive-path index from all existing intents in cache. + if err := populateSensitivePathIndex(ctx, ds.sensitivePathIndex, ccb); err != nil { + cancel() + return nil, fmt.Errorf("failed to populate sensitive path index: %w", err) + } + go func() { // init sbi, this is a blocking call err := ds.connectSBI(ctx, opts...) @@ -228,7 +243,7 @@ func (d *Datastore) Stop(ctx context.Context) error { return nil } -func (d *Datastore) BlameConfig(ctx context.Context, includeDefaults bool) (*sdcpb.BlameTreeElement, error) { +func (d *Datastore) BlameConfig(ctx context.Context, includeDefaults, exposeSensitive bool) (*sdcpb.BlameTreeElement, error) { log := logger.FromContext(ctx).WithName("BlameConfig") ctx = logger.IntoContext(ctx, log) @@ -239,13 +254,19 @@ func (d *Datastore) BlameConfig(ctx context.Context, includeDefaults bool) (*sdc if err != nil { return nil, err } - // load all intents + // load all intents into the tree for blame; sensitive paths come from the index. _, err = d.LoadAllButRunningIntents(ctx, root) if err != nil { return nil, err } - bcp := processors.NewBlameConfigProcessor(&processors.BlameConfigProcessorParams{IncludeDefaults: includeDefaults}) + bcp := processors.NewBlameConfigProcessor(&processors.BlameConfigProcessorParams{ + IncludeDefaults: includeDefaults, + RenderOpts: ops.RenderOpts{ + IncludeSensitive: exposeSensitive, + SensitivePathSet: d.sensitivePathIndex, + }, + }) bte, err := bcp.Run(ctx, root.Entry, d.taskPool) // set the root level elements name to the target name @@ -272,3 +293,15 @@ func (dra *DatastoreRollbackAdapter) TransactionRollback(ctx context.Context, tr // Assure the types.RollbackInterface is implemented by the DatastoreRollbackAdapter var _ types.RollbackInterface = &DatastoreRollbackAdapter{} + +// populateSensitivePathIndex scans all intents in cache and loads their +// sensitive_paths into s. It is called once during Datastore startup, before +// the first northbound read is served. +func populateSensitivePathIndex(ctx context.Context, s *treetypes.SensitivePathIndex, cc cache.CacheClientBound) error { + return forEachIntent(ctx, cc, nil, func(intent *tree_persist.Intent) error { + if len(intent.GetSensitivePaths()) > 0 { + s.Set(intent.GetIntentName(), intent.GetSensitivePaths()) + } + return nil + }) +} diff --git a/pkg/datastore/deviations.go b/pkg/datastore/deviations.go index 964817ec..17d2f675 100644 --- a/pkg/datastore/deviations.go +++ b/pkg/datastore/deviations.go @@ -138,11 +138,16 @@ func (d DeviationsStats) LogValue() map[string]map[string]int { return stats } +// SendDeviations reads deviation entries from ch and forwards them to all +// active deviation clients. Sensitivity redaction is applied upstream in the +// ops layer (GetDeviations with IncludeSensitive=false), so no masking is done +// here. func (d *Datastore) SendDeviations(ctx context.Context, ch <-chan *treetypes.DeviationEntry, deviationClients map[string]sdcpb.DataServer_WatchDeviationsServer) { log := logf.FromContext(ctx) deviationsStats := make(DeviationsStats) for deviation := range ch { deviationsStats.Add(deviation) + for clientIdentifier, dc := range deviationClients { if dc.Context().Err() != nil { continue @@ -212,7 +217,15 @@ func (d *Datastore) calculateDeviations(ctx context.Context) (<-chan *treetypes. deviationChan <- treetypes.NewDeviationEntry(n, treetypes.DeviationReasonIntentExists, nil) } - err := ops.GetDeviations(ctx, deviationTree.Entry, &ops.GetDeviationParams{Ch: deviationChan}, d.taskPool) + // IncludeSensitive=false: redaction is applied in the ops layer so that + // sensitive values never leave the tree as plaintext. + err := ops.GetDeviations(ctx, deviationTree.Entry, &ops.GetDeviationParams{ + Ch: deviationChan, + RenderOpts: ops.RenderOpts{ + IncludeSensitive: false, + SensitivePathSet: d.sensitivePathIndex, + }, + }, d.taskPool) if err != nil { log.Error(err, "failed to run deviation processor") } diff --git a/pkg/datastore/deviations_test.go b/pkg/datastore/deviations_test.go new file mode 100644 index 00000000..fb87c216 --- /dev/null +++ b/pkg/datastore/deviations_test.go @@ -0,0 +1,183 @@ +package datastore + +import ( + "context" + "runtime" + "sync" + "testing" + + "github.com/openconfig/ygot/ygot" + "github.com/sdcio/data-server/mocks/mockcacheclient" + "github.com/sdcio/data-server/pkg/config" + "github.com/sdcio/data-server/pkg/pool" + schemaClientPkg "github.com/sdcio/data-server/pkg/datastore/clients/schema" + treetypes "github.com/sdcio/data-server/pkg/tree/types" + "github.com/sdcio/data-server/pkg/utils/testhelper" + sdcio_schema "github.com/sdcio/data-server/tests/sdcioygot" + tree_persist "github.com/sdcio/sdc-protos/tree_persist" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" + "go.uber.org/mock/gomock" + "google.golang.org/grpc/metadata" +) + +// fakeDeviationStream is a minimal in-memory DataServer_WatchDeviationsServer. +// Only Send and Context are called by SendDeviations; everything else is a no-op. +type fakeDeviationStream struct { + ctx context.Context + mu sync.Mutex + received []*sdcpb.WatchDeviationResponse +} + +func newFakeDeviationStream(ctx context.Context) *fakeDeviationStream { + return &fakeDeviationStream{ctx: ctx} +} + +func (f *fakeDeviationStream) Send(r *sdcpb.WatchDeviationResponse) error { + f.mu.Lock() + defer f.mu.Unlock() + f.received = append(f.received, r) + return nil +} + +func (f *fakeDeviationStream) updateResponses() []*sdcpb.WatchDeviationResponse { + f.mu.Lock() + defer f.mu.Unlock() + var out []*sdcpb.WatchDeviationResponse + for _, r := range f.received { + if r.GetEvent() == sdcpb.DeviationEvent_UPDATE { + out = append(out, r) + } + } + return out +} + +func (f *fakeDeviationStream) Context() context.Context { return f.ctx } +func (f *fakeDeviationStream) SetHeader(metadata.MD) error { return nil } +func (f *fakeDeviationStream) SendHeader(metadata.MD) error { return nil } +func (f *fakeDeviationStream) SetTrailer(metadata.MD) {} +func (f *fakeDeviationStream) SendMsg(any) error { return nil } +func (f *fakeDeviationStream) RecvMsg(any) error { return nil } + +var _ sdcpb.DataServer_WatchDeviationsServer = (*fakeDeviationStream)(nil) + +// deviationEntry builds a DeviationEntry with string-valued ExpectedValue and CurrentValue. +func deviationEntry(path *sdcpb.Path, expected, current string) *treetypes.DeviationEntry { + return treetypes.NewDeviationEntry("test-intent", treetypes.DeviationReasonNotApplied, path). + SetExpectedValue(&sdcpb.TypedValue{Value: &sdcpb.TypedValue_StringVal{StringVal: expected}}). + SetCurrentValue(&sdcpb.TypedValue{Value: &sdcpb.TypedValue_StringVal{StringVal: current}}) +} + +// simplePath builds a root-based sdcpb.Path with a single element. +func simplePath(name string) *sdcpb.Path { + return &sdcpb.Path{ + IsRootBased: true, + Elem: []*sdcpb.PathElem{{Name: name}}, + } +} + +// TestSendDeviations_PassthroughUnredacted verifies that SendDeviations forwards +// deviation entries to clients without modifying values. Sensitivity redaction is +// now the responsibility of the ops layer (GetDeviations), not the datastore layer. +func TestSendDeviations_PassthroughUnredacted(t *testing.T) { + ctx := context.Background() + + ds := &Datastore{ + config: &config.DatastoreConfig{Name: "test-ds", Validation: config.NewValidationConfig()}, + } + + path := simplePath("plain-leaf") + ch := make(chan *treetypes.DeviationEntry, 1) + ch <- deviationEntry(path, "expected-value", "current-value") + close(ch) + + stream := newFakeDeviationStream(ctx) + ds.SendDeviations(ctx, ch, map[string]sdcpb.DataServer_WatchDeviationsServer{"fake": stream}) + + updates := stream.updateResponses() + if len(updates) != 1 { + t.Fatalf("expected 1 UPDATE response, got %d", len(updates)) + } + if got := updates[0].GetExpectedValue().GetStringVal(); got != "expected-value" { + t.Errorf("ExpectedValue = %q, want %q", got, "expected-value") + } + if got := updates[0].GetCurrentValue().GetStringVal(); got != "current-value" { + t.Errorf("CurrentValue = %q, want %q", got, "current-value") + } +} + +// TestWatchDeviations_SensitivePathMasking is the integration acceptance test +// for Issue 05. +// +// Scenario: one intent sets "patterntest" to "running-secret"; a second (marker) +// intent marks /patterntest in its sensitive_paths. The sync tree has no value +// for patterntest, so the deviation engine sees a mismatch. Both ExpectedValue +// and CurrentValue in the emitted deviation must be "***". +func TestWatchDeviations_SensitivePathMasking(t *testing.T) { + ctx := context.Background() + + sc, schema, err := testhelper.InitSDCIOSchema() + if err != nil { + t.Fatal(err) + } + // Plain schema client — patterntest is NOT schema-sensitive. + scb := schemaClientPkg.NewSchemaClientBound(schema, sc) + + runningDevice := &sdcio_schema.Device{Patterntest: ygot.String("running-secret")} + dataIntent := buildTestIntent(t, ctx, scb, runningDevice, "data-intent", 10, nil) + markerIntent := buildTestIntent(t, ctx, scb, nil, "marker-intent", 5, []*sdcpb.Path{{Elem: []*sdcpb.PathElem{{Name: "patterntest"}}, IsRootBased: true}}) + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + ccb := mockcacheclient.NewMockCacheClientBound(ctrl) + ccb.EXPECT(). + IntentGetAll(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ []string, intentChan chan<- *tree_persist.Intent, errChan chan<- error) { + intentChan <- dataIntent + intentChan <- markerIntent + close(intentChan) + close(errChan) + }) + + // Pre-populate index with marker-intent's sensitive paths. + idx := treetypes.NewSensitivePathIndex() + idx.Set("marker-intent", markerIntent.GetSensitivePaths()) + + // Empty sync tree → running device has no patterntest → triggers deviation. + syncRoot := buildEmptySyncTree(t, ctx, scb) + + ds := &Datastore{ + config: &config.DatastoreConfig{Name: "test-ds", Validation: config.NewValidationConfig()}, + syncTree: syncRoot, + syncTreeMutex: &sync.RWMutex{}, + taskPool: pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)), + cacheClient: ccb, + schemaClient: scb, + sensitivePathIndex: idx, + } + + deviationChan, calcErr := ds.calculateDeviations(ctx) + if calcErr != nil { + t.Fatalf("calculateDeviations() error: %v", calcErr) + } + + stream := newFakeDeviationStream(ctx) + ds.SendDeviations(ctx, deviationChan, map[string]sdcpb.DataServer_WatchDeviationsServer{"fake": stream}) + + found := false + for _, r := range stream.updateResponses() { + elems := r.GetPath().GetElem() + if len(elems) > 0 && elems[len(elems)-1].GetName() == "patterntest" { + found = true + if got := r.GetExpectedValue().GetStringVal(); got != "***" { + t.Errorf("patterntest ExpectedValue = %q, want %q", got, "***") + } + if got := r.GetCurrentValue().GetStringVal(); got != "***" { + t.Errorf("patterntest CurrentValue = %q, want %q", got, "***") + } + } + } + if !found { + t.Error("no deviation found for patterntest — check that the deviation engine emits it") + } +} diff --git a/pkg/datastore/intent_rpc.go b/pkg/datastore/intent_rpc.go index 974384d5..6b70d971 100644 --- a/pkg/datastore/intent_rpc.go +++ b/pkg/datastore/intent_rpc.go @@ -26,6 +26,7 @@ import ( "github.com/sdcio/data-server/pkg/tree/api/adapter" "github.com/sdcio/data-server/pkg/tree/consts" "github.com/sdcio/data-server/pkg/tree/importer/proto" + "github.com/sdcio/data-server/pkg/tree/ops" "github.com/sdcio/data-server/pkg/tree/types" "github.com/sdcio/data-server/pkg/utils" logf "github.com/sdcio/logger" @@ -58,8 +59,10 @@ func (d *Datastore) applyIntent(ctx context.Context, source targettypes.TargetSo return rsp, nil } -func (d *Datastore) GetIntent(ctx context.Context, intentName string) (GetIntentResponse, error) { - // serve running from synctree +func (d *Datastore) GetIntent(ctx context.Context, intentName string, exposeSensitive bool) (GetIntentResponse, error) { + // serve running from synctree; sensitive paths are the cross-intent union + // (running has no own markers — its values may have been echoed back by + // the device verbatim, so any intent's classification must apply). if intentName == consts.RunningIntentName { d.syncTreeMutex.RLock() defer d.syncTreeMutex.RUnlock() @@ -75,12 +78,18 @@ func (d *Datastore) GetIntent(ctx context.Context, intentName string) (GetIntent Orphan: false, NonRevertive: false, ExplicitDeletes: nil, + RenderOpts: ops.RenderOpts{ + IncludeSensitive: exposeSensitive, + SensitivePathSet: d.sensitivePathIndex, + }, } return result, nil } - // otherwise consult cache + // For a regular intent GET, sensitive-path redaction is scoped to that + // intent's own markers only. Another intent's classification does not + // affect how this intent's data is presented (see ADR 0004). root, err := tree.NewTreeRoot(ctx, tree.NewTreeContext(d.schemaClient, d.taskPool)) if err != nil { return nil, err @@ -102,6 +111,9 @@ func (d *Datastore) GetIntent(ctx context.Context, intentName string) (GetIntent return nil, err } + intentSensitivePaths := types.NewSensitivePathIndex() + intentSensitivePaths.Add(tp.GetSensitivePaths()...) + result := &adapter.IntentResponseAdapter{ Entry: root.Entry, IntentName: tp.GetIntentName(), @@ -109,6 +121,10 @@ func (d *Datastore) GetIntent(ctx context.Context, intentName string) (GetIntent Orphan: tp.GetOrphan(), NonRevertive: tp.GetNonRevertive(), ExplicitDeletes: tp.GetExplicitDeletes(), + RenderOpts: ops.RenderOpts{ + IncludeSensitive: exposeSensitive, + SensitivePathSet: intentSensitivePaths, + }, } return result, nil } diff --git a/pkg/datastore/sensitive_path_union_test.go b/pkg/datastore/sensitive_path_union_test.go new file mode 100644 index 00000000..452bae95 --- /dev/null +++ b/pkg/datastore/sensitive_path_union_test.go @@ -0,0 +1,476 @@ +package datastore + +import ( + "context" + "runtime" + "sync" + "testing" + + "github.com/openconfig/ygot/ygot" + "github.com/sdcio/data-server/mocks/mockcacheclient" + "github.com/sdcio/data-server/pkg/config" + schemaClient "github.com/sdcio/data-server/pkg/datastore/clients/schema" + "github.com/sdcio/data-server/pkg/pool" + "github.com/sdcio/data-server/pkg/tree" + "github.com/sdcio/data-server/pkg/tree/consts" + "github.com/sdcio/data-server/pkg/tree/ops" + treetypes "github.com/sdcio/data-server/pkg/tree/types" + "github.com/sdcio/data-server/pkg/utils/testhelper" + sdcio_schema "github.com/sdcio/data-server/tests/sdcioygot" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" + tree_persist "github.com/sdcio/sdc-protos/tree_persist" + "go.uber.org/mock/gomock" +) + +// buildTestIntent builds a tree_persist.Intent from a device struct using +// TreeExport, then stamps the provided sensitive_paths onto it. If device is +// nil the Root will be empty (useful for marker-only intents). +func buildTestIntent(t *testing.T, ctx context.Context, scb schemaClient.SchemaClientBound, device *sdcio_schema.Device, intentName string, prio int32, sensitivePaths []*sdcpb.Path) *tree_persist.Intent { + t.Helper() + + flagsNew := treetypes.NewUpdateInsertFlags() + flagsNew.SetNewFlag() + + tc := tree.NewTreeContext(scb, pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))) + root, err := tree.NewTreeRoot(ctx, tc) + if err != nil { + t.Fatal(err) + } + + if device != nil { + _, err = testhelper.LoadYgotStructIntoTreeRoot(ctx, device, root.Entry, intentName, prio, false, flagsNew) + if err != nil { + t.Fatal(err) + } + } + if err := root.FinishInsertionPhase(ctx); err != nil { + t.Fatal(err) + } + + if device == nil { + return &tree_persist.Intent{ + IntentName: intentName, + Priority: prio, + SensitivePaths: sensitivePaths, + } + } + + intent, err := ops.TreeExport(root.Entry, intentName, prio, false) + if err != nil { + t.Fatalf("TreeExport failed: %v", err) + } + intent.SensitivePaths = sensitivePaths + return intent +} + +// buildEmptySyncTree returns a RootEntry with an empty (finished) syncTree, +// suitable for use as the datastore syncTree in tests. +func buildEmptySyncTree(t *testing.T, ctx context.Context, scb schemaClient.SchemaClientBound) *tree.RootEntry { + t.Helper() + tc := tree.NewTreeContext(scb, pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))) + root, err := tree.NewTreeRoot(ctx, tc) + if err != nil { + t.Fatal(err) + } + if err := root.FinishInsertionPhase(ctx); err != nil { + t.Fatal(err) + } + return root +} + +// TestBlameConfig_CrossIntentSensitivePathRedaction verifies the cross-intent +// rule for BlameConfig: if any intent marks /patterntest in its sensitive_paths, +// BlameConfig redacts the value even when the schema does not mark that leaf +// sensitive and even when the owning intent did not declare it sensitive. +// +// Acceptance criteria covered: +// - BlameConfig: cross-intent sensitive path causes "***" redaction +// - include_sensitive=true bypasses path-marker redaction +// - Intent with no sensitive_paths contributes nothing to union +func TestBlameConfig_CrossIntentSensitivePathRedaction(t *testing.T) { + ctx := context.Background() + + sc, schema, err := testhelper.InitSDCIOSchema() + if err != nil { + t.Fatal(err) + } + // Plain schema client — patterntest is NOT schema-sensitive. + scb := schemaClient.NewSchemaClientBound(schema, sc) + + // data-intent: holds the actual patterntest value, no sensitive_paths. + dataDevice := &sdcio_schema.Device{Patterntest: ygot.String("secret-value")} + dataIntent := buildTestIntent(t, ctx, scb, dataDevice, "data-intent", 10, nil) + + // marker-intent: no data, but marks /patterntest sensitive. + markerIntent := buildTestIntent(t, ctx, scb, nil, "marker-intent", 5, []*sdcpb.Path{{Elem: []*sdcpb.PathElem{{Name: "patterntest"}}, IsRootBased: true}}) + + tests := []struct { + name string + exposeSensitive bool + wantValue string + }{ + { + name: "redacts path-marker sensitive value when include_sensitive=false", + exposeSensitive: false, + wantValue: "***", + }, + { + name: "reveals path-marker sensitive value when include_sensitive=true", + exposeSensitive: true, + wantValue: "secret-value", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + ccb := mockcacheclient.NewMockCacheClientBound(ctrl) + // LoadAllButRunningIntents streams via IntentGetAll (excludes running). + ccb.EXPECT(). + IntentGetAll(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ []string, intentChan chan<- *tree_persist.Intent, errChan chan<- error) { + intentChan <- dataIntent + intentChan <- markerIntent + close(intentChan) + close(errChan) + }) + + // Pre-populate the index with marker-intent's sensitive paths — + // BlameConfig reads the union from the index. + idx := treetypes.NewSensitivePathIndex() + idx.Set("marker-intent", markerIntent.GetSensitivePaths()) + + syncTree := buildEmptySyncTree(t, ctx, scb) + ds := &Datastore{ + config: &config.DatastoreConfig{Name: "test-ds", Validation: config.NewValidationConfig()}, + syncTree: syncTree, + syncTreeMutex: &sync.RWMutex{}, + taskPool: pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)), + cacheClient: ccb, + schemaClient: scb, + sensitivePathIndex: idx, + } + + bte, err := ds.BlameConfig(ctx, false, tt.exposeSensitive) + if err != nil { + t.Fatalf("BlameConfig() error: %v", err) + } + + // Walk the blame tree to find "patterntest". + var patterntestValue string + found := false + for _, child := range bte.GetChilds() { + if child.GetName() == "patterntest" { + found = true + patterntestValue = child.GetValue().GetStringVal() + break + } + } + if !found { + t.Fatal("patterntest not found in blame tree") + } + if patterntestValue != tt.wantValue { + t.Errorf("patterntest value = %q, want %q", patterntestValue, tt.wantValue) + } + }) + } +} + +// TestGetIntent_CrossIntentPathsDoNotRedact verifies the scoped semantics for +// GetIntent(regular): another intent's sensitive_paths markers do NOT cause +// redaction in the fetched intent's response. Only the fetched intent's own +// markers apply (see ADR 0004). +// +// Acceptance criteria covered: +// - GetIntent(regular): cross-intent path markers are ignored +// - include_sensitive flag does not change this — the value is already plain +func TestGetIntent_CrossIntentPathsDoNotRedact(t *testing.T) { + ctx := context.Background() + + sc, schema, err := testhelper.InitSDCIOSchema() + if err != nil { + t.Fatal(err) + } + // Plain schema client — patterntest is NOT schema-sensitive. + scb := schemaClient.NewSchemaClientBound(schema, sc) + + // data-intent: holds the actual patterntest value, no sensitive_paths. + dataDevice := &sdcio_schema.Device{Patterntest: ygot.String("secret-value")} + dataIntent := buildTestIntent(t, ctx, scb, dataDevice, "data-intent", 10, nil) + + // marker-intent marks the path sensitive, but data-intent is the one being fetched. + markerSensitivePaths := []*sdcpb.Path{{Elem: []*sdcpb.PathElem{{Name: "patterntest"}}, IsRootBased: true}} + + tests := []struct { + name string + exposeSensitive bool + }{ + { + name: "cross-intent marker does not redact when include_sensitive=false", + exposeSensitive: false, + }, + { + name: "cross-intent marker does not redact when include_sensitive=true", + exposeSensitive: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + ccb := mockcacheclient.NewMockCacheClientBound(ctrl) + // GetIntent loads the specific intent only — no IntentGetAll. + ccb.EXPECT(). + IntentGet(gomock.Any(), "data-intent"). + Return(dataIntent, nil) + + // Index has marker-intent's paths but data-intent is NOT asking for them. + idx := treetypes.NewSensitivePathIndex() + idx.Set("marker-intent", markerSensitivePaths) + + syncTree := buildEmptySyncTree(t, ctx, scb) + ds := &Datastore{ + config: &config.DatastoreConfig{Name: "test-ds", Validation: config.NewValidationConfig()}, + syncTree: syncTree, + syncTreeMutex: &sync.RWMutex{}, + taskPool: pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)), + cacheClient: ccb, + schemaClient: scb, + sensitivePathIndex: idx, + } + + resp, err := ds.GetIntent(ctx, "data-intent", tt.exposeSensitive) + if err != nil { + t.Fatalf("GetIntent() error: %v", err) + } + + updates, err := resp.ToProtoUpdates(ctx) + if err != nil { + t.Fatalf("ToProtoUpdates() error: %v", err) + } + + for _, u := range updates { + elems := u.GetPath().GetElem() + if len(elems) > 0 && elems[len(elems)-1].GetName() == "patterntest" { + got := u.GetValue().GetStringVal() + if got != "secret-value" { + t.Errorf("patterntest value = %q, want %q (cross-intent redaction must not apply)", got, "secret-value") + } + return + } + } + t.Errorf("patterntest update not found in GetIntent response; all updates: %v", updates) + }) + } +} + +// TestGetIntent_OwnSensitivePaths_Redacted verifies that an intent's own +// sensitive_paths markers DO cause redaction in GetIntent for that intent. +func TestGetIntent_OwnSensitivePaths_Redacted(t *testing.T) { + ctx := context.Background() + + sc, schema, err := testhelper.InitSDCIOSchema() + if err != nil { + t.Fatal(err) + } + scb := schemaClient.NewSchemaClientBound(schema, sc) + + // data-intent: holds patterntest value AND marks it sensitive itself. + dataDevice := &sdcio_schema.Device{Patterntest: ygot.String("my-secret")} + ownSensitivePaths := []*sdcpb.Path{{Elem: []*sdcpb.PathElem{{Name: "patterntest"}}, IsRootBased: true}} + dataIntent := buildTestIntent(t, ctx, scb, dataDevice, "data-intent", 10, ownSensitivePaths) + + tests := []struct { + name string + exposeSensitive bool + wantValue string + }{ + { + name: "own sensitive path is redacted when include_sensitive=false", + exposeSensitive: false, + wantValue: "***", + }, + { + name: "own sensitive path is revealed when include_sensitive=true", + exposeSensitive: true, + wantValue: "my-secret", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + ccb := mockcacheclient.NewMockCacheClientBound(ctrl) + ccb.EXPECT(). + IntentGet(gomock.Any(), "data-intent"). + Return(dataIntent, nil) + + syncTree := buildEmptySyncTree(t, ctx, scb) + ds := &Datastore{ + config: &config.DatastoreConfig{Name: "test-ds", Validation: config.NewValidationConfig()}, + syncTree: syncTree, + syncTreeMutex: &sync.RWMutex{}, + taskPool: pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)), + cacheClient: ccb, + schemaClient: scb, + sensitivePathIndex: treetypes.NewSensitivePathIndex(), + } + + resp, err := ds.GetIntent(ctx, "data-intent", tt.exposeSensitive) + if err != nil { + t.Fatalf("GetIntent() error: %v", err) + } + updates, err := resp.ToProtoUpdates(ctx) + if err != nil { + t.Fatalf("ToProtoUpdates() error: %v", err) + } + + for _, u := range updates { + elems := u.GetPath().GetElem() + if len(elems) > 0 && elems[len(elems)-1].GetName() == "patterntest" { + got := u.GetValue().GetStringVal() + if got != tt.wantValue { + t.Errorf("patterntest value = %q, want %q", got, tt.wantValue) + } + return + } + } + t.Errorf("patterntest update not found in GetIntent response") + }) + } +} + +// TestGetIntent_NoSensitivePaths_NoRedaction verifies that an intent with no +// sensitive_paths does not cause redaction via the path-marker mechanism +// (schema-flag redaction is tested separately in the ops package). +func TestGetIntent_NoSensitivePaths_NoRedaction(t *testing.T) { + ctx := context.Background() + + sc, schema, err := testhelper.InitSDCIOSchema() + if err != nil { + t.Fatal(err) + } + scb := schemaClient.NewSchemaClientBound(schema, sc) + + // data-intent: holds patterntest, no sensitive_paths. + dataDevice := &sdcio_schema.Device{Patterntest: ygot.String("plain-value")} + dataIntent := buildTestIntent(t, ctx, scb, dataDevice, "data-intent", 10, nil) + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + ccb := mockcacheclient.NewMockCacheClientBound(ctrl) + ccb.EXPECT(). + IntentGet(gomock.Any(), "data-intent"). + Return(dataIntent, nil) + + syncTree := buildEmptySyncTree(t, ctx, scb) + ds := &Datastore{ + config: &config.DatastoreConfig{Name: "test-ds", Validation: config.NewValidationConfig()}, + syncTree: syncTree, + syncTreeMutex: &sync.RWMutex{}, + taskPool: pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)), + cacheClient: ccb, + schemaClient: scb, + sensitivePathIndex: treetypes.NewSensitivePathIndex(), + } + + resp, err := ds.GetIntent(ctx, "data-intent", false) + if err != nil { + t.Fatalf("GetIntent() error: %v", err) + } + updates, err := resp.ToProtoUpdates(ctx) + if err != nil { + t.Fatalf("ToProtoUpdates() error: %v", err) + } + + for _, u := range updates { + elems := u.GetPath().GetElem() + if len(elems) > 0 && elems[len(elems)-1].GetName() == "patterntest" { + got := u.GetValue().GetStringVal() + if got != "plain-value" { + t.Errorf("patterntest value = %q, want %q (no redaction expected)", got, "plain-value") + } + return + } + } + t.Error("patterntest update not found in GetIntent response") +} + +// TestGetIntent_RunningIntent_CrossIntentRedaction verifies that fetching the +// running intent applies the full cross-intent union from the sensitivePathIndex. +// The running intent has no own markers; any intent's classification must apply +// to prevent device-echoed plaintext values from leaking. +func TestGetIntent_RunningIntent_CrossIntentRedaction(t *testing.T) { + ctx := context.Background() + + sc, schema, err := testhelper.InitSDCIOSchema() + if err != nil { + t.Fatal(err) + } + scb := schemaClient.NewSchemaClientBound(schema, sc) + + // marker-intent marks /patterntest sensitive — pre-loaded into index. + markerSensitivePaths := []*sdcpb.Path{{Elem: []*sdcpb.PathElem{{Name: "patterntest"}}, IsRootBased: true}} + idx := treetypes.NewSensitivePathIndex() + idx.Set("marker-intent", markerSensitivePaths) + + // Build syncTree with a running value for patterntest. + flagsExisting := treetypes.NewUpdateInsertFlags() + tc := tree.NewTreeContext(scb, pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))) + syncRoot, err := tree.NewTreeRoot(ctx, tc) + if err != nil { + t.Fatal(err) + } + runningDevice := &sdcio_schema.Device{Patterntest: ygot.String("running-secret")} + _, err = testhelper.LoadYgotStructIntoTreeRoot(ctx, runningDevice, syncRoot.Entry, consts.RunningIntentName, consts.RunningValuesPrio, false, flagsExisting) + if err != nil { + t.Fatal(err) + } + if err := syncRoot.FinishInsertionPhase(ctx); err != nil { + t.Fatal(err) + } + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // GetIntent(running) does not call IntentGetAll — it reads from the index. + ccb := mockcacheclient.NewMockCacheClientBound(ctrl) + + ds := &Datastore{ + config: &config.DatastoreConfig{Name: "test-ds", Validation: config.NewValidationConfig()}, + syncTree: syncRoot, + syncTreeMutex: &sync.RWMutex{}, + taskPool: pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)), + cacheClient: ccb, + schemaClient: scb, + sensitivePathIndex: idx, + } + + resp, err := ds.GetIntent(ctx, consts.RunningIntentName, false) + if err != nil { + t.Fatalf("GetIntent(running) error: %v", err) + } + updates, err := resp.ToProtoUpdates(ctx) + if err != nil { + t.Fatalf("ToProtoUpdates() error: %v", err) + } + + for _, u := range updates { + elems := u.GetPath().GetElem() + if len(elems) > 0 && elems[len(elems)-1].GetName() == "patterntest" { + got := u.GetValue().GetStringVal() + if got != "***" { + t.Errorf("running intent patterntest = %q, want %q (running value should be redacted)", got, "***") + } + return + } + } + t.Error("patterntest update not found in running intent response") +} diff --git a/pkg/datastore/sync.go b/pkg/datastore/sync.go index 42101ec5..b48f1035 100644 --- a/pkg/datastore/sync.go +++ b/pkg/datastore/sync.go @@ -155,7 +155,7 @@ func (d *Datastore) performRevert(ctx context.Context, t *tree.RootEntry) error // if no deletes, check if we have updates if !performApply { - updList, err := ops.ToProtoUpdates(ctx, t.Entry, true) + updList, err := ops.ToProtoUpdates(ctx, t.Entry, ops.RenderOpts{OnlyNewOrUpdated: true, IncludeSensitive: true}) if err != nil { return err } diff --git a/pkg/datastore/transaction_rpc.go b/pkg/datastore/transaction_rpc.go index d89d55b1..d072f959 100644 --- a/pkg/datastore/transaction_rpc.go +++ b/pkg/datastore/transaction_rpc.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "github.com/sdcio/data-server/pkg/cache" "github.com/sdcio/data-server/pkg/datastore/types" "github.com/sdcio/data-server/pkg/tree" "github.com/sdcio/data-server/pkg/tree/api" @@ -63,6 +64,9 @@ func (d *Datastore) SdcpbTransactionIntentToInternalTI(ctx context.Context, req ti.AddRevertPaths(req.GetRevertPaths()...) } } + + ti.SetSensitivePaths(req.GetSensitivePaths()) + // convert the sdcpb.updates to tree.UpdateSlice Updates, err := treetypes.ExpandAndConvertIntent(ctx, d.schemaClient, req.GetIntent(), req.GetPriority(), req.GetUpdate(), time.Now().Unix()) if err != nil { @@ -159,47 +163,58 @@ func (d *Datastore) replaceIntent(ctx context.Context, transaction *types.Transa return warnings, nil } -func (d *Datastore) LoadAllButRunningIntents(ctx context.Context, root *tree.RootEntry) ([]string, error) { - log := logger.FromContext(ctx) - - intentNames := []string{} - IntentChan := make(chan *tree_persist.Intent) - ErrChan := make(chan error, 1) - - go d.cacheClient.IntentGetAll(ctx, []string{consts.RunningIntentName}, IntentChan, ErrChan) - - for { - selectLoop: +// forEachIntent streams every intent from cc (excluding any names in exclude) +// and calls fn for each one. It returns the first error from fn or from the +// cache stream. Both channels are drained to completion before returning. +func forEachIntent( + ctx context.Context, + cc cache.CacheClientBound, + exclude []string, + fn func(*tree_persist.Intent) error, +) error { + intentChan := make(chan *tree_persist.Intent) + errChan := make(chan error, 1) + go cc.IntentGetAll(ctx, exclude, intentChan, errChan) + for errChan != nil || intentChan != nil { select { - case err, ok := <-ErrChan: + case err, ok := <-errChan: if !ok { - // ErrChan already closed which is fine, continue - ErrChan = nil - break selectLoop + errChan = nil + continue } - return nil, err + return err case <-ctx.Done(): - return nil, fmt.Errorf("context closed while retrieving all intents") - case intent, ok := <-IntentChan: + return fmt.Errorf("context closed while iterating intents") + case intent, ok := <-intentChan: if !ok { - // IntentChan closed due to finish - IntentChan = nil - break selectLoop + intentChan = nil + continue } - log.V(logger.VDebug).Info("adding intent to tree", "intent", intent.GetIntentName()) - log.V(logger.VTrace).Info("adding intent to tree", "intent", intent.GetIntentName(), "content", utils.FormatProtoJSON(intent)) - - intentNames = append(intentNames, intent.GetIntentName()) - protoLoader := treeproto.NewProtoTreeImporter(intent) - _, err := root.ImportConfig(ctx, nil, protoLoader, treetypes.NewUpdateInsertFlags(), d.taskPool) - if err != nil { - return nil, err + if err := fn(intent); err != nil { + return err } } - if ErrChan == nil && IntentChan == nil { - return intentNames, nil - } } + return nil +} + +// LoadAllButRunningIntents streams all non-running intents from cache and imports +// each into root. Returns the list of loaded intent names. +func (d *Datastore) LoadAllButRunningIntents(ctx context.Context, root *tree.RootEntry) ([]string, error) { + log := logger.FromContext(ctx) + var intentNames []string + err := forEachIntent(ctx, d.cacheClient, []string{consts.RunningIntentName}, func(intent *tree_persist.Intent) error { + log.V(logger.VDebug).Info("adding intent to tree", "intent", intent.GetIntentName()) + log.V(logger.VTrace).Info("adding intent to tree", "intent", intent.GetIntentName(), "content", utils.FormatProtoJSON(intent)) + intentNames = append(intentNames, intent.GetIntentName()) + protoLoader := treeproto.NewProtoTreeImporter(intent) + _, err := root.ImportConfig(ctx, nil, protoLoader, treetypes.NewUpdateInsertFlags(), d.taskPool) + return err + }) + if err != nil { + return nil, err + } + return intentNames, nil } // lowlevelTransactionSet @@ -381,15 +396,18 @@ func (d *Datastore) lowlevelTransactionSet(ctx context.Context, transaction *typ if err != nil { log.Error(err, "failed deleting intent from store") } + d.sensitivePathIndex.Delete(intent.GetName()) log.V(logger.VDebug).Info("delete intent from cache") continue case err != nil: return nil, err } + protoIntent.SensitivePaths = intent.GetSensitivePaths() err = d.cacheClient.IntentModify(ctx, protoIntent) if err != nil { return nil, fmt.Errorf("failed updating the intended store for %s: %w", d.Name(), err) } + d.sensitivePathIndex.Set(intent.GetName(), intent.GetSensitivePaths()) } // OPTIMISTIC WRITEBACK TO RUNNING / syncTree diff --git a/pkg/datastore/transaction_rpc_test.go b/pkg/datastore/transaction_rpc_test.go index 26eea6a4..5e35bf07 100644 --- a/pkg/datastore/transaction_rpc_test.go +++ b/pkg/datastore/transaction_rpc_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/openconfig/ygot/ygot" "github.com/sdcio/data-server/mocks/mockcacheclient" "github.com/sdcio/data-server/mocks/mocktarget" @@ -169,13 +170,14 @@ func TestTransactionSet_PreviouslyApplied(t *testing.T) { Validation: config.NewValidationConfig(), Name: "test-ds", }, - syncTreeMutex: &sync.RWMutex{}, - syncTree: syncTreeRoot, // Pre-populated syncTree - taskPool: vpf, - cacheClient: ccb, - sbi: sbi, - dmutex: &sync.Mutex{}, - schemaClient: scb, + syncTreeMutex: &sync.RWMutex{}, + syncTree: syncTreeRoot, // Pre-populated syncTree + taskPool: vpf, + cacheClient: ccb, + sbi: sbi, + dmutex: &sync.Mutex{}, + schemaClient: scb, + sensitivePathIndex: treetypes.NewSensitivePathIndex(), } ds.transactionManager = types.NewTransactionManager(NewDatastoreRollbackAdapter(ds)) @@ -225,3 +227,162 @@ func TestTransactionSet_PreviouslyApplied(t *testing.T) { }) } } + +// TestTransactionSet_SensitivePathsPersisted verifies that sensitive_paths set +// on a TransactionIntent are written to the tree_persist.Intent passed to +// IntentModify (the cache write). This is the tracer bullet for Issue 03. +func TestTransactionSet_SensitivePathsPersisted(t *testing.T) { + ctx := context.Background() + + sc, schema, err := testhelper.InitSDCIOSchema() + if err != nil { + t.Fatal(err) + } + scb := schemaClient.NewSchemaClientBound(schema, sc) + + wantPaths := []*sdcpb.Path{ + {Elem: []*sdcpb.PathElem{{Name: "interface"}, {Name: "description"}}, IsRootBased: true}, + {Elem: []*sdcpb.PathElem{{Name: "bgp"}, {Name: "neighbors"}, {Name: "auth-password"}}, IsRootBased: true}, + } + + // Build a minimal intent payload with one leaf so TreeExport produces + // a non-empty tree_persist.Intent and IntentModify is called (not IntentDelete). + intentDevice := &sdcio_schema.Device{ + Interface: map[string]*sdcio_schema.SdcioModel_Interface{ + "ethernet-1/1": { + Name: ygot.String("ethernet-1/1"), + Description: ygot.String("sensitive-test"), + }, + }, + } + intentJSON, err := ygot.EmitJSON(intentDevice, &ygot.EmitJSONConfig{ + Format: ygot.RFC7951, + SkipValidation: false, + }) + if err != nil { + t.Fatalf("failed to marshal intent: %v", err) + } + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + var capturedIntent *tree_persist.Intent + ccb := mockcacheclient.NewMockCacheClientBound(ctrl) + ccb.EXPECT(). + IntentGetAll(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, excludeIntentNames []string, intentChan chan<- *tree_persist.Intent, errChan chan<- error) { + close(intentChan) + close(errChan) + }).AnyTimes() + ccb.EXPECT(). + IntentModify(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, intent *tree_persist.Intent) error { + if intent.GetIntentName() == "intent-sensitive" { + capturedIntent = intent + } + return nil + }).AnyTimes() + + sbi := mocktarget.NewMockTarget(ctrl) + sbi.EXPECT().Set(gomock.Any(), gomock.Any()).Return(&sdcpb.SetDataResponse{}, nil).AnyTimes() + + vpf := pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)) + tc := tree.NewTreeContext(scb, vpf) + syncTreeRoot, err := tree.NewTreeRoot(ctx, tc) + if err != nil { + t.Fatal(err) + } + err = syncTreeRoot.FinishInsertionPhase(ctx) + if err != nil { + t.Fatal(err) + } + + ds := &Datastore{ + config: &config.DatastoreConfig{ + Validation: config.NewValidationConfig(), + Name: "test-ds", + }, + syncTreeMutex: &sync.RWMutex{}, + syncTree: syncTreeRoot, + taskPool: vpf, + cacheClient: ccb, + sbi: sbi, + dmutex: &sync.Mutex{}, + schemaClient: scb, + sensitivePathIndex: treetypes.NewSensitivePathIndex(), + } + ds.transactionManager = types.NewTransactionManager(NewDatastoreRollbackAdapter(ds)) + + ti := types.NewTransactionIntent("intent-sensitive", 10) + ti.SetSensitivePaths(wantPaths) + + updates, err := treetypes.ExpandAndConvertIntent(ctx, scb, "intent-sensitive", 10, []*sdcpb.Update{{ + Path: &sdcpb.Path{}, + Value: &sdcpb.TypedValue{ + Value: &sdcpb.TypedValue_JsonVal{JsonVal: []byte(intentJSON)}, + }, + }}, time.Now().Unix()) + if err != nil { + t.Fatalf("failed to expand intent: %v", err) + } + ti.AddUpdates(updates) + + _, err = ds.TransactionSet(ctx, "txn-sensitive", []*types.TransactionIntent{ti}, nil, 10*time.Second, false) + if err != nil { + t.Fatalf("TransactionSet failed: %v", err) + } + + if capturedIntent == nil { + t.Fatal("IntentModify was not called for intent-sensitive") + } + gotXPaths := make([]string, 0, len(capturedIntent.GetSensitivePaths())) + for _, p := range capturedIntent.GetSensitivePaths() { + gotXPaths = append(gotXPaths, p.ToXPath(true)) + } + wantXPaths := make([]string, 0, len(wantPaths)) + for _, p := range wantPaths { + wantXPaths = append(wantXPaths, p.ToXPath(true)) + } + if diff := cmp.Diff(wantXPaths, gotXPaths); diff != "" { + t.Errorf("SensitivePaths mismatch (-want +got):\n%s", diff) + } +} + +// TestSdcpbTransactionIntentToInternalTI_SensitivePaths verifies that +// sensitive_paths schema.Path values are passed through to the internal +// TransactionIntent unchanged. +func TestSdcpbTransactionIntentToInternalTI_SensitivePaths(t *testing.T) { + ctx := context.Background() + + sc, schema, err := testhelper.InitSDCIOSchema() + if err != nil { + t.Fatal(err) + } + scb := schemaClient.NewSchemaClientBound(schema, sc) + ds := &Datastore{schemaClient: scb} + + paths := []*sdcpb.Path{ + {Elem: []*sdcpb.PathElem{{Name: "bgp"}, {Name: "neighbors"}, {Name: "auth-password"}}, IsRootBased: true}, + {Elem: []*sdcpb.PathElem{{Name: "interface"}, {Name: "description"}}, IsRootBased: true}, + } + req := &sdcpb.TransactionIntent{ + Intent: "test-intent", + Priority: 10, + Delete: true, + SensitivePaths: paths, + } + + ti, err := ds.SdcpbTransactionIntentToInternalTI(ctx, req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + got := ti.GetSensitivePaths() + if len(got) != len(paths) { + t.Fatalf("got %d sensitive paths, want %d", len(got), len(paths)) + } + for i, p := range paths { + if got[i].ToXPath(true) != p.ToXPath(true) { + t.Errorf("sensitive_paths[%d]: got %q, want %q", i, got[i].ToXPath(true), p.ToXPath(true)) + } + } +} diff --git a/pkg/datastore/types/transaction_intent.go b/pkg/datastore/types/transaction_intent.go index 78137dec..9f582a9f 100644 --- a/pkg/datastore/types/transaction_intent.go +++ b/pkg/datastore/types/transaction_intent.go @@ -19,6 +19,10 @@ type TransactionIntent struct { previouslyApplied bool // revertPaths paths that are meant to be reverted. This is used for non-revertive intents. revertPaths []*sdcpb.Path + // sensitivePaths schema paths (key-free) identifying leaves whose values + // must be redacted in northbound responses. Absence is treated as an empty + // slice (non-sensitive). + sensitivePaths []*sdcpb.Path } func NewTransactionIntent(name string, priority int32) *TransactionIntent { @@ -116,3 +120,11 @@ func (ti *TransactionIntent) GetRevertPaths() []*sdcpb.Path { func (ti *TransactionIntent) AddRevertPaths(p ...*sdcpb.Path) { ti.revertPaths = append(ti.revertPaths, p...) } + +func (ti *TransactionIntent) SetSensitivePaths(paths []*sdcpb.Path) { + ti.sensitivePaths = paths +} + +func (ti *TransactionIntent) GetSensitivePaths() []*sdcpb.Path { + return ti.sensitivePaths +} diff --git a/pkg/server/datastore.go b/pkg/server/datastore.go index 10746556..03222e7b 100644 --- a/pkg/server/datastore.go +++ b/pkg/server/datastore.go @@ -342,7 +342,7 @@ func (s *Server) BlameConfig(ctx context.Context, req *sdcpb.BlameConfigRequest) return nil, err } - tree, err := ds.BlameConfig(ctx, req.GetIncludeDefaults()) + tree, err := ds.BlameConfig(ctx, req.GetIncludeDefaults(), req.GetIncludeSensitive()) if err != nil { return nil, err } diff --git a/pkg/server/intent.go b/pkg/server/intent.go index 45f7c56c..8a8c4ad1 100644 --- a/pkg/server/intent.go +++ b/pkg/server/intent.go @@ -67,7 +67,7 @@ func (s *Server) GetIntent(ctx context.Context, req *sdcpb.GetIntentRequest) (*s return nil, status.Error(codes.NotFound, err.Error()) } - rsp, err := ds.GetIntent(ctx, req.GetIntent()) + rsp, err := ds.GetIntent(ctx, req.GetIntent(), req.GetIncludeSensitive()) if err != nil { return nil, err } diff --git a/pkg/tree/api/adapter/entryoutputadapter.go b/pkg/tree/api/adapter/entryoutputadapter.go index 5222d807..3ed7cb58 100644 --- a/pkg/tree/api/adapter/entryoutputadapter.go +++ b/pkg/tree/api/adapter/entryoutputadapter.go @@ -20,19 +20,28 @@ func NewEntryOutputAdapter(e api.Entry) *EntryOutputAdapter { } func (t *EntryOutputAdapter) ToJson(ctx context.Context, onlyNewOrUpdated bool) (any, error) { - return ops.ToJson(ctx, t.entry, onlyNewOrUpdated) + // Southbound: always send real values to the device, never redact. + return ops.ToJson(ctx, t.entry, ops.RenderOpts{OnlyNewOrUpdated: onlyNewOrUpdated, IncludeSensitive: true}) } func (t *EntryOutputAdapter) ToJsonIETF(ctx context.Context, onlyNewOrUpdated bool) (any, error) { - return ops.ToJsonIETF(ctx, t.entry, onlyNewOrUpdated) + // Southbound: always send real values to the device, never redact. + return ops.ToJsonIETF(ctx, t.entry, ops.RenderOpts{OnlyNewOrUpdated: onlyNewOrUpdated, IncludeSensitive: true}) } func (t *EntryOutputAdapter) ToXML(ctx context.Context, onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool) (*etree.Document, error) { - return ops.ToXML(ctx, t.entry, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove) + // Southbound: always send real values to the device, never redact. + return ops.ToXML(ctx, t.entry, ops.XMLRenderOpts{ + RenderOpts: ops.RenderOpts{OnlyNewOrUpdated: onlyNewOrUpdated, IncludeSensitive: true}, + HonorNamespace: honorNamespace, + OperationWithNamespace: operationWithNamespace, + UseOperationRemove: useOperationRemove, + }) } func (t *EntryOutputAdapter) ToProtoUpdates(ctx context.Context, onlyNewOrUpdated bool) ([]*sdcpb.Update, error) { - return ops.ToProtoUpdates(ctx, t.entry, onlyNewOrUpdated) + // Southbound: always send real values to the device, never redact. + return ops.ToProtoUpdates(ctx, t.entry, ops.RenderOpts{OnlyNewOrUpdated: onlyNewOrUpdated, IncludeSensitive: true}) } func (t *EntryOutputAdapter) ToProtoDeletes(ctx context.Context) ([]*sdcpb.Path, error) { diff --git a/pkg/tree/api/adapter/intentresponse.go b/pkg/tree/api/adapter/intentresponse.go index e221c752..fc8f765c 100644 --- a/pkg/tree/api/adapter/intentresponse.go +++ b/pkg/tree/api/adapter/intentresponse.go @@ -16,6 +16,7 @@ type IntentResponseAdapter struct { Orphan bool NonRevertive bool ExplicitDeletes []*sdcpb.Path + RenderOpts ops.RenderOpts } func (t *IntentResponseAdapter) GetIntentName() string { @@ -39,19 +40,22 @@ func (t *IntentResponseAdapter) GetExplicitDeletes() []*sdcpb.Path { } func (t *IntentResponseAdapter) ToJson(ctx context.Context) (any, error) { - return ops.ToJson(ctx, t.Entry, false) + return ops.ToJson(ctx, t.Entry, t.RenderOpts) } func (t *IntentResponseAdapter) ToJsonIETF(ctx context.Context) (any, error) { - return ops.ToJsonIETF(ctx, t.Entry, false) + return ops.ToJsonIETF(ctx, t.Entry, t.RenderOpts) } func (t *IntentResponseAdapter) ToXML(ctx context.Context) (*etree.Document, error) { - return ops.ToXML(ctx, t.Entry, false, true, false, false) + return ops.ToXML(ctx, t.Entry, ops.XMLRenderOpts{ + RenderOpts: t.RenderOpts, + HonorNamespace: true, + }) } func (t *IntentResponseAdapter) ToProtoUpdates(ctx context.Context) ([]*sdcpb.Update, error) { - return ops.ToProtoUpdates(ctx, t.Entry, false) + return ops.ToProtoUpdates(ctx, t.Entry, t.RenderOpts) } func (t *IntentResponseAdapter) ToProtoDeletes(ctx context.Context) ([]*sdcpb.Path, error) { @@ -59,5 +63,5 @@ func (t *IntentResponseAdapter) ToProtoDeletes(ctx context.Context) ([]*sdcpb.Pa } func (t *IntentResponseAdapter) ToXPath(ctx context.Context) (*sdcpb.PathValues, error) { - return ops.ToXPath(ctx, t.Entry, false, false) + return ops.ToXPath(ctx, t.Entry, ops.XPathRenderOpts{RenderOpts: t.RenderOpts}) } diff --git a/pkg/tree/api/leaf_variants.go b/pkg/tree/api/leaf_variants.go index 8558e30b..958b81f2 100644 --- a/pkg/tree/api/leaf_variants.go +++ b/pkg/tree/api/leaf_variants.go @@ -461,7 +461,8 @@ func (lv *LeafVariants) DeleteByOwner(owner string) *LeafEntry { return nil } -func (lv *LeafVariants) GetDeviations(ctx context.Context, ch chan<- *types.DeviationEntry, isActiveCase bool) { + +func (lv *LeafVariants) GetDeviations(ctx context.Context, ch chan<- *types.DeviationEntry, isActiveCase bool, includeSensitive bool, sps types.SensitivePathChecker) { lv.lesMutex.RLock() defer lv.lesMutex.RUnlock() @@ -473,10 +474,19 @@ func (lv *LeafVariants) GetDeviations(ctx context.Context, ch chan<- *types.Devi // is valid for all entries sdcpbPath := lv.parentEntry.SdcpbPath() + sensitive := types.ShouldRedact(includeSensitive, lv.parentEntry.GetSchema(), sdcpbPath, sps) + + redact := func(v *sdcpb.TypedValue) *sdcpb.TypedValue { + if sensitive { + return types.RedactedTypedValue + } + return v + } + // we are part of an inactive case of a choice if !isActiveCase { for _, le := range lv.les { - ch <- types.NewDeviationEntry(le.Owner(), types.DeviationReasonOverruled, sdcpbPath).SetExpectedValue(le.Value()) + ch <- types.NewDeviationEntry(le.Owner(), types.DeviationReasonOverruled, sdcpbPath).SetExpectedValue(redact(le.Value())) } return } @@ -520,7 +530,7 @@ func (lv *LeafVariants) GetDeviations(ctx context.Context, ch chan<- *types.Devi // skip if higher prio equals the overruled continue } - ch <- de.SetCurrentValue(highest.Value()) + ch <- de.SetExpectedValue(redact(de.ExpectedValue())).SetCurrentValue(redact(highest.Value())) } // if there is no running and no highest (probably a default), skip @@ -530,16 +540,21 @@ func (lv *LeafVariants) GetDeviations(ctx context.Context, ch chan<- *types.Devi // unhandled -> running but no intent data if running != nil && highest == nil { - ch <- types.NewDeviationEntry(running.Owner(), types.DeviationReasonUnhandled, sdcpbPath).SetCurrentValue(running.Value()) + ch <- types.NewDeviationEntry(running.Owner(), types.DeviationReasonUnhandled, sdcpbPath).SetCurrentValue(redact(running.Value())) return } // if highest exists but not running OR running != highest if (running == nil && highest != nil) || !running.Value().Equal(highest.Value()) { - de := types.NewDeviationEntry(highest.Owner(), types.DeviationReasonNotApplied, sdcpbPath).SetExpectedValue(highest.Value()) + var currentVal *sdcpb.TypedValue if running != nil { - de.SetCurrentValue(running.Value()) + currentVal = running.Value() } + // When sensitive and running is absent, still emit a redacted CurrentValue + // so that the absence of a device value is not exposed. + de := types.NewDeviationEntry(highest.Owner(), types.DeviationReasonNotApplied, sdcpbPath). + SetExpectedValue(redact(highest.Value())). + SetCurrentValue(redact(currentVal)) ch <- de } diff --git a/pkg/tree/entry_test.go b/pkg/tree/entry_test.go index 2bef7ab3..56249f0e 100644 --- a/pkg/tree/entry_test.go +++ b/pkg/tree/entry_test.go @@ -2254,7 +2254,7 @@ func Test_RevertNonRevertive(t *testing.T) { } t.Logf("Deletes: %v", deletes) - j, err := ops.ToJson(ctx, root.Entry, true) + j, err := ops.ToJson(ctx, root.Entry, ops.RenderOpts{OnlyNewOrUpdated: true}) if err != nil { t.Fatalf("failed to convert to JSON: %v", err) } diff --git a/pkg/tree/importer/xml/xml_tree_importer_test.go b/pkg/tree/importer/xml/xml_tree_importer_test.go index 5b396913..bb7ecb0c 100644 --- a/pkg/tree/importer/xml/xml_tree_importer_test.go +++ b/pkg/tree/importer/xml/xml_tree_importer_test.go @@ -118,7 +118,7 @@ func TestXmlTreeImporter(t *testing.T) { t.Error(err) } - result, err := ops.ToXML(ctx, root.Entry, false, false, false, false) + result, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{}) if err != nil { t.Fatal(err) } @@ -200,7 +200,7 @@ func TestXmlTreeImporterElement_IdentityRef(t *testing.T) { t.Log(root.String()) - result, err := ops.ToXML(ctx, root.Entry, false, false, false, false) + result, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{}) if err != nil { t.Fatal(err) } diff --git a/pkg/tree/ops/getdeviations.go b/pkg/tree/ops/getdeviations.go index d10fee06..2ce2d0aa 100644 --- a/pkg/tree/ops/getdeviations.go +++ b/pkg/tree/ops/getdeviations.go @@ -9,8 +9,10 @@ import ( "github.com/sdcio/data-server/pkg/tree/types" ) + type GetDeviationParams struct { Ch chan<- *types.DeviationEntry + RenderOpts } func GetDeviations(ctx context.Context, e api.Entry, config *GetDeviationParams, poolFactory pool.VirtualPoolFactory) error { @@ -58,7 +60,7 @@ func (dt *deviationTask) Run(ctx context.Context, submit func(pool.Task) error) if evalLeafvariants { // calculate Deviation on the LeafVariants - dt.entry.GetLeafVariants().GetDeviations(ctx, dt.config.Ch, dt.isActiveCase) + dt.entry.GetLeafVariants().GetDeviations(ctx, dt.config.Ch, dt.isActiveCase, dt.config.IncludeSensitive, dt.config.SensitivePathSet) } // iterate through all childs diff --git a/pkg/tree/ops/getdeviations_test.go b/pkg/tree/ops/getdeviations_test.go new file mode 100644 index 00000000..494bbaee --- /dev/null +++ b/pkg/tree/ops/getdeviations_test.go @@ -0,0 +1,233 @@ +package ops_test + +import ( + "context" + "runtime" + "testing" + + mockTreeEntry "github.com/sdcio/data-server/mocks/mocktreeentry" + "github.com/sdcio/data-server/pkg/pool" + "github.com/sdcio/data-server/pkg/tree/api" + schemaClient "github.com/sdcio/data-server/pkg/datastore/clients/schema" + . "github.com/sdcio/data-server/pkg/tree/consts" + "github.com/sdcio/data-server/pkg/tree/ops" + "github.com/sdcio/data-server/pkg/tree/types" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" + "go.uber.org/mock/gomock" +) + +// fakeTreeContext is a minimal api.TreeContext for GetDeviations tests. +type fakeTreeContext struct{} + +func (f *fakeTreeContext) PoolFactory() pool.VirtualPoolFactory { return nil } +func (f *fakeTreeContext) SchemaClient() schemaClient.SchemaClientBound { return nil } +func (f *fakeTreeContext) DeepCopy() api.TreeContext { return f } +func (f *fakeTreeContext) ExplicitDeletes() *api.DeletePathSet { return nil } +func (f *fakeTreeContext) NonRevertiveInfo() api.NonRevertiveInfos { return nil } + +// leafDeviationEntry builds a LeafVariants with one intent entry (value expectedVal) +// and one running entry (value currentVal) attached to parentEntry. +func leafDeviationEntry(tc api.TreeContext, parentEntry api.Entry, expectedVal, currentVal string) *api.LeafVariants { + lv := api.NewLeafVariants(tc, parentEntry) + + intentVal := &sdcpb.TypedValue{Value: &sdcpb.TypedValue_StringVal{StringVal: expectedVal}} + lv.Add(api.NewLeafEntry( + types.NewUpdate(parentEntry, intentVal, 10, "test-intent", 0), + types.NewUpdateInsertFlags(), + parentEntry, + )) + + runningVal := &sdcpb.TypedValue{Value: &sdcpb.TypedValue_StringVal{StringVal: currentVal}} + lv.Add(api.NewLeafEntry( + types.NewUpdate(parentEntry, runningVal, RunningValuesPrio, RunningIntentName, 0), + types.NewUpdateInsertFlags(), + parentEntry, + )) + + return lv +} + +// collectDeviations runs ops.GetDeviations and returns all emitted DeviationEntries. +func collectDeviations(t *testing.T, entry api.Entry, params *ops.GetDeviationParams) []*types.DeviationEntry { + t.Helper() + taskPool := pool.NewSharedTaskPool(context.Background(), runtime.GOMAXPROCS(0)) + ch := make(chan *types.DeviationEntry, 32) + params.Ch = ch + + err := ops.GetDeviations(context.Background(), entry, params, taskPool) + close(ch) + if err != nil { + t.Fatalf("GetDeviations error: %v", err) + } + + var result []*types.DeviationEntry + for de := range ch { + result = append(result, de) + } + return result +} + +// setupLeafMock wires a MockEntry to behave as a single leaf node (no children) +// with the given schema and leaf variants. +func setupLeafMock(ctrl *gomock.Controller, schema *sdcpb.SchemaElem, tc api.TreeContext, lv *api.LeafVariants) *mockTreeEntry.MockEntry { + e := mockTreeEntry.NewMockEntry(ctrl) + path := &sdcpb.Path{Elem: []*sdcpb.PathElem{{Name: "test-leaf"}}, IsRootBased: true} + e.EXPECT().SdcpbPath().Return(path).AnyTimes() + e.EXPECT().GetSchema().Return(schema).AnyTimes() + e.EXPECT().GetChilds(types.DescendMethodActiveChilds).Return(api.EntryMap{}).AnyTimes() + e.EXPECT().GetChildMap().Return(api.NewChildMap()).AnyTimes() + e.EXPECT().GetLeafVariants().Return(lv).AnyTimes() + return e +} + +// TestGetDeviations_SchemaSensitiveRedacted is the tracer bullet. +// IncludeSensitive=false must replace both expected and current values with "***" +// when the leaf's schema marks it sensitive. +func TestGetDeviations_SchemaSensitiveRedacted(t *testing.T) { + ctrl := gomock.NewController(t) + + sensitiveSchema := &sdcpb.SchemaElem{ + Schema: &sdcpb.SchemaElem_Field{Field: &sdcpb.LeafSchema{Sensitive: true}}, + } + tc := &fakeTreeContext{} + + e := mockTreeEntry.NewMockEntry(ctrl) + path := &sdcpb.Path{Elem: []*sdcpb.PathElem{{Name: "secret-leaf"}}, IsRootBased: true} + e.EXPECT().SdcpbPath().Return(path).AnyTimes() + e.EXPECT().GetSchema().Return(sensitiveSchema).AnyTimes() + e.EXPECT().GetChilds(types.DescendMethodActiveChilds).Return(api.EntryMap{}).AnyTimes() + e.EXPECT().GetChildMap().Return(api.NewChildMap()).AnyTimes() + + lv := leafDeviationEntry(tc, e, "secret-expected", "secret-current") + e.EXPECT().GetLeafVariants().Return(lv).AnyTimes() + + deviations := collectDeviations(t, e, &ops.GetDeviationParams{RenderOpts: ops.RenderOpts{IncludeSensitive: false}}) + + if len(deviations) == 0 { + t.Fatal("expected at least one deviation, got none") + } + for _, de := range deviations { + if ev := de.ExpectedValue(); ev != nil { + if got := ev.GetStringVal(); got != "***" { + t.Errorf("ExpectedValue = %q, want %q", got, "***") + } + } + if cv := de.CurrentValue(); cv != nil { + if got := cv.GetStringVal(); got != "***" { + t.Errorf("CurrentValue = %q, want %q", got, "***") + } + } + } +} + +// TestGetDeviations_ExposeSensitivePassthrough verifies that IncludeSensitive=true +// leaves schema-sensitive values untouched. +func TestGetDeviations_ExposeSensitivePassthrough(t *testing.T) { + ctrl := gomock.NewController(t) + + sensitiveSchema := &sdcpb.SchemaElem{ + Schema: &sdcpb.SchemaElem_Field{Field: &sdcpb.LeafSchema{Sensitive: true}}, + } + tc := &fakeTreeContext{} + + e := mockTreeEntry.NewMockEntry(ctrl) + path := &sdcpb.Path{Elem: []*sdcpb.PathElem{{Name: "secret-leaf"}}, IsRootBased: true} + e.EXPECT().SdcpbPath().Return(path).AnyTimes() + e.EXPECT().GetSchema().Return(sensitiveSchema).AnyTimes() + e.EXPECT().GetChilds(types.DescendMethodActiveChilds).Return(api.EntryMap{}).AnyTimes() + e.EXPECT().GetChildMap().Return(api.NewChildMap()).AnyTimes() + + lv := leafDeviationEntry(tc, e, "raw-expected", "raw-current") + e.EXPECT().GetLeafVariants().Return(lv).AnyTimes() + + deviations := collectDeviations(t, e, &ops.GetDeviationParams{RenderOpts: ops.RenderOpts{IncludeSensitive: true}}) + + if len(deviations) == 0 { + t.Fatal("expected at least one deviation, got none") + } + de := deviations[0] + if got := de.ExpectedValue().GetStringVal(); got != "raw-expected" { + t.Errorf("ExpectedValue = %q, want %q (must not be redacted)", got, "raw-expected") + } + if got := de.CurrentValue().GetStringVal(); got != "raw-current" { + t.Errorf("CurrentValue = %q, want %q (must not be redacted)", got, "raw-current") + } +} + +// TestGetDeviations_IntentMarkerSensitiveRedacted verifies that IncludeSensitive=false +// redacts values when the path appears in GetDeviationParams.SensitivePathSet +// (intent-marker check), even when the schema does not mark it sensitive. +func TestGetDeviations_IntentMarkerSensitiveRedacted(t *testing.T) { + ctrl := gomock.NewController(t) + + nonSensitiveSchema := &sdcpb.SchemaElem{ + Schema: &sdcpb.SchemaElem_Field{Field: &sdcpb.LeafSchema{Sensitive: false}}, + } + leafPath := &sdcpb.Path{Elem: []*sdcpb.PathElem{{Name: "marker-leaf"}}, IsRootBased: true} + + sps := types.NewSensitivePathIndex() + sps.Add(leafPath) + tc := &fakeTreeContext{} + + e := mockTreeEntry.NewMockEntry(ctrl) + e.EXPECT().SdcpbPath().Return(leafPath).AnyTimes() + e.EXPECT().GetSchema().Return(nonSensitiveSchema).AnyTimes() + e.EXPECT().GetChilds(types.DescendMethodActiveChilds).Return(api.EntryMap{}).AnyTimes() + e.EXPECT().GetChildMap().Return(api.NewChildMap()).AnyTimes() + + lv := leafDeviationEntry(tc, e, "marker-expected", "marker-current") + e.EXPECT().GetLeafVariants().Return(lv).AnyTimes() + + deviations := collectDeviations(t, e, &ops.GetDeviationParams{RenderOpts: ops.RenderOpts{IncludeSensitive: false, SensitivePathSet: sps}}) + + if len(deviations) == 0 { + t.Fatal("expected at least one deviation, got none") + } + for _, de := range deviations { + if ev := de.ExpectedValue(); ev != nil { + if got := ev.GetStringVal(); got != "***" { + t.Errorf("ExpectedValue = %q, want %q (path-marker sensitive)", got, "***") + } + } + if cv := de.CurrentValue(); cv != nil { + if got := cv.GetStringVal(); got != "***" { + t.Errorf("CurrentValue = %q, want %q (path-marker sensitive)", got, "***") + } + } + } +} + +// TestGetDeviations_NonSensitiveNotRedacted verifies that IncludeSensitive=false +// does NOT redact values for paths that are neither schema-sensitive nor in +// GetDeviationParams.SensitivePathSet. +func TestGetDeviations_NonSensitiveNotRedacted(t *testing.T) { + ctrl := gomock.NewController(t) + + nonSensitiveSchema := &sdcpb.SchemaElem{ + Schema: &sdcpb.SchemaElem_Field{Field: &sdcpb.LeafSchema{Sensitive: false}}, + } + tc := &fakeTreeContext{} + + e := mockTreeEntry.NewMockEntry(ctrl) + path := &sdcpb.Path{Elem: []*sdcpb.PathElem{{Name: "plain-leaf"}}, IsRootBased: true} + e.EXPECT().SdcpbPath().Return(path).AnyTimes() + e.EXPECT().GetSchema().Return(nonSensitiveSchema).AnyTimes() + e.EXPECT().GetChilds(types.DescendMethodActiveChilds).Return(api.EntryMap{}).AnyTimes() + e.EXPECT().GetChildMap().Return(api.NewChildMap()).AnyTimes() + + lv := leafDeviationEntry(tc, e, "plain-expected", "plain-current") + e.EXPECT().GetLeafVariants().Return(lv).AnyTimes() + + deviations := collectDeviations(t, e, &ops.GetDeviationParams{RenderOpts: ops.RenderOpts{IncludeSensitive: false}}) + + if len(deviations) == 0 { + t.Fatal("expected at least one deviation, got none") + } + de := deviations[0] + if got := de.ExpectedValue().GetStringVal(); got != "plain-expected" { + t.Errorf("ExpectedValue = %q, want %q (must not be redacted)", got, "plain-expected") + } + if got := de.CurrentValue().GetStringVal(); got != "plain-current" { + t.Errorf("CurrentValue = %q, want %q (must not be redacted)", got, "plain-current") + } +} diff --git a/pkg/tree/ops/json.go b/pkg/tree/ops/json.go index 14ba8357..38198257 100644 --- a/pkg/tree/ops/json.go +++ b/pkg/tree/ops/json.go @@ -11,27 +11,25 @@ import ( sdcpb "github.com/sdcio/sdc-protos/sdcpb" ) -func ToJson(ctx context.Context, e api.Entry, onlyNewOrUpdated bool) (any, error) { - result, err := toJsonInternal(ctx, e, onlyNewOrUpdated, false) +func ToJson(ctx context.Context, e api.Entry, opts RenderOpts) (any, error) { + result, err := toJsonInternal(ctx, e, opts, false) if err != nil { return nil, err } return result, err } -func ToJsonIETF(ctx context.Context, e api.Entry, onlyNewOrUpdated bool) (any, error) { - result, err := toJsonInternal(ctx, e, onlyNewOrUpdated, true) +func ToJsonIETF(ctx context.Context, e api.Entry, opts RenderOpts) (any, error) { + result, err := toJsonInternal(ctx, e, opts, true) if err != nil { return nil, err } return result, err } -// ToJson returns the Branch of the tree as a struct that can be marshalled as JSON +// toJsonInternal returns the Branch of the tree as a struct that can be marshalled as JSON. // If the ietf parameter is set to true, JSON_IETF encoding is used. -// The actualPrefix is used only for the JSON_IETF encoding and can be ignored for JSON -// In the initial / users call with ietf == true, actualPrefix should be set to "" -func toJsonInternal(ctx context.Context, e api.Entry, onlyNewOrUpdated bool, ietf bool) (any, error) { +func toJsonInternal(ctx context.Context, e api.Entry, opts RenderOpts, ietf bool) (any, error) { switch e.GetSchema().GetSchema().(type) { case nil: // we're operating on a key level, no schema attached, but the @@ -42,7 +40,7 @@ func toJsonInternal(ctx context.Context, e api.Entry, onlyNewOrUpdated bool, iet ancest, _ := GetFirstAncestorWithSchema(e) prefixedKey := jsonGetIetfPrefixConditional(key, c, ancest, ietf) // recurse the call - js, err := toJsonInternal(ctx, c, onlyNewOrUpdated, ietf) + js, err := toJsonInternal(ctx, c, opts, ietf) if err != nil { return nil, err } @@ -70,7 +68,7 @@ func toJsonInternal(ctx context.Context, e api.Entry, onlyNewOrUpdated bool, iet result := make([]any, 0, len(childs)) for _, c := range childs { - j, err := toJsonInternal(ctx, c, onlyNewOrUpdated, ietf) + j, err := toJsonInternal(ctx, c, opts, ietf) if err != nil { return nil, err } @@ -84,13 +82,13 @@ func toJsonInternal(ctx context.Context, e api.Entry, onlyNewOrUpdated bool, iet return result, nil case e.GetSchema().GetContainer().IsPresence && ContainsOnlyDefaults(e): // Presence container without any childs - if onlyNewOrUpdated { + if opts.OnlyNewOrUpdated { // presence containers have leafvariantes with typedValue_Empty, so check that if e.GetLeafVariants().ShouldDelete() { return nil, nil } le := e.GetLeafVariants().GetHighestPrecedence(false, false, false) - if le == nil || onlyNewOrUpdated && !(le.IsNew || le.IsUpdated) { + if le == nil || opts.OnlyNewOrUpdated && !(le.IsNew || le.IsUpdated) { return nil, nil } } @@ -100,7 +98,7 @@ func toJsonInternal(ctx context.Context, e api.Entry, onlyNewOrUpdated bool, iet result := map[string]any{} for key, c := range e.GetChilds(types.DescendMethodActiveChilds) { prefixedKey := jsonGetIetfPrefixConditional(key, c, e, ietf) - js, err := toJsonInternal(ctx, c, onlyNewOrUpdated, ietf) + js, err := toJsonInternal(ctx, c, opts, ietf) if err != nil { return nil, err } @@ -118,10 +116,13 @@ func toJsonInternal(ctx context.Context, e api.Entry, onlyNewOrUpdated bool, iet if e.GetLeafVariants().CanDelete() { return nil, nil } - le := e.GetLeafVariants().GetHighestPrecedence(onlyNewOrUpdated, false, false) + le := e.GetLeafVariants().GetHighestPrecedence(opts.OnlyNewOrUpdated, false, false) if le == nil { return nil, nil } + if ShouldRedact(e, opts.IncludeSensitive, opts.SensitivePathSet) { + return types.RedactedStringValue, nil + } return utils.GetJsonValue(le.Value(), ietf) } return nil, fmt.Errorf("unable to convert to json (%s)", e.SdcpbPath().ToXPath(false)) diff --git a/pkg/tree/ops/json_test.go b/pkg/tree/ops/json_test.go index 84d677d4..8a5526df 100644 --- a/pkg/tree/ops/json_test.go +++ b/pkg/tree/ops/json_test.go @@ -9,7 +9,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/openconfig/ygot/ygot" + "github.com/sdcio/data-server/mocks/mockschemaclientbound" "github.com/sdcio/data-server/pkg/pool" + "github.com/sdcio/data-server/pkg/tree" "github.com/sdcio/data-server/pkg/tree/consts" "github.com/sdcio/data-server/pkg/tree/ops" @@ -21,6 +23,18 @@ import ( "go.uber.org/mock/gomock" ) +// newSchemaClientMarkingSensitive returns a mock schema client that behaves like +// the standard test schema client but marks the leaf named sensitiveLeafName as +// Sensitive=true in its returned schema. +func newSchemaClientMarkingSensitive(t *testing.T, ctrl *gomock.Controller, sensitiveLeafName string) *mockschemaclientbound.MockSchemaClientBound { + t.Helper() + scb, err := testhelper.GetSchemaClientBoundMarkingLeafSensitive(t, ctrl, sensitiveLeafName) + if err != nil { + t.Fatal(err) + } + return scb +} + func TestToJsonTable(t *testing.T) { var tests = []struct { @@ -431,13 +445,14 @@ func TestToJsonTable(t *testing.T) { var jsonStruct any + renderOpts := ops.RenderOpts{OnlyNewOrUpdated: tt.onlyNewOrUpdated} if tt.ietf { - jsonStruct, err = ops.ToJsonIETF(ctx, root.Entry, tt.onlyNewOrUpdated) + jsonStruct, err = ops.ToJsonIETF(ctx, root.Entry, renderOpts) if err != nil { t.Fatal(err) } } else { - jsonStruct, err = ops.ToJson(ctx, root.Entry, tt.onlyNewOrUpdated) + jsonStruct, err = ops.ToJson(ctx, root.Entry, renderOpts) if err != nil { t.Fatal(err) } @@ -462,3 +477,75 @@ func TestToJsonTable(t *testing.T) { }) } } + +func TestToJsonSensitiveRedaction(t *testing.T) { + flagsOld := types.NewUpdateInsertFlags() + + tests := []struct { + name string + exposeSensitive bool + wantPatterntest string + }{ + { + name: "redacts sensitive leaf when IncludeSensitive=false", + exposeSensitive: false, + wantPatterntest: "***", + }, + { + name: "reveals sensitive leaf when IncludeSensitive=true", + exposeSensitive: true, + wantPatterntest: "hallo 00", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + scb := newSchemaClientMarkingSensitive(t, ctrl, "patterntest") + + ctx := context.Background() + tc := tree.NewTreeContext(scb, pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))) + root, err := tree.NewTreeRoot(ctx, tc) + if err != nil { + t.Fatal(err) + } + converter := utils.NewConverter(scb) + + upds, err := testhelper.ExpandUpdateFromConfig(ctx, testhelper.Config1(), converter) + if err != nil { + t.Fatal(err) + } + if err := testhelper.AddToRoot(ctx, root.Entry, upds, flagsOld, "owner1", 5); err != nil { + t.Fatal(err) + } + if err := root.FinishInsertionPhase(ctx); err != nil { + t.Fatal(err) + } + + jsonStruct, err := ops.ToJson(ctx, root.Entry, ops.RenderOpts{IncludeSensitive: tt.exposeSensitive}) + if err != nil { + t.Fatal(err) + } + + jsonBytes, err := json.Marshal(jsonStruct) + if err != nil { + t.Fatal(err) + } + + var result map[string]any + if err := json.Unmarshal(jsonBytes, &result); err != nil { + t.Fatal(err) + } + + got, ok := result["patterntest"].(string) + if !ok { + t.Fatalf("patterntest not found or not a string in JSON output: %s", string(jsonBytes)) + } + if diff := cmp.Diff(tt.wantPatterntest, got); diff != "" { + t.Errorf("patterntest mismatch (-want +got):\n%s", diff) + } + + ctrl.Finish() + }) + } +} diff --git a/pkg/tree/ops/proto.go b/pkg/tree/ops/proto.go index e840cff7..fe18789c 100644 --- a/pkg/tree/ops/proto.go +++ b/pkg/tree/ops/proto.go @@ -4,13 +4,21 @@ import ( "context" "github.com/sdcio/data-server/pkg/tree/api" + "github.com/sdcio/data-server/pkg/tree/types" sdcpb "github.com/sdcio/sdc-protos/sdcpb" ) -func ToProtoUpdates(ctx context.Context, e api.Entry, onlyNewOrUpdated bool) ([]*sdcpb.Update, error) { - result := api.LeafVariantSlice{} - result = GetHighestPrecedence(e, onlyNewOrUpdated, false, true) - return result.ToSdcpbUpdateSlice(), nil +func ToProtoUpdates(ctx context.Context, e api.Entry, opts RenderOpts) ([]*sdcpb.Update, error) { + lvs := GetHighestPrecedence(e, opts.OnlyNewOrUpdated, false, true) + result := make([]*sdcpb.Update, 0, len(lvs)) + for _, lv := range lvs { + if !opts.IncludeSensitive && ShouldRedact(lv.GetEntry(), false, opts.SensitivePathSet) { + result = append(result, &sdcpb.Update{Path: lv.GetEntry().SdcpbPath(), Value: types.RedactedTypedValue}) + } else { + result = append(result, lv.ToSdcpbUpdate()) + } + } + return result, nil } func ToProtoDeletes(ctx context.Context, e api.Entry) ([]*sdcpb.Path, error) { diff --git a/pkg/tree/ops/render_opts.go b/pkg/tree/ops/render_opts.go new file mode 100644 index 00000000..df05c3fc --- /dev/null +++ b/pkg/tree/ops/render_opts.go @@ -0,0 +1,38 @@ +package ops + +import ( + "github.com/sdcio/data-server/pkg/tree/api" + "github.com/sdcio/data-server/pkg/tree/types" +) + +// ShouldRedact reports whether the value at e must be replaced with the +// redaction sentinel. Delegates to types.ShouldRedact with the entry's +// schema-sensitivity flag and path pre-computed. +func ShouldRedact(e api.Entry, includeSensitive bool, sps types.SensitivePathChecker) bool { + return types.ShouldRedact(includeSensitive, e.GetSchema(), e.SdcpbPath(), sps) +} + +// RenderOpts holds common options shared across all northbound render operations. +type RenderOpts struct { + OnlyNewOrUpdated bool + IncludeSensitive bool + // SensitivePathSet is the union of sensitive_paths from all in-scope intents. + // A nil value means no path-marker-based redaction is applied. + // Accepts any types.SensitivePathChecker — e.g. a *types.SensitivePathIndex + // in snapshot mode (Add) or the live datastore index (Set/Delete). + SensitivePathSet types.SensitivePathChecker +} + +// XMLRenderOpts extends RenderOpts with XML-specific flags. +type XMLRenderOpts struct { + RenderOpts + HonorNamespace bool + OperationWithNamespace bool + UseOperationRemove bool +} + +// XPathRenderOpts extends RenderOpts with XPath-specific flags. +type XPathRenderOpts struct { + RenderOpts + IncludeDefaults bool +} diff --git a/pkg/tree/ops/sensitive_test.go b/pkg/tree/ops/sensitive_test.go new file mode 100644 index 00000000..1434aa79 --- /dev/null +++ b/pkg/tree/ops/sensitive_test.go @@ -0,0 +1,98 @@ +package ops_test + +import ( + "testing" + + mockTreeEntry "github.com/sdcio/data-server/mocks/mocktreeentry" + "github.com/sdcio/data-server/pkg/tree/ops" + "github.com/sdcio/data-server/pkg/tree/types" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" + "go.uber.org/mock/gomock" +) + +// alwaysContains is a SensitivePathChecker that treats every path as sensitive. +type alwaysContains struct{} + +func (alwaysContains) Contains(*sdcpb.Path) bool { return true } + +func TestShouldRedact(t *testing.T) { + somePath := &sdcpb.Path{Elem: []*sdcpb.PathElem{{Name: "x"}}} + + sensitiveLeaf := &sdcpb.SchemaElem{ + Schema: &sdcpb.SchemaElem_Field{Field: &sdcpb.LeafSchema{Sensitive: true}}, + } + plainLeaf := &sdcpb.SchemaElem{ + Schema: &sdcpb.SchemaElem_Field{Field: &sdcpb.LeafSchema{Sensitive: false}}, + } + + tests := []struct { + name string + schema *sdcpb.SchemaElem + includeSensitive bool + sps types.SensitivePathChecker + want bool + }{ + { + name: "schema-sensitive leaf, IncludeSensitive=false → redact", + schema: sensitiveLeaf, + want: true, + }, + { + name: "schema-sensitive leaf, IncludeSensitive=true → reveal", + schema: sensitiveLeaf, + includeSensitive: true, + want: false, + }, + { + name: "non-sensitive leaf → pass through", + schema: plainLeaf, + want: false, + }, + { + name: "sensitive leaf-list → redact", + schema: &sdcpb.SchemaElem{ + Schema: &sdcpb.SchemaElem_Leaflist{Leaflist: &sdcpb.LeafListSchema{Sensitive: true}}, + }, + want: true, + }, + { + name: "container → pass through", + schema: &sdcpb.SchemaElem{ + Schema: &sdcpb.SchemaElem_Container{Container: &sdcpb.ContainerSchema{}}, + }, + want: false, + }, + { + name: "nil schema → pass through", + schema: nil, + want: false, + }, + { + name: "path in SensitivePathSet → redact", + schema: plainLeaf, + sps: alwaysContains{}, + want: true, + }, + { + name: "path in SensitivePathSet, IncludeSensitive=true → reveal", + schema: plainLeaf, + sps: alwaysContains{}, + includeSensitive: true, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + e := mockTreeEntry.NewMockEntry(ctrl) + e.EXPECT().GetSchema().Return(tt.schema) + e.EXPECT().SdcpbPath().Return(somePath) + + got := ops.ShouldRedact(e, tt.includeSensitive, tt.sps) + if got != tt.want { + t.Errorf("ShouldRedact() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/tree/ops/xml.go b/pkg/tree/ops/xml.go index d1d2c35e..066d7b9b 100644 --- a/pkg/tree/ops/xml.go +++ b/pkg/tree/ops/xml.go @@ -14,20 +14,20 @@ import ( sdcpb "github.com/sdcio/sdc-protos/sdcpb" ) -// ToXML yields the xml representation of the tree. Either updates only (onlyNewOrUpdated flag) or the actual view on the whole tree. -// If honorNamespace is set, the xml elements will carry their respective namespace attributes. -// If operationWithNamespace is set, the operation attributes added to the to be deleted alements will also carry the Netconf Base namespace. -// If useOperationRemove is set, the remove operation will be used for deletes, instead of the delete operation. -func ToXML(ctx context.Context, e api.Entry, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove bool) (*etree.Document, error) { +// ToXML yields the xml representation of the tree. Either updates only (OnlyNewOrUpdated flag) or the actual view on the whole tree. +// If HonorNamespace is set, the xml elements will carry their respective namespace attributes. +// If OperationWithNamespace is set, the operation attributes added to the to be deleted elements will also carry the Netconf Base namespace. +// If UseOperationRemove is set, the remove operation will be used for deletes, instead of the delete operation. +func ToXML(ctx context.Context, e api.Entry, opts XMLRenderOpts) (*etree.Document, error) { doc := etree.NewDocument() - _, err := toXmlInternal(ctx, e, &doc.Element, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove) + _, err := toXmlInternal(ctx, e, &doc.Element, opts) if err != nil { return nil, err } return doc, nil } -func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool) (doAdd bool, err error) { +func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, opts XMLRenderOpts) (doAdd bool, err error) { switch e.GetSchema().GetSchema().(type) { case nil: @@ -35,7 +35,7 @@ func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, only if e.ShouldDelete() { // If the element is to be deleted // add the delete operation to the parent element - utils.AddXMLOperation(parent, utils.XMLOperationDelete, operationWithNamespace, useOperationRemove) + utils.AddXMLOperation(parent, utils.XMLOperationDelete, opts.OperationWithNamespace, opts.UseOperationRemove) // retrieve the parent schema, we need to extract the key names // values are the tree level names xmlAddKeyElements(e, parent) @@ -78,7 +78,7 @@ func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, only for _, k := range keys { // recurse the call // no additional element is created, since we're on a key level, so add to parent element - doAdd, err := toXmlInternal(ctx, childs[k], parent, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove) + doAdd, err := toXmlInternal(ctx, childs[k], parent, opts) if err != nil { return false, err } @@ -107,9 +107,9 @@ func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, only // create the element for the child, that in the recursed call will appear as parent newElem := etree.NewElement(e.PathName()) // process the honorNamespace instruction - xmlAddNamespaceConditional(e, e.GetParent(), newElem, honorNamespace) + xmlAddNamespaceConditional(e, e.GetParent(), newElem, opts.HonorNamespace) // recurse the call - doAdd, err := toXmlInternal(ctx, child, newElem, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove) + doAdd, err := toXmlInternal(ctx, child, newElem, opts) if err != nil { return false, err } @@ -120,7 +120,7 @@ func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, only replaceAttr := newElem.SelectAttr("nc:operation") if replaceAttr != nil && replaceAttr.Value == string(utils.XMLOperationReplace) { // If we are replacing, we need to have all child values added to the xml tree else they will be removed from the device - err := xmlAddAllChildValues(ctx, child, newElem, honorNamespace, operationWithNamespace, useOperationRemove) + err := xmlAddAllChildValues(ctx, child, newElem, opts) if err != nil { return false, err } @@ -137,25 +137,25 @@ func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, only // if delete, create the element as child of parent newElem := parent.CreateElement(e.PathName()) // add namespace if we create doc with namespace and the actual namespace differs from the parent namespace - xmlAddNamespaceConditional(e, e.GetParent(), newElem, honorNamespace) + xmlAddNamespaceConditional(e, e.GetParent(), newElem, opts.HonorNamespace) // add the delete / remove operation - utils.AddXMLOperation(newElem, utils.XMLOperationDelete, operationWithNamespace, useOperationRemove) + utils.AddXMLOperation(newElem, utils.XMLOperationDelete, opts.OperationWithNamespace, opts.UseOperationRemove) return true, nil case e.GetSchema().GetContainer().IsPresence && ContainsOnlyDefaults(e): // process presence containers with no childs - if onlyNewOrUpdated { + if opts.OnlyNewOrUpdated { // presence containers have leafvariantes with typedValue_Empty, so check that if e.GetLeafVariants().ShouldDelete() { return false, nil } le := e.GetLeafVariants().GetHighestPrecedence(false, false, false) - if le == nil || onlyNewOrUpdated && !(le.IsNew || le.IsUpdated) { + if le == nil || opts.OnlyNewOrUpdated && !(le.IsNew || le.IsUpdated) { return false, nil } } newElem := parent.CreateElement(e.PathName()) // process the honorNamespace instruction - xmlAddNamespaceConditional(e, e.GetParent(), newElem, honorNamespace) + xmlAddNamespaceConditional(e, e.GetParent(), newElem, opts.HonorNamespace) return true, nil default: @@ -181,7 +181,7 @@ func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, only if e.GetParent() != nil { // only if not the root level, we can check if parent namespace != actual elements namespace // so if we need to add namespaces, check if they are equal, if not add the namespace attribute - xmlAddNamespaceConditional(e, e.GetParent(), newElem, honorNamespace) + xmlAddNamespaceConditional(e, e.GetParent(), newElem, opts.HonorNamespace) } else { // if this is the root node, we take the given element from the parent parameter as p // avoiding wrongly adding an additional level in the xml doc. @@ -193,7 +193,7 @@ func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, only return false, fmt.Errorf("child %s does not exist for %s", k, e.SdcpbPath().ToXPath(false)) } // TODO: Do we also need to xmlAddAllChildValues here too? - doAdd, err := toXmlInternal(ctx, child, newElem, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove) + doAdd, err := toXmlInternal(ctx, child, newElem, opts) if err != nil { return false, err } @@ -217,13 +217,13 @@ func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, only ancestorWithSchema, _ := GetFirstAncestorWithSchema(e.GetParent()) if ancestorWithSchema != nil && slices.Contains(GetSchemaKeys(ancestorWithSchema), e.PathName()) { // Key leaf deletions should delete the list entry identified by key value. - utils.AddXMLOperation(parent, utils.XMLOperationDelete, operationWithNamespace, useOperationRemove) + utils.AddXMLOperation(parent, utils.XMLOperationDelete, opts.OperationWithNamespace, opts.UseOperationRemove) xmlAddKeyElements(e.GetParent(), parent) return true, nil } } // if not, add the remove / delete op - utils.AddXMLOperation(parent.CreateElement(e.PathName()), utils.XMLOperationDelete, operationWithNamespace, useOperationRemove) + utils.AddXMLOperation(parent.CreateElement(e.PathName()), utils.XMLOperationDelete, opts.OperationWithNamespace, opts.UseOperationRemove) // see case nil for an explanation of this, it is basically the same if e.GetParent().GetSchema() == nil { xmlAddKeyElements(e.GetParent(), parent) @@ -231,18 +231,22 @@ func toXmlInternal(ctx context.Context, e api.Entry, parent *etree.Element, only return true, nil } // if the Field or Leaflist remains to exist - // get highes Precedence value - le := e.GetLeafVariants().GetHighestPrecedence(onlyNewOrUpdated, false, false) + // get highest precedence value + le := e.GetLeafVariants().GetHighestPrecedence(opts.OnlyNewOrUpdated, false, false) if le == nil { return false, nil } ns := "" // process the namespace attribute - if e.GetParent() == nil || (honorNamespace && !namespaceIsEqual(e, e.GetParent())) { + if e.GetParent() == nil || (opts.HonorNamespace && !namespaceIsEqual(e, e.GetParent())) { ns = utils.GetNamespaceFromGetSchema(e.GetSchema()) } + value := le.Value() + if ShouldRedact(e, opts.IncludeSensitive, opts.SensitivePathSet) { + value = types.RedactedTypedValue + } // convert value to XML and add to parent - utils.TypedValueToXML(parent, le.Value(), e.PathName(), ns, onlyNewOrUpdated, operationWithNamespace, useOperationRemove) + utils.TypedValueToXML(parent, value, e.PathName(), ns, opts.OnlyNewOrUpdated, opts.OperationWithNamespace, opts.UseOperationRemove) return true, nil } return false, fmt.Errorf("unable to convert to xml (%s)", e.SdcpbPath().ToXPath(false)) @@ -306,9 +310,16 @@ func xmlAddKeyElements(s api.Entry, parent *etree.Element) { } } -func xmlAddAllChildValues(ctx context.Context, s api.Entry, parent *etree.Element, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool) error { +func xmlAddAllChildValues(ctx context.Context, s api.Entry, parent *etree.Element, opts XMLRenderOpts) error { parent.Child = make([]etree.Token, 0) - _, err := toXmlInternal(ctx, s, parent, false, honorNamespace, operationWithNamespace, useOperationRemove) + // NETCONF replace semantics: the device replaces the entire list entry with exactly what is + // sent. If only changed children were included (OnlyNewOrUpdated=true), the device would + // treat the absent children as deletions. Override to false so every child is emitted, + // regardless of whether it changed. All other opts (namespace, sensitive-path filtering, …) + // are preserved from the caller. + allOpts := opts + allOpts.OnlyNewOrUpdated = false + _, err := toXmlInternal(ctx, s, parent, allOpts) if err != nil { return err } diff --git a/pkg/tree/ops/xml_test.go b/pkg/tree/ops/xml_test.go index d76bcfb0..150ef3e9 100644 --- a/pkg/tree/ops/xml_test.go +++ b/pkg/tree/ops/xml_test.go @@ -616,7 +616,12 @@ func TestToXMLTable(t *testing.T) { t.Error(err) } - xmlDoc, err := ops.ToXML(ctx, root.Entry, tt.onlyNewOrUpdated, tt.honorNamespace, tt.operationWithNamespace, tt.useOperationRemove) + xmlDoc, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{ + RenderOpts: ops.RenderOpts{OnlyNewOrUpdated: tt.onlyNewOrUpdated}, + HonorNamespace: tt.honorNamespace, + OperationWithNamespace: tt.operationWithNamespace, + UseOperationRemove: tt.useOperationRemove, + }) if err != nil { t.Fatal(err) } @@ -697,7 +702,7 @@ func TestToXML_KeyLeafDeleteUpgradesToListEntryDelete(t *testing.T) { t.Fatal(err) } - xmlDoc, err := ops.ToXML(ctx, root.Entry, true, false, false, false) + xmlDoc, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{RenderOpts: ops.RenderOpts{OnlyNewOrUpdated: true}}) if err != nil { t.Fatal(err) } @@ -773,7 +778,7 @@ func TestToXML_DoubleKeyLeafDeleteUpgradesToListEntryDelete(t *testing.T) { t.Fatal(err) } - xmlDoc, err := ops.ToXML(ctx, root.Entry, true, false, false, false) + xmlDoc, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{RenderOpts: ops.RenderOpts{OnlyNewOrUpdated: true}}) if err != nil { t.Fatal(err) } @@ -865,7 +870,7 @@ func TestToXML_DoubleKeyLeafDeleteOnlyDeletesTargetEntry(t *testing.T) { t.Fatal(err) } - xmlDoc, err := ops.ToXML(ctx, root.Entry, true, false, false, false) + xmlDoc, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{RenderOpts: ops.RenderOpts{OnlyNewOrUpdated: true}}) if err != nil { t.Fatal(err) } @@ -949,7 +954,7 @@ func TestToXML_DoubleKeyLeafDeleteViaKey1(t *testing.T) { t.Fatal(err) } - xmlDoc, err := ops.ToXML(ctx, root.Entry, true, false, false, false) + xmlDoc, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{RenderOpts: ops.RenderOpts{OnlyNewOrUpdated: true}}) if err != nil { t.Fatal(err) } @@ -1028,7 +1033,7 @@ func TestToXML_DoubleKeyNonKeyLeafDeleteDoesNotPromote(t *testing.T) { t.Fatal(err) } - xmlDoc, err := ops.ToXML(ctx, root.Entry, true, false, false, false) + xmlDoc, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{RenderOpts: ops.RenderOpts{OnlyNewOrUpdated: true}}) if err != nil { t.Fatal(err) } @@ -1052,6 +1057,77 @@ func TestToXML_DoubleKeyNonKeyLeafDeleteDoesNotPromote(t *testing.T) { } } +// TestToXML_SensitiveRedaction verifies that ToXML replaces sensitive-leaf values +// with the redaction sentinel when IncludeSensitive=false, and returns the real +// value when IncludeSensitive=true. +func TestToXML_SensitiveRedaction(t *testing.T) { + flagsOld := types.NewUpdateInsertFlags() + + tests := []struct { + name string + exposeSensitive bool + wantPatterntest string + }{ + { + name: "redacts sensitive leaf when IncludeSensitive=false", + exposeSensitive: false, + wantPatterntest: "***", + }, + { + name: "reveals sensitive leaf when IncludeSensitive=true", + exposeSensitive: true, + wantPatterntest: "hallo 00", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + scb := newSchemaClientMarkingSensitive(t, ctrl, "patterntest") + + ctx := context.Background() + tc := tree.NewTreeContext(scb, pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))) + root, err := tree.NewTreeRoot(ctx, tc) + if err != nil { + t.Fatal(err) + } + converter := utils.NewConverter(scb) + + upds, err := testhelper.ExpandUpdateFromConfig(ctx, testhelper.Config1(), converter) + if err != nil { + t.Fatal(err) + } + if err := testhelper.AddToRoot(ctx, root.Entry, upds, flagsOld, "owner1", 5); err != nil { + t.Fatal(err) + } + if err := root.FinishInsertionPhase(ctx); err != nil { + t.Fatal(err) + } + + xmlDoc, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{ + RenderOpts: ops.RenderOpts{IncludeSensitive: tt.exposeSensitive}, + }) + if err != nil { + t.Fatal(err) + } + utils.XmlRecursiveSortElementsByTagName(&xmlDoc.Element) + xmlDoc.Indent(2) + xmlDocStr, err := xmlDoc.WriteToString() + if err != nil { + t.Fatal(err) + } + + pattElem := xmlDoc.FindElement("//patterntest") + if pattElem == nil { + t.Fatalf("patterntest element not found in XML output:\n%s", xmlDocStr) + } + if diff := cmp.Diff(tt.wantPatterntest, pattElem.Text()); diff != "" { + t.Errorf("patterntest value mismatch (-want +got):\n%s", diff) + } + }) + } +} + // TestToXML_DoubleKeyTwoSimultaneousListEntryDeletes verifies that deleting a key // leaf on two different list entries at the same time produces two independent // operation="delete" entries in the output. @@ -1125,7 +1201,7 @@ func TestToXML_DoubleKeyTwoSimultaneousListEntryDeletes(t *testing.T) { t.Fatal(err) } - xmlDoc, err := ops.ToXML(ctx, root.Entry, true, false, false, false) + xmlDoc, err := ops.ToXML(ctx, root.Entry, ops.XMLRenderOpts{RenderOpts: ops.RenderOpts{OnlyNewOrUpdated: true}}) if err != nil { t.Fatal(err) } diff --git a/pkg/tree/ops/xpath.go b/pkg/tree/ops/xpath.go index 344e61d4..14f5267c 100644 --- a/pkg/tree/ops/xpath.go +++ b/pkg/tree/ops/xpath.go @@ -4,19 +4,24 @@ import ( "context" "github.com/sdcio/data-server/pkg/tree/api" + "github.com/sdcio/data-server/pkg/tree/types" "github.com/sdcio/sdc-protos/sdcpb" ) // ToXPath converts the branch under e to a list of XPath entries. It returns only the highest precedence value for each path. -func ToXPath(_ context.Context, e api.Entry, onlyNewOrUpdated, includeDefaults bool) (*sdcpb.PathValues, error) { - lvs := GetHighestPrecedence(e, onlyNewOrUpdated, includeDefaults, false) +func ToXPath(_ context.Context, e api.Entry, opts XPathRenderOpts) (*sdcpb.PathValues, error) { + lvs := GetHighestPrecedence(e, opts.OnlyNewOrUpdated, opts.IncludeDefaults, false) xpaths := make([]*sdcpb.PathValue, 0, len(lvs)) for _, lv := range lvs { + value := lv.Value() + if ShouldRedact(lv.GetEntry(), opts.IncludeSensitive, opts.SensitivePathSet) { + value = types.RedactedTypedValue + } xpaths = append(xpaths, &sdcpb.PathValue{ Path: lv.SdcpbPath(), - Value: lv.Value(), + Value: value, }) } return &sdcpb.PathValues{PathValues: xpaths}, nil diff --git a/pkg/tree/ops/xpath_test.go b/pkg/tree/ops/xpath_test.go index 7599f1e4..d8ba888f 100644 --- a/pkg/tree/ops/xpath_test.go +++ b/pkg/tree/ops/xpath_test.go @@ -39,7 +39,7 @@ func TestToXPath_FromConfig1(t *testing.T) { t.Fatal(err) } - pvs, err := ops.ToXPath(ctx, root.Entry, false, false) + pvs, err := ops.ToXPath(ctx, root.Entry, ops.XPathRenderOpts{}) if err != nil { t.Fatal(err) } @@ -113,12 +113,12 @@ func TestToXPath_OnlyNewOrUpdated_WithSameNewAndExistingConfig1(t *testing.T) { t.Fatal(err) } - onlyNewOrUpdated, err := ops.ToXPath(ctx, root.Entry, true, false) + onlyNewOrUpdated, err := ops.ToXPath(ctx, root.Entry, ops.XPathRenderOpts{RenderOpts: ops.RenderOpts{OnlyNewOrUpdated: true}}) if err != nil { t.Fatal(err) } - allValues, err := ops.ToXPath(ctx, root.Entry, false, false) + allValues, err := ops.ToXPath(ctx, root.Entry, ops.XPathRenderOpts{}) if err != nil { t.Fatal(err) } diff --git a/pkg/tree/processors/processor_blame_config.go b/pkg/tree/processors/processor_blame_config.go index 3a935832..04cafde5 100644 --- a/pkg/tree/processors/processor_blame_config.go +++ b/pkg/tree/processors/processor_blame_config.go @@ -25,6 +25,9 @@ func NewBlameConfigProcessor(params *BlameConfigProcessorParams) *BlameConfigPro type BlameConfigProcessorParams struct { IncludeDefaults bool + // RenderOpts carries sensitive-redaction flags (IncludeSensitive, + // SensitivePathSet) shared with all other northbound render operations. + ops.RenderOpts } // Run processes the entry tree starting from e, building a blame tree showing which owner @@ -89,18 +92,27 @@ func (t *BlameConfigTask) Run(ctx context.Context, submit func(pool.Task) error) highestLe := t.selfEntry.GetLeafVariants().GetHighestPrecedence(false, true, true) if highestLe != nil { if highestLe.Update.Owner() != consts.DefaultsIntentName || t.context.IncludeDefaults { - t.self.SetValue(highestLe.Update.Value()).SetOwner(highestLe.Update.Owner()) - - // check if running equals the expected - runningLe := t.selfEntry.GetLeafVariants().GetRunning() - - switch { - case runningLe != nil: - // if running value is different from the highest precedence value, then we have a deviation, - // so we set the deviation value to the running value - if !proto.Equal(runningLe.Update.Value(), highestLe.Update.Value()) { - t.self.SetDeviationValue(runningLe.Value()) + shouldRedact := ops.ShouldRedact(t.selfEntry, t.context.IncludeSensitive, t.context.SensitivePathSet) + value := highestLe.Update.Value() + if shouldRedact { + value = types.RedactedTypedValue + } + t.self.SetValue(value).SetOwner(highestLe.Update.Owner()) + + // check if running equals the expected + runningLe := t.selfEntry.GetLeafVariants().GetRunning() + + switch { + case runningLe != nil: + // if running value is different from the highest precedence value, then we have a deviation, + // so we set the deviation value to the running value + if !proto.Equal(runningLe.Update.Value(), highestLe.Update.Value()) { + deviationValue := runningLe.Value() + if shouldRedact { + deviationValue = types.RedactedTypedValue } + t.self.SetDeviationValue(deviationValue) + } case runningLe == nil && highestLe.GetUpdate().Owner() != consts.DefaultsIntentName: // if running is nil and highest is not from default, then the deviation is from a non-existing running value, // so we set it to empty diff --git a/pkg/tree/processors/processor_blame_config_test.go b/pkg/tree/processors/processor_blame_config_test.go index 9e1bcfcd..0be60730 100644 --- a/pkg/tree/processors/processor_blame_config_test.go +++ b/pkg/tree/processors/processor_blame_config_test.go @@ -11,6 +11,7 @@ import ( "github.com/sdcio/data-server/pkg/pool" "github.com/sdcio/data-server/pkg/tree" "github.com/sdcio/data-server/pkg/tree/consts" + "github.com/sdcio/data-server/pkg/tree/ops" "github.com/sdcio/data-server/pkg/tree/processors" "github.com/sdcio/data-server/pkg/utils/testhelper" sdcio_schema "github.com/sdcio/data-server/tests/sdcioygot" @@ -185,3 +186,95 @@ func Test_sharedEntryAttributes_BlameConfig(t *testing.T) { }) } } + +func TestBlameConfigSensitiveRedaction(t *testing.T) { + ctx := context.TODO() + owner1 := "owner1" + + tests := []struct { + name string + exposeSensitive bool + wantPatterntestValue string + wantDeviationValue string + }{ + { + name: "redacts value and deviationValue when IncludeSensitive=false", + exposeSensitive: false, + wantPatterntestValue: "***", + wantDeviationValue: "***", + }, + { + name: "reveals value and deviationValue when IncludeSensitive=true", + exposeSensitive: true, + wantPatterntestValue: "hallo 00", + wantDeviationValue: "hallo 0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + scb, err := testhelper.GetSchemaClientBoundMarkingLeafSensitive(t, mockCtrl, "patterntest") + if err != nil { + t.Fatal(err) + } + + tc := tree.NewTreeContext(scb, pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))) + root, err := tree.NewTreeRoot(ctx, tc) + if err != nil { + t.Fatal(err) + } + + conf1 := testhelper.Config1() + _, err = testhelper.LoadYgotStructIntoTreeRoot(ctx, conf1, root.Entry, owner1, 5, false, flagsNew) + if err != nil { + t.Fatal(err) + } + + // running has a different patterntest so we get a deviationValue + running := testhelper.Config1() + running.Patterntest = ygot.String("hallo 0") + _, err = testhelper.LoadYgotStructIntoTreeRoot(ctx, running, root.Entry, consts.RunningIntentName, consts.RunningValuesPrio, false, flagsExisting) + if err != nil { + t.Fatal(err) + } + + if err := root.FinishInsertionPhase(ctx); err != nil { + t.Fatal(err) + } + + sharedPool := pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0)) + bp := processors.NewBlameConfigProcessor(&processors.BlameConfigProcessorParams{ + RenderOpts: ops.RenderOpts{IncludeSensitive: tt.exposeSensitive}, + }) + got, err := bp.Run(ctx, root.Entry, sharedPool) + if err != nil { + t.Fatalf("BlameConfig() error: %s", err) + } + + // Find patterntest in the blame tree + var patterntestElem *sdcpb.BlameTreeElement + for _, child := range got.GetChilds() { + if child.GetName() == "patterntest" { + patterntestElem = child + break + } + } + if patterntestElem == nil { + t.Fatal("patterntest element not found in blame tree") + } + + gotValue := patterntestElem.GetValue().GetStringVal() + if diff := cmp.Diff(tt.wantPatterntestValue, gotValue); diff != "" { + t.Errorf("patterntest Value mismatch (-want +got):\n%s", diff) + } + + gotDeviation := patterntestElem.GetDeviationValue().GetStringVal() + if diff := cmp.Diff(tt.wantDeviationValue, gotDeviation); diff != "" { + t.Errorf("patterntest DeviationValue mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/pkg/tree/types/sensitive_path_index.go b/pkg/tree/types/sensitive_path_index.go new file mode 100644 index 00000000..3a852808 --- /dev/null +++ b/pkg/tree/types/sensitive_path_index.go @@ -0,0 +1,135 @@ +package types + +import ( + "sync" + + sdcpb "github.com/sdcio/sdc-protos/sdcpb" +) + +// RedactedStringValue is the sentinel substituted in place of sensitive leaf +// values in deviation, render, and blame outputs. +const RedactedStringValue = "***" + +// RedactedTypedValue is the canonical TypedValue emitted in place of a +// sensitive leaf. Callers must not mutate the returned pointer. +var RedactedTypedValue = &sdcpb.TypedValue{Value: &sdcpb.TypedValue_StringVal{StringVal: RedactedStringValue}} + +// SensitivePathChecker is the read-only interface consumed by tree-layer +// render and deviation operations. Both *SensitivePathIndex (snapshot mode via +// Add) and *SensitivePathIndex (live mode via Set/Delete) satisfy it. +type SensitivePathChecker interface { + Contains(path *sdcpb.Path) bool +} + +// ShouldRedact reports whether a leaf value must be replaced with the +// redaction sentinel. It is the single authoritative implementation of the +// two-source sensitivity check; both pkg/tree/ops (entry-aware wrapper) and +// pkg/tree/api (leaf_variants.go) delegate here. +// +// Parameters: +// - includeSensitive: caller has opted in to receiving real values — skip redaction. +// - schema: the entry's schema element (pass entry.GetSchema()); nil is safe. +// - path: the entry's sdcpb path, used for intent path-marker lookup. +// - sps: cross-intent path-marker set; nil means no path-marker redaction. +func ShouldRedact(includeSensitive bool, schema *sdcpb.SchemaElem, path *sdcpb.Path, sps SensitivePathChecker) bool { + if includeSensitive { + return false + } + return (schema != nil && schema.IsSensitive()) || (sps != nil && sps.Contains(path)) +} + +var _ SensitivePathChecker = (*SensitivePathIndex)(nil) + +// SensitivePathIndex is a concurrent-safe index of sensitive schema paths. +// +// It supports two usage modes that share one struct: +// +// - Live index (datastore-wide): use Set/Delete for incremental per-intent +// updates. The map value tracks which intents own each path so that Delete +// can surgically remove only that intent's contribution. +// +// - Snapshot (per-intent): use Add to mark presence without intent tracking. +// The map value is nil — key existence is the only signal Contains needs. +// +// The two modes must not be mixed on the same instance. +type SensitivePathIndex struct { + mu sync.RWMutex + index map[string][]string // key-pruned XPath → intent names (nil = presence-only) +} + +// NewSensitivePathIndex returns a ready-to-use, empty SensitivePathIndex. +func NewSensitivePathIndex() *SensitivePathIndex { + return &SensitivePathIndex{index: make(map[string][]string)} +} + +// Add marks each path as sensitive without associating it with an intent name +// (presence-only mode). Safe to call concurrently. +func (s *SensitivePathIndex) Add(paths ...*sdcpb.Path) { + s.mu.Lock() + defer s.mu.Unlock() + for _, p := range paths { + if p != nil { + s.index[p.ToXPath(true)] = nil + } + } +} + +// Set replaces the sensitive paths contributed by intentName with paths. +// Existing entries for intentName are removed before the new ones are added. +// Safe to call concurrently. +func (s *SensitivePathIndex) Set(intentName string, paths []*sdcpb.Path) { + s.mu.Lock() + defer s.mu.Unlock() + s.removeLocked(intentName) + for _, p := range paths { + if p == nil { + continue + } + key := p.ToXPath(true) + s.index[key] = append(s.index[key], intentName) + } +} + +// Delete removes all sensitive-path contributions from intentName. +// Safe to call concurrently. +func (s *SensitivePathIndex) Delete(intentName string) { + s.mu.Lock() + defer s.mu.Unlock() + s.removeLocked(intentName) +} + +// removeLocked removes intentName from every entry in the index. +// Entries whose slice becomes empty are deleted from the map. +// Caller must hold the write lock. +func (s *SensitivePathIndex) removeLocked(intentName string) { + for key, names := range s.index { + if names == nil { + continue // presence-only entry (Add mode) — not managed by intent tracking + } + j := 0 + for _, n := range names { + if n != intentName { + names[j] = n + j++ + } + } + if j == 0 { + delete(s.index, key) + } else { + s.index[key] = names[:j] + } + } +} + +// Contains reports whether path is in the index. The lookup uses the same +// key-pruned XPath form as the stored keys, so /interface[name=eth0]/secret +// matches a stored /interface/secret. Safe to call on a nil receiver. +func (s *SensitivePathIndex) Contains(path *sdcpb.Path) bool { + if s == nil || path == nil { + return false + } + s.mu.RLock() + defer s.mu.RUnlock() + _, ok := s.index[path.ToXPath(true)] + return ok +} diff --git a/pkg/tree/types/sensitive_path_index_test.go b/pkg/tree/types/sensitive_path_index_test.go new file mode 100644 index 00000000..19300d15 --- /dev/null +++ b/pkg/tree/types/sensitive_path_index_test.go @@ -0,0 +1,101 @@ +package types_test + +import ( + "testing" + + "github.com/sdcio/data-server/pkg/tree/types" + sdcpb "github.com/sdcio/sdc-protos/sdcpb" +) + +func mustParsePath(t *testing.T, s string) *sdcpb.Path { + t.Helper() + p, err := sdcpb.ParsePath(s) + if err != nil { + t.Fatalf("mustParsePath(%q): %v", s, err) + } + return p +} + +// Cycle 1 — Add marks a path as sensitive; Contains returns true. +func TestSensitivePathIndex_AddContains(t *testing.T) { + idx := types.NewSensitivePathIndex() + idx.Add(mustParsePath(t, "/bgp/neighbors/auth-password")) + + if !idx.Contains(mustParsePath(t, "/bgp/neighbors/auth-password")) { + t.Error("Contains() = false after Add, want true") + } +} + +// Cycle 2 — Contains returns false for a path that was never added. +func TestSensitivePathIndex_ContainsUnknownPath(t *testing.T) { + idx := types.NewSensitivePathIndex() + idx.Add(mustParsePath(t, "/bgp/neighbors/auth-password")) + + if idx.Contains(mustParsePath(t, "/bgp/neighbors/other")) { + t.Error("Contains() = true for unknown path, want false") + } +} + +// Cycle 3 — key-pruned matching: keyed lookup matches keyless stored path and vice-versa. +func TestSensitivePathIndex_KeyPrunedMatch(t *testing.T) { + t.Run("keyless stored, keyed lookup", func(t *testing.T) { + idx := types.NewSensitivePathIndex() + idx.Add(mustParsePath(t, "/interface/secret")) + + if !idx.Contains(mustParsePath(t, "/interface[name=eth0]/secret")) { + t.Error("Contains() = false for keyed lookup against keyless stored path, want true") + } + }) + + t.Run("keyed stored, keyless lookup", func(t *testing.T) { + idx := types.NewSensitivePathIndex() + idx.Add(mustParsePath(t, "/interface[name=eth0]/secret")) + + if !idx.Contains(mustParsePath(t, "/interface/secret")) { + t.Error("Contains() = false for keyless lookup against keyed stored path, want true") + } + }) +} + +// Cycle 4 — Set(intentName, paths) makes paths discoverable via Contains. +func TestSensitivePathIndex_SetContains(t *testing.T) { + idx := types.NewSensitivePathIndex() + idx.Set("my-intent", []*sdcpb.Path{mustParsePath(t, "/ospf/auth-key")}) + + if !idx.Contains(mustParsePath(t, "/ospf/auth-key")) { + t.Error("Contains() = false after Set, want true") + } +} + +// Cycle 5 — Delete removes all paths contributed by that intent. +func TestSensitivePathIndex_DeleteRemovesIntentPaths(t *testing.T) { + idx := types.NewSensitivePathIndex() + idx.Set("my-intent", []*sdcpb.Path{mustParsePath(t, "/ospf/auth-key")}) + idx.Delete("my-intent") + + if idx.Contains(mustParsePath(t, "/ospf/auth-key")) { + t.Error("Contains() = true after Delete, want false") + } +} + +// Cycle 6 — Delete does not remove a path still contributed by another intent. +func TestSensitivePathIndex_DeleteKeepsSharedPath(t *testing.T) { + sharedPath := mustParsePath(t, "/bgp/neighbors/auth-password") + idx := types.NewSensitivePathIndex() + idx.Set("intent-a", []*sdcpb.Path{sharedPath}) + idx.Set("intent-b", []*sdcpb.Path{sharedPath}) + + idx.Delete("intent-a") + + if !idx.Contains(sharedPath) { + t.Error("Contains() = false after deleting one intent, want true (path still owned by intent-b)") + } +} + +// Cycle 7 — nil receiver returns false without panicking. +func TestSensitivePathIndex_NilReceiverContains(t *testing.T) { + var idx *types.SensitivePathIndex + if idx.Contains(mustParsePath(t, "/bgp/neighbors/auth-password")) { + t.Error("Contains() on nil receiver = true, want false") + } +} diff --git a/pkg/utils/testhelper/utils.go b/pkg/utils/testhelper/utils.go index 8d170367..2dcb2ecc 100644 --- a/pkg/utils/testhelper/utils.go +++ b/pkg/utils/testhelper/utils.go @@ -133,6 +133,44 @@ func GetSchemaClientBound(t *testing.T, mockCtrl *gomock.Controller) (*mockschem return mockscb, nil } +// GetSchemaClientBoundMarkingLeafSensitive creates a SchemaClientBound mock that behaves +// like GetSchemaClientBound but marks the leaf with the given name as Sensitive=true in +// its returned schema. Use in tests that need to exercise sensitive-data redaction paths. +func GetSchemaClientBoundMarkingLeafSensitive(t *testing.T, mockCtrl *gomock.Controller, sensitiveLeafName string) (*mockschemaclientbound.MockSchemaClientBound, error) { + t.Helper() + x, schema, err := InitSDCIOSchema() + if err != nil { + return nil, err + } + + sdcpbSchema := &sdcpb.Schema{ + Name: schema.Name, + Vendor: schema.Vendor, + Version: schema.Version, + } + + mockscb := mockschemaclientbound.NewMockSchemaClientBound(mockCtrl) + mockscb.EXPECT().GetSchemaSdcpbPath(gomock.Any(), gomock.Any()).AnyTimes().DoAndReturn( + func(ctx context.Context, path *sdcpb.Path) (*sdcpb.GetSchemaResponse, error) { + resp, err := x.GetSchema(ctx, &sdcpb.GetSchemaRequest{ + Path: path, + Schema: sdcpbSchema, + }) + if err != nil || resp == nil { + return resp, err + } + elems := path.GetElem() + if len(elems) > 0 && elems[len(elems)-1].GetName() == sensitiveLeafName { + if field := resp.GetSchema().GetField(); field != nil { + field.Sensitive = true + } + } + return resp, nil + }, + ) + return mockscb, nil +} + type ImportStatsInterface interface { Changed() bool GetNewCount() int64