Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

eval command support #423

Draft
wants to merge 21 commits into
base: support_admin_netpolicy
Choose a base branch
from
Draft

eval command support #423

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5e90022
eval command support + unit tests
shireenf-ibm Oct 20, 2024
1e8ccce
command line tests with anps + updating support for workloads resources
shireenf-ibm Oct 21, 2024
4d9cc94
Merge branch 'support_admin_netpolicy' into support_anp_for_eval_cmd
shireenf-ibm Oct 30, 2024
3eac6cf
adding one more test so all god paths are covered
shireenf-ibm Oct 30, 2024
6faec8d
Merge branch 'support_admin_netpolicy' into support_anp_for_eval_cmd
shireenf-ibm Nov 3, 2024
ccb1ea7
Update pkg/netpol/eval/internal/k8s/netpol.go
shireenf-ibm Nov 4, 2024
3d1b4f6
Merge branch 'support_admin_netpolicy' into support_anp_for_eval_cmd
shireenf-ibm Nov 4, 2024
3d6431a
first fixes
shireenf-ibm Nov 6, 2024
b171aa6
common code
shireenf-ibm Nov 6, 2024
31ef6cf
revert accepting workloads as input for evaluate cmd-line
shireenf-ibm Nov 6, 2024
20aae29
adding tests from dir to command and eval tests; command-line needs g…
shireenf-ibm Nov 10, 2024
fedb422
generating the tmp dir in the project path, since permissions are den…
shireenf-ibm Nov 10, 2024
279b759
fixes to fit github permissions
shireenf-ibm Nov 10, 2024
0f9cdc9
Merge branch 'support_admin_netpolicy' into support_anp_for_eval_cmd
shireenf-ibm Nov 12, 2024
480676e
comments + changing mode
shireenf-ibm Nov 12, 2024
d5406d1
renaming struct field
shireenf-ibm Nov 12, 2024
957e4f0
updating mode fields
shireenf-ibm Nov 12, 2024
0f4520a
rename func
shireenf-ibm Nov 12, 2024
fa67555
tmp dir
shireenf-ibm Nov 12, 2024
ffdcefc
renaming attributes
shireenf-ibm Nov 12, 2024
088cb77
modifying func
shireenf-ibm Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 71 additions & 1 deletion pkg/cli/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,10 @@ func TestEvalCommandOutput(t *testing.T) {
cases := []struct {
dir string
sourcePod string
sourceNs string
destNs string
destPod string
protocol string
port string
evalResult bool
}{
Expand All @@ -368,13 +371,80 @@ func TestEvalCommandOutput(t *testing.T) {
port: "80",
evalResult: false,
},
{
dir: "anp_demo",
sourceNs: "gryffindor",
sourcePod: "harry-potter-1",
destPod: "luna-lovegood-1",
destNs: "ravenclaw",
protocol: "udp",
port: "52",
evalResult: true,
},
{
dir: "anp_test_6",
sourceNs: "network-policy-conformance-slytherin",
sourcePod: "draco-malfoy-1",
destPod: "cedric-diggory-1",
destNs: "network-policy-conformance-hufflepuff",
protocol: "udp",
port: "5353",
evalResult: false,
},
{
dir: "anp_test_multiple_anps",
sourceNs: "network-policy-conformance-ravenclaw",
sourcePod: "luna-lovegood-1",
destPod: "draco-malfoy-1",
destNs: "network-policy-conformance-slytherin",
protocol: "sctp",
port: "9003",
evalResult: false,
},
{
dir: "anp_with_np_and_banp_pass_test",
sourceNs: "ns2",
sourcePod: "pod1-1",
destPod: "pod1-1",
destNs: "ns1",
port: "80",
evalResult: true,
},
{
dir: "anp_with_np_pass_test",
sourceNs: "ns2",
sourcePod: "pod1-1",
destPod: "pod1-1",
destNs: "ns1",
port: "8080",
evalResult: false,
},
{
dir: "anp_banp_core_test",
sourceNs: "network-policy-conformance-gryffindor",
sourcePod: "harry-potter-1",
destPod: "cedric-diggory-1",
destNs: "network-policy-conformance-hufflepuff",
port: "8080",
evalResult: true,
},
}
for _, tt := range cases {
tt := tt
testName := "eval_" + tt.dir + "_from_" + tt.sourcePod + "_to_" + tt.destPod
t.Run(testName, func(t *testing.T) {
if tt.protocol == "" {
tt.protocol = defaultProtocol
}
if tt.sourceNs == "" {
tt.sourceNs = defaultNs
}
if tt.destNs == "" {
tt.destNs = defaultNs
}
args := []string{"eval", "--dirpath", testutils.GetTestDirPath(tt.dir),
"-s", tt.sourcePod, "-d", tt.destPod, "-p", tt.port}
"-s", tt.sourcePod, "-d", tt.destPod, "-p", tt.port, "--protocol", tt.protocol,
"-n", tt.sourceNs, "--destination-namespace", tt.destNs}
actualOut, err := buildAndExecuteCommand(args)
require.Nil(t, err, "test: %q", testName)
require.Contains(t, actualOut, fmt.Sprintf("%v", tt.evalResult),
Expand Down
33 changes: 30 additions & 3 deletions pkg/cli/evaluate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,16 @@ import (
// Currently adds many options flags, so wait until cobra supports something
// like NamedFlagSet's.

const (
defaultNs = "default"
defaultProtocol = "tcp"
)

adisos marked this conversation as resolved.
Show resolved Hide resolved
var (
// evaluated connection information
protocol = "tcp"
sourcePod = types.NamespacedName{Namespace: "default"}
destinationPod = types.NamespacedName{Namespace: "default"}
protocol = defaultProtocol
sourcePod = types.NamespacedName{Namespace: defaultNs}
destinationPod = types.NamespacedName{Namespace: defaultNs}
srcExternalIP string
dstExternalIP string
port string
Expand Down Expand Up @@ -63,6 +68,7 @@ func validateEvalFlags() error {
return nil
}

//gocyclo:ignore
func updatePolicyEngineObjectsFromDirPath(pe *eval.PolicyEngine, podNames []types.NamespacedName) error {
// get relevant resources from dir path
eLogger := logger.NewDefaultLoggerWithVerbosity(determineLogVerbosity())
Expand Down Expand Up @@ -92,12 +98,33 @@ func updatePolicyEngineObjectsFromDirPath(pe *eval.PolicyEngine, podNames []type
for i := range objectsList {
obj := objectsList[i]
switch obj.Kind {
// workloads kinds
case parser.Pod:
err = pe.InsertObject(obj.Pod)
case parser.ReplicaSet:
err = pe.InsertObject(obj.ReplicaSet)
case parser.Deployment:
err = pe.InsertObject(obj.Deployment)
case parser.DaemonSet:
err = pe.InsertObject(obj.DaemonSet)
case parser.StatefulSet:
err = pe.InsertObject(obj.StatefulSet)
case parser.ReplicationController:
err = pe.InsertObject(obj.ReplicationController)
case parser.Job:
err = pe.InsertObject(obj.Job)
case parser.CronJob:
err = pe.InsertObject(obj.CronJob)
// ns kind
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if eval only supports pods, why adding these resources?
If the dir path only contains workload manifests and not pods, how would one infer the pod name for this command?
maybe this should be a separate issue, to extend eval support for workloads, but in this PR can just focus on pods?
the tests automation can add the pods resources and adjust the input, as if those manifests would exist for testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure understood last sentence;
my tests dir paths contain manifests with StatefulSet workloads (I used replica name as the input of the test (pod name);
The tests failed without adding these cases (my manifests were not inserted to the policy-engine because of the lack of these cases; );
should I revert these changes and create new tests with pods only in the manifests?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for these tests to work, you can add test automation (for eval tests) that add pods resources as if they existed in the manifests.. the changes here better be removed, until eval actually supports such resources, because in the current eval usage workload resources are not supported, and a user cannot infer the pod name to be used in this command when providing only workload resources.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified and added tests:

  1. command_test.go : since command-line needs the "dir" as an argument to execute the cmd;
    for those tests (which don't contain pods in the manifests), I've generated a "temp dir" which is a copy of the original dir + generated yamls for pods (for the given src + dst arguments) so the command runs on the "temporary dir" successfully.
    generating pod yaml for a workload considers also its labels (which may be used in the netpols rules and are essential for computing the result);

  2. eval_test.go uses the internal eval funcs, so it works successfully with the original dirs.
    in this file we had tests from parsed resources (that tests the "eval" functionality) ;
    and I have added eval tests from dir paths.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, please open a separate issue to consider supporting eval with input workloads, not just pods, and also add a comment for this test functionality added here, that it can be removed when this eval extension is added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, new issue #440 ; + comments were added in the relevant files

case parser.Namespace:
err = pe.InsertObject(obj.Namespace)
// netpols kinds
case parser.NetworkPolicy:
err = pe.InsertObject(obj.NetworkPolicy)
case parser.AdminNetworkPolicy:
err = pe.InsertObject(obj.AdminNetworkPolicy)
case parser.BaselineAdminNetworkPolicy:
err = pe.InsertObject(obj.BaselineAdminNetworkPolicy)
default:
continue
}
Expand Down
40 changes: 40 additions & 0 deletions pkg/manifests/parser/k8sobj.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ var policyKinds = map[string]bool{
BaselineAdminNetworkPolicy: true,
}

//nolint:funlen // cases may not be shorten
//gocyclo:ignore
func FilterObjectsList(allObjects []K8sObject, podNames []types.NamespacedName) []K8sObject {
podNamesMap := make(map[string]bool, 0)
nsMap := make(map[string]bool, 0)
Expand All @@ -234,6 +236,40 @@ func FilterObjectsList(allObjects []K8sObject, podNames []types.NamespacedName)
if _, ok := podNamesMap[types.NamespacedName{Name: obj.Pod.Name, Namespace: obj.Pod.Namespace}.String()]; ok {
res = append(res, obj)
}
// the input pod-names is the name of the pod (replica);
// so if the manifest resources contain a "workload" type (other than Pod);
// the pod name is actually a name of a replica of the workload, and not the name of the workload (obj.Name);
// so here we will compare only namespaces;
// and podName will be compared later after creating the pods (replicas) from the
// workload in the eval package
case StatefulSet:
if _, ok := nsMap[obj.StatefulSet.Namespace]; ok {
res = append(res, obj)
}
case DaemonSet:
if _, ok := nsMap[obj.DaemonSet.Namespace]; ok {
res = append(res, obj)
}
case Deployment:
if _, ok := nsMap[obj.Deployment.Namespace]; ok {
res = append(res, obj)
}
case ReplicaSet:
if _, ok := nsMap[obj.ReplicaSet.Namespace]; ok {
res = append(res, obj)
}
case Job:
if _, ok := nsMap[obj.Job.Namespace]; ok {
res = append(res, obj)
}
case CronJob:
if _, ok := nsMap[obj.CronJob.Namespace]; ok {
res = append(res, obj)
}
case ReplicationController:
if _, ok := nsMap[obj.ReplicationController.Namespace]; ok {
res = append(res, obj)
}
case Service:
if _, ok := nsMap[obj.Service.Namespace]; ok {
res = append(res, obj)
Expand All @@ -246,6 +282,10 @@ func FilterObjectsList(allObjects []K8sObject, podNames []types.NamespacedName)
if _, ok := nsMap[obj.Ingress.Namespace]; ok {
res = append(res, obj)
}
case AdminNetworkPolicy:
res = append(res, obj)
case BaselineAdminNetworkPolicy:
res = append(res, obj)
default:
continue
}
Expand Down
Loading