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

Changed to have to specify namespace and context. #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ykwyuta
Copy link

@ykwyuta ykwyuta commented Oct 6, 2021

I want you to make it mandatory to specify the namespace and context.
I find it very dangerous to run a script whose namespace and context are not named. We operate hundreds of Kubernetes clusters on our system, and without explicit context, operators run the risk of accidentally capturing packets in different environments. You may also not be able to deploy privileged containers in the default namespace on clusters that have Pod Security Standards applied.

@tdihp
Copy link
Owner

tdihp commented Nov 8, 2021

Hi @ykwyuta , thanks for sending this PR! I think supporting namespace is great, , and it is good to also include context, however I'd prefer to have a default value for both, so it always easily work for simpler situations.

Do you think it works for you to have a default following example of projects such as helm? Helm supports --namespace and --kube-context including many other configurations, but always follow the implicit pattern that the current kubectl context does if none is supplied. I'd be happy to make improvement into this direction.

@tdihp
Copy link
Owner

tdihp commented Jan 11, 2024

My apologies @ykwyuta , I didn't check this PR after you made change. I'll check and update.

@tdihp
Copy link
Owner

tdihp commented Jan 11, 2024

@ykwyuta @yukirii I'd like your opinion if it works better that we forward all args of dspcap_start and dspcap_stop to kubectl commands. This way we simply do kubectl "$@" apply -f instead of kubectl --context "$CONTEXT" --namespace "$NAMESPACE"

@yukirii
Copy link
Contributor

yukirii commented Jan 15, 2024

I'd prefer that dspcap has a default value and follows to current context of kubectl by default.

This way we simply do kubectl "$@" apply -f instead of kubectl --context "$CONTEXT" --namespace "$NAMESPACE"

Using the method @tdihp suggested, we can easily achieve both using default values and using explicit options when run command.

# Specify namespace explicitly when run dspcap
./dspcap-start --namespace={NAMESPACE}
./dspcap-stop --namespace={NAMESPACE}

# Using default values (following current context)
kubectl config set-context {CONTEXT} --namespace={NAMESPACE}
./dspcap-start
./dspcap-stop

This method is flexible. We can also use other options defined by kubectl options.

# Specify kubeconfig file
./dspcap-start --kubeconfig=/path/to/kubeconfig
./dspcap-stop --kubeconfig=/path/to/kubeconfig

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.

3 participants