Fix bug where simple union enums causes panic within ygot.Diff. (#461)

Missing coverage added.
openconfig · Sep 30, 2020 · fc02f43 · fc02f43
1 parent 003238b
commit fc02f43
Show file tree

Hide file tree

Showing 2 changed files with 110 additions and 1 deletion.
diff --git a/ygot/diff.go b/ygot/diff.go
@@ -272,7 +272,13 @@ func findSetLeaves(s GoStruct, opts ...DiffOpt) (map[*pathSpec]interface{}, erro
 		// If this is an enumerated value in the output structs, then check whether
 		// it is set. Only include values that are set to a non-zero value.
 		if _, isEnum := ival.(GoEnum); isEnum {
-			if ni.FieldValue.Int() == 0 {
+			val := ni.FieldValue
+			// If the value is a simple union enum, then extract
+			// the underlying enum value from the interface.
+			if val.Kind() == reflect.Interface {
+				val = val.Elem()
+			}
+			if val.Int() == 0 {
 				return
 			}
 		}

diff --git a/ygot/diff_test.go b/ygot/diff_test.go
@@ -911,6 +911,7 @@ func TestDiff(t *testing.T) {
 				testutil.UnionString("hello"),
 				testutil.UnionInt64(42),
 				testutil.UnionFloat64(3.14),
+				EnumTestVALONE,
 				testBinary,
 				testutil.UnionBool(true),
 				testutil.YANGEmpty(false),
@@ -991,6 +992,7 @@ func TestDiff(t *testing.T) {
 							{Value: &gnmipb.TypedValue_StringVal{"hello"}},
 							{Value: &gnmipb.TypedValue_IntVal{42}},
 							{Value: &gnmipb.TypedValue_FloatVal{3.14}},
+							{Value: &gnmipb.TypedValue_StringVal{"VAL_ONE"}},
 							{Value: &gnmipb.TypedValue_BytesVal{[]byte(base64testString)}},
 							{Value: &gnmipb.TypedValue_BoolVal{true}},
 							{Value: &gnmipb.TypedValue_BoolVal{false}}},
@@ -1012,6 +1014,102 @@ func TestDiff(t *testing.T) {
 				Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_BoolVal{true}},
 			}},
 		},
+	}, {
+		desc:   "union addition: enum",
+		inOrig: &renderExample{},
+		inMod: &renderExample{
+			UnionValSimple: EnumTestVALONE,
+		},
+		want: &gnmipb.Notification{
+			Update: []*gnmipb.Update{{
+				Path: &gnmipb.Path{
+					Elem: []*gnmipb.PathElem{{
+						Name: "union-val-simple",
+					}},
+				},
+				Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{"VAL_ONE"}},
+			}},
+		},
+	}, {
+		desc:   "union addition: int64",
+		inOrig: &renderExample{},
+		inMod: &renderExample{
+			UnionValSimple: testutil.UnionInt64(1),
+		},
+		want: &gnmipb.Notification{
+			Update: []*gnmipb.Update{{
+				Path: &gnmipb.Path{
+					Elem: []*gnmipb.PathElem{{
+						Name: "union-val-simple",
+					}},
+				},
+				Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_IntVal{1}},
+			}},
+		},
+	}, {
+		desc:   "union addition: float64",
+		inOrig: &renderExample{},
+		inMod: &renderExample{
+			UnionValSimple: testutil.UnionFloat64(3.14),
+		},
+		want: &gnmipb.Notification{
+			Update: []*gnmipb.Update{{
+				Path: &gnmipb.Path{
+					Elem: []*gnmipb.PathElem{{
+						Name: "union-val-simple",
+					}},
+				},
+				Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_FloatVal{3.14}},
+			}},
+		},
+	}, {
+		desc:   "union addition: bool",
+		inOrig: &renderExample{},
+		inMod: &renderExample{
+			UnionValSimple: testutil.UnionBool(true),
+		},
+		want: &gnmipb.Notification{
+			Update: []*gnmipb.Update{{
+				Path: &gnmipb.Path{
+					Elem: []*gnmipb.PathElem{{
+						Name: "union-val-simple",
+					}},
+				},
+				Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_BoolVal{true}},
+			}},
+		},
+	}, {
+		desc:   "union addition: empty",
+		inOrig: &renderExample{},
+		inMod: &renderExample{
+			UnionValSimple: testutil.YANGEmpty(true),
+		},
+		want: &gnmipb.Notification{
+			Update: []*gnmipb.Update{{
+				Path: &gnmipb.Path{
+					Elem: []*gnmipb.PathElem{{
+						Name: "union-val-simple",
+					}},
+				},
+				Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_BoolVal{true}},
+			}},
+		},
+	}, {
+		desc:   "union addition: binary",
+		inOrig: &renderExample{},
+		inMod: &renderExample{
+			UnionValSimple: testBinary,
+		},
+		want: &gnmipb.Notification{
+			Update: []*gnmipb.Update{{
+				Path: &gnmipb.Path{
+					Elem: []*gnmipb.PathElem{{
+						Name: "union-val-simple",
+					}},
+				},
+				Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_BytesVal{[]byte(base64testString)}},
+			}},
+		},
 	}, {
 		desc: "multiple element set in both - no diff",
 		inOrig: &renderExample{
@@ -1028,6 +1126,7 @@ func TestDiff(t *testing.T) {
 				testutil.UnionString("hello"),
 				testutil.UnionInt64(42),
 				testutil.UnionFloat64(3.14),
+				EnumTestVALONE,
 				testBinary,
 				testutil.UnionBool(true),
 				testutil.YANGEmpty(false),
@@ -1049,6 +1148,7 @@ func TestDiff(t *testing.T) {
 				testutil.UnionString("hello"),
 				testutil.UnionInt64(42),
 				testutil.UnionFloat64(3.14),
+				EnumTestVALONE,
 				testBinary,
 				testutil.UnionBool(true),
 				testutil.YANGEmpty(false),
@@ -1073,6 +1173,7 @@ func TestDiff(t *testing.T) {
 				testutil.UnionString("hello"),
 				testutil.UnionInt64(42),
 				testutil.UnionFloat64(3.14),
+				EnumTestVALONE,
 				testBinary,
 				testutil.UnionBool(true),
 				testutil.YANGEmpty(false),
@@ -1094,6 +1195,7 @@ func TestDiff(t *testing.T) {
 				testutil.UnionString("world"),
 				testutil.UnionInt64(84),
 				testutil.UnionFloat64(6.28),
+				EnumTestVALTWO,
 				testBinary1,
 				testutil.UnionBool(false),
 				testutil.YANGEmpty(true),
@@ -1174,6 +1276,7 @@ func TestDiff(t *testing.T) {
 							{Value: &gnmipb.TypedValue_StringVal{"world"}},
 							{Value: &gnmipb.TypedValue_IntVal{84}},
 							{Value: &gnmipb.TypedValue_FloatVal{6.28}},
+							{Value: &gnmipb.TypedValue_StringVal{"VAL_TWO"}},
 							{Value: &gnmipb.TypedValue_BytesVal{[]byte("abc")}},
 							{Value: &gnmipb.TypedValue_BoolVal{false}},
 							{Value: &gnmipb.TypedValue_BoolVal{true}}},