From fc7267fddb113ecfc9efb9f81af69b7a7aedcb41 Mon Sep 17 00:00:00 2001 From: irbekrm Date: Wed, 30 Aug 2023 11:53:46 +0100 Subject: [PATCH] cmd/k8s-operator: proxy Service conditions Validate service with Tailscale annotations/load balancer class, set a false TailscaleStatus status condition for invalid Services Updates https://github.com/tailscale/tailscale/issues/8184 Signed-off-by: irbekrm --- cmd/k8s-operator/svc.go | 108 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/cmd/k8s-operator/svc.go b/cmd/k8s-operator/svc.go index 3bab3532a..f55e99d0a 100644 --- a/cmd/k8s-operator/svc.go +++ b/cmd/k8s-operator/svc.go @@ -14,11 +14,22 @@ import ( "go.uber.org/zap" "golang.org/x/exp/slices" corev1 "k8s.io/api/core/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/clock" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +const ( + reasonInvalidTailscaleService = "InvalidTailscaleService" + conditionTailscaleStatus = "TailscaleStatus" +) + +// Clock is defined as a package var so it can be stubbed out during tests. +var Clock clock.Clock = clock.RealClock{} + type ServiceReconciler struct { client.Client ssr *tailscaleSTSReconciler @@ -82,10 +93,16 @@ func (a *ServiceReconciler) maybeCleanup(ctx context.Context, logger *zap.Sugare } svc.Finalizers = append(svc.Finalizers[:ix], svc.Finalizers[ix+1:]...) + if err := a.Update(ctx, svc); err != nil { return fmt.Errorf("failed to remove finalizer: %w", err) } + removeTailscaleCondition(svc, logger) + if err := a.Status().Update(ctx, svc); err != nil { + return fmt.Errorf("failed to remove Tailscale condition: %w", err) + } + // Unlike most log entries in the reconcile loop, this will get printed // exactly once at the very end of cleanup, because the final step of // cleanup removes the tailscale finalizer, which will make all future @@ -116,6 +133,28 @@ func (a *ServiceReconciler) maybeProvision(ctx context.Context, logger *zap.Suga return fmt.Errorf("failed to add finalizer: %w", err) } } + + isInvalid, msg := a.isInvalid(svc) + if isInvalid { + logger.Infof("Service is an invalid Tailscale proxy Service: %s", msg) + + // TODO (irbekrm): Service status conditions update should be a deferred + // function -we want to ensure that status gets updated + // correctly in both success and failure cases + oldSvc := svc.DeepCopy() + setServiceCondition(svc, metav1.ConditionFalse, conditionTailscaleStatus, reasonInvalidTailscaleService, msg) + + if !apiequality.Semantic.DeepEqual(oldSvc.Status, svc.Status) { + logger.Info("udpating Service status") + if err := a.Status().Update(ctx, svc); err != nil { + logger.Errorf("Failed to update Service status: %v", err) + return err + } + } + // we will reconcile the Service when the user fixes it + return nil + } + crl := childResourceLabels(svc.Name, svc.Namespace, "svc") var tags []string if tstr, ok := svc.Annotations[AnnotationTags]; ok { @@ -190,9 +229,41 @@ func (a *ServiceReconciler) maybeProvision(ctx context.Context, logger *zap.Suga if err := a.Status().Update(ctx, svc); err != nil { return fmt.Errorf("failed to update service status: %w", err) } + return nil } +// hasViolations reports whether the given Service represents an invalid +// Tailscale egress/ingress service. Also returns a message describing the first +// found violation. +func (a *ServiceReconciler) isInvalid(svc *corev1.Service) (isInvalid bool, msg string) { + if !a.shouldExpose(svc) && !a.hasTailnetTargetAnnotation(svc) { + return false, "" + } + if a.hasTailnetTargetAnnotation(svc) && a.hasLoadBalancerClass(svc) { + return true, "Service has both tailscale.com/tailnet-target-ip annotation and tailscale load balancer class set." + } + if a.hasTailnetTargetAnnotation(svc) && a.hasExposeAnnotation(svc) { + return true, "Service has both tailscale.com/tailnet-target-ip and tailscale.com/expose annotation set." + } + if a.hasTailnetTargetAnnotation(svc) { + if svc.Spec.Type != corev1.ServiceTypeExternalName { + return true, fmt.Sprintf("Service has tailscale.com/tailnet-target-ip annotation, but service type is %s. Only Services of type External Name can be used.", svc.Spec.Type) + } + if a.hasTailnetTargetAnnotation(svc) && len(svc.Spec.Ports) > 0 { + return true, "Service has tailscale.com/tailnet-target-ip annotation, and has ports defined. Ports are not allowed." + } + if a.hasTailnetTargetAnnotation(svc) && len(svc.Spec.Ports) > 0 { + return true, "Service has tailscale.com/tailnet-target-ip annotation, and has ports defined. Ports are not allowed." + } + if a.hasLoadBalancerClass(svc) && svc.Spec.Selector != nil { + return true, "Service has tailscale.com/tailnet-target-ip annotation, and has ports defined. Selector is not allowed." + } + + } + return false, "" +} + func (a *ServiceReconciler) shouldExpose(svc *corev1.Service) bool { // Headless services can't be exposed, since there is no ClusterIP to // forward to. @@ -221,3 +292,40 @@ func (a *ServiceReconciler) hasExposeAnnotation(svc *corev1.Service) bool { func (a *ServiceReconciler) hasTailnetTargetAnnotation(svc *corev1.Service) bool { return svc != nil && svc.Annotations[AnnotationTailnetTargetIP] != "" } + +// conditon-related logic inspired by +// https://github.com/cert-manager/cert-manager/blob/v1.12.3/pkg/api/util/conditions.go +func setServiceCondition(svc *corev1.Service, status metav1.ConditionStatus, typ, reason, msg string) { + newCond := metav1.Condition{ + Type: typ, + Status: status, + Reason: reason, + Message: msg, + } + nowTime := metav1.NewTime(Clock.Now()) + newCond.LastTransitionTime = nowTime + + for idx, cond := range svc.Status.Conditions { + if cond.Type != typ { + continue + } + if cond.Status == status { + newCond.LastTransitionTime = cond.LastTransitionTime + } + svc.Status.Conditions[idx] = newCond + return + } + svc.Status.Conditions = append(svc.Status.Conditions, newCond) +} + +func removeTailscaleCondition(svc *corev1.Service, logger *zap.SugaredLogger) { + newConds := make([]metav1.Condition, 0) + for _, cond := range svc.Status.Conditions { + if cond.Type == conditionTailscaleStatus { + logger.Info("removing %s condition from Service", conditionTailscaleStatus) + continue + } + newConds = append(newConds, cond) + } + svc.Status.Conditions = newConds +}