Skip to content

Commit

Permalink
Deny unprivileged users from setting "k8s.cloudscale.ch/loadbalancer-…
Browse files Browse the repository at this point in the history
  • Loading branch information
bastjan authored Jul 26, 2024
1 parent dce5254 commit 5398c08
Show file tree
Hide file tree
Showing 5 changed files with 235 additions and 0 deletions.
21 changes: 21 additions & 0 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,24 @@ webhooks:
- '*'
scope: Namespaced
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-service-cloudscale-lb
failurePolicy: Fail
matchPolicy: Equivalent
name: validate-service-cloudscale-lb.appuio.io
rules:
- apiGroups:
- ""
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- services
sideEffects: None
13 changes: 13 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ func main() {
var selectedUsageProfile string
flag.StringVar(&selectedUsageProfile, "usage-profile", "", "UsageProfile to use. Applies all profiles if empty. Dynamic selection is not supported yet.")

var cloudscaleLoadbalancerValidationEnabled bool
flag.BoolVar(&cloudscaleLoadbalancerValidationEnabled, "cloudscale-loadbalancer-validation-enabled", false, "Enable Cloudscale Loadbalancer validation. Validates that the k8s.cloudscale.ch/loadbalancer-uuid annotation cannot be changed by unprivileged users.")

var qps, burst int
flag.IntVar(&qps, "qps", 20, "QPS to use for the controller-runtime client")
flag.IntVar(&burst, "burst", 100, "Burst to use for the controller-runtime client")
Expand Down Expand Up @@ -242,6 +245,16 @@ func main() {
},
})

mgr.GetWebhookServer().Register("/validate-service-cloudscale-lb", &webhook.Admission{
Handler: &webhooks.ServiceCloudscaleLBValidator{
Decoder: admission.NewDecoder(mgr.GetScheme()),
Skipper: skipper.NewMultiSkipper(
skipper.StaticSkipper{ShouldSkip: !cloudscaleLoadbalancerValidationEnabled},
psk,
),
},
})

if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
setupLog.Error(err, "unable to setup health endpoint")
os.Exit(1)
Expand Down
76 changes: 76 additions & 0 deletions webhooks/service_cloudscale_lb_validator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package webhooks

import (
"context"
"fmt"
"net/http"

corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/appuio/appuio-cloud-agent/skipper"
)

// +kubebuilder:webhook:path=/validate-service-cloudscale-lb,name=validate-service-cloudscale-lb.appuio.io,admissionReviewVersions=v1,sideEffects=none,mutating=false,failurePolicy=Fail,groups="",resources=services,verbs=create;update,versions=v1,matchPolicy=equivalent

const (
CloudscaleLoadbalancerUUIDAnnotation = "k8s.cloudscale.ch/loadbalancer-uuid"
)

// ServiceCloudscaleLBValidator checks if a user is allowed to create a namespace.
// The user or the namespace must have a label with the organization name.
// The organization name is used to count the number of namespaces for the organization.
type ServiceCloudscaleLBValidator struct {
Decoder admission.Decoder

Skipper skipper.Skipper
}

// Handle handles the admission requests
func (v *ServiceCloudscaleLBValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
ctx = log.IntoContext(ctx, log.FromContext(ctx).
WithName("webhook.validate-namespace-quota.appuio.io").
WithValues("id", req.UID, "user", req.UserInfo.Username).
WithValues("namespace", req.Namespace, "name", req.Name,
"group", req.Kind.Group, "version", req.Kind.Version, "kind", req.Kind.Kind))

return logAdmissionResponse(ctx, v.handle(ctx, req))
}

func (v *ServiceCloudscaleLBValidator) handle(ctx context.Context, req admission.Request) admission.Response {
l := log.FromContext(ctx)

skip, err := v.Skipper.Skip(ctx, req)
if err != nil {
l.Error(err, "error while checking skipper")
return admission.Errored(http.StatusInternalServerError, err)
}
if skip {
return admission.Allowed("skipped")
}

var newService corev1.Service
if err := v.Decoder.Decode(req, &newService); err != nil {
l.Error(err, "failed to decode request")
return admission.Errored(http.StatusBadRequest, err)
}

var oldService corev1.Service
if req.OldObject.Raw != nil {
if err := v.Decoder.DecodeRaw(req.OldObject, &oldService); err != nil {
l.Error(err, "failed to decode old object")
return admission.Errored(http.StatusBadRequest, err)
}
}

oldAnnotiation := oldService.GetAnnotations()[CloudscaleLoadbalancerUUIDAnnotation]
newAnnotiation := newService.GetAnnotations()[CloudscaleLoadbalancerUUIDAnnotation]

if oldAnnotiation != newAnnotiation {
l.Info("Loadbalancer UUID changed", "old", oldAnnotiation, "new", newAnnotiation)
return admission.Denied(fmt.Sprintf("%s annotation cannot be changed", CloudscaleLoadbalancerUUIDAnnotation))
}

return admission.Allowed("allowed")
}
94 changes: 94 additions & 0 deletions webhooks/service_cloudscale_lb_validator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package webhooks

