Skip to content

fix: pass --statedir alongside --state= for SSM-backed state - #102

Open
dadiv wants to merge 3 commits into
masterpointio:mainfrom
soomo:fix/statedir-with-portable-state
Open

fix: pass --statedir alongside --state= for SSM-backed state#102
dadiv wants to merge 3 commits into
masterpointio:mainfrom
soomo:fix/statedir-with-portable-state

Conversation

@dadiv

@dadiv dadiv commented Jun 2, 2026

Copy link
Copy Markdown

what

  • When ssm_state_enabled = true, the module now also passes --statedir=/var/lib/tailscale to tailscaled alongside the existing --state=arn:aws:ssm:… flag.
  • No new variables, no breaking changes — the flag is injected automatically and only when the SSM-backed portable state store is in use.
  • Callers who already pass their own --statedir via var.tailscaled_extra_flags are unaffected (Go's flag.Parse is last-wins).

why

  • Tailscale's --state= accepts portable URLs (arn:…, kube:…, mem:) that can hold the daemon's persistent prefs/keys outside the local filesystem — which is exactly what ssm_state_enabled opts callers into so node identity survives ASG instance replacement.
  • However, when --state= points at a portable store, tailscaled refuses to use the local filesystem as its "var root" for sidecar files unless --statedir is given explicitly. Without it, several features silently break:
    • The in-process Tailscale SSH server can't persist SSH host keys, so it never comes up. The journal records:
      warning: unable to get SSH host keys, SSH will appear as disabled
        for this node: no var root for ssh keys
      
      The node's Hostinfo.SSH_HostKeys stays empty, the control plane reports the node as not-SSH-capable, and inbound ssh / tailscale ssh connections hang at the banner exchange (TCP handshake succeeds, no SSH banner is ever sent).
    • Taildrop is disabled (taildrop: no Taildrop directory configured).
    • TKA / network-lock state can't be read (cannot fetch existing TKA state; no state directory for network-lock).
    • Per-profile data storage is unavailable (profile data directory: profile local data storage unavailable).
  • This is silent — tailscale up --ssh succeeds, tailscale debug prefs shows RunSSH: true, and the daemon advertises the cap/ssh capability — but the SSH server is never actually serving. Every consumer of ssm_state_enabled is exposed to it.
  • The Tailscale rpm/deb package already declares StateDirectory=tailscale on its systemd unit (i.e. /var/lib/tailscale exists with the right ownership/permissions), so pinning --statedir to that path keeps the daemon aligned with the packaging defaults and re-enables SSH, taildrop, and TKA with no other configuration required.

references

Summary by CodeRabbit

Release Notes

  • Improvements
    • Enhanced SSH host key persistence configuration to ensure proper storage behavior when state management is enabled.

When ssm_state_enabled = true, the module appends --state=arn:aws:ssm:...
to tailscaled's flags so the daemon's state is persisted in SSM Parameter
Store across instance replacements. With Go's flag.Parse() last-wins
semantics, this --state=arn:... overrides the systemd unit's default
--state=/var/lib/tailscale/tailscaled.state, which is the intended
behavior.

However, when --state= points at a portable store (arn:..., kube:...,
mem:), tailscaled refuses to use the local filesystem as its 'var root'
for sidecar files unless --statedir is also given explicitly. Without
it, the in-process Tailscale SSH server can't persist host keys and
silently stays disabled, along with taildrop, TKA (network-lock), and
the per-profile cache. The journal reports:

  warning: unable to get SSH host keys, SSH will appear as disabled
  for this node: no var root for ssh keys
  taildrop: no Taildrop directory configured
  cannot fetch existing TKA state; no state directory for network-lock

The Tailscale rpm/deb already configures StateDirectory=tailscale on
the systemd unit (so /var/lib/tailscale exists with the right perms),
so pinning --statedir there preserves the package's expectations and
re-enables SSH/taildrop/TKA without any other changes.
@dadiv
dadiv requested a review from a team as a code owner June 2, 2026 15:52
@dadiv
dadiv requested a review from dudymas June 2, 2026 15:52
@dadiv dadiv changed the title fix: Pass --statedir alongside --state= for SSM-backed state fix: pass --statedir alongside --state= for SSM-backed state Jun 2, 2026
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR extends the Tailscale daemon configuration in main.tf by adding support for SSH host key persistence. A new local variable local.ssm_statedir_flag conditionally sets the --statedir=/var/lib/tailscale flag when var.ssm_state_enabled is true. The local.tailscaled_extra_flags variable is updated to combine both the --state and --statedir flags (when enabled) using compact() and join(). The comment block is expanded to document why both flags are required for SSH host key persistence.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • masterpointio/terraform-aws-tailscale#41: Directly modifies main.tf to extend Tailscale daemon flag construction when ssm_state_enabled is enabled, following the same pattern of updating state-related flag logic.

Suggested reviewers

  • natemccurdy
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: adding the --statedir flag alongside --state= for SSM-backed state configurations, which matches the core objective of the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@main.tf`:
- Line 23: The tailscaled_extra_flags currently concatenates
var.tailscaled_extra_flags before the module-injected local.ssm_state_flag and
local.ssm_statedir_flag which makes the module flags override caller-provided
--statedir (last-wins); change the concat order so the module flags come first
and var.tailscaled_extra_flags come last (i.e., concat([local.ssm_state_flag,
local.ssm_statedir_flag], var.tailscaled_extra_flags)) and keep the existing
compact(...) and join(...) so caller-supplied flags take precedence when
var.ssm_state_enabled is true.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c6d1b42c-80fa-43c7-b38c-0fe289e0c519

📥 Commits

Reviewing files that changed from the base of the PR and between 0223d7b and 72b35a8.

📒 Files selected for processing (1)
  • main.tf

Comment thread main.tf Outdated
dadiv added 2 commits June 2, 2026 10:55
Reorder tailscaled extra flags so caller-provided flags are applied
last, matching tailscaled's last-wins parsing behavior.

Add a lifecycle precondition to fail plans when
`ssm_state_enabled = true` and caller flags include `--state` or
`--statedir`, preventing silent misconfiguration of module-managed
SSM state.
@dadiv
dadiv requested a review from gberenice June 2, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants