Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deny unprivileged users from setting "k8s.cloudscale.ch/loadbalancer-uuid" #106

Merged
merged 1 commit into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading