From 09f0e5aa8a636b949fdec39b51c0ed4b0785f7a7 Mon Sep 17 00:00:00 2001 From: Andrey <84432317+andreykont@users.noreply.github.com> Date: Wed, 3 Apr 2024 09:03:24 +0300 Subject: [PATCH] feat: validate extraargs (#99) Closes: #77 --------- Co-authored-by: Andrey Kontyakov Co-authored-by: Sergey Shevchenko --- api/v1alpha1/etcdcluster_webhook.go | 67 +++++++++++++++++-- .../etcd.aenix.io_v1alpha1_etcdcluster.yaml | 2 +- internal/controller/factory/statefulset.go | 23 +++++-- .../controller/factory/statefulset_test.go | 6 +- 4 files changed, 85 insertions(+), 13 deletions(-) diff --git a/api/v1alpha1/etcdcluster_webhook.go b/api/v1alpha1/etcdcluster_webhook.go index 8c5ca56b..5cb6923c 100644 --- a/api/v1alpha1/etcdcluster_webhook.go +++ b/api/v1alpha1/etcdcluster_webhook.go @@ -19,12 +19,14 @@ package v1alpha1 import ( "fmt" "math" + "strings" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" @@ -73,11 +75,26 @@ var _ webhook.Validator = &EtcdCluster{} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type func (r *EtcdCluster) ValidateCreate() (admission.Warnings, error) { etcdclusterlog.Info("validate create", "name", r.Name) - warnings, err := r.validatePdb() - if err != nil { - return nil, errors.NewInvalid( + + var allErrors field.ErrorList + + warnings, pdbErr := r.validatePdb() + if pdbErr != nil { + allErrors = append(allErrors, pdbErr...) + } + + if errExtraArgs := validateExtraArgs(r); errExtraArgs != nil { + allErrors = append(allErrors, field.Invalid( + field.NewPath("spec", "podSpec", "extraArgs"), + r.Spec.PodSpec.ExtraArgs, + errExtraArgs.Error())) + } + + if len(allErrors) > 0 { + err := errors.NewInvalid( schema.GroupKind{Group: GroupVersion.Group, Kind: "EtcdCluster"}, - r.Name, err) + r.Name, allErrors) + return warnings, err } return warnings, nil @@ -110,6 +127,13 @@ func (r *EtcdCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, er warnings = append(warnings, pdbWarnings...) } + if errExtraArgs := validateExtraArgs(r); errExtraArgs != nil { + allErrors = append(allErrors, field.Invalid( + field.NewPath("spec", "podSpec", "extraArgs"), + r.Spec.PodSpec.ExtraArgs, + errExtraArgs.Error())) + } + if len(allErrors) > 0 { err := errors.NewInvalid( schema.GroupKind{Group: GroupVersion.Group, Kind: "EtcdCluster"}, @@ -219,3 +243,38 @@ func (r *EtcdCluster) validatePdb() (admission.Warnings, field.ErrorList) { return warnings, nil } + +func validateExtraArgs(cluster *EtcdCluster) error { + if len(cluster.Spec.PodSpec.ExtraArgs) == 0 { + return nil + } + + systemflags := map[string]struct{}{ + "name": {}, + "listen-peer-urls": {}, + "listen-client-urls": {}, + "initial-advertise-peer-urls": {}, + "data-dir": {}, + "auto-tls": {}, + "peer-auto-tls": {}, + "advertise-client-urls": {}, + } + + errlist := []error{} + + for name := range cluster.Spec.PodSpec.ExtraArgs { + if strings.HasPrefix(name, "-") || strings.HasSuffix(name, "-") { + errlist = append(errlist, fmt.Errorf("Extra arg should not start with dash and have trailing dash, "+ + "but it can contain dashes in the middle. flag: %s", name)) + + continue + } + + if _, exists := systemflags[name]; exists { + errlist = append(errlist, fmt.Errorf("can't use extra argument '%s' in .Spec.PodSpec.ExtraArgs "+ + "because it generated by operator for cluster setup", name)) + } + } + + return kerrors.NewAggregate(errlist) +} diff --git a/config/samples/etcd.aenix.io_v1alpha1_etcdcluster.yaml b/config/samples/etcd.aenix.io_v1alpha1_etcdcluster.yaml index e3d04c43..d9bd8f62 100644 --- a/config/samples/etcd.aenix.io_v1alpha1_etcdcluster.yaml +++ b/config/samples/etcd.aenix.io_v1alpha1_etcdcluster.yaml @@ -12,8 +12,8 @@ spec: storage: emptyDir: sizeLimit: 1Gi - podDisruptionBudget: {} podSpec: + extraArgs: {} resources: requests: cpu: 100m diff --git a/internal/controller/factory/statefulset.go b/internal/controller/factory/statefulset.go index 4762d855..ab84eca8 100644 --- a/internal/controller/factory/statefulset.go +++ b/internal/controller/factory/statefulset.go @@ -94,7 +94,8 @@ func CreateOrUpdateStatefulSet( Name: "etcd", Image: cluster.Spec.PodSpec.Image, ImagePullPolicy: cluster.Spec.PodSpec.ImagePullPolicy, - Command: generateEtcdCommand(cluster), + Command: generateEtcdCommand(), + Args: generateEtcdArgs(cluster), Ports: []corev1.ContainerPort{ {Name: "peer", ContainerPort: 2380}, {Name: "client", ContainerPort: 2379}, @@ -201,9 +202,14 @@ func CreateOrUpdateStatefulSet( return reconcileStatefulSet(ctx, rclient, cluster.Name, statefulSet) } -func generateEtcdCommand(cluster *etcdaenixiov1alpha1.EtcdCluster) []string { - command := []string{ +func generateEtcdCommand() []string { + return []string{ "etcd", + } +} + +func generateEtcdArgs(cluster *etcdaenixiov1alpha1.EtcdCluster) []string { + args := []string{ "--name=$(POD_NAME)", "--listen-peer-urls=https://0.0.0.0:2380", // for first version disable TLS for client access @@ -216,8 +222,15 @@ func generateEtcdCommand(cluster *etcdaenixiov1alpha1.EtcdCluster) []string { } for name, value := range cluster.Spec.PodSpec.ExtraArgs { - command = append(command, fmt.Sprintf("--%s=%s", name, value)) + flag := "--" + name + if len(value) == 0 { + args = append(args, flag) + + continue + } + + args = append(args, fmt.Sprintf("%s=%s", flag, value)) } - return command + return args } diff --git a/internal/controller/factory/statefulset_test.go b/internal/controller/factory/statefulset_test.go index 49f3ece9..901d6edf 100644 --- a/internal/controller/factory/statefulset_test.go +++ b/internal/controller/factory/statefulset_test.go @@ -127,7 +127,7 @@ var _ = Describe("CreateOrUpdateStatefulSet handler", func() { Expect(sts.Spec.Template.ObjectMeta.Annotations).To(Equal(etcdcluster.Spec.PodSpec.PodMetadata.Annotations)) By("Checking the extraArgs") - Expect(sts.Spec.Template.Spec.Containers[0].Command).To(Equal(generateEtcdCommand(etcdcluster))) + Expect(sts.Spec.Template.Spec.Containers[0].Command).To(Equal(generateEtcdCommand())) By("Deleting the statefulset") Expect(k8sClient.Delete(ctx, sts)).To(Succeed()) @@ -180,9 +180,9 @@ var _ = Describe("CreateOrUpdateStatefulSet handler", func() { }, } - command := generateEtcdCommand(etcdcluster) + args := generateEtcdArgs(etcdcluster) - Expect(command).To(ContainElements([]string{ + Expect(args).To(ContainElements([]string{ "--key1=value1", "--key2=value2", }))