From 001d48929dcbe4f3a8eeb3db2a7bfdce5a90f81a Mon Sep 17 00:00:00 2001
From: Paul Holzinger <pholzing@redhat.com>
Date: Wed, 3 Nov 2021 14:54:48 +0100
Subject: [PATCH] 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>
---
libpod/container_config.go | 2 +-
libpod/network/cni/run.go | 2 +-
libpod/network/cni/run_test.go | 14 ++---
libpod/network/types/network.go | 49 ++++++++++++++++-
libpod/network/types/network_test.go | 82 ++++++++++++++++++++++++++++
libpod/options.go | 2 +-
pkg/domain/entities/pods.go | 4 +-
pkg/specgen/podspecgen.go | 2 +-
pkg/specgen/specgen.go | 2 +-
pkg/specgenutil/specgen.go | 4 +-
test/upgrade/test-upgrade.bats | 1 +
11 files changed, 148 insertions(+), 16 deletions(-)
create mode 100644 libpod/network/types/network_test.go
diff --git a/libpod/container_config.go b/libpod/container_config.go
index 33ea731fda..8f803d7447 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 99b2adce5c..7795dfeebb 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 965203c2ae..3169cd0eb2 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 657c1ca6a6..5cf523e201 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 0000000000..91ee936922
--- /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 135b2f363e..f1cf015f8b 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 3d8579acfe..1df18be58c 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 7713ea26c0..32d5be79aa 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 5a07af0f92..593d91c64d 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 683cd29182..4e8f954fba 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 5cb302a85e..e9910f3d28 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