Skip to content

Conversation

@caalberts
Copy link
Contributor

@caalberts caalberts commented Jan 29, 2025

This fixes a bug on the @dracula-kubernetes-context-label option in kubernetes_context plugin.

kubernetes_context.sh is called with positional args as such:

kubernetes_context.sh $eks_hide_arn $eks_extract_account $hide_kubernetes_user $show_only_kubernetes_context $show_kubernetes_context_label

When show_only_kubernetes_context defaults to "", arg $5 show_kubernetes_context_label, becomes arg $4 show_only_kubernetes_context. Effectively, the value of @dracula-kubernetes-context-label is passed to the script as show_only_kubernetes_context instead of show_kubernetes_context_label So the default value for show_only_kubernetes_context needs to be a non-zero string.

Here are screenshots showing the bug:

@dracula-kubernetes-context-label has no effect when @dracula-show-only-kubernetes-context is "".
Screenshot 2025-01-29 at 7 59 05 PM

@dracula-kubernetes-context-label has no effect when @dracula-show-only-kubernetes-context is not set.
Screenshot 2025-01-29 at 7 58 34 PM

Setting @dracula-kubernetes-context-label to "true" is effectively the same as setting @dracula-show-only-kubernetes-context to "true".
Screenshot 2025-01-29 at 8 06 36 PM

@dracula-kubernetes-context-label only takes effect when @dracula-show-only-kubernetes-context is explicitly set to "false" or other non-zero length string other than "true".
Screenshot 2025-01-29 at 7 58 02 PM

`kubernetes_context.sh` is called with positional args as such:

```
kubernetes_context.sh $eks_hide_arn $eks_extract_account $hide_kubernetes_user $show_only_kubernetes_context $show_kubernetes_context_label
```

When `show_only_kubernetes_context` defaults to "", the positional arg $5 for  `show_kubernetes_context_label`,  becomes arg $4 for `show_only_kubernetes_context`. So the default value for `show_only_kubernetes_context` needs to be a non-zero string.
@ethancedwards8 ethancedwards8 merged commit dd1a7ab into dracula:master Jan 29, 2025
1 check passed
@ethancedwards8
Copy link
Member

Good catch. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants