Remove "all" namespace from managed namespaces (elastic#5187)

* Don't append the 'all' namespaces when storage class validation is enabled, as
  it causes 'unknown namespace' errors in controller-runtime, and cluster-scoped
  resources are handled properly now in ctrl-runtime.
  see kubernetes-sigs/controller-runtime#1418

* Webhook ignores requests from namespaces that it doesn't manage
Updates webhook tests to ensure behavior.

* remove spaces in if err block
move log line down for consistency

* Move namespace validation within the Handle func to reduce duplication.

* ES spelling in pkg/controller/elasticsearch/validation/webhook.go

Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com>

* ES spelling in pkg/controller/elasticsearch/validation/webhook.go

Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com>

* Use set operations to enhance readability.

* Create new package to duplicate less code for setting up webhooks,
  and to allow managedNamespaces in webhooks to be handled properly
  across all webhooks.
Adjust all managed objects to use new webhook package on setup.

* Adjust common webhook to copy object properly, and use metav1.Object to query namespace.
Add logging when skipping resource validation.
Add additional information to 'reason' for allowing request.

* add missing header

* Check type assertion.

* Ensure that the set of managed namespaces isn't all before checking whether the given namespaces is managed.

* Simplify common webhook validation
Adjust how update is handled

* Remove debugging from webhook

* Proper casing in comments.
Decode the old object in upgrade, not the original object.

* Adding unit tests for webhook validation.

* Use keys in the webhook test structs

Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com>

cmd/manager/main.go

@@ -39,6 +39,7 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/elastic/cloud-on-k8s/pkg/about"
 	agentv1alpha1 "github.com/elastic/cloud-on-k8s/pkg/apis/agent/v1alpha1"
@@ -66,6 +67,7 @@ import (
 	controllerscheme "github.com/elastic/cloud-on-k8s/pkg/controller/common/scheme"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/common/tracing"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/common/version"
+	commonwebhook "github.com/elastic/cloud-on-k8s/pkg/controller/common/webhook"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch"
 	esclient "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/client"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings"
@@ -482,11 +484,6 @@ func startOperator(ctx context.Context) error {
 		// The managed cache should always include the operator namespace so that we can work with operator-internal resources.
 		managedNamespaces = append(managedNamespaces, operatorNamespace)
 
-		// Add the empty namespace to allow watching cluster-scoped resources if storage class validation is enabled.
-		if viper.GetBool(operator.ValidateStorageClassFlag) {
-			managedNamespaces = append(managedNamespaces, "")
-		}
-
 		opts.NewCache = cache.MultiNamespacedCacheBuilder(managedNamespaces)
 	}
 
@@ -580,7 +577,7 @@ func startOperator(ctx context.Context) error {
 	}
 
 	if viper.GetBool(operator.EnableWebhookFlag) {
-		setupWebhook(mgr, params.CertRotation, params.ValidateStorageClass, clientset, exposedNodeLabels)
+		setupWebhook(mgr, params.CertRotation, params.ValidateStorageClass, clientset, exposedNodeLabels, managedNamespaces)
 	}
 
 	enforceRbacOnRefs := viper.GetBool(operator.EnforceRBACOnRefsFlag)
@@ -845,7 +842,8 @@ func setupWebhook(
 	certRotation certificates.RotationParams,
 	validateStorageClass bool,
 	clientset kubernetes.Interface,
-	exposedNodeLabels esvalidation.NodeLabels) {
+	exposedNodeLabels esvalidation.NodeLabels,
+	managedNamespaces []string) {
 	manageWebhookCerts := viper.GetBool(operator.ManageWebhookCertsFlag)
 	if manageWebhookCerts {
 		log.Info("Automatic management of the webhook certificates enabled")
@@ -879,7 +877,8 @@ func setupWebhook(
 	// setup webhooks for supported types
 	webhookObjects := []interface {
 		runtime.Object
-		SetupWebhookWithManager(manager.Manager) error
+		admission.Validator
+		WebhookPath() string
 	}{
 		&agentv1alpha1.Agent{},
 		&apmv1.ApmServer{},
@@ -893,14 +892,19 @@ func setupWebhook(
 		&emsv1alpha1.ElasticMapsServer{},
 	}
 	for _, obj := range webhookObjects {
-		if err := obj.SetupWebhookWithManager(mgr); err != nil {
+		if err := commonwebhook.SetupValidatingWebhookWithConfig(&commonwebhook.Config{
+			Manager:          mgr,
+			WebhookPath:      obj.WebhookPath(),
+			ManagedNamespace: managedNamespaces,
+			Validator:        obj,
+		}); err != nil {
 			gvk := obj.GetObjectKind().GroupVersionKind()
 			log.Error(err, "Failed to setup webhook", "group", gvk.Group, "version", gvk.Version, "kind", gvk.Kind)
 		}
 	}
 
 	// esv1 validating webhook is wired up differently, in order to access the k8s client
-	esvalidation.RegisterWebhook(mgr, validateStorageClass, exposedNodeLabels)
+	esvalidation.RegisterWebhook(mgr, validateStorageClass, exposedNodeLabels, managedNamespaces)
 
 	// wait for the secret to be populated in the local filesystem before returning
 	interval := time.Second * 1

pkg/apis/agent/v1alpha1/webhook.go

@@ -11,12 +11,16 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	ulog "github.com/elastic/cloud-on-k8s/pkg/utils/log"
 )
 
+const (
+	// webhookPath is the HTTP path for the Elastic Agent validating webhook.
+	webhookPath = "/validate-agent-k8s-elastic-co-v1alpha1-agent"
+)
+
 var (
 	groupKind     = schema.GroupKind{Group: GroupVersion.Group, Kind: Kind}
 	validationLog = ulog.Log.WithName("agent-v1alpha1-validation")
@@ -26,22 +30,22 @@ var (
 
 var _ webhook.Validator = &Agent{}
 
-func (a *Agent) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).
-		For(a).
-		Complete()
-}
-
+// ValidateCreate is called by the validating webhook to validate the create operation.
+// Satisfies the webhook.Validator interface.
 func (a *Agent) ValidateCreate() error {
 	validationLog.V(1).Info("Validate create", "name", a.Name)
 	return a.validate(nil)
 }
 
+// ValidateDelete is called by the validating webhook to validate the delete operation.
+// Satisfies the webhook.Validator interface.
 func (a *Agent) ValidateDelete() error {
 	validationLog.V(1).Info("Validate delete", "name", a.Name)
 	return nil
 }
 
+// ValidateUpdate is called by the validating webhook to validate the update operation.
+// Satisfies the webhook.Validator interface.
 func (a *Agent) ValidateUpdate(old runtime.Object) error {
 	validationLog.V(1).Info("Validate update", "name", a.Name)
 	oldObj, ok := old.(*Agent)
@@ -52,6 +56,11 @@ func (a *Agent) ValidateUpdate(old runtime.Object) error {
 	return a.validate(oldObj)
 }
 
+// WebhookPath returns the HTTP path used by the validating webhook.
+func (a *Agent) WebhookPath() string {
+	return webhookPath
+}
+
 func (a *Agent) validate(old *Agent) error {
 	var errors field.ErrorList
 	if old != nil {

pkg/apis/apm/v1/webhook.go

@@ -12,14 +12,18 @@ import (
 	runtime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	commonv1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/common/version"
 	ulog "github.com/elastic/cloud-on-k8s/pkg/utils/log"
 )
 
+const (
+	// webhookPath is the HTTP path for the APM Server validating webhook.
+	webhookPath = "/validate-apm-k8s-elastic-co-v1-apmserver"
+)
+
 var (
 	groupKind     = schema.GroupKind{Group: GroupVersion.Group, Kind: Kind}
 	validationLog = ulog.Log.WithName("apm-v1-validation")
@@ -43,22 +47,22 @@ var (
 
 var _ webhook.Validator = &ApmServer{}
 
-func (as *ApmServer) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).
-		For(as).
-		Complete()
-}
-
+// ValidateCreate is called by the validating webhook to validate the create operation.
+// Satisfies the webhook.Validator interface.
 func (as *ApmServer) ValidateCreate() error {
 	validationLog.V(1).Info("Validate create", "name", as.Name)
 	return as.validate(nil)
 }
 
+// ValidateDelete is called by the validating webhook to validate the delete operation.
+// Satisfies the webhook.Validator interface.
 func (as *ApmServer) ValidateDelete() error {
 	validationLog.V(1).Info("Validate delete", "name", as.Name)
 	return nil
 }
 
+// ValidateUpdate is called by the validating webhook to validate the update operation.
+// Satisfies the webhook.Validator interface.
 func (as *ApmServer) ValidateUpdate(old runtime.Object) error {
 	validationLog.V(1).Info("Validate update", "name", as.Name)
 	oldObj, ok := old.(*ApmServer)
@@ -69,6 +73,11 @@ func (as *ApmServer) ValidateUpdate(old runtime.Object) error {
 	return as.validate(oldObj)
 }
 
+// WebhookPath returns the HTTP path used by the validating webhook.
+func (as *ApmServer) WebhookPath() string {
+	return webhookPath
+}
+
 func (as *ApmServer) validate(old *ApmServer) error {
 	var errors field.ErrorList
 	if old != nil {

pkg/apis/apm/v1beta1/webhook.go

@@ -11,14 +11,18 @@ import (
 	runtime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	commonv1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1"
 	"github.com/elastic/cloud-on-k8s/pkg/controller/common/version"
 	ulog "github.com/elastic/cloud-on-k8s/pkg/utils/log"
 )
 
+const (
+	// webhookPath is the HTTP path for the APM Server validating webhook.
+	webhookPath = "/validate-apm-k8s-elastic-co-v1beta1-apmserver"
+)
+
 var (
 	groupKind     = schema.GroupKind{Group: GroupVersion.Group, Kind: "ApmServer"}
 	validationLog = ulog.Log.WithName("apm-v1beta1-validation")
@@ -38,22 +42,22 @@ var (
 
 var _ webhook.Validator = &ApmServer{}
 
-func (as *ApmServer) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).
-		For(as).
-		Complete()
-}
-
+// ValidateCreate is called by the validating webhook to validate the create operation.
+// Satisfies the webhook.Validator interface.
 func (as *ApmServer) ValidateCreate() error {
 	validationLog.V(1).Info("Validate create", "name", as.Name)
 	return as.validate(nil)
 }
 
+// ValidateDelete is called by the validating webhook to validate the delete operation.
+// Satisfies the webhook.Validator interface.
 func (as *ApmServer) ValidateDelete() error {
 	validationLog.V(1).Info("Validate delete", "name", as.Name)
 	return nil
 }
 
+// ValidateUpdate is called by the validating webhook to validate the update operation.
+// Satisfies the webhook.Validator interface.
 func (as *ApmServer) ValidateUpdate(old runtime.Object) error {
 	validationLog.V(1).Info("Validate update", "name", as.Name)
 	oldObj, ok := old.(*ApmServer)
@@ -64,6 +68,11 @@ func (as *ApmServer) ValidateUpdate(old runtime.Object) error {
 	return as.validate(oldObj)
 }
 
+// WebhookPath returns the HTTP path used by the validating webhook.
+func (as *ApmServer) WebhookPath() string {
+	return webhookPath
+}
+
 func (as *ApmServer) validate(old *ApmServer) error {
 	var errors field.ErrorList
 	if old != nil {

pkg/apis/beat/v1beta1/webhook.go

@@ -11,12 +11,16 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	ulog "github.com/elastic/cloud-on-k8s/pkg/utils/log"
 )
 
+const (
+	// webhookPath is the HTTP path for the Elastic Beats validating webhook.
+	webhookPath = "/validate-beat-k8s-elastic-co-v1beta1-beat"
+)
+
 var (
 	groupKind     = schema.GroupKind{Group: GroupVersion.Group, Kind: Kind}
 	validationLog = ulog.Log.WithName("beat-v1beta1-validation")
@@ -26,22 +30,22 @@ var (
 
 var _ webhook.Validator = &Beat{}
 
-func (b *Beat) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).
-		For(b).
-		Complete()
-}
-
+// ValidateCreate is called by the validating webhook to validate the create operation.
+// Satisfies the webhook.Validator interface.
 func (b *Beat) ValidateCreate() error {
 	validationLog.V(1).Info("Validate create", "name", b.Name)
 	return b.validate(nil)
 }
 
+// ValidateDelete is called by the validating webhook to validate the delete operation.
+// Satisfies the webhook.Validator interface.
 func (b *Beat) ValidateDelete() error {
 	validationLog.V(1).Info("Validate delete", "name", b.Name)
 	return nil
 }
 
+// ValidateUpdate is called by the validating webhook to validate the update operation.
+// Satisfies the webhook.Validator interface.
 func (b *Beat) ValidateUpdate(old runtime.Object) error {
 	validationLog.V(1).Info("Validate update", "name", b.Name)
 	oldObj, ok := old.(*Beat)
@@ -52,6 +56,11 @@ func (b *Beat) ValidateUpdate(old runtime.Object) error {
 	return b.validate(oldObj)
 }
 
+// WebhookPath returns the HTTP path used by the validating webhook.
+func (b *Beat) WebhookPath() string {
+	return webhookPath
+}
+
 func (b *Beat) validate(old *Beat) error {
 	var errors field.ErrorList
 	if old != nil {

pkg/apis/elasticsearch/v1beta1/webhook.go

@@ -11,34 +11,36 @@ import (
 	runtime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	ulog "github.com/elastic/cloud-on-k8s/pkg/utils/log"
 )
 
-// +kubebuilder:webhook:path=/validate-elasticsearch-k8s-elastic-co-v1beta1-elasticsearch,mutating=false,failurePolicy=ignore,groups=elasticsearch.k8s.elastic.co,resources=elasticsearches,verbs=create;update,versions=v1beta1,name=elastic-es-validation-v1beta1.k8s.elastic.co,sideEffects=None,admissionReviewVersions=v1;v1beta1,matchPolicy=Exact
+const (
+	// webhookPath is the HTTP path for the Elasticsearch validating webhook.
+	webhookPath = "/validate-elasticsearch-k8s-elastic-co-v1beta1-elasticsearch"
+)
 
-func (es *Elasticsearch) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).
-		For(es).
-		Complete()
-}
+// +kubebuilder:webhook:path=/validate-elasticsearch-k8s-elastic-co-v1beta1-elasticsearch,mutating=false,failurePolicy=ignore,groups=elasticsearch.k8s.elastic.co,resources=elasticsearches,verbs=create;update,versions=v1beta1,name=elastic-es-validation-v1beta1.k8s.elastic.co,sideEffects=None,admissionReviewVersions=v1;v1beta1,matchPolicy=Exact
 
 var eslog = ulog.Log.WithName("es-validation")
 
 var _ webhook.Validator = &Elasticsearch{}
 
+// ValidateCreate is called by the validating webhook to validate the create operation.
+// Satisfies the webhook.Validator interface.
 func (es *Elasticsearch) ValidateCreate() error {
 	eslog.V(1).Info("validate create", "name", es.Name)
 	return es.validateElasticsearch()
 }
 
-// ValidateDelete is required to implement webhook.Validator, but we do not actually validate deletes
+// ValidateDelete is required to implement webhook.Validator, but we do not actually validate deletes.
 func (es *Elasticsearch) ValidateDelete() error {
 	return nil
 }
 
+// ValidateUpdate is called by the validating webhook to validate the update operation.
+// Satisfies the webhook.Validator interface.
 func (es *Elasticsearch) ValidateUpdate(old runtime.Object) error {
 	eslog.V(1).Info("validate update", "name", es.Name)
 	oldEs, ok := old.(*Elasticsearch)
@@ -60,6 +62,11 @@ func (es *Elasticsearch) ValidateUpdate(old runtime.Object) error {
 	return es.validateElasticsearch()
 }
 
+// WebhookPath returns the HTTP path used by the validating webhook.
+func (es *Elasticsearch) WebhookPath() string {
+	return webhookPath
+}
+
 func (es *Elasticsearch) validateElasticsearch() error {
 	errs := es.check(validations)
 	if len(errs) > 0 {