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

allow_k8s_contexts not properly enforced when changing context with kubectl #6295

Open
PeterStolz opened this issue Jan 19, 2024 · 5 comments
Labels
bug Something isn't working needs repro case

Comments

@PeterStolz
Copy link

Expected Behavior

Assume tilt is already running with a allow_k8s_contexts allowing docker-desktop. Changing the context now on my system using kubectl config use-context example. I would assume it checks before applying an operation that it is still in the correct context. However I just deployed some stuff to a cluster that I should only modify with caution.

Current Behavior

I do not know too much about tilt internals but I would assume it runs the check when starting up tilt and not before applying resources. Therefore there is always a window where tilt applies stuff to a cluster that is not explicitly enabled.
I added the allow_k8s_contexts when tilt had already started which might be the key to this issue.
When running tilt down it correctly recognized that it should not touch that cluster, so I deleted the created resources manually.

Steps to Reproduce

  1. Start tilt
  2. Add allow_k8s_contexts to Tiltfile
  3. Change context in your kubeconfig. e.g. kubectl config use-context asdf
  4. Trigger an update to a tilt resource triggering an update

Context

tilt doctor Output

$ tilt doctor
Tilt: v0.33.9, built 2023-12-08
System: darwin-arm64
---
Docker
- Host: unix:///Users/peterstolz/.docker/run/docker.sock
- Server Version: 24.0.7
- API Version: 1.43
- Builder: 2
- Compose Version: v2.23.3-desktop.2
---
Kubernetes
- Env: unknown
- Context: *redacted*
- Cluster Name: *redacted*
- Namespace: default
- Container Runtime: containerd
- Version: v1.25.16
- Cluster Local Registry: none
---
Thanks for seeing the Tilt Doctor!
Please send the info above when filing bug reports. 💗

The info below helps us understand how you're using Tilt so we can improve,
but is not required to ask for help.
---
Analytics Settings
--> (These results reflect your personal opt in/out status and may be overridden by an `analytics_settings` call in your Tiltfile)
- User Mode: opt-out
- Machine: 3d67e811e6a72261b4bd216a21e03ef2
- Repo: xlez7frlKiLxs5U+1n3k/w==

About Your Use Case

I forgot that tilt was still running somewhere in the background. I did some troubleshooting on another cluster and tiggered a tilt update through renaming a file. Then Tilt installed the deployment on the cluster it is not allowed to use.

We could remember the allowed context when allow_k8s_contexts is used and always set --context=<allowed-context>.
Though I can also see that this may lead to other people running into issues that change their clusters when developing.
Another approach would be to reverify that an allowed context is used before applying resources.

@PeterStolz PeterStolz added the bug Something isn't working label Jan 19, 2024
@nicks
Copy link
Member

nicks commented Jan 19, 2024

hmmmmm....i was not able to reproduce the behavior you're describing. But I think there's a bit of confusion about the expected behavior. Here's what I tried:

The behavior I would expect is that Tilt reads the kubeconfig at startup, and continues to deploy to the same cluster, even if your current context changes.

Do you have additional repro steps?

@PeterStolz
Copy link
Author

Unfortunately after playing around a bunch I also can't reproduce the bug.
To fill in some additional defails.

  1. Starting tilt with the remote cluster as context yields the expected error due to beeing potentially on production.
    -> I could not have started tilt by accident on the affected cluster as it would not have applied anything. I am certain the allowed context is and was docker-desktop as I did not change that at all. Also docker-desktop is the local cluster and not some sadistic rename.
  2. This happened after my laptop was suspended for around 14 hours.
  3. Parts of tilt were likely building a container and a local helm chart was likely reinstalled, because the affected file was not in the .helmignore or .dockerignore

To me this feels like a very weird race condition. I am not familiar with the codebase, but do you know how tilt handles the context internally? Are kubectl commands always applied with a pinned context?
At some point tilt must have switched the context even skipping the k8s_context validation.

@PeterStolz
Copy link
Author

Is tilt maybe working with tokens to authenticate to the api and because >14h passed, it expired and it had to renew it bypassing the checks and using the current-context?

@nicks
Copy link
Member

nicks commented Jan 19, 2024

tilt reads the kubeconfig at startup, minifies it (removing all the other kube contexts), writes a new minified kubeconfig to a different place on disk, and uses that one for all future connection settings. If you run tilt get cluster -o yaml, you can see what it's using.

@PeterStolz
Copy link
Author

The config it writes to:
tilt get cluster -ojsonpath='{.items[*].status.connection.kubernetes.configPath}'
is not used by tilt itself? When deleting it tilt still works.
There is a config at ~/.tilt-dev though and even deleting that does not prevent tilt from talking to the cluster. There has to be some in memory state.
Shooting in the dark here, but the long time between the tilt updates and communication with the apiserver is probably key to this. Maybe there is some broken error handling when reestablishing a connection?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs repro case
Projects
None yet
Development

No branches or pull requests

2 participants