Add framework to init config in a plugin #182
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/assign @droot |
|
I've not read the code, but I don't think we plan on having anything live directly into |
|
Took a quick look. A few minor comments about the code. Though I am yet to understand the motivation of the code and where it belong in pkg structure especially given that plugins are still an Alpha thing. Lets have a chat this week to go over it. |
pkg/framework/plugin_client.go
Outdated
| // then the value of the KUBECONFIG env var (if any), and defaulting | ||
| // to ~/.kube/config as a last resort. | ||
| home := os.Getenv("HOME") | ||
| kubeconfig := home + "/.kube/config" |
There was a problem hiding this comment.
filepath.Join(home, ".kube", "config") ? That will probably make the code more portable across different OS's ?
| WjWmyvFisfZpuBuNGYLAi21qKIKUEItH71w120AbuR14x8YkK/Afp1+mh67gDgdL | ||
| Rcggy/IJOIuqJLvynmfb17iED+7HVw/4ujkg5LbLSC1iQLO8HjlNUu/dvSIg6oHQ | ||
| 0UUz9d+Yzg== | ||
| -----END CERTIFICATE----- |
There was a problem hiding this comment.
Looks like this is test data. It will better to have them in directory testData to be more explicit ?
pkg/framework/assets/client.crt
Outdated
| dpYw+p8tH/3ZY+RRD6B0xHlPLXuxjl/vz5rHaSVta4jigtPUJGAy81noevkwYdhq | ||
| HKK2EKK7+Q8lEYxMUm0KremgwettiVUBO+FR1xRRrsMcKX6sWNRhbH8oxfv++EmU | ||
| 0h+zLBSC8GVsba27MVpjd8sy | ||
| -----END CERTIFICATE----- |
There was a problem hiding this comment.
same comment as above (have them under testData dir ?)
pkg/framework/assets/client.key
Outdated
| iophhQKBgH/oS7xi2AFrGrscIVGL8V0M2GeACAhR7VKEw8PNbloH2HayfTY7fMlb | ||
| hY+kHxp52Ycinj1xiaCRIdceI/cXxENcEDyIrKny0Xs4shO3NXFzW6JhMB3hjDY7 | ||
| 2dPsYPzdEQTldmWHGS45XmKBMNgIOPNTXVl8i8UEZqtHIQohI1Wj | ||
| -----END RSA PRIVATE KEY----- |
pkg/framework/assets/config
Outdated
| user: | ||
| as-user-extra: {} | ||
| username: foo | ||
| password: bar |
pkg/framework/plugin_client.go
Outdated
| @@ -0,0 +1,176 @@ | |||
| /* | |||
| Copyright 2016 The Kubernetes Authors. | |||
pkg/framework/plugin_client.go
Outdated
| limitations under the License. | ||
| */ | ||
|
|
||
| package framework |
There was a problem hiding this comment.
Doesn't look like it belong under framework. looks like it belongs under "pluginutils" under pkg ?
pkg/framework/plugin_client.go
Outdated
|
|
||
| func clientFromConfig(path string) (*restclient.Config, string, error) { | ||
| if path == "-" { | ||
| cfg, err := restclient.InClusterConfig() |
There was a problem hiding this comment.
Not sure if this is useful for plugins ?
pkg/framework/plugin_client_test.go
Outdated
| ) | ||
|
|
||
| var _ = Describe("InitConfig", func() { | ||
| var () |
|
@droot I took a look at your comments and did some refactoring. I would love to chat whenever you're free. I'm @jberkhahn on the kubernetes slack as well. |
|
I'm sorry for all my packages comment, but Go recommends not using underscores in package names. |
jberkhahn
force-pushed
the
plugin_framework
branch
from
d867e20
to
0ee4ef2
Compare
k8s-ci-robot
added
size/XL
jberkhahn
force-pushed
the
plugin_framework
branch
from
0ee4ef2
to
7bb3a0b
Compare
k8s-ci-robot
added
size/L
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jberkhahn Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
pkg/pluginutils/plugin_client.go
Outdated
| } | ||
|
|
||
| if len(kubeconfig) == 0 { | ||
| return nil, errors.New(fmt.Sprintf("error iniializing config. The KUBECONFIG environment variable must be defined.")) |
There was a problem hiding this comment.
errors.New(fmt.Sprintf) ---> fmt.Errorf(....) ?
pkg/pluginutils/plugin_client.go
Outdated
|
|
||
| clientConfig, _, err := clientFromConfig(kubeconfig) | ||
| if err != nil { | ||
| return nil, errors.New(fmt.Sprintf("error obtaining kubectl config: %v", err)) |
pkg/pluginutils/plugin_client.go
Outdated
|
|
||
| err = applyGlobalOptionsToConfig(clientConfig) | ||
| if err != nil { | ||
| return nil, errors.New(fmt.Sprintf("error processing global plugin options: %v", err)) |
Hello. This is a PR of the extracted framework from the service-catalog plugins I've been writing. I discussed it with @pwittrock at Kubecon, and he expressed interest in me contributing it here for general use.
It's use is to create a config based on the system kubectl config and the environment variables passed in from the calling plugin framework. I've got most of them worked out, however the overrides for the config cluster, context, and user still need to be done. I thought I would get this out here and get some feedback on what I have so far.
Thanks,
@jberkhahn