Merge pull request #94 from openconfig/fix-type-update

Do not report type updates as invalid when incompatibilities are allowed
openconfig · Nov 20, 2023 · 05dd074 · 05dd074
2 parents f098639 + c8db1af
commit 05dd074
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 35 deletions.
diff --git a/openconfig-ci/ocdiff/ocdiff.go b/openconfig-ci/ocdiff/ocdiff.go
@@ -46,7 +46,7 @@ func NewDiffReport(oldpaths, newpaths, oldfiles, newfiles []string) (*DiffReport
 type yangNodeInfo struct {
 	path              string
 	schema            *yang.Entry
-	allowIncompat     bool
+	incompatAllowed   bool
 	versionChangeDesc string
 }
 
@@ -56,7 +56,7 @@ type yangNodeUpdateInfo struct {
 	path              string
 	oldSchema         *yang.Entry
 	newSchema         *yang.Entry
-	allowIncompat     bool
+	incompatAllowed   bool
 	versionChangeDesc string
 	incompatComments  []string
 }
@@ -113,13 +113,19 @@ func (r *DiffReport) Report(options ...Option) string {
 	}
 	var b strings.Builder
 	for _, del := range r.deletedNodes {
-		if !opts.onlyReportDisallowedIncompats || !del.allowIncompat {
-			if del.schema.IsLeaf() || del.schema.IsLeafList() {
-				b.WriteString(fmt.Sprintf(fmtstr, "leaf", "deleted", del.path, del.versionChangeDesc))
-			}
+		// All deletions are breaking changes.
+		if opts.onlyReportDisallowedIncompats && del.incompatAllowed {
+			continue
+		}
+		if del.schema.IsLeaf() || del.schema.IsLeafList() {
+			b.WriteString(fmt.Sprintf(fmtstr, "leaf", "deleted", del.path, del.versionChangeDesc))
 		}
 	}
 	for _, upd := range r.updatedNodes {
+		// All type updates are breaking changes.
+		if opts.onlyReportDisallowedIncompats && upd.incompatAllowed {
+			continue
+		}
 		nodeTypeDesc := "non-leaf"
 		if upd.oldSchema.IsLeaf() || upd.oldSchema.IsLeafList() {
 			nodeTypeDesc = "leaf"
@@ -175,7 +181,7 @@ func (r *DiffReport) getModuleAndVersions(e *yang.Entry) (string, *semver.Versio
 	return moduleName, r.oldModuleVersions[moduleName], r.newModuleVersions[moduleName]
 }
 
-func incompatAllowed(oldVersion, newVersion *semver.Version) bool {
+func isIncompatAllowed(oldVersion, newVersion *semver.Version) bool {
 	switch {
 	case oldVersion == nil, newVersion == nil:
 		// This can happen if the openconfig-version is not found (e.g. in IETF modules).
@@ -195,7 +201,7 @@ func incompatAllowed(oldVersion, newVersion *semver.Version) bool {
 func (r *DiffReport) addPair(o *yang.Entry, n *yang.Entry) error {
 	moduleName, oldVersion, newVersion := r.getModuleAndVersions(o)
 	versionChangeDesc := fmt.Sprintf("%q: openconfig-version %v -> %v", moduleName, oldVersion, newVersion)
-	allowIncompat := incompatAllowed(oldVersion, newVersion)
+	incompatAllowed := isIncompatAllowed(oldVersion, newVersion)
 
 	switch {
 	case o == nil && n == nil:
@@ -210,15 +216,15 @@ func (r *DiffReport) addPair(o *yang.Entry, n *yang.Entry) error {
 		r.deletedNodes = append(r.deletedNodes, &yangNodeInfo{
 			schema:            o,
 			path:              o.Path(),
-			allowIncompat:     allowIncompat,
+			incompatAllowed:   incompatAllowed,
 			versionChangeDesc: versionChangeDesc,
 		})
 	default:
 		upd := &yangNodeUpdateInfo{
 			oldSchema:         o,
 			newSchema:         n,
 			path:              o.Path(),
-			allowIncompat:     allowIncompat,
+			incompatAllowed:   incompatAllowed,
 			versionChangeDesc: versionChangeDesc,
 		}
 		updated := false

diff --git a/openconfig-ci/ocdiff/testdata/disallowed-incompats.txt b/openconfig-ci/ocdiff/testdata/disallowed-incompats.txt
@@ -1,7 +1,2 @@
 leaf deleted: /openconfig-platform/components/component/linecard/state/slot-id ("openconfig-platform-linecard": openconfig-version 1.1.0 -> 1.2.0)
-leaf updated: /openconfig-platform/components/component/chassis/utilization/resources/resource/state/used: type changed from uint64 to uint32 ("openconfig-platform": openconfig-version 0.23.0 -> 0.24.0)
-leaf updated: /openconfig-platform/components/component/integrated-circuit/utilization/resources/resource/state/used: type changed from uint64 to uint32 ("openconfig-platform": openconfig-version 0.23.0 -> 0.24.0)
 leaf updated: /openconfig-platform/components/component/linecard/state/colour: type changed from string to binary ("openconfig-platform-linecard": openconfig-version 1.1.0 -> 1.2.0)
-leaf updated: /openconfig-platform/components/component/linecard/utilization/resources/resource/state/used: type changed from uint64 to uint32 ("openconfig-platform": openconfig-version 0.23.0 -> 0.24.0)
-leaf updated: /openconfig-platform/components/component/port/breakout-mode/groups/group/config/num-physical-channels: type changed from uint8 to uint16 ("openconfig-platform-port": openconfig-version 1.0.1 -> 2.0.0)
-leaf updated: /openconfig-platform/components/component/port/breakout-mode/groups/group/state/num-physical-channels: type changed from uint8 to uint16 ("openconfig-platform-port": openconfig-version 1.0.1 -> 2.0.0)
diff --git a/openconfig-ci/ocdiff/testdata/github-comment-disallowed-incompats.txt b/openconfig-ci/ocdiff/testdata/github-comment-disallowed-incompats.txt
@@ -1,27 +1,7 @@
 leaf deleted: `/openconfig-platform/components/component/linecard/state/slot-id`
 * ("openconfig-platform-linecard": openconfig-version 1.1.0 -> 1.2.0)
 
-leaf updated: `/openconfig-platform/components/component/chassis/utilization/resources/resource/state/used`
-* type changed from uint64 to uint32
-* ("openconfig-platform": openconfig-version 0.23.0 -> 0.24.0)
-
-leaf updated: `/openconfig-platform/components/component/integrated-circuit/utilization/resources/resource/state/used`
-* type changed from uint64 to uint32
-* ("openconfig-platform": openconfig-version 0.23.0 -> 0.24.0)
-
 leaf updated: `/openconfig-platform/components/component/linecard/state/colour`
 * type changed from string to binary
 * ("openconfig-platform-linecard": openconfig-version 1.1.0 -> 1.2.0)
 
-leaf updated: `/openconfig-platform/components/component/linecard/utilization/resources/resource/state/used`
-* type changed from uint64 to uint32
-* ("openconfig-platform": openconfig-version 0.23.0 -> 0.24.0)
-
-leaf updated: `/openconfig-platform/components/component/port/breakout-mode/groups/group/config/num-physical-channels`
-* type changed from uint8 to uint16
-* ("openconfig-platform-port": openconfig-version 1.0.1 -> 2.0.0)
-
-leaf updated: `/openconfig-platform/components/component/port/breakout-mode/groups/group/state/num-physical-channels`
-* type changed from uint8 to uint16
-* ("openconfig-platform-port": openconfig-version 1.0.1 -> 2.0.0)
-