matching namespaced services + restrict to used ports (#56)

* New example

Signed-off-by: Ziv Nevo <[email protected]>

* renamed function

Signed-off-by: Ziv Nevo <[email protected]>

* reduce indexing and check ConfigMapKeyRef value

Signed-off-by: Ziv Nevo <[email protected]>

* match namespaced service patterns

also restrict netpol ports to only used ports

Signed-off-by: Ziv Nevo <[email protected]>

* Ensure same namespace for resource and its service

Signed-off-by: Ziv Nevo <[email protected]>

* port names needed for multi-port service

Signed-off-by: Ziv Nevo <[email protected]>

* NamespaceSelectors in synthesized netpols

Signed-off-by: Ziv Nevo <[email protected]>

Author: zivnevo

pkg/analyzer/scan.go

@@ -80,7 +80,7 @@ func ScanK8sConfigmapObject(kind string, objDataBuf []byte) (common.CfgMap, erro
 	fullName := obj.ObjectMeta.Namespace + "/" + obj.ObjectMeta.Name
 	data := map[string]string{}
 	for k, v := range obj.Data {
-		isPotentialAddress := identifyAddressValue(v)
+		isPotentialAddress := IsNetworkAddressValue(v)
 		if isPotentialAddress {
 			data[k] = v
 		}
@@ -129,7 +129,7 @@ func parseDeployResource(podSpec *v1.PodTemplateSpec, obj metaV1.Object, resourc
 		}
 		for _, e := range container.Env {
 			if e.Value != "" {
-				isPotentialAddress := identifyAddressValue(e.Value)
+				isPotentialAddress := IsNetworkAddressValue(e.Value)
 				if isPotentialAddress {
 					resourceCtx.Resource.Envs = append(resourceCtx.Resource.Envs, e.Value)
 				}
@@ -149,8 +149,8 @@ func parseDeployResource(podSpec *v1.PodTemplateSpec, obj metaV1.Object, resourc
 	}
 }
 
-// identifyAddressValue checks if a given string is a potential network address
-func identifyAddressValue(value string) bool {
+// IsNetworkAddressValue checks if a given string is a potential network address
+func IsNetworkAddressValue(value string) bool {
 	_, err := url.Parse(value)
 	if err != nil {
 		return false

pkg/common/types.go

@@ -43,7 +43,7 @@ type Resource struct {
 		Envs             []string
 		ConfigMapRefs    []string       `json:"-"`
 		ConfigMapKeyRefs []CfgMapKeyRef `json:"-"`
-		UsedPorts        []int
+		UsedPorts        []SvcNetworkAttr
 	} `json:"resource,omitempty"`
 }
 

pkg/controller/connections.go

@@ -17,7 +17,7 @@ func discoverConnections(resources []common.Resource, links []common.Service) ([
 	connections := []common.Connections{}
 	for destResIdx := range resources {
 		destRes := &resources[destResIdx]
-		deploymentServices := findServices(destRes.Resource.Selectors, links)
+		deploymentServices := findServices(destRes, links)
 		for svcIdx := range deploymentServices {
 			svc := &deploymentServices[svcIdx]
 			srcRes := findSource(resources, svc)
@@ -35,9 +35,10 @@ func discoverConnections(resources []common.Resource, links []common.Service) ([
 }
 
 // areSelectorsContained returns true if selectors2 is contained in selectors1
-func areSelectorsContained(selectors1, selectors2 []string) bool {
+func areSelectorsContained(selectors1 map[string]string, selectors2 []string) bool {
 	elementMap := make(map[string]string)
-	for _, s := range selectors1 {
+	for k, v := range selectors1 {
+		s := fmt.Sprintf("%s:%s", k, v)
 		elementMap[s] = ""
 	}
 	for _, val := range selectors2 {
@@ -49,14 +50,16 @@ func areSelectorsContained(selectors1, selectors2 []string) bool {
 	return true
 }
 
-// findServices returns a list of services that may be in front of a given deployment (represented by its selectors)
-func findServices(selectors []string, links []common.Service) []common.Service {
+// findServices returns a list of services that may be in front of a given workload resource
+func findServices(resource *common.Resource, links []common.Service) []common.Service {
 	var matchedSvc []common.Service
-	//TODO: refer to namespaces - the matching services and input deployment should be in the same namespace
 	for linkIdx := range links {
 		link := &links[linkIdx]
+		if link.Resource.Namespace != resource.Resource.Namespace {
+			continue
+		}
 		// all service selector values should be contained in the input selectors of the deployment
-		res := areSelectorsContained(selectors, link.Resource.Selectors)
+		res := areSelectorsContained(resource.Resource.Labels, link.Resource.Selectors)
 		if res {
 			matchedSvc = append(matchedSvc, *link)
 		}
@@ -72,22 +75,58 @@ func findServices(selectors []string, links []common.Service) []common.Service {
 func findSource(resources []common.Resource, service *common.Service) []*common.Resource {
 	tRes := []*common.Resource{}
 	for resIdx := range resources {
-		res := &resources[resIdx]
-		for _, envVal := range res.Resource.Envs {
-			envVal = strings.TrimPrefix(envVal, "http://")
-			if service.Resource.Name == envVal { // A match without port name
-				tRes = append(tRes, res)
-			}
-			for _, p := range service.Resource.Network {
-				serviceWithPort := fmt.Sprintf("%s:%d", service.Resource.Name, p.Port)
-				if serviceWithPort == envVal {
-					foundSrc := *res
-					// specify the used ports for target by the found src
-					foundSrc.Resource.UsedPorts = []int{p.Port}
-					tRes = append(tRes, &foundSrc)
+		resource := &resources[resIdx]
+		serviceAddresses := getPossibleServiceAddresses(service, resource)
+		foundSrc := *resource // We copy the resource so we can specify the ports used by the source found
+		matched := false
+		for _, envVal := range resource.Resource.Envs {
+			match, port := envValueMatchesService(envVal, service, serviceAddresses)
+			if match {
+				matched = true
+				if port.Port > 0 {
+					foundSrc.Resource.UsedPorts = append(foundSrc.Resource.UsedPorts, port)
 				}
 			}
 		}
+		if matched {
+			tRes = append(tRes, &foundSrc)
+		}
 	}
 	return tRes
 }
+
+func getPossibleServiceAddresses(service *common.Service, resource *common.Resource) []string {
+	svcAddresses := []string{}
+	if service.Resource.Namespace != "" {
+		serviceDotNamespace := fmt.Sprintf("%s.%s", service.Resource.Name, service.Resource.Namespace)
+		svcAddresses = append(svcAddresses, serviceDotNamespace, serviceDotNamespace+".svc.cluster.local")
+	}
+	if service.Resource.Namespace == resource.Resource.Namespace { // both service and resource live in the same namespace
+		svcAddresses = append(svcAddresses, service.Resource.Name)
+	}
+
+	return svcAddresses
+}
+
+func envValueMatchesService(envVal string, service *common.Service, serviceAddresses []string) (bool, common.SvcNetworkAttr) {
+	envVal = strings.TrimPrefix(envVal, "http://")
+	envVal = strings.TrimPrefix(envVal, "https://")
+
+	// first look for matches without specified port
+	for _, svcAddress := range serviceAddresses {
+		if svcAddress == envVal {
+			return true, common.SvcNetworkAttr{} // this means no specified port
+		}
+	}
+
+	// Now look for matches that have port specified
+	for _, p := range service.Resource.Network {
+		for _, svcAddress := range serviceAddresses {
+			serviceWithPort := fmt.Sprintf("%s:%d", svcAddress, p.Port)
+			if envVal == serviceWithPort {
+				return true, p
+			}
+		}
+	}
+	return false, common.SvcNetworkAttr{}
+}

pkg/controller/controller_test.go

@@ -82,6 +82,9 @@ func TestNetpolsJsonOutput(t *testing.T) {
 	tests["wordpress"] = TestDetails{dirPath: filepath.Join(testsDir, "k8s_wordpress_example"),
 		outFile:        filepath.Join(testsDir, "k8s_wordpress_example", "output.json"),
 		expectedOutput: filepath.Join(testsDir, "k8s_wordpress_example", "expected_netpol_output.json")}
+	tests["guestbook"] = TestDetails{dirPath: filepath.Join(testsDir, "k8s_guestbook"),
+		outFile:        filepath.Join(testsDir, "k8s_guestbook", "output.json"),
+		expectedOutput: filepath.Join(testsDir, "k8s_guestbook", "expected_netpol_output.json")}
 
 	for testName, testDetails := range tests {
 		args := getTestArgs(testDetails.dirPath, testDetails.outFile, true)
@@ -170,15 +173,16 @@ func compareFiles(expectedFile, actualFile string) (bool, error) {
 		return false, errors.New("error reading lines from file")
 	}
 	if len(expectedLines) != len(actualLines) {
-		fmt.Printf("Files line count is different: expected: %d, actual: %d", len(expectedLines), len(actualLines))
+		fmt.Printf("Files line count is different: expected(%s): %d, actual(%s): %d",
+			expectedFile, len(expectedLines), actualFile, len(actualLines))
 		return false, nil
 	}
 
 	for i := 0; i < len(expectedLines); i++ {
 		lineExpected := expectedLines[i]
 		lineActual := actualLines[i]
 		if lineExpected != lineActual && !strings.Contains(lineExpected, "\"filepath\"") {
-			fmt.Printf("Gap in line %d: expected: %s, actual: %s", i, lineExpected, lineActual)
+			fmt.Printf("Gap in line %d: expected(%s): %s, actual(%s): %s", i, expectedFile, lineExpected, actualFile, lineActual)
 			return false, nil
 		}
 	}

pkg/controller/engine.go

@@ -102,28 +102,27 @@ func parseResources(objs []parsedK8sObjects, args common.InArgs) ([]common.Resou
 		}
 	}
 	for idx := range resources {
-		resources[idx].CommitID = *args.CommitID
-		resources[idx].GitBranch = *args.GitBranch
-		resources[idx].GitURL = *args.GitURL
+		res := &resources[idx]
+		res.CommitID = *args.CommitID
+		res.GitBranch = *args.GitBranch
+		res.GitURL = *args.GitURL
 
 		// handle config maps data to be associated into relevant deployments resource objects
-		for _, cfgMapRef := range resources[idx].Resource.ConfigMapRefs {
-			configmapFullName := resources[idx].Resource.Namespace + "/" + cfgMapRef
+		for _, cfgMapRef := range res.Resource.ConfigMapRefs {
+			configmapFullName := res.Resource.Namespace + "/" + cfgMapRef
 			if cfgMap, ok := configmaps[configmapFullName]; ok {
-				// add to d Envs the values from data
-				// TODO: keep only data values with addresses of known services names
-				// pattern for relevant data value: http://[svc name]:[port] or [svc-name]:[port] or [svc-name] or  http://[svc name] (implied port 80)
-				// The port is optional when it is the default port for a given protocol (e.g., HTTP=80).
 				for _, v := range cfgMap.Data {
-					resources[idx].Resource.Envs = append(resources[idx].Resource.Envs, v)
+					res.Resource.Envs = append(res.Resource.Envs, v)
 				}
 			}
 		}
-		for _, cfgMapKeyRef := range resources[idx].Resource.ConfigMapKeyRefs {
-			configmapFullName := resources[idx].Resource.Namespace + "/" + cfgMapKeyRef.Name
+		for _, cfgMapKeyRef := range res.Resource.ConfigMapKeyRefs {
+			configmapFullName := res.Resource.Namespace + "/" + cfgMapKeyRef.Name
 			if cfgMap, ok := configmaps[configmapFullName]; ok {
 				if val, ok := cfgMap.Data[cfgMapKeyRef.Key]; ok {
-					resources[idx].Resource.Envs = append(resources[idx].Resource.Envs, val)
+					if analyzer.IsNetworkAddressValue(val) {
+						res.Resource.Envs = append(res.Resource.Envs, val)
+					}
 				}
 			}
 		}

pkg/controller/synth_netpols.go

@@ -3,7 +3,6 @@ package controller
 import (
 	"reflect"
 	"sort"
-	"strings"
 
 	core "k8s.io/api/core/v1"
 	network "k8s.io/api/networking/v1"
@@ -55,17 +54,20 @@ func determineConnectivityPerDeployment(connections []common.Connections) []*Dep
 		conn := &connections[idx]
 		srcDeploy := findOrAddDeploymentConn(conn.Source, deploysConnectivity)
 		dstDeploy := findOrAddDeploymentConn(conn.Target, deploysConnectivity)
-		targetPorts := toNetpolPorts(conn.Link.Resource.Network) // TODO: filter by src ports
+		targetPorts := toNetpolPorts(conn.Link.Resource.Network)
+		if conn.Source != nil && len(conn.Source.Resource.UsedPorts) > 0 {
+			targetPorts = toNetpolPorts(conn.Source.Resource.UsedPorts)
+		}
 
-		egressNetpolPeer := []network.NetworkPolicyPeer{{PodSelector: getDeployConnSelector(dstDeploy)}}
 		if srcDeploy != nil {
-			srcDeploy.addEgressRule(egressNetpolPeer, targetPorts)
+			netpolPeer := getNetpolPeer(srcDeploy, dstDeploy)
+			srcDeploy.addEgressRule([]network.NetworkPolicyPeer{netpolPeer}, targetPorts)
 		}
 
 		if conn.Link.Resource.Type == "LoadBalancer" || conn.Link.Resource.Type == "NodePort" {
 			dstDeploy.addIngressRule([]network.NetworkPolicyPeer{}, targetPorts) // in these cases we want to allow traffic from all sources
 		} else if conn.Source != nil {
-			netpolPeer := network.NetworkPolicyPeer{PodSelector: getDeployConnSelector(srcDeploy)}
+			netpolPeer := getNetpolPeer(dstDeploy, srcDeploy)
 			dstDeploy.addIngressRule([]network.NetworkPolicyPeer{netpolPeer}, targetPorts) // allow traffic only from this specific source
 		}
 	}
@@ -94,18 +96,20 @@ func findOrAddDeploymentConn(resource *common.Resource, deployConns map[string]*
 	return &deploy
 }
 
-func getDeployConnSelector(deployConn *DeploymentConnectivity) *metaV1.LabelSelector {
-	selectorsMap := map[string]string{}
-	for _, selector := range deployConn.Resource.Resource.Selectors {
-		colonPos := strings.Index(selector, ":")
-		if colonPos == -1 {
-			continue
-		}
-		key := selector[:colonPos]
-		value := selector[colonPos+1:]
-		selectorsMap[key] = value
+func getNetpolPeer(netpolDeploy, otherDeploy *DeploymentConnectivity) network.NetworkPolicyPeer {
+	netpolPeer := network.NetworkPolicyPeer{PodSelector: getDeployConnSelector(otherDeploy)}
+	if netpolDeploy.Resource.Resource.Namespace != otherDeploy.Resource.Resource.Namespace {
+		if otherDeploy.Resource.Resource.Namespace != "" {
+			netpolPeer.NamespaceSelector = &metaV1.LabelSelector{
+				MatchLabels: map[string]string{"kubernetes.io/metadata.name": otherDeploy.Resource.Resource.Namespace},
+			}
+		} // if otherDeploy has no namespace specified, we assume it is in the same namespace as the netpolDeploy
 	}
-	return &metaV1.LabelSelector{MatchLabels: selectorsMap}
+	return netpolPeer
+}
+
+func getDeployConnSelector(deployConn *DeploymentConnectivity) *metaV1.LabelSelector {
+	return &metaV1.LabelSelector{MatchLabels: deployConn.Resource.Resource.Labels}
 }
 
 func toNetpolPorts(ports []common.SvcNetworkAttr) []network.NetworkPolicyPort {