[Configuration] Support exec for kubeconfig #12
Conversation
k8s-ci-robot
added
cncf-cla: yes
kubernetes/config/exec_provider.c
Outdated
| return -1; | ||
| } | ||
| snprintf(env_string, env_string_length, "%s=%s", pair->key, (char *) (pair->value)); | ||
| rc = putenv(env_string); |
There was a problem hiding this comment.
Use setenv instead of putenv for clearer/simpler memory semantics. Otherwise this will leak memory.
There was a problem hiding this comment.
OK. The function setenv is very good to use.
kubernetes/config/kube_config.c
Outdated
| @@ -21,15 +24,34 @@ static int setBasePath(char **pBasePath, char *basePath) | |||
| return -1; | |||
| } | |||
|
|
|||
| static char *kubeconfig_mk_cert_key_tempfile(const char *b64data) | |||
| static int is_cert_or_key_base64_encoded(const char *data) | |||
There was a problem hiding this comment.
I think we can assume c99 at this point, so static bool is_cert_or_key_base64_encoded(...)
kubernetes/config/kube_config.c
Outdated
| const char *cert_key_data = NULL; | ||
| int cert_key_data_bytes = 0; | ||
|
|
||
| if (0 == is_cert_or_key_base64_encoded(data)) { |
There was a problem hiding this comment.
remove 0 == once we switch to bool return value.
| fprintf(stderr, "%s: Base64 decodes failed.\n", fname); | ||
| return NULL; | ||
| } | ||
| cert_key_data = b64decode; |
There was a problem hiding this comment.
This memory is leaked, I think. We should free cert_key_data
There was a problem hiding this comment.
Yes. you're right. I will handle it.
kubernetes/config/kube_config.c
Outdated
| int new_command_length = strlen(kube_config_dirname) + strlen("/") + strlen(original_command) + 1 /* 1 for the terminal of string */ ; | ||
| exec->command = calloc(1, new_command_length); | ||
| if (!exec->command) { | ||
| fprintf(stderr, "%s: Cannot allocate memory for exec command.[%s]\n", fname, strerror(errno)); |
There was a problem hiding this comment.
this is going to leak the memory in original_command
|
Some comments. We should consider adding |
|
Thank you @brendandburns. I have addressed your comments. And I also have run |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns, ityuhui The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
support
execfor kube config( https://kubernetes.io/docs/reference/access-authn-authz/authentication/#configuration )
/cc @brendandburns