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

Avoid Passing nil in Exec Call #240

Closed
wants to merge 1 commit into from
Closed

Conversation

peixian
Copy link

@peixian peixian commented Mar 17, 2023

Currently, if you're using GKE and an updated Kube cluster with the gke-auth-plugin (https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke), it'll write a config file that looks like

users: 
- name: <CLUSTER>
  user:
    exec:
      apiVersion: client.authentication.k8s.io/v1beta1
      command: /Users/peixian/miniconda3/envs/gcloud/share/google-cloud-sdk-390.0.0-0/bin/gke-gcloud-auth-plugin
      installHint: Install gke-gcloud-auth-plugin for use with kubectl by following
        https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke
      provideClusterInfo: true

This doesn't set an args, which causes a nil error when checking config.args. This PR just adds a null default for cases like this.

Requirements for all pull requests

  • Entry in CHANGELOG.md was created

Additional requirements for new features

  • New unit tests were created
  • New integration tests were created
  • PR description contains a link to the according kubernetes documentation

@mruoss
Copy link
Collaborator

mruoss commented Mar 19, 2023

Oh well yes, I see the problem. However, I'd rather define default values inside defstruct instead:

  defstruct [:command, :env, args: []]

You wanna update your PR or shall I commit that change?

@mruoss
Copy link
Collaborator

mruoss commented Mar 19, 2023

Never mind. Fixed in 2f0525e

@mruoss mruoss closed this Mar 19, 2023
@peixian
Copy link
Author

peixian commented Mar 20, 2023

excellent, thanks!

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.

2 participants