-
Notifications
You must be signed in to change notification settings - Fork 303
Decouple yaml file and KubernetesClientConfiguration object #30
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
Conversation
e6fab82
to
cfceb89
Compare
LGTM. Thanks for the PR. |
{ | ||
handler.ServerCertificateCustomValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; | ||
} | ||
else |
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.
combine the else
and if
into an else if
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.
oh, wait, nevermind, I see there's a statement under the if...
"unable to load in-cluster configuration, KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT must be defined"); | ||
} | ||
|
||
var token = File.ReadAllText("/var/run/secrets/kubernetes.io/serviceaccount/" + ServiceAccountTokenKey); |
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.
extract "/var/run/secrets/kubernetes.io/serviceaccount/"
and other strings out as constants?
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.
another small question
Is this still not supported by windows after 1709?
Needs a rebase, but then LGTM. |
KubernetesClientConfiguration now can be created without yaml config
first step to address #24
Introduce go client style config creating functions