⚒️ simplify file resolution, style

Signed-off-by: Alexander Eldeib <[email protected]>
Co-authored-by: Vince Prignano <[email protected]>

bootstrap/kubeadm/api/v1alpha2/conversion.go

@@ -40,6 +40,12 @@ func (src *KubeadmConfig) ConvertTo(dstRaw conversion.Hub) error {
 	dst.Spec.Verbosity = restored.Spec.Verbosity
 	dst.Spec.UseExperimentalRetryJoin = restored.Spec.UseExperimentalRetryJoin
 	dst.Spec.Files = restored.Spec.Files
+	for i := range restored.Spec.Files {
+		file := restored.Spec.Files[i]
+		if file.ContentFrom != nil {
+			dst.Spec.Files = append(dst.Spec.Files, file)
+		}
+	}
 
 	return nil
 }

bootstrap/kubeadm/api/v1alpha2/conversion_test.go

@@ -39,7 +39,7 @@ func TestConvertKubeadmConfig(t *testing.T) {
 					Files: []v1alpha3.File{
 						{
 							ContentFrom: &v1alpha3.FileSource{
-								Secret: &v1alpha3.SecretFileSource{
+								Secret: v1alpha3.SecretFileSource{
 									Name: "foo",
 									Key:  "bar",
 								},

bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go

@@ -185,7 +185,7 @@ type File struct {
 // sources of data for target systems should add them here.
 type FileSource struct {
 	// Secret represents a secret that should populate this file.
-	Secret *SecretFileSource `json:"secret"`
+	Secret SecretFileSource `json:"secret"`
 }
 
 // Adapts a Secret into a FileSource.

bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types_test.go

@@ -57,7 +57,7 @@ func TestClusterValidate(t *testing.T) {
 					Files: []File{
 						{
 							ContentFrom: &FileSource{
-								Secret: &SecretFileSource{
+								Secret: SecretFileSource{
 									Name: "foo",
 									Key:  "bar",
 								},
@@ -94,7 +94,7 @@ func TestClusterValidate(t *testing.T) {
 					Files: []File{
 						{
 							ContentFrom: &FileSource{
-								Secret: &SecretFileSource{
+								Secret: SecretFileSource{
 									Key: "bar",
 								},
 							},
@@ -115,7 +115,7 @@ func TestClusterValidate(t *testing.T) {
 					Files: []File{
 						{
 							ContentFrom: &FileSource{
-								Secret: &SecretFileSource{
+								Secret: SecretFileSource{
 									Name: "foo",
 								},
 							},

bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_webhook.go

@@ -45,24 +45,24 @@ var _ webhook.Validator = &KubeadmConfig{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (c *KubeadmConfig) ValidateCreate() error {
-	return c.validate()
+	return c.Spec.validate(c.Name)
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (c *KubeadmConfig) ValidateUpdate(old runtime.Object) error {
-	return c.validate()
+	return c.Spec.validate(c.Name)
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
 func (c *KubeadmConfig) ValidateDelete() error {
 	return nil
 }
 
-func (c *KubeadmConfig) validate() error {
+func (c *KubeadmConfigSpec) validate(name string) error {
 	var allErrs field.ErrorList
 
-	for i := range c.Spec.Files {
-		file := c.Spec.Files[i]
+	for i := range c.Files {
+		file := c.Files[i]
 		if file.Content != "" && file.ContentFrom != nil {
 			allErrs = append(
 				allErrs,
@@ -76,7 +76,7 @@ func (c *KubeadmConfig) validate() error {
 		// n.b.: if we ever add types besides Secret as a ContentFrom
 		// Source, we must add webhook validation here for one of the
 		// sources being non-nil.
-		if file.ContentFrom != nil && file.ContentFrom.Secret != nil {
+		if file.ContentFrom != nil {
 			if file.ContentFrom.Secret.Name == "" {
 				allErrs = append(
 					allErrs,
@@ -103,5 +103,5 @@ func (c *KubeadmConfig) validate() error {
 	if len(allErrs) == 0 {
 		return nil
 	}
-	return apierrors.NewInvalid(GroupVersion.WithKind("KubeadmConfig").GroupKind(), c.Name, allErrs)
+	return apierrors.NewInvalid(GroupVersion.WithKind("KubeadmConfig").GroupKind(), name, allErrs)
 }

bootstrap/kubeadm/config/webhook/webhookcainjection_patch.yaml

@@ -1,11 +1,3 @@
-# This patch add annotation to admission webhook config and
-# the variables $(CERTIFICATE_NAMESPACE) and $(CERTIFICATE_NAME) will be substituted by kustomize.
-# apiVersion: admissionregistration.k8s.io/v1beta1
-# kind: MutatingWebhookConfiguration
-# metadata:
-#   name: mutating-webhook-configuration
-#   annotations:
-#     cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME)
 ---
 apiVersion: admissionregistration.k8s.io/v1beta1
 kind: ValidatingWebhookConfiguration

bootstrap/kubeadm/controllers/kubeadmconfig_controller.go

@@ -366,16 +366,12 @@ func (r *KubeadmConfigReconciler) handleClusterNotInitialized(ctx context.Contex
 		verbosityFlag = fmt.Sprintf("--v %s", strconv.Itoa(int(*scope.Config.Spec.Verbosity)))
 	}
 
-	files := append(certificates.AsFiles(), scope.Config.Spec.Files...)
-
-	additionalFiles, err := r.resolveFiles(ctx, scope.Config)
+	files, err := r.resolveFiles(ctx, scope.Config, certificates.AsFiles()...)
 	if err != nil {
 		scope.Error(err, "Failed to resolve files")
 		return ctrl.Result{}, err
 	}
 
-	files = append(files, additionalFiles...)
-
 	cloudInitData, err := cloudinit.NewInitControlPlane(&cloudinit.ControlPlaneInput{
 		BaseUserData: cloudinit.BaseUserData{
 			AdditionalFiles:     files,
@@ -444,14 +440,12 @@ func (r *KubeadmConfigReconciler) joinWorker(ctx context.Context, scope *Scope)
 		verbosityFlag = fmt.Sprintf("--v %s", strconv.Itoa(int(*scope.Config.Spec.Verbosity)))
 	}
 
-	additionalFiles, err := r.resolveFiles(ctx, scope.Config)
+	files, err := r.resolveFiles(ctx, scope.Config)
 	if err != nil {
 		scope.Error(err, "Failed to resolve files")
 		return ctrl.Result{}, err
 	}
 
-	var files = append(scope.Config.Spec.Files, additionalFiles...)
-
 	cloudJoinData, err := cloudinit.NewNode(&cloudinit.NodeInput{
 		BaseUserData: cloudinit.BaseUserData{
 			AdditionalFiles:      files,
@@ -518,21 +512,15 @@ func (r *KubeadmConfigReconciler) joinControlplane(ctx context.Context, scope *S
 
 	verbosityFlag := ""
 	if scope.Config.Spec.Verbosity != nil {
-		verbosityFlag =
-
-			fmt.Sprintf("--v %s", strconv.Itoa(int(*scope.Config.Spec.Verbosity)))
+		verbosityFlag = fmt.Sprintf("--v %s", strconv.Itoa(int(*scope.Config.Spec.Verbosity)))
 	}
 
-	var files = append(certificates.AsFiles(), scope.Config.Spec.Files...)
-
-	additionalFiles, err := r.resolveFiles(ctx, scope.Config)
+	files, err := r.resolveFiles(ctx, scope.Config, certificates.AsFiles()...)
 	if err != nil {
 		scope.Error(err, "Failed to resolve files")
 		return ctrl.Result{}, err
 	}
 
-	files = append(files, additionalFiles...)
-
 	cloudJoinData, err := cloudinit.NewJoinControlPlane(&cloudinit.ControlPlaneJoinInput{
 		JoinConfiguration: joinData,
 		Certificates:      certificates,
@@ -561,64 +549,41 @@ func (r *KubeadmConfigReconciler) joinControlplane(ctx context.Context, scope *S
 
 // resolveFiles maps .Spec.Files into cloudinit.Files, resolving any object references
 // along the way.
-func (r *KubeadmConfigReconciler) resolveFiles(ctx context.Context, cfg *bootstrapv1.KubeadmConfig) ([]bootstrapv1.File, error) {
-	var converted []bootstrapv1.File
-
-	for i := range cfg.Spec.Files {
-		in := cfg.Spec.Files[i]
-		switch {
-		case in.ContentFrom != nil:
-			file, err := r.resolveContent(ctx, cfg.Namespace, in)
+func (r *KubeadmConfigReconciler) resolveFiles(ctx context.Context, cfg *bootstrapv1.KubeadmConfig, merge ...bootstrapv1.File) ([]bootstrapv1.File, error) {
+	collected := append(cfg.Spec.Files, merge...)
+
+	for i := range collected {
+		in := collected[i]
+		if in.ContentFrom != nil {
+			data, err := r.resolveSecretFileContent(ctx, cfg.Namespace, in)
 			if err != nil {
 				return nil, errors.Wrapf(err, "failed to resolve file source")
 			}
-			converted = append(converted, file)
-		case in.Content != "":
-			converted = append(converted, in)
-		default:
-			return nil, fmt.Errorf("could not find content to populate file: %s", in.Path)
+			in.ContentFrom = nil
+			in.Content = string(data)
+			collected[i] = in
 		}
-	}
 
-	return converted, nil
-}
-
-func (r *KubeadmConfigReconciler) resolveContent(ctx context.Context, ns string, source bootstrapv1.File) (bootstrapv1.File, error) {
-	switch {
-	case source.ContentFrom.Secret != nil:
-		return r.resolveSecretFileContent(ctx, ns, source)
-	default:
-		return bootstrapv1.File{}, errors.New("could not find non-nil source for file content")
 	}
+
+	return collected, nil
 }
 
 // resolveSecretFileContent returns file content fetched from a referenced secret object.
-func (r *KubeadmConfigReconciler) resolveSecretFileContent(ctx context.Context, ns string, source bootstrapv1.File) (bootstrapv1.File, error) {
-	var secret corev1.Secret
-	nn := types.NamespacedName{Namespace: ns, Name: source.ContentFrom.Secret.Name}
-	if err := r.Client.Get(ctx, nn, &secret); err != nil {
+func (r *KubeadmConfigReconciler) resolveSecretFileContent(ctx context.Context, ns string, source bootstrapv1.File) ([]byte, error) {
+	secret := &corev1.Secret{}
+	key := types.NamespacedName{Namespace: ns, Name: source.ContentFrom.Secret.Name}
+	if err := r.Client.Get(ctx, key, secret); err != nil {
 		if apierrors.IsNotFound(err) {
-			return bootstrapv1.File{}, errors.Wrapf(err, "secret not found: %s", nn)
+			return nil, errors.Wrapf(err, "secret not found: %s", key)
 		}
-		return bootstrapv1.File{}, errors.Wrapf(err, "getting Secret %s", nn)
+		return nil, errors.Wrapf(err, "failed to retrieve Secret %q", key)
 	}
-
-	return secretToFile(&secret, source)
-}
-
-func secretToFile(secret *corev1.Secret, source bootstrapv1.File) (bootstrapv1.File, error) {
 	data, ok := secret.Data[source.ContentFrom.Secret.Key]
 	if !ok {
-		return bootstrapv1.File{}, fmt.Errorf("secret references non-existent secret key: %s", source.ContentFrom.Secret.Key)
+		return nil, errors.Errorf("secret references non-existent secret key: %q", source.ContentFrom.Secret.Key)
 	}
-
-	return bootstrapv1.File{
-		Owner:       source.Owner,
-		Permissions: source.Permissions,
-		Encoding:    source.Encoding,
-		Content:     string(data),
-		Path:        source.Path,
-	}, nil
+	return data, nil
 }
 
 // ClusterToKubeadmConfigs is a handler.ToRequestsFunc to be used to enqeue

bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go

@@ -24,7 +24,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/google/go-cmp/cmp"
 	. "github.com/onsi/gomega"
 
 	corev1 "k8s.io/api/core/v1"
@@ -472,83 +471,6 @@ func TestKubeadmConfigReconciler_Reconcile_GenerateCloudConfigData(t *testing.T)
 	g.Expect(err).NotTo(HaveOccurred())
 }
 
-func TestKubeadmConfigReconcile_SecretSource(t *testing.T) {
-	var (
-		secretName       string = "foo"
-		secretDataKey    string = "/foo/bar"
-		missingKey       string = "/foo/bar/baz"
-		secretContent    string = "fooBarBaz"
-		fileOwner        string = "baz"
-		filePermissions  string = "foobar"
-		filePath         string = "bazbar"
-		fileRemappedPath string = "foobazbar"
-	)
-
-	tests := map[string]struct {
-		input         bootstrapv1.File
-		output        bootstrapv1.File
-		expectFailure bool
-	}{
-		"defaults": {
-			input: bootstrapv1.File{
-				ContentFrom: &bootstrapv1.FileSource{
-					Secret: &bootstrapv1.SecretFileSource{
-						Name: secretName,
-						Key:  secretDataKey,
-					},
-				},
-				Path:  filePath,
-				Owner: fileOwner,
-			},
-			output: bootstrapv1.File{
-				Content: secretContent,
-				Path:    filePath,
-				Owner:   fileOwner,
-			},
-		},
-		"failure": {
-			input: bootstrapv1.File{
-				ContentFrom: &bootstrapv1.FileSource{
-					Secret: &bootstrapv1.SecretFileSource{
-						Name: secretName,
-						Key:  missingKey,
-					},
-				},
-				Path:        fileRemappedPath,
-				Owner:       fileOwner,
-				Permissions: filePermissions,
-			},
-			output:        bootstrapv1.File{},
-			expectFailure: true,
-		},
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: secretName,
-		},
-		Data: map[string][]byte{
-			secretDataKey: []byte(secretContent),
-		},
-	}
-
-	for name, tc := range tests {
-		t.Run(name, func(t *testing.T) {
-			g := NewWithT(t)
-			files, err := secretToFile(secret, tc.input)
-			if tc.expectFailure {
-				g.Expect(err).To(HaveOccurred())
-			} else {
-				g.Expect(err).NotTo(HaveOccurred())
-				diff := cmp.Diff(tc.output, files)
-				if diff != "" {
-					t.Fatalf(diff)
-				}
-			}
-		})
-	}
-}
-
 // If a control plane has no JoinConfiguration, then we will create a default and no error will occur
 func TestKubeadmConfigReconciler_Reconcile_ErrorIfJoiningControlPlaneHasInvalidConfiguration(t *testing.T) {
 	g := NewWithT(t)

go.mod

@@ -11,7 +11,6 @@ require (
 	github.com/evanphx/json-patch v4.5.0+incompatible
 	github.com/go-logr/logr v0.1.0
 	github.com/gogo/protobuf v1.3.1
-	github.com/google/go-cmp v0.4.0
 	github.com/google/go-github v17.0.0+incompatible
 	github.com/google/go-querystring v1.0.0 // indirect
 	github.com/google/gofuzz v1.1.0