From 09277e9f3d77ef0d557dc9832d000affe8f27133 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sat, 31 Jul 2021 23:08:35 +0200 Subject: [PATCH] feat: Ingress hostname collision scope at Tenant level --- api/v1alpha1/conversion_hub.go | 22 ++- api/v1alpha1/conversion_hub_test.go | 8 +- api/v1beta1/hostname_collision_scope.go | 14 ++ api/v1beta1/ingress_options.go | 13 ++ api/v1beta1/tenant_types.go | 2 +- api/v1beta1/zz_generated.deepcopy.go | 6 +- controllers/tenant/namespaces.go | 2 +- main.go | 6 +- pkg/indexer/indexer.go | 23 +-- pkg/indexer/ingress/hostname.go | 52 ------- pkg/indexer/ingress/hostname_path.go | 54 +++++++ pkg/indexer/ingress/utils.go | 71 +++++++++ pkg/indexer/namespace/namespaces.go | 2 +- pkg/indexer/tenant/hostnames.go | 2 +- pkg/indexer/tenant/namespaces.go | 8 +- pkg/webhook/ingress/types.go | 113 ++++++++++---- pkg/webhook/ingress/validate_class.go | 6 +- pkg/webhook/ingress/validate_collision.go | 182 ++++++++++++++-------- pkg/webhook/ingress/validate_hostnames.go | 34 ++-- pkg/webhook/tenant/hostnames_collision.go | 2 +- pkg/webhook/tenant/ingressclass_regex.go | 2 +- 21 files changed, 430 insertions(+), 194 deletions(-) create mode 100644 api/v1beta1/hostname_collision_scope.go delete mode 100644 pkg/indexer/ingress/hostname.go create mode 100644 pkg/indexer/ingress/hostname_path.go create mode 100644 pkg/indexer/ingress/utils.go diff --git a/api/v1alpha1/conversion_hub.go b/api/v1alpha1/conversion_hub.go index e9dd23cf..d77b6f53 100644 --- a/api/v1alpha1/conversion_hub.go +++ b/api/v1alpha1/conversion_hub.go @@ -43,6 +43,8 @@ const ( enablePriorityClassListingAnnotation = "capsule.clastix.io/enable-priorityclass-listing" enablePriorityClassUpdateAnnotation = "capsule.clastix.io/enable-priorityclass-update" enablePriorityClassDeletionAnnotation = "capsule.clastix.io/enable-priorityclass-deletion" + + ingressHostnameCollisionScope = "ingress.capsule.clastix.io/hostname-collision-scope" ) func (t *Tenant) convertV1Alpha1OwnerToV1Beta1() capsulev1beta1.OwnerListSpec { @@ -170,19 +172,21 @@ func (t *Tenant) ConvertTo(dstRaw conversion.Hub) error { Regex: t.Spec.StorageClasses.Regex, } } - if t.Spec.IngressClasses != nil { - if dst.Spec.IngressOptions == nil { - dst.Spec.IngressOptions = &capsulev1beta1.IngressOptions{} + if v, ok := t.Annotations[ingressHostnameCollisionScope]; ok { + switch v { + case string(capsulev1beta1.HostnameCollisionScopeCluster), string(capsulev1beta1.HostnameCollisionScopeTenant), string(capsulev1beta1.HostnameCollisionScopeNamespace): + dst.Spec.IngressOptions.HostnameCollisionScope = capsulev1beta1.HostnameCollisionScope(v) + default: + dst.Spec.IngressOptions.HostnameCollisionScope = capsulev1beta1.HostnameCollisionScopeDisabled } + } + if t.Spec.IngressClasses != nil { dst.Spec.IngressOptions.AllowedClasses = &capsulev1beta1.AllowedListSpec{ Exact: t.Spec.IngressClasses.Exact, Regex: t.Spec.IngressClasses.Regex, } } if t.Spec.IngressHostnames != nil { - if dst.Spec.IngressOptions == nil { - dst.Spec.IngressOptions = &capsulev1beta1.IngressOptions{} - } dst.Spec.IngressOptions.AllowedHostnames = &capsulev1beta1.AllowedListSpec{ Exact: t.Spec.IngressHostnames.Exact, Regex: t.Spec.IngressHostnames.Regex, @@ -321,6 +325,7 @@ func (t *Tenant) ConvertTo(dstRaw conversion.Hub) error { delete(dst.ObjectMeta.Annotations, enablePriorityClassUpdateAnnotation) delete(dst.ObjectMeta.Annotations, enablePriorityClassDeletionAnnotation) delete(dst.ObjectMeta.Annotations, resourceQuotaScopeAnnotation) + delete(dst.ObjectMeta.Annotations, ingressHostnameCollisionScope) return nil } @@ -459,13 +464,14 @@ func (t *Tenant) ConvertFrom(srcRaw conversion.Hub) error { Regex: src.Spec.StorageClasses.Regex, } } - if src.Spec.IngressOptions != nil && src.Spec.IngressOptions.AllowedClasses != nil { + t.Annotations[ingressHostnameCollisionScope] = string(src.Spec.IngressOptions.HostnameCollisionScope) + if src.Spec.IngressOptions.AllowedClasses != nil { t.Spec.IngressClasses = &AllowedListSpec{ Exact: src.Spec.IngressOptions.AllowedClasses.Exact, Regex: src.Spec.IngressOptions.AllowedClasses.Regex, } } - if src.Spec.IngressOptions != nil && src.Spec.IngressOptions.AllowedHostnames != nil { + if src.Spec.IngressOptions.AllowedHostnames != nil { t.Spec.IngressHostnames = &AllowedListSpec{ Exact: src.Spec.IngressOptions.AllowedHostnames.Exact, Regex: src.Spec.IngressOptions.AllowedHostnames.Regex, diff --git a/api/v1alpha1/conversion_hub_test.go b/api/v1alpha1/conversion_hub_test.go index 4fa5cb3d..663933ca 100644 --- a/api/v1alpha1/conversion_hub_test.go +++ b/api/v1alpha1/conversion_hub_test.go @@ -232,9 +232,10 @@ func generateTenantsSpecs() (Tenant, capsulev1beta1.Tenant) { NamespaceOptions: v1beta1NamespaceOptions, ServiceOptions: v1beta1ServiceOptions, StorageClasses: v1beta1AllowedListSpec, - IngressOptions: &capsulev1beta1.IngressOptions{ - AllowedClasses: v1beta1AllowedListSpec, - AllowedHostnames: v1beta1AllowedListSpec, + IngressOptions: capsulev1beta1.IngressOptions{ + HostnameCollisionScope: capsulev1beta1.HostnameCollisionScopeDisabled, + AllowedClasses: v1beta1AllowedListSpec, + AllowedHostnames: v1beta1AllowedListSpec, }, ContainerRegistries: v1beta1AllowedListSpec, NodeSelector: nodeSelector, @@ -299,6 +300,7 @@ func generateTenantsSpecs() (Tenant, capsulev1beta1.Tenant) { enableIngressClassDeletionAnnotation: "alice,jack", enablePriorityClassListingAnnotation: "jack", resourceQuotaScopeAnnotation: "Namespace", + ingressHostnameCollisionScope: "Disabled", }, }, Spec: TenantSpec{ diff --git a/api/v1beta1/hostname_collision_scope.go b/api/v1beta1/hostname_collision_scope.go new file mode 100644 index 00000000..6bed62b9 --- /dev/null +++ b/api/v1beta1/hostname_collision_scope.go @@ -0,0 +1,14 @@ +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package v1beta1 + +const ( + HostnameCollisionScopeCluster HostnameCollisionScope = "Cluster" + HostnameCollisionScopeTenant HostnameCollisionScope = "Tenant" + HostnameCollisionScopeNamespace HostnameCollisionScope = "Namespace" + HostnameCollisionScopeDisabled HostnameCollisionScope = "Disabled" +) + +// +kubebuilder:validation:Enum=Cluster;Tenant;Namespace;Disabled +type HostnameCollisionScope string diff --git a/api/v1beta1/ingress_options.go b/api/v1beta1/ingress_options.go index eaa23efe..d748e472 100644 --- a/api/v1beta1/ingress_options.go +++ b/api/v1beta1/ingress_options.go @@ -6,6 +6,19 @@ package v1beta1 type IngressOptions struct { // Specifies the allowed IngressClasses assigned to the Tenant. Capsule assures that all Ingress resources created in the Tenant can use only one of the allowed IngressClasses. Optional. AllowedClasses *AllowedListSpec `json:"allowedClasses,omitempty"` + // Defines the scope of hostname collision check performed when Tenant Owners create Ingress with allowed hostnames. + // + // + // - Cluster: disallow the creation of an Ingress if the pair hostname and path is already used across the Namespaces managed by Capsule. + // + // - Tenant: disallow the creation of an Ingress if the pair hostname and path is already used across the Namespaces of the Tenant. + // + // - Namespace: disallow the creation of an Ingress if the pair hostname and path is already used in the Ingress Namespace. + // + // + // Optional. + // +kubebuilder:default=Disabled + HostnameCollisionScope HostnameCollisionScope `json:"hostnameCollisionScope,omitempty"` // Specifies the allowed hostnames in Ingresses for the given Tenant. Capsule assures that all Ingress resources created in the Tenant can use only one of the allowed hostnames. Optional. AllowedHostnames *AllowedListSpec `json:"allowedHostnames,omitempty"` } diff --git a/api/v1beta1/tenant_types.go b/api/v1beta1/tenant_types.go index c3b25805..31dfa3ce 100644 --- a/api/v1beta1/tenant_types.go +++ b/api/v1beta1/tenant_types.go @@ -18,7 +18,7 @@ type TenantSpec struct { // Specifies the allowed StorageClasses assigned to the Tenant. Capsule assures that all PersistentVolumeClaim resources created in the Tenant can use only one of the allowed StorageClasses. Optional. StorageClasses *AllowedListSpec `json:"storageClasses,omitempty"` // Specifies options for the Ingress resources, such as allowed hostnames and IngressClass. Optional. - IngressOptions *IngressOptions `json:"ingressOptions,omitempty"` + IngressOptions IngressOptions `json:"ingressOptions,omitempty"` // Specifies the trusted Image Registries assigned to the Tenant. Capsule assures that all Pods resources created in the Tenant can use only one of the allowed trusted registries. Optional. ContainerRegistries *AllowedListSpec `json:"containerRegistries,omitempty"` // Specifies the label to control the placement of pods on a given pool of worker nodes. All namesapces created within the Tenant will have the node selector annotation. This annotation tells the Kubernetes scheduler to place pods on the nodes having the selector label. Optional. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 69bf367e..6260ba75 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -442,11 +442,7 @@ func (in *TenantSpec) DeepCopyInto(out *TenantSpec) { *out = new(AllowedListSpec) (*in).DeepCopyInto(*out) } - if in.IngressOptions != nil { - in, out := &in.IngressOptions, &out.IngressOptions - *out = new(IngressOptions) - (*in).DeepCopyInto(*out) - } + in.IngressOptions.DeepCopyInto(&out.IngressOptions) if in.ContainerRegistries != nil { in, out := &in.ContainerRegistries, &out.ContainerRegistries *out = new(AllowedListSpec) diff --git a/controllers/tenant/namespaces.go b/controllers/tenant/namespaces.go index cfbf9a85..d587f330 100644 --- a/controllers/tenant/namespaces.go +++ b/controllers/tenant/namespaces.go @@ -64,7 +64,7 @@ func (r *Manager) syncNamespaceMetadata(namespace string, tnt *capsulev1beta1.Te annotations["scheduler.alpha.kubernetes.io/node-selector"] = strings.Join(selector, ",") } - if tnt.Spec.IngressOptions != nil && tnt.Spec.IngressOptions.AllowedClasses != nil { + if tnt.Spec.IngressOptions.AllowedClasses != nil { if len(tnt.Spec.IngressOptions.AllowedClasses.Exact) > 0 { annotations[capsulev1beta1.AvailableIngressClassesAnnotation] = strings.Join(tnt.Spec.IngressOptions.AllowedClasses.Exact, ",") } diff --git a/main.go b/main.go index 0042ec4a..059aeab2 100644 --- a/main.go +++ b/main.go @@ -222,13 +222,15 @@ func main() { os.Exit(1) } - if err = indexer.AddToManager(manager); err != nil { + ctx := ctrl.SetupSignalHandler() + + if err = indexer.AddToManager(manager, ctx); err != nil { setupLog.Error(err, "unable to setup indexers") os.Exit(1) } setupLog.Info("starting manager") - if err = manager.Start(ctrl.SetupSignalHandler()); err != nil { + if err = manager.Start(ctx); err != nil { setupLog.Error(err, "problem running manager") os.Exit(1) } diff --git a/pkg/indexer/indexer.go b/pkg/indexer/indexer.go index 9e6aa36b..41458e66 100644 --- a/pkg/indexer/indexer.go +++ b/pkg/indexer/indexer.go @@ -6,15 +6,16 @@ package indexer import ( "context" - "github.com/clastix/capsule/pkg/indexer/ingress" - "github.com/clastix/capsule/pkg/indexer/namespace" - "github.com/clastix/capsule/pkg/indexer/tenant" - "github.com/clastix/capsule/pkg/webhook/utils" extensionsv1beta1 "k8s.io/api/extensions/v1beta1" networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/clastix/capsule/pkg/indexer/ingress" + "github.com/clastix/capsule/pkg/indexer/namespace" + "github.com/clastix/capsule/pkg/indexer/tenant" + "github.com/clastix/capsule/pkg/webhook/utils" ) type CustomIndexer interface { @@ -23,23 +24,27 @@ type CustomIndexer interface { Func() client.IndexerFunc } -func AddToManager(m manager.Manager) error { +func AddToManager(m manager.Manager, ctx context.Context) error { indexers := append([]CustomIndexer{}, tenant.IngressHostnames{}, tenant.NamespacesReference{}, tenant.OwnerReference{}, namespace.OwnerReference{}, - ingress.Hostname{Obj: &extensionsv1beta1.Ingress{}}, - ingress.Hostname{Obj: &networkingv1beta1.Ingress{}}, ) majorVer, minorVer, _, _ := utils.GetK8sVersion() + if majorVer == 1 && minorVer < 22 { + indexers = append(indexers, + ingress.HostnamePath{Obj: &extensionsv1beta1.Ingress{}}, + ingress.HostnamePath{Obj: &networkingv1beta1.Ingress{}}, + ) + } if majorVer == 1 && minorVer >= 19 { - indexers = append(indexers, ingress.Hostname{Obj: &networkingv1.Ingress{}}) + indexers = append(indexers, ingress.HostnamePath{Obj: &networkingv1.Ingress{}}) } for _, f := range indexers { - if err := m.GetFieldIndexer().IndexField(context.TODO(), f.Object(), f.Field(), f.Func()); err != nil { + if err := m.GetFieldIndexer().IndexField(ctx, f.Object(), f.Field(), f.Func()); err != nil { return err } } diff --git a/pkg/indexer/ingress/hostname.go b/pkg/indexer/ingress/hostname.go deleted file mode 100644 index 11bd22c2..00000000 --- a/pkg/indexer/ingress/hostname.go +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package ingress - -import ( - extensionsv1beta1 "k8s.io/api/extensions/v1beta1" - networkingv1 "k8s.io/api/networking/v1" - networkingv1beta1 "k8s.io/api/networking/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -const ( - HostnameFieldSelector = "host" -) - -type Hostname struct { - Obj metav1.Object -} - -func (h Hostname) Object() client.Object { - return h.Obj.(client.Object) -} - -func (h Hostname) Field() string { - return HostnameFieldSelector -} - -func (h Hostname) Func() client.IndexerFunc { - return func(object client.Object) (hostnames []string) { - switch ing := h.Obj.(type) { - case *networkingv1.Ingress: - for _, r := range ing.Spec.Rules { - hostnames = append(hostnames, r.Host) - } - return - case *networkingv1beta1.Ingress: - for _, r := range ing.Spec.Rules { - hostnames = append(hostnames, r.Host) - } - return - case *extensionsv1beta1.Ingress: - for _, r := range ing.Spec.Rules { - hostnames = append(hostnames, r.Host) - } - return - default: - return - } - } -} diff --git a/pkg/indexer/ingress/hostname_path.go b/pkg/indexer/ingress/hostname_path.go new file mode 100644 index 00000000..b1decc16 --- /dev/null +++ b/pkg/indexer/ingress/hostname_path.go @@ -0,0 +1,54 @@ +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package ingress + +import ( + "fmt" + + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" + networkingv1 "k8s.io/api/networking/v1" + networkingv1beta1 "k8s.io/api/networking/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + HostPathPair = "hostnamePathPair" +) + +type HostnamePath struct { + Obj metav1.Object +} + +func (s HostnamePath) Object() client.Object { + return s.Obj.(client.Object) +} + +func (s HostnamePath) Field() string { + return HostPathPair +} + +func (s HostnamePath) Func() client.IndexerFunc { + return func(object client.Object) (entries []string) { + hostPathMap := make(map[string]sets.String) + + switch ing := object.(type) { + case *networkingv1.Ingress: + hostPathMap = hostPathMapForNetworkingV1(ing) + case *networkingv1beta1.Ingress: + hostPathMap = hostPathMapForNetworkingV1Beta1(ing) + case *extensionsv1beta1.Ingress: + hostPathMap = hostPathMapForExtensionsV1Beta1(ing) + } + + for host, paths := range hostPathMap { + for path := range paths { + entries = append(entries, fmt.Sprintf("%s;%s", host, path)) + } + } + + return + } +} diff --git a/pkg/indexer/ingress/utils.go b/pkg/indexer/ingress/utils.go new file mode 100644 index 00000000..fa008f92 --- /dev/null +++ b/pkg/indexer/ingress/utils.go @@ -0,0 +1,71 @@ +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package ingress + +import ( + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" + networkingv1 "k8s.io/api/networking/v1" + networkingv1beta1 "k8s.io/api/networking/v1beta1" + "k8s.io/apimachinery/pkg/util/sets" +) + +func hostPathMapForExtensionsV1Beta1(ing *extensionsv1beta1.Ingress) map[string]sets.String { + hostPathMap := make(map[string]sets.String) + + for _, r := range ing.Spec.Rules { + if r.HTTP == nil { + continue + } + + if _, ok := hostPathMap[r.Host]; !ok { + hostPathMap[r.Host] = sets.NewString() + } + + for _, path := range r.HTTP.Paths { + hostPathMap[r.Host].Insert(path.Path) + } + } + + return hostPathMap +} + +func hostPathMapForNetworkingV1Beta1(ing *networkingv1beta1.Ingress) map[string]sets.String { + hostPathMap := make(map[string]sets.String) + + for _, r := range ing.Spec.Rules { + if r.HTTP == nil { + continue + } + + if _, ok := hostPathMap[r.Host]; !ok { + hostPathMap[r.Host] = sets.NewString() + } + + for _, path := range r.HTTP.Paths { + hostPathMap[r.Host].Insert(path.Path) + } + } + + return hostPathMap +} + +func hostPathMapForNetworkingV1(ing *networkingv1.Ingress) map[string]sets.String { + hostPathMap := make(map[string]sets.String) + + for _, r := range ing.Spec.Rules { + if r.HTTP == nil { + continue + } + + if _, ok := hostPathMap[r.Host]; !ok { + hostPathMap[r.Host] = sets.NewString() + } + + for _, path := range r.HTTP.Paths { + hostPathMap[r.Host].Insert(path.Path) + } + } + + return hostPathMap +} diff --git a/pkg/indexer/namespace/namespaces.go b/pkg/indexer/namespace/namespaces.go index e0f37e11..ce929ab5 100644 --- a/pkg/indexer/namespace/namespaces.go +++ b/pkg/indexer/namespace/namespaces.go @@ -23,7 +23,7 @@ func (o OwnerReference) Field() string { func (o OwnerReference) Func() client.IndexerFunc { return func(object client.Object) []string { - var res []string + res := []string{} ns := object.(*v1.Namespace) for _, or := range ns.OwnerReferences { if or.APIVersion == capsulev1beta1.GroupVersion.String() { diff --git a/pkg/indexer/tenant/hostnames.go b/pkg/indexer/tenant/hostnames.go index 5e69a200..2d88a5c9 100644 --- a/pkg/indexer/tenant/hostnames.go +++ b/pkg/indexer/tenant/hostnames.go @@ -23,7 +23,7 @@ func (IngressHostnames) Field() string { func (IngressHostnames) Func() client.IndexerFunc { return func(object client.Object) (out []string) { tenant := object.(*capsulev1beta1.Tenant) - if tenant.Spec.IngressOptions != nil && tenant.Spec.IngressOptions.AllowedHostnames != nil { + if tenant.Spec.IngressOptions.AllowedHostnames != nil { out = append(out, tenant.Spec.IngressOptions.AllowedHostnames.Exact...) } return diff --git a/pkg/indexer/tenant/namespaces.go b/pkg/indexer/tenant/namespaces.go index a9481b06..e22a525e 100644 --- a/pkg/indexer/tenant/namespaces.go +++ b/pkg/indexer/tenant/namespaces.go @@ -22,6 +22,12 @@ func (o NamespacesReference) Field() string { func (o NamespacesReference) Func() client.IndexerFunc { return func(object client.Object) []string { - return object.(*capsulev1beta1.Tenant).DeepCopy().Status.Namespaces + namespaces := object.(*capsulev1beta1.Tenant).DeepCopy().Status.Namespaces + + if namespaces == nil { + return []string{} + } + + return namespaces } } diff --git a/pkg/webhook/ingress/types.go b/pkg/webhook/ingress/types.go index c49ab848..0ee96f76 100644 --- a/pkg/webhook/ingress/types.go +++ b/pkg/webhook/ingress/types.go @@ -9,6 +9,7 @@ import ( extensionsv1beta1 "k8s.io/api/extensions/v1beta1" networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" + "k8s.io/apimachinery/pkg/util/sets" ) const ( @@ -19,7 +20,7 @@ type Ingress interface { IngressClass() *string Namespace() string Name() string - Hostnames() []string + HostnamePathsPairs() map[string]sets.String } type NetworkingV1 struct { @@ -46,13 +47,31 @@ func (n NetworkingV1) Namespace() string { return n.GetNamespace() } -func (n NetworkingV1) Hostnames() []string { - rules := n.Spec.Rules - var hostnames []string - for _, el := range rules { - hostnames = append(hostnames, el.Host) +// nolint:dupl +func (n NetworkingV1) HostnamePathsPairs() (pairs map[string]sets.String) { + pairs = make(map[string]sets.String) + + for _, rule := range n.Spec.Rules { + host := rule.Host + + if _, ok := pairs[host]; !ok { + pairs[host] = sets.NewString() + } + + if http := rule.IngressRuleValue.HTTP; http != nil { + for _, path := range http.Paths { + pairs[host].Insert(path.Path) + } + } + + if http := rule.HTTP; http != nil { + for _, path := range http.Paths { + pairs[host].Insert(path.Path) + } + } } - return hostnames + + return pairs } type NetworkingV1Beta1 struct { @@ -79,13 +98,31 @@ func (n NetworkingV1Beta1) Namespace() string { return n.GetNamespace() } -func (n NetworkingV1Beta1) Hostnames() []string { - rules := n.Spec.Rules - var hostnames []string - for _, rule := range rules { - hostnames = append(hostnames, rule.Host) +// nolint:dupl +func (n NetworkingV1Beta1) HostnamePathsPairs() (pairs map[string]sets.String) { + pairs = make(map[string]sets.String) + + for _, rule := range n.Spec.Rules { + host := rule.Host + + if _, ok := pairs[host]; !ok { + pairs[host] = sets.NewString() + } + + if http := rule.IngressRuleValue.HTTP; http != nil { + for _, path := range http.Paths { + pairs[host].Insert(path.Path) + } + } + + if http := rule.HTTP; http != nil { + for _, path := range http.Paths { + pairs[host].Insert(path.Path) + } + } } - return hostnames + + return pairs } type Extension struct { @@ -112,32 +149,50 @@ func (e Extension) Namespace() string { return e.GetNamespace() } -func (e Extension) Hostnames() []string { - rules := e.Spec.Rules - var hostnames []string - for _, el := range rules { - hostnames = append(hostnames, el.Host) +// nolint:dupl +func (e Extension) HostnamePathsPairs() (pairs map[string]sets.String) { + pairs = make(map[string]sets.String) + + for _, rule := range e.Spec.Rules { + host := rule.Host + + if _, ok := pairs[host]; !ok { + pairs[host] = sets.NewString() + } + + if http := rule.IngressRuleValue.HTTP; http != nil { + for _, path := range http.Paths { + pairs[host].Insert(path.Path) + } + } + + if http := rule.HTTP; http != nil { + for _, path := range http.Paths { + pairs[host].Insert(path.Path) + } + } } - return hostnames + + return pairs } type HostnamesList []string -func (hostnames HostnamesList) Len() int { - return len(hostnames) +func (h HostnamesList) Len() int { + return len(h) } -func (hostnames HostnamesList) Swap(i, j int) { - hostnames[i], hostnames[j] = hostnames[j], hostnames[i] +func (h HostnamesList) Swap(i, j int) { + h[i], h[j] = h[j], h[i] } -func (hostnames HostnamesList) Less(i, j int) bool { - return hostnames[i] < hostnames[j] +func (h HostnamesList) Less(i, j int) bool { + return h[i] < h[j] } -func (hostnames HostnamesList) IsStringInList(value string) (ok bool) { - sort.Sort(hostnames) - i := sort.SearchStrings(hostnames, value) - ok = i < hostnames.Len() && hostnames[i] == value +func (h HostnamesList) IsStringInList(value string) (ok bool) { + sort.Sort(h) + i := sort.SearchStrings(h, value) + ok = i < h.Len() && h[i] == value return } diff --git a/pkg/webhook/ingress/validate_class.go b/pkg/webhook/ingress/validate_class.go index 58fd2449..bb5c78ea 100644 --- a/pkg/webhook/ingress/validate_class.go +++ b/pkg/webhook/ingress/validate_class.go @@ -26,6 +26,7 @@ func Class(configuration configuration.Configuration) capsulewebhook.Handler { return &class{configuration: configuration} } +// nolint:dupl func (r *class) OnCreate(client client.Client, decoder *admission.Decoder, recorder record.EventRecorder) capsulewebhook.Func { return func(ctx context.Context, req admission.Request) *admission.Response { ingress, err := ingressFromRequest(req, decoder) @@ -66,6 +67,7 @@ func (r *class) OnCreate(client client.Client, decoder *admission.Decoder, recor } } +// nolint:dupl func (r *class) OnUpdate(client client.Client, decoder *admission.Decoder, recorder record.EventRecorder) capsulewebhook.Func { return func(ctx context.Context, req admission.Request) *admission.Response { ingress, err := ingressFromRequest(req, decoder) @@ -84,7 +86,9 @@ func (r *class) OnUpdate(client client.Client, decoder *admission.Decoder, recor return nil } - err = r.validateClass(*tenant, ingress.IngressClass()) + if err = r.validateClass(*tenant, ingress.IngressClass()); err == nil { + return nil + } var forbiddenErr *ingressClassForbidden diff --git a/pkg/webhook/ingress/validate_collision.go b/pkg/webhook/ingress/validate_collision.go index af766c8c..f7f6c4a6 100644 --- a/pkg/webhook/ingress/validate_collision.go +++ b/pkg/webhook/ingress/validate_collision.go @@ -5,6 +5,7 @@ package ingress import ( "context" + "fmt" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -12,11 +13,13 @@ import ( networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" + "github.com/clastix/capsule/pkg/indexer/ingress" "github.com/clastix/capsule/pkg/configuration" capsulewebhook "github.com/clastix/capsule/pkg/webhook" @@ -31,32 +34,33 @@ func Collision(configuration configuration.Configuration) capsulewebhook.Handler return &collision{configuration: configuration} } +// nolint:dupl func (r *collision) OnCreate(client client.Client, decoder *admission.Decoder, recorder record.EventRecorder) capsulewebhook.Func { return func(ctx context.Context, req admission.Request) *admission.Response { - ingress, err := ingressFromRequest(req, decoder) + ing, err := ingressFromRequest(req, decoder) if err != nil { return utils.ErroredResponse(err) } var tenant *capsulev1beta1.Tenant - tenant, err = tenantFromIngress(ctx, client, ingress) + tenant, err = tenantFromIngress(ctx, client, ing) if err != nil { return utils.ErroredResponse(err) } - if tenant == nil { + if tenant == nil || tenant.Spec.IngressOptions.HostnameCollisionScope == capsulev1beta1.HostnameCollisionScopeDisabled { return nil } - if err = r.validateCollision(ctx, client, ingress); err == nil { + if err = r.validateCollision(ctx, client, ing, tenant.Spec.IngressOptions.HostnameCollisionScope); err == nil { return nil } var collisionErr *ingressHostnameCollision if errors.As(err, &collisionErr) { - recorder.Eventf(tenant, corev1.EventTypeWarning, "IngressHostnameCollision", "Ingress %s/%s hostname is colliding", ingress.Namespace(), ingress.Name()) + recorder.Eventf(tenant, corev1.EventTypeWarning, "IngressHostnameCollision", "Ingress %s/%s hostname is colliding", ing.Namespace(), ing.Name()) } response := admission.Denied(err.Error()) @@ -65,30 +69,33 @@ func (r *collision) OnCreate(client client.Client, decoder *admission.Decoder, r } } +// nolint:dupl func (r *collision) OnUpdate(client client.Client, decoder *admission.Decoder, recorder record.EventRecorder) capsulewebhook.Func { return func(ctx context.Context, req admission.Request) *admission.Response { - ingress, err := ingressFromRequest(req, decoder) + ing, err := ingressFromRequest(req, decoder) if err != nil { return utils.ErroredResponse(err) } var tenant *capsulev1beta1.Tenant - tenant, err = tenantFromIngress(ctx, client, ingress) + tenant, err = tenantFromIngress(ctx, client, ing) if err != nil { return utils.ErroredResponse(err) } - if tenant == nil { + if tenant == nil || tenant.Spec.IngressOptions.HostnameCollisionScope == capsulev1beta1.HostnameCollisionScopeDisabled { return nil } - err = r.validateCollision(ctx, client, ingress) + if err = r.validateCollision(ctx, client, ing, tenant.Spec.IngressOptions.HostnameCollisionScope); err == nil { + return nil + } var collisionErr *ingressHostnameCollision if errors.As(err, &collisionErr) { - recorder.Eventf(tenant, corev1.EventTypeWarning, "IngressHostnameCollision", "Ingress %s/%s hostname is colliding", ingress.Namespace(), ingress.Name()) + recorder.Eventf(tenant, corev1.EventTypeWarning, "IngressHostnameCollision", "Ingress %s/%s hostname is colliding", ing.Namespace(), ing.Name()) } response := admission.Denied(err.Error()) @@ -103,73 +110,114 @@ func (r *collision) OnDelete(client.Client, *admission.Decoder, record.EventReco } } -func (r *collision) validateCollision(ctx context.Context, clt client.Client, ingress Ingress) error { +func (r *collision) validateCollision(ctx context.Context, clt client.Client, ing Ingress, scope capsulev1beta1.HostnameCollisionScope) error { if r.configuration.AllowIngressHostnameCollision() { return nil } - for _, hostname := range ingress.Hostnames() { - switch ingress.(type) { - case Extension: - ingressObjList := &extensionsv1beta1.IngressList{} - err := clt.List(ctx, ingressObjList, client.MatchingFieldsSelector{ - Selector: fields.OneTermEqualSelector(".spec.rules[*].host", hostname), - }) - if err != nil { + for hostname, paths := range ing.HostnamePathsPairs() { + for path := range paths { + var ingressObjList client.ObjectList + + switch ing.(type) { + case Extension: + ingressObjList = &extensionsv1beta1.IngressList{} + case NetworkingV1: + ingressObjList = &networkingv1.IngressList{} + case NetworkingV1Beta1: + ingressObjList = &networkingv1beta1.IngressList{} + } + + namespaces := sets.NewString() + + switch scope { + case capsulev1beta1.HostnameCollisionScopeCluster: + tenantList := &capsulev1beta1.TenantList{} + if err := clt.List(ctx, tenantList); err != nil { + return err + } + + for _, tenant := range tenantList.Items { + namespaces.Insert(tenant.Status.Namespaces...) + } + case capsulev1beta1.HostnameCollisionScopeTenant: + selector := client.MatchingFieldsSelector{Selector: fields.OneTermEqualSelector(".status.namespaces", ing.Namespace())} + + tenantList := &capsulev1beta1.TenantList{} + if err := clt.List(ctx, tenantList, selector); err != nil { + return err + } + + for _, tenant := range tenantList.Items { + namespaces.Insert(tenant.Status.Namespaces...) + } + case capsulev1beta1.HostnameCollisionScopeNamespace: + namespaces.Insert(ing.Namespace()) + } + + fieldSelector := fields.OneTermEqualSelector(ingress.HostPathPair, fmt.Sprintf("%s;%s", hostname, path)) + + if err := clt.List(ctx, ingressObjList, client.MatchingFieldsSelector{Selector: fieldSelector}); err != nil { return err } - switch len(ingressObjList.Items) { - case 0: - break - case 1: - if ingressObj := ingressObjList.Items[0]; ingressObj.GetName() == ingress.Name() && ingressObj.GetNamespace() == ingress.Namespace() { - break - } - fallthrough - default: - return NewIngressHostnameCollision(hostname) - } - case NetworkingV1: - ingressObjList := &networkingv1.IngressList{} - err := clt.List(ctx, ingressObjList, client.MatchingFieldsSelector{ - Selector: fields.OneTermEqualSelector(".spec.rules[*].host", hostname), - }) - if err != nil { - return errors.Wrap(err, "cannot list *networkingv1.IngressList by MatchingFieldsSelector") - } + ingressList := sets.NewInt() - switch len(ingressObjList.Items) { - case 0: - break - case 1: - if ingressObj := ingressObjList.Items[0]; ingressObj.GetName() == ingress.Name() && ingressObj.GetNamespace() == ingress.Namespace() { - break - } - fallthrough - default: - return NewIngressHostnameCollision(hostname) - } - case NetworkingV1Beta1: - ingressObjList := &networkingv1beta1.IngressList{} - err := clt.List(ctx, ingressObjList, client.MatchingFieldsSelector{ - Selector: fields.OneTermEqualSelector(".spec.rules[*].host", hostname), - }) - if err != nil { - return errors.Wrap(err, "cannot list *networkingv1beta1.IngressList by MatchingFieldsSelector") - } - - switch len(ingressObjList.Items) { - case 0: - break - case 1: - if ingressObj := ingressObjList.Items[0]; ingressObj.GetName() == ingress.Name() && ingressObj.GetNamespace() == ingress.Namespace() { - break + switch list := ingressObjList.(type) { + case *extensionsv1beta1.IngressList: + for index, item := range list.Items { + if namespaces.Has(item.GetNamespace()) { + ingressList.Insert(index) + } } - fallthrough - default: - return NewIngressHostnameCollision(hostname) + switch len(ingressList) { + case 0: + break + case 1: + if index := ingressList.List()[0]; list.Items[index].GetName() == ing.Name() && list.Items[index].GetNamespace() == ing.Namespace() { + break + } + fallthrough + default: + return NewIngressHostnameCollision(hostname) + } + case *networkingv1.IngressList: + for index, item := range list.Items { + if namespaces.Has(item.GetNamespace()) { + ingressList.Insert(index) + } + } + + switch len(ingressList) { + case 0: + break + case 1: + if index := ingressList.List()[0]; list.Items[index].GetName() == ing.Name() && list.Items[index].GetNamespace() == ing.Namespace() { + break + } + fallthrough + default: + return NewIngressHostnameCollision(hostname) + } + case *networkingv1beta1.IngressList: + for index, item := range list.Items { + if namespaces.Has(item.GetNamespace()) { + ingressList.Insert(index) + } + } + + switch len(ingressList) { + case 0: + break + case 1: + if index := ingressList.List()[0]; list.Items[index].GetName() == ing.Name() && list.Items[index].GetNamespace() == ing.Namespace() { + break + } + fallthrough + default: + return NewIngressHostnameCollision(hostname) + } } } } diff --git a/pkg/webhook/ingress/validate_hostnames.go b/pkg/webhook/ingress/validate_hostnames.go index 31ba829e..294827d2 100644 --- a/pkg/webhook/ingress/validate_hostnames.go +++ b/pkg/webhook/ingress/validate_hostnames.go @@ -9,6 +9,7 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -41,11 +42,16 @@ func (r *hostnames) OnCreate(c client.Client, decoder *admission.Decoder, record return utils.ErroredResponse(err) } - if tenant == nil { + if tenant == nil || tenant.Spec.IngressOptions.AllowedHostnames == nil { return nil } - if err = r.validateHostnames(*tenant, ingress.Hostnames()); err == nil { + hostnameList := sets.NewString() + for hostname := range ingress.HostnamePathsPairs() { + hostnameList.Insert(hostname) + } + + if err = r.validateHostnames(*tenant, hostnameList); err == nil { return nil } @@ -81,7 +87,14 @@ func (r *hostnames) OnUpdate(c client.Client, decoder *admission.Decoder, record return nil } - err = r.validateHostnames(*tenant, ingress.Hostnames()) + hostnameSet := sets.NewString() + for hostname := range ingress.HostnamePathsPairs() { + hostnameSet.Insert(hostname) + } + + if err = r.validateHostnames(*tenant, hostnameSet); err == nil { + return nil + } var hostnameNotValidErr *ingressHostnameNotValid @@ -103,20 +116,19 @@ func (r *hostnames) OnDelete(client.Client, *admission.Decoder, record.EventReco } } -func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames []string) error { - if tenant.Spec.IngressOptions == nil || tenant.Spec.IngressOptions.AllowedHostnames == nil { +func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames sets.String) error { + if tenant.Spec.IngressOptions.AllowedHostnames == nil { return nil } var valid, matched bool + tenantHostnameSet := sets.NewString(tenant.Spec.IngressOptions.AllowedHostnames.Exact...) + var invalidHostnames []string if len(hostnames) > 0 { - for _, currentHostname := range hostnames { - isPresent := HostnamesList(tenant.Spec.IngressOptions.AllowedHostnames.Exact).IsStringInList(currentHostname) - if !isPresent { - invalidHostnames = append(invalidHostnames, currentHostname) - } + if diff := hostnames.Difference(tenantHostnameSet); len(diff) > 0 { + invalidHostnames = append(invalidHostnames, diff.List()...) } if len(invalidHostnames) == 0 { valid = true @@ -126,7 +138,7 @@ func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames [] var notMatchingHostnames []string allowedRegex := tenant.Spec.IngressOptions.AllowedHostnames.Regex if len(allowedRegex) > 0 { - for _, currentHostname := range hostnames { + for currentHostname := range hostnames { matched, _ = regexp.MatchString(allowedRegex, currentHostname) if !matched { notMatchingHostnames = append(notMatchingHostnames, currentHostname) diff --git a/pkg/webhook/tenant/hostnames_collision.go b/pkg/webhook/tenant/hostnames_collision.go index 5c3ea3f4..db7239e2 100644 --- a/pkg/webhook/tenant/hostnames_collision.go +++ b/pkg/webhook/tenant/hostnames_collision.go @@ -33,7 +33,7 @@ func (h *hostnamesCollisionHandler) validateTenant(ctx context.Context, req admi return utils.ErroredResponse(err) } - if !h.configuration.AllowTenantIngressHostnamesCollision() && tenant.Spec.IngressOptions != nil && tenant.Spec.IngressOptions.AllowedHostnames != nil && len(tenant.Spec.IngressOptions.AllowedHostnames.Exact) > 0 { + if !h.configuration.AllowTenantIngressHostnamesCollision() && tenant.Spec.IngressOptions.AllowedHostnames != nil && len(tenant.Spec.IngressOptions.AllowedHostnames.Exact) > 0 { for _, h := range tenant.Spec.IngressOptions.AllowedHostnames.Exact { tntList := &capsulev1beta1.TenantList{} if err := clt.List(ctx, tntList, client.MatchingFieldsSelector{ diff --git a/pkg/webhook/tenant/ingressclass_regex.go b/pkg/webhook/tenant/ingressclass_regex.go index e77ab197..10852dc2 100644 --- a/pkg/webhook/tenant/ingressclass_regex.go +++ b/pkg/webhook/tenant/ingressclass_regex.go @@ -30,7 +30,7 @@ func (h *ingressClassRegexHandler) validate(decoder *admission.Decoder, req admi return utils.ErroredResponse(err) } - if tenant.Spec.IngressOptions != nil && tenant.Spec.IngressOptions.AllowedClasses != nil && len(tenant.Spec.IngressOptions.AllowedClasses.Regex) > 0 { + if tenant.Spec.IngressOptions.AllowedClasses != nil && len(tenant.Spec.IngressOptions.AllowedClasses.Regex) > 0 { if _, err := regexp.Compile(tenant.Spec.IngressOptions.AllowedClasses.Regex); err != nil { response := admission.Denied("unable to compile ingressClasses allowedRegex")