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.

Sign up for GitHub

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

roll our own strict config loading #912

Merged
merged 4 commits into from
Oct 15, 2019
Merged

roll our own strict config loading #912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d0f4a5a
roll our own config loading
BenTheElder Oct 3, 2019
0447dd2
switch to yaml.v3
BenTheElder Oct 15, 2019
9f8ce52
deepcopy before converting
BenTheElder Oct 15, 2019
53bd2eb
handle custom decode
BenTheElder Oct 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ go 1.13

require (
github.com/alessio/shellescape v0.0.0-20190409004728-b115ca0f9053
github.com/google/gofuzz v1.0.0
github.com/google/uuid v1.1.1
github.com/pkg/errors v0.8.1
github.com/spf13/cobra v0.0.3
golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8
gopkg.in/yaml.v3 v3.0.0-20191010095647-fc94e3f71652
k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b // indirect
k8s.io/apimachinery v0.0.0-20190404173353-6a84e37a896d
sigs.k8s.io/kustomize/v3 v3.2.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE=
gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v3 v3.0.0-20191010095647-fc94e3f71652 h1:VKvJ/mQ4BgCjZUDggYFxTe0qv9jPMHsZPD4Xt91Y5H4=
gopkg.in/yaml.v3 v3.0.0-20191010095647-fc94e3f71652/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
k8s.io/api v0.0.0-20190313235455-40a48860b5ab/go.mod h1:iuAfoD4hCxJ8Onx9kaTIt30j7jUFS00AXQi6QMi99vA=
k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b h1:aBGgKJUM9Hk/3AE8WaZIApnTxG35kbuQba2w+SXqezo=
k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b/go.mod h1:iuAfoD4hCxJ8Onx9kaTIt30j7jUFS00AXQi6QMi99vA=
Expand Down
6 changes: 0 additions & 6 deletions hack/update/generated.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ cd "${REPO_ROOT}"

# build the generators using the tools module
cd "hack/tools"
"${REPO_ROOT}/hack/go_container.sh" go build -o /out/defaulter-gen k8s.io/code-generator/cmd/defaulter-gen
"${REPO_ROOT}/hack/go_container.sh" go build -o /out/deepcopy-gen k8s.io/code-generator/cmd/deepcopy-gen
"${REPO_ROOT}/hack/go_container.sh" go build -o /out/conversion-gen k8s.io/code-generator/cmd/conversion-gen
# go back to the root
cd "${REPO_ROOT}"

Expand All @@ -47,11 +45,7 @@ cd "${FAKE_REPOPATH}"

# run the generators
bin/deepcopy-gen -i ./pkg/internal/apis/config/ -O zz_generated.deepcopy --go-header-file hack/tools/boilerplate.go.txt
bin/defaulter-gen -i ./pkg/internal/apis/config/ -O zz_generated.default --go-header-file hack/tools/boilerplate.go.txt

bin/deepcopy-gen -i ./pkg/apis/config/v1alpha3 -O zz_generated.deepcopy --go-header-file hack/tools/boilerplate.go.txt
bin/defaulter-gen -i ./pkg/apis/config/v1alpha3 -O zz_generated.default --go-header-file hack/tools/boilerplate.go.txt
bin/conversion-gen -i ./pkg/internal/apis/config/v1alpha3 -O zz_generated.conversion --go-header-file hack/tools/boilerplate.go.txt

# set module mode back, return to repo root and gofmt to ensure we format generated code
export GO111MODULE="on"
Expand Down
23 changes: 9 additions & 14 deletions pkg/apis/config/v1alpha3/default.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,14 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

// this comment makes golint ignore this file, feel free to edit the file.
// Code generated by not-actually-generated-but-go-away-golint. DO NOT EDIT.
// https://github.com/kubernetes/code-generator/issues/30

package v1alpha3

import (
"k8s.io/apimachinery/pkg/runtime"

"sigs.k8s.io/kind/pkg/apis/config/defaults"
)

func addDefaultingFuncs(scheme *runtime.Scheme) error {
return RegisterDefaults(scheme)
}

// SetDefaults_Cluster sets uninitialized fields to their default value.
func SetDefaults_Cluster(obj *Cluster) {
// SetDefaultsCluster sets uninitialized fields to their default value.
func SetDefaultsCluster(obj *Cluster) {
// default to a one node cluster
if len(obj.Nodes) == 0 {
obj.Nodes = []Node{
Expand All @@ -41,6 +31,11 @@ func SetDefaults_Cluster(obj *Cluster) {
},
}
}
// default the nodes
for i := range obj.Nodes {
a := &obj.Nodes[i]
SetDefaultsNode(a)
}
if obj.Networking.IPFamily == "" {
obj.Networking.IPFamily = "ipv4"
}
Expand Down Expand Up @@ -70,8 +65,8 @@ func SetDefaults_Cluster(obj *Cluster) {
}
}

// SetDefaults_Node sets uninitialized fields to their default value.
func SetDefaults_Node(obj *Node) {
// SetDefaultsNode sets uninitialized fields to their default value.
func SetDefaultsNode(obj *Node) {
if obj.Image == "" {
obj.Image = defaults.Image
}
Expand Down
55 changes: 0 additions & 55 deletions pkg/apis/config/v1alpha3/register.go
View file
Open in desktop

This file was deleted.

54 changes: 28 additions & 26 deletions pkg/apis/config/v1alpha3/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,38 +17,40 @@ limitations under the License.
package v1alpha3

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"sigs.k8s.io/kind/pkg/container/cri"
)

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// Cluster contains kind cluster configuration
type Cluster struct {
// TypeMeta representing the type of the object and its API schema version.
metav1.TypeMeta `json:",inline"`
TypeMeta `yaml:",inline" json:",inline"`

// Nodes contains the list of nodes defined in the `kind` Cluster
// If unset this will default to a single control-plane node
// Note that if more than one control plane is specified, an external
// control plane load balancer will be provisioned implicitly
Nodes []Node `json:"nodes,omitempty"`
Nodes []Node `yaml:"nodes,omitempty" json:"nodes,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for not having a look at this PR again.

i think this dropped support for user provided JSON kind config files?
which is not very critical but diverges from the "component config" / api-machinery trend.

Copy link
Member Author

@BenTheElder BenTheElder Oct 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • this also has json tags for now? (reread the diff?)
  • we never supported JSON config files. I don't intend to. yaml is a superset of json.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kubernetes needs a seperate JSON parser for other reasons (because it gives better errors?). yaml.v3 also gives better errors kubernetes/kubernetes#78946

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed the extra tags in the diff.


/* Advanced fields */

// Networking contains cluster wide network settings
Networking Networking `json:"networking,omitempty"`
Networking Networking `yaml:"networking,omitempty" json:"networking,omitempty"`

// KubeadmConfigPatches are applied to the generated kubeadm config as
// strategic merge patches to `kustomize build` internally
// https://github.com/kubernetes/community/blob/master/contributors/devel/strategic-merge-patch.md
// This should be an inline yaml blob-string
KubeadmConfigPatches []string `json:"kubeadmConfigPatches,omitempty"`
KubeadmConfigPatches []string `yaml:"kubeadmConfigPatches,omitempty" json:"kubeadmConfigPatches,omitempty"`

// KubeadmConfigPatchesJSON6902 are applied to the generated kubeadm config
// as patchesJson6902 to `kustomize build`
KubeadmConfigPatchesJSON6902 []PatchJSON6902 `json:"kubeadmConfigPatchesJson6902,omitempty"`
KubeadmConfigPatchesJSON6902 []PatchJSON6902 `yaml:"kubeadmConfigPatchesJson6902,omitempty" json:"kubeadmConfigPatchesJson6902,omitempty"`
}

// TypeMeta partially copies apimachinery/pkg/apis/meta/v1.TypeMeta
// No need for a direct dependence; the fields are stable.
type TypeMeta struct {
Kind string `json:"kind,omitempty" yaml:"kind,omitempty"`
APIVersion string `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty"`
}

// Node contains settings for a node in the `kind` Cluster.
Expand All @@ -59,22 +61,22 @@ type Node struct {
// created by kind
//
// Defaults to "control-plane"
Role NodeRole `json:"role,omitempty"`
Role NodeRole `yaml:"role,omitempty" json:"role,omitempty"`

// Image is the node image to use when creating this node
// If unset a default image will be used, see defaults.Image
Image string `json:"image,omitempty"`
Image string `yaml:"image,omitempty" json:"image,omitempty"`

/* Advanced fields */

// TODO: cri-like types should be inline instead
// ExtraMounts describes additional mount points for the node container
// These may be used to bind a hostPath
ExtraMounts []cri.Mount `json:"extraMounts,omitempty"`
ExtraMounts []cri.Mount `yaml:"extraMounts,omitempty" json:"extraMounts,omitempty"`

// ExtraPortMappings describes additional port mappings for the node container
// binded to a host Port
ExtraPortMappings []cri.PortMapping `json:"extraPortMappings,omitempty"`
ExtraPortMappings []cri.PortMapping `yaml:"extraPortMappings,omitempty" json:"extraPortMappings,omitempty"`
}

// NodeRole defines possible role for nodes in a Kubernetes cluster managed by `kind`
Expand All @@ -93,24 +95,24 @@ const (
// Networking contains cluster wide network settings
type Networking struct {
// IPFamily is the network cluster model, currently it can be ipv4 or ipv6
IPFamily ClusterIPFamily `json:"ipFamily,omitempty"`
IPFamily ClusterIPFamily `yaml:"ipFamily,omitempty" json:"ipFamily,omitempty"`
// APIServerPort is the listen port on the host for the Kubernetes API Server
// Defaults to a random port on the host
APIServerPort int32 `json:"apiServerPort,omitempty"`
APIServerPort int32 `yaml:"apiServerPort,omitempty" json:"apiServerPort,omitempty"`
// APIServerAddress is the listen address on the host for the Kubernetes
// API Server. This should be an IP address.
//
// Defaults to 127.0.0.1
APIServerAddress string `json:"apiServerAddress,omitempty"`
APIServerAddress string `yaml:"apiServerAddress,omitempty" json:"apiServerAddress,omitempty"`
// PodSubnet is the CIDR used for pod IPs
// kind will select a default if unspecified
PodSubnet string `json:"podSubnet,omitempty"`
PodSubnet string `yaml:"podSubnet,omitempty" json:"podSubnet,omitempty"`
// ServiceSubnet is the CIDR used for services VIPs
// kind will select a default if unspecified for IPv6
ServiceSubnet string `json:"serviceSubnet,omitempty"`
ServiceSubnet string `yaml:"serviceSubnet,omitempty" json:"serviceSubnet,omitempty"`
// If DisableDefaultCNI is true, kind will not install the default CNI setup.
// Instead the user should install their own CNI after creating the cluster.
DisableDefaultCNI bool `json:"disableDefaultCNI,omitempty"`
DisableDefaultCNI bool `yaml:"disableDefaultCNI,omitempty" json:"disableDefaultCNI,omitempty"`
}

// ClusterIPFamily defines cluster network IP family
Expand All @@ -127,14 +129,14 @@ const (
// https://tools.ietf.org/html/rfc6902
type PatchJSON6902 struct {
// these fields specify the patch target resource
Group string `json:"group"`
Version string `json:"version"`
Kind string `json:"kind"`
Group string `yaml:"group" json:"group"`
Version string `yaml:"version" json:"version"`
Kind string `yaml:"kind" json:"kind"`
// Name and Namespace are optional
// NOTE: technically name is required now, but we default it elsewhere
// Third party users of this type / library would need to set it.
Name string `json:"name,omitempty"`
Namespace string `json:"namespace,omitempty"`
Name string `yaml:"name,omitempty" json:"name,omitempty"`
Namespace string `yaml:"name,omitempty" json:"namespace,omitempty"`
// Patch should contain the contents of the json patch as a string
Patch string `json:"patch"`
Patch string `yaml:"patch" json:"patch"`
}
25 changes: 16 additions & 9 deletions pkg/apis/config/v1alpha3/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 0 additions & 41 deletions pkg/apis/config/v1alpha3/zz_generated.default.go
View file
Open in desktop

This file was deleted.

5 changes: 2 additions & 3 deletions pkg/cluster/create/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ func WithConfigFile(path string) ClusterOption {
// WithV1Alpha3 configures creating the cluster with a v1alpha3 config
func WithV1Alpha3(cluster *v1alpha3.Cluster) ClusterOption {
return func(o *internaltypes.ClusterOptions) (*internaltypes.ClusterOptions, error) {
cfg, err := internalencoding.V1Alpha3ToInternal(cluster)
o.Config = cfg
return o, err
o.Config = internalencoding.V1Alpha3ToInternal(cluster)
return o, nil
}
}

Expand Down
Loading