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.

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

eval command support #423

Draft
wants to merge 21 commits into
base: support_admin_netpolicy
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/openshift/api v0.0.0-20230502160752-c71432710382
github.com/spf13/cobra v1.8.1
github.com/stretchr/testify v1.9.0
gopkg.in/yaml.v2 v2.4.0
k8s.io/api v0.29.2
k8s.io/apimachinery v0.29.2
k8s.io/cli-runtime v0.29.2
Expand Down Expand Up @@ -56,7 +57,6 @@ require (
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/protobuf v1.33.0 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/klog/v2 v2.110.1 // indirect
k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect
Expand Down
146 changes: 82 additions & 64 deletions pkg/cli/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,14 +348,15 @@ func TestDiffCommandOutput(t *testing.T) {
// TestEvalCommandOutput tests the output of legal eval command
func TestEvalCommandOutput(t *testing.T) {
cases := []struct {
dir string
sourcePod string
sourceNs string
destNs string
destPod string
protocol string
port string
evalResult bool
dir string
sourcePod string
sourceNs string
destNs string
destPod string
protocol string
port string
evalResult bool
generateManifests bool // indicates if the test dir does not contain pods - to be generated
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename generateManifests to generatePodManifests

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

}{
{
dir: "onlineboutique",
Expand All @@ -372,61 +373,67 @@ func TestEvalCommandOutput(t *testing.T) {
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,
dir: "anp_demo",
sourceNs: "gryffindor",
sourcePod: "harry-potter",
destPod: "luna-lovegood",
destNs: "ravenclaw",
protocol: "udp",
port: "52",
evalResult: true,
generateManifests: true,
},
{
dir: "anp_test_6",
sourceNs: "network-policy-conformance-slytherin",
sourcePod: "draco-malfoy",
destPod: "cedric-diggory",
destNs: "network-policy-conformance-hufflepuff",
protocol: "udp",
port: "5353",
evalResult: false,
generateManifests: true,
},
{
dir: "anp_test_multiple_anps",
sourceNs: "network-policy-conformance-ravenclaw",
sourcePod: "luna-lovegood",
destPod: "draco-malfoy",
destNs: "network-policy-conformance-slytherin",
protocol: "sctp",
port: "9003",
evalResult: false,
generateManifests: true,
},
{
dir: "anp_with_np_and_banp_pass_test",
sourceNs: "ns2",
sourcePod: "pod1",
destPod: "pod1",
destNs: "ns1",
port: "80",
evalResult: true,
generateManifests: true,
},
{
dir: "anp_with_np_pass_test",
sourceNs: "ns2",
sourcePod: "pod1",
destPod: "pod1",
destNs: "ns1",
port: "8080",
evalResult: false,
generateManifests: true,
},
{
dir: "anp_banp_core_test",
sourceNs: "network-policy-conformance-gryffindor",
sourcePod: "harry-potter",
destPod: "cedric-diggory",
destNs: "network-policy-conformance-hufflepuff",
port: "8080",
evalResult: true,
generateManifests: true,
},
}
for _, tt := range cases {
Expand All @@ -442,7 +449,18 @@ func TestEvalCommandOutput(t *testing.T) {
if tt.destNs == "" {
tt.destNs = defaultNs
}
args := []string{"eval", "--dirpath", testutils.GetTestDirPath(tt.dir),
dirPath := testutils.GetTestDirPath(tt.dir)
var err error
if tt.generateManifests {
// getting here means the test dir contains workloads in the manifests (not pods)
// but since eval command only supports pods, we will generate a copy of the dirs with
// pods yaml files from the matching workload resource of the tt's source and dst.
// so the command may be executed with the given args
dirPath, err = testutils.GenerateTempDirWithPods(dirPath, tt.sourcePod, tt.sourceNs, tt.destPod, tt.destNs)
require.Nil(t, err, "test: %q", testName)
defer os.RemoveAll(dirPath) // clean up after finishing the test
}
args := []string{"eval", "--dirpath", dirPath,
"-s", tt.sourcePod, "-d", tt.destPod, "-p", tt.port, "--protocol", tt.protocol,
"-n", tt.sourceNs, "--destination-namespace", tt.destNs}
actualOut, err := buildAndExecuteCommand(args)
Expand Down
188 changes: 188 additions & 0 deletions pkg/internal/testutils/generate_pod_yamls.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
/*
Copyright 2023- IBM Inc. All Rights Reserved.

SPDX-License-Identifier: Apache-2.0
*/
package testutils

import (
"bytes"
"os"
"path/filepath"
"text/template"

"gopkg.in/yaml.v2"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// a test util for eval command-line; generating a new temporary directory with pod files to be used for testing.
// eval command supports only test directories with Pod objects.
// some of our testing directories are containing other Workload types in the manifests; in order to be able to test
// eval command on those dirs (in unit-testing); this file funcs are used.
// we copy the test-die into a new temporary dir, and generate into the tempDir :
// Pod yaml files for given src and dst peers from their workload resources

// GenerateTempDirWithPods generates new temporary dir that copies origDir and adds yaml files of Pod kind
// matching the input workload resources of the src and dst
// the function returns the path of the generated temp dir.
func GenerateTempDirWithPods(origDir, srcName, srcNs, dstName, dstNs string) (string, error) {
// making temp directory in the temp path
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this required? isn't it sufficient to just use TmpDir ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, done

tmpDir, err := os.MkdirTemp("", "temp-*")
if err != nil {
return "", err
}
// copy orig dir into the temp dir
err = copyDir(origDir, tmpDir, srcName, srcNs, dstName, dstNs)
return tmpDir, err
}

// copyDir copies files of network-policies from origDir into tempDir
// and generates into the tempDir : Pod yaml files for given src and dst peers from their workload resources
// in the origDir
func copyDir(origDir, tempDir, srcName, srcNs, dstName, dstNs string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

consider renaming copyDir to something like copyAndExtendDir or copyDirAndAddPods

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

return filepath.Walk(origDir, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() { // nothing to do
return nil
}
origFile := filepath.Join(origDir, info.Name())
tempFile := filepath.Join(tempDir, info.Name())
// if the file contains workload with given srcName or dstName : get the workload object and
// generate matching pod yaml file in the temp dir
// this is needed since we need also a copy of the workload labels which may be used in the netpols rules

// find src and get its labels
srcLabels, err := checkFileContainsInputWorkloadAndGetItsLabels(origFile, srcName, srcNs)
if err != nil {
return err
}
if srcLabels != nil { // nil means the src was not found in that file
err = generatePodYaml(tempDir, srcName, srcNs, srcLabels)
if err != nil {
return err
}
}

// find dst and get its labels
dstLabels, err := checkFileContainsInputWorkloadAndGetItsLabels(origFile, dstName, dstNs)
if err != nil {
return err
}
if dstLabels != nil { // nil means the dst was not found in that file
err = generatePodYaml(tempDir, dstName, dstNs, dstLabels)
if err != nil {
return err
}
}
// copy the orig file (having objects with kinds other than ns, pod, netpols will not affect the result
// since it will not be parsed with the eval command)
return copyFile(origFile, tempFile)
})
}

// PodInfo contains metadata of the pod so we can :
// 1. extract relevant workload from input resources
// 2. generate relevant pod template
type PodInfo struct {
Name string `yaml:"name"`
Namespace string `yaml:"namespace,omitempty"`
Labels map[string]string `yaml:"labels,omitempty"`
}

// workloadMetadata the yaml is unmarshal to workload metadata struct which is the only interesting part for our goals
type WorkloadMetadata struct {
Metadata PodInfo `yaml:"metadata"`
}

// checkFileContainsInputWorkloadAndGetItsLabels gets yaml contents and checks if it contains a workload object with the
// given name and namespace
// note that this assumes that if the object name and namespace matches the input, it is the workload object
// since our tests-dirs contain unique names for workloads (different than policies names)
// @todo : verify the kind is of a workload type too
func checkFileContainsInputWorkloadAndGetItsLabels(origFile, podName, podNs string) (map[string]string, error) {
fileContents, err := os.ReadFile(origFile)
if err != nil {
return nil, err
}
// Splitting the YAML into multiple documents
docs := splitYamlDocs(fileContents)

// we are interested in Metadata of the workload only.
// Iterate through objects to find the matching one
for _, doc := range docs {
var obj WorkloadMetadata
if err := yaml.Unmarshal(doc, &obj); err != nil {
return nil, err
}
if obj.Metadata.Name == podName && (obj.Metadata.Namespace == podNs ||
Comment on lines +121 to +131
Copy link
Collaborator

Choose a reason for hiding this comment

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

can use instead the already existing functions from manifests.GetResourceInfosFromDirPath() and
parser.ResourceInfoListToK8sObjectsList() ?

Copy link
Contributor Author

@shireenf-ibm shireenf-ibm Nov 12, 2024

Choose a reason for hiding this comment

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

we may use these funcs but I think that the written solution without using them is shorter and more efficient for my goal.

  1. I need to copy the yaml files to the temp dir (as it is the arg of the command); in this case those funcs may not help and i will have to walk the dir anyway.

  2. I don't need to convert all objects in the dir to k8s objects, I just want the specific workload resources of the src and dst (just 2 objects of many converted - I have nothing to do with k8s objects - they are not the args of the cmd;)

  3. I can not send those funcs only the workload files (assuming we can indicate what files are those; and ignore copying these files);

  • the file with the workload resource may also contain Namespace object and I must have this yaml in the output; because Ns labels may be used as selectors in the policy rules.

(obj.Metadata.Namespace == "" && podNs == metav1.NamespaceDefault)) {
if obj.Metadata.Labels == nil {
return map[string]string{}, nil
}
return obj.Metadata.Labels, nil
}
}
return nil, nil
}

const yamlSep = "---"

// splitYamlDocs splits a YAML file into separate documents.
// It returns a slice of byte slices, where each byte slice represents a YAML document.
func splitYamlDocs(data []byte) (docs [][]byte) {
// Split on YAML document separator
for _, docYAML := range bytes.Split(data, []byte(yamlSep)) {
if len(bytes.TrimSpace(docYAML)) == 0 {
continue
}
docs = append(docs, docYAML)
}
return docs
}

// copyFile copies origFile to tempFile
func copyFile(origFile, tempFile string) error {
contents, err := os.ReadFile(origFile)
if err != nil {
return err
}
err = os.WriteFile(tempFile, contents, os.ModeAppend)
return err
}

const podYamlTemplate = `apiVersion: v1
kind: Pod
metadata:
name: {{ .Name }}
namespace: {{ .Namespace }}
labels:
{{- range $key, $value := .Labels }}
{{ $key }}: {{ $value }}
{{- end }}
spec:
containers:
- name: container-1
image: nginx:latest
`

const yamlSuffix = ".yaml"

// generatePodYaml generates a YAML file for a given pod data.
func generatePodYaml(dirName, podName, podNs string, labels map[string]string) error {
pod := PodInfo{Name: podName, Namespace: podNs, Labels: labels}
fileName := podNs + "_" + podName + yamlSuffix
podFile := filepath.Join(dirName, fileName)
// write the pod template using the pod data
podTmpl, err := template.New("pod").Parse(podYamlTemplate)
if err != nil {
return err
}
var buf bytes.Buffer
if err := podTmpl.Execute(&buf, pod); err != nil {
return err
}
// write to file
return os.WriteFile(podFile, buf.Bytes(), os.ModeAppend)
}
Loading
Loading