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.

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

[release-1.2] 🐛 Fix marshaling of taints, so an empty slice is preserved #7366

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 45 additions & 0 deletions bootstrap/kubeadm/api/v1beta1/kubeadm_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1beta1

import (
"encoding/json"
"fmt"
"strings"

Expand Down Expand Up @@ -214,6 +215,7 @@ type APIEndpoint struct {
}

// NodeRegistrationOptions holds fields that relate to registering a new control-plane or node to the cluster, either via "kubeadm init" or "kubeadm join".
// Note: The NodeRegistrationOptions struct has to be kept in sync with the structs in MarshalJSON.
type NodeRegistrationOptions struct {

// Name is the `.Metadata.Name` field of the Node API object that will be created in this `kubeadm init` or `kubeadm join` operation.
Expand Down Expand Up @@ -243,6 +245,49 @@ type NodeRegistrationOptions struct {
IgnorePreflightErrors []string `json:"ignorePreflightErrors,omitempty"`
}

// MarshalJSON marshals NodeRegistrationOptions in a way that an empty slice in Taints is preserved.
// Taints are then rendered as:
// * nil => omitted from the marshalled JSON
// * [] => rendered as empty array (`[]`)
// * [regular-array] => rendered as usual
// We have to do this as the regular Golang JSON marshalling would just omit
// the empty slice (xref: https://github.com/golang/go/issues/22480).
// Note: We can't re-use the original struct as that would lead to an infinite recursion.
// Note: The structs in this func have to be kept in sync with the NodeRegistrationOptions struct.
func (n *NodeRegistrationOptions) MarshalJSON() ([]byte, error) {
// Marshal an empty Taints slice array without omitempty so it's preserved.
if n.Taints != nil && len(n.Taints) == 0 {
return json.Marshal(struct {
Name string `json:"name,omitempty"`
CRISocket string `json:"criSocket,omitempty"`
Taints []corev1.Taint `json:"taints"`
KubeletExtraArgs map[string]string `json:"kubeletExtraArgs,omitempty"`
IgnorePreflightErrors []string `json:"ignorePreflightErrors,omitempty"`
}{
Name: n.Name,
CRISocket: n.CRISocket,
Taints: n.Taints,
KubeletExtraArgs: n.KubeletExtraArgs,
IgnorePreflightErrors: n.IgnorePreflightErrors,
})
}

// If Taints is nil or not empty we can use omitempty.
return json.Marshal(struct {
Name string `json:"name,omitempty"`
CRISocket string `json:"criSocket,omitempty"`
Taints []corev1.Taint `json:"taints,omitempty"`
KubeletExtraArgs map[string]string `json:"kubeletExtraArgs,omitempty"`
IgnorePreflightErrors []string `json:"ignorePreflightErrors,omitempty"`
}{
Name: n.Name,
CRISocket: n.CRISocket,
Taints: n.Taints,
KubeletExtraArgs: n.KubeletExtraArgs,
IgnorePreflightErrors: n.IgnorePreflightErrors,
})
}

// Networking contains elements describing cluster's networking configuration.
type Networking struct {
// ServiceSubnet is the subnet used by k8s services.
Expand Down
66 changes: 62 additions & 4 deletions bootstrap/kubeadm/api/v1beta1/kubeadm_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,67 @@ import (

. "github.com/onsi/gomega"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
)

func TestMarshalJSON(t *testing.T) {
func TestNodeRegistrationOptionsMarshalJSON(t *testing.T) {
var tests = []struct {
name string
opts NodeRegistrationOptions
expected string
}{
{
name: "marshal nil taints",
opts: NodeRegistrationOptions{
Name: "node-1",
CRISocket: "unix:///var/run/containerd/containerd.sock",
Taints: nil,
KubeletExtraArgs: map[string]string{"abc": "def"},
IgnorePreflightErrors: []string{"ignore-1"},
},
expected: `{"name":"node-1","criSocket":"unix:///var/run/containerd/containerd.sock","kubeletExtraArgs":{"abc":"def"},"ignorePreflightErrors":["ignore-1"]}`,
},
{
name: "marshal empty taints",
opts: NodeRegistrationOptions{
Name: "node-1",
CRISocket: "unix:///var/run/containerd/containerd.sock",
Taints: []corev1.Taint{},
KubeletExtraArgs: map[string]string{"abc": "def"},
IgnorePreflightErrors: []string{"ignore-1"},
},
expected: `{"name":"node-1","criSocket":"unix:///var/run/containerd/containerd.sock","taints":[],"kubeletExtraArgs":{"abc":"def"},"ignorePreflightErrors":["ignore-1"]}`,
},
{
name: "marshal regular taints",
opts: NodeRegistrationOptions{
Name: "node-1",
CRISocket: "unix:///var/run/containerd/containerd.sock",
Taints: []corev1.Taint{
{
Key: "key",
Value: "value",
Effect: "effect",
},
},
KubeletExtraArgs: map[string]string{"abc": "def"},
IgnorePreflightErrors: []string{"ignore-1"},
},
expected: `{"name":"node-1","criSocket":"unix:///var/run/containerd/containerd.sock","taints":[{"key":"key","value":"value","effect":"effect"}],"kubeletExtraArgs":{"abc":"def"},"ignorePreflightErrors":["ignore-1"]}`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

b, err := tt.opts.MarshalJSON()
g.Expect(err).NotTo(HaveOccurred())
g.Expect(string(b)).To(Equal(tt.expected))
})
}
}

func TestBootstrapTokenStringMarshalJSON(t *testing.T) {
var tests = []struct {
bts BootstrapTokenString
expected string
Expand All @@ -45,7 +103,7 @@ func TestMarshalJSON(t *testing.T) {
}
}

func TestUnmarshalJSON(t *testing.T) {
func TestBootstrapTokenStringUnmarshalJSON(t *testing.T) {
var tests = []struct {
input string
bts *BootstrapTokenString
Expand Down Expand Up @@ -76,7 +134,7 @@ func TestUnmarshalJSON(t *testing.T) {
}
}

func TestJSONRoundtrip(t *testing.T) {
func TestBootstrapTokenStringJSONRoundtrip(t *testing.T) {
var tests = []struct {
input string
bts *BootstrapTokenString
Expand Down Expand Up @@ -130,7 +188,7 @@ func roundtrip(input string, bts *BootstrapTokenString) error {
return nil
}

func TestTokenFromIDAndSecret(t *testing.T) {
func TestBootstrapTokenStringTokenFromIDAndSecret(t *testing.T) {
var tests = []struct {
bts BootstrapTokenString
expected string
Expand Down