Merge pull request #184 from lpiwowar/fix/no-new-priv

Disable AllowPrivilegeEscalation

api/bases/test.openstack.org_ansibletests.yaml

@@ -125,6 +125,16 @@ spec:
                 description: OpenStackConfigSecret is the name of the Secret containing
                   the secure.yaml
                 type: string
+              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).'
+                type: boolean
               storageClass:
                 default: local-storage
                 description: StorageClass used to create PVCs that store the logs
@@ -203,6 +213,15 @@ spec:
                       description: OpenStackConfigSecret is the name of the Secret
                         containing the secure.yaml
                       type: string
+                    privileged:
+                      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).'
+                      type: boolean
                     stepName:
                       description: Name of a workflow step. The step name will be
                         used for example to create a logs directory.

api/bases/test.openstack.org_horizontests.yaml

@@ -106,6 +106,16 @@ spec:
                 description: Password is the password for the user running the Horizon
                   tests.
                 type: string
+              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).'
+                type: boolean
               projectName:
                 default: horizontest
                 description: ProjectName is the name of the OpenStack project for

api/bases/test.openstack.org_tempests.yaml

@@ -133,6 +133,16 @@ spec:
                   if multiple instances of test-operator related CRs exist. If you
                   want to turn off this behaviour then set this option to true.
                 type: boolean
+              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).'
+                type: boolean
               storageClass:
                 default: local-storage
                 description: Name of a storage class that is used to create PVCs for

api/bases/test.openstack.org_tobikoes.yaml

@@ -102,6 +102,16 @@ spec:
                 default: ""
                 description: Private Key
                 type: string
+              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).'
+                type: boolean
               publicKey:
                 default: ""
                 description: Public Key
@@ -208,6 +218,15 @@ spec:
                     privateKey:
                       description: Private Key
                       type: string
+                    privileged:
+                      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).'
+                      type: boolean
                     publicKey:
                       description: Public Key
                       type: string

api/v1beta1/ansibletest_types.go

@@ -26,6 +26,8 @@ import (
 
 // AnsibleTestSpec defines the desired state of AnsibleTest
 type AnsibleTestSpec struct {
+	CommonParameters `json:",inline"`
+
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// +kubebuilder:validation:Optional
 	// Extra configmaps for mounting in the pod.
@@ -126,6 +128,8 @@ type AnsibleTestSpec struct {
 }
 
 type AnsibleTestWorkflowSpec struct {
+	WorkflowCommonParameters `json:",inline"`
+
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// +kubebuilder:validation:Optional
 	// Extra configmaps for mounting in the pod

api/v1beta1/ansibletest_webhook.go

@@ -23,6 +23,8 @@ limitations under the License.
 package v1beta1
 
 import (
+	"fmt"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -59,8 +61,13 @@ var _ webhook.Validator = &AnsibleTest{}
 func (r *AnsibleTest) ValidateCreate() (admission.Warnings, error) {
 	ansibletestlog.Info("validate create", "name", r.Name)
 
-	// TODO(user): fill in your validation logic upon object creation.
-	return nil, nil
+	var allWarnings admission.Warnings
+
+	if r.Spec.Privileged {
+		allWarnings = append(allWarnings, fmt.Sprintf(WarnPrivilegedModeOn, "AnsibleTest"))
+	}
+
+	return allWarnings, nil
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type

api/v1beta1/common.go

@@ -16,6 +16,33 @@ limitations under the License.
 
 package v1beta1
 
+type CommonParameters struct {
+	// +operator-sdk:csv:customresourcedefinitions:type=spec
+	// +kubebuilder:validation:optional
+	// +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).
+	// 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"`
+}
+
+type WorkflowCommonParameters struct {
+	// +operator-sdk:csv:customresourcedefinitions:type=spec
+	// +kubebuilder:validation:optional
+	// +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).
+	// 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,omitempty"`
+}
+
 type extraConfigmapsMounts struct {
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// +kubebuilder:validation:Required

api/v1beta1/common_webhook.go

@@ -0,0 +1,21 @@
+package v1beta1
+
+const (
+	// ErrPrivilegedModeRequired
+	ErrPrivilegedModeRequired = "%s.Spec.Privileged is requied in order to successfully " +
+	                            "execute tests with the provided configuration."
+)
+
+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)."
+
+	// WarnPrivilegedModeOff
+	WarnPrivilegedModeOff = "%[1]s.Spec.Privileged is set to false. Note, that a certain " +
+	                        "set of tests might fail, as this configuration may be " +
+													"required for the tests to run successfully. Before enabling" +
+													"this parameter, consult documentation of the %[1]s CR."
+)

api/v1beta1/horizontest_types.go

@@ -26,6 +26,8 @@ import (
 
 // HorizonTestSpec defines the desired state of HorizonTest
 type HorizonTestSpec struct {
+	CommonParameters `json:",inline"`
+
 	// +kubebuilder:validation:Required
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// +kubebuilder:default:="local-storage"
@@ -146,7 +148,6 @@ type HorizonTestStatus struct {
 
 	// NetworkAttachments status of the deployment pods
 	NetworkAttachments map[string][]string `json:"networkAttachments,omitempty"`
-
 }
 
 // +kubebuilder:object:root=true

api/v1beta1/horizontest_webhook.go

@@ -23,6 +23,8 @@ limitations under the License.
 package v1beta1
 
 import (
+	"fmt"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -59,8 +61,13 @@ var _ webhook.Validator = &HorizonTest{}
 func (r *HorizonTest) ValidateCreate() (admission.Warnings, error) {
 	horizontestlog.Info("validate create", "name", r.Name)
 
-	// TODO(user): fill in your validation logic upon object creation.
-	return nil, nil
+	var allWarnings admission.Warnings
+
+	if r.Spec.Privileged {
+		allWarnings = append(allWarnings, fmt.Sprintf(WarnPrivilegedModeOn, "HorizonTest"))
+	}
+
+	return allWarnings, nil
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type

api/v1beta1/tempest_types.go

@@ -377,6 +377,8 @@ type TempestconfRunSpec struct {
 // TempestSpec - configuration of execution of tempest. For specific configuration
 // of tempest see TempestRunSpec and for discover-tempest-config see TempestconfRunSpec.
 type TempestSpec struct {
+	CommonParameters `json:",inline"`
+
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// +kubebuilder:validation:Optional
 	// Extra configmaps for mounting in the pod.

api/v1beta1/tempest_types_workflow.go

@@ -261,7 +261,6 @@ type WorkflowTempestSpec struct {
 	// test pods that are spawned by the test-operator.
 	Tolerations *[]corev1.Toleration `json:"tolerations,omitempty"`
 
-
 	// +kubebuilder:validation:Optional
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// OpenStackConfigMap is the name of the ConfigMap containing the clouds.yaml

api/v1beta1/tempest_webhook.go

@@ -23,15 +23,18 @@ limitations under the License.
 package v1beta1
 
 import (
-        "errors"
+	"errors"
+	"fmt"
 
 	"github.com/google/go-cmp/cmp"
-
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
-        "sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
 // TempestDefaults -
@@ -58,12 +61,12 @@ var _ webhook.Defaulter = &Tempest{}
 func (r *Tempest) Default() {
 	tempestlog.Info("default", "name", r.Name)
 
-        r.Spec.Default()
+	r.Spec.Default()
 }
 
 // Default - set defaults for this Tempest spec.
 func (spec *TempestSpec) Default() {
-        if spec.ContainerImage == "" {
+	if spec.ContainerImage == "" {
 		spec.ContainerImage = tempestDefaults.ContainerImageURL
 	}
 
@@ -72,6 +75,10 @@ func (spec *TempestSpec) Default() {
 	}
 }
 
+func (r *Tempest) PrivilegedRequired() bool {
+	return len(r.Spec.TempestRun.ExtraImages) > 0
+}
+
 // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
 //+kubebuilder:webhook:path=/validate-test-openstack-org-v1beta1-tempest,mutating=false,failurePolicy=fail,sideEffects=None,groups=test.openstack.org,resources=tempests,verbs=create;update,versions=v1beta1,name=vtempest.kb.io,admissionReviewVersions=v1
 
@@ -81,10 +88,35 @@ var _ webhook.Validator = &Tempest{}
 func (r *Tempest) ValidateCreate() (admission.Warnings, error) {
 	tempestlog.Info("validate create", "name", r.Name)
 
-        if len(r.Spec.Workflow) > 0 && r.Spec.Debug {
-            return nil, errors.New("Workflow variable must be empty to run debug mode!")
-        }
-	return nil, nil
+	var allErrs field.ErrorList
+	var allWarnings admission.Warnings
+
+	if len(r.Spec.Workflow) > 0 && r.Spec.Debug {
+		return nil, errors.New("Workflow variable must be empty to run debug mode!")
+	}
+
+	if !r.Spec.Privileged && r.PrivilegedRequired() {
+		allErrs = append(allErrs, &field.Error{
+			Type:     field.ErrorTypeRequired,
+			BadValue: r.Spec.Privileged,
+			Detail:   fmt.Sprintf(ErrPrivilegedModeRequired, "Tempest"),
+		},
+		)
+	}
+
+	if r.Spec.Privileged {
+		allWarnings = append(allWarnings, fmt.Sprintf(WarnPrivilegedModeOn, "Tempest"))
+	}
+
+	if len(allErrs) > 0 {
+		return allWarnings, apierrors.NewInvalid(
+			schema.GroupKind{
+				Group: GroupVersion.WithKind("Tempest").Group,
+				Kind:  GroupVersion.WithKind("Tempest").Kind,
+			}, r.GetName(), allErrs)
+	}
+
+	return allWarnings, nil
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
@@ -98,8 +130,8 @@ func (r *Tempest) ValidateUpdate(old runtime.Object) (admission.Warnings, error)
 
 	if !cmp.Equal(oldTempest.Spec, r.Spec) {
 		warnings := admission.Warnings{}
-		warnings = append(warnings, "You are updating an already existing instance of a " +
-					    "Tempest CR! Be aware that changes won't be applied.")
+		warnings = append(warnings, "You are updating an already existing instance of a "+
+			"Tempest CR! Be aware that changes won't be applied.")
 
 		return warnings, errors.New("Updating an existing Tempest CR is not supported!")
 	}