Skip to content

Commit

Permalink
cmd/k8s-operator,ipn/conf.go: fix --accept-routes for proxies (#11453)
Browse files Browse the repository at this point in the history 
Fix a bug where all proxies got configured with --accept-routes set to true.
The bug was introduced in #11238.

Updates#cleanup

Signed-off-by: Irbe Krumina <irbe@tailscale.com>
  • Loading branch information
@irbekrm
irbekrm committed Mar 19, 2024
1 parent 7fe4cbb commit b0c3e6f
Show file tree
Hide file tree
Showing 7 changed files with 190 additions and 145 deletions.
40 changes: 14 additions & 26 deletions cmd/k8s-operator/connector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,38 +73,34 @@ func TestConnector(t *testing.T) {
hostname: "test-connector",
isExitNode: true,
subnetRoutes: "10.40.0.0/14",
confFileHash: "9321660203effb80983eaecc7b5ac5a8c53934926f46e895b9fe295dcfc5a904",
}
expectEqual(t, fc, expectedSecret(t, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSecret(t, opts), nil)
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// Add another route to be advertised.
mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) {
conn.Spec.SubnetRouter.AdvertiseRoutes = []tsapi.Route{"10.40.0.0/14", "10.44.0.0/20"}
})
opts.subnetRoutes = "10.40.0.0/14,10.44.0.0/20"
opts.confFileHash = "fb6c4daf67425f983985750cd8d6f2beae77e614fcb34176604571f5623d6862"
expectReconciled(t, cr, "", "test")

expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// Remove a route.
mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) {
conn.Spec.SubnetRouter.AdvertiseRoutes = []tsapi.Route{"10.44.0.0/20"}
})
opts.subnetRoutes = "10.44.0.0/20"
opts.confFileHash = "bacba177bcfe3849065cf6fee53d658a9bb4144197ac5b861727d69ea99742bb"
expectReconciled(t, cr, "", "test")
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// Remove the subnet router.
mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) {
conn.Spec.SubnetRouter = nil
})
opts.subnetRoutes = ""
opts.confFileHash = "7c421a99128eb80e79a285a82702f19f8f720615542a15bd794858a6275d8079"
expectReconciled(t, cr, "", "test")
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// Re-add the subnet router.
mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) {
Expand All @@ -113,9 +109,8 @@ func TestConnector(t *testing.T) {
}
})
opts.subnetRoutes = "10.44.0.0/20"
opts.confFileHash = "bacba177bcfe3849065cf6fee53d658a9bb4144197ac5b861727d69ea99742bb"
expectReconciled(t, cr, "", "test")
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// Delete the Connector.
if err = fc.Delete(context.Background(), cn); err != nil {
Expand Down Expand Up @@ -156,19 +151,17 @@ func TestConnector(t *testing.T) {
parentType: "connector",
subnetRoutes: "10.40.0.0/14",
hostname: "test-connector",
confFileHash: "57d922331890c9b1c8c6ae664394cb254334c551d9cd9db14537b5d9da9fb17e",
}
expectEqual(t, fc, expectedSecret(t, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSecret(t, opts), nil)
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// Add an exit node.
mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) {
conn.Spec.ExitNode = true
})
opts.isExitNode = true
opts.confFileHash = "1499b591fd97a50f0330db6ec09979792c49890cf31f5da5bb6a3f50dba1e77a"
expectReconciled(t, cr, "", "test")
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// Delete the Connector.
if err = fc.Delete(context.Background(), cn); err != nil {
Expand Down Expand Up @@ -243,10 +236,9 @@ func TestConnectorWithProxyClass(t *testing.T) {
hostname: "test-connector",
isExitNode: true,
subnetRoutes: "10.40.0.0/14",
confFileHash: "9321660203effb80983eaecc7b5ac5a8c53934926f46e895b9fe295dcfc5a904",
}
expectEqual(t, fc, expectedSecret(t, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSecret(t, opts), nil)
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// 2. Update Connector to specify a ProxyClass. ProxyClass is not yet
// ready, so its configuration is NOT applied to the Connector
Expand All @@ -255,7 +247,7 @@ func TestConnectorWithProxyClass(t *testing.T) {
conn.Spec.ProxyClass = "custom-metadata"
})
expectReconciled(t, cr, "", "test")
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// 3. ProxyClass is set to Ready by proxy-class reconciler. Connector
// get reconciled and configuration from the ProxyClass is applied to
Expand All @@ -269,12 +261,8 @@ func TestConnectorWithProxyClass(t *testing.T) {
}}}
})
opts.proxyClass = pc.Name
// We lose the auth key on second reconcile, because in code it's set to
// StringData, but is actually read from Data. This works with a real
// API server, but not with our test setup here.
opts.confFileHash = "1499b591fd97a50f0330db6ec09979792c49890cf31f5da5bb6a3f50dba1e77a"
expectReconciled(t, cr, "", "test")
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// 4. Connector.spec.proxyClass field is unset, Connector gets
// reconciled and configuration from the ProxyClass is removed from the
Expand All @@ -284,5 +272,5 @@ func TestConnectorWithProxyClass(t *testing.T) {
})
opts.proxyClass = ""
expectReconciled(t, cr, "", "test")
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)
}
50 changes: 21 additions & 29 deletions cmd/k8s-operator/ingress_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,21 @@ func TestTailscaleIngress(t *testing.T) {

fullName, shortName := findGenName(t, fc, "default", "test", "ingress")
opts := configOpts{
stsName: shortName,
secretName: fullName,
namespace: "default",
parentType: "ingress",
hostname: "default-test",
confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549",
stsName: shortName,
secretName: fullName,
namespace: "default",
parentType: "ingress",
hostname: "default-test",
}
serveConfig := &ipn.ServeConfig{
TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}},
Web: map[ipn.HostPort]*ipn.WebServerConfig{"${TS_CERT_DOMAIN}:443": {Handlers: map[string]*ipn.HTTPHandler{"/": {Proxy: "http://1.2.3.4:8080/"}}}},
}
opts.serveConfig = serveConfig

