Skip to content

Add PathElemsMatchQuery helper for checking path membership to a telemetry path #467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions util/gnmi.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,41 @@ func FindPathElemPrefix(paths []*gpb.Path) *gpb.Path {
}
}

// pathElemMatchesQuery returns true if the given concrete (no wildcard)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify concrete here? I think that you're trying to say if the given query element matches the reference element -- but the no wildcard disclaimer is throwing me off, because it seems like wildcards are handled?

// PathElem is a possible gNMI telemetry name match for the reference PathElem.
// In particular, it matches wildcards for names and list keys, but *not* "...".
// See https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-path-conventions.md
func pathElemMatchesQuery(elem, refElem *gpb.PathElem) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to call the first argument here queryElem and the second pathElem so that it's clear which of the two we're trying to match against each other.

if elem == nil || refElem == nil {
return elem == nil && refElem == nil
}

if refElem.Name != "*" && elem.Name != refElem.Name {
return false
}

for k, ref := range refElem.Key {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one confusing thing here -- you say that the reference path must not include wildcards, but then we check whether ref == "*", which can presumably never happen.

We should probably note that if the key is not specified in the reference path, then we never check the value in the queried path, so therefore we assume that the element matches. This seems fine, but we should say that this function isn't schema aware -- it just takes in good faith that both paths are valid schema paths.

if v, ok := elem.Key[k]; !ok || (ref != "*" && v != ref) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If ref == "*" then you don't need to check that v != ref right? Because even if v == ref then if ref == * we'll return true.

return false
}
}
return true
}

// PathElemsMatchQuery returns true if the given concrete (no wildcard)
// PathElem path slice is a possible gNMI telemetry path match for the
// reference PathElem path slice.
// In particular, it matches wildcards for names and list keys, but *not* "...".
// See https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-path-conventions.md
func PathElemsMatchQuery(elems, refElems []*gpb.PathElem) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment here as above about making the concrete wording clearer, and the argument names.

for i := 0; i != len(elems) && i != len(refElems); i++ {
if !pathElemMatchesQuery(elems[i], refElems[i]) {
return false
}
}
return len(refElems) <= len(elems)
}

// PopGNMIPath returns the supplied GNMI path with the first path element
// removed. If the path is empty, it returns an empty path.
func PopGNMIPath(path *gpb.Path) *gpb.Path {
Expand Down
105 changes: 105 additions & 0 deletions util/gnmi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,111 @@ func TestFindPathElemPrefix(t *testing.T) {
}
}

func mustPathElem(s string) []*gpb.PathElem {
p, err := stringToStructuredPath(s)
if err != nil {
panic(err)
}
return p.Elem
}

func TestPathElemsMatchQuery(t *testing.T) {
tests := []struct {
desc string
inRefElems []*gpb.PathElem
inMatchingElems [][]*gpb.PathElem
inNonMatchingElems [][]*gpb.PathElem
}{{
desc: "no-wildcard, non-list path",
inRefElems: mustPathElem("/alpha/bravo/charlie"),
inMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha/bravo/charlie"),
mustPathElem("/alpha/bravo/charlie/delta"),
mustPathElem("/alpha/bravo/charlie/echo"),
},
inNonMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha/bravo/delta"),
mustPathElem("/alpha/bravo/delta/charlie"),
mustPathElem("/alpha/bravo/delta/echo"),
},
}, {
desc: "wildcard, non-list path",
inRefElems: mustPathElem("/alpha/*/charlie"),
inMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha/bravo/charlie"),
mustPathElem("/alpha/zulu/charlie/delta"),
mustPathElem("/alpha/yankee/charlie/echo"),
},
inNonMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha/bravo/delta"),
mustPathElem("/alpha/zulu/delta/charlie"),
mustPathElem("/bravo/yankee/charlie/echo"),
},
}, {
desc: "no-wildcard, list path",
inRefElems: mustPathElem("/alpha/bravo[key=value]/charlie"),
inMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha/bravo[key=value]/charlie"),
mustPathElem("/alpha/bravo[key=value]/charlie/delta"),
},
inNonMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha/bravo[key=value2]/charlie"),
mustPathElem("/alpha/bravo[key=value2]/charlie/echo"),
mustPathElem("/alpha/bravo/charlie"),
mustPathElem("/alpha/bravo/charlie/echo"),
},
}, {
desc: "wildcard, list path",
inRefElems: mustPathElem("/alpha/bravo[key=*]/charlie"),
inMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha/bravo[key=value]/charlie"),
mustPathElem("/alpha/bravo[key=value]/charlie/delta"),
mustPathElem("/alpha/bravo[key=value2]/charlie"),
mustPathElem("/alpha/bravo[key=value2]/charlie/echo"),
},
inNonMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha/bravo/charlie"),
mustPathElem("/alpha/bravo/charlie/foxtrot"),
mustPathElem("/alpha/bravo/charlie"),
mustPathElem("/alpha/bravo/charlie/echo"),
},
}, {
desc: "multi-wildcard, list path",
inRefElems: mustPathElem("/alpha[asn=15169]/bravo[key=*]/*/delta[name=*]/echo"),
inMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha[asn=15169]/bravo[key=tincan][key2=kale]/charlie[k=v]/delta[name=lamp]/echo[a=b]/"),
mustPathElem("/alpha[asn=15169]/bravo[key=tincan]/charlie/delta[name=lamp]/echo/"),
mustPathElem("/alpha[asn=15169]/bravo[key=tincan]/whiskey/delta[name=lamp]/echo/"),
mustPathElem("/alpha[asn=15169]/bravo[key=tincan]/charlie/delta[name=lamp]/echo/a[name=bulb]/b/c"),
mustPathElem("/alpha[asn=15169]/bravo[key=tincan]/charlie/delta[name=lamp]/echo/f[name=bulb]"),
},
inNonMatchingElems: [][]*gpb.PathElem{
mustPathElem("/alpha[asn=30]/bravo[key=tincan]/charlie/delta[name=lamp]/echo/b/c[name=bulb]/d"),
mustPathElem("/alpha[asn=15169]/bravo/charlie/delta[name=lamp]/echo/f[name=bulb]"),
mustPathElem("/quebec[asn=15169]/bravo/charlie/delta[name=lamp]/echo/f[name=bulb]"),
mustPathElem("/alpha[password=15169]/bravo[key=tincan]/charlie/delta[name=lamp]/echo/"),
mustPathElem("/alpha/bravo[key=tincan]/charlie/delta[name=lamp]/echo/f[name=bulb]"),
mustPathElem("/alpha/bravo[key=tincan]/charlie/delta[name=lamp]/echo/f[name=bulb]"),
mustPathElem("/alpha[asn=15169]/bravo[key=tincan]/charlie/delta/echo/f[name=bulb]"),
},
}}

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
for _, matchElems := range tt.inMatchingElems {
if !PathElemsMatchQuery(matchElems, tt.inRefElems) {
t.Errorf("unexpected non-matching result for %v\nreference path elems: %v", matchElems, tt.inRefElems)
}
}
for _, nonMatchElems := range tt.inNonMatchingElems {
if PathElemsMatchQuery(nonMatchElems, tt.inRefElems) {
t.Errorf("unexpected matching result for %v\nreference path elems: %v", nonMatchElems, tt.inRefElems)
}
}
})
}
}

