cmd/k8s-operator,k8s-operator,go.{mod,sum}: publish proxy status condition for annotated services (#12463)

Adds a new TailscaleProxyReady condition type for use in corev1.Service
conditions.

Also switch our CRDs to use metav1.Condition instead of
ConnectorCondition. The Go structs are seralized identically, but it
updates some descriptions and validation rules. Update k8s
controller-tools and controller-runtime deps to fix the documentation
generation for metav1.Condition so that it excludes comments and
TODOs.

Stop expecting the fake client to populate TypeMeta in tests. See
kubernetes-sigs/controller-runtime#2633 for details of the change.

Finally, make some minor improvements to validation for service hostnames.

Fixes #12216

Co-authored-by: Irbe Krumina <irbe@tailscale.com>
Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>
This commit is contained in:
Tom Proctor
2024-06-18 19:01:40 +01:00
committed by GitHub
parent 45d2f4301f
commit 3099323976
24 changed files with 4200 additions and 1067 deletions

View File

@ -9,6 +9,7 @@ import (
"context"
"fmt"
"testing"
"time"
"github.com/google/go-cmp/cmp"
"go.uber.org/zap"
@ -17,10 +18,13 @@ import (
networkingv1 "k8s.io/api/networking/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
tsapi "tailscale.com/k8s-operator/apis/v1alpha1"
"tailscale.com/net/dns/resolvconffile"
"tailscale.com/tstest"
"tailscale.com/tstime"
"tailscale.com/types/ptr"
"tailscale.com/util/dnsname"
"tailscale.com/util/mak"
@ -33,6 +37,7 @@ func TestLoadBalancerClass(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -42,11 +47,13 @@ func TestLoadBalancerClass(t *testing.T) {
operatorNamespace: "operator-ns",
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
logger: zl.Sugar(),
clock: clock,
recorder: record.NewFakeRecorder(100),
}
// Create a service that we should manage, and check that the initial round
// of objects looks right.
// Create a service that we should manage, but start with a miconfiguration
// in the annotations.
mustCreate(t, fc, &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
@ -55,6 +62,9 @@ func TestLoadBalancerClass(t *testing.T) {
// doesn't. So, set it explicitly because other code later depends
// on it being set.
UID: types.UID("1234-UID"),
Annotations: map[string]string{
AnnotationTailnetTargetFQDN: "invalid.example.com",
},
},
Spec: corev1.ServiceSpec{
ClusterIP: "10.20.30.40",
@ -65,6 +75,46 @@ func TestLoadBalancerClass(t *testing.T) {
expectReconciled(t, sr, "default", "test")
// The expected value of .status.conditions[0].LastTransitionTime until the
// proxy becomes ready.
t0 := conditionTime(clock)
// Should have an error about invalid config.
want := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
UID: types.UID("1234-UID"),
Annotations: map[string]string{
AnnotationTailnetTargetFQDN: "invalid.example.com",
},
},
Spec: corev1.ServiceSpec{
ClusterIP: "10.20.30.40",
Type: corev1.ServiceTypeLoadBalancer,
LoadBalancerClass: ptr.To("tailscale"),
},
Status: corev1.ServiceStatus{
Conditions: []metav1.Condition{{
Type: string(tsapi.ProxyReady),
Status: metav1.ConditionFalse,
LastTransitionTime: t0,
Reason: reasonProxyInvalid,
Message: `unable to provision proxy resources: invalid Service: invalid value of annotation tailscale.com/tailnet-fqdn: "invalid.example.com" does not appear to be a valid MagicDNS name`,
}},
},
}
expectEqual(t, fc, want, nil)
// Delete the misconfiguration so the proxy starts getting created on the
// next reconcile.
mustUpdate(t, fc, "default", "test", func(s *corev1.Service) {
s.ObjectMeta.Annotations = nil
})
clock.Advance(time.Second)
expectReconciled(t, sr, "default", "test")
fullName, shortName := findGenName(t, fc, "default", "test", "svc")
opts := configOpts{
stsName: shortName,
@ -79,6 +129,19 @@ func TestLoadBalancerClass(t *testing.T) {
expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil)
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)
want.Annotations = nil
want.ObjectMeta.Finalizers = []string{"tailscale.com/finalizer"}
want.Status = corev1.ServiceStatus{
Conditions: []metav1.Condition{{
Type: string(tsapi.ProxyReady),
Status: metav1.ConditionFalse,
LastTransitionTime: t0, // Status is still false, no update to transition time
Reason: reasonProxyPending,
Message: "no Tailscale hostname known yet, waiting for proxy pod to finish auth",
}},
}
expectEqual(t, fc, want, nil)
// Normally the Tailscale proxy pod would come up here and write its info
// into the secret. Simulate that, then verify reconcile again and verify
// that we get to the end.
@ -90,33 +153,16 @@ func TestLoadBalancerClass(t *testing.T) {
s.Data["device_fqdn"] = []byte("tailscale.device.name.")
s.Data["device_ips"] = []byte(`["100.99.98.97", "2c0a:8083:94d4:2012:3165:34a5:3616:5fdf"]`)
})
clock.Advance(time.Second)
expectReconciled(t, sr, "default", "test")
want := &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
Finalizers: []string{"tailscale.com/finalizer"},
UID: types.UID("1234-UID"),
},
Spec: corev1.ServiceSpec{
ClusterIP: "10.20.30.40",
Type: corev1.ServiceTypeLoadBalancer,
LoadBalancerClass: ptr.To("tailscale"),
},
Status: corev1.ServiceStatus{
LoadBalancer: corev1.LoadBalancerStatus{
Ingress: []corev1.LoadBalancerIngress{
{
Hostname: "tailscale.device.name",
},
{
IP: "100.99.98.97",
},
},
want.Status.Conditions = proxyCreatedCondition(clock)
want.Status.LoadBalancer = corev1.LoadBalancerStatus{
Ingress: []corev1.LoadBalancerIngress{
{
Hostname: "tailscale.device.name",
},
{
IP: "100.99.98.97",
},
},
}
@ -144,11 +190,9 @@ func TestLoadBalancerClass(t *testing.T) {
expectMissing[appsv1.StatefulSet](t, fc, "operator-ns", shortName)
expectMissing[corev1.Service](t, fc, "operator-ns", shortName)
expectMissing[corev1.Secret](t, fc, "operator-ns", fullName)
// Note that the Tailscale-specific condition status should be gone now.
want = &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -170,6 +214,7 @@ func TestTailnetTargetFQDNAnnotation(t *testing.T) {
t.Fatal(err)
}
tailnetTargetFQDN := "foo.bar.ts.net."
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -180,6 +225,7 @@ func TestTailnetTargetFQDNAnnotation(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
}
// Create a service that we should manage, and check that the initial round
@ -220,10 +266,6 @@ func TestTailnetTargetFQDNAnnotation(t *testing.T) {
expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil)
expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation)
want := &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -238,6 +280,9 @@ func TestTailnetTargetFQDNAnnotation(t *testing.T) {
Type: corev1.ServiceTypeExternalName,
Selector: nil,
},
Status: corev1.ServiceStatus{
Conditions: proxyCreatedCondition(clock),
},
}
expectEqual(t, fc, want, nil)
expectEqual(t, fc, expectedSecret(t, fc, o), nil)
@ -280,6 +325,7 @@ func TestTailnetTargetIPAnnotation(t *testing.T) {
t.Fatal(err)
}
tailnetTargetIP := "100.66.66.66"
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -290,6 +336,7 @@ func TestTailnetTargetIPAnnotation(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
}
// Create a service that we should manage, and check that the initial round
@ -330,10 +377,6 @@ func TestTailnetTargetIPAnnotation(t *testing.T) {
expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil)
expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation)
want := &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -348,6 +391,9 @@ func TestTailnetTargetIPAnnotation(t *testing.T) {
Type: corev1.ServiceTypeExternalName,
Selector: nil,
},
Status: corev1.ServiceStatus{
Conditions: proxyCreatedCondition(clock),
},
}
expectEqual(t, fc, want, nil)
expectEqual(t, fc, expectedSecret(t, fc, o), nil)
@ -389,6 +435,7 @@ func TestAnnotations(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -399,6 +446,7 @@ func TestAnnotations(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
}
// Create a service that we should manage, and check that the initial round
@ -437,10 +485,6 @@ func TestAnnotations(t *testing.T) {
expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil)
expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation)
want := &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -454,6 +498,9 @@ func TestAnnotations(t *testing.T) {
ClusterIP: "10.20.30.40",
Type: corev1.ServiceTypeClusterIP,
},
Status: corev1.ServiceStatus{
Conditions: proxyCreatedCondition(clock),
},
}
expectEqual(t, fc, want, nil)
@ -473,10 +520,6 @@ func TestAnnotations(t *testing.T) {
expectMissing[corev1.Service](t, fc, "operator-ns", shortName)
expectMissing[corev1.Secret](t, fc, "operator-ns", fullName)
want = &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -497,6 +540,7 @@ func TestAnnotationIntoLB(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -507,6 +551,7 @@ func TestAnnotationIntoLB(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
}
// Create a service that we should manage, and check that the initial round
@ -558,10 +603,6 @@ func TestAnnotationIntoLB(t *testing.T) {
})
expectReconciled(t, sr, "default", "test")
want := &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -575,6 +616,9 @@ func TestAnnotationIntoLB(t *testing.T) {
ClusterIP: "10.20.30.40",
Type: corev1.ServiceTypeClusterIP,
},
Status: corev1.ServiceStatus{
Conditions: proxyCreatedCondition(clock),
},
}
expectEqual(t, fc, want, nil)
@ -592,10 +636,6 @@ func TestAnnotationIntoLB(t *testing.T) {
// ... but the service should have a LoadBalancer status.
want = &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -618,6 +658,7 @@ func TestAnnotationIntoLB(t *testing.T) {
},
},
},
Conditions: proxyCreatedCondition(clock),
},
}
expectEqual(t, fc, want, nil)
@ -630,6 +671,7 @@ func TestLBIntoAnnotation(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -640,6 +682,7 @@ func TestLBIntoAnnotation(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
}
// Create a service that we should manage, and check that the initial round
@ -689,10 +732,6 @@ func TestLBIntoAnnotation(t *testing.T) {
})
expectReconciled(t, sr, "default", "test")
want := &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -715,6 +754,7 @@ func TestLBIntoAnnotation(t *testing.T) {
},
},
},
Conditions: proxyCreatedCondition(clock),
},
}
expectEqual(t, fc, want, nil)
@ -740,10 +780,6 @@ func TestLBIntoAnnotation(t *testing.T) {
expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation)
want = &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -757,6 +793,9 @@ func TestLBIntoAnnotation(t *testing.T) {
ClusterIP: "10.20.30.40",
Type: corev1.ServiceTypeClusterIP,
},
Status: corev1.ServiceStatus{
Conditions: proxyCreatedCondition(clock),
},
}
expectEqual(t, fc, want, nil)
}
@ -768,6 +807,7 @@ func TestCustomHostname(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -778,6 +818,7 @@ func TestCustomHostname(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
}
// Create a service that we should manage, and check that the initial round
@ -817,10 +858,6 @@ func TestCustomHostname(t *testing.T) {
expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil)
expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation)
want := &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -835,6 +872,9 @@ func TestCustomHostname(t *testing.T) {
ClusterIP: "10.20.30.40",
Type: corev1.ServiceTypeClusterIP,
},
Status: corev1.ServiceStatus{
Conditions: proxyCreatedCondition(clock),
},
}
expectEqual(t, fc, want, nil)
@ -854,10 +894,6 @@ func TestCustomHostname(t *testing.T) {
expectMissing[corev1.Service](t, fc, "operator-ns", shortName)
expectMissing[corev1.Secret](t, fc, "operator-ns", fullName)
want = &corev1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
@ -881,6 +917,7 @@ func TestCustomPriorityClassName(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -892,6 +929,7 @@ func TestCustomPriorityClassName(t *testing.T) {
proxyPriorityClassName: "custom-priority-class-name",
},
logger: zl.Sugar(),
clock: clock,
}
// Create a service that we should manage, and check that the initial round
@ -954,6 +992,7 @@ func TestProxyClassForService(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -964,6 +1003,7 @@ func TestProxyClassForService(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
}
// 1. A new tailscale LoadBalancer Service is created without any
@ -1012,9 +1052,9 @@ func TestProxyClassForService(t *testing.T) {
// applied to the proxy resources.
mustUpdateStatus(t, fc, "", "custom-metadata", func(pc *tsapi.ProxyClass) {
pc.Status = tsapi.ProxyClassStatus{
Conditions: []tsapi.ConnectorCondition{{
Conditions: []metav1.Condition{{
Status: metav1.ConditionTrue,
Type: tsapi.ProxyClassready,
Type: string(tsapi.ProxyClassready),
ObservedGeneration: pc.Generation,
}}}
})
@ -1041,6 +1081,7 @@ func TestDefaultLoadBalancer(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -1051,6 +1092,7 @@ func TestDefaultLoadBalancer(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
isDefaultLoadBalancer: true,
}
@ -1095,6 +1137,7 @@ func TestProxyFirewallMode(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -1106,6 +1149,7 @@ func TestProxyFirewallMode(t *testing.T) {
tsFirewallMode: "nftables",
},
logger: zl.Sugar(),
clock: clock,
isDefaultLoadBalancer: true,
}
@ -1148,6 +1192,7 @@ func TestTailscaledConfigfileHash(t *testing.T) {
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -1158,6 +1203,7 @@ func TestTailscaledConfigfileHash(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
isDefaultLoadBalancer: true,
}
@ -1439,6 +1485,7 @@ func Test_externalNameService(t *testing.T) {
// 1. A External name Service that should be exposed via Tailscale gets
// created.
clock := tstest.NewClock(tstest.ClockOpts{})
sr := &ServiceReconciler{
Client: fc,
ssr: &tailscaleSTSReconciler{
@ -1449,6 +1496,7 @@ func Test_externalNameService(t *testing.T) {
proxyImage: "tailscale/tailscale",
},
logger: zl.Sugar(),
clock: clock,
}
// 1. Create an ExternalName Service that we should manage, and check that the initial round
@ -1504,3 +1552,18 @@ func toFQDN(t *testing.T, s string) dnsname.FQDN {
}
return fqdn
}
func proxyCreatedCondition(clock tstime.Clock) []metav1.Condition {
return []metav1.Condition{{
Type: string(tsapi.ProxyReady),
Status: metav1.ConditionTrue,
ObservedGeneration: 0,
LastTransitionTime: conditionTime(clock),
Reason: reasonProxyCreated,
Message: reasonProxyCreated,
}}
}
func conditionTime(clock tstime.Clock) metav1.Time {
return metav1.NewTime(clock.Now().Truncate(time.Second))
}