expectEqual(t, fc, expectedSecret(t, opts))
expectEqual(t, fc, expectedHeadlessService(shortName, "ingress"))
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts))
expectEqual(t, fc, expectedSecret(t, opts), nil)
expectEqual(t, fc, expectedHeadlessService(shortName, "ingress"), nil)
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation)

// 2. Ingress status gets updated with ingress proxy's MagicDNS name
// once that becomes available.
Expand All @@ -118,19 +117,16 @@ func TestTailscaleIngress(t *testing.T) {
{Hostname: "foo.tailnetxyz.ts.net", Ports: []networkingv1.IngressPortStatus{{Port: 443, Protocol: "TCP"}}},
},
}
expectEqual(t, fc, ing)
expectEqual(t, fc, ing, nil)

// 3. Resources get created for Ingress that should allow forwarding
// cluster traffic
mustUpdate(t, fc, "default", "test", func(ing *networkingv1.Ingress) {
mak.Set(&ing.ObjectMeta.Annotations, AnnotationExperimentalForwardClusterTrafficViaL7IngresProxy, "true")
})
opts.shouldEnableForwardingClusterTrafficViaIngress = true
// configfile hash changed at this point in test env only because we
// lost auth key due to how changes are applied in test client.
opts.confFileHash = "fb9006e30ecda75e88c29dcd0ca2dd28a2ae964d001c66e1be3efe159cc3821d"
expectReconciled(t, ingR, "default", "test")
expectEqual(t, fc, expectedSTS(t, fc, opts))
expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation)

// 4. Resources get cleaned up when Ingress class is unset
mustUpdate(t, fc, "default", "test", func(ing *networkingv1.Ingress) {
Expand Down Expand Up @@ -223,30 +219,29 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) {

fullName, shortName := findGenName(t, fc, "default", "test", "ingress")
opts := configOpts{
stsName: shortName,
secretName: fullName,
namespace: "default",
parentType: "ingress",
hostname: "default-test",
confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549",
stsName: shortName,
secretName: fullName,
namespace: "default",
parentType: "ingress",
hostname: "default-test",
}
serveConfig := &ipn.ServeConfig{
TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}},
Web: map[ipn.HostPort]*ipn.WebServerConfig{"${TS_CERT_DOMAIN}:443": {Handlers: map[string]*ipn.HTTPHandler{"/": {Proxy: "http://1.2.3.4:8080/"}}}},
}
opts.serveConfig = serveConfig

expectEqual(t, fc, expectedSecret(t, opts))
expectEqual(t, fc, expectedHeadlessService(shortName, "ingress"))
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts))
expectEqual(t, fc, expectedSecret(t, opts), nil)
expectEqual(t, fc, expectedHeadlessService(shortName, "ingress"), nil)
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation)

// 2. Ingress is updated to specify a ProxyClass, ProxyClass is not yet
// ready, so proxy resource configuration does not change.
mustUpdate(t, fc, "default", "test", func(ing *networkingv1.Ingress) {
mak.Set(&ing.ObjectMeta.Labels, LabelProxyClass, "custom-metadata")
})
expectReconciled(t, ingR, "default", "test")
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts))
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation)

// 3. ProxyClass is set to Ready by proxy-class reconciler. Ingress get
// reconciled and configuration from the ProxyClass is applied to the
Expand All @@ -261,10 +256,7 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) {
})
expectReconciled(t, ingR, "default", "test")
opts.proxyClass = pc.Name
// configfile hash changed at this point in test env only because we
// lost auth key due to how changes are applied in test client.
opts.confFileHash = "fb9006e30ecda75e88c29dcd0ca2dd28a2ae964d001c66e1be3efe159cc3821d"
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts))
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation)

// 4. tailscale.com/proxy-class label is removed from the Ingress, the
// Ingress gets reconciled and the custom ProxyClass configuration is
Expand All @@ -274,5 +266,5 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) {
})
expectReconciled(t, ingR, "default", "test")
opts.proxyClass = ""
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts))
expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation)
}

0 comments on commit b0c3e6f

Please sign in to comment.