The e2e suite covered the operator's in-process API server proxy but
not the ProxyGroup-based one. Add tests for both proxy modes that
drive a ConfigMap through its lifecycle via the proxy, verify a
forbidden request is rejected, and check that deleting the ProxyGroup
cleans up its StatefulSet and Tailscale Service.
Fixestailscale/corp#38009
Change-Id: Ifc0be47ce32dd96f8daa748af6785a8b82ec19a7
Signed-off-by: David Bond <davidsbond93@gmail.com>
Clients can advertise an opaque app name in their ClientInfo but the
server previously did nothing with it.
Constrain app names to at most 32 bytes of printable ASCII, enforced
both in derp.NewClient and by the server when it parses the ClientInfo.
Extend the peerPresent frame, following its existing pattern of
appending optional fields, with a length-prefixed app name after the
flags byte, so trusted mesh watchers (other DERP nodes and stats
tools) can attribute connections by app. Old clients ignore the extra
bytes; old servers send frames without them.
Also add a derper --disallow-app-names flag taking a comma-separated
list of app names whose connections are refused, except for trusted
mesh peers.
Updates tailscale/corp#24454
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I6e721258675145833aafa1355fabf7fc05a5a204
The set-config loop uses uint16 endpoints. When Last is 65535, the increment wraps to zero and file or Unix targets continue indefinitely.
Break after applying Last so every closed range terminates without changing ordinary range behavior.
Fixes#20873
Signed-off-by: Bonobo <github@in9.at>
The service-pg-reconciler reconciles Services annotated for an ingress
ProxyGroup, but its ProxyGroup watch reused ingressProxyGroupFilter,
which is ingressesFromIngressProxyGroup. That handler lists Ingresses
and returns Ingress keys, so when a ProxyGroup became Available the
requests it produced never matched a Service and the reconciler's Get
just came back NotFound.
This fixes this by adding servicesFromIngressProxyGroup, which lists the
Services indexed for the ProxyGroup and returns their keys, matching what
the egress path already does with egressSvcsFromEgressProxyGroup.
Fixes#20944
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Also reports when shard results are missing, unreadable, duplicated, or
empty, so the pass count and percentage can't overstate what actually ran.
Updates #20931
Signed-off-by: Yaruk Asghar <yaruk@tailscale.com>
Guard optional annotations maps in generated CEL expressions before
looking up expose or proxy-group annotations. This lets ordinary
annotationless Services and Ingresses pass admission while preserving
deny-all and allowlist behavior for present proxy-group annotations.
RELNOTE: Kubernetes ProxyGroupPolicy now permits annotationless resources.
Fixes#20906
Change-Id: I8b9475d34c003ca6d233246c021ac656e0530fb1
Signed-off-by: joshrzemien <joshua.k.rzemien@gmail.com>
Historically, when DERP regions were switched away from strings to
numeric identifiers in PR #14641, tailcfg.Node.HomeDERP was declared
as an int instead of its own type.
This PR declares a new tailcfg.DERPRegionID type, represented by an
int64, and converts the following fields to use this type:
- netcheck.Report.PreferredDERP
- netcheck.Report.RegionLatency
- netcheck.Report.RegionV4Latency
- netcheck.Report.RegionV6Latency
- tailcfg.DERPHomeParams.RegionScore
- tailcfg.DERPMap.Regions
- tailcfg.DERPNode.RegionID
- tailcfg.DERPRegion.RegionID
- tailcfg.NetInfo.PreferredDERP
- tailcfg.Node.HomeDERP
- tailcfg.PeerChange.DERPRegion
- tailcfg.PingResponse.DERPRegionID
Note that the original field was an int, while the new field is backed
by an int64. This change makes DERPRegionID the same size on both
32-bit and 64-bit architectures.
Fixes: #20165
Change-Id: Ic6f795a6d791dd16f756f246d5a02085443e212f
Signed-off-by: Simon Law <sfllaw@tailscale.com>
Let clients pin favorite devices, exit nodes, and services so GUIs can
surface & change them. Pins are stored per login profile in the new
favorites feature module, keyed per category; devices and exit nodes by
StableNodeID, services by ServiceName.
The item types live in a leaf package feature/favorites/pintype,
keeping them out of the core ipn hierarchy. Each category has
its own type (pintype.Device, pintype.ExitNode, pintype.Service).
Exposed over LocalAPI at GET/POST /localapi/v0/pins, where POST replaces
only the categories named in the request so a client can update one
category without clobbering the others. Pins are local to the device and
are not synced across a user's devices.
updates tailscale/corp#44836
Signed-off-by: Will Hannah <willh@tailscale.com>
Co-authored-by: Will Hannah <wph@Wills-Virtual-Machine.local>
containerboot waits up to 60s for the initial map response before
failing. Slow map responses (>60s seen in production) leaves
containerboot to timeout and fail.
Add a TS_BOOT_TIMEOUT env var to override the default. Falls back to
60s when unset.
Fixes#20912
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
In cases where cigocacher is using a proxy that handles the auth, it's
possible to fetch stats without knowing our own access token. Don't fail
early if the access token isn't passed. If there's no proxy, it will
fail with the error from the gocached server.
Updates tailscale/corp#45427
Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>
Deploy k8s nameserver during e2e test setup, and point the cluster resolver
(CoreDNS or kube-dns) to it so tests can resolve MagicDNS names inside the
cluster.
Add a test to verify singleton L7 Ingress is reachable from inside the
cluster using its MagicDNS Name.
Update existing egress tests to use a dedicated tailnet target per test (to
avoid conflicts). Egress tests now also verify that an egress target is
reachable from within the cluster using both its Service and MagicDNS name.
To successfully curl using the target's MagicDNS name, publish test CAs as a
ConfigMap to the cluster, and mount these for each curl pod.
Fixestailscale/corp#38027
Signed-off-by: Becky Pauley <becky@tailscale.com>
kubeconfigPath dereferences FileInfo for non-ENOENT stat errors even though os.Stat returns nil FileInfo. Preserve the failing list entry so checkKubeconfigWritable reports the existing access error instead of panicking.
Updates #11604
Signed-off-by: Bonobo <github@in9.at>
Static size checks (iossize) catch binary dirty-page growth but nothing
covered runtime heap cost, which is what actually consumes the iOS
Network Extension's 50 MiB jetsam budget. Bring up a tsnet backend
(with the full condregister feature set, matching shipping clients)
against an in-process testcontrol server and assert live post-GC heap
budgets for (a) backend startup with zero peers and (b) marginal cost
per netmap peer.
The startup test measures 1.3 MiB today and fails loudly on the
conn25 flow-table pre-allocation regression (17 MiB) that jetsam-killed
the iOS extension on large tailnets.
Budgets are deliberately generous (6-12x current measurements) to stay
flake-free while still catching the multi-MiB regressions that matter
for mobile.
A new debugknob enables us to constrain the GSO/GRO batch size to 1 for
these tests so as to avoid the memory allocation associated with those
buffers, which are a known issue with their own work stream.
Updates tailscale/corp#46408
Updates tailscale/corp#18514
Signed-off-by: James Tucker <james@tailscale.com>
This commit modifies the "generate" tool we use on the kubernetes operator
that produces helm chart and static manifest assets for CRDs.
Previously, this required always remembering to add new constants to
a `main.go` and did not have any mechanism to fail in CI if you forgot
to. Now this tool will iterate over all the CRDs and ensures that they're
in the places they're expected to be, with a test that will fail if they
are not.
This removes the requirement for remembering to add these constants
every time you have a new CRD.
Closes: #20594
Signed-off-by: David Bond <davidsbond93@gmail.com>
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
This commit changes how PeerRelay services are exposed on AWS. A Network
Load Balancer only forwards to targets in an availability zone enabled
on it, and spec.aws.elasticIPs pins each service to a single subnet,
which enables just one zone. A replica scheduled anywhere else silently
receives nothing while still reporting PeerRelayReady with an address in
status.endpoints.
Without spec.aws we now leave the subnet unpinned, so the AWS Load
Balancer Controller spreads the load balancer over every zone it finds,
and cross-zone load balancing is on by default so any of its addresses
reach the pod. Hostname resolution is no longer gated on the
eip-allocations annotation, which had left these unpinned services in
EndpointsPending forever, and a failure to resolve now logs at debug
since it is expected while a load balancer provisions.
Such a load balancer has an address per zone, and AWS bills for each, so
every one of them is now advertised rather than only the lowest sorted.
That also lets a peer reach the relay when one zone is unreachable.
status.endpoints gains address as a second list map key so a replica can
hold an entry per address; no field changes, so existing readers of
endpoints[].address keep working. Readiness counts the replicas that
have an endpoint rather than the entries, so a replica with several
addresses cannot mask one that has none.
The pods now serve containerboot's health check endpoint and the load
balancer is pointed at it over HTTP. A peer relay listens only on UDP,
so the default TCP check against the port the load balancer forwards
could never succeed and every target sat unhealthy while relaying
perfectly well. /healthz reports 200 once the device has tailnet
addresses, which is the condition that actually matters.
The CRD docs now describe spec.aws as the exception, note that it also
needs a ProxyClass pinning pods to the zone of the subnets it names, and
drop the claim that an Elastic IP has an availability zone of its own.
Fixes: https://github.com/tailscale/tailscale/issues/20833
Signed-off-by: David Bond <davidsbond93@gmail.com>
The egressPodsReconciler (added in #14792) only sets the
tailscale.com/egress-services readiness condition on egress ProxyGroup
replica Pods that declare the corresponding readiness gate. However, the
gate was never actually added to the egress Pod template, so the
reconciler always hit its early-return and the readiness condition was
never set. This commit adds said readiness gate to the egress Pod template.
Updates #14326
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
ProxyGroup, Recorder, and PeerRelay each carried a near-identical copy of
the auth key re-issuance state machine (in-flight tracking, per-parent rate
limiting, stale-device cleanup). A bug fix had to land in three places and
could silently drift.
Extract it into a shared tailscaled.Reissuer, alongside the other tailscaled
workload helpers (NewAuthKey, AuthKeyFromConfigSecret, DeviceIDFromStateSecret)
that the callers already use. It owns its own mutex, tracks in-flight reissues
per replica keyed by parent, and rate-limits re-issuance per parent; the three
reconcilers drive it via EnsureState/RemoveState/ShouldReissue. The device
deletion helper is shared too, so the reissue state machine and its tests now
live in one place.
Updates #20544
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
This test was the repo's slowest at 60 seconds of wall time, all of it
spent sleeping: three of its subtests reach the unconditional 20 second
failover wait in kube/services.EnsureServicesAdvertised, despite using
a pure in-memory FakeLocalClient with no real control or I/O.
Run each subtest in a testing/synctest bubble so the wait elapses on
the fake clock instead. The test now completes in milliseconds.
Fixes#20792
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I3442815f7efcf6de740f893197ee0461ab049bb2
Fix the small number of existing violations of this check, and enable it for
future runs. The fixes needed were:
- Clean up a few misspelled package names (probably renames).
- Clean up a few lexical nits ("Package x" instead of "The x package").
- Add lint directives to some files affected by build tag variance.
- Add a missing package comment and re-generate the k8s docs.
The lint overrides are a little ugly, but there are only a few places where we
need them, and it's probably worthwhile to enable the check on the rest of the
repo. Rather than replicate the docs around the build tag, I made the lint
diagnotics reference the "correct" file.
Updates #cleanup
Change-Id: I0d97f2f468542af456a0396cf9a023f04f23e436
Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
The json/v2 prototype used to support a `format` tag option,
which has been removed for the initial release of json/v2 in Go 1.27.
The wrapper types in this package provide a way to avoid using
the `format` tag option for all existing use-cases.
The types are written to cooperate with other tag options
such as `string`, which may stringify JSON numbers.
We adjust cmd/vet/jsontags accordingly.
Updates #20220
Updates tailscale/corp#45953
Change-Id: Ie1fcea41dc30983e9acc43085f42a6e8ee49d26e
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Package tailcfg defines the types and constants used by the Tailscale
protocol, but since everything is all in one package, it’s difficult
to sift through the docs: https://pkg.go.dev/tailscale.com/tailcfg
We define and enumerate capabilities as string constants for
tailcfg.NodeCapability and tailcfg.PeerCapability. This PR extracts
them into their own packages:
- tailcfg.CapabilityFileSharing becomes nodecap.FileSharing
- tailcfg.NodeAttrOnlyTCP443 becomes nodecap.OnlyTCP443
- tailcfg.PeerCapabilityTaildrive becomes peercap.Taildrive
We originally intended for CapabilityFoo to grant an entitlement or
permission for Foo, and for NodeAttrBar to configure Bar in the
nodeAttrs section of the policy file. However, there was no technical
enforcement of this convention, so new capabilities have used the
NodeAttr prefix regardless of meaning. Therefore, this PR unifies
tailcfg.CapabilityFoo and tailcfg.NodeAttrBar into a single package as
nodecap.Foo and nodecap.Bar.
Ran `go fix -inline ./...` and committed the changes that replaced
uses of the tailcfg aliases with the authoritative ones.
Updates #20259
Change-Id: Ieb7e7e6c8247c39faf42fdf15c68cdc7c621c730
Signed-off-by: Simon Law <sfllaw@tailscale.com>
The sysctler init container shells out to sysctl to turn on IP
forwarding for non-userspace proxies. That binary ships in the
procps-ng package, which Alpine has but Red Hat's UBI does not, so
on UBI the init container exits 127 and every proxy Pod is stuck in
PodInitializing and never registers a device.
Updates: https://github.com/tailscale/corp/issues/45981
Updates: https://github.com/tailscale/corp/issues/44443
Signed-off-by: David Bond <davidsbond93@gmail.com>
This commit adds a new `--base-image` flag to the e2e test suite
so that tests can build the operator and operator accessories
with a different base docker image. We want this so that we can
try things out with red hat's UBI as part of getting the operator
up and running on openshift clusters.
We can then modify the e2e test runner to use a matrix for normal
alpine base and redhat's.
Updates: https://github.com/tailscale/corp/issues/45981
Updates: https://github.com/tailscale/corp/issues/44443
Signed-off-by: David Bond <davidsbond93@gmail.com>
The Helm chart hardcoded the operator's Deployment, ServiceAccount,
Role, and RoleBinding names to "operator".
Adds a standard _helpers.tpl with name/fullname template functions and
use them for the operator's resources, including the oauth Secret and
the cluster-scoped ClusterRole/ClusterRoleBinding. When neither
nameOverride nor fullnameOverride is set, the fullname resolves to the
historical hardcoded names rather than the release name, so existing
installations upgrade with no resource renames; rendering with default
values is byte-identical to before.
Fixestailscale/tailscale#18232
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
The nameserver Deployment created for a DNSConfig was the only
operator-managed workload with no way to configure imagePullSecrets, so its
pods could not pull the nameserver image from a private registry.
Adds imagePullSecrets to NameserverPod and thread it through to the nameserver
Deployment pod spec.
Updates #16772
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
The Helm chart only applied imagePullSecrets to the operator Deployment's
pod spec, so proxy pods (which use a different ServiceAccount) never
received them and could not pull images from private registries.
Apply imagePullSecrets to the operator, proxies, and kube-apiserver-auth-proxy
ServiceAccounts.
Updates #16772
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
This commit adds a new end-to-end test for the `PeerRelay` custom
resource.
This test is currently quite limited due to the fact that we are running
our tests within a kind cluster within github actions. This means it's
not really possible to give the peer relays a proper public IP address
via the `LoadBalancer` type services that we spin up.
That being said, we intend to expand our e2e test suite with actual
real clusters in future so this can be expanded upon at a later date.
For now, this test spins up a single and multi-replica deployment of
a peer relay and confirms that it has been registered with control
and is configured to act as a peer relay.
This test also caught a small bug where the server URL was not being
passed into the peer relay's configuration, which has been fixed here.
Closes: https://github.com/tailscale/corp/issues/45731
Closes: https://github.com/tailscale/corp/issues/45737
Signed-off-by: David Bond <davidsbond93@gmail.com>
PR #20388 (commit ca9f6971e) bumped k8s.io/* from v0.34 to v0.35 as a
transitive effect of pulling in helm v3.21 (for containerd CVE fixes).
The old controller-runtime v0.19.4 is aligned with k8s 1.31 APIs and
regresses at runtime against v0.35 client-go (informer reflectors stall
on Watch responses), leaving the operator unable to reconcile.
v0.23.3 is the release aligned with k8s.io/* v0.35 / Kubernetes 1.35.
The operator uses a narrow slice of controller-runtime (manager +
builder + client + reconcile + handler + source), so this bump is a
lift-and-shift: no reconciler wiring changes needed.
Test scaffolding fix: controller-runtime v0.20+ populates TypeMeta
(Kind/APIVersion) on objects returned by the fake client. The shared
expectEqual helper now strips TypeMeta before diffing so existing
tests continue to work without needing every 'want' object updated.
Tested: deployed to live clusters across Kubernetes 1.33, 1.34, 1.35,
and 1.36 (identical build per cluster). Verified the operator starts and
reconciles cleanly on all versions with no RBAC/forbidden errors, CRDs
establish and enforce validation, and the field indexers resolve
correctly under annotation churn (no stale-index misrouting). Exercised
the Service, ProxyClass, Connector, egress (simple + ProxyGroup, incl.
the EndpointSlice reconciler), and HA ingress-for-pg reconcilers.
Confirmed informer/watch recovery after an API server restart forces a
re-list/re-watch (the exact regression this bump fixes).
Updates tailscale/corp#44898
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Go 1.27 requires this new v0.8.0-rc.1.
But staticcheck 0.8's SA4023 gets stricter and points out that
modifiedExternallyError and handleListenersAccept always return
non-nil errors, and that MonitorHealth's callers don't need a separate
nil check before errors.Is. Simplify all three call sites; no behavior
change.
But then a handful of other places that SA4023 is angry about are
wrong (because it's not considering build tags) and can't be addressed
by ignore directives (again not considering build tags), so we just
disable SA4023 for now.
Updates #20220
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I2fefe3b986b5798c2e01624a0e9820839d21a569
This patch adds examples of unmarshalling tslockjsonv1.LogResponse
and tslockjsonv1.StatusResponse to the documentation.
Updates #17613
Signed-off-by: Simon Law <sfllaw@tailscale.com>
This patch pulls the printing and JSON-encoding out of
feature/tailnetlock/tslockjsonv1 into their callers, so that this
package only handles type conversions.
In cmd/tailscale/cli/tailnet-lock.go, it extracts the
printTailnetLockStatus function from runTailnetLockStatus to mirror
printTailnetLockLog and runTailnetLockLog.
Updates #17613
Signed-off-by: Simon Law <sfllaw@tailscale.com>
This patch extracts the functions used to marshal the JSON output of
the `tailscale lock` subcommand.
Updates #17613
Signed-off-by: Simon Law <sfllaw@tailscale.com>
This patch exports the structs used to unmarshal the JSON output of
the `tailscale lock` subcommand.
Updates #17613
Change-Id: I615723ff9dbc631e452dbf6bbdeab7b97e96dbb3
Signed-off-by: Simon Law <sfllaw@tailscale.com>
This patch extracts the handling of the TKA types for the
`tailscale lock` subcommand into its own tka.go file.
Updates #17613
Signed-off-by: Simon Law <sfllaw@tailscale.com>
This patch extracts the JSON handling for the `tailscale lock`
subcommand from the jsonoutput package into its own tslockjsonv1
package.
Updates #17613
Signed-off-by: Simon Law <sfllaw@tailscale.com>
This commit moves the proxyclass reconciliation logic from its original
home in the rather large main package of the k8s-operator to a dedicated
package within `k8s-operator/reconciler/proxyclass`.
Closes: https://github.com/tailscale/corp/issues/37085
Signed-off-by: David Bond <davidsbond93@gmail.com>
By default the controlhttp dialer tries the plaintext port 80 upgrade
path first with a port 443 TLS fallback, so it's not deterministic
which of the two server code paths a debugging session exercises. Add
a --force-port flag to pin the noise connection to one or the other:
443 uses the dialer's existing TS_FORCE_NOISE_443 knob, and both
values are also enforced at the dial layer so the other port can't be
used by a silent fallback.
Updates tailscale/corp#29053
Change-Id: I5641db9bd2083d458ba7e0e766c7f9da535ba3e0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Add a flag to dial a specific IP for the noise connection while
keeping the --host value for the /key fetch, TLS SNI, and Host
header. This permits testing a single control plane frontend (such
as a staging or canary trunkd instance with no DNS record) end to
end with a real ts2021 upgrade and noise handshake.
Updates tailscale/corp#29053
Change-Id: I8cd0208d6b01219eabd33f8fdd77cc490fffe29e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
There are two places in the code where we need to compare the
latencies of a netcheck report. In both cases, the comparison wasn’t
well tested.
This PR extracts that logic into a Compare method of the new
RegionLatency type. This type wraps the map of latency measurements
keyed by region ID.
Updates #cleanup
Change-Id: I7f248988973007c2f452283c1b82f84b03068f77
Signed-off-by: Simon Law <sfllaw@tailscale.com>
When running k8s operator e2e tests against real tailnets, use Let's
Encrypt's staging environment to avoid production rate limits.
Updates tailscale/corp#45571
Signed-off-by: Becky Pauley <becky@tailscale.com>
This commit adds 3 end-to-end tests to the suite for the
operator's `Connector` resources. Connectors are used
to deploy subnet routers, exit nodes & app connectors
within a Kubernetes cluster.
One test exists for each respective mode of the resource
and assertions are made against control that the machine
is running in the expected mode using the device identifier
within the pod's state secret.
Closes: https://github.com/tailscale/corp/issues/38025
Signed-off-by: David Bond <davidsbond93@gmail.com>
Add end-to-end tests for egress to a tailnet target via its FQDN, IPv4 and
IPv6 addresses.
Update the kind cluster used in tests to support dual-stack (to allow
testing egress to both IPv4 and IPv6 tailnet targets). For pre-existing
clusters, detect on test setup which IP families the cluster supports.
IPv4/IPv6 subtests are skipped if not supported.
Run the end-to-end tests in parallel to improve the speed of test runs. To
enable this, run TestProxyGroupPolicy in a dedicated namespace so its
ValidatingAdmissionPolicies don't affect other parallel tests sharing the
same namespace.
Updates tailscale/corp#34834
Signed-off-by: Becky Pauley <becky@tailscale.com>
This patch extracts all the DNS related JSON handling from the
cmd/tailscale/jsonoutput package into a new tsdnsjsonv0 package.
It adds package documentation for tsdnsjsonv0 with a big WARNING that
this is an unstable format with no backwards compatibility guarantees.
When we stabilize this format, we should spin off a new tsdnsjsonv1
package that uses jsonoutput.ResponseEnvelope to declare version 1.
Updates #13326
Updates #18750
Signed-off-by: Simon Law <sfllaw@tailscale.com>
Flatten the cmd/tailscale package hierarchy by extracting the
jsonoutput package out of the cmd/tailscale/cli package.
Updates #cleanup
Change-Id: I92f80db75b0328e82f1596b6a42f6f6ef5a94bfa
Signed-off-by: Simon Law <sfllaw@tailscale.com>
Files whose names contain characters with Unicode decompositions (such
as umlauts or voiced kana) could not be opened or written over
Taildrive.
Background: keyboards and IMEs emit NFC (precomposed) characters on
every platform, so filenames on Linux (ext4 etc) and Windows (NTFS)
disks are usually NFC bytes. NFD (decomposed) names mostly come from
Apple software: HFS+ forced a variant of NFD on write, and Apple's
frameworks still decompose paths via fileSystemRepresentation. APFS
preserves whatever bytes it is given but does normalization-insensitive
lookups (it stores a hash of the normalized name), so canonically
equivalent names find the same file. ext4 and NTFS lookups, by
contrast, are byte-exact.
On the wire, the macOS WebDAV client sends paths in NFD form (they
pass through the decomposing file system representation, and unlike
Apple's NFS client there is no "nfc" mount option). Windows and Linux
WebDAV clients pass names through as the application provided them,
typically NFC. WebDAV itself mandates no normalization, and PROPFIND
hrefs reflect the server's on-disk bytes.
The two forms are canonically equivalent but byte-wise different, so a
macOS client requesting the NFD form of an NFC-named file on a Linux
or Windows host got a 404 from the exact-byte lookup. Even against an
APFS host, where the filesystem absorbs the mismatch, the client-side
StatCache could still infer a 404: a cached directory listing in one
form caused depth 0 PROPFINDs in the other form to be treated as not
found without ever reaching the server. The inverse direction (NFD
bytes on a Linux disk, copied there from a Mac, requested in NFC form
by a Windows or Linux client) was broken too.
Alternative regimes considered: normalizing names at storage time (as
Nextcloud and Syncthing's autoNormalize do) would rename user files in
shared directories as a side effect of serving them; normalizing
request paths to a fixed form on the wire is unsound because the
on-disk form is unknowable a priori (ext4 can hold either form, or
both). Instead, adopt the APFS model: preserve bytes, but make lookups
normalization-insensitive.
Concretely, wrap the remote file server's webdav.Dir in a
normalizingFS that, when an exact path lookup fails, rescans the
parent directory for an entry whose name is canonically equivalent,
comparing the NFC form of both sides (which also sidesteps Apple's
nonstandard decomposition tables). Exact matches always win, and newly
created files keep the exact bytes the client sent. Also NFC-normalize
StatCache keys so canonically equivalent names share a cache entry.
The change is covered at three levels: unit tests for the StatCache,
an in-process two-node test in drive/driveimpl, and a new TestTaildrive
VM integration test in tstest/natlab/vmtest that shares a directory
between two Ubuntu VMs and exercises the NFC/NFD cases over the real
stack with curl playing the part of a macOS WebDAV client.
Fixes#15020
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I9c2f157e604efc629828581e08d5b3191dbb7d4e
TestContainerBoot/kube_shutdown_during_state_write flaked with exit
code 1 instead of 0 when SIGTERM arrived while "tailscale up" was
still running. Two problems combined:
tailscaleUp and tailscaleSet wrapped errors with %v, flattening the
error chain, so main's errors.Is(err, context.Canceled) check could
not recognize a graceful shutdown.
Even with %w, cmd.Run under a canceled context usually reports the
death of the killed subprocess ("signal: killed") rather than the
context error that caused it, since Wait prefers the process error.
Check ctx.Err() explicitly and return it (wrapped with %w) so that
a shutdown-driven cancellation is recognized wherever it lands
relative to the subprocess lifetime.
Before: the exit-code failure reproduced 4 times in 808 stress runs
under CPU starvation. After: 0 in 1195 runs.
Fixes#19380
Change-Id: Ie15ca722d2d5ac2a3f79b2d0ab01fb71d4b9220d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
David Bond