Removal and exchange duplicate test for privileged containers #2119
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
horecoli
force-pushed
the
privileged_exchange
branch
2 times, most recently
from
9a47a29
to
8c7f5c2
Compare
kosstennbl
requested changes
Aug 1, 2024
spec/workload/security_spec.cr
Outdated
| begin | ||
| result = ShellCmd.run_testsuite("cnf_setup cnf-config=sample-cnfs/sample-statefulset-cnf/cnf-testsuite.yml") | ||
| Log.debug { result[:output] } | ||
| result = ShellCmd.run_testsuite("privileged verbose") | ||
| result = ShellCmd.run_testsuite("privileged_containers verbose") | ||
| result[:status].success?.should be_true | ||
| (/Found.*privileged containers.*coredns/ =~ result[:output]).should be_nil |
There was a problem hiding this comment.
This output check doesn't seem right when comparing to test outputs, the test will always pass.
There was a problem hiding this comment.
I am not sure what you mean, I didn't change anything related to output, I just renamed test during its execution.
There was a problem hiding this comment.
but yes, there was issue with regex before, which we didn't know about it. I fixed it in last commit. Thanks.
spec/workload/security_spec.cr
Outdated
| @@ -4,35 +4,35 @@ require "../../src/tasks/utils/utils.cr" | |||
|
|
|||
| describe "Security" do | |||
|
|
|||
| it "'privileged' should pass with a non-privileged cnf", tags: ["privileged"] do | |||
| it "'privileged_containers' should pass with a non-privileged cnf", tags: ["privileged"] do | |||
There was a problem hiding this comment.
Maybe tag could be changed to "privileged_containers"? And same would be used for privilege escalation tests.
horecoli
force-pushed
the
privileged_exchange
branch
from
8c7f5c2
to
1fd4080
Compare
kosstennbl
requested changes
Aug 5, 2024
.github/workflows/actions.yml
Outdated
| @@ -402,7 +402,7 @@ jobs: | |||
| ./cnf-testsuite setup | |||
| wget -O cnf-testsuite.yml https://raw.githubusercontent.com/cnti-testcatalog/testsuite/main/example-cnfs/coredns/cnf-testsuite.yml | |||
| ./cnf-testsuite cnf_setup cnf-config=./cnf-testsuite.yml | |||
| LOG_LEVEL=info ./cnf-testsuite all ~compatibility ~resilience ~reasonable_startup_time ~reasonable_image_size ~platform ~privileged ~increase_capacity ~decrease_capacity ~install_script_helm ~helm_chart_valid ~helm_chart_published verbose | |||
| LOG_LEVEL=info ./cnf-testsuite all ~compatibility ~resilience ~reasonable_startup_time ~reasonable_image_size ~platform ~privileges ~increase_capacity ~decrease_capacity ~install_script_helm ~helm_chart_valid ~helm_chart_published verbose | |||
There was a problem hiding this comment.
I think task names are used there, not spec tags. Should be ~privileged_containers
There was a problem hiding this comment.
yes, you right, this is just excluding and we don't want to exclude it now, so I just removed it.
.github/workflows/actions.yml
Outdated
| @@ -477,7 +477,7 @@ jobs: | |||
| ./cnf-testsuite setup | |||
| wget -O cnf-testsuite.yml https://raw.githubusercontent.com/cnti-testcatalog/testsuite/main/example-cnfs/coredns/cnf-testsuite.yml | |||
| ./cnf-testsuite cnf_setup cnf-config=./cnf-testsuite.yml | |||
| LOG_LEVEL=info ./cnf-testsuite all ~resilience ~compatibility ~pod_network_latency ~platform ~privileged ~increase_capacity ~decrease_capacity ~ip_addresses ~liveness ~readiness ~rolling_update ~rolling_downgrade ~rolling_version_change ~nodeport_not_used ~hostport_not_used ~hardcoded_ip_addresses_in_k8s_runtime_configuration ~install_script_helm ~helm_chart_valid ~helm_chart_published ~rollback ~secrets_used ~immutable_configmap verbose | |||
| LOG_LEVEL=info ./cnf-testsuite all ~resilience ~compatibility ~pod_network_latency ~platform ~privileges ~increase_capacity ~decrease_capacity ~ip_addresses ~liveness ~readiness ~rolling_update ~rolling_downgrade ~rolling_version_change ~nodeport_not_used ~hostport_not_used ~hardcoded_ip_addresses_in_k8s_runtime_configuration ~install_script_helm ~helm_chart_valid ~helm_chart_published ~rollback ~secrets_used ~immutable_configmap verbose | |||
This change removes the Kubescape implementation of test for privileged containers. The reason behind this decision is the absence of functionality to exclude containers from the test. Instead, it has been replaced with an in-house implementation of this test, which has been renamed from "privileged" to "privileged_containers". Implements: cnti-testcatalog#2115 Signed-off-by: horecoli <[email protected]>
horecoli
force-pushed
the
privileged_exchange
branch
from
1fd4080
to
860c576
Compare
martin-mat
requested review from
taylor,
collivier,
martin-mat,
rich-l,
Smitholi67,
barmull and
svteb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This change removes the Kubescape implementation of test for privileged containers. The reason behind this decision is the absence of functionality to exclude containers from the test. Instead, it has been replaced with an in-house implementation of this test, which has been renamed from "privileged" to "privileged_containers".
Issues:
Refs: #2115
How has this been tested:
Types of changes:
Checklist:
Documentation
Code Review
Issue