func TestFindModelData(t *testing.T) {
tests := []struct {
name string
Expand Down
131 changes: 131 additions & 0 deletions util/pathstrings.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Copyright 2020 Google Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package util

import (
"bytes"
"errors"
"fmt"
"strings"

gnmipb "github.com/openconfig/gnmi/proto/gnmi"
)

// stringToStructuredPath takes a string representing a path, and converts it to
// a gnmi.Path, using the PathElem element message that is defined in gNMI 0.4.0.
// XXX: This is copied code from ygot package. ygot's code should probably
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here about making the change to avoid code duplication.

// live in this package instead.
func stringToStructuredPath(path string) (*gnmipb.Path, error) {
parts := PathStringToElements(path)

gpath := &gnmipb.Path{}
for _, p := range parts {
name, kv, err := extractKV(p)
if err != nil {
return nil, fmt.Errorf("error parsing path %s: %v", path, err)
}
gpath.Elem = append(gpath.Elem, &gnmipb.PathElem{
Name: name,
Key: kv,
})
}
return gpath, nil
}

// extractKV extracts key value predicates from the input string in. It returns
// the name of the element, a map keyed by key name with values of the predicates
// specified. It removes escape characters from keys and values where they are
// specified.
// XXX: This is copied code from ygot package. ygot's code should probably
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above - please don't duplicate code.

// live in this package instead.
func extractKV(in string) (string, map[string]string, error) {
var inEscape, inKey, inValue bool
var name, currentKey string
var buf bytes.Buffer
keys := map[string]string{}

for _, ch := range in {
switch {
case ch == '[' && !inEscape && !inValue && inKey:
return "", nil, fmt.Errorf("received an unescaped [ in key of element %s", name)
case ch == '[' && !inEscape && !inKey:
inKey = true
if len(keys) == 0 {
if buf.Len() == 0 {
return "", nil, errors.New("received a value when the element name was null")
}
name = buf.String()
buf.Reset()
}
continue
case ch == ']' && !inEscape && !inKey:
return "", nil, fmt.Errorf("received an unescaped ] when not in a key for element %s", buf.String())
case ch == ']' && !inEscape:
inKey = false
inValue = false
if err := addKey(keys, name, currentKey, buf.String()); err != nil {
return "", nil, err
}
buf.Reset()
currentKey = ""
continue
case ch == '\\' && !inEscape:
inEscape = true
continue
case ch == '=' && inKey && !inEscape && !inValue:
currentKey = buf.String()
buf.Reset()
inValue = true
continue
}

buf.WriteRune(ch)
inEscape = false
}

if len(keys) == 0 {
name = buf.String()
}

if len(keys) != 0 && buf.Len() != 0 {
// In this case, we have trailing garbage following the key.
return "", nil, fmt.Errorf("trailing garbage following keys in element %s, got: %v", name, buf.String())
}

if strings.Contains(name, " ") {
return "", nil, fmt.Errorf("invalid space character included in element name '%s'", name)
}

return name, keys, nil
}

// addKey adds key k with value v to the key's map. The key, value pair is specified
// to be for an element named e.
// XXX: This is copied code from ygot package. ygot's code should probably
// live in this package instead.
func addKey(keys map[string]string, e, k, v string) error {
switch {
case strings.Contains(k, " "):
return fmt.Errorf("received an invalid space in element %s key name '%s'", e, k)
case e == "":
return fmt.Errorf("received null element value with key and value %s=%s", k, v)
case k == "":
return fmt.Errorf("received null key name for element %s", e)
case v == "":
return fmt.Errorf("received null value for key %s of element %s", k, e)
}
keys[k] = v
return nil
}
Loading