state, filter: Fix the interpretation of ifaces names (#709)

* state: Interpret basic state through an explicit structure

In order to ease the interpretation of the state yaml data, use a type
structure that represents the basic root items of state:
- interfaces
- routes:
  - config
  - running

In order to keep the generic nature of these items content,
they use as value a generic Go interface{}.

The filter helper functions, with this change, have been also made
immutable. They now return the filtered values.

Signed-off-by: Edward Haas <[email protected]>

* state, filter: Introduce unit tests for numeric iface names

Interfaces names with numeric values are accepted by the kernel.
Introduce tests that checks the behavior when using numeric names.

Signed-off-by: Edward Haas <[email protected]>

* state, filter: Do not panic on interpretation of ifaces names

In scenarios where an interface name is composed of numbers in
scientific notation without a dot, the application panics with a failure
to extract a string type from a Go interface (which is actually a float64).

In order to fix the problem, the parsing of the interface name is performed
using a dedicated interface-state structure.

Note: this change does not solve the problem of incorrectly representing
valid scientific numeric values like `10e+02` as strings (they are now
represented back as full integers: "1000").
This is due to the YAML-to-JSON conversion which does not obey to
the defined member type.

ref: yaml/pyyaml#173

Signed-off-by: Edward Haas <[email protected]>

* state, filter: Fix interpretation of scientific numeric iface names

This change solves the problem of incorrectly representing
valid scientific numeric valuesi without a dot in them (e.g. `10e20`)
as strings (e.g. represented back as `1000`).

The problem originates from the YAML-to-JSON conversion which does not obey
to the defined member type.

Fixed by using the go-yaml package (which does not perform such a
conversion from YAML to JSON).
The go-yaml is used just to unmarshal the name from the original raw
byte stream and update the state structure with the proper name string.

Signed-off-by: Edward Haas <[email protected]>

pkg/state/filter.go

@@ -7,6 +7,7 @@ import (
 	"github.com/nmstate/kubernetes-nmstate/api/shared"
 	"github.com/nmstate/kubernetes-nmstate/pkg/environment"
 
+	goyaml "gopkg.in/yaml.v2"
 	yaml "sigs.k8s.io/yaml"
 )
 
@@ -29,32 +30,16 @@ func FilterOut(currentState shared.State) (shared.State, error) {
 	return filterOut(currentState, interfacesFilterGlobFromEnv)
 }
 
-func filterOutRoutes(kind string, state map[string]interface{}, interfacesFilterGlob glob.Glob) {
-	routesRaw, hasRoutes := state["routes"]
-	if !hasRoutes {
-		return
-	}
-
-	routes, ok := routesRaw.(map[string]interface{})
-	if !ok {
-		return
-	}
-
-	routesByKind := routes[kind].([]interface{})
-
-	if routesByKind == nil {
-		return
-	}
-
+func filterOutRoutes(routes []interface{}, interfacesFilterGlob glob.Glob) []interface{} {
 	filteredRoutes := []interface{}{}
-	for _, route := range routesByKind {
+	for _, route := range routes {
 		name := route.(map[string]interface{})["next-hop-interface"]
 		if !interfacesFilterGlob.Match(name.(string)) {
 			filteredRoutes = append(filteredRoutes, route)
 		}
 	}
 
-	state["routes"].(map[string]interface{})[kind] = filteredRoutes
+	return filteredRoutes
 }
 
 func filterOutDynamicAttributes(iface map[string]interface{}) {
@@ -89,35 +74,49 @@ func filterOutDynamicAttributes(iface map[string]interface{}) {
 	delete(options, "hello-timer")
 }
 
-func filterOutInterfaces(state map[string]interface{}, interfacesFilterGlob glob.Glob) {
-	interfaces := state["interfaces"]
-	filteredInterfaces := []interface{}{}
-
-	for _, iface := range interfaces.([]interface{}) {
-		name := iface.(map[string]interface{})["name"]
-		if !interfacesFilterGlob.Match(name.(string)) {
-			filterOutDynamicAttributes(iface.(map[string]interface{}))
+func filterOutInterfaces(ifacesState []interfaceState, interfacesFilterGlob glob.Glob) []interfaceState {
+	filteredInterfaces := []interfaceState{}
+	for _, iface := range ifacesState {
+		if !interfacesFilterGlob.Match(iface.Name) {
+			filterOutDynamicAttributes(iface.Data)
 			filteredInterfaces = append(filteredInterfaces, iface)
 		}
 	}
-	state["interfaces"] = filteredInterfaces
+	return filteredInterfaces
 }
 
 func filterOut(currentState shared.State, interfacesFilterGlob glob.Glob) (shared.State, error) {
-	var state map[string]interface{}
-	err := yaml.Unmarshal(currentState.Raw, &state)
-	if err != nil {
+	var state rootState
+	if err := yaml.Unmarshal(currentState.Raw, &state); err != nil {
 		return currentState, err
 	}
 
-	filterOutInterfaces(state, interfacesFilterGlob)
-	filterOutRoutes("running", state, interfacesFilterGlob)
-	filterOutRoutes("config", state, interfacesFilterGlob)
+	if err := normalizeInterfacesNames(currentState.Raw, &state); err != nil {
+		return currentState, err
+	}
 
+	state.Interfaces = filterOutInterfaces(state.Interfaces, interfacesFilterGlob)
+	if state.Routes != nil {
+		state.Routes.Running = filterOutRoutes(state.Routes.Running, interfacesFilterGlob)
+		state.Routes.Config = filterOutRoutes(state.Routes.Config, interfacesFilterGlob)
+	}
 	filteredState, err := yaml.Marshal(state)
 	if err != nil {
 		return currentState, err
 	}
 
 	return shared.NewState(string(filteredState)), nil
 }
+
+// normalizeInterfacesNames fixes the unmarshal of numeric values in the interfaces names
+// Numeric values, including the ones with a base prefix (e.g. 0x123) should be stringify.
+func normalizeInterfacesNames(rawState []byte, state *rootState) error {
+	var stateForNormalization rootState
+	if err := goyaml.Unmarshal(rawState, &stateForNormalization); err != nil {
+		return err
+	}
+	for i, iface := range stateForNormalization.Interfaces {
+		state.Interfaces[i].Name = iface.Name
+	}
+	return nil
+}

pkg/state/filter_test.go

@@ -331,4 +331,56 @@ interfaces:
 		})
 	})
 
+	Context("when the interfaces has numeric characters quoted", func() {
+		BeforeEach(func() {
+			state = nmstate.NewState(`
+interfaces:
+- name: eth0
+- name: '0'
+- name: '1101010'
+- name: '0.0'
+- name: '1.0'
+- name: '0xfe'
+- name: '60.e+02'
+`)
+			filteredState = nmstate.NewState(`
+interfaces:
+- name: eth0
+- name: '1101010'
+- name: '1.0'
+- name: '60.e+02'
+`)
+			interfacesFilterGlob = glob.MustCompile("0*")
+		})
+
+		It("should filter out interfaces correctly", func() {
+			returnedState, err := filterOut(state, interfacesFilterGlob)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(returnedState).To(MatchYAML(filteredState))
+		})
+	})
+
+	// See https://github.com/yaml/pyyaml/issues/173 for why this scenario is checked.
+	Context("when the interfaces names have numbers in scientific notation without dot", func() {
+		BeforeEach(func() {
+			state = nmstate.NewState(`
+interfaces:
+- name: eth0
+- name: 10e+02
+- name: 60e+02
+`)
+			filteredState = nmstate.NewState(`
+interfaces:
+- name: eth0
+- name: "60e+02"
+`)
+			interfacesFilterGlob = glob.MustCompile("10e*")
+		})
+
+		It("does not filter out interfaces correctly and does not represent them correctly", func() {
+			returnedState, err := filterOut(state, interfacesFilterGlob)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(returnedState).To(MatchYAML(filteredState))
+		})
+	})
 })

pkg/state/type.go

@@ -0,0 +1,46 @@
+package state
+
+import (
+	"encoding/json"
+	"fmt"
+	"sigs.k8s.io/yaml"
+)
+
+type rootState struct {
+	Interfaces []interfaceState `json:"interfaces" yaml:"interfaces"`
+	Routes     *routesState     `json:"routes,omitempty" yaml:"routes,omitempty"`
+}
+
+type routesState struct {
+	Config  []interface{} `json:"config" yaml:"config"`
+	Running []interface{} `json:"running" yaml:"running"`
+}
+
+type interfaceState struct {
+	interfaceFields `yaml:",inline"`
+	Data            map[string]interface{}
+}
+
+// interfaceFields allows unmarshaling directly into the defined fields
+type interfaceFields struct {
+	Name string `json:"name" yaml:"name"`
+}
+
+func (i interfaceState) MarshalJSON() (output []byte, err error) {
+	i.Data["name"] = i.Name
+	return json.Marshal(i.Data)
+}
+
+func (i *interfaceState) UnmarshalJSON(b []byte) error {
+	if err := yaml.Unmarshal(b, &i.Data); err != nil {
+		return fmt.Errorf("failed Unmarshaling b: %w", err)
+	}
+
+	var ifaceFields interfaceFields
+	if err := yaml.Unmarshal(b, &ifaceFields); err != nil {
+		return fmt.Errorf("failed Unmarshaling raw: %w", err)
+	}
+	i.Data["name"] = ifaceFields.Name
+	i.interfaceFields = ifaceFields
+	return nil
+}