[main/2.10.2] Add resource request and limit validation when creating…

… a namespace (#550)

* Add resource request and limit validation when creating a namespace

* Update test for number of namespace admitters

* cleaning up lint errors

* Allow for empty resource limit annotation to be present

* Update to allow for partial request/limits

docs.md

@@ -60,6 +60,10 @@ The following labels are considered relevant for PSA enforcement:
 - pod-security.kubernetes.io/warn
 - pod-security.kubernetes.io/warn-version
 
+#### Namespace resource limit validation
+
+Validation ensures that the limits for cpu/memory must not be less than the requests for cpu/memory.
+
 ## Secret
 
 ### Validation Checks

pkg/resources/core/v1/namespace/Namespace.md

@@ -20,3 +20,6 @@ The following labels are considered relevant for PSA enforcement:
 - pod-security.kubernetes.io/warn
 - pod-security.kubernetes.io/warn-version
 
+### Namespace resource limit validation
+
+Validation ensures that the limits for cpu/memory must not be less than the requests for cpu/memory.

pkg/resources/core/v1/namespace/requestlimits.go

@@ -0,0 +1,123 @@
+package namespace
+
+import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/rancher/webhook/pkg/admission"
+	objectsv1 "github.com/rancher/webhook/pkg/generated/objects/core/v1"
+	admissionv1 "k8s.io/api/admission/v1"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/utils/trace"
+)
+
+const resourceLimitAnnotation = "field.cattle.io/containerDefaultResourceLimit"
+
+type requestLimitAdmitter struct{}
+
+// Admit ensures that the resource requests are within the limits.
+func (r *requestLimitAdmitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) {
+	listTrace := trace.New("Namespace Admit", trace.Field{Key: "user", Value: request.UserInfo.Username})
+	defer listTrace.LogIfLong(admission.SlowTraceDuration)
+
+	switch request.Operation {
+	case admissionv1.Create:
+		ns, err := objectsv1.NamespaceFromRequest(&request.AdmissionRequest)
+		if err != nil {
+			return nil, fmt.Errorf("failed to decode namespace from request: %w", err)
+		}
+		return r.admitCommonCreateUpdate(nil, ns)
+	case admissionv1.Update:
+		oldns, ns, err := objectsv1.NamespaceOldAndNewFromRequest(&request.AdmissionRequest)
+		if err != nil {
+			return nil, fmt.Errorf("failed to decode namespace from request: %w", err)
+		}
+		return r.admitCommonCreateUpdate(oldns, ns)
+	}
+	return admission.ResponseAllowed(), nil
+}
+
+type ResourceLimits struct {
+	LimitsCPU      string `json:"limitsCpu"`
+	LimitsMemory   string `json:"limitsMemory"`
+	RequestsCPU    string `json:"requestsCpu"`
+	RequestsMemory string `json:"requestsMemory"`
+}
+
+// admitCommonCreateUpdate will extract the annotation values that contain the resource limits and will call
+// the validateResourceLimitsWithUnits function to determine whether or not the request is valid.
+func (r *requestLimitAdmitter) admitCommonCreateUpdate(_, newNamespace *v1.Namespace) (*admissionv1.AdmissionResponse, error) {
+	annotations := newNamespace.Annotations
+	if annotations == nil {
+		return admission.ResponseAllowed(), nil
+	}
+
+	resourceLimitJSON, exists := annotations[resourceLimitAnnotation]
+	if !exists || resourceLimitJSON == "{}" {
+		return admission.ResponseAllowed(), nil
+	}
+
+	var resourceLimits ResourceLimits
+	if err := json.Unmarshal([]byte(resourceLimitJSON), &resourceLimits); err != nil {
+		return admission.ResponseBadRequest(fmt.Sprintf("invalid resource limits annotation: %v", err)), nil
+	}
+
+	if err := validateResourceLimitsWithUnits(resourceLimits); err != nil {
+		return admission.ResponseBadRequest(err.Error()), nil
+	}
+
+	return admission.ResponseAllowed(), nil
+}
+
+// validateResourceLimitsWithUnits takes a set of CPU/memory requests/limits and validates them.
+// It parses all provided values. If both a request and a limit exist for CPU or memory, it ensures
+// that the request is not greater than the limit. Missing values are parsed but ignored in comparison.
+func validateResourceLimitsWithUnits(limits ResourceLimits) error {
+	var requestsCPU, limitsCPU resource.Quantity
+	var err error
+	if limits.RequestsCPU != "" {
+		requestsCPU, err = resource.ParseQuantity(limits.RequestsCPU)
+		if err != nil {
+			return fmt.Errorf("invalid requestsCpu value: %v", err)
+		}
+	}
+
+	if limits.LimitsCPU != "" {
+		limitsCPU, err = resource.ParseQuantity(limits.LimitsCPU)
+		if err != nil {
+			return fmt.Errorf("invalid limitsCpu value: %v", err)
+		}
+	}
+
+	// Compare CPU requests and limits if both are provided
+	if limits.RequestsCPU != "" && limits.LimitsCPU != "" {
+		if requestsCPU.Cmp(limitsCPU) > 0 {
+			return fmt.Errorf("requestsCpu (%s) cannot be greater than limitsCpu (%s)", requestsCPU.String(), limitsCPU.String())
+		}
+	}
+
+	var requestsMemory, limitsMemory resource.Quantity
+	if limits.RequestsMemory != "" {
+		requestsMemory, err = resource.ParseQuantity(limits.RequestsMemory)
+		if err != nil {
+			return fmt.Errorf("invalid requestsMemory value: %v", err)
+		}
+	}
+
+	if limits.LimitsMemory != "" {
+		limitsMemory, err = resource.ParseQuantity(limits.LimitsMemory)
+		if err != nil {
+			return fmt.Errorf("invalid limitsMemory value: %v", err)
+		}
+	}
+
+	// Compare memory requests and limits if both are provided
+	if limits.RequestsMemory != "" && limits.LimitsMemory != "" {
+		if requestsMemory.Cmp(limitsMemory) > 0 {
+			return fmt.Errorf("requestsMemory (%s) cannot be greater than limitsMemory (%s)", requestsMemory.String(), limitsMemory.String())
+		}
+	}
+
+	return nil
+}

