Disable insecure parameters by default

This patch sets by default: - 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 to into the test pods. This would allow them to modify the state of the OCP cluster. As of now this is not required by any of the test pods. This behavior can be overriden by setting privileged: true.
openstack-k8s-operators · Dec 9, 2024 · b20e5cb · b20e5cb
1 parent da180e1
commit b20e5cb
Show file tree

Hide file tree

Showing 15 changed files with 105 additions and 80 deletions.
diff --git a/api/bases/test.openstack.org_ansibletests.yaml b/api/bases/test.openstack.org_ansibletests.yaml
@@ -139,12 +139,12 @@ spec:
               privileged:
                 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). 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).'
                 type: boolean
               storageClass:
                 default: local-storage

diff --git a/api/bases/test.openstack.org_horizontests.yaml b/api/bases/test.openstack.org_horizontests.yaml
@@ -151,12 +151,12 @@ spec:
               privileged:
                 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). 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).'
                 type: boolean
               projectName:
                 default: horizontest

diff --git a/api/bases/test.openstack.org_tempests.yaml b/api/bases/test.openstack.org_tempests.yaml
@@ -142,12 +142,12 @@ spec:
               privileged:
                 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). 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).'
                 type: boolean
               storageClass:
                 default: local-storage

diff --git a/api/bases/test.openstack.org_tobikoes.yaml b/api/bases/test.openstack.org_tobikoes.yaml
@@ -133,12 +133,12 @@ spec:
               privileged:
                 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). 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).'
                 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/config/crd/bases/test.openstack.org_ansibletests.yaml b/config/crd/bases/test.openstack.org_ansibletests.yaml
@@ -139,12 +139,12 @@ spec:
               privileged:
                 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). 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).'
                 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
@@ -151,12 +151,12 @@ spec:
               privileged:
                 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). 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).'
                 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
@@ -142,12 +142,12 @@ spec:
               privileged:
                 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). 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).'
                 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
@@ -133,12 +133,12 @@ spec:
               privileged:
                 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). 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).'
                 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