From bdf97df32d59e6797b80714d664c04a3b3140720 Mon Sep 17 00:00:00 2001 From: Lukas Piwowarski Date: Fri, 6 Dec 2024 11:04:59 -0500 Subject: [PATCH 1/2] 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. --- .../test.openstack.org_ansibletests.yaml | 13 +++--- .../test.openstack.org_horizontests.yaml | 13 +++--- api/bases/test.openstack.org_tempests.yaml | 13 +++--- api/bases/test.openstack.org_tobikoes.yaml | 13 +++--- api/v1beta1/common.go | 13 +++--- api/v1beta1/common_webhook.go | 6 +-- .../test.openstack.org_ansibletests.yaml | 13 +++--- .../test.openstack.org_horizontests.yaml | 13 +++--- .../bases/test.openstack.org_tempests.yaml | 13 +++--- .../bases/test.openstack.org_tobikoes.yaml | 13 +++--- .../test-operator.clusterserviceversion.yaml | 44 ++++++++++--------- pkg/ansibletest/job.go | 5 ++- pkg/horizontest/job.go | 5 ++- pkg/tempest/job.go | 5 ++- pkg/tobiko/job.go | 5 ++- pkg/util/common.go | 6 +++ 16 files changed, 108 insertions(+), 85 deletions(-) diff --git a/api/bases/test.openstack.org_ansibletests.yaml b/api/bases/test.openstack.org_ansibletests.yaml index 5a177047..e13f9b0f 100644 --- a/api/bases/test.openstack.org_ansibletests.yaml +++ b/api/bases/test.openstack.org_ansibletests.yaml @@ -145,12 +145,13 @@ spec: privileged: default: false description: |- - Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false 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). + Use with caution! This parameter specifies whether test-operator should spawn + test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, + 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 index 243a1bf6..b80a1af6 100644 --- a/api/bases/test.openstack.org_horizontests.yaml +++ b/api/bases/test.openstack.org_horizontests.yaml @@ -158,12 +158,13 @@ spec: privileged: default: false description: |- - Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false 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). + Use with caution! This parameter specifies whether test-operator should spawn + test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, + 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 index 81f54d1c..b61d49ee 100644 --- a/api/bases/test.openstack.org_tempests.yaml +++ b/api/bases/test.openstack.org_tempests.yaml @@ -152,12 +152,13 @@ spec: privileged: default: false description: |- - Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false 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). + Use with caution! This parameter specifies whether test-operator should spawn + test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, + 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 index 6b44c6e7..19679d6b 100644 --- a/api/bases/test.openstack.org_tobikoes.yaml +++ b/api/bases/test.openstack.org_tobikoes.yaml @@ -142,12 +142,13 @@ spec: privileged: default: false description: |- - Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false 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). + Use with caution! This parameter specifies whether test-operator should spawn + test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, + 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 index cd93ae5e..0eeb5e39 100644 --- a/api/v1beta1/common.go +++ b/api/v1beta1/common.go @@ -45,12 +45,13 @@ type CommonOptions struct { // +kubebuilder:validation:optional // +kubebuilder:default=false // +optional - // Use with caution! This parameter specifies whether test-operator should spawn test - // pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false 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). + // Use with caution! This parameter specifies whether test-operator should spawn + // test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, + // 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). Privileged bool `json:"privileged"` // +operator-sdk:csv:customresourcedefinitions:type=spec diff --git a/api/v1beta1/common_webhook.go b/api/v1beta1/common_webhook.go index 4c3f3280..7dd95bb0 100644 --- 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, readOnlyRootFilesystem: false " + - "and default capabilities on top of those required by the test operator " + - "(NET_ADMIN, NET_RAW)." + "are spawned with allowPrivilegedEscalation: true, readOnlyRootFilesystem: false, " + + "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 index 5a177047..e13f9b0f 100644 --- a/config/crd/bases/test.openstack.org_ansibletests.yaml +++ b/config/crd/bases/test.openstack.org_ansibletests.yaml @@ -145,12 +145,13 @@ spec: privileged: default: false description: |- - Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false 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). + Use with caution! This parameter specifies whether test-operator should spawn + test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, + 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 index 243a1bf6..b80a1af6 100644 --- a/config/crd/bases/test.openstack.org_horizontests.yaml +++ b/config/crd/bases/test.openstack.org_horizontests.yaml @@ -158,12 +158,13 @@ spec: privileged: default: false description: |- - Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false 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). + Use with caution! This parameter specifies whether test-operator should spawn + test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, + 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 index 81f54d1c..b61d49ee 100644 --- a/config/crd/bases/test.openstack.org_tempests.yaml +++ b/config/crd/bases/test.openstack.org_tempests.yaml @@ -152,12 +152,13 @@ spec: privileged: default: false description: |- - Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false 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). + Use with caution! This parameter specifies whether test-operator should spawn + test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, + 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 index 6b44c6e7..19679d6b 100644 --- a/config/crd/bases/test.openstack.org_tobikoes.yaml +++ b/config/crd/bases/test.openstack.org_tobikoes.yaml @@ -142,12 +142,13 @@ spec: privileged: default: false description: |- - Use with caution! This parameter specifies whether test-operator should spawn test - pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false 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). + Use with caution! This parameter specifies whether test-operator should spawn + test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, + 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 index 0ba88774..551aead9 100644 --- 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, readOnlyRootFilesystem: + false, 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, readOnlyRootFilesystem: + false, 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, readOnlyRootFilesystem: + false, 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. @@ -904,11 +907,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, readOnlyRootFilesystem: + false, 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 index b88e9aab..a62a299a 100644 --- 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 index 5a9123d5..1ec60581 100644 --- 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 index 6e73a112..055db269 100644 --- 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 index 21c2f971..2d39ff64 100644 --- 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 index 81a860b7..ceb7656e 100644 --- a/pkg/util/common.go +++ b/pkg/util/common.go @@ -29,6 +29,7 @@ func GetSecurityContext( RunAsUser: &runAsUser, RunAsGroup: &runAsUser, ReadOnlyRootFilesystem: &trueVar, + RunAsNonRoot: &trueVar, AllowPrivilegeEscalation: &falseVar, Capabilities: &corev1.Capabilities{}, SeccompProfile: &corev1.SeccompProfile{ @@ -37,6 +38,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 From 0a23722800e57466ff21ac542b806331863a7afc Mon Sep 17 00:00:00 2001 From: Lukas Piwowarski Date: Fri, 10 Jan 2025 04:57:20 -0500 Subject: [PATCH 2/2] Add expectedFailuresList to CSV The epectedFailuresList parameter was added as part of this PR [1] but unfortunatel it was not added to the CSV. This commit fixes that. [1] https://github.com/openstack-k8s-operators/test-operator/pull/233 --- .../bases/test-operator.clusterserviceversion.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/config/manifests/bases/test-operator.clusterserviceversion.yaml b/config/manifests/bases/test-operator.clusterserviceversion.yaml index 551aead9..3f3f9c3c 100644 --- a/config/manifests/bases/test-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/test-operator.clusterserviceversion.yaml @@ -401,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. @@ -665,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.