From 8f234e6424bdb86101960bc7116748af9f21a72a Mon Sep 17 00:00:00 2001 From: steiler Date: Wed, 1 Nov 2023 18:21:53 +0100 Subject: [PATCH 1/5] fixing stacktrace on wrong brief link format --- links/link.go | 11 +++++++++-- links/link_host.go | 6 ++++-- links/link_macvlan.go | 6 ++++-- links/link_mgmt-net.go | 5 ++++- links/link_veth.go | 5 ++++- 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/links/link.go b/links/link.go index 2e39a803e..7a4b07768 100644 --- a/links/link.go +++ b/links/link.go @@ -307,19 +307,26 @@ type Link interface { GetMTU() int } -func extractHostNodeInterfaceData(lb *LinkBriefRaw, specialEPIndex int) (host, hostIf, node, nodeIf string) { +func extractHostNodeInterfaceData(lb *LinkBriefRaw, specialEPIndex int) (host, hostIf, node, nodeIf string, err error) { // the index of the node is the specialEndpointIndex +1 modulo 2 nodeindex := (specialEPIndex + 1) % 2 hostData := strings.SplitN(lb.Endpoints[specialEPIndex], ":", 2) nodeData := strings.SplitN(lb.Endpoints[nodeindex], ":", 2) + if len(hostData) != 2 { + return "", "", "", "", fmt.Errorf("Invalid link endpoint format. expected is :, got %s", lb.Endpoints[specialEPIndex]) + } + if len(nodeData) != 2 { + return "", "", "", "", fmt.Errorf("Invalid link endpoint format. expected is :, got %s", lb.Endpoints[specialEPIndex]) + } + host = hostData[0] hostIf = hostData[1] node = nodeData[0] nodeIf = nodeData[1] - return host, hostIf, node, nodeIf + return host, hostIf, node, nodeIf, nil } func genRandomIfName() string { diff --git a/links/link_host.go b/links/link_host.go index 125fad7ff..596bc029f 100644 --- a/links/link_host.go +++ b/links/link_host.go @@ -29,8 +29,10 @@ func (r *LinkHostRaw) ToLinkBriefRaw() *LinkBriefRaw { } func hostLinkFromBrief(lb *LinkBriefRaw, specialEPIndex int) (*LinkHostRaw, error) { - _, hostIf, node, nodeIf := extractHostNodeInterfaceData(lb, specialEPIndex) - + _, hostIf, node, nodeIf, err := extractHostNodeInterfaceData(lb, specialEPIndex) + if err != nil { + return nil, err + } link := &LinkHostRaw{ LinkCommonParams: lb.LinkCommonParams, HostInterface: hostIf, diff --git a/links/link_macvlan.go b/links/link_macvlan.go index cbed0a620..362518523 100644 --- a/links/link_macvlan.go +++ b/links/link_macvlan.go @@ -39,8 +39,10 @@ func (*LinkMacVlanRaw) GetType() LinkType { } func macVlanLinkFromBrief(lb *LinkBriefRaw, specialEPIndex int) (*LinkMacVlanRaw, error) { - _, hostIf, node, nodeIf := extractHostNodeInterfaceData(lb, specialEPIndex) - + _, hostIf, node, nodeIf, err := extractHostNodeInterfaceData(lb, specialEPIndex) + if err != nil { + return nil, err + } link := &LinkMacVlanRaw{ LinkCommonParams: lb.LinkCommonParams, HostInterface: hostIf, diff --git a/links/link_mgmt-net.go b/links/link_mgmt-net.go index 7b179b3d6..4d9d94f9f 100644 --- a/links/link_mgmt-net.go +++ b/links/link_mgmt-net.go @@ -81,7 +81,10 @@ func (*LinkMgmtNetRaw) GetType() LinkType { } func mgmtNetLinkFromBrief(lb *LinkBriefRaw, specialEPIndex int) (*LinkMgmtNetRaw, error) { - _, hostIf, node, nodeIf := extractHostNodeInterfaceData(lb, specialEPIndex) + _, hostIf, node, nodeIf, err := extractHostNodeInterfaceData(lb, specialEPIndex) + if err != nil { + return nil, err + } link := &LinkMgmtNetRaw{ LinkCommonParams: lb.LinkCommonParams, diff --git a/links/link_veth.go b/links/link_veth.go index 003e6fbfe..204334083 100644 --- a/links/link_veth.go +++ b/links/link_veth.go @@ -71,7 +71,10 @@ func (r *LinkVEthRaw) Resolve(params *ResolveParams) (Link, error) { // linkVEthRawFromLinkBriefRaw creates a raw veth link from a LinkBriefRaw. func linkVEthRawFromLinkBriefRaw(lb *LinkBriefRaw) (*LinkVEthRaw, error) { - host, hostIf, node, nodeIf := extractHostNodeInterfaceData(lb, 0) + host, hostIf, node, nodeIf, err := extractHostNodeInterfaceData(lb, 0) + if err != nil { + return nil, err + } link := &LinkVEthRaw{ LinkCommonParams: lb.LinkCommonParams, From 956e72b3df778d3533c809006c9d853d0043adab Mon Sep 17 00:00:00 2001 From: steiler Date: Wed, 1 Nov 2023 18:38:58 +0100 Subject: [PATCH 2/5] fixing staticcheck --- links/link.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/links/link.go b/links/link.go index 7a4b07768..a70c4a308 100644 --- a/links/link.go +++ b/links/link.go @@ -315,10 +315,10 @@ func extractHostNodeInterfaceData(lb *LinkBriefRaw, specialEPIndex int) (host, h nodeData := strings.SplitN(lb.Endpoints[nodeindex], ":", 2) if len(hostData) != 2 { - return "", "", "", "", fmt.Errorf("Invalid link endpoint format. expected is :, got %s", lb.Endpoints[specialEPIndex]) + return "", "", "", "", fmt.Errorf("invalid link endpoint format. expected is :, got %s", lb.Endpoints[specialEPIndex]) } if len(nodeData) != 2 { - return "", "", "", "", fmt.Errorf("Invalid link endpoint format. expected is :, got %s", lb.Endpoints[specialEPIndex]) + return "", "", "", "", fmt.Errorf("invalid link endpoint format. expected is :, got %s", lb.Endpoints[specialEPIndex]) } host = hostData[0] From 631fcf7cd6ce5895cfa0433dc5c79907a6d34a6c Mon Sep 17 00:00:00 2001 From: steiler Date: Wed, 1 Nov 2023 18:59:03 +0100 Subject: [PATCH 3/5] adding unit tests --- links/link_test.go | 111 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/links/link_test.go b/links/link_test.go index bf715119d..e89e75c2b 100644 --- a/links/link_test.go +++ b/links/link_test.go @@ -300,3 +300,114 @@ func TestUnmarshalRawLinksYaml(t *testing.T) { }) } } + +func Test_extractHostNodeInterfaceData(t *testing.T) { + type args struct { + lb *LinkBriefRaw + specialEPIndex int + } + tests := []struct { + name string + args args + wantHost string + wantHostIf string + wantNode string + wantNodeIf string + wantErr bool + }{ + { + name: "Valid input", + args: args{ + lb: &LinkBriefRaw{ + Endpoints: []string{ + "node1:eth1", + "node2:eth2", + }, + }, + specialEPIndex: 0, + }, + wantHost: "node1", + wantHostIf: "eth1", + wantNode: "node2", + wantNodeIf: "eth2", + wantErr: false, + }, + { + name: "Valid input other specialindex", + args: args{ + lb: &LinkBriefRaw{ + Endpoints: []string{ + "node1:eth1", + "node2:eth2", + }, + }, + specialEPIndex: 1, + }, + wantHost: "node2", + wantHostIf: "eth2", + wantNode: "node1", + wantNodeIf: "eth1", + wantErr: false, + }, + { + name: "Invalid specialNode", + args: args{ + lb: &LinkBriefRaw{ + Endpoints: []string{ + "node1:eth1", + "node2eth2", + }, + }, + specialEPIndex: 1, + }, + wantErr: true, + }, + { + name: "Invalid Node", + args: args{ + lb: &LinkBriefRaw{ + Endpoints: []string{ + "node1eth1", + "node2:eth2", + }, + }, + specialEPIndex: 1, + }, + wantErr: true, + }, + { + name: "Invalid Node too many colons", + args: args{ + lb: &LinkBriefRaw{ + Endpoints: []string{ + "node1eth1", + "node2:et:h2", + }, + }, + specialEPIndex: 1, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotHost, gotHostIf, gotNode, gotNodeIf, err := extractHostNodeInterfaceData(tt.args.lb, tt.args.specialEPIndex) + if (err != nil) != tt.wantErr { + t.Errorf("extractHostNodeInterfaceData() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotHost != tt.wantHost { + t.Errorf("extractHostNodeInterfaceData() gotHost = %v, want %v", gotHost, tt.wantHost) + } + if gotHostIf != tt.wantHostIf { + t.Errorf("extractHostNodeInterfaceData() gotHostIf = %v, want %v", gotHostIf, tt.wantHostIf) + } + if gotNode != tt.wantNode { + t.Errorf("extractHostNodeInterfaceData() gotNode = %v, want %v", gotNode, tt.wantNode) + } + if gotNodeIf != tt.wantNodeIf { + t.Errorf("extractHostNodeInterfaceData() gotNodeIf = %v, want %v", gotNodeIf, tt.wantNodeIf) + } + }) + } +} From a3daf2c1dc0ebd347bb2d982f2f03145b344eead Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Mon, 6 Nov 2023 11:58:51 +0200 Subject: [PATCH 4/5] remove extra if clause --- links/link.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/links/link.go b/links/link.go index a70c4a308..d16121328 100644 --- a/links/link.go +++ b/links/link.go @@ -314,11 +314,9 @@ func extractHostNodeInterfaceData(lb *LinkBriefRaw, specialEPIndex int) (host, h hostData := strings.SplitN(lb.Endpoints[specialEPIndex], ":", 2) nodeData := strings.SplitN(lb.Endpoints[nodeindex], ":", 2) - if len(hostData) != 2 { - return "", "", "", "", fmt.Errorf("invalid link endpoint format. expected is :, got %s", lb.Endpoints[specialEPIndex]) - } - if len(nodeData) != 2 { - return "", "", "", "", fmt.Errorf("invalid link endpoint format. expected is :, got %s", lb.Endpoints[specialEPIndex]) + if len(hostData) != 2 || len(nodeData) != 2 { + return "", "", "", "", + fmt.Errorf("invalid link endpoint format. expected :, got %s", lb.Endpoints[specialEPIndex]) } host = hostData[0] From 4a1d7ecd930a3781f77a057f679e33458a1127c4 Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Tue, 7 Nov 2023 19:57:56 +0200 Subject: [PATCH 5/5] bring back proper error message --- links/link.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/links/link.go b/links/link.go index d16121328..2598ccf89 100644 --- a/links/link.go +++ b/links/link.go @@ -314,11 +314,16 @@ func extractHostNodeInterfaceData(lb *LinkBriefRaw, specialEPIndex int) (host, h hostData := strings.SplitN(lb.Endpoints[specialEPIndex], ":", 2) nodeData := strings.SplitN(lb.Endpoints[nodeindex], ":", 2) - if len(hostData) != 2 || len(nodeData) != 2 { + if len(hostData) != 2 { return "", "", "", "", fmt.Errorf("invalid link endpoint format. expected :, got %s", lb.Endpoints[specialEPIndex]) } + if len(nodeData) != 2 { + return "", "", "", "", + fmt.Errorf("invalid link endpoint format. expected :, got %s", lb.Endpoints[nodeindex]) + } + host = hostData[0] hostIf = hostData[1] node = nodeData[0]