-
Notifications
You must be signed in to change notification settings - Fork 99
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
Support getting certificate information from a kubeconfig file #61
Conversation
c39d2f2
to
2dc31e3
Compare
Hi @treydock, thank you for this PR. It looks really good, however it doesn't seem to support paths relative to the kubeconfig file:
Cluster config looks like this and is valid:
|
@ribbybibby I added a commit that should fix the issue where a relative path is used. Let me know if the changes work for you. It assumes if the path is |
prober/kubeconfig.go
Outdated
// Path is relative to kubeconfig path | ||
if !filepath.IsAbs(c.Cluster.CertificateAuthority) { | ||
newPath := filepath.Join(filepath.Dir(k.Path), c.Cluster.CertificateAuthority) | ||
c.Cluster.CertificateAuthority = newPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relative paths still aren't working for me, I'm getting the same error. I don't think this assignment is actually modifying the value that's passed to collectKubeconfigMetrics
.
Perhaps you could move this logic into collectKubeconfigMetrics
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the issue is I was trying to modify the slice element from within the loop which won't work. Instead I am creating a new slice using modified value and using that new slice for the main struct. I am also moving the parsing logic entirely to parsing function so can test the parsing without having to actually have a full cert setup. I added a test case from your example YAML.
… parsing into parsing specific function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the prompt responses to my review, I'm sorry I haven't been so responsive in following up! This looks all good now.
@ribbybibby Would it be possible to get a tag/release that includes this pull request? |
The idea is to query the user certificate inside
/etc/kubernetes/admin.conf
. Example output queried on my dev Kubernetes cluster:Also works to read kubelet configs and their associated certificates: