Skip to content

Commit

Permalink
CVL error reporting enhancements (sonic-net#97)
Browse files Browse the repository at this point in the history
1) Enhanced cvl syntax validator to return consistent error response
when data validation fails. All syntax errors will have table, keys,
and error message filled. A validation error will also include errored
field name and value. ConstraintErrMsg and ErrAppTag are filled if the
sonic yang node has error-message and error-app-tag values.

2) Libyang is setup to fill the node path in ly_err_first() response.
Attempt to resolve table name, key values and field name from this
error path.

For sonic yangs, the libynag returned paths will be like
"/sonic-port:sonic-port/PORT/PORT_LIST[name='Ethernet0']/mtu".
Elem[1] will be the table name, elem[2] predicates will be the keys
and elem[3] will be the field name.

3) Try to resolve table, key and fied info from the error message if
libyang did not return an error path. If not possible, table and key
info are filled from the validation request data.

4) Enable error message prefixing in the cvl schema generator, which
adds a "[Error]" prefix to all the error-message strings in the schema
file. This helps cvl to differentiate user defined erorr messages from
linyang's messages (ly_err_item struct has a single field for both).
Cvl removes this prefix before filling it in ConstraintErrMsg field.

5) Modified existing test cases to verify all fields of CVLErrorInfo
instead of just success/error check

Signed-off-by: Sachin Holla <sachin.holla@broadcom.com>
  • Loading branch information
sachinholla authored Sep 18, 2023
1 parent 4a88fe7 commit 4cfc882
Show file tree
Hide file tree
Showing 13 changed files with 933 additions and 1,354 deletions.
2 changes: 0 additions & 2 deletions cvl/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ schema: $(CVL_SCHEMA)

$(CVL_SCHEMA): $(SONIC_YANG_FILES) | $$(@D)/.
$(TOP_DIR)/tools/pyang/generate_yin.py \
--no-err-prefix \
--path=$(SONIC_YANG_DIR) \
--path=$(YANG_SRC_DIR)/common \
--out-dir=$(@D)
Expand All @@ -69,7 +68,6 @@ test-schema: $(CVL_TEST_SCHEMA)

$(CVL_TEST_SCHEMA): $(CVL_TEST_YANGS) | $$(@D)/.
$(TOP_DIR)/tools/pyang/generate_yin.py \
--no-err-prefix \
--path=testdata/schema \
--path=$(YANG_SRC_DIR)/common \
--path=$(YANG_SRC_DIR)/sonic/common \
Expand Down
28 changes: 25 additions & 3 deletions cvl/cvl.go
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,13 @@ func splitRedisKey(key string) (string, string) {
return tblName, key[prefixLen:]
}

func splitKeyComponents(table, keyComps string) []string {
if m := modelInfo.tableInfo[table]; m != nil {
return strings.Split(keyComps, m.redisKeyDelim)
}
return nil
}

//Get the YANG list name from Redis key and table name
//This just returns same YANG list name as Redis table name
//when 1:1 mapping is there. For one Redis table to
Expand Down Expand Up @@ -878,12 +885,27 @@ func (c *CVL) validate (data *yparser.YParserNode) CVLRetCode {
return CVL_SUCCESS
}

func createCVLErrObj(errObj yparser.YParserError) CVLErrorInfo {
func createCVLErrObj(errObj yparser.YParserError, srcNode *jsonquery.Node) CVLErrorInfo {
errCode := CVLRetCode(errObj.ErrCode)
if errObj.ErrCode == yparser.YP_INTERNAL_UNKNOWN {
errCode = CVL_INTERNAL_UNKNOWN
}

// YParserError may not contain table or key info when creating a field (libyang issue)
// Fill the missing info from source json data tree
if srcNode != nil && srcNode.Parent != nil {
if len(errObj.TableName) == 0 {
errObj.TableName = srcNode.Parent.Data
errObj.Keys = splitKeyComponents(errObj.TableName, srcNode.Data)
} else if len(errObj.Keys) == 0 && errObj.TableName == srcNode.Parent.Data {
errObj.Keys = splitKeyComponents(errObj.TableName, srcNode.Data)
}
}

cvlErrObj := CVLErrorInfo {
TableName : errObj.TableName,
ErrCode : CVLRetCode(errObj.ErrCode),
CVLErrDetails : cvlErrorMap[CVLRetCode(errObj.ErrCode)],
ErrCode : errCode,
CVLErrDetails : cvlErrorMap[errCode],
Keys : errObj.Keys,
Value : errObj.Value,
Field : errObj.Field,
Expand Down
14 changes: 14 additions & 0 deletions cvl/cvl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,8 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
//Check max-element constraint
if ret := c.checkMaxElemConstraint(OP_CREATE, tbl); ret != CVL_SUCCESS {
cvlErrObj.ErrCode = CVL_SYNTAX_ERROR
cvlErrObj.TableName = tbl
cvlErrObj.Keys = splitKeyComponents(tbl, key)
cvlErrObj.ErrAppTag = "too-many-elements"
cvlErrObj.Msg = "Max elements limit reached"
cvlErrObj.CVLErrDetails = cvlErrorMap[cvlErrObj.ErrCode]
Expand All @@ -399,6 +401,9 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
for field := range cfgData[i].Data {
if (c.checkDeleteConstraint(cfgData, tbl, key, field) != CVL_SUCCESS) {
cvlErrObj.ErrCode = CVL_SEMANTIC_ERROR
cvlErrObj.TableName = tbl
cvlErrObj.Keys = splitKeyComponents(tbl, key)
cvlErrObj.Field = field
cvlErrObj.Msg = "Validation failed for Delete operation, given instance is in use"
cvlErrObj.CVLErrDetails = cvlErrorMap[cvlErrObj.ErrCode]
cvlErrObj.ErrAppTag = "instance-in-use"
Expand All @@ -411,6 +416,7 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
cvlErrObj.ErrCode = CVL_SEMANTIC_ERROR
cvlErrObj.Msg = "Mandatory field getting deleted"
cvlErrObj.TableName = tbl
cvlErrObj.Keys = splitKeyComponents(tbl, key)
cvlErrObj.Field = field
cvlErrObj.CVLErrDetails = cvlErrorMap[cvlErrObj.ErrCode]
cvlErrObj.ErrAppTag = "mandatory-field-delete"
Expand All @@ -427,6 +433,8 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
//Now check delete constraints
if (c.checkDeleteConstraint(cfgData, tbl, key, "") != CVL_SUCCESS) {
cvlErrObj.ErrCode = CVL_SEMANTIC_ERROR
cvlErrObj.TableName = tbl
cvlErrObj.Keys = splitKeyComponents(tbl, key)
cvlErrObj.Msg = "Validation failed for Delete operation, given instance is in use"
cvlErrObj.CVLErrDetails = cvlErrorMap[cvlErrObj.ErrCode]
cvlErrObj.ErrAppTag = "instance-in-use"
Expand Down Expand Up @@ -498,6 +506,8 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
CVL_LOG(WARNING, "\nValidateEditConfig(): Key = %s already exists", cfgData[i].Key)
cvlErrObj.ErrCode = CVL_SEMANTIC_KEY_ALREADY_EXIST
cvlErrObj.CVLErrDetails = cvlErrorMap[cvlErrObj.ErrCode]
cvlErrObj.TableName = tbl
cvlErrObj.Keys = splitKeyComponents(tbl, key)
return cvlErrObj, CVL_SEMANTIC_KEY_ALREADY_EXIST

} else {
Expand All @@ -514,6 +524,8 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
CVL_LOG(WARNING, "\nValidateEditConfig(): Key = %s does not exist", cfgData[i].Key)
cvlErrObj.ErrCode = CVL_SEMANTIC_KEY_NOT_EXIST
cvlErrObj.CVLErrDetails = cvlErrorMap[cvlErrObj.ErrCode]
cvlErrObj.TableName = tbl
cvlErrObj.Keys = splitKeyComponents(tbl, key)
return cvlErrObj, CVL_SEMANTIC_KEY_NOT_EXIST
}

Expand All @@ -530,6 +542,8 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
CVL_LOG(WARNING, "\nValidateEditConfig(): Key = %s does not exist", cfgData[i].Key)
cvlErrObj.ErrCode = CVL_SEMANTIC_KEY_NOT_EXIST
cvlErrObj.CVLErrDetails = cvlErrorMap[cvlErrObj.ErrCode]
cvlErrObj.TableName = tbl
cvlErrObj.Keys = splitKeyComponents(tbl, key)
return cvlErrObj, CVL_SEMANTIC_KEY_NOT_EXIST
}

Expand Down
85 changes: 85 additions & 0 deletions cvl/cvl_error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
////////////////////////////////////////////////////////////////////////////////
// //
// Copyright 2023 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. //
// //
////////////////////////////////////////////////////////////////////////////////

package cvl_test

import (
"reflect"
"strings"
"testing"

"github.com/Azure/sonic-mgmt-common/cvl"
)

const (
CVL_SUCCESS = cvl.CVL_SUCCESS
CVL_SYNTAX_ERROR = cvl.CVL_SYNTAX_ERROR
CVL_SEMANTIC_ERROR = cvl.CVL_SEMANTIC_ERROR
CVL_SYNTAX_MAXIMUM_INVALID = cvl.CVL_SYNTAX_MAXIMUM_INVALID
CVL_SYNTAX_MINIMUM_INVALID = cvl.CVL_SYNTAX_MINIMUM_INVALID
)

var Success = CVLErrorInfo{ErrCode: CVL_SUCCESS}

func compareErr(val, exp CVLErrorInfo) bool {
return (val.ErrCode == exp.ErrCode) &&
(len(exp.TableName) == 0 || val.TableName == exp.TableName) &&
(len(exp.Keys) == 0 || reflect.DeepEqual(val.Keys, exp.Keys)) &&
(len(exp.Field) == 0 || val.Field == exp.Field) &&
(len(exp.Value) == 0 || val.Value == exp.Value) &&
(len(exp.Msg) == 0 || val.Msg == exp.Msg) &&
(len(exp.CVLErrDetails) == 0 || val.CVLErrDetails == exp.CVLErrDetails) &&
(len(exp.ConstraintErrMsg) == 0 || val.ConstraintErrMsg == exp.ConstraintErrMsg) &&
(len(exp.ErrAppTag) == 0 || val.ErrAppTag == exp.ErrAppTag)
}

func verifyErr(t *testing.T, res, exp CVLErrorInfo) {
t.Helper()
expandMessagePatterns(&exp)
if !compareErr(res, exp) {
t.Fatalf("CVLErrorInfo verification failed\nExpected: %#v\nReceived: %#v", exp, res)
}
}

func verifyValidateEditConfig(t *testing.T, data []CVLEditConfigData, exp CVLErrorInfo) {
t.Helper()
c := NewTestSession(t)
res, _ := c.ValidateEditConfig(data)
verifyErr(t, res, exp)
}

func expandMessagePatterns(ex *CVLErrorInfo) {
switch ex.Msg {
case invalidValueErrMessage:
ex.Msg = strings.ReplaceAll(ex.Msg, "{{field}}", ex.Field)
ex.Msg = strings.ReplaceAll(ex.Msg, "{{value}}", ex.Value)
ex.Msg = strings.TrimSuffix(ex.Msg, " \"\"") // if value is empty
case unknownFieldErrMessage:
ex.Msg = strings.ReplaceAll(ex.Msg, "{{field}}", ex.Field)
}
}

const (
invalidValueErrMessage = "Field \"{{field}}\" has invalid value \"{{value}}\""
unknownFieldErrMessage = "Unknown field \"{{field}}\""
genericValueErrMessage = "Data validation failed"
mustExpressionErrMessage = "Must expression validation failed"
whenExpressionErrMessage = "When expression validation failed"
instanceInUseErrMessage = "Validation failed for Delete operation, given instance is in use"
)
Loading

0 comments on commit 4cfc882

Please sign in to comment.