feat: validate extraargs (#99)

Closes: #77 --------- Co-authored-by: Andrey Kontyakov <[email protected]> Co-authored-by: Sergey Shevchenko <[email protected]>
aenix-io · Apr 3, 2024 · 09f0e5a · 09f0e5a
1 parent 39aec69
commit 09f0e5a
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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",
 			}))