pkg/resources/core/v1/namespace/requestlimits_test.go

@@ -0,0 +1,161 @@
+package namespace
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	"github.com/rancher/webhook/pkg/admission"
+	"github.com/stretchr/testify/assert"
+	v1 "k8s.io/api/admission/v1"
+	authenticationv1 "k8s.io/api/authentication/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+const testNs = "test-ns"
+
+func TestRequestLimitAdmitter(t *testing.T) {
+	tests := []struct {
+		name             string
+		operationType    v1.Operation
+		limitsAnnotation string
+		wantAllowed      bool
+	}{
+		{
+			name:             "create ns within resource limits",
+			operationType:    v1.Create,
+			limitsAnnotation: `{"limitsCpu": "500m", "requestsCpu": "100m", "limitsMemory": "128Mi", "requestsMemory": "64Mi"}`,
+			wantAllowed:      true,
+		},
+		{
+			name:             "create ns exceeds resource limits",
+			operationType:    v1.Create,
+			limitsAnnotation: `{"limitsCpu": "200m", "limitsMemory": "256Mi", "requestsCpu": "1", "requestsMemory": "1Gi"}`,
+			wantAllowed:      false,
+		},
+		{
+			name:             "create ns invalid JSON in annotation",
+			operationType:    v1.Create,
+			limitsAnnotation: `invalid-json`,
+			wantAllowed:      false,
+		},
+		{
+			name:             "create ns no request annotation",
+			operationType:    v1.Create,
+			limitsAnnotation: "",
+			wantAllowed:      true,
+		},
+		{
+			name:             "update ns within resource limits",
+			operationType:    v1.Update,
+			limitsAnnotation: `{"limitsCpu": "500m", "requestsCpu": "100m", "limitsMemory": "128Mi", "requestsMemory": "64Mi"}`,
+			wantAllowed:      true,
+		},
+		{
+			name:             "update ns exceeds resource limits",
+			operationType:    v1.Update,
+			limitsAnnotation: `{"limitsCpu": "200m", "limitsMemory": "256Mi", "requestsCpu": "1", "requestsMemory": "1Gi"}`,
+			wantAllowed:      false,
+		},
+		{
+			name:             "create ns within only cpu req and limit",
+			operationType:    v1.Create,
+			limitsAnnotation: `{"limitsCpu": "500m", "requestsCpu": "100m"}`,
+			wantAllowed:      true,
+		},
+		{
+			name:             "create ns within invalid cpu req and limit",
+			operationType:    v1.Create,
+			limitsAnnotation: `{"limitsCpu": "100m", "requestsCpu": "500m"}`,
+			wantAllowed:      false,
+		},
+		{
+			name:             "update ns within only memory resource req and limit",
+			operationType:    v1.Update,
+			limitsAnnotation: `{"limitsCpu": "500m", "requestsCpu": "100m"}`,
+			wantAllowed:      true,
+		},
+		{
+			name:             "create ns within only memory req and cpu limit",
+			operationType:    v1.Create,
+			limitsAnnotation: `{"limitsMemory": "256Mi", "limitssCpu": "100m"}`,
+			wantAllowed:      true,
+		},
+		{
+			name:             "create ns within empty resource limits",
+			operationType:    v1.Create,
+			limitsAnnotation: `{}`,
+			wantAllowed:      true,
+		},
+		{
+			name:             "create ns within invalid cpu limits",
+			operationType:    v1.Create,
+			limitsAnnotation: `{"limitsCpu": "25invalid"}`,
+			wantAllowed:      false,
+		},
+		{
+			name:             "update ns within invalid memory limits",
+			operationType:    v1.Update,
+			limitsAnnotation: `{"limitsMemory": "77foobar"}`,
+			wantAllowed:      false,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			admitter := requestLimitAdmitter{}
+			request, err := createRequestLimitRequest(test.limitsAnnotation, test.operationType)
+			if test.operationType == v1.Update {
+				request.AdmissionRequest.OldObject.Raw, err = json.Marshal(corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: testNs,
+					},
+				})
+			}
+			assert.NoError(t, err)
+			response, err := admitter.Admit(request)
+			assert.NoError(t, err)
+			assert.Equal(t, test.wantAllowed, response.Allowed)
+		})
+	}
+}
+
+func createRequestLimitRequest(limitsAnnotation string, operation v1.Operation) (*admission.Request, error) {
+	gvk := metav1.GroupVersionKind{Version: "v1", Kind: "Namespace"}
+	gvr := metav1.GroupVersionResource{Version: "v1", Resource: "namespace"}
+
+	ns := corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: testNs,
+		},
+	}
+	if limitsAnnotation != "" {
+		ns.Annotations = map[string]string{
+			resourceLimitAnnotation: limitsAnnotation,
+		}
+	}
+
+	req := &admission.Request{
+		AdmissionRequest: v1.AdmissionRequest{
+			UID:             "",
+			Kind:            gvk,
+			Resource:        gvr,
+			RequestKind:     &gvk,
+			RequestResource: &gvr,
+			Name:            ns.Name,
+			Operation:       operation,
+			UserInfo:        authenticationv1.UserInfo{Username: "test-user", UID: ""},
+		},
+		Context: context.Background(),
+	}
+
+	var err error
+	req.Object.Raw, err = json.Marshal(ns)
+	if err != nil {
+		return nil, err
+	}
+	return req, nil
+}

pkg/resources/core/v1/namespace/validator.go

@@ -18,8 +18,9 @@ var projectsGVR = schema.GroupVersionResource{
 
 // Validator validates the namespace admission request.
 type Validator struct {
-	psaAdmitter              psaLabelAdmitter
-	projectNamespaceAdmitter projectNamespaceAdmitter
+	psaAdmitter                psaLabelAdmitter
+	projectNamespaceAdmitter   projectNamespaceAdmitter
+	requestWithinLimitAdmitter requestLimitAdmitter
 }
 
 // NewValidator returns a new validator used for validation of namespace requests.
@@ -31,6 +32,7 @@ func NewValidator(sar authorizationv1.SubjectAccessReviewInterface) *Validator {
 		projectNamespaceAdmitter: projectNamespaceAdmitter{
 			sar: sar,
 		},
+		requestWithinLimitAdmitter: requestLimitAdmitter{},
 	}
 }
 
@@ -90,5 +92,5 @@ func (v *Validator) ValidatingWebhook(clientConfig admissionv1.WebhookClientConf
 
 // Admitters returns the psaAdmitter and the projectNamespaceAdmitter for namespaces.
 func (v *Validator) Admitters() []admission.Admitter {
-	return []admission.Admitter{&v.psaAdmitter, &v.projectNamespaceAdmitter}
+	return []admission.Admitter{&v.psaAdmitter, &v.projectNamespaceAdmitter, &v.requestWithinLimitAdmitter}
 }

pkg/resources/core/v1/namespace/validator_test.go

@@ -28,7 +28,7 @@ func TestOperations(t *testing.T) {
 func TestAdmitters(t *testing.T) {
 	validator := NewValidator(nil)
 	admitters := validator.Admitters()
-	assert.Len(t, admitters, 2)
+	assert.Len(t, admitters, 3)
 	hasPSAAdmitter := false
 	hasProjectNamespaceAdmitter := false
 	for i := range admitters {