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

kc.loadFromCluster() doesn't setup getCurrentCluster() correctly. #2168

Closed
koooge opened this issue Jan 20, 2025 · 5 comments
Closed

kc.loadFromCluster() doesn't setup getCurrentCluster() correctly. #2168

koooge opened this issue Jan 20, 2025 · 5 comments

Comments

@koooge
Copy link

koooge commented Jan 20, 2025

Hi there,
patch-example.js doesn't work with kc.loadFromCluster() instead of kc.loadFromDefault(). The app is running in a pod, and it causes Error: Cluster is undefined #2117 (comment)

I'm not sure if it's a bug or if there is another usage to fix it. At least, loadFromCluster works in v0 for me.

@brendandburns
Copy link
Contributor

it looks like currentCluster is undefined in the loadFromCluster function. You could try substituting getClusters()[0] instead of getCurrentCluster() and see if that works.

But we should probably check and fix to make it so that getCurrentCluster() works.

@brendandburns brendandburns changed the title patch-example.js doesn't work with kc.loadFromCluster() kc.loadFromCluster() doesn't setup getCurrentCluster() correctly. Jan 21, 2025
@cjihrig
Copy link
Contributor

cjihrig commented Jan 30, 2025

I think this is a minimal reproduction:

import * as k8s from '@kubernetes/client-node';
const kc = new k8s.KubeConfig();
kc.loadFromCluster();
const currentContext = kc.getCurrentContext();
const currentCluster = kc.getCluster(currentContext);
console.log(currentCluster); // Logs null

When kc.loadFromCluster() is called, kc.clusters is set to:

[
  {
      name: 'inCluster',
      caFile: `${pathPrefix}${SERVICEACCOUNT_CA_PATH}`,
      server: `${scheme}://${serverHost}:${port}`,
      skipTLSVerify: false,
  },
]

The call to kc.getCurrentContext() always returns the value 'inClusterContext'.

So, you're always executing kc.getCluster('inClusterContext'). Internally, this searches the list of clusters for one whose name is 'inClusterContext'. A cluster with the same name as the context does not exist.

@cjihrig
Copy link
Contributor

cjihrig commented Jan 30, 2025

But we should probably check and fix to make it so that getCurrentCluster() works.

It looks like that does work.

@cjihrig
Copy link
Contributor

cjihrig commented Jan 31, 2025

@koooge can you confirm if changing the code from kc.getCluster(currentContext); to kc.getCurrentCluster(); gets you past that error. It seems to me like the example should be using that code.

@brendandburns
Copy link
Contributor

@cjihrig thanks for looking closer at this. You are correct, there is a bug in the code from @koooge

It should be:

kc.getCluster(kc.getContextObject(kc.getCurrentContext()).cluster)

but really kc.getCurrentCluster() is easier.

I did notice that our code coverage for this code is low, so I added #2191

I'm going to close this as working as intended.

cjihrig added a commit to cjihrig/javascript that referenced this issue Jan 31, 2025
This commit updates the patch examples to use getCurrentCluster().
The existing code relied on the current context having the same
name as a cluster.

Refs: kubernetes-client#2168
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

No branches or pull requests

3 participants