Skip to content
This repository has been archived by the owner on Nov 19, 2020. It is now read-only.

Sync kubeconfig structure as part of generate #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LukeShu
Copy link
Contributor

@LukeShu LukeShu commented Feb 5, 2019

config.go is copied from client-go/tools/clientcmd/api/v1/types.go, and then modified a bit to kinda-sorta make gopkg.in/yaml.v2 happy (modified in #71 for #60, but still kinda broken #81).

Since it's still broken with gopkg.in/yaml.v2, let's just ditch the modifications in #71, so that we can copy the file as part of generate, instead of trying to keep it in-sync manually. This has the potential to break users who use gopkg.in/yaml.v2 (and rely on no -data fields being set in the YAML they're parsing). I figure since it's already kinda broken, most users aren't using gopkg.in/yaml.v2, especially since the docs point them toward github.com/ghodss/yaml.

When we "generate" it, put it in ./config/v1/, to make it clear that it is a versioned type imported from client-go. I think it's cleaner that way.

But, to avoid totally breaking existing users, don't entirely remove config.go, but replace it with a list of Go 1.9 type aliases, so that github.com/ericchiang/k8s.Config is an alias for github.com/ericchiang/k8s/client/v1.Config and so on. This bumps the required Go version from 1.7 to 1.9. This bump could be avoided at the cost of breaking existing users.

Alternatively, the generate script could be edited to keep it in the current package.

I'm not sure what your feelings are on breaking changes at the expense of clarity, or whether this type of change is welcome in general.

Place it in config/v1/.  Add Go 1.9 type aliases to the old names, to
avoid breaking users.
@ericchiang
Copy link
Owner

What part of the kubeconfig parsing is broken exactly?

@LukeShu
Copy link
Contributor Author

LukeShu commented Feb 5, 2019

What #81 says, that gopkg.in/yaml.v2 refuses to unmarshal certificate-authority-data as a []byte instead of as a string.

As you recommend, github.com/ghodss/yaml does not have that issue, and also totally ignores the yaml: struct tags added in #71, instead paying attention to the json: struct tags. The yaml: struct tags only serve to try to make gopkg.in/yaml.v2 happy.

@ericchiang
Copy link
Owner

My main concern is that we're going to break users using github.com/ghodss/yaml. Can we commit the generated config then provide a convince to convert from the generated one to the existing one?

@LukeShu
Copy link
Contributor Author

LukeShu commented Feb 5, 2019

This should not break users using github.com/ghodss/yaml.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants