Skip to content

Commit df38949

Browse files
danwinshipdanwinship
committed
Remove error returns for netutilsv2.IsDualStack* funcs
99% of callers are calling these functions on already-validated data, making the error return values just annoying.
1 parent d31969d commit df38949

File tree

2 files changed

+17
-64
lines changed

2 files changed

+17
-64
lines changed

‎net/v2/ipfamily.go‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package net
1818

1919
import (
20-
"fmt"
2120
"net"
2221
)
2322

@@ -35,32 +34,32 @@ const (
3534
// IsDualStackIPs returns true if:
3635
// - all elements of ips are valid
3736
// - at least one IP from each family (v4 and v6) is present
38-
func IsDualStackIPs(ips []net.IP) (bool, error) {
37+
func IsDualStackIPs(ips []net.IP) bool {
3938
v4Found := false
4039
v6Found := false
41-
for i, ip := range ips {
40+
for _, ip := range ips {
4241
switch IPFamilyOf(ip) {
4342
case IPv4:
4443
v4Found = true
4544
case IPv6:
4645
v6Found = true
4746
default:
48-
return false, fmt.Errorf("invalid IP[%d]: %v", i, ip)
47+
return false
4948
}
5049
}
5150

52-
return (v4Found && v6Found), nil
51+
return (v4Found && v6Found)
5352
}
5453

5554
// IsDualStackIPStrings returns true if:
5655
// - all elements of ips can be parsed as IPs
5756
// - at least one IP from each family (v4 and v6) is present
58-
func IsDualStackIPStrings(ips []string) (bool, error) {
57+
func IsDualStackIPStrings(ips []string) bool {
5958
parsedIPs := make([]net.IP, 0, len(ips))
60-
for i, ip := range ips {
59+
for _, ip := range ips {
6160
parsedIP := ParseIPSloppy(ip)
6261
if parsedIP == nil {
63-
return false, fmt.Errorf("invalid IP[%d]: %v", i, ip)
62+
return false
6463
}
6564
parsedIPs = append(parsedIPs, parsedIP)
6665
}
@@ -70,30 +69,30 @@ func IsDualStackIPStrings(ips []string) (bool, error) {
7069
// IsDualStackCIDRs returns true if:
7170
// - all elements of cidrs are non-nil
7271
// - at least one CIDR from each family (v4 and v6) is present
73-
func IsDualStackCIDRs(cidrs []*net.IPNet) (bool, error) {
72+
func IsDualStackCIDRs(cidrs []*net.IPNet) bool {
7473
v4Found := false
7574
v6Found := false
76-
for i, cidr := range cidrs {
75+
for _, cidr := range cidrs {
7776
switch IPFamilyOfCIDR(cidr) {
7877
case IPv4:
7978
v4Found = true
8079
case IPv6:
8180
v6Found = true
8281
default:
83-
return false, fmt.Errorf("invalid CIDR[%d]: %v", i, cidr)
82+
return false
8483
}
8584
}
8685

87-
return (v4Found && v6Found), nil
86+
return (v4Found && v6Found)
8887
}
8988

9089
// IsDualStackCIDRStrings returns if
9190
// - all elements of cidrs can be parsed as CIDRs
9291
// - at least one CIDR from each family (v4 and v6) is present
93-
func IsDualStackCIDRStrings(cidrs []string) (bool, error) {
92+
func IsDualStackCIDRStrings(cidrs []string) bool {
9493
parsedCIDRs, err := ParseCIDRs(cidrs)
9594
if err != nil {
96-
return false, err
95+
return false
9796
}
9897
return IsDualStackCIDRs(parsedCIDRs)
9998
}

‎net/v2/ipfamily_test.go‎

Lines changed: 4 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -27,85 +27,67 @@ func TestDualStackIPs(t *testing.T) {
2727
desc string
2828
ips []string
2929
expectedResult bool
30-
expectError bool
3130
}{
3231
{
3332
desc: "should fail because length is not at least 2",
3433
ips: []string{"1.1.1.1"},
3534
expectedResult: false,
36-
expectError: false,
3735
},
3836
{
3937
desc: "should fail because length is not at least 2",
4038
ips: []string{},
4139
expectedResult: false,
42-
expectError: false,
4340
},
4441
{
4542
desc: "should fail because all are v4",
4643
ips: []string{"1.1.1.1", "2.2.2.2", "3.3.3.3"},
4744
expectedResult: false,
48-
expectError: false,
4945
},
5046
{
5147
desc: "should fail because all are v6",
5248
ips: []string{"fd92:20ba:ca:34f7:ffff:ffff:ffff:ffff", "fd92:20ba:ca:34f7:ffff:ffff:ffff:fff0", "fd92:20ba:ca:34f7:ffff:ffff:ffff:fff1"},
5349
expectedResult: false,
54-
expectError: false,
5550
},
5651
{
5752
desc: "should fail because 2nd ip is invalid",
5853
ips: []string{"1.1.1.1", "not-a-valid-ip"},
5954
expectedResult: false,
60-
expectError: true,
6155
},
6256
{
6357
desc: "should fail because 1st ip is invalid",
6458
ips: []string{"not-a-valid-ip", "fd92:20ba:ca:34f7:ffff:ffff:ffff:ffff"},
6559
expectedResult: false,
66-
expectError: true,
6760
},
6861
{
6962
desc: "should fail despite dual-stack because 3rd ip is invalid",
7063
ips: []string{"1.1.1.1", "fd92:20ba:ca:34f7:ffff:ffff:ffff:ffff", "not-a-valid-ip"},
7164
expectedResult: false,
72-
expectError: true,
7365
},
7466
{
7567
desc: "dual-stack ipv4-primary",
7668
ips: []string{"1.1.1.1", "fd92:20ba:ca:34f7:ffff:ffff:ffff:ffff"},
7769
expectedResult: true,
78-
expectError: false,
7970
},
8071
{
8172
desc: "dual-stack, multiple ipv6",
8273
ips: []string{"fd92:20ba:ca:34f7:ffff:ffff:ffff:ffff", "1.1.1.1", "fd92:20ba:ca:34f7:ffff:ffff:ffff:fff0"},
8374
expectedResult: true,
84-
expectError: false,
8575
},
8676
{
8777
desc: "dual-stack, multiple ipv4",
8878
ips: []string{"1.1.1.1", "fd92:20ba:ca:34f7:ffff:ffff:ffff:ffff", "10.0.0.0"},
8979
expectedResult: true,
90-
expectError: false,
9180
},
9281
{
9382
desc: "dual-stack, ipv6-primary",
9483
ips: []string{"fd92:20ba:ca:34f7:ffff:ffff:ffff:ffff", "1.1.1.1"},
9584
expectedResult: true,
96-
expectError: false,
9785
},
9886
}
9987
// for each test case, test the regular func and the string func
10088
for _, tc := range testCases {
10189
t.Run(tc.desc, func(t *testing.T) {
102-
dualStack, err := IsDualStackIPStrings(tc.ips)
103-
if err == nil && tc.expectError {
104-
t.Fatalf("expected an error from IsDualStackIPStrings")
105-
}
106-
if err != nil && !tc.expectError {
107-
t.Fatalf("unexpected error from IsDualStackIPStrings: %v", err)
108-
}
90+
dualStack := IsDualStackIPStrings(tc.ips)
10991
if dualStack != tc.expectedResult {
11092
t.Errorf("expected IsDualStackIPStrings=%v, got %v", tc.expectedResult, dualStack)
11193
}
@@ -115,13 +97,7 @@ func TestDualStackIPs(t *testing.T) {
11597
parsedIP := ParseIPSloppy(ip)
11698
ips = append(ips, parsedIP)
11799
}
118-
dualStack, err = IsDualStackIPs(ips)
119-
if err == nil && tc.expectError {
120-
t.Fatalf("expected an error from IsDualStackIPs")
121-
}
122-
if err != nil && !tc.expectError {
123-
t.Fatalf("unexpected error from IsDualStackIPs: %v", err)
124-
}
100+
dualStack = IsDualStackIPs(ips)
125101
if dualStack != tc.expectedResult {
126102
t.Errorf("expected IsDualStackIPs=%v, got %v", tc.expectedResult, dualStack)
127103
}
@@ -134,74 +110,58 @@ func TestDualStackCIDRs(t *testing.T) {
134110
desc string
135111
cidrs []string
136112
expectedResult bool
137-
expectError bool
138113
}{
139114
{
140115
desc: "should fail because length is not at least 2",
141116
cidrs: []string{"10.10.10.10/8"},
142117
expectedResult: false,
143-
expectError: false,
144118
},
145119
{
146120
desc: "should fail because length is not at least 2",
147121
cidrs: []string{},
148122
expectedResult: false,
149-
expectError: false,
150123
},
151124
{
152125
desc: "should fail because all cidrs are v4",
153126
cidrs: []string{"10.10.10.10/8", "20.20.20.20/8", "30.30.30.30/8"},
154127
expectedResult: false,
155-
expectError: false,
156128
},
157129
{
158130
desc: "should fail because all cidrs are v6",
159131
cidrs: []string{"2000::/10", "3000::/10"},
160132
expectedResult: false,
161-
expectError: false,
162133
},
163134
{
164135
desc: "should fail because 2nd cidr is invalid",
165136
cidrs: []string{"10.10.10.10/8", "not-a-valid-cidr"},
166137
expectedResult: false,
167-
expectError: true,
168138
},
169139
{
170140
desc: "should fail because 1st cidr is invalid",
171141
cidrs: []string{"not-a-valid-ip", "2000::/10"},
172142
expectedResult: false,
173-
expectError: true,
174143
},
175144
{
176145
desc: "dual-stack, ipv4-primary",
177146
cidrs: []string{"10.10.10.10/8", "2000::/10"},
178147
expectedResult: true,
179-
expectError: false,
180148
},
181149
{
182150
desc: "dual-stack, ipv6-primary",
183151
cidrs: []string{"2000::/10", "10.10.10.10/8"},
184152
expectedResult: true,
185-
expectError: false,
186153
},
187154
{
188155
desc: "dual-stack, multiple IPv6",
189156
cidrs: []string{"2000::/10", "10.10.10.10/8", "3000::/10"},
190157
expectedResult: true,
191-
expectError: false,
192158
},
193159
}
194160

195161
// for each test case, test the regular func and the string func
196162
for _, tc := range testCases {
197163
t.Run(tc.desc, func(t *testing.T) {
198-
dualStack, err := IsDualStackCIDRStrings(tc.cidrs)
199-
if err == nil && tc.expectError {
200-
t.Fatalf("expected an error from IsDualStackCIDRStrings")
201-
}
202-
if err != nil && !tc.expectError {
203-
t.Fatalf("unexpected error from IsDualStackCIDRStrings: %v", err)
204-
}
164+
dualStack := IsDualStackCIDRStrings(tc.cidrs)
205165
if dualStack != tc.expectedResult {
206166
t.Errorf("expected IsDualStackCIDRStrings=%v, got %v", tc.expectedResult, dualStack)
207167
}
@@ -212,13 +172,7 @@ func TestDualStackCIDRs(t *testing.T) {
212172
cidrs = append(cidrs, parsedCIDR)
213173
}
214174

215-
dualStack, err = IsDualStackCIDRs(cidrs)
216-
if err == nil && tc.expectError {
217-
t.Fatalf("expected an error from IsDualStackCIDRs")
218-
}
219-
if err != nil && !tc.expectError {
220-
t.Fatalf("unexpected error from IsDualStackCIDRs: %v", err)
221-
}
175+
dualStack = IsDualStackCIDRs(cidrs)
222176
if dualStack != tc.expectedResult {
223177
t.Errorf("expected IsDualStackCIDRs=%v, got %v", tc.expectedResult, dualStack)
224178
}

0 commit comments

Comments
 (0)