⚒️ simplify file resolution, style

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

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/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,40 @@ 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
+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 cfg.Spec.Files {
 		in := cfg.Spec.Files[i]
-		switch {
-		case in.ContentFrom != nil:
-			file, err := r.resolveContent(ctx, cfg.Namespace, in)
+		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)
+			cfg.Spec.Files[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

@@ -492,7 +492,7 @@ func TestKubeadmConfigReconcile_SecretSource(t *testing.T) {
 		"defaults": {
 			input: bootstrapv1.File{
 				ContentFrom: &bootstrapv1.FileSource{
-					Secret: &bootstrapv1.SecretFileSource{
+					Secret: bootstrapv1.SecretFileSource{
 						Name: secretName,
 						Key:  secretDataKey,
 					},
@@ -509,7 +509,7 @@ func TestKubeadmConfigReconcile_SecretSource(t *testing.T) {
 		"failure": {
 			input: bootstrapv1.File{
 				ContentFrom: &bootstrapv1.FileSource{
-					Secret: &bootstrapv1.SecretFileSource{
+					Secret: bootstrapv1.SecretFileSource{
 						Name: secretName,
 						Key:  missingKey,
 					},