import (
"context"
"testing"

"github.com/go-logr/logr/testr"
"github.com/stretchr/testify/require"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/appuio/appuio-cloud-agent/skipper"
)

func Test_ServiceCloudscaleLBValidator_Handle(t *testing.T) {
ctx := log.IntoContext(context.Background(), testr.New(t))

tests := map[string]struct {
object client.Object
oldObject client.Object
allowed bool
skip bool
matchMessage string
}{
"Create allow no annotations": {
object: newService("test", nil, nil),
allowed: true,
},
"Create allow other annotation": {
object: newService("test", nil, map[string]string{"other": "value"}),
allowed: true,
},
"Create deny lb annotation": {
object: newService("test", nil, map[string]string{CloudscaleLoadbalancerUUIDAnnotation: "value"}),
allowed: false,
matchMessage: "k8s.cloudscale.ch/loadbalancer-uuid annotation cannot be changed",
},

"Create allow skipped": {
object: newService("test", nil, map[string]string{CloudscaleLoadbalancerUUIDAnnotation: "value"}),
allowed: true,
skip: true,
matchMessage: "skipped",
},

"Update allow no annotations": {
object: newService("test", nil, nil),
oldObject: newService("test", nil, nil),
allowed: true,
},
"Update allow other annotation": {
object: newService("test", nil, map[string]string{"other": "value"}),
oldObject: newService("test", nil, map[string]string{"other": "value2"}),
allowed: true,
},
"Update allow new other annotation": {
object: newService("test", nil, map[string]string{"other": "value"}),
oldObject: newService("test", nil, nil),
allowed: true,
},
"Update allow delete other annotation": {
object: newService("test", nil, nil),
oldObject: newService("test", nil, map[string]string{"other": "value"}),
allowed: true,
},
"Update deny add lb annotation": {
object: newService("test", nil, map[string]string{CloudscaleLoadbalancerUUIDAnnotation: "value"}),
oldObject: newService("test", nil, nil),
allowed: false,
matchMessage: "k8s.cloudscale.ch/loadbalancer-uuid annotation cannot be changed",
},
"Update deny update lb annotation": {
object: newService("test", nil, map[string]string{CloudscaleLoadbalancerUUIDAnnotation: "value2"}),
oldObject: newService("test", nil, map[string]string{CloudscaleLoadbalancerUUIDAnnotation: "value"}),
allowed: false,
matchMessage: "k8s.cloudscale.ch/loadbalancer-uuid annotation cannot be changed",
},
}

_, scheme, dec := prepareClient(t)
for name, tC := range tests {
t.Run(name, func(t *testing.T) {
subject := &ServiceCloudscaleLBValidator{
Decoder: dec,
Skipper: skipper.StaticSkipper{ShouldSkip: tC.skip},
}
res := subject.Handle(ctx, admissionRequestForObjectWithOldObject(t, tC.object, tC.oldObject, scheme))
require.Equal(t, tC.allowed, res.Allowed)
if tC.matchMessage != "" {
require.Contains(t, res.Result.Message, tC.matchMessage)
}
})
}
}
31 changes: 31 additions & 0 deletions webhooks/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,26 @@ import (
func admissionRequestForObject(t *testing.T, object client.Object, scheme *runtime.Scheme) admission.Request {
t.Helper()

return admissionRequestForObjectWithOldObject(t, object, nil, scheme)
}

func admissionRequestForObjectWithOldObject(t *testing.T, object, oldObject client.Object, scheme *runtime.Scheme) admission.Request {
t.Helper()

testutils.EnsureGroupVersionKind(t, scheme, object)
gvk := object.GetObjectKind().GroupVersionKind()

raw, err := json.Marshal(object)
require.NoError(t, err)

var oldRaw []byte
if oldObject != nil {
testutils.EnsureGroupVersionKind(t, scheme, oldObject)
r, err := json.Marshal(oldObject)
require.NoError(t, err)
oldRaw = r
}

return admission.Request{
AdmissionRequest: admissionv1.AdmissionRequest{
UID: "e515f52d-7181-494d-a3d3-f0738856bd97",
Expand All @@ -50,6 +64,9 @@ func admissionRequestForObject(t *testing.T, object client.Object, scheme *runti
Object: runtime.RawExtension{
Raw: raw,
},
OldObject: runtime.RawExtension{
Raw: oldRaw,
},
},
}
}
Expand All @@ -68,6 +85,20 @@ func newNamespace(name string, labels, annotations map[string]string) *corev1.Na
}
}

func newService(name string, labels, annotations map[string]string) *corev1.Service {
return &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Labels: labels,
Annotations: annotations,
},
}
}

func prepareClient(t *testing.T, initObjs ...client.Object) (client.WithWatch, *runtime.Scheme, admission.Decoder) {
t.Helper()

Expand Down

0 comments on commit 5398c08

Please sign in to comment.