Refactor validations (#2072)

.golangci.yml

@@ -64,7 +64,7 @@ issues:
       text: 'struct of size 96 bytes could be of size 80 bytes'
     - path: pkg/controller/elasticsearch/reconcile/resources_state\.go
       text: 'ifElseChain: rewrite if\-else to switch statement'
-    - path: pkg/controller/elasticsearch/settings/fields\.go
+    - path: pkg/apis/elasticsearch/v1beta1/fields\.go
       text: 'const .* should be'
     - path: pkg/controller/elasticsearch/user/reconciler\.go
       text: 'Consider preallocating `allUsers`'
@@ -90,6 +90,10 @@ issues:
       text: 'Consider preallocating `errs`'
     - path: test/e2e/test/elasticsearch/steps_license\.go
       text: 'G101: Potential hardcoded credentials'
+    - path: pkg/apis/elasticsearch/v1beta1/name\.go
+      text: 'G101: Potential hardcoded credentials'
+    - path: pkg/controller/elasticsearch/driver/version\.go
+      text: 'Consider preallocating `vs`'
     - linters:
         - stylecheck
       text: 'ST1016: methods on the same type should have the same receiver name.*"in".*'

Makefile

@@ -106,7 +106,7 @@ dependencies:
 generate: controller-gen
 	# we use this in pkg/controller/common/license
 	go generate -tags='$(GO_TAGS)' ./pkg/... ./cmd/...
-	$(CONTROLLER_GEN) object:headerFile=./hack/boilerplate.go.txt paths=./pkg/apis/...
+	$(CONTROLLER_GEN) webhook object:headerFile=./hack/boilerplate.go.txt paths=./pkg/apis/...
 	# Generate manifests e.g. CRD, RBAC etc.
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) paths="./pkg/apis/..." output:crd:artifacts:config=config/crds
 	# verify that the available crd flavors still can generate cleanly

config/crds/elasticsearch.k8s.elastic.co_elasticsearches.yaml

@@ -5575,6 +5575,7 @@ spec:
                     count:
                       description: Count of Elasticsearch nodes to deploy.
                       format: int32
+                      minimum: 1
                       type: integer
                     name:
                       description: Name of this set of nodes. Becomes a part of the
@@ -10369,8 +10370,10 @@ spec:
                         type: object
                       type: array
                   required:
+                  - count
                   - name
                   type: object
+                minItems: 1
                 type: array
               podDisruptionBudget:
                 description: PodDisruptionBudget provides access to the default pod
@@ -10517,6 +10520,8 @@ spec:
               version:
                 description: Version of Elasticsearch.
                 type: string
+            required:
+            - nodeSets
             type: object
           status:
             description: ElasticsearchStatus defines the observed state of Elasticsearch

config/webhook/manifests.yaml

@@ -0,0 +1,26 @@
+
+---
+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-elasticsearch
+  failurePolicy: Ignore
+  name: elastic-es-validation
+  rules:
+  - apiGroups:
+    - elasticsearch.k8s.elastic.co
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - elasticsearches

pkg/apis/elasticsearch/v1beta1/elasticsearch_types.go

@@ -22,13 +22,16 @@ type ElasticsearchSpec struct {
 	Image string `json:"image,omitempty"`
 
 	// HTTP holds HTTP layer settings for Elasticsearch.
+	// +kubebuilder:validation:Optional
 	HTTP commonv1beta1.HTTPConfig `json:"http,omitempty"`
 
 	// NodeSets allow specifying groups of Elasticsearch nodes sharing the same configuration and Pod templates.
 	// See: https://www.elastic.co/guide/en/cloud-on-k8s/current/k8s-orchestration.html
-	NodeSets []NodeSet `json:"nodeSets,omitempty"`
+	// +kubebuilder:validation:MinItems=1
+	NodeSets []NodeSet `json:"nodeSets"`
 
 	// UpdateStrategy specifies how updates to the cluster should be performed.
+	// +kubebuilder:validation:Optional
 	UpdateStrategy UpdateStrategy `json:"updateStrategy,omitempty"`
 
 	// PodDisruptionBudget provides access to the default pod disruption budget for the Elasticsearch cluster.
@@ -39,6 +42,7 @@ type ElasticsearchSpec struct {
 
 	// SecureSettings is a list of references to Kubernetes secrets containing sensitive configuration options for Elasticsearch.
 	// See: https://www.elastic.co/guide/en/cloud-on-k8s/current/k8s-es-secure-settings.html
+	// +kubebuilder:validation:Optional
 	SecureSettings []commonv1beta1.SecretSource `json:"secureSettings,omitempty"`
 }
 
@@ -62,7 +66,8 @@ type NodeSet struct {
 	Config *commonv1beta1.Config `json:"config,omitempty"`
 
 	// Count of Elasticsearch nodes to deploy.
-	Count int32 `json:"count,omitempty"`
+	// +kubebuilder:validation:Minimum=1
+	Count int32 `json:"count"`
 
 	// PodTemplate provides customisation options (labels, annotations, affinity rules, resource requests, and so on) for the Pods belonging to this NodeSet.
 	// +kubebuilder:validation:Optional

pkg/controller/elasticsearch/settings/fields.go → pkg/apis/elasticsearch/v1beta1/fields.go

@@ -2,7 +2,7 @@
 // or more contributor license agreements. Licensed under the Elastic License;
 // you may not use this file except in compliance with the Elastic License.
 
-package settings
+package v1beta1
 
 const (
 	ClusterName = "cluster.name"
@@ -41,7 +41,7 @@ const (
 	XPackSecurityTransportSslVerificationMode       = "xpack.security.transport.ssl.verification_mode"
 )
 
-var Blacklist = []string{
+var SettingsBlacklist = []string{
 	ClusterName,
 	DiscoveryZenMinimumMasterNodes,
 	ClusterInitialMasterNodes,

pkg/controller/elasticsearch/name/name.go → pkg/apis/elasticsearch/v1beta1/name.go

@@ -2,19 +2,17 @@
 // or more contributor license agreements. Licensed under the Elastic License;
 // you may not use this file except in compliance with the Elastic License.
 
-package name
+package v1beta1
 
 import (
 	"fmt"
 	"strconv"
 	"strings"
 
-	"github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1beta1"
 	common_name "github.com/elastic/cloud-on-k8s/pkg/controller/common/name"
-	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
 	"github.com/pkg/errors"
 	apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
-	"k8s.io/apimachinery/pkg/util/validation"
+	utilvalidation "k8s.io/apimachinery/pkg/util/validation"
 )
 
 const (
@@ -52,37 +50,35 @@ var (
 	}
 )
 
-// Validate checks the validity of resource names that will be generated by the given Elasticsearch object.
-func Validate(es v1beta1.Elasticsearch) error {
-	esName := k8s.ExtractNamespacedName(&es).Name
-	if len(esName) > common_name.MaxResourceNameLength {
+// validateNames checks the validity of resource names that will be generated by the given Elasticsearch object.
+func validateNames(es *Elasticsearch) error {
+	if len(es.Name) > common_name.MaxResourceNameLength {
 		return fmt.Errorf("name exceeds maximum allowed length of %d", common_name.MaxResourceNameLength)
 	}
-
 	// validate ssets
 	for _, nodeSet := range es.Spec.NodeSets {
 		if errs := apimachineryvalidation.NameIsDNSSubdomain(nodeSet.Name, false); len(errs) > 0 {
 			return fmt.Errorf("invalid nodeSet name '%s': [%s]", nodeSet.Name, strings.Join(errs, ","))
 		}
 
-		ssetName, err := ESNamer.SafeSuffix(esName, nodeSet.Name)
+		ssetName, err := ESNamer.SafeSuffix(es.Name, nodeSet.Name)
 		if err != nil {
 			return errors.Wrapf(err, "error generating StatefulSet name for nodeSet: '%s'", nodeSet.Name)
 		}
 
 		// length of the ordinal suffix that will be added to the pods of this sset (dash + ordinal)
 		podOrdinalSuffixLen := len(strconv.FormatInt(int64(nodeSet.Count), 10)) + 1
 		// there should be enough space for the ordinal suffix and the controller revision hash
-		if validation.LabelValueMaxLength-len(ssetName) < podOrdinalSuffixLen+controllerRevisionHashLen {
+		if utilvalidation.LabelValueMaxLength-len(ssetName) < podOrdinalSuffixLen+controllerRevisionHashLen {
 			return fmt.Errorf("generated StatefulSet name '%s' exceeds allowed length of %d",
 				ssetName,
-				validation.LabelValueMaxLength-podOrdinalSuffixLen-controllerRevisionHashLen)
+				utilvalidation.LabelValueMaxLength-podOrdinalSuffixLen-controllerRevisionHashLen)
 		}
 	}
 
 	// validate other suffixes
 	for _, suffix := range suffixes {
-		if _, err := ESNamer.SafeSuffix(esName, suffix); err != nil {
+		if _, err := ESNamer.SafeSuffix(es.Name, suffix); err != nil {
 			return err
 		}
 	}

pkg/controller/elasticsearch/name/name_test.go → pkg/apis/elasticsearch/v1beta1/name_test.go

@@ -2,12 +2,11 @@
 // or more contributor license agreements. Licensed under the Elastic License;
 // you may not use this file except in compliance with the Elastic License.
 
-package name
+package v1beta1
 
 import (
 	"testing"
 
-	"github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1beta1"
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -51,19 +50,19 @@ func TestValidate(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			es := v1beta1.Elasticsearch{
+			es := &Elasticsearch{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      tc.esName,
 					Namespace: "test",
 				},
-				Spec: v1beta1.ElasticsearchSpec{},
+				Spec: ElasticsearchSpec{},
 			}
 
 			for _, nodeSpecName := range tc.nodeSpecNames {
-				es.Spec.NodeSets = append(es.Spec.NodeSets, v1beta1.NodeSet{Name: nodeSpecName, Count: 10})
+				es.Spec.NodeSets = append(es.Spec.NodeSets, NodeSet{Name: nodeSpecName, Count: 10})
 			}
 
-			err := Validate(es)
+			err := validateNames(es)
 			if tc.wantErr {
 				require.Error(t, err)
 				require.Contains(t, err.Error(), tc.wantErrMsg)