From 9a60b76e26cb2c1cbb939652a359d2c3b9d294ec Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 16 Dec 2020 09:01:41 -0500 Subject: [PATCH 1/3] cni: prevent NPE if no interface has sandbox field set When we iterate over the interfaces returned from CNI setup, we filter for one with the `Sandbox` field set. Ensure that if none of the interfaces has that field set that we still return an available interface. --- client/allocrunner/networking_cni.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/client/allocrunner/networking_cni.go b/client/allocrunner/networking_cni.go index 0fdd8a617cbf..8354499f7f96 100644 --- a/client/allocrunner/networking_cni.go +++ b/client/allocrunner/networking_cni.go @@ -114,13 +114,15 @@ func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Alloc netStatus := new(structs.AllocNetworkStatus) if len(res.Interfaces) > 0 { - iface, name := func(r *cni.CNIResult) (*cni.Config, string) { - for i := range r.Interfaces { - if r.Interfaces[i].Sandbox != "" { - return r.Interfaces[i], i + // find an interface with Sandbox set, or any one of them if no + // interface has it set + iface, name := func(r *cni.CNIResult) (iface *cni.Config, name string) { + for name, iface = range r.Interfaces { + if iface.Sandbox != "" { + return } } - return nil, "" + return }(res) netStatus.InterfaceName = name From 00814f307db6320b38aa4a5a6578a994857f52a7 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 16 Dec 2020 09:14:12 -0500 Subject: [PATCH 2/3] code review: remove lambda, add guard --- client/allocrunner/networking_cni.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/client/allocrunner/networking_cni.go b/client/allocrunner/networking_cni.go index 8354499f7f96..6306882dc1b9 100644 --- a/client/allocrunner/networking_cni.go +++ b/client/allocrunner/networking_cni.go @@ -116,14 +116,18 @@ func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Alloc if len(res.Interfaces) > 0 { // find an interface with Sandbox set, or any one of them if no // interface has it set - iface, name := func(r *cni.CNIResult) (iface *cni.Config, name string) { - for name, iface = range r.Interfaces { - if iface.Sandbox != "" { - return - } + var iface *cni.Config + var name string + for name, iface = range res.Interfaces { + if iface != nil && iface.Sandbox != "" { + break } - return - }(res) + } + if iface == nil { + // this should never happen but this value is coming from external + // plugins so we should guard against it + return nil, fmt.Errorf("failed to configure network: no valid interface") + } netStatus.InterfaceName = name if len(iface.IPConfigs) > 0 { From 67d990f9c37c81ef142158790523767b1e3ad407 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 16 Dec 2020 09:32:24 -0500 Subject: [PATCH 3/3] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e12bbc454a0..0a00ec387aa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ IMPROVEMENTS: BUG FIXES: * core: Fixed a bug where ACLToken and ACLPolicy changes were ignored by the event stream [[GH-9595](https://github.com/hashicorp/nomad/issues/9595)] * core: Fixed a bug to honor HCL2 variables set by environment variables or variable files [[GH-9592](https://github.com/hashicorp/nomad/issues/9592)] [[GH-9623](https://github.com/hashicorp/nomad/issues/9623)] + * cni: Fixed a bug where plugins that do not set the interface sandbox value could crash the Nomad client. [[GH-9648](https://github.com/hashicorp/nomad/issues/9648)] ## 1.0.0 (December 8, 2020)