Fix marshaling of taints, so an empty slice is preserved

Signed-off-by: Stefan Büringer [email protected]
kubernetes-sigs · Oct 5, 2022 · 447947c · 447947c
1 parent 82f4792
commit 447947c
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 4 deletions.
diff --git a/bootstrap/kubeadm/api/v1beta1/kubeadm_types.go b/bootstrap/kubeadm/api/v1beta1/kubeadm_types.go
@@ -17,6 +17,7 @@ limitations under the License.
 package v1beta1
 
 import (
+	"encoding/json"
 	"fmt"
 	"strings"
 
@@ -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.
@@ -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.

diff --git a/bootstrap/kubeadm/api/v1beta1/kubeadm_types_test.go b/bootstrap/kubeadm/api/v1beta1/kubeadm_types_test.go
@@ -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
@@ -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
@@ -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
@@ -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