From 2c634fa378f930f1428df07052f2c5e008a1ff6e Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Wed, 6 Apr 2022 17:13:56 +0200 Subject: [PATCH 1/5] Add annotation to disable request ration warnings per namespace --- .codeclimate.yml | 5 ++ main.go | 3 +- webhooks/ratio_validator.go | 79 +++++++++++++++++++++++--------- webhooks/ratio_validator_test.go | 13 +++++- 4 files changed, 74 insertions(+), 26 deletions(-) diff --git a/.codeclimate.yml b/.codeclimate.yml index 79b2483..bbc230d 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,4 +1,9 @@ version: "2" +checks: + return-statements: + enabled: true + config: + threshold: 8 plugins: shellcheck: enabled: true diff --git a/main.go b/main.go index 64b3a7a..b15ba06 100644 --- a/main.go +++ b/main.go @@ -22,12 +22,11 @@ var ( commit = "-dirty-" date = time.Now().Format("2006-01-02") - // TODO: Adjust app name appName = "appuio-cloud-agent" appLongName = "agent running on every APPUiO Cloud Zone" scheme = runtime.NewScheme() - setupLog = ctrl.Log.WithName("setup") + setupLog = ctrl.Log.WithName("setup").WithValues("version", version, "commit", commit, "date", date) ) //go:generate go run sigs.k8s.io/controller-tools/cmd/controller-gen object paths="./..." diff --git a/webhooks/ratio_validator.go b/webhooks/ratio_validator.go index d7b3856..e7dedcf 100644 --- a/webhooks/ratio_validator.go +++ b/webhooks/ratio_validator.go @@ -27,6 +27,9 @@ type RatioValidator struct { RatioLimit *resource.Quantity } +// RatioValidatiorDisableAnnotation is the key for an annotion on a namespace to disable request ratio warnings +var RatioValidatiorDisableAnnotation = "validate-request-ratio.appuio.io/disable" + // Handle handles the admission requests func (v *RatioValidator) Handle(ctx context.Context, req admission.Request) admission.Response { l := log.FromContext(ctx). @@ -40,6 +43,16 @@ func (v *RatioValidator) Handle(ctx context.Context, req admission.Request) admi return admission.Allowed("system user") } + disabled, err := v.isNamespaceDisabled(ctx, req.Namespace) + if err != nil { + l.Error(err, "failed to get namespace") + return errored(http.StatusInternalServerError, err) + } + if disabled { + l.V(1).Info("allowed: warning disabled") + return admission.Allowed("system user") + } + r, err := v.getRatio(ctx, req.Namespace) if err != nil { l.Error(err, "failed to get ratio") @@ -50,28 +63,10 @@ func (v *RatioValidator) Handle(ctx context.Context, req admission.Request) admi // If we are creating an object with resource requests, we add them to the current ratio // We cannot easily do this when updating resources. if req.Operation == admissionv1.Create { - switch req.Kind.Kind { - case "Pod": - pod := corev1.Pod{} - if err := v.decoder.Decode(req, &pod); err != nil { - l.Error(err, "failed to decode pod") - return errored(http.StatusBadRequest, err) - } - r = r.RecordPod(pod) - case "Deployment": - deploy := appsv1.Deployment{} - if err := v.decoder.Decode(req, &deploy); err != nil { - l.Error(err, "failed to decode deployment") - return errored(http.StatusBadRequest, err) - } - r = r.RecordDeployment(deploy) - case "StatefulSet": - sts := appsv1.StatefulSet{} - if err := v.decoder.Decode(req, &sts); err != nil { - l.Error(err, "failed to decode statefulset") - return errored(http.StatusBadRequest, err) - } - r = r.RecordStatefulSet(sts) + r, err = v.recodObject(ctx, r, req) + if err != nil { + l.Error(err, "failed to record object") + return errored(http.StatusBadRequest, err) } } l = l.WithValues("ratio", r.String()) @@ -91,6 +86,46 @@ func (v *RatioValidator) Handle(ctx context.Context, req admission.Request) admi return admission.Allowed("ok") } +func (v *RatioValidator) recodObject(ctx context.Context, r *Ratio, req admission.Request) (*Ratio, error) { + switch req.Kind.Kind { + case "Pod": + pod := corev1.Pod{} + if err := v.decoder.Decode(req, &pod); err != nil { + return r, err + } + r = r.RecordPod(pod) + case "Deployment": + deploy := appsv1.Deployment{} + if err := v.decoder.Decode(req, &deploy); err != nil { + return r, err + } + r = r.RecordDeployment(deploy) + case "StatefulSet": + sts := appsv1.StatefulSet{} + if err := v.decoder.Decode(req, &sts); err != nil { + return r, err + } + r = r.RecordStatefulSet(sts) + } + return r, nil +} + +func (v *RatioValidator) isNamespaceDisabled(ctx context.Context, nsName string) (bool, error) { + ns := corev1.Namespace{} + err := v.client.Get(ctx, client.ObjectKey{ + Name: nsName, + }, &ns) + if err != nil { + return false, err + } + + disabled, ok := ns.Annotations[RatioValidatiorDisableAnnotation] + if !ok { + return false, err + } + return disabled == "True", nil +} + func (v *RatioValidator) getRatio(ctx context.Context, ns string) (*Ratio, error) { r := NewRatio() pods := corev1.PodList{} diff --git a/webhooks/ratio_validator_test.go b/webhooks/ratio_validator_test.go index c0b31b2..156002e 100644 --- a/webhooks/ratio_validator_test.go +++ b/webhooks/ratio_validator_test.go @@ -104,13 +104,14 @@ func TestRatioValidator_Handle(t *testing.T) { user: "bar", namespace: "fail-bar", resources: []client.Object{ + testNamespace("fail-bar"), podFromResources("pod1", "foo", podResource{ {cpu: "100m", memory: "3G"}, }), podFromResources("pod2", "foo", podResource{ {cpu: "50m", memory: "1Gi"}, }), - podFromResources("unfair", "bar", podResource{ + podFromResources("unfair", "fail-bar", podResource{ {cpu: "8", memory: "1Gi"}, }), }, @@ -242,7 +243,7 @@ func prepareTest(t *testing.T, initObjs ...client.Object) *RatioValidator { decoder, err := admission.NewDecoder(scheme) require.NoError(t, err) - + initObjs = append(initObjs, testNamespace("foo"), testNamespace("bar")) client := fake.NewClientBuilder(). WithScheme(scheme). WithObjects(initObjs...). @@ -256,6 +257,14 @@ func prepareTest(t *testing.T, initObjs ...client.Object) *RatioValidator { return uv } +func testNamespace(name string) *corev1.Namespace { + return &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } +} + func podFromResources(name, namespace string, res podResource) *corev1.Pod { p := corev1.Pod{ TypeMeta: metav1.TypeMeta{ From 832c7f65dced98df78bc5f9a6144dfaa406beae8 Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Thu, 7 Apr 2022 09:57:00 +0200 Subject: [PATCH 2/5] Add tests for disabling warnings --- webhooks/ratio_validator.go | 7 ++++- webhooks/ratio_validator_test.go | 49 +++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/webhooks/ratio_validator.go b/webhooks/ratio_validator.go index e7dedcf..c716665 100644 --- a/webhooks/ratio_validator.go +++ b/webhooks/ratio_validator.go @@ -4,11 +4,13 @@ import ( "context" "fmt" "net/http" + "strconv" "strings" admissionv1 "k8s.io/api/admission/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,6 +48,9 @@ func (v *RatioValidator) Handle(ctx context.Context, req admission.Request) admi disabled, err := v.isNamespaceDisabled(ctx, req.Namespace) if err != nil { l.Error(err, "failed to get namespace") + if apierrors.IsNotFound(err) { + return errored(http.StatusNotFound, err) + } return errored(http.StatusInternalServerError, err) } if disabled { @@ -123,7 +128,7 @@ func (v *RatioValidator) isNamespaceDisabled(ctx context.Context, nsName string) if !ok { return false, err } - return disabled == "True", nil + return strconv.ParseBool(disabled) } func (v *RatioValidator) getRatio(ctx context.Context, ns string) (*Ratio, error) { diff --git a/webhooks/ratio_validator_test.go b/webhooks/ratio_validator_test.go index 156002e..7dcf642 100644 --- a/webhooks/ratio_validator_test.go +++ b/webhooks/ratio_validator_test.go @@ -83,6 +83,29 @@ func TestRatioValidator_Handle(t *testing.T) { limit: "1Gi", warn: true, }, + "Allow_DisabledUnfairNamespace": { + user: "appuio#foo", + namespace: "disabled-foo", + resources: []client.Object{ + podFromResources("unfair", "disabled-foo", podResource{ + {cpu: "8", memory: "1Gi"}, + }), + }, + limit: "1Gi", + warn: false, + }, + "Allow_LowercaseDisabledUnfairNamespace": { + user: "appuio#foo", + namespace: "disabled-bar", + resources: []client.Object{ + podFromResources("unfair", "disabled-bar", podResource{ + {cpu: "8", memory: "1Gi"}, + }), + }, + limit: "1Gi", + warn: false, + }, + "Allow_ServiceAccount": { user: "system:serviceaccount:bar", namespace: "bar", @@ -120,6 +143,16 @@ func TestRatioValidator_Handle(t *testing.T) { fail: true, statusCode: http.StatusInternalServerError, }, + "NamespaceNotExists": { + user: "bar", + namespace: "notexits", + resources: []client.Object{}, + limit: "1Gi", + warn: false, + fail: true, + statusCode: http.StatusNotFound, + }, + "Warn_ConsiderNewPod": { user: "appuio#foo", namespace: "foo", @@ -243,7 +276,21 @@ func prepareTest(t *testing.T, initObjs ...client.Object) *RatioValidator { decoder, err := admission.NewDecoder(scheme) require.NoError(t, err) - initObjs = append(initObjs, testNamespace("foo"), testNamespace("bar")) + barNs := testNamespace("bar") + barNs.Annotations = map[string]string{ + RatioValidatiorDisableAnnotation: "False", + } + + disabledNs := testNamespace("disabled-foo") + disabledNs.Annotations = map[string]string{ + RatioValidatiorDisableAnnotation: "True", + } + otherDisabledNs := testNamespace("disabled-bar") + otherDisabledNs.Annotations = map[string]string{ + RatioValidatiorDisableAnnotation: "true", + } + + initObjs = append(initObjs, testNamespace("foo"), barNs, disabledNs, otherDisabledNs) client := fake.NewClientBuilder(). WithScheme(scheme). WithObjects(initObjs...). From 244a52a61a12a417c40e70d14ec3486a470b7f16 Mon Sep 17 00:00:00 2001 From: Fabian Fischer <10788152+glrf@users.noreply.github.com> Date: Thu, 7 Apr 2022 11:09:17 +0200 Subject: [PATCH 3/5] Explicitly return nil instead of nil error Co-authored-by: Sebastian Widmer --- webhooks/ratio_validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webhooks/ratio_validator.go b/webhooks/ratio_validator.go index c716665..12cc535 100644 --- a/webhooks/ratio_validator.go +++ b/webhooks/ratio_validator.go @@ -126,7 +126,7 @@ func (v *RatioValidator) isNamespaceDisabled(ctx context.Context, nsName string) disabled, ok := ns.Annotations[RatioValidatiorDisableAnnotation] if !ok { - return false, err + return false, nil } return strconv.ParseBool(disabled) } From 2cadbfa7326b50440ca93362d44a58554e5aff63 Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Thu, 7 Apr 2022 11:24:30 +0200 Subject: [PATCH 4/5] Fix typo --- webhooks/ratio_validator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webhooks/ratio_validator.go b/webhooks/ratio_validator.go index 12cc535..ba26705 100644 --- a/webhooks/ratio_validator.go +++ b/webhooks/ratio_validator.go @@ -68,7 +68,7 @@ func (v *RatioValidator) Handle(ctx context.Context, req admission.Request) admi // If we are creating an object with resource requests, we add them to the current ratio // We cannot easily do this when updating resources. if req.Operation == admissionv1.Create { - r, err = v.recodObject(ctx, r, req) + r, err = v.recordObject(ctx, r, req) if err != nil { l.Error(err, "failed to record object") return errored(http.StatusBadRequest, err) @@ -91,7 +91,7 @@ func (v *RatioValidator) Handle(ctx context.Context, req admission.Request) admi return admission.Allowed("ok") } -func (v *RatioValidator) recodObject(ctx context.Context, r *Ratio, req admission.Request) (*Ratio, error) { +func (v *RatioValidator) recordObject(ctx context.Context, r *Ratio, req admission.Request) (*Ratio, error) { switch req.Kind.Kind { case "Pod": pod := corev1.Pod{} From d1016f1417ca0c778f631def461c01fc59fb46eb Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Thu, 7 Apr 2022 11:25:02 +0200 Subject: [PATCH 5/5] Add RBAC rules for namespaces --- config/rbac/role.yaml | 8 ++++++++ webhooks/ratio_validator.go | 1 + 2 files changed, 9 insertions(+) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 7972ed8..b8d485a 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -5,6 +5,14 @@ metadata: creationTimestamp: null name: appuio-cloud-agent rules: +- apiGroups: + - "" + resources: + - namespaces + verbs: + - get + - list + - watch - apiGroups: - "" resources: diff --git a/webhooks/ratio_validator.go b/webhooks/ratio_validator.go index ba26705..76f26ea 100644 --- a/webhooks/ratio_validator.go +++ b/webhooks/ratio_validator.go @@ -20,6 +20,7 @@ import ( ) // +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch // RatioValidator checks for every action in a namespace whether the Memory to CPU ratio limit is exceeded and will return a warning if it is. type RatioValidator struct {