CVL Infra Enhancments and Fixes

1. Enhancement - Support TABLE keys modelled as container
2. Enhancement - DBAL CountKeysPattern API
3. Optimisation: removing libyang syntax checks for Full entry delete
4. Enhancement - CVL support for SONiC YANG table with distinct YANG
4.1 Enhancement - For multi-list tables correct Redis pattern and sep
  are matched and picked.
4.2 Fix - The cvl code to build table expression based on YANG list name
  instead of table name.
4.3 Fix - CVL maintains tableInfo using a LIST name, fixed the
  inconsistent
4.4 Enhancement - code to lookup using list name instead of table name.
4.5 Fix - recognition of table and fields if the table contains mutiple
  lists and if table name as prefix is not present in any of the list
name
5. Optimisation: removing extra checks in CVL for non existing fields
   for update operation
6. Fixes: CVL does not honours "not equal" operator in must expression
   predicate
7. Fixes: clear depDataCache along with semantic cache, when CVL
   validation completes for clen state.
```text
    CVL maintains a separate cache to check if dependent data is already
to
    the semantic cache. If the entry is present in this cache, the
semantic
    cache is not updated.
    When we clear clear the semantic cache after every
ValidateEditConfig
    we also need to clear depDataCache.
```
8. Fixes: clear Xpath engine cache.
```text
   CVL maintains a seesion level cache for semantic validation, this
    cache generates entry for table each time they are operated on.
    As a result this cache contains duplicate entries for a table
    and in some cases it creates a stale entries for leaf, if they are
    deleted in the Latest request.
```
9 Fixes: stale entries in semantic documents
```text
    CVL maintains a cache of semantic document at the session level.
    If Delete for an entry is successful, the cache
    still contains a deleted entry, this is resulting in must expression
    failure for count cases, the logic in count() considers this entry
in to
    consideration. Therefore cache is cleared for the deleted entry.

    - Also modified addYangDataForMustExp() to avoid using global cache
      and use CVL wrapper for DBAL.
    - Modified fetchTableDataToTmpCache() to use CVL wrapper for DBAL's
      pipeline.
```
10: Fixes: Incorrect match for depdata in checkDeleteInRequestCache
```text
    For find in request key, case - T2*|K1. If the non-delete
configuration exists
    for dependent data in the Requestcache, its getting matched, and cvl
considers
    its already present in deleteCache and therefore ignores validation.
    modified the condition to match only if entry is deleted.

    Further enhanced the code for find in request hash-field case -
T2*|K2:{H1: K1}
    to match correcly for UPDATE cases. Also in case of leaflist.
```
11: Enhancement: Use DBAL's CVL wrapper for pipeline - All pipelines
    were modified use CVL's DB wrapper.
12: Enhancement: Use CVL's DB wrapper in validateLeafref and fixes for
    multi-key table instance
13: Fixes - handle incremental build and stale files removal - Fixed yin
    generator
```text
    Tool does not write if the file is not modified and writes only when
    yin modified or added as new.
    Also it removes any stale yin files (if present)
```
14. Enhancement: min-elements support for leaf-list
15. Enhancement - Advanced search functions in cvl DBAccess
16. Go test enhancements
17. Update the test YANGs with more complex cases.
18. Enhancement: changes to the count and filter lua scripts to check
    both the db and candidate config data; modified the count and lookup
19. Enhancement: Added Pipe query support for HGetAll, HGet, HMGet;
    NewValidationSession, cvlDBAccess changes, removing
validatedDataQuery
20. Fixes: Modify the session cache type from interface to
    map[string]interface in the custom validations, and used specific
cache key name to access its own cache to avoid overwriting the same
cache object by other module validation code
21. Enhancement: Added DB access adapter like APIs for redis APIs in CVL
    layer to access the DB access methods from the CVL layer.
22. Fixes: Added support for NULL field in the Keys, Exists API, fixed
    merging issue in HGetAll, HGet, HMGet in case of delete operation
type
23. Fixes: Made changes to handle the redis.Nil error returned by redis
    client API.
24. Fixes: Leafref check should check for all DELETE and CREATE of
    referred configs in request cache.
25. Fixes: Removed DB ping call in CVL whenever a new DB client is
    created.
26. Enhacement: Debugging and log enhancement.
27. Enhancement: CVL logs to appear in Rest-server logs.
28. Improved CVL performance for bulk configs like ACL rules creation.
```text
    For evaluation of leafref, When and Must expressions, CVL queries
    dependent data and merge with config data and prepares im-memory XML
to
    be evaluated by XPath engine. The 'addDependentData" call was adding
    same dependent data multiple whenever it has to evaluate leafref,
when
    or must expression. When multiple ACL Rules within same Acl-set are
    configured in single request, CVL was adding Acl-set multiple times
dues
    to which XML size becomes too huge and XPath engines takes long to
    evaluate. For 750 Acl rules creation there is 95% improvement in
    performance.
```
29. Added support for multiple custom validation callbacks per node in
    CVL
30. Fixes: or Replace operations, before performing semantic validation
    on Update request, delete fields from yang tree that are provided
with Delete request.
31. Enhancement: Added new extension to define dependency between
    tables.
```text
    This new extension 'dependent-on <list-name>' has to be used when
dependencies between tables can't
    be defined using leafref.
    This has to be added under list.
    It should not be used under container or leaf/leaf-list nodes.
    Only one list-name can be added to this extension.
```
32. Fixes: When mandatory node is getting deleted during cascade-delete,
    entire entry to be deleted. So find dependent entries in this entry
and mark them too for delete.
33. Fixed memory leak observed in CVL CGo code that uses libyang third
    party library for Syntax validation.

Co-authored-by: Mohammed Faraaz C <[email protected]>
Co-authored-by: Sachin Holla <[email protected]>
Co-authored-by: Balachandar Mani <[email protected]>
Co-authored-by: Mayank Maheshwari <[email protected]>
Co-authored-by: Shyam Mohan <[email protected]>

Author: 5 people

azure-pipelines.yml

@@ -90,8 +90,10 @@ stages:
         STATUS=0
         DEBDIR=$(realpath debian/sonic-mgmt-common)
 
-        [[ -f tools/test/database_config.json ]] && \
-            export DB_CONFIG_PATH=${PWD}/tools/test/database_config.json
+        # Update unixsocket path in database_config.json
+        tools/test/dbconfig.py -o build/tests/database_config.json
+        export DB_CONFIG_PATH=${PWD}/build/tests/database_config.json
+
         # Run CVL tests
 
         pushd build/tests/cvl

cvl/Makefile

@@ -20,67 +20,85 @@
 GO?=go
 SRC_FILES=$(shell find . -name '*.go' | grep -v '_test.go' | grep -v '/tests/')
 TEST_FILES=$(wildcard *_test.go)
-TOP_DIR := ..
-BUILD_DIR:=$(TOP_DIR)/build/cvl
+TOPDIR ?= ..
+BUILD_DIR:=$(TOPDIR)/build/cvl
+
+CVL_TEST_DIR = $(TOPDIR)/build/tests/cvl
+CVL_TEST_BIN = $(CVL_TEST_DIR)/cvl.test
+CVL_TEST_SCHEMA_DIR = $(CVL_TEST_DIR)/testdata/schema
+CVL_TEST_CONFIG = $(CVL_TEST_DIR)/cvl_cfg.json
+CVL_TEST_DB_CONFIG = $(CVL_TEST_DIR)/database_config.json
 
 CVL_SCHEMA_DIR = $(BUILD_DIR)/schema
-CVL_SCHEMA     = $(CVL_SCHEMA_DIR)/.done
-SONIC_YANG_DIR   = $(TOP_DIR)/build/yang/sonic
+CVL_SCHEMA = $(CVL_SCHEMA_DIR)/.done
+SONIC_YANG_DIR   = $(TOPDIR)/build/yang/sonic
 SONIC_YANG_FILES = $(shell find $(SONIC_YANG_DIR) -name '*.yang')
 YANG_SRC_DIR     = ../models/yang
 
-CVL_TEST_DIR = $(TOP_DIR)/build/tests/cvl
-CVL_TEST_BIN = $(CVL_TEST_DIR)/cvl.test
-CVL_TEST_SCHEMA_DIR = $(CVL_TEST_DIR)/testdata/schema
-CVL_TEST_SCHEMA     = $(CVL_TEST_SCHEMA_DIR)/.done
-CVL_TEST_YANGS      = $(wildcard testdata/schema/*.yang)
-CVL_TEST_YANGS     += $(wildcard $(YANG_SRC_DIR)/sonic/common/*.yang)
+SONIC_YANG_COMMON := $(TOPDIR)/models/yang/sonic/common
+CVL_TEST_SCHEMA   := $(CVL_TEST_SCHEMA_DIR)/.done
+CVL_TEST_YANGS     = $(shell find testdata/schema -name '*.yang')
+CVL_TEST_YANGS    += $(wildcard $(SONIC_YANG_COMMON)/*.yang)
 
-DEFAULT_TARGETS = $(CVL_SCHEMA)
+ifdef DEBUG
+	GOFLAGS += -gcflags="all=-N -l"
+endif
+
+DEFAULT_TARGETS = $(CVL_SCHEMA) $(STATIC_CHECK) $(FORMAT_CHECK)
 ifeq ($(NO_TEST_BINS),)
-DEFAULT_TARGETS += $(CVL_TEST_BIN)
+DEFAULT_TARGETS += $(CVL_TEST_BIN) $(CVL_TEST_SCHEMA)
 endif
 
 all: $(DEFAULT_TARGETS)
 
-.SECONDEXPANSION:
-
 .PRECIOUS: %/.
 %/.:
 	mkdir -p $@
 
-$(CVL_TEST_BIN): $(TEST_FILES) $(SRC_FILES) $(CVL_TEST_SCHEMA)
-	cp -r testdata/*.json $(@D)/testdata
-	$(GO) test -mod=vendor -cover -coverpkg=../cvl,../cvl/internal/util,../cvl/internal/yparser -c ../cvl -o $@
+.SECONDEXPANSION:
 
 .PHONY: schema
 schema: $(CVL_SCHEMA)
 
 $(CVL_SCHEMA): $(SONIC_YANG_FILES) | $$(@D)/.
-	$(TOP_DIR)/tools/pyang/generate_yin.py \
+	tools/generate_yin.py \
 		--path=$(SONIC_YANG_DIR) \
 		--path=$(YANG_SRC_DIR)/common \
 		--out-dir=$(@D)
 	touch $@
 
+$(CVL_TEST_BIN): $(TEST_FILES) $(SRC_FILES) | $$(@D)/testdata/.
+	cp -r testdata/*.json $(@D)/testdata
+	$(GO) test -mod=vendor -cover -coverpkg=../cvl,../cvl/internal/util,../cvl/internal/yparser -c ../cvl -o $@
+
 .PHONY: test-schema
 test-schema: $(CVL_TEST_SCHEMA)
 
 $(CVL_TEST_SCHEMA): $(CVL_TEST_YANGS) | $$(@D)/.
-	$(TOP_DIR)/tools/pyang/generate_yin.py \
+	tools/generate_yin.py \
 		--path=testdata/schema \
 		--path=$(YANG_SRC_DIR)/common \
 		--path=$(YANG_SRC_DIR)/sonic/common \
 		--out-dir=$(@D)
 	touch $@
 
-gotest: $(CVL_TEST_SCHEMA)
-	CVL_CFG_FILE=$(abspath .)/conf/cvl_cfg.json \
-	CVL_SCHEMA_PATH=$(abspath $(CVL_TEST_SCHEMA_DIR)) \
+$(CVL_TEST_CONFIG): conf/cvl_cfg.json
+	sed -E 's/((TRACE|LOG).*)\"false\"/\1\"true\"/' conf/cvl_cfg.json > $@
+
+$(CVL_TEST_DB_CONFIG): $(TOPDIR)/tools/test/database_config.json
+	$(TOPDIR)/tools/test/dbconfig.py -o $@
+
+gotest: $(CVL_TEST_SCHEMA) $(CVL_TEST_CONFIG) $(CVL_TEST_DB_CONFIG)
+	CVL_CFG_FILE=$(abspath $(CVL_TEST_CONFIG)) \
+	CVL_SCHEMA_PATH=$(CVL_TEST_SCHEMA_DIR) \
+	DB_CONFIG_PATH=$(abspath $(CVL_TEST_DB_CONFIG)) \
 	tests/run_test.sh
 
 clean:
-	$(RM) -r $(CVL_TEST_DIR) $(BUILD_DIR)
+	make -C tests clean
+	$(RM) -r $(BUILD_DIR)
+	$(RM) -r $(wildcard $(PKG_BUILD_DIR)/*/cvl)
+	$(RM) -r $(CVL_TEST_DIR)
 
 cleanall:clean
 

cvl/README.md

@@ -24,7 +24,7 @@ Below steps need to be done to enable CVL logging.
 	{
 		"TRACE_CACHE": "true",
 		"TRACE_LIBYANG": "true",
-		"TRACE_YPARSER": "true",
+		"TRACE_YPARSER": "true", 
 		"TRACE_CREATE": "true",
 		"TRACE_UPDATE": "true",
 		"TRACE_DELETE": "true",

cvl/common/db_utils.go

@@ -0,0 +1,64 @@
+package common
+
+// KeyMatch checks if the value matches a key pattern.
+// vIndex and pIndex are start positions of value and pattern strings to match.
+// Mimics redis pattern matcher - i.e, glob like pattern matcher which
+// matches '/' against wildcard.
+// Supports '*' and '?' wildcards with '\' as the escape character.
+// '*' matches any char sequence or none; '?' matches exactly one char.
+// Character classes are not supported (redis supports it).
+func KeyMatch(value, pattern string) bool {
+	return keyMatch(value, 0, pattern, 0)
+}
+
+func keyMatch(value string, vIndex int, pattern string, pIndex int) bool {
+	for pIndex < len(pattern) {
+		switch pattern[pIndex] {
+		case '*':
+			// Skip successive *'s in the pattern
+			pIndex++
+			for pIndex < len(pattern) && pattern[pIndex] == '*' {
+				pIndex++
+			}
+			// Pattern ends with *. Its a match always
+			if pIndex == len(pattern) {
+				return true
+			}
+			// Try to match remaining pattern with every value substring
+			for ; vIndex < len(value); vIndex++ {
+				if keyMatch(value, vIndex, pattern, pIndex) {
+					return true
+				}
+			}
+			// No match for remaining pattern
+			return false
+
+		case '?':
+			// Accept any char.. there should be at least one
+			if vIndex >= len(value) {
+				return false
+			}
+			vIndex++
+			pIndex++
+
+		case '\\':
+			// Do not treat \ as escape char if it is the last pattern char.
+			// Redis commands behave this way.
+			if pIndex+1 < len(pattern) {
+				pIndex++
+			}
+			fallthrough
+
+		default:
+			if vIndex >= len(value) || pattern[pIndex] != value[vIndex] {
+				return false
+			}
+			vIndex++
+			pIndex++
+		}
+	}
+
+	// All pattern chars have been compared.
+	// It is a match if all value chars have been exhausted too.
+	return (vIndex == len(value))
+}

cvl/custom_validation/common.go

@@ -20,13 +20,13 @@
 package custom_validation
 
 import (
+	"fmt"
 	"reflect"
 
 	"github.com/Azure/sonic-mgmt-common/cvl/common"
 	"github.com/Azure/sonic-mgmt-common/cvl/internal/util"
 	"github.com/Azure/sonic-mgmt-common/cvl/internal/yparser"
 	"github.com/antchfx/xmlquery"
-	"github.com/go-redis/redis/v7"
 )
 
 type CustomValidation struct{}
@@ -85,6 +85,7 @@ type CVLEditConfigData struct {
 	VOp       CVLOperation      //Operation type
 	Key       string            //Key format : "PORT|Ethernet4"
 	Data      map[string]string //Value :  {"alias": "40GE0/28", "mtu" : 9100,  "admin_status":  down}
+	ReplaceOp bool
 }
 
 // CVLErrorInfo CVL Error Structure
@@ -101,7 +102,8 @@ type CVLErrorInfo struct {
 }
 
 type CustValidationCache struct {
-	Data interface{}
+	Data map[string]interface{}
+	Hint map[string]interface{}
 }
 
 // CustValidationCtxt Custom validation context passed to custom validation function
@@ -112,7 +114,7 @@ type CustValidationCtxt struct {
 	YNodeVal  string               //YANG node value, leaf-list will have "," separated value
 	YCur      *xmlquery.Node       //YANG data tree
 	SessCache *CustValidationCache //Session cache, can be used for storing data, persistent in session
-	RClient   *redis.Client        //Redis client
+	RClient   common.DBAccess      //Db access interface
 }
 
 // Search criteria for advanced lookup through DBAccess APIs
@@ -137,3 +139,23 @@ func InvokeCustomValidation(cv *CustomValidation, name string, args ...interface
 
 	return CVLErrorInfo{ErrCode: CVL_SUCCESS}
 }
+
+func (err CVLErrorInfo) String() string {
+	var s string
+	if CVL_SUCCESS == err.ErrCode {
+		s = "Success"
+	} else {
+		s = fmt.Sprintf("ErrCode[%v]: ErrDetails[%s], Msg[%s], ConstraintErrMsg[%s], Table[%s:%v], Field[%s:%s]", err.ErrCode, err.CVLErrDetails, err.Msg, err.ConstraintErrMsg, err.TableName, err.Keys, err.Field, err.Value)
+	}
+
+	return s
+}
+
+func (vc *CustValidationCtxt) ContainsAnyFields(fields ...string) bool {
+	for _, f := range fields {
+		if _, ok := vc.CurCfg.Data[f]; ok {
+			return true
+		}
+	}
+	return false
+}

cvl/custom_validation/test_validations.go

@@ -0,0 +1,37 @@
+////////////////////////////////////////////////////////////////////////////////
+//                                                                            //
+//  Copyright 2019 Broadcom. The term Broadcom refers to Broadcom Inc. and/or //
+//  its subsidiaries.                                                         //
+//                                                                            //
+//  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.                                            //
+//                                                                            //
+////////////////////////////////////////////////////////////////////////////////
+
+//go:build test
+// +build test
+
+package custom_validation
+
+func (t *CustomValidation) ValidateIfExtraFieldValidationCalled(
+	vc *CustValidationCtxt) CVLErrorInfo {
+	vc.SessCache.Hint["ExtraFieldValidationCalled"] = true
+	return CVLErrorInfo{ErrCode: CVL_SUCCESS}
+
+}
+
+func (t *CustomValidation) ValidateIfListLevelValidationCalled(
+	vc *CustValidationCtxt) CVLErrorInfo {
+	vc.SessCache.Hint["ListLevelValidationCalled"] = true
+	return CVLErrorInfo{ErrCode: CVL_SUCCESS}
+
+}