Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persist tailscale state #15911

Closed
2 tasks done
delaman opened this issue Dec 9, 2023 · 28 comments
Closed
2 tasks done

Persist tailscale state #15911

delaman opened this issue Dec 9, 2023 · 28 comments
Labels
enhancement New feature or request
Milestone

Comments

@delaman
Copy link

delaman commented Dec 9, 2023

Is your feature request related to a problem?

The hostname I set does not persist. For example if I set the hostname to cool-app after the pod restarts for whatever reason the hostname is changed to to cool-app-1. The number increases every time.

Describe the solution you'd like

Add a tick box that adds the tailscale state to a kuberentes secret. tailscale kuberentes secret feature.

Describe alternatives you've considered

Mounting a path in the tailscale container. This option is not supported with truechart's vpn feature.

Additional context

No response

I've read and agree with the following

  • I've checked all open and closed issues and my request is not there.
  • I've checked all open and closed pull requests and my request is not there.
@delaman delaman added the enhancement New feature or request label Dec 9, 2023
@delaman
Copy link
Author

delaman commented Dec 12, 2023

Looks like 50%-ish of the work is there already.
Looks like all that is missing are the Kuberentes RBAC permissions.

@PrivatePuffin
Copy link
Member

Best to actually read the code if you link to it.

@delaman
Copy link
Author

delaman commented Dec 12, 2023

True that.

Couldnt we just create a new k8s RBAC role and bind the RBAC role to whatever kuberentes service account the app is using. Binding a new role shouldnt effect the app.

@stavros-k
Copy link
Collaborator

True that.

Couldnt we just create a new k8s RBAC role and bind the RBAC role to whatever kuberentes service account the app is using. Binding a new role shouldnt effect the app.

That means that every app that uses the tailscale sidecar will have access to that secret.
Do you trust all the apps with that?

@delaman
Copy link
Author

delaman commented Dec 12, 2023

Do you trust all the apps with that?

No I dont. Good point. What about a new service account just for tailscale sidecar?

@stavros-k
Copy link
Collaborator

Do you trust all the apps with that?

No I dont. Good point. What about a new service account just for tailscale sidecar?

SA is bound to the pod.
Tailscale is just another container in the same pod as the main application.

If it was run on another pod this wouldnt be an issue, but then tailscale is on a different network than your app.

@delaman
Copy link
Author

delaman commented Dec 12, 2023

Good point again. What about making a tiny PVC to store the tailscale state?

@stavros-k
Copy link
Collaborator

That's a possible solution yes.
But what exactly TS stores there?
Will be it okay if the PVC ends up on network storage? eg sqlite does not like that.
I would assume its not sqlite tho, since original datastore is a k8s secret.

@delaman
Copy link
Author

delaman commented Dec 12, 2023

The tailscale state file is one single JSON file.

{
    "$schema": "http://json-schema.org/draft-06/schema#",
    "$ref": "#/definitions/tailscale01",
    "definitions": {
        "tailscale01": {
            "type": "object",
            "additionalProperties": false,
            "properties": {
                "_current-profile": {
                    "type": "string"
                },
                "_machinekey": {
                    "type": "string"
                },
                "_profiles": {
                    "type": "string"
                },
                "_serve/470c": {
                    "type": "string"
                },
                "profile-470c": {
                    "type": "string"
                }
            },
            "required": [
                "_current-profile",
                "_machinekey",
                "_profiles",
                "_serve/470c",
                "profile-470c"
            ],
            "title": "tailscale01"
        }
    }
}

@stavros-k
Copy link
Collaborator

stavros-k commented Dec 12, 2023

@Ornias1993 I'm fine with a 1-10Mi pvc. what do you think?

EDIT: scaled down pvc size

@delaman
Copy link
Author

delaman commented Dec 12, 2023

Tailscale file size is approx 2.5K. So miniscule.

EDIT: The state file does contain secrets. Is there a way to encrypt the file and decrypt on the fly? Or something similar?

@PrivatePuffin
Copy link
Member

@stavros-k Much rather use secrets with namespaced rbac.

Sneaking in persistence into something that can be stateless seems like a bad practice.

@delaman
Copy link
Author

delaman commented Dec 12, 2023

@stavros-k Much rather use secrets with namespaced rbac.

Sneaking in persistence into something that can be stateless seems like a bad practice.

dotfiles in a secret volume fits the bill!

@stavros-k
Copy link
Collaborator

@stavros-k Much rather use secrets with namespaced rbac.
Sneaking in persistence into something that can be stateless seems like a bad practice.

dotfiles in a secret volume fits the bill!

Can we please not linking random stuff? :)
Even if you mount the file it will be readOnly (regardless if you set it to readOnly or not).
So TS will still not be able to write its state.

@stavros-k
Copy link
Collaborator

@stavros-k Much rather use secrets with namespaced rbac.

Sneaking in persistence into something that can be stateless seems like a bad practice.

Namespaced would be anyway. But still all other containers on that pod will have access to that secret, since the SA will be mounted to the POD.

@delaman
Copy link
Author

delaman commented Dec 12, 2023

No it will be mounted in a container:
Example below

apiVersion: v1
kind: Secret
metadata:
  name: dotfile-secret
data:
  .secret-file: dmFsdWUtMg0KDQo=
---
apiVersion: v1
kind: Pod
metadata:
  name: secret-dotfiles-pod
spec:
  volumes:
    - name: secret-volume
      secret:
        secretName: dotfile-secret
  containers:
    - name: dotfile-test-container
      image: registry.k8s.io/busybox
      command:
        - ls
        - "-l"
        - "/etc/secret-volume"
      volumeMounts:
        - name: secret-volume
          readOnly: true
          mountPath: "/etc/secret-volume"

@stavros-k
Copy link
Collaborator

No it will be mounted in a container: Example below

apiVersion: v1
kind: Secret
metadata:
  name: dotfile-secret
data:
  .secret-file: dmFsdWUtMg0KDQo=
---
apiVersion: v1
kind: Pod
metadata:
  name: secret-dotfiles-pod
spec:
  volumes:
    - name: secret-volume
      secret:
        secretName: dotfile-secret
  containers:
    - name: dotfile-test-container
      image: registry.k8s.io/busybox
      command:
        - ls
        - "-l"
        - "/etc/secret-volume"
      volumeMounts:
        - name: secret-volume
          readOnly: true
          mountPath: "/etc/secret-volume"

So, you mount it in the container. How would Tailscale WRITE its state to that?

@delaman
Copy link
Author

delaman commented Dec 12, 2023

No it will be mounted in a container: Example below

apiVersion: v1
kind: Secret
metadata:
  name: dotfile-secret
data:
  .secret-file: dmFsdWUtMg0KDQo=
---
apiVersion: v1
kind: Pod
metadata:
  name: secret-dotfiles-pod
spec:
  volumes:
    - name: secret-volume
      secret:
        secretName: dotfile-secret
  containers:
    - name: dotfile-test-container
      image: registry.k8s.io/busybox
      command:
        - ls
        - "-l"
        - "/etc/secret-volume"
      volumeMounts:
        - name: secret-volume
          readOnly: true
          mountPath: "/etc/secret-volume"

So, you mount it in the container. How would Tailscale WRITE its state to that?

Like any other file.

EDIT: If the file is updated then the k8s secret is also updated.

@stavros-k
Copy link
Collaborator

stavros-k commented Dec 12, 2023

Like any other file.

But its not like any other file.

kubernetes/kubernetes#62099 (comment)
Also that FG is no longer available anyway
kubernetes/website#34026

@delaman
Copy link
Author

delaman commented Dec 12, 2023

Dang. What about Truechart supporting Tailscale k8s operator?

@stavros-k
Copy link
Collaborator

Dang. What about Truechart supporting Tailscale k8s operator?

Feel free to do the work :) But afaik its still ALPHA
If I were to give an estimate from my side it would be probably never. I don't use such services,
so I won't spend any time. But still TS wont be on the same POD as your other app.

Solution(s) have been layed out few messages ago.
It storage would be picked, I can PR that this year.
If it's RBAC, I'm not a fan, so I will stay put, someone else can probably do it.

@delaman
Copy link
Author

delaman commented Dec 12, 2023

Lets just go with the miniscule PVC.

I'll wait for the tailscale kuberentes operator to become mature. I will do the work at that time.

@PrivatePuffin
Copy link
Member

PrivatePuffin commented Dec 12, 2023

@delaman Let me stop you right there, from wasting more time.
And please don't make calls on how things should be done. Thats a maintainers call, not yours.

@stavros-k Setup a pretty limited RBAC, but I don't care much about the container secret leaking to other containers tbh.
But I'm okey in making it a toggle secret/pvc as well.

But default to secret please, we should evade defaulting to using persistence.

@stavros-k
Copy link
Collaborator

@delaman Let me stop you right there, from wasting more time. And please don't make calls on how things should be done. Thats a maintainers call, not yours.

@stavros-k Setup a pretty limited RBAC, but I don't care much about the container secret leaking to other containers tbh. But I'm okey in making it a toggle secret/pvc as well.

But default to secret please, we should evade defaulting to using persistence.

Sorry I can't spend time on this any time soon.
As it needs considerable work to not conflict/break with existing RBACs and SAs on some apps.

@delaman
Copy link
Author

delaman commented Dec 12, 2023

@delaman Let me stop you right there, from wasting more time. And please don't make calls on how things should be done. Thats a maintainers call, not yours.
@stavros-k Setup a pretty limited RBAC, but I don't care much about the container secret leaking to other containers tbh. But I'm okey in making it a toggle secret/pvc as well.
But default to secret please, we should evade defaulting to using persistence.

Sorry I can't spend time on this any time soon. As it needs considerable work to not conflict/break with existing RBACs and SAs on some apps.

@Ornias1993 Mentioned being okay with a toggle switch. Maybe adding the PVC version now then the RBAC version later?

@PrivatePuffin
Copy link
Member

PrivatePuffin commented Dec 12, 2023

@delaman Let me stop you right there, from wasting more time. And please don't make calls on how things should be done. Thats a maintainers call, not yours.
@stavros-k Setup a pretty limited RBAC, but I don't care much about the container secret leaking to other containers tbh. But I'm okey in making it a toggle secret/pvc as well.
But default to secret please, we should evade defaulting to using persistence.

Sorry I can't spend time on this any time soon. As it needs considerable work to not conflict/break with existing RBACs and SAs on some apps.

I agree it's not a priority any-time-soon.
It's a nice one for the backlog for 2025.

@PrivatePuffin
Copy link
Member

PrivatePuffin commented Dec 12, 2023

@delaman Let me stop you right there, from wasting more time. And please don't make calls on how things should be done. Thats a maintainers call, not yours.
@stavros-k Setup a pretty limited RBAC, but I don't care much about the container secret leaking to other containers tbh. But I'm okey in making it a toggle secret/pvc as well.
But default to secret please, we should evade defaulting to using persistence.

Sorry I can't spend time on this any time soon. As it needs considerable work to not conflict/break with existing RBACs and SAs on some apps.

@Ornias1993 Mentioned being okay with a toggle switch. Maybe adding the PVC version now then the RBAC version later?

You don't seem to be getting it:
RBAC is prefered to be added first, PVC is basically vetoéd till that's done.

And we as maintainers, are both not going to do it, as it's not a priority priority at-all.

@truecharts truecharts locked and limited conversation to collaborators Dec 12, 2023
@PrivatePuffin PrivatePuffin added this to the backlog milestone Dec 12, 2023
@PrivatePuffin
Copy link
Member

Closed in favor of:
#27349

@PrivatePuffin PrivatePuffin closed this as not planned Won't fix, can't repro, duplicate, stale Dec 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants