-
Notifications
You must be signed in to change notification settings - Fork 207
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
test: add containerd seccomp default profile for golden profile test data #5230
base: dev
Are you sure you want to change the base?
Conversation
…file changes, the test will fail
@@ -0,0 +1,13 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the meaning of 48.json
for the kernel ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those syscalls are added if you are using kernel 4.8 and above.
i have not included those in the current e2e check yet (which now only contains base checks)..
@@ -185,3 +189,34 @@ func leakedSecretsValidators(scenario *Scenario) []*LiveVMValidator { | |||
} | |||
return validators | |||
} | |||
|
|||
func createKubeletDebugPod(ctx context.Context, t *testing.T, kube *Kubeclient, nodeName string, isAirgap bool) { | |||
testPodName := "security-context-profile-test-pod" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a Generic DebugPod is a security-context-profile test pods ? the function name doesnt seem to match what the function is doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed. The pod's intention is to create 2 containers
- one with specified securityContext
- one without, so it will be using kubelet default
- runtimeDefault if seccompDefault flag is enabled
- unconfined if not.
would createSeccompProfileDebugPod be more accurate for the moment? although i do notice there is a lack of coverage on customKubeletConfig, but we can improve that later.
func createKubeletDebugPod(ctx context.Context, t *testing.T, kube *Kubeclient, nodeName string, isAirgap bool) { | ||
testPodName := "security-context-profile-test-pod" | ||
testPodManifest := getSecurityContextPodTemplate(isAirgap, nodeName, testPodName) | ||
t.Logf("Custom kubelet config scenario: running debug pod on node %s ...", nodeName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which custom kubelet config ? is the entention to run a security-context pod when ever a custom kubelet config is provided ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct. checks on when tag KubeletCustomConfig is set to true
AgentBaker/e2e/scenario_helpers_test.go
Line 129 in 979c12f
if scenario.Tags.KubeletCustomConfig { |
@@ -126,6 +126,9 @@ func createAndValidateVM(ctx context.Context, t *testing.T, scenario *Scenario) | |||
validateWasm(ctx, t, scenario.Runtime.Cluster.Kube, nodeName) | |||
} | |||
|
|||
if scenario.Tags.KubeletCustomConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are all Kubelet Custom Config scenarios requiring a debugPod to properly test them ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes and no. i have introduced the tag and have only used it in 2 e2e scenarios so far
AgentBaker/e2e/scenario_test.go
Line 1258 in 979c12f
func Test_Ubuntu2204_KubeletCustomConfig(t *testing.T) { |
i did not use tag "SeccompDefaultProfile" ish tag for the reason that this is only one of the flags we used in customKubeletConfig. since this is e2e, i think customKubeletConfig tag is at a better abstraction level.
leaving space for other customKubeletConfig checks when we want to include in retrospect and to consolidate the e2e test scenarios maybe around VHD * OS SKU matrix .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is special case for a single case. I would instead add another hook to Scenario
.
Something like DoExtraStuff: func(s *Scenario, t *testing.T)
or PrepareCluster
(it's all bad names, come up with something better)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively, can it be a part of validator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels unnecessary deep. I would put it in testdata/seccomp_profile_amd64.json
@@ -126,6 +126,9 @@ func createAndValidateVM(ctx context.Context, t *testing.T, scenario *Scenario) | |||
validateWasm(ctx, t, scenario.Runtime.Cluster.Kube, nodeName) | |||
} | |||
|
|||
if scenario.Tags.KubeletCustomConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is special case for a single case. I would instead add another hook to Scenario
.
Something like DoExtraStuff: func(s *Scenario, t *testing.T)
or PrepareCluster
(it's all bad names, come up with something better)
actualSyscallsConsolidated := make(map[string][]string) | ||
for _, syscall := range actual.Syscalls { | ||
actualSyscallsConsolidated[syscall.Action] = append(syscall.Names, actualSyscallsConsolidated[syscall.Action]...) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it feels like assert.JSONEq(t *testing.T, stdout, baseProfileFile)
@@ -126,6 +126,9 @@ func createAndValidateVM(ctx context.Context, t *testing.T, scenario *Scenario) | |||
validateWasm(ctx, t, scenario.Runtime.Cluster.Kube, nodeName) | |||
} | |||
|
|||
if scenario.Tags.KubeletCustomConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively, can it be a part of validator?
What type of PR is this?
/kind test
What this PR does / why we need it:
add default seccomp profile for containerd so that we are alerted when the profile is changed (which could break existing workload when kubelet seccomp feature is enabled and RuntimeDefault profile is used
Which issue(s) this PR fixes:
Fixes #
Requirements:
Special notes for your reviewer:
Release note: