From 4f353d3a8b542dcaf7a0ecca056c52a69543ffb1 Mon Sep 17 00:00:00 2001 From: Alexander Eldeib Date: Thu, 21 May 2020 19:39:41 -0700 Subject: [PATCH 1/4] :sparkles: implement api, controllers, and webhook Co-authored-by: Vince Prignano Signed-off-by: Alexander Eldeib --- bootstrap/kubeadm/api/v1alpha2/conversion.go | 6 ++ .../v1alpha3/kubeadmbootstrapconfig_types.go | 27 ++++- .../api/v1alpha3/kubeadmconfig_webhook.go | 84 ++++++++++++++- .../controllers/kubeadmconfig_controller.go | 101 +++++++++++++++++- 4 files changed, 211 insertions(+), 7 deletions(-) diff --git a/bootstrap/kubeadm/api/v1alpha2/conversion.go b/bootstrap/kubeadm/api/v1alpha2/conversion.go index 33e121058dc4..9cf4439d09a4 100644 --- a/bootstrap/kubeadm/api/v1alpha2/conversion.go +++ b/bootstrap/kubeadm/api/v1alpha2/conversion.go @@ -39,6 +39,7 @@ func (src *KubeadmConfig) ConvertTo(dstRaw conversion.Hub) error { dst.Status.DataSecretName = restored.Status.DataSecretName dst.Spec.Verbosity = restored.Spec.Verbosity dst.Spec.UseExperimentalRetryJoin = restored.Spec.UseExperimentalRetryJoin + dst.Spec.Files = restored.Spec.Files return nil } @@ -129,3 +130,8 @@ func Convert_v1alpha2_KubeadmConfigSpec_To_v1alpha3_KubeadmConfigSpec(in *Kubead func Convert_v1alpha3_KubeadmConfigSpec_To_v1alpha2_KubeadmConfigSpec(in *kubeadmbootstrapv1alpha3.KubeadmConfigSpec, out *KubeadmConfigSpec, s apiconversion.Scope) error { return autoConvert_v1alpha3_KubeadmConfigSpec_To_v1alpha2_KubeadmConfigSpec(in, out, s) } + +// Convert_v1alpha3_File_To_v1alpha2_File converts from the Hub version (v1alpha3) of the File to this version. +func Convert_v1alpha3_File_To_v1alpha2_File(in *kubeadmbootstrapv1alpha3.File, out *File, s apiconversion.Scope) error { + return autoConvert_v1alpha3_File_To_v1alpha2_File(in, out, s) +} diff --git a/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go b/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go index 5a99cca6150c..1877ebcb53d9 100644 --- a/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go +++ b/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go @@ -172,7 +172,32 @@ type File struct { Encoding Encoding `json:"encoding,omitempty"` // Content is the actual content of the file. - Content string `json:"content"` + // +optional + Content string `json:"content,omitempty"` + + // ContentFrom is a referenced source of content to populate the file. + // +optional + ContentFrom *FileSource `json:"contentFrom,omitempty"` +} + +// FileSource is a union of all possible external source types for file data. +// Only one field may be populated in any given instance. Developers adding new +// 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"` +} + +// Adapts a Secret into a FileSource. +// +// The contents of the target Secret's Data field will be presented +// as files using the keys in the Data field as the file names. +type SecretFileSource struct { + // Name of the secret in the KubeadmBootstrapConfig's namespace to use. + Name string `json:"name"` + + // Key is the key in the secret's data map for this value. + Key string `json:"key"` } // User defines the input for a generated user in cloud-init. diff --git a/bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_webhook.go b/bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_webhook.go index d08b3e1598d8..0bb1f30308b8 100644 --- a/bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_webhook.go +++ b/bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_webhook.go @@ -17,11 +17,91 @@ limitations under the License. package v1alpha3 import ( + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook" +) + +var ( + ConflictingFileSourceMsg = "only one of content of contentFrom may be specified for a single file" + MissingFileSourceMsg = "source for file content must be specified if contenFrom is non-nil" + MissingSecretNameMsg = "secret file source must specify non-empty secret name" + MissingSecretKeyMsg = "secret file source must specify non-empty secret key" ) -func (r *KubeadmConfig) SetupWebhookWithManager(mgr ctrl.Manager) error { +func (c *KubeadmConfig) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). - For(r). + For(c). Complete() } + +// +kubebuilder:webhook:verbs=create;update,path=/validate-bootstrap-cluster-x-k8s-io-v1alpha3-kubeadmconfig,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=bootstrap.cluster.x-k8s.io,resources=kubeadmconfigs,versions=v1alpha3,name=validation.kubeadmconfig.bootstrap.cluster.x-k8s.io,sideEffects=None + +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() +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type +func (c *KubeadmConfig) ValidateUpdate(old runtime.Object) error { + return c.validate() +} + +// 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 { + var allErrs field.ErrorList + + for i := range c.Spec.Files { + file := c.Spec.Files[i] + if file.Content != "" && file.ContentFrom != nil { + allErrs = append( + allErrs, + field.Invalid( + field.NewPath("spec", "files", fmt.Sprintf("%d", i)), + file, + ConflictingFileSourceMsg, + ), + ) + } + // 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.Secret.Name == "" { + allErrs = append( + allErrs, + field.Invalid( + field.NewPath("spec", "files", fmt.Sprintf("%d", i), "contentFrom", "secret", "name"), + file, + MissingSecretNameMsg, + ), + ) + } + if file.ContentFrom.Secret.Key == "" { + allErrs = append( + allErrs, + field.Invalid( + field.NewPath("spec", "files", fmt.Sprintf("%d", i), "contentFrom", "secret", "key"), + file, + MissingSecretKeyMsg, + ), + ) + } + } + } + + if len(allErrs) == 0 { + return nil + } + return apierrors.NewInvalid(GroupVersion.WithKind("KubeadmConfig").GroupKind(), c.Name, allErrs) +} diff --git a/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go b/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go index 8001b364e6bf..9369719f1333 100644 --- a/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go +++ b/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go @@ -29,6 +29,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" @@ -365,9 +366,19 @@ 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) + 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: scope.Config.Spec.Files, + AdditionalFiles: files, NTP: scope.Config.Spec.NTP, PreKubeadmCommands: scope.Config.Spec.PreKubeadmCommands, PostKubeadmCommands: scope.Config.Spec.PostKubeadmCommands, @@ -433,9 +444,17 @@ 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) + 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: scope.Config.Spec.Files, + AdditionalFiles: files, NTP: scope.Config.Spec.NTP, PreKubeadmCommands: scope.Config.Spec.PreKubeadmCommands, PostKubeadmCommands: scope.Config.Spec.PostKubeadmCommands, @@ -499,14 +518,26 @@ 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) + 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, BaseUserData: cloudinit.BaseUserData{ - AdditionalFiles: scope.Config.Spec.Files, + AdditionalFiles: files, NTP: scope.Config.Spec.NTP, PreKubeadmCommands: scope.Config.Spec.PreKubeadmCommands, PostKubeadmCommands: scope.Config.Spec.PostKubeadmCommands, @@ -528,6 +559,68 @@ func (r *KubeadmConfigReconciler) joinControlplane(ctx context.Context, scope *S return ctrl.Result{}, nil } +// 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) + 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) + } + } + + 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") + } +} + +// 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 { + if apierrors.IsNotFound(err) { + return bootstrapv1.File{}, errors.Wrapf(err, "secret not found: %s", nn) + } + return bootstrapv1.File{}, errors.Wrapf(err, "getting Secret %s", nn) + } + + 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 bootstrapv1.File{ + Owner: source.Owner, + Permissions: source.Permissions, + Encoding: source.Encoding, + Content: string(data), + Path: source.Path, + }, nil +} + // ClusterToKubeadmConfigs is a handler.ToRequestsFunc to be used to enqeue // requests for reconciliation of KubeadmConfigs. func (r *KubeadmConfigReconciler) ClusterToKubeadmConfigs(o handler.MapObject) []ctrl.Request { From 3d1345d9d8be278f3aaf4736d9535ac47f6a02ef Mon Sep 17 00:00:00 2001 From: Alexander Eldeib Date: Thu, 21 May 2020 19:40:35 -0700 Subject: [PATCH 2/4] :hammer_and_pick: add Go tests --- .../kubeadm/api/v1alpha2/conversion_test.go | 16 +- .../api/v1alpha2/zz_generated.conversion.go | 40 +++-- .../kubeadmbootstrapconfig_types_test.go | 157 +++++++++++++----- .../api/v1alpha3/zz_generated.deepcopy.go | 46 ++++- .../kubeadmconfig_controller_test.go | 78 +++++++++ go.mod | 1 + 6 files changed, 277 insertions(+), 61 deletions(-) diff --git a/bootstrap/kubeadm/api/v1alpha2/conversion_test.go b/bootstrap/kubeadm/api/v1alpha2/conversion_test.go index 6e9c93837f06..44f0a23846f5 100644 --- a/bootstrap/kubeadm/api/v1alpha2/conversion_test.go +++ b/bootstrap/kubeadm/api/v1alpha2/conversion_test.go @@ -35,7 +35,21 @@ func TestConvertKubeadmConfig(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "hub", }, - Spec: v1alpha3.KubeadmConfigSpec{}, + Spec: v1alpha3.KubeadmConfigSpec{ + Files: []v1alpha3.File{ + { + ContentFrom: &v1alpha3.FileSource{ + Secret: &v1alpha3.SecretFileSource{ + Name: "foo", + Key: "bar", + }, + }, + }, + { + Content: "baz", + }, + }, + }, Status: v1alpha3.KubeadmConfigStatus{ Ready: true, DataSecretName: pointer.StringPtr("secret-data"), diff --git a/bootstrap/kubeadm/api/v1alpha2/zz_generated.conversion.go b/bootstrap/kubeadm/api/v1alpha2/zz_generated.conversion.go index 147e2f07ddb8..a828996deb0d 100644 --- a/bootstrap/kubeadm/api/v1alpha2/zz_generated.conversion.go +++ b/bootstrap/kubeadm/api/v1alpha2/zz_generated.conversion.go @@ -41,11 +41,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1alpha3.File)(nil), (*File)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha3_File_To_v1alpha2_File(a.(*v1alpha3.File), b.(*File), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*KubeadmConfig)(nil), (*v1alpha3.KubeadmConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha2_KubeadmConfig_To_v1alpha3_KubeadmConfig(a.(*KubeadmConfig), b.(*v1alpha3.KubeadmConfig), scope) }); err != nil { @@ -136,6 +131,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha3.File)(nil), (*File)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha3_File_To_v1alpha2_File(a.(*v1alpha3.File), b.(*File), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1alpha3.KubeadmConfigSpec)(nil), (*KubeadmConfigSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_KubeadmConfigSpec_To_v1alpha2_KubeadmConfigSpec(a.(*v1alpha3.KubeadmConfigSpec), b.(*KubeadmConfigSpec), scope) }); err != nil { @@ -169,14 +169,10 @@ func autoConvert_v1alpha3_File_To_v1alpha2_File(in *v1alpha3.File, out *File, s out.Permissions = in.Permissions out.Encoding = Encoding(in.Encoding) out.Content = in.Content + // WARNING: in.ContentFrom requires manual conversion: does not exist in peer-type return nil } -// Convert_v1alpha3_File_To_v1alpha2_File is an autogenerated conversion function. -func Convert_v1alpha3_File_To_v1alpha2_File(in *v1alpha3.File, out *File, s conversion.Scope) error { - return autoConvert_v1alpha3_File_To_v1alpha2_File(in, out, s) -} - func autoConvert_v1alpha2_KubeadmConfig_To_v1alpha3_KubeadmConfig(in *KubeadmConfig, out *v1alpha3.KubeadmConfig, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_v1alpha2_KubeadmConfigSpec_To_v1alpha3_KubeadmConfigSpec(&in.Spec, &out.Spec, s); err != nil { @@ -255,7 +251,17 @@ func autoConvert_v1alpha2_KubeadmConfigSpec_To_v1alpha3_KubeadmConfigSpec(in *Ku out.ClusterConfiguration = (*v1beta1.ClusterConfiguration)(unsafe.Pointer(in.ClusterConfiguration)) out.InitConfiguration = (*v1beta1.InitConfiguration)(unsafe.Pointer(in.InitConfiguration)) out.JoinConfiguration = (*v1beta1.JoinConfiguration)(unsafe.Pointer(in.JoinConfiguration)) - out.Files = *(*[]v1alpha3.File)(unsafe.Pointer(&in.Files)) + if in.Files != nil { + in, out := &in.Files, &out.Files + *out = make([]v1alpha3.File, len(*in)) + for i := range *in { + if err := Convert_v1alpha2_File_To_v1alpha3_File(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Files = nil + } out.PreKubeadmCommands = *(*[]string)(unsafe.Pointer(&in.PreKubeadmCommands)) out.PostKubeadmCommands = *(*[]string)(unsafe.Pointer(&in.PostKubeadmCommands)) out.Users = *(*[]v1alpha3.User)(unsafe.Pointer(&in.Users)) @@ -268,7 +274,17 @@ func autoConvert_v1alpha3_KubeadmConfigSpec_To_v1alpha2_KubeadmConfigSpec(in *v1 out.ClusterConfiguration = (*v1beta1.ClusterConfiguration)(unsafe.Pointer(in.ClusterConfiguration)) out.InitConfiguration = (*v1beta1.InitConfiguration)(unsafe.Pointer(in.InitConfiguration)) out.JoinConfiguration = (*v1beta1.JoinConfiguration)(unsafe.Pointer(in.JoinConfiguration)) - out.Files = *(*[]File)(unsafe.Pointer(&in.Files)) + if in.Files != nil { + in, out := &in.Files, &out.Files + *out = make([]File, len(*in)) + for i := range *in { + if err := Convert_v1alpha3_File_To_v1alpha2_File(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Files = nil + } out.PreKubeadmCommands = *(*[]string)(unsafe.Pointer(&in.PreKubeadmCommands)) out.PostKubeadmCommands = *(*[]string)(unsafe.Pointer(&in.PostKubeadmCommands)) out.Users = *(*[]User)(unsafe.Pointer(&in.Users)) diff --git a/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types_test.go b/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types_test.go index 2fa9d415f902..b084e67048a3 100644 --- a/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types_test.go +++ b/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types_test.go @@ -17,62 +17,127 @@ limitations under the License. package v1alpha3 import ( - . "github.com/onsi/ginkgo" + "testing" + . "github.com/onsi/gomega" - "golang.org/x/net/context" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" ) // These tests are written in BDD-style using Ginkgo framework. Refer to // http://onsi.github.io/ginkgo to learn more. -var _ = Describe("KubeadmConfig", func() { - var ( - key types.NamespacedName - created, fetched *KubeadmConfig - ) - - BeforeEach(func() { - // Add any setup steps that needs to be executed before each test - }) - - AfterEach(func() { - // Add any teardown steps that needs to be executed after each test - }) - - // Add Tests for OpenAPI validation (or additional CRD features) specified in - // your API definition. - // Avoid adding tests for vanilla CRUD operations because they would - // test Kubernetes API server, which isn't the goal here. - Context("Create API", func() { - - It("should create an object successfully", func() { - - key = types.NamespacedName{ - Name: "foo", - Namespace: "default", - } - created = &KubeadmConfig{ +func TestClusterValidate(t *testing.T) { + cases := map[string]struct { + in *KubeadmConfig + expectErr bool + }{ + "valid content": { + in: &KubeadmConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "default", + }, + Spec: KubeadmConfigSpec{ + Files: []File{ + { + Content: "foo", + }, + }, + }, + }, + }, + "valid contentFrom": { + in: &KubeadmConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "default", + }, + Spec: KubeadmConfigSpec{ + Files: []File{ + { + ContentFrom: &FileSource{ + Secret: &SecretFileSource{ + Name: "foo", + Key: "bar", + }, + }, + }, + }, + }, + }, + }, + "invalid content and contentFrom": { + in: &KubeadmConfig{ ObjectMeta: metav1.ObjectMeta{ - Name: "foo", + Name: "baz", Namespace: "default", }, + Spec: KubeadmConfigSpec{ + Files: []File{ + { + ContentFrom: &FileSource{}, + Content: "foo", + }, + }, + }, + }, + expectErr: true, + }, + "invalid contentFrom without name": { + in: &KubeadmConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "default", + }, + Spec: KubeadmConfigSpec{ + Files: []File{ + { + ContentFrom: &FileSource{ + Secret: &SecretFileSource{ + Key: "bar", + }, + }, + Content: "foo", + }, + }, + }, + }, + expectErr: true, + }, + "invalid contentFrom without key": { + in: &KubeadmConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Namespace: "default", + }, + Spec: KubeadmConfigSpec{ + Files: []File{ + { + ContentFrom: &FileSource{ + Secret: &SecretFileSource{ + Name: "foo", + }, + }, + Content: "foo", + }, + }, + }, + }, + expectErr: true, + }, + } + + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + g := NewWithT(t) + if tt.expectErr { + g.Expect(tt.in.ValidateCreate()).NotTo(Succeed()) + g.Expect(tt.in.ValidateUpdate(nil)).NotTo(Succeed()) + } else { + g.Expect(tt.in.ValidateCreate()).To(Succeed()) + g.Expect(tt.in.ValidateUpdate(nil)).To(Succeed()) } - - By("creating an API obj") - Expect(k8sClient.Create(context.TODO(), created)).To(Succeed()) - - fetched = &KubeadmConfig{} - Expect(k8sClient.Get(context.TODO(), key, fetched)).To(Succeed()) - Expect(fetched).To(Equal(created)) - - By("deleting the created object") - Expect(k8sClient.Delete(context.TODO(), created)).To(Succeed()) - Expect(k8sClient.Get(context.TODO(), key, created)).ToNot(Succeed()) }) - - }) - -}) + } +} diff --git a/bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go b/bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go index ec9804d5aab5..fdcfe797b439 100644 --- a/bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go +++ b/bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go @@ -21,13 +21,18 @@ limitations under the License. package v1alpha3 import ( - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *File) DeepCopyInto(out *File) { *out = *in + if in.ContentFrom != nil { + in, out := &in.ContentFrom, &out.ContentFrom + *out = new(FileSource) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new File. @@ -40,6 +45,26 @@ func (in *File) DeepCopy() *File { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FileSource) DeepCopyInto(out *FileSource) { + *out = *in + if in.Secret != nil { + in, out := &in.Secret, &out.Secret + *out = new(SecretFileSource) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FileSource. +func (in *FileSource) DeepCopy() *FileSource { + if in == nil { + return nil + } + out := new(FileSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KubeadmConfig) DeepCopyInto(out *KubeadmConfig) { *out = *in @@ -120,7 +145,9 @@ func (in *KubeadmConfigSpec) DeepCopyInto(out *KubeadmConfigSpec) { if in.Files != nil { in, out := &in.Files, &out.Files *out = make([]File, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.PreKubeadmCommands != nil { in, out := &in.PreKubeadmCommands, &out.PreKubeadmCommands @@ -301,6 +328,21 @@ func (in *NTP) DeepCopy() *NTP { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretFileSource) DeepCopyInto(out *SecretFileSource) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretFileSource. +func (in *SecretFileSource) DeepCopy() *SecretFileSource { + if in == nil { + return nil + } + out := new(SecretFileSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *User) DeepCopyInto(out *User) { *out = *in diff --git a/bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go b/bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go index 2d0745c562e5..ccec4394c233 100644 --- a/bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go +++ b/bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go @@ -24,6 +24,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" @@ -471,6 +472,83 @@ 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) diff --git a/go.mod b/go.mod index 3cacb81226e5..f2ab225e1498 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ 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 From 574fd86663665bb19e0cc7f9d475f9f69ab7fd16 Mon Sep 17 00:00:00 2001 From: Alexander Eldeib Date: Thu, 21 May 2020 19:40:41 -0700 Subject: [PATCH 3/4] :hammer_and_pick: update generated CRDs --- ...strap.cluster.x-k8s.io_kubeadmconfigs.yaml | 24 +++++++++++++++- ...uster.x-k8s.io_kubeadmconfigtemplates.yaml | 24 +++++++++++++++- .../kubeadm/config/webhook/kustomization.yaml | 2 +- .../kubeadm/config/webhook/manifests.yaml | 28 +++++++++++++++++++ .../webhook/webhookcainjection_patch.yaml | 14 +++++----- ...cluster.x-k8s.io_kubeadmcontrolplanes.yaml | 24 +++++++++++++++- 6 files changed, 105 insertions(+), 11 deletions(-) diff --git a/bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigs.yaml b/bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigs.yaml index 556d7edf2572..082bad5f48f3 100644 --- a/bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigs.yaml +++ b/bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigs.yaml @@ -1169,6 +1169,29 @@ spec: content: description: Content is the actual content of the file. type: string + contentFrom: + description: ContentFrom is a referenced source of content to + populate the file. + properties: + secret: + description: Secret represents a secret that should populate + this file. + properties: + key: + description: Key is the key in the secret's data map + for this value. + type: string + name: + description: Name of the secret in the KubeadmBootstrapConfig's + namespace to use. + type: string + required: + - key + - name + type: object + required: + - secret + type: object encoding: description: Encoding specifies the encoding of the file contents. enum: @@ -1189,7 +1212,6 @@ spec: to the file, e.g. "0640". type: string required: - - content - path type: object type: array diff --git a/bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigtemplates.yaml b/bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigtemplates.yaml index a812c7388a0e..3fa17261f61e 100644 --- a/bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigtemplates.yaml +++ b/bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigtemplates.yaml @@ -1227,6 +1227,29 @@ spec: content: description: Content is the actual content of the file. type: string + contentFrom: + description: ContentFrom is a referenced source of content + to populate the file. + properties: + secret: + description: Secret represents a secret that should + populate this file. + properties: + key: + description: Key is the key in the secret's + data map for this value. + type: string + name: + description: Name of the secret in the KubeadmBootstrapConfig's + namespace to use. + type: string + required: + - key + - name + type: object + required: + - secret + type: object encoding: description: Encoding specifies the encoding of the file contents. @@ -1248,7 +1271,6 @@ spec: assign to the file, e.g. "0640". type: string required: - - content - path type: object type: array diff --git a/bootstrap/kubeadm/config/webhook/kustomization.yaml b/bootstrap/kubeadm/config/webhook/kustomization.yaml index cf9bcbdd1d04..23314b7710e3 100644 --- a/bootstrap/kubeadm/config/webhook/kustomization.yaml +++ b/bootstrap/kubeadm/config/webhook/kustomization.yaml @@ -11,7 +11,7 @@ configurations: patchesStrategicMerge: - manager_webhook_patch.yaml -# - webhookcainjection_patch.yaml +- webhookcainjection_patch.yaml vars: # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. diff --git a/bootstrap/kubeadm/config/webhook/manifests.yaml b/bootstrap/kubeadm/config/webhook/manifests.yaml index e69de29bb2d1..a343c138a9eb 100644 --- a/bootstrap/kubeadm/config/webhook/manifests.yaml +++ b/bootstrap/kubeadm/config/webhook/manifests.yaml @@ -0,0 +1,28 @@ + +--- +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: ValidatingWebhookConfiguration +metadata: + creationTimestamp: null + name: validating-webhook-configuration +webhooks: +- clientConfig: + caBundle: Cg== + service: + name: webhook-service + namespace: system + path: /validate-bootstrap-cluster-x-k8s-io-v1alpha3-kubeadmconfig + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.kubeadmconfig.bootstrap.cluster.x-k8s.io + rules: + - apiGroups: + - bootstrap.cluster.x-k8s.io + apiVersions: + - v1alpha3 + operations: + - CREATE + - UPDATE + resources: + - kubeadmconfigs + sideEffects: None diff --git a/bootstrap/kubeadm/config/webhook/webhookcainjection_patch.yaml b/bootstrap/kubeadm/config/webhook/webhookcainjection_patch.yaml index 02ab515d4281..275782aa1b48 100644 --- a/bootstrap/kubeadm/config/webhook/webhookcainjection_patch.yaml +++ b/bootstrap/kubeadm/config/webhook/webhookcainjection_patch.yaml @@ -1,13 +1,13 @@ # 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/v1 -kind: MutatingWebhookConfiguration -metadata: - name: mutating-webhook-configuration - annotations: - cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) +# 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/v1 +apiVersion: admissionregistration.k8s.io/v1beta1 kind: ValidatingWebhookConfiguration metadata: name: validating-webhook-configuration diff --git a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml index 164e6a3d6982..b4510a484897 100644 --- a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml +++ b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml @@ -443,6 +443,29 @@ spec: content: description: Content is the actual content of the file. type: string + contentFrom: + description: ContentFrom is a referenced source of content + to populate the file. + properties: + secret: + description: Secret represents a secret that should + populate this file. + properties: + key: + description: Key is the key in the secret's data + map for this value. + type: string + name: + description: Name of the secret in the KubeadmBootstrapConfig's + namespace to use. + type: string + required: + - key + - name + type: object + required: + - secret + type: object encoding: description: Encoding specifies the encoding of the file contents. @@ -464,7 +487,6 @@ spec: to the file, e.g. "0640". type: string required: - - content - path type: object type: array From 9b7df762cc14ea8b164f223648aa48aac212a7d7 Mon Sep 17 00:00:00 2001 From: Alexander Eldeib Date: Fri, 22 May 2020 15:24:56 -0700 Subject: [PATCH 4/4] :hammer_and_pick: simplify file resolution, style Signed-off-by: Alexander Eldeib Co-authored-by: Vince Prignano --- bootstrap/kubeadm/api/v1alpha2/conversion.go | 6 ++ .../kubeadm/api/v1alpha2/conversion_test.go | 2 +- .../v1alpha3/kubeadmbootstrapconfig_types.go | 2 +- .../kubeadmbootstrapconfig_types_test.go | 6 +- .../api/v1alpha3/kubeadmconfig_webhook.go | 14 ++-- .../api/v1alpha3/zz_generated.deepcopy.go | 8 +- .../webhook/webhookcainjection_patch.yaml | 8 -- .../controllers/kubeadmconfig_controller.go | 83 ++++++------------- .../kubeadmconfig_controller_test.go | 78 ----------------- go.mod | 1 - 10 files changed, 44 insertions(+), 164 deletions(-) diff --git a/bootstrap/kubeadm/api/v1alpha2/conversion.go b/bootstrap/kubeadm/api/v1alpha2/conversion.go index 9cf4439d09a4..98a98c0e390d 100644 --- a/bootstrap/kubeadm/api/v1alpha2/conversion.go +++ b/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 } diff --git a/bootstrap/kubeadm/api/v1alpha2/conversion_test.go b/bootstrap/kubeadm/api/v1alpha2/conversion_test.go index 44f0a23846f5..76940f494dad 100644 --- a/bootstrap/kubeadm/api/v1alpha2/conversion_test.go +++ b/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", }, diff --git a/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go b/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go index 1877ebcb53d9..5fcf4d200c9e 100644 --- a/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go +++ b/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. diff --git a/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types_test.go b/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types_test.go index b084e67048a3..3d72ff76bb95 100644 --- a/bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types_test.go +++ b/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", }, }, diff --git a/bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_webhook.go b/bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_webhook.go index 0bb1f30308b8..c8e95a9063a5 100644 --- a/bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_webhook.go +++ b/bootstrap/kubeadm/api/v1alpha3/kubeadmconfig_webhook.go @@ -45,12 +45,12 @@ 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 @@ -58,11 +58,11 @@ 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) } diff --git a/bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go b/bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go index fdcfe797b439..72c1e952f200 100644 --- a/bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go +++ b/bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go @@ -31,7 +31,7 @@ func (in *File) DeepCopyInto(out *File) { if in.ContentFrom != nil { in, out := &in.ContentFrom, &out.ContentFrom *out = new(FileSource) - (*in).DeepCopyInto(*out) + **out = **in } } @@ -48,11 +48,7 @@ func (in *File) DeepCopy() *File { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FileSource) DeepCopyInto(out *FileSource) { *out = *in - if in.Secret != nil { - in, out := &in.Secret, &out.Secret - *out = new(SecretFileSource) - **out = **in - } + out.Secret = in.Secret } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FileSource. diff --git a/bootstrap/kubeadm/config/webhook/webhookcainjection_patch.yaml b/bootstrap/kubeadm/config/webhook/webhookcainjection_patch.yaml index 275782aa1b48..e838a4bf1d25 100644 --- a/bootstrap/kubeadm/config/webhook/webhookcainjection_patch.yaml +++ b/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 diff --git a/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go b/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go index 9369719f1333..2c1f670657d6 100644 --- a/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go +++ b/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 diff --git a/bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go b/bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go index ccec4394c233..2d0745c562e5 100644 --- a/bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go +++ b/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) diff --git a/go.mod b/go.mod index f2ab225e1498..3cacb81226e5 100644 --- a/go.mod +++ b/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