From d0a5b747b2d22cc034926d20324f3197baabf24b Mon Sep 17 00:00:00 2001 From: ZIV NEVO Date: Wed, 7 Feb 2024 16:31:34 +0200 Subject: [PATCH 1/3] Also check suffixes of value for being a network address Signed-off-by: ZIV NEVO --- pkg/analyzer/info_to_resource.go | 40 ++++++++++++++++++++++++++- pkg/analyzer/info_to_resource_test.go | 8 ++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/pkg/analyzer/info_to_resource.go b/pkg/analyzer/info_to_resource.go index cca8d11..e89ddd8 100644 --- a/pkg/analyzer/info_to_resource.go +++ b/pkg/analyzer/info_to_resource.go @@ -11,6 +11,7 @@ import ( "net/url" "strconv" "strings" + "unicode" ocroutev1 "github.com/openshift/api/route/v1" appsv1 "k8s.io/api/apps/v1" @@ -218,7 +219,19 @@ func appendNetworkAddresses(networkAddresses, values []string) []string { // because it decides if the given string is an evidence for a potentially required connectivity. // If it succeeds, a "cleaned" network address is returned as a string, together with the value true. // Otherwise (there does not seem to be a network address in "value"), it returns "" with the value false. +// As value may be in the form of e.g. "key:val" where "val" holds the network address, we will check several possible suffixes. func networkAddressFromStr(value string) (string, bool) { + suffixes := possibleSuffixes(value) + for _, suffix := range suffixes { + addr, ok := networkAddressFromSuffix(suffix) + if ok { + return addr, ok + } + } + return "", false +} + +func networkAddressFromSuffix(value string) (string, bool) { host, err := getHostFromURL(value) if err != nil { return "", false // value cannot be interpreted as a URL @@ -226,8 +239,15 @@ func networkAddressFromStr(value string) (string, bool) { hostNoPort := host colonPos := strings.Index(host, ":") - if colonPos >= 0 { + if colonPos >= 0 { // host includes port number or port name hostNoPort = host[:colonPos] + port := host[colonPos+1:] // now validate the port + if len(validation.IsValidPortName(port)) > 0 { + portInt, _ := strconv.Atoi(port) + if len(validation.IsValidPortNum(portInt)) > 0 { + return "", false + } + } } errs := validation.IsDNS1123Subdomain(hostNoPort) @@ -242,6 +262,24 @@ func networkAddressFromStr(value string) (string, bool) { return host, true } +// Sometimes the given value includes the network address as its suffix. +// For example, a command-line arg may look like "server-addr=my_server:5000" +// If we are unable to convert "value" to a network address, we may also want to check its suffixes. +// This function returns all suffixes that start with a letter, and have ':', ' ' or '=' just before this initial letter. +func possibleSuffixes(value string) []string { + res := []string{value} + + var prevRune rune + for i, r := range value { + if i > 1 && unicode.IsLetter(r) && (prevRune == ':' || prevRune == '=' || prevRune == ' ') { + res = append(res, value[i:]) + } + prevRune = r + } + + return res +} + // Attempts to parse the given string as a URL, and extract its Host part. // Returns an error if the string cannot be interpreted as a URL func getHostFromURL(urlStr string) (string, error) { diff --git a/pkg/analyzer/info_to_resource_test.go b/pkg/analyzer/info_to_resource_test.go index dc7dbfe..b3a0d34 100644 --- a/pkg/analyzer/info_to_resource_test.go +++ b/pkg/analyzer/info_to_resource_test.go @@ -27,6 +27,7 @@ func TestNetworkAddressValue(t *testing.T) { strings.Repeat("abc", 500): {"", false}, "not%a*url": {"", false}, "123": {"", false}, + "olm-operator-heap-:https://olm-operator-metrics:8443/debug/pprof/heap": {"olm-operator-metrics:8443", true}, } for val, expectedAnswer := range valuesToCheck { @@ -53,8 +54,9 @@ func TestScanningDeploymentWithArgs(t *testing.T) { res, err := k8sWorkloadObjectFromInfo(resourceInfo) require.Nil(t, err) require.Equal(t, "carts", res.Resource.Name) - require.Len(t, res.Resource.NetworkAddrs, 1) - require.Equal(t, "carts-db:27017", res.Resource.NetworkAddrs[0]) + require.Len(t, res.Resource.NetworkAddrs, 2) + require.Equal(t, "false", res.Resource.NetworkAddrs[0]) + require.Equal(t, "carts-db:27017", res.Resource.NetworkAddrs[1]) require.Len(t, res.Resource.Labels, 1) require.Equal(t, "carts", res.Resource.Labels["name"]) } @@ -124,7 +126,7 @@ func TestScanningCronJob(t *testing.T) { require.Nil(t, err) require.Equal(t, "collect-profiles", res.Resource.Name) require.Equal(t, cronJob, res.Resource.Kind) - require.Len(t, res.Resource.NetworkAddrs, 1) + require.Len(t, res.Resource.NetworkAddrs, 3) require.Len(t, res.Resource.Labels, 0) } From 2e4eb7b532796bc07293ce7ef91e6ce681da8029 Mon Sep 17 00:00:00 2001 From: ZIV NEVO Date: Wed, 7 Feb 2024 16:31:58 +0200 Subject: [PATCH 2/3] e2e test Signed-off-by: ZIV NEVO --- cmd/nettop/main_test.go | 9 ++ tests/openshift/expected_netpol_output.yaml | 97 +++++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 tests/openshift/expected_netpol_output.yaml diff --git a/cmd/nettop/main_test.go b/cmd/nettop/main_test.go index 3b70eae..928de3e 100644 --- a/cmd/nettop/main_test.go +++ b/cmd/nettop/main_test.go @@ -137,6 +137,15 @@ var ( false, []string{"qotd", "expected_netpol_output.json"}, }, + { + "CronJobWithNontrivialNetworkAddresses", + [][]string{{"openshift", "openshift-operator-lifecycle-manager-resources.yaml"}}, + yamlFormat, + true, + []string{"-v"}, + false, + []string{"openshift", "expected_netpol_output.yaml"}, + }, { "SpecifyDNSPort", [][]string{{"acs-security-demos"}}, diff --git a/tests/openshift/expected_netpol_output.yaml b/tests/openshift/expected_netpol_output.yaml new file mode 100644 index 0000000..752bf50 --- /dev/null +++ b/tests/openshift/expected_netpol_output.yaml @@ -0,0 +1,97 @@ +apiVersion: networking.k8s.io/v1 +items: + - apiVersion: networking.k8s.io/v1 + kind: NetworkPolicy + metadata: + creationTimestamp: null + name: catalog-operator-netpol + namespace: openshift-operator-lifecycle-manager + spec: + ingress: + - from: + - podSelector: {} + ports: + - port: 8443 + protocol: TCP + podSelector: + matchLabels: + app: catalog-operator + policyTypes: + - Ingress + - Egress + - apiVersion: networking.k8s.io/v1 + kind: NetworkPolicy + metadata: + creationTimestamp: null + name: collect-profiles-netpol + namespace: openshift-operator-lifecycle-manager + spec: + egress: + - ports: + - port: 8443 + protocol: TCP + to: + - podSelector: + matchLabels: + app: catalog-operator + - ports: + - port: 8443 + protocol: TCP + to: + - podSelector: + matchLabels: + app: olm-operator + - ports: + - port: 53 + protocol: UDP + to: + - namespaceSelector: {} + podSelector: {} + policyTypes: + - Ingress + - Egress + - apiVersion: networking.k8s.io/v1 + kind: NetworkPolicy + metadata: + creationTimestamp: null + name: olm-operator-netpol + namespace: openshift-operator-lifecycle-manager + spec: + ingress: + - from: + - podSelector: {} + ports: + - port: 8443 + protocol: TCP + podSelector: + matchLabels: + app: olm-operator + policyTypes: + - Ingress + - Egress + - apiVersion: networking.k8s.io/v1 + kind: NetworkPolicy + metadata: + creationTimestamp: null + name: packageserver-netpol + namespace: openshift-operator-lifecycle-manager + spec: + podSelector: + matchLabels: + app: packageserver + policyTypes: + - Ingress + - Egress + - apiVersion: networking.k8s.io/v1 + kind: NetworkPolicy + metadata: + creationTimestamp: null + name: default-deny-in-namespace-openshift-operator-lifecycle-manager + namespace: openshift-operator-lifecycle-manager + spec: + podSelector: {} + policyTypes: + - Ingress + - Egress +kind: NetworkPolicyList +metadata: {} From 356d8b8e4f5cdecdb7b3c70e02cdf2b4a2c03729 Mon Sep 17 00:00:00 2001 From: ZIV NEVO Date: Thu, 8 Feb 2024 18:17:25 +0200 Subject: [PATCH 3/3] A few more tests Signed-off-by: ZIV NEVO --- pkg/analyzer/info_to_resource_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/analyzer/info_to_resource_test.go b/pkg/analyzer/info_to_resource_test.go index b3a0d34..7f50658 100644 --- a/pkg/analyzer/info_to_resource_test.go +++ b/pkg/analyzer/info_to_resource_test.go @@ -28,6 +28,8 @@ func TestNetworkAddressValue(t *testing.T) { "not%a*url": {"", false}, "123": {"", false}, "olm-operator-heap-:https://olm-operator-metrics:8443/debug/pprof/heap": {"olm-operator-metrics:8443", true}, + "-server=my-server:5024": {"my-server:5024", true}, + "-server=my-server:502%4": {"", false}, // port number is invalid } for val, expectedAnswer := range valuesToCheck {