MAC address json unmarshal should allow strings

Create a new mac address type which supports json marshal/unmarshal from
and to string. This change is backwards compatible with the previous
versions as the unmarshal method still accepts the old byte array or
base64 encoded string.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

11 files changed, 148 insertions, 16 deletions

diff --git a/libpod/container_config.go b/libpod/container_config.go
index 33ea731fd..8f803d744 100644
--- a/libpod/container_config.go
+++ b/libpod/container_config.go
@@ -228,7 +228,7 @@ type ContainerNetworkConfig struct {
 	// StaticMAC is a static MAC to request for the container.
 	// This cannot be set unless CreateNetNS is set.
 	// If not set, the container will be dynamically assigned a MAC by CNI.
-	StaticMAC net.HardwareAddr `json:"staticMAC"`
+	StaticMAC types.HardwareAddr `json:"staticMAC"`
 	// PortMappings are the ports forwarded to the container's network
 	// namespace
 	// These are not used unless CreateNetNS is true
diff --git a/libpod/network/cni/run.go b/libpod/network/cni/run.go
index 99b2adce5..7795dfeeb 100644
--- a/libpod/network/cni/run.go
+++ b/libpod/network/cni/run.go
@@ -160,7 +160,7 @@ func CNIResultToStatus(res cnitypes.Result) (types.StatusBlock, error) {
 				return result, err
 			}
 			interfaces[cniInt.Name] = types.NetInterface{
-				MacAddress: mac,
+				MacAddress: types.HardwareAddr(mac),
 				Networks: []types.NetAddress{{
 					Subnet:  types.IPNet{IPNet: ip.Address},
 					Gateway: ip.Gateway,
diff --git a/libpod/network/cni/run_test.go b/libpod/network/cni/run_test.go
index 965203c2a..3169cd0eb 100644
--- a/libpod/network/cni/run_test.go
+++ b/libpod/network/cni/run_test.go
@@ -398,7 +398,7 @@ var _ = Describe("run CNI", func() {
 					i, err := net.InterfaceByName(intName1)
 					Expect(err).To(BeNil())
 					Expect(i.Name).To(Equal(intName1))
-					Expect(i.HardwareAddr).To(Equal(macInt1))
+					Expect(i.HardwareAddr).To(Equal((net.HardwareAddr)(macInt1)))
 					addrs, err := i.Addrs()
 					Expect(err).To(BeNil())
 					subnet := &net.IPNet{
@@ -448,7 +448,7 @@ var _ = Describe("run CNI", func() {
 					i, err := net.InterfaceByName(intName1)
 					Expect(err).To(BeNil())
 					Expect(i.Name).To(Equal(intName1))
-					Expect(i.HardwareAddr).To(Equal(macInt1))
+					Expect(i.HardwareAddr).To(Equal(net.HardwareAddr(macInt1)))
 					addrs, err := i.Addrs()
 					Expect(err).To(BeNil())
 					subnet := &net.IPNet{
@@ -460,7 +460,7 @@ var _ = Describe("run CNI", func() {
 					i, err = net.InterfaceByName(intName2)
 					Expect(err).To(BeNil())
 					Expect(i.Name).To(Equal(intName2))
-					Expect(i.HardwareAddr).To(Equal(macInt2))
+					Expect(i.HardwareAddr).To(Equal(net.HardwareAddr(macInt2)))
 					addrs, err = i.Addrs()
 					Expect(err).To(BeNil())
 					subnet = &net.IPNet{
@@ -600,7 +600,7 @@ var _ = Describe("run CNI", func() {
 					i, err := net.InterfaceByName(intName1)
 					Expect(err).To(BeNil())
 					Expect(i.Name).To(Equal(intName1))
-					Expect(i.HardwareAddr).To(Equal(macInt1))
+					Expect(i.HardwareAddr).To(Equal(net.HardwareAddr(macInt1)))
 					addrs, err := i.Addrs()
 					Expect(err).To(BeNil())
 					subnet := &net.IPNet{
@@ -612,7 +612,7 @@ var _ = Describe("run CNI", func() {
 					i, err = net.InterfaceByName(intName2)
 					Expect(err).To(BeNil())
 					Expect(i.Name).To(Equal(intName2))
-					Expect(i.HardwareAddr).To(Equal(macInt2))
+					Expect(i.HardwareAddr).To(Equal(net.HardwareAddr(macInt2)))
 					addrs, err = i.Addrs()
 					Expect(err).To(BeNil())
 					subnet = &net.IPNet{
@@ -690,7 +690,7 @@ var _ = Describe("run CNI", func() {
 							netName: {
 								InterfaceName: interfaceName,
 								StaticIPs:     []net.IP{ip1, ip2},
-								StaticMAC:     mac,
+								StaticMAC:     types.HardwareAddr(mac),
 							},
 						},
 					},
@@ -708,7 +708,7 @@ var _ = Describe("run CNI", func() {
 				Expect(res[netName].Interfaces[interfaceName].Networks[1].Subnet.IP.String()).To(Equal(ip2.String()))
 				Expect(res[netName].Interfaces[interfaceName].Networks[1].Subnet.Mask).To(Equal(subnet2.Mask))
 				Expect(res[netName].Interfaces[interfaceName].Networks[1].Gateway).To(Equal(net.ParseIP("fd41:0a75:2ca0:48a9::1")))
-				Expect(res[netName].Interfaces[interfaceName].MacAddress).To(Equal(mac))
+				Expect(res[netName].Interfaces[interfaceName].MacAddress).To(Equal(types.HardwareAddr(mac)))
 				// default network has no dns
 				Expect(res[netName].DNSServerIPs).To(BeEmpty())
 				Expect(res[netName].DNSSearchDomains).To(BeEmpty())
diff --git a/libpod/network/types/network.go b/libpod/network/types/network.go
index 657c1ca6a..5cf523e20 100644
--- a/libpod/network/types/network.go
+++ b/libpod/network/types/network.go
@@ -1,6 +1,7 @@
 package types
 
 import (
+	"encoding/json"
 	"net"
 	"time"
 )
@@ -94,6 +95,50 @@ func (n *IPNet) UnmarshalText(text []byte) error {
 	return nil
 }
 
+// HardwareAddr is the same as net.HardwareAddr except
+// that it adds the json marshal/unmarshal methods.
+// This allows us to read the mac from a json string
+// and a byte array.
+type HardwareAddr net.HardwareAddr
+
+func (h *HardwareAddr) String() string {
+	return (*net.HardwareAddr)(h).String()
+}
+
+func (h *HardwareAddr) MarshalText() ([]byte, error) {
+	return []byte((*net.HardwareAddr)(h).String()), nil
+}
+
+func (h *HardwareAddr) UnmarshalJSON(text []byte) error {
+	if len(text) == 0 {
+		*h = nil
+		return nil
+	}
+
+	// if the json string start with a quote we got a string
+	// unmarshal the string and parse the mac from this string
+	if string(text[0]) == `"` {
+		var macString string
+		err := json.Unmarshal(text, &macString)
+		if err == nil {
+			mac, err := net.ParseMAC(macString)
+			if err == nil {
+				*h = HardwareAddr(mac)
+				return nil
+			}
+		}
+	}
+	// not a string or got an error fallback to the normal parsing
+	mac := make(net.HardwareAddr, 0, 6)
+	// use the standard json unmarshal for backwards compat
+	err := json.Unmarshal(text, &mac)
+	if err != nil {
+		return err
+	}
+	*h = HardwareAddr(mac)
+	return nil
+}
+
 type Subnet struct {
 	// Subnet for this Network in CIDR form.
 	// swagger:strfmt string
@@ -134,7 +179,7 @@ type NetInterface struct {
 	// Networks list of assigned subnets with their gateway.
 	Networks []NetAddress `json:"networks,omitempty"`
 	// MacAddress for this Interface.
-	MacAddress net.HardwareAddr `json:"mac_address"`
+	MacAddress HardwareAddr `json:"mac_address"`
 }
 
 // NetAddress contains the subnet and gateway.
@@ -157,7 +202,7 @@ type PerNetworkOptions struct {
 	// Optional.
 	Aliases []string `json:"aliases,omitempty"`
 	// StaticMac for this container. Optional.
-	StaticMAC net.HardwareAddr `json:"static_mac,omitempty"`
+	StaticMAC HardwareAddr `json:"static_mac,omitempty"`
 	// InterfaceName for this container. Required.
 	InterfaceName string `json:"interface_name"`
 }
diff --git a/libpod/network/types/network_test.go b/libpod/network/types/network_test.go
new file mode 100644
index 000000000..91ee93692
--- /dev/null
+++ b/libpod/network/types/network_test.go
@@ -0,0 +1,82 @@
+package types_test
+
+import (
+	"encoding/json"
+	"reflect"
+	"testing"
+
+	"github.com/containers/podman/v3/libpod/network/types"
+)
+
+func TestUnmarshalMacAddress(t *testing.T) {
+	tests := []struct {
+		name    string
+		json    string
+		want    types.HardwareAddr
+		wantErr bool
+	}{
+		{
+			name: "mac as string with colon",
+			json: `"52:54:00:1c:2e:46"`,
+			want: types.HardwareAddr{0x52, 0x54, 0x00, 0x1c, 0x2e, 0x46},
+		},
+		{
+			name: "mac as string with dash",
+			json: `"52-54-00-1c-2e-46"`,
+			want: types.HardwareAddr{0x52, 0x54, 0x00, 0x1c, 0x2e, 0x46},
+		},
+		{
+			name: "mac as byte array",
+			json: `[82, 84, 0, 28, 46, 70]`,
+			want: types.HardwareAddr{0x52, 0x54, 0x00, 0x1c, 0x2e, 0x46},
+		},
+		{
+			name: "null value",
+			json: `null`,
+			want: nil,
+		},
+		{
+			name: "mac as base64",
+			json: `"qrvM3e7/"`,
+			want: types.HardwareAddr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff},
+		},
+		{
+			name:    "invalid string",
+			json:    `"52:54:00:1c:2e`,
+			wantErr: true,
+		},
+		{
+			name:    "invalid array",
+			json:    `[82, 84, 0, 28, 46`,
+			wantErr: true,
+		},
+
+		{
+			name:    "invalid value",
+			json:    `ab`,
+			wantErr: true,
+		},
+		{
+			name:    "invalid object",
+			json:    `{}`,
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		test := tt
+		t.Run(test.name, func(t *testing.T) {
+			mac := types.HardwareAddr{}
+			err := json.Unmarshal([]byte(test.json), &mac)
+			if (err != nil) != test.wantErr {
+				t.Errorf("types.HardwareAddress Unmarshal() error = %v, wantErr %v", err, test.wantErr)
+				return
+			}
+			if test.wantErr {
+				return
+			}
+			if !reflect.DeepEqual(mac, test.want) {
+				t.Errorf("types.HardwareAddress Unmarshal() got = %v, want %v", mac, test.want)
+			}
+		})
+	}
+}
diff --git a/libpod/options.go b/libpod/options.go
index 135b2f363..f1cf015f8 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -1104,7 +1104,7 @@ func WithNetworkOptions(options map[string][]string) CtrCreateOption {
 // It cannot be set unless WithNetNS has already been passed.
 // Further, it cannot be set if additional CNI networks to join have been
 // specified.
-func WithStaticMAC(mac net.HardwareAddr) CtrCreateOption {
+func WithStaticMAC(mac nettypes.HardwareAddr) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
 			return define.ErrCtrFinalized
diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go
index 3d8579acf..1df18be58 100644
--- a/pkg/domain/entities/pods.go
+++ b/pkg/domain/entities/pods.go
@@ -7,6 +7,7 @@ import (
 
 	commonFlag "github.com/containers/common/pkg/flag"
 	"github.com/containers/podman/v3/libpod/define"
+	"github.com/containers/podman/v3/libpod/network/types"
 	"github.com/containers/podman/v3/pkg/specgen"
 	"github.com/containers/podman/v3/pkg/util"
 	"github.com/opencontainers/runtime-spec/specs-go"
@@ -318,7 +319,8 @@ func ToPodSpecGen(s specgen.PodSpecGenerator, p *PodCreateOptions) (*specgen.Pod
 	if p.Net != nil {
 		s.NetNS = p.Net.Network
 		s.StaticIP = p.Net.StaticIP
-		s.StaticMAC = p.Net.StaticMAC
+		// type cast to types.HardwareAddr
+		s.StaticMAC = (*types.HardwareAddr)(p.Net.StaticMAC)
 		s.PortMappings = p.Net.PublishPorts
 		s.CNINetworks = p.Net.CNINetworks
 		s.NetworkOptions = p.Net.NetworkOptions
diff --git a/pkg/specgen/podspecgen.go b/pkg/specgen/podspecgen.go
index 7713ea26c..32d5be79a 100644
--- a/pkg/specgen/podspecgen.go
+++ b/pkg/specgen/podspecgen.go
@@ -99,7 +99,7 @@ type PodNetworkConfig struct {
 	// Only available if NetNS is set to Bridge (the default for root).
 	// As such, conflicts with NoInfra=true by proxy.
 	// Optional.
-	StaticMAC *net.HardwareAddr `json:"static_mac,omitempty"`
+	StaticMAC *types.HardwareAddr `json:"static_mac,omitempty"`
 	// PortMappings is a set of ports to map into the infra container.
 	// As, by default, containers share their network with the infra
 	// container, this will forward the ports to the entire pod.
diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go
index 5a07af0f9..593d91c64 100644
--- a/pkg/specgen/specgen.go
+++ b/pkg/specgen/specgen.go
@@ -401,7 +401,7 @@ type ContainerNetworkConfig struct {
 	// StaticMAC is a static MAC address to set in the container.
 	// Only available if NetNS is set to bridge.
 	// Optional.
-	StaticMAC *net.HardwareAddr `json:"static_mac,omitempty"`
+	StaticMAC *nettypes.HardwareAddr `json:"static_mac,omitempty"`
 	// PortBindings is a set of ports to map into the container.
 	// Only available if NetNS is set to bridge or slirp.
 	// Optional.
diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go
index 683cd2918..4e8f954fb 100644
--- a/pkg/specgenutil/specgen.go
+++ b/pkg/specgenutil/specgen.go
@@ -11,6 +11,7 @@ import (
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/podman/v3/cmd/podman/parse"
 	"github.com/containers/podman/v3/libpod/define"
+	"github.com/containers/podman/v3/libpod/network/types"
 	ann "github.com/containers/podman/v3/pkg/annotations"
 	"github.com/containers/podman/v3/pkg/domain/entities"
 	envLib "github.com/containers/podman/v3/pkg/env"
@@ -457,7 +458,8 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 		s.DNSSearch = c.Net.DNSSearch
 		s.DNSOptions = c.Net.DNSOptions
 		s.StaticIP = c.Net.StaticIP
-		s.StaticMAC = c.Net.StaticMAC
+		// type cast to types.HardwareAddr
+		s.StaticMAC = (*types.HardwareAddr)(c.Net.StaticMAC)
 		s.NetworkOptions = c.Net.NetworkOptions
 		s.UseImageHosts = c.Net.NoHosts
 	}
diff --git a/test/upgrade/test-upgrade.bats b/test/upgrade/test-upgrade.bats
index 5cb302a85..e9910f3d2 100644
--- a/test/upgrade/test-upgrade.bats
+++ b/test/upgrade/test-upgrade.bats
@@ -99,6 +99,7 @@ podman \$opts run -d --name myrunningcontainer --label mylabel=$LABEL_RUNNING \
                                                -p $HOST_PORT:80 \
                                                -v $pmroot/var/www:/var/www \
                                                -w /var/www \
+                                               --mac-address aa:bb:cc:dd:ee:ff \
                                                $IMAGE /bin/busybox-extras httpd -f -p 80
 
 podman \$opts pod create --name mypod