[BUG] tailscale up fails when changing non-default flags

[fredrikekre]

Describe the bug
tailscale up fails after changing default arguments, such as e.g. --operator.

To Reproduce

1. Bring up tailscale with tailscale_args: "--operator={{ ansible_user_id }}" successfully
2. Remove the operator argument
3. Trying to execute the play again now fails with

   fatal: [hostname]: FAILED! => {
     "changed": false,
     "msg": [
       "Error: changing settings via 'tailscale up' requires mentioning all",
       "non-default flags. To proceed, either re-run your command with --reset or",
       "use the command below to explicitly mention the current value of",
       "all non-default settings:",
       "",
       "tailscale up --auth-key=<<REDACTED>> --timeout=2m0s --operator=ubuntu"
     ]
   }
   

Note that the builtin redacting of the auth key failed here. I redacted it myself for this issue.

Expected behavior

1. Would be nice if a change in arguments like this could be handled automatically.
2. The auth key should be redacted

Screenshots
If applicable, add screenshots to help explain your problem.

Target (please complete the following information):

• OS: Ubuntu 22
• Ansible version: 2.16.5
• artis3n.tailscale version: 4.4.4.
• Tailscale version (set verbose to true): Ver: 1.62.0-tdf4d4ebd4-gd0454003c

[fredrikekre]

The auth key should be redacted

This didn't happen because I use a node key without the tskey prefix (https://github.com/artis3n/ansible-role-tailscale/blob/7ebbe49a0413b8912c02bd9a81aa54a02538f6f5/tasks/install.yml#L170C65-L170C76). Such keys are accepted by headscale. Perhaps the regex can look for --auth-key\s*=\s*.*\s or something instead?

[artis3n]

Thanks for this issue and it's linked PR! I want to think about how best to account for this. Tailscale's default behavior is to fail instead of silently add --reset. Should this role change this default behavior? Should we provide a tailscale_reset or similar parameter on the role that conditionally includes --reset? Should we expect end users to do this themselves when passing in tailscale_args?

[fredrikekre]

I think one can argue for having different defaults for --reset in an interactive context (i.e. when you interact with the CLI yourself) and in a non-interactive context like when using this role. In an interacitve context it is easier to forget some (non-default) arguments compared to when you put the arguments in code (like with this role). When using this role for configuring tailscale I would expect that the configuration I put in my code is what ends up on the remote host no matter what previus configuration I used when deploying the last time.

Should we provide a tailscale_reset or similar parameter on the role that conditionally includes --reset?

This is what I initially did, but if one subscribes to the idea that the code should always override any existing state it doesn't hurt to always include --reset. I am happy to change my PR to make it configurable. I do think resetting by default make sense though, but at least there will be a way to opt out for anyone who doesn't like it. Should I update the PR to do this?

Should we expect end users to do this themselves when passing in tailscale_args?

This is what I do now as a workaround.

[artis3n]

I think that's a persuasive reason to --reset by default, but I want to think about it for a bit before merging. I have other breaking changes on my mind that would go into a new major version as well.

[artis3n]

@McSim85 Moving your comment here, you are saying you are in favor of this role applying --reset by default, or that doing so would introduce more unexpected issues for you?

#457 (comment)

[McSim85]

I'm sorry for not getting back to you sooner.
I will extend my previous comment:

Let me add my 2 cents.
I prefer to run the role with state: absent -> state: present, if cli parameters has changed.
That will bring less unexpected problems.

In most cases, Ansible inventory is automated. Inventory or role variables define server configuration. Ansible roles are expected to be idempotent.
So, changing tailscale cli parameters is not the thing that happens often.

In case your environment expects to change cli params by admins manually, there is a way to pass --reset flag by tailscale_args variable in your role:

- name: Servers
  hosts: all
  roles:
    - role: artis3n.tailscale
      vars:
        tailscale_args: "--reset"

I might be wrong, but the --reset param allows you to enforce the new tags (--advertise-tags=tag:XXX instead of previous --advertise-tags=tag:YYY) and this can assign the server unexpected permissions (if you use tags in ACLs).
Which should not happen by default.

To sum up, I would not add --reset option by default into the role.