Consistently use namespaceMatchLabels across rules #316

georgibaltiev · 2024-10-11T05:39:19Z

What this PR does / why we need it:
This PR refactors the accepted pod structs implemented for rules #242414 and #242415 by aggregating the label matching options into a singular shared structure.
The accepted pod struct for #242417 is refactored to utilize the new structure and now can add exemptions from the ruleset checks by matching namespaces by labels.
The accepted pod struct for #242383 is refactored as well to use namespace labels for matching its rule exemptions.

Which issue(s) this PR fixes:
Fixes #286

Special notes for your reviewer:

Release note:

This change affects only the `managedk8s` provider. Configuration for DISA K8s STIG rules `242383` and `242417` now does not accept namespace names directly but rely on matching namespaces by labels. Please, see [the example configuration options](https://github.com/gardener/diki/blob/main/example/config/managedk8s.yaml) for more details.

… base structure

AleksandarSavchev

Please adapt the example managedk8s diki config file to use the new namespaceMatchLabels option:

diki/example/config/managedk8s.yaml

Lines 29 to 33 in 1529d1b

    
           #       # Defaults to ["default", "kube-public", "kube-node-lease"] 
        
           #       namespaceNames: 
        
           #       - default 
        
           #       - kube-public 
        
           #       - kube-node-lease

diki/example/config/managedk8s.yaml

Lines 105 to 108 in 1529d1b

    
           #       namespaceNames: 
        
           #       - kube-system 
        
           #       - kube-public 
        
           #       - kube-node-lease

Also write a comment describing which namespaces can be selected. Example for 242383:

# Only namespaces in ["default", "kube-public", "kube-node-lease"]
# can be selected with namespaceMatchLabels

example/guides/partial-disa-k8s-stig-shoot.yaml

pkg/shared/ruleset/disak8sstig/option/options.go

pkg/provider/gardener/ruleset/disak8sstig/v1r11_ruleset.go

pkg/provider/gardener/ruleset/disak8sstig/v2r1_ruleset.go

pkg/shared/ruleset/disak8sstig/option/options.go

pkg/shared/ruleset/disak8sstig/rules/242417.go

pkg/shared/ruleset/disak8sstig/rules/242383.go

georgibaltiev · 2024-10-11T11:56:30Z

I've added 3 new commits that may resolve the issues. The unnecessary newlines are removed, and the structs are renamed accordingly. New configurations are added in the yaml files as well

dimityrmirchev · 2024-10-14T08:25:18Z

example/config/managedk8s.yaml

-    #       - default
-    #       - kube-public
-    #       - kube-node-lease
+    #       # only namespaces in ["default", "kube-public", "kube-node-lease" can be selected with namespaceMatchLabels


Suggested change

# # only namespaces in ["default", "kube-public", "kube-node-lease" can be selected with namespaceMatchLabels

# # only namespaces in ["default", "kube-public", "kube-node-lease"] are meaningful to be selected with namespaceMatchLabels

# since the rule does not perform checks on objects in namespaces different from the listed above

dimityrmirchev · 2024-10-14T08:25:32Z

example/config/managedk8s.yaml

-    #       - kube-system
-    #       - kube-public
-    #       - kube-node-lease
+    #       # only pods in namespaces ["kube-system", "kube-public", "kube-node-lease"] can be selected with namespaceMatchLabels


Same as above

dimityrmirchev · 2024-10-14T08:25:46Z

example/guides/partial-disa-k8s-stig-shoot.yaml

          justification: "Pods managed by Gardener are not considered as user pods"
-    - ruleID: "242449"
+    - ruleID: "242449" 


Suggested change

- ruleID: "242449"

- ruleID: "242449"

pkg/provider/gardener/ruleset/disak8sstig/v1r11_ruleset.go

dimityrmirchev · 2024-10-14T08:28:32Z

pkg/provider/gardener/ruleset/disak8sstig/v2r1_ruleset.go

@@ -352,12 +350,15 @@ func (r *Ruleset) registerV2R1Rules(ruleOptions map[string]config.RuleOptionsCon
 			Options: &sharedrules.Options242417{
 				AcceptedPods: []sharedrules.AcceptedPods242417{
 					{
-						PodMatchLabels: map[string]string{
+						PodSelector: option.PodSelector{PodMatchLabels: map[string]string{


Suggested change

PodSelector: option.PodSelector{PodMatchLabels: map[string]string{

PodSelector: option.PodSelector{

PodMatchLabels: map[string]string{

dimityrmirchev · 2024-10-14T08:46:20Z

pkg/shared/ruleset/disak8sstig/rules/242383.go

@@ -88,19 +107,23 @@ func (r *Rule242383) Name() string {
 }

 func (r *Rule242383) Run(ctx context.Context) (rule.RuleResult, error) {
+	allNamespaces, err := kubeutils.GetNamespaces(ctx, r.Client)
+	if err != nil {
+		return rule.SingleCheckResult(r, rule.ErroredCheckResult(err.Error(), rule.NewTarget("", ""))), nil


Suggested change

return rule.SingleCheckResult(r, rule.ErroredCheckResult(err.Error(), rule.NewTarget("", ""))), nil

return rule.SingleCheckResult(r, rule.ErroredCheckResult(err.Error(), rule.NewTarget())), nil

dimityrmirchev · 2024-10-14T08:46:40Z

pkg/shared/ruleset/disak8sstig/rules/242417.go

 	selector := labels.NewSelector().Add(*notDikiPodReq)

+	allNamespaces, err := kubeutils.GetNamespaces(ctx, r.Client)
+	if err != nil {
+		return rule.SingleCheckResult(r, rule.ErroredCheckResult(err.Error(), rule.NewTarget("", ""))), nil


Suggested change

return rule.SingleCheckResult(r, rule.ErroredCheckResult(err.Error(), rule.NewTarget("", ""))), nil

return rule.SingleCheckResult(r, rule.ErroredCheckResult(err.Error(), rule.NewTarget()), nil

dimityrmirchev · 2024-10-14T08:49:37Z

pkg/shared/ruleset/disak8sstig/rules/242383.go

 				APIVersion: "v1",
 				Kind:       "Service",
 				MatchLabels: map[string]string{
 					"component": "apiserver",
 					"provider":  "kubernetes",
 				},
-				NamespaceNames: []string{"default"},
+				NamespaceMatchLabels: allNamespaces["default"].Labels,


Let's match the default namespace by the metadata name label similarly to how we match other namespaces. It would be better to keep this consistent since it will be easier for developers to search similar usages through the code.

dimityrmirchev · 2024-10-14T08:52:44Z

pkg/shared/ruleset/disak8sstig/option/options.go

@@ -106,10 +132,9 @@ var _ Option = (*Options242415)(nil)

 // AcceptedPods242415 contains option specifications for appected pods


Suggested change

// AcceptedPods242415 contains option specifications for appected pods

// AcceptedPods242415 contains option specifications for accepted pods

dimityrmirchev · 2024-10-14T08:53:12Z

pkg/shared/ruleset/disak8sstig/option/options.go

@@ -70,10 +98,9 @@ var _ Option = (*Options242414)(nil)

 // AcceptedPods242414 contains option specifications for appected pods


Suggested change

// AcceptedPods242414 contains option specifications for appected pods

// AcceptedPods242414 contains option specifications for accepted pods

…els comments.

example/config/managedk8s.yaml

dimityrmirchev

/lgtm

AleksandarSavchev

/lgtm

georgibaltiev added 4 commits October 10, 2024 17:45

Refactor label matching options for 242414 and 242415 into a singular…

1803c63

… base structure

Refactor rules 242417 and 242383

ad85476

add 242417.go

a0f045a

Readjust file formatting and add modified partial-disa-configuration

eae6d13

georgibaltiev requested a review from a team as a code owner October 11, 2024 05:39

gardener-robot added needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Oct 11, 2024

AleksandarSavchev self-requested a review October 11, 2024 06:04

dimityrmirchev self-requested a review October 11, 2024 06:15

AleksandarSavchev requested changes Oct 11, 2024

View reviewed changes

gardener-robot added the needs/changes Needs (more) changes label Oct 11, 2024

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 11, 2024

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 11, 2024

Reformat whitespaces. Refactor variable names and configuration files

05d3749

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 11, 2024

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 11, 2024

georgibaltiev requested a review from AleksandarSavchev October 11, 2024 12:56

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 11, 2024

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 11, 2024

dimityrmirchev requested changes Oct 14, 2024

View reviewed changes

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 14, 2024

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 14, 2024

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 14, 2024

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 14, 2024

georgibaltiev force-pushed the unify-namespace-labels branch from 7330903 to 05d3749 Compare October 14, 2024 13:16