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.

Sign up for GitHub

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

Jump to bottom

python kubeconfig loading doesn't match kubectl config loading... #445

Closed
brendandburns opened this issue Jan 27, 2018 · 3 comments · Fixed by kubernetes-client/python-base#47
Closed

python kubeconfig loading doesn't match kubectl config loading... #445

brendandburns opened this issue Jan 27, 2018 · 3 comments · Fixed by kubernetes-client/python-base#47
Assignees
@roycaihw

Comments

@brendandburns
Copy link
Contributor

I had a .kube/config file with duplicate clusters with the same name (but different information) in the config file, the three different entries each pointed to different clusters, two were dead, but one was alive (I had be repeatedly creating and deleting clusters)

kubectl command line happened to chose the working one and worked correctly.

python client library chose the first one in the file (which wasn't working) and failed.

This lead to a very confused experience until I figured out what was going on.

It will be difficult to emulate the kubectl parsing exactly, but we should detect duplicate entries and throw an exception. (and I would argue that kubectl probably should too)

This actually probably applies to all client libraries in different languages.

@mbohlool
@roycaihw roycaihw self-assigned this Feb 15, 2018
@roycaihw
Copy link
Member

kubectl seems to choose the last one if there are multiple clusters with the same name:

The config loader https://github.com/kubernetes/kubernetes/blob/b114a117f8688310b9162961d8e9b15192eacd50/pkg/kubectl/cmd/util/factory_client_access.go#L175-L176
ends up being https://github.com/kubernetes/kubernetes/blob/b114a117f8688310b9162961d8e9b15192eacd50/staging/src/k8s.io/client-go/tools/clientcmd/loader.go#L386-L399, which uses https://github.com/kubernetes/kubernetes/blob/b114a117f8688310b9162961d8e9b15192eacd50/staging/src/k8s.io/client-go/tools/clientcmd/api/latest/latest.go#L59-L65

I agree it should detect duplicate entries. Just curious would it solve the problem if the python client chose the last one in the file?

@brendandburns
Copy link
Contributor Author

brendandburns commented Feb 22, 2018 via email

@roycaihw
Copy link
Member

@brendandburns Totally agree. Just curious if kubectl in your test was picking the last one or behaving differently. I will open a PR to add the duplication detection.

@roycaihw roycaihw mentioned this issue Feb 22, 2018
Merged
@roycaihw roycaihw mentioned this issue Feb 27, 2018
Merged
@roycaihw roycaihw mentioned this issue Mar 8, 2018
Merged
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue May 19, 2018
Merge pull request #60464 from roycaihw/loadconfig
7909712 
Automatic merge from submit-queue (batch tested with PRs 63598, 63913, 63459, 63963, 60464). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Client-go raises error on duplicated name in kubeconfig

(for NamedCluster, NamedContext, NamedUser, NamedExtension)



**What this PR does / why we need it**:
Client should detect duplicated name when loading `name-value` based lists in kubeconfig: `users`, `clusters`, `contexts`. Currently if there are multiple value with same name, `client-python` will pick the first one, while `client-go` will pick the last.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
ref: kubernetes-client/python#445, kubernetes-client/python-base#47

**Special notes for your reviewer**:

**Release note**:

```release-note
kubectl and client-go now detects duplicated name for user, cluster and context when loading kubeconfig and reports error 
```

/sig api-machinery

cc @brendandburns @mbohlool
k8s-publishing-bot added a commit to kubernetes/client-go that referenced this issue May 19, 2018
@k8s-publishing-bot
Merge pull request #60464 from roycaihw/loadconfig
e1b1ad3 
Automatic merge from submit-queue (batch tested with PRs 63598, 63913, 63459, 63963, 60464). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Client-go raises error on duplicated name in kubeconfig

(for NamedCluster, NamedContext, NamedUser, NamedExtension)

**What this PR does / why we need it**:
Client should detect duplicated name when loading `name-value` based lists in kubeconfig: `users`, `clusters`, `contexts`. Currently if there are multiple value with same name, `client-python` will pick the first one, while `client-go` will pick the last.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
ref: kubernetes-client/python#445, kubernetes-client/python-base#47

**Special notes for your reviewer**:

**Release note**:

```release-note
kubectl and client-go now detects duplicated name for user, cluster and context when loading kubeconfig and reports error
```

/sig api-machinery

cc @brendandburns @mbohlool

Kubernetes-commit: 7909712ca574c536901bc7858dfbf72a3a6ee7dd
sttts pushed a commit to sttts/client-go that referenced this issue Jun 8, 2018
@k8s-publishing-bot
Merge pull request #60464 from roycaihw/loadconfig
7821c82 
Automatic merge from submit-queue (batch tested with PRs 63598, 63913, 63459, 63963, 60464). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Client-go raises error on duplicated name in kubeconfig

(for NamedCluster, NamedContext, NamedUser, NamedExtension)

**What this PR does / why we need it**:
Client should detect duplicated name when loading `name-value` based lists in kubeconfig: `users`, `clusters`, `contexts`. Currently if there are multiple value with same name, `client-python` will pick the first one, while `client-go` will pick the last.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
ref: kubernetes-client/python#445, kubernetes-client/python-base#47

**Special notes for your reviewer**:

**Release note**:

```release-note
kubectl and client-go now detects duplicated name for user, cluster and context when loading kubeconfig and reports error
```

/sig api-machinery

cc @brendandburns @mbohlool

Kubernetes-commit: 7909712ca574c536901bc7858dfbf72a3a6ee7dd
k8s-publishing-bot added a commit to kubernetes/client-go that referenced this issue Jun 8, 2018
@k8s-publishing-bot
Merge pull request #60464 from roycaihw/loadconfig
d1038de 
Automatic merge from submit-queue (batch tested with PRs 63598, 63913, 63459, 63963, 60464). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Client-go raises error on duplicated name in kubeconfig

(for NamedCluster, NamedContext, NamedUser, NamedExtension)

**What this PR does / why we need it**:
Client should detect duplicated name when loading `name-value` based lists in kubeconfig: `users`, `clusters`, `contexts`. Currently if there are multiple value with same name, `client-python` will pick the first one, while `client-go` will pick the last.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
ref: kubernetes-client/python#445, kubernetes-client/python-base#47

**Special notes for your reviewer**:

**Release note**:

```release-note
kubectl and client-go now detects duplicated name for user, cluster and context when loading kubeconfig and reports error
```

/sig api-machinery

cc @brendandburns @mbohlool

Kubernetes-commit: 7909712ca574c536901bc7858dfbf72a3a6ee7dd
@tomplus tomplus mentioned this issue Oct 1, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@roycaihw roycaihw

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Raise exception on duplicated name in kubeconfig roycaihw/python-base
2 participants
@brendandburns @roycaihw