truecharts / public

Community Helm Chart Repository
https://truecharts.org
GNU Affero General Public License v3.0
1.13k stars 617 forks source link

Persist tailscale state #15911

Closed delaman closed 9 months ago

delaman commented 10 months ago

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

delaman commented 10 months ago

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

PrivatePuffin commented 10 months ago

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

delaman commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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

stavros-k commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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

EDIT: scaled down pvc size

delaman commented 10 months ago

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 commented 10 months ago

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

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

delaman commented 10 months ago

@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 commented 10 months ago

@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 commented 10 months ago

@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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

Like any other file.

But its not like any other file.

https://github.com/kubernetes/kubernetes/issues/62099#issuecomment-378770047 Also that FG is no longer available anyway https://github.com/kubernetes/website/issues/34026

delaman commented 10 months ago

Dang. What about Truechart supporting Tailscale k8s operator?

stavros-k commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

@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 commented 10 months ago

@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 commented 10 months ago

@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 commented 10 months ago

@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 commented 10 months ago

@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.

PrivatePuffin commented 9 months ago

Closed in favor of: https://github.com/truecharts/charts/issues/27349