Disable insecure parameters by default

This patch sets by default for all test pods: - runAsNonRoot: true - automountServiceAccountToken: false These two parameters shouldn't be enabled unless necessary for any pods in the OCP cluster. The runAsNonRoot parameter ensures that the container process does not run as root nor does it switch to root user after it has been spawned. And the automountServiceAccountToken makes sure that we are not mounting service account tokens into the test pods. This would allow them to modify the state of the OCP cluster. As of now the service account tokens are not required by any of the test pods. This behavior can be overridden by setting privileged: true.
openstack-k8s-operators · Dec 10, 2024 · 038565e · 038565e
1 parent b2b1b46
commit 038565e
Show file tree

Hide file tree

Showing 16 changed files with 100 additions and 67 deletions.
diff --git a/api/bases/test.openstack.org_ansibletests.yaml b/api/bases/test.openstack.org_ansibletests.yaml
@@ -146,11 +146,12 @@ spec:
                 default: false
                 description: |-
                   Use with caution! This parameter specifies whether test-operator should spawn test
-                  pods with allowedPrivilegedEscalation: true and the default capabilities on
-                  top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
+                  pods with allowedPrivilegedEscalation: true, runAsNonRoot: false,
+                  automountServiceAccountToken: true, and the default capabilities on top of
+                  capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
                   This parameter is deemed insecure but it is needed for certain test-operator
-                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set
-                  of tobiko tests).
+                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain
+                  set of tobiko tests).
                 type: boolean
               storageClass:
                 default: local-storage

diff --git a/api/bases/test.openstack.org_horizontests.yaml b/api/bases/test.openstack.org_horizontests.yaml
@@ -159,11 +159,12 @@ spec:
                 default: false
                 description: |-
                   Use with caution! This parameter specifies whether test-operator should spawn test
-                  pods with allowedPrivilegedEscalation: true and the default capabilities on
-                  top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
+                  pods with allowedPrivilegedEscalation: true, runAsNonRoot: false,
+                  automountServiceAccountToken: true, and the default capabilities on top of
+                  capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
                   This parameter is deemed insecure but it is needed for certain test-operator
-                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set
-                  of tobiko tests).
+                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain
+                  set of tobiko tests).
                 type: boolean
               projectName:
                 default: horizontest

diff --git a/api/bases/test.openstack.org_tempests.yaml b/api/bases/test.openstack.org_tempests.yaml
@@ -153,11 +153,12 @@ spec:
                 default: false
                 description: |-
                   Use with caution! This parameter specifies whether test-operator should spawn test
-                  pods with allowedPrivilegedEscalation: true and the default capabilities on
-                  top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
+                  pods with allowedPrivilegedEscalation: true, runAsNonRoot: false,
+                  automountServiceAccountToken: true, and the default capabilities on top of
+                  capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
                   This parameter is deemed insecure but it is needed for certain test-operator
-                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set
-                  of tobiko tests).
+                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain
+                  set of tobiko tests).
                 type: boolean
               storageClass:
                 default: local-storage

diff --git a/api/bases/test.openstack.org_tobikoes.yaml b/api/bases/test.openstack.org_tobikoes.yaml
@@ -143,11 +143,12 @@ spec:
                 default: false
                 description: |-
                   Use with caution! This parameter specifies whether test-operator should spawn test
-                  pods with allowedPrivilegedEscalation: true and the default capabilities on
-                  top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
+                  pods with allowedPrivilegedEscalation: true, runAsNonRoot: false,
+                  automountServiceAccountToken: true, and the default capabilities on top of
+                  capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
                   This parameter is deemed insecure but it is needed for certain test-operator
-                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set
-                  of tobiko tests).
+                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain
+                  set of tobiko tests).
                 type: boolean
               publicKey:
                 default: ""

diff --git a/api/v1beta1/common.go b/api/v1beta1/common.go
@@ -46,11 +46,12 @@ type CommonOptions struct {
 	// +kubebuilder:default=false
 	// +optional
 	// Use with caution! This parameter specifies whether test-operator should spawn test
-	// pods with allowedPrivilegedEscalation: true and the default capabilities on
-	// top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
+	// pods with allowedPrivilegedEscalation: true, runAsNonRoot: false,
+	// automountServiceAccountToken: true, and the default capabilities on top of
+	// capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
 	// This parameter is deemed insecure but it is needed for certain test-operator
-	// functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set
-	// of tobiko tests).
+	// functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain
+	// set of tobiko tests).
 	Privileged bool `json:"privileged"`
 
 	// +operator-sdk:csv:customresourcedefinitions:type=spec

diff --git a/api/v1beta1/common_webhook.go b/api/v1beta1/common_webhook.go
@@ -12,9 +12,9 @@ const (
 const (
 	// WarnPrivilegedModeOn
 	WarnPrivilegedModeOn = "%s.Spec.Privileged is set to true. This means that test pods " +
-		"are spawned with allowPrivilegedEscalation: true and default " +
-		"capabilities on top of those required by the test operator " +
-		"(NET_ADMIN, NET_RAW)."
+		"are spawned with allowPrivilegedEscalation: true, runAsNonRoot: false, " +
+		"automountServiceAccountToken: true and default capabilities on top of " +
+		"those required by the test operator (NET_ADMIN, NET_RAW)."
 
 	// WarnPrivilegedModeOff
 	WarnPrivilegedModeOff = "%[1]s.Spec.Privileged is set to false. Note, that a certain " +

diff --git a/config/crd/bases/test.openstack.org_ansibletests.yaml b/config/crd/bases/test.openstack.org_ansibletests.yaml
@@ -146,11 +146,12 @@ spec:
                 default: false
                 description: |-
                   Use with caution! This parameter specifies whether test-operator should spawn test
-                  pods with allowedPrivilegedEscalation: true and the default capabilities on
-                  top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
+                  pods with allowedPrivilegedEscalation: true, runAsNonRoot: false,
+                  automountServiceAccountToken: true, and the default capabilities on top of
+                  capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
                   This parameter is deemed insecure but it is needed for certain test-operator
-                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set
-                  of tobiko tests).
+                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain
+                  set of tobiko tests).
                 type: boolean
               storageClass:
                 default: local-storage

diff --git a/config/crd/bases/test.openstack.org_horizontests.yaml b/config/crd/bases/test.openstack.org_horizontests.yaml
@@ -159,11 +159,12 @@ spec:
                 default: false
                 description: |-
                   Use with caution! This parameter specifies whether test-operator should spawn test
-                  pods with allowedPrivilegedEscalation: true and the default capabilities on
-                  top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
+                  pods with allowedPrivilegedEscalation: true, runAsNonRoot: false,
+                  automountServiceAccountToken: true, and the default capabilities on top of
+                  capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
                   This parameter is deemed insecure but it is needed for certain test-operator
-                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set
-                  of tobiko tests).
+                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain
+                  set of tobiko tests).
                 type: boolean
               projectName:
                 default: horizontest

diff --git a/config/crd/bases/test.openstack.org_tempests.yaml b/config/crd/bases/test.openstack.org_tempests.yaml
@@ -153,11 +153,12 @@ spec:
                 default: false
                 description: |-
                   Use with caution! This parameter specifies whether test-operator should spawn test
-                  pods with allowedPrivilegedEscalation: true and the default capabilities on
-                  top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
+                  pods with allowedPrivilegedEscalation: true, runAsNonRoot: false,
+                  automountServiceAccountToken: true, and the default capabilities on top of
+                  capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
                   This parameter is deemed insecure but it is needed for certain test-operator
-                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set
-                  of tobiko tests).
+                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain
+                  set of tobiko tests).
                 type: boolean
               storageClass:
                 default: local-storage

diff --git a/config/crd/bases/test.openstack.org_tobikoes.yaml b/config/crd/bases/test.openstack.org_tobikoes.yaml
@@ -143,11 +143,12 @@ spec:
                 default: false
                 description: |-
                   Use with caution! This parameter specifies whether test-operator should spawn test
-                  pods with allowedPrivilegedEscalation: true and the default capabilities on
-                  top of capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
+                  pods with allowedPrivilegedEscalation: true, runAsNonRoot: false,
+                  automountServiceAccountToken: true, and the default capabilities on top of
+                  capabilities that are usually needed by the test pods (NET_ADMIN, NET_RAW).
                   This parameter is deemed insecure but it is needed for certain test-operator
-                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain set
-                  of tobiko tests).
+                  functionalities to work properly (e.g.: extraRPMs in Tempest CR, or certain
+                  set of tobiko tests).
                 type: boolean
               publicKey:
                 default: ""

diff --git a/config/manifests/bases/test-operator.clusterserviceversion.yaml b/config/manifests/bases/test-operator.clusterserviceversion.yaml
@@ -89,11 +89,12 @@ spec:
         displayName: Open Stack Config Secret
         path: openStackConfigSecret
       - description: 'Use with caution! This parameter specifies whether test-operator
-          should spawn test pods with allowedPrivilegedEscalation: true and the default
-          capabilities on top of capabilities that are usually needed by the test
-          pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed
-          for certain test-operator functionalities to work properly (e.g.: extraRPMs
-          in Tempest CR, or certain set of tobiko tests).'
+          should spawn test pods with allowedPrivilegedEscalation: true, runAsNonRoot:
+          false, automountServiceAccountToken: true, and the default capabilities
+          on top of capabilities that are usually needed by the test pods (NET_ADMIN,
+          NET_RAW). This parameter is deemed insecure but it is needed for certain
+          test-operator functionalities to work properly (e.g.: extraRPMs in Tempest
+          CR, or certain set of tobiko tests).'
         displayName: Privileged
         path: privileged
       - description: StorageClass used to create any test-operator related PVCs.
@@ -283,11 +284,12 @@ spec:
         displayName: Password
         path: password
       - description: 'Use with caution! This parameter specifies whether test-operator
-          should spawn test pods with allowedPrivilegedEscalation: true and the default
-          capabilities on top of capabilities that are usually needed by the test
-          pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed
-          for certain test-operator functionalities to work properly (e.g.: extraRPMs
-          in Tempest CR, or certain set of tobiko tests).'
+          should spawn test pods with allowedPrivilegedEscalation: true, runAsNonRoot:
+          false, automountServiceAccountToken: true, and the default capabilities
+          on top of capabilities that are usually needed by the test pods (NET_ADMIN,
+          NET_RAW). This parameter is deemed insecure but it is needed for certain
+          test-operator functionalities to work properly (e.g.: extraRPMs in Tempest
+          CR, or certain set of tobiko tests).'
         displayName: Privileged
         path: privileged
       - description: ProjectName is the name of the OpenStack project for Horizon
@@ -380,11 +382,12 @@ spec:
         displayName: Parallel
         path: parallel
       - description: 'Use with caution! This parameter specifies whether test-operator
-          should spawn test pods with allowedPrivilegedEscalation: true and the default
-          capabilities on top of capabilities that are usually needed by the test
-          pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed
-          for certain test-operator functionalities to work properly (e.g.: extraRPMs
-          in Tempest CR, or certain set of tobiko tests).'
+          should spawn test pods with allowedPrivilegedEscalation: true, runAsNonRoot:
+          false, automountServiceAccountToken: true, and the default capabilities
+          on top of capabilities that are usually needed by the test pods (NET_ADMIN,
+          NET_RAW). This parameter is deemed insecure but it is needed for certain
+          test-operator functionalities to work properly (e.g.: extraRPMs in Tempest
+          CR, or certain set of tobiko tests).'
         displayName: Privileged
         path: privileged
       - description: StorageClass used to create any test-operator related PVCs.
@@ -398,6 +401,11 @@ spec:
       - description: A content of exclude.txt file that is passed to tempest via --exclude-list
         displayName: Exclude List
         path: tempestRun.excludeList
+      - description: The expectedFailuresList parameter contains tests that should
+          not count as failures. When a test from this list fails, the test pod ends
+          with Completed state rather than with Error state.
+        displayName: Expected Failures List
+        path: tempestRun.expectedFailuresList
       - description: ExternalPlugin contains information about plugin that should
           be installed within the tempest test pod. If this option is specified then
           only tests that are part of the external plugin can be executed.
@@ -662,6 +670,11 @@ spec:
       - description: A content of exclude.txt file that is passed to tempest via --exclude-list
         displayName: Exclude List
         path: workflow[0].tempestRun.excludeList
+      - description: The expectedFailuresList parameter contains tests that should
+          not count as failures. When a test from this list fails, the test pod ends
+          with Completed state rather than with Error state.
+        displayName: Expected Failures List
+        path: workflow[0].tempestRun.expectedFailuresList
       - description: ExternalPlugin contains information about plugin that should
           be installed within the tempest test pod. If this option is specified then
           only tests that are part of the external plugin can be executed.
@@ -904,11 +917,12 @@ spec:
         displayName: Private Key
         path: privateKey
       - description: 'Use with caution! This parameter specifies whether test-operator
-          should spawn test pods with allowedPrivilegedEscalation: true and the default
-          capabilities on top of capabilities that are usually needed by the test
-          pods (NET_ADMIN, NET_RAW). This parameter is deemed insecure but it is needed
-          for certain test-operator functionalities to work properly (e.g.: extraRPMs
-          in Tempest CR, or certain set of tobiko tests).'
+          should spawn test pods with allowedPrivilegedEscalation: true, runAsNonRoot:
+          false, automountServiceAccountToken: true, and the default capabilities
+          on top of capabilities that are usually needed by the test pods (NET_ADMIN,
+          NET_RAW). This parameter is deemed insecure but it is needed for certain
+          test-operator functionalities to work properly (e.g.: extraRPMs in Tempest
+          CR, or certain set of tobiko tests).'
         displayName: Privileged
         path: privileged
       - description: Public Key

diff --git a/pkg/ansibletest/job.go b/pkg/ansibletest/job.go
@@ -47,8 +47,9 @@ func Job(
 					Labels: labels,
 				},
 				Spec: corev1.PodSpec{
-					RestartPolicy:      corev1.RestartPolicyNever,
-					ServiceAccountName: instance.RbacResourceName(),
+					AutomountServiceAccountToken: &instance.Spec.Privileged,
+					RestartPolicy:                corev1.RestartPolicyNever,
+					ServiceAccountName:           instance.RbacResourceName(),
 					SecurityContext: &corev1.PodSecurityContext{
 						RunAsUser:  &runAsUser,
 						RunAsGroup: &runAsGroup,

diff --git a/pkg/horizontest/job.go b/pkg/horizontest/job.go
@@ -46,8 +46,9 @@ func Job(
 					Labels: labels,
 				},
 				Spec: corev1.PodSpec{
-					RestartPolicy:      corev1.RestartPolicyNever,
-					ServiceAccountName: instance.RbacResourceName(),
+					AutomountServiceAccountToken: &instance.Spec.Privileged,
+					RestartPolicy:                corev1.RestartPolicyNever,
+					ServiceAccountName:           instance.RbacResourceName(),
 					SecurityContext: &corev1.PodSecurityContext{
 						RunAsUser:  &runAsUser,
 						RunAsGroup: &runAsGroup,

diff --git a/pkg/tempest/job.go b/pkg/tempest/job.go
@@ -43,8 +43,9 @@ func Job(
 					Labels:      labels,
 				},
 				Spec: corev1.PodSpec{
-					RestartPolicy:      corev1.RestartPolicyNever,
-					ServiceAccountName: instance.RbacResourceName(),
+					AutomountServiceAccountToken: &instance.Spec.Privileged,
+					RestartPolicy:                corev1.RestartPolicyNever,
+					ServiceAccountName:           instance.RbacResourceName(),
 					SecurityContext: &corev1.PodSecurityContext{
 						RunAsUser:  &runAsUser,
 						RunAsGroup: &runAsGroup,

diff --git a/pkg/tobiko/job.go b/pkg/tobiko/job.go
@@ -51,8 +51,9 @@ func Job(
 					Labels:      labels,
 				},
 				Spec: corev1.PodSpec{
-					RestartPolicy:      corev1.RestartPolicyNever,
-					ServiceAccountName: instance.RbacResourceName(),
+					AutomountServiceAccountToken: &instance.Spec.Privileged,
+					RestartPolicy:                corev1.RestartPolicyNever,
+					ServiceAccountName:           instance.RbacResourceName(),
 					SecurityContext: &corev1.PodSecurityContext{
 						RunAsUser:  &runAsUser,
 						RunAsGroup: &runAsGroup,

diff --git a/pkg/util/common.go b/pkg/util/common.go
@@ -22,6 +22,7 @@ func GetSecurityContext(
 	securityContext := corev1.SecurityContext{
 		RunAsUser:                &runAsUser,
 		RunAsGroup:               &runAsUser,
+		RunAsNonRoot:             &trueVar,
 		AllowPrivilegeEscalation: &falseVar,
 		Capabilities:             &corev1.Capabilities{},
 		SeccompProfile: &corev1.SeccompProfile{
@@ -30,6 +31,11 @@ func GetSecurityContext(
 	}
 
 	if privileged {
+		// Sometimes we require the test pods run sudo to be able to install
+		// additional packages or run commands with elevated priveleges (e.g.,
+		// tcpdump in case of Tobiko)
+		securityContext.RunAsNonRoot = &falseVar
+
 		// We need to run pods with AllowPrivilegedEscalation: true to remove
 		// nosuid from the pod (in order to be able to run sudo)
 		securityContext.AllowPrivilegeEscalation = &trueVar