Skip to content

Commit 48e3b23

Browse files
authored
Improved search for network addresses within a given string (#355)
* Also check suffixes of value for being a network address * e2e test * A few more tests Signed-off-by: ZIV NEVO <[email protected]>
1 parent 631eb6c commit 48e3b23

File tree

4 files changed

+152
-4
lines changed

4 files changed

+152
-4
lines changed

cmd/nettop/main_test.go

+9
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,15 @@ var (
137137
false,
138138
[]string{"qotd", "expected_netpol_output.json"},
139139
},
140+
{
141+
"CronJobWithNontrivialNetworkAddresses",
142+
[][]string{{"openshift", "openshift-operator-lifecycle-manager-resources.yaml"}},
143+
yamlFormat,
144+
true,
145+
[]string{"-v"},
146+
false,
147+
[]string{"openshift", "expected_netpol_output.yaml"},
148+
},
140149
{
141150
"SpecifyDNSPort",
142151
[][]string{{"acs-security-demos"}},

pkg/analyzer/info_to_resource.go

+39-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"net/url"
1212
"strconv"
1313
"strings"
14+
"unicode"
1415

1516
ocroutev1 "github.com/openshift/api/route/v1"
1617
appsv1 "k8s.io/api/apps/v1"
@@ -218,16 +219,35 @@ func appendNetworkAddresses(networkAddresses, values []string) []string {
218219
// because it decides if the given string is an evidence for a potentially required connectivity.
219220
// If it succeeds, a "cleaned" network address is returned as a string, together with the value true.
220221
// Otherwise (there does not seem to be a network address in "value"), it returns "" with the value false.
222+
// As value may be in the form of e.g. "key:val" where "val" holds the network address, we will check several possible suffixes.
221223
func networkAddressFromStr(value string) (string, bool) {
224+
suffixes := possibleSuffixes(value)
225+
for _, suffix := range suffixes {
226+
addr, ok := networkAddressFromSuffix(suffix)
227+
if ok {
228+
return addr, ok
229+
}
230+
}
231+
return "", false
232+
}
233+
234+
func networkAddressFromSuffix(value string) (string, bool) {
222235
host, err := getHostFromURL(value)
223236
if err != nil {
224237
return "", false // value cannot be interpreted as a URL
225238
}
226239

227240
hostNoPort := host
228241
colonPos := strings.Index(host, ":")
229-
if colonPos >= 0 {
242+
if colonPos >= 0 { // host includes port number or port name
230243
hostNoPort = host[:colonPos]
244+
port := host[colonPos+1:] // now validate the port
245+
if len(validation.IsValidPortName(port)) > 0 {
246+
portInt, _ := strconv.Atoi(port)
247+
if len(validation.IsValidPortNum(portInt)) > 0 {
248+
return "", false
249+
}
250+
}
231251
}
232252

233253
errs := validation.IsDNS1123Subdomain(hostNoPort)
@@ -242,6 +262,24 @@ func networkAddressFromStr(value string) (string, bool) {
242262
return host, true
243263
}
244264

265+
// Sometimes the given value includes the network address as its suffix.
266+
// For example, a command-line arg may look like "server-addr=my_server:5000"
267+
// If we are unable to convert "value" to a network address, we may also want to check its suffixes.
268+
// This function returns all suffixes that start with a letter, and have ':', ' ' or '=' just before this initial letter.
269+
func possibleSuffixes(value string) []string {
270+
res := []string{value}
271+
272+
var prevRune rune
273+
for i, r := range value {
274+
if i > 1 && unicode.IsLetter(r) && (prevRune == ':' || prevRune == '=' || prevRune == ' ') {
275+
res = append(res, value[i:])
276+
}
277+
prevRune = r
278+
}
279+
280+
return res
281+
}
282+
245283
// Attempts to parse the given string as a URL, and extract its Host part.
246284
// Returns an error if the string cannot be interpreted as a URL
247285
func getHostFromURL(urlStr string) (string, error) {

pkg/analyzer/info_to_resource_test.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ func TestNetworkAddressValue(t *testing.T) {
2727
strings.Repeat("abc", 500): {"", false},
2828
"not%a*url": {"", false},
2929
"123": {"", false},
30+
"olm-operator-heap-:https://olm-operator-metrics:8443/debug/pprof/heap": {"olm-operator-metrics:8443", true},
31+
"-server=my-server:5024": {"my-server:5024", true},
32+
"-server=my-server:502%4": {"", false}, // port number is invalid
3033
}
3134

3235
for val, expectedAnswer := range valuesToCheck {
@@ -53,8 +56,9 @@ func TestScanningDeploymentWithArgs(t *testing.T) {
5356
res, err := k8sWorkloadObjectFromInfo(resourceInfo)
5457
require.Nil(t, err)
5558
require.Equal(t, "carts", res.Resource.Name)
56-
require.Len(t, res.Resource.NetworkAddrs, 1)
57-
require.Equal(t, "carts-db:27017", res.Resource.NetworkAddrs[0])
59+
require.Len(t, res.Resource.NetworkAddrs, 2)
60+
require.Equal(t, "false", res.Resource.NetworkAddrs[0])
61+
require.Equal(t, "carts-db:27017", res.Resource.NetworkAddrs[1])
5862
require.Len(t, res.Resource.Labels, 1)
5963
require.Equal(t, "carts", res.Resource.Labels["name"])
6064
}
@@ -124,7 +128,7 @@ func TestScanningCronJob(t *testing.T) {
124128
require.Nil(t, err)
125129
require.Equal(t, "collect-profiles", res.Resource.Name)
126130
require.Equal(t, cronJob, res.Resource.Kind)
127-
require.Len(t, res.Resource.NetworkAddrs, 1)
131+
require.Len(t, res.Resource.NetworkAddrs, 3)
128132
require.Len(t, res.Resource.Labels, 0)
129133
}
130134

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
apiVersion: networking.k8s.io/v1
2+
items:
3+
- apiVersion: networking.k8s.io/v1
4+
kind: NetworkPolicy
5+
metadata:
6+
creationTimestamp: null
7+
name: catalog-operator-netpol
8+
namespace: openshift-operator-lifecycle-manager
9+
spec:
10+
ingress:
11+
- from:
12+
- podSelector: {}
13+
ports:
14+
- port: 8443
15+
protocol: TCP
16+
podSelector:
17+
matchLabels:
18+
app: catalog-operator
19+
policyTypes:
20+
- Ingress
21+
- Egress
22+
- apiVersion: networking.k8s.io/v1
23+
kind: NetworkPolicy
24+
metadata:
25+
creationTimestamp: null
26+
name: collect-profiles-netpol
27+
namespace: openshift-operator-lifecycle-manager
28+
spec:
29+
egress:
30+
- ports:
31+
- port: 8443
32+
protocol: TCP
33+
to:
34+
- podSelector:
35+
matchLabels:
36+
app: catalog-operator
37+
- ports:
38+
- port: 8443
39+
protocol: TCP
40+
to:
41+
- podSelector:
42+
matchLabels:
43+
app: olm-operator
44+
- ports:
45+
- port: 53
46+
protocol: UDP
47+
to:
48+
- namespaceSelector: {}
49+
podSelector: {}
50+
policyTypes:
51+
- Ingress
52+
- Egress
53+
- apiVersion: networking.k8s.io/v1
54+
kind: NetworkPolicy
55+
metadata:
56+
creationTimestamp: null
57+
name: olm-operator-netpol
58+
namespace: openshift-operator-lifecycle-manager
59+
spec:
60+
ingress:
61+
- from:
62+
- podSelector: {}
63+
ports:
64+
- port: 8443
65+
protocol: TCP
66+
podSelector:
67+
matchLabels:
68+
app: olm-operator
69+
policyTypes:
70+
- Ingress
71+
- Egress
72+
- apiVersion: networking.k8s.io/v1
73+
kind: NetworkPolicy
74+
metadata:
75+
creationTimestamp: null
76+
name: packageserver-netpol
77+
namespace: openshift-operator-lifecycle-manager
78+
spec:
79+
podSelector:
80+
matchLabels:
81+
app: packageserver
82+
policyTypes:
83+
- Ingress
84+
- Egress
85+
- apiVersion: networking.k8s.io/v1
86+
kind: NetworkPolicy
87+
metadata:
88+
creationTimestamp: null
89+
name: default-deny-in-namespace-openshift-operator-lifecycle-manager
90+
namespace: openshift-operator-lifecycle-manager
91+
spec:
92+
podSelector: {}
93+
policyTypes:
94+
- Ingress
95+
- Egress
96+
kind: NetworkPolicyList
97+
metadata: {}

0 commit comments

Comments
 (0)