From 658ab09648c236bbd872f42a48aa0a05a50da1fe Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Fri, 30 Jul 2021 17:11:09 +0200 Subject: [PATCH 01/16] refactor: grouping Ingress options into defined struct --- api/v1alpha1/conversion_hub.go | 22 +++++++++----- api/v1alpha1/conversion_hub_test.go | 12 ++++---- api/v1beta1/ingress_options.go | 11 +++++++ api/v1beta1/tenant_types.go | 6 ++-- api/v1beta1/zz_generated.deepcopy.go | 36 ++++++++++++++++++----- controllers/tenant/namespaces.go | 10 +++---- pkg/indexer/tenant/hostnames.go | 4 +-- pkg/webhook/ingress/validate_class.go | 12 ++++---- pkg/webhook/ingress/validate_hostnames.go | 11 ++++--- pkg/webhook/tenant/hostname_regex.go | 4 +-- pkg/webhook/tenant/hostnames_collision.go | 4 +-- pkg/webhook/tenant/ingressclass_regex.go | 4 +-- 12 files changed, 86 insertions(+), 50 deletions(-) create mode 100644 api/v1beta1/ingress_options.go diff --git a/api/v1alpha1/conversion_hub.go b/api/v1alpha1/conversion_hub.go index 9c92c438..3f0c535c 100644 --- a/api/v1alpha1/conversion_hub.go +++ b/api/v1alpha1/conversion_hub.go @@ -171,13 +171,19 @@ func (t *Tenant) ConvertTo(dstRaw conversion.Hub) error { } } if t.Spec.IngressClasses != nil { - dst.Spec.IngressClasses = &capsulev1beta1.AllowedListSpec{ + if dst.Spec.IngressOptions == nil { + dst.Spec.IngressOptions = &capsulev1beta1.IngressOptions{} + } + dst.Spec.IngressOptions.IngressClasses = &capsulev1beta1.AllowedListSpec{ Exact: t.Spec.IngressClasses.Exact, Regex: t.Spec.IngressClasses.Regex, } } if t.Spec.IngressHostnames != nil { - dst.Spec.IngressHostnames = &capsulev1beta1.AllowedListSpec{ + if dst.Spec.IngressOptions == nil { + dst.Spec.IngressOptions = &capsulev1beta1.IngressOptions{} + } + dst.Spec.IngressOptions.IngressHostnames = &capsulev1beta1.AllowedListSpec{ Exact: t.Spec.IngressHostnames.Exact, Regex: t.Spec.IngressHostnames.Regex, } @@ -453,16 +459,16 @@ func (t *Tenant) ConvertFrom(srcRaw conversion.Hub) error { Regex: src.Spec.StorageClasses.Regex, } } - if src.Spec.IngressClasses != nil { + if src.Spec.IngressOptions != nil && src.Spec.IngressOptions.IngressClasses != nil { t.Spec.IngressClasses = &AllowedListSpec{ - Exact: src.Spec.IngressClasses.Exact, - Regex: src.Spec.IngressClasses.Regex, + Exact: src.Spec.IngressOptions.IngressClasses.Exact, + Regex: src.Spec.IngressOptions.IngressClasses.Regex, } } - if src.Spec.IngressHostnames != nil { + if src.Spec.IngressOptions != nil && src.Spec.IngressOptions.IngressHostnames != nil { t.Spec.IngressHostnames = &AllowedListSpec{ - Exact: src.Spec.IngressHostnames.Exact, - Regex: src.Spec.IngressHostnames.Regex, + Exact: src.Spec.IngressOptions.IngressHostnames.Exact, + Regex: src.Spec.IngressOptions.IngressHostnames.Regex, } } if src.Spec.ContainerRegistries != nil { diff --git a/api/v1alpha1/conversion_hub_test.go b/api/v1alpha1/conversion_hub_test.go index b92fbf7a..4c87629c 100644 --- a/api/v1alpha1/conversion_hub_test.go +++ b/api/v1alpha1/conversion_hub_test.go @@ -229,11 +229,13 @@ func generateTenantsSpecs() (Tenant, capsulev1beta1.Tenant) { }, }, }, - NamespaceOptions: v1beta1NamespaceOptions, - ServiceOptions: v1beta1ServiceOptions, - StorageClasses: v1beta1AllowedListSpec, - IngressClasses: v1beta1AllowedListSpec, - IngressHostnames: v1beta1AllowedListSpec, + NamespaceOptions: v1beta1NamespaceOptions, + ServiceOptions: v1beta1ServiceOptions, + StorageClasses: v1beta1AllowedListSpec, + IngressOptions: &capsulev1beta1.IngressOptions{ + IngressClasses: v1beta1AllowedListSpec, + IngressHostnames: v1beta1AllowedListSpec, + }, ContainerRegistries: v1beta1AllowedListSpec, NodeSelector: nodeSelector, NetworkPolicies: &capsulev1beta1.NetworkPolicySpec{ diff --git a/api/v1beta1/ingress_options.go b/api/v1beta1/ingress_options.go new file mode 100644 index 00000000..fd87000f --- /dev/null +++ b/api/v1beta1/ingress_options.go @@ -0,0 +1,11 @@ +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +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. + IngressClasses *AllowedListSpec `json:"ingressClasses,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. + IngressHostnames *AllowedListSpec `json:"ingressHostnames,omitempty"` +} diff --git a/api/v1beta1/tenant_types.go b/api/v1beta1/tenant_types.go index a0f59ba1..c3b25805 100644 --- a/api/v1beta1/tenant_types.go +++ b/api/v1beta1/tenant_types.go @@ -17,10 +17,8 @@ type TenantSpec struct { ServiceOptions *ServiceOptions `json:"serviceOptions,omitempty"` // 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 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. - IngressClasses *AllowedListSpec `json:"ingressClasses,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. - IngressHostnames *AllowedListSpec `json:"ingressHostnames,omitempty"` + // Specifies options for the Ingress resources, such as allowed hostnames and IngressClass. Optional. + 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 0d9423bf..4239df7d 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -149,6 +149,31 @@ func (in *ExternalServiceIPsSpec) DeepCopy() *ExternalServiceIPsSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IngressOptions) DeepCopyInto(out *IngressOptions) { + *out = *in + if in.IngressClasses != nil { + in, out := &in.IngressClasses, &out.IngressClasses + *out = new(AllowedListSpec) + (*in).DeepCopyInto(*out) + } + if in.IngressHostnames != nil { + in, out := &in.IngressHostnames, &out.IngressHostnames + *out = new(AllowedListSpec) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IngressOptions. +func (in *IngressOptions) DeepCopy() *IngressOptions { + if in == nil { + return nil + } + out := new(IngressOptions) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LimitRangesSpec) DeepCopyInto(out *LimitRangesSpec) { *out = *in @@ -417,14 +442,9 @@ func (in *TenantSpec) DeepCopyInto(out *TenantSpec) { *out = new(AllowedListSpec) (*in).DeepCopyInto(*out) } - if in.IngressClasses != nil { - in, out := &in.IngressClasses, &out.IngressClasses - *out = new(AllowedListSpec) - (*in).DeepCopyInto(*out) - } - if in.IngressHostnames != nil { - in, out := &in.IngressHostnames, &out.IngressHostnames - *out = new(AllowedListSpec) + if in.IngressOptions != nil { + in, out := &in.IngressOptions, &out.IngressOptions + *out = new(IngressOptions) (*in).DeepCopyInto(*out) } if in.ContainerRegistries != nil { diff --git a/controllers/tenant/namespaces.go b/controllers/tenant/namespaces.go index 7df6a52c..0246b77e 100644 --- a/controllers/tenant/namespaces.go +++ b/controllers/tenant/namespaces.go @@ -64,12 +64,12 @@ func (r *Manager) syncNamespaceMetadata(namespace string, tnt *capsulev1beta1.Te annotations["scheduler.alpha.kubernetes.io/node-selector"] = strings.Join(selector, ",") } - if tnt.Spec.IngressClasses != nil { - if len(tnt.Spec.IngressClasses.Exact) > 0 { - annotations[capsulev1beta1.AvailableIngressClassesAnnotation] = strings.Join(tnt.Spec.IngressClasses.Exact, ",") + if tnt.Spec.IngressOptions != nil && tnt.Spec.IngressOptions.IngressClasses != nil { + if len(tnt.Spec.IngressOptions.IngressClasses.Exact) > 0 { + annotations[capsulev1beta1.AvailableIngressClassesAnnotation] = strings.Join(tnt.Spec.IngressOptions.IngressClasses.Exact, ",") } - if len(tnt.Spec.IngressClasses.Regex) > 0 { - annotations[capsulev1beta1.AvailableIngressClassesRegexpAnnotation] = tnt.Spec.IngressClasses.Regex + if len(tnt.Spec.IngressOptions.IngressClasses.Regex) > 0 { + annotations[capsulev1beta1.AvailableIngressClassesRegexpAnnotation] = tnt.Spec.IngressOptions.IngressClasses.Regex } } diff --git a/pkg/indexer/tenant/hostnames.go b/pkg/indexer/tenant/hostnames.go index 7812d785..4cd8a614 100644 --- a/pkg/indexer/tenant/hostnames.go +++ b/pkg/indexer/tenant/hostnames.go @@ -23,8 +23,8 @@ func (IngressHostnames) Field() string { func (IngressHostnames) Func() client.IndexerFunc { return func(object client.Object) (out []string) { tenant := object.(*capsulev1beta1.Tenant) - if tenant.Spec.IngressHostnames != nil { - out = append(out, tenant.Spec.IngressHostnames.Exact...) + if tenant.Spec.IngressOptions != nil && tenant.Spec.IngressOptions.IngressHostnames != nil { + out = append(out, tenant.Spec.IngressOptions.IngressHostnames.Exact...) } return } diff --git a/pkg/webhook/ingress/validate_class.go b/pkg/webhook/ingress/validate_class.go index ef960684..3fb73a98 100644 --- a/pkg/webhook/ingress/validate_class.go +++ b/pkg/webhook/ingress/validate_class.go @@ -111,23 +111,23 @@ func (r *class) OnDelete(client.Client, *admission.Decoder, record.EventRecorder } func (r *class) validateClass(tenant capsulev1beta1.Tenant, ingressClass *string) error { - if tenant.Spec.IngressClasses == nil { + if tenant.Spec.IngressOptions.IngressClasses == nil { return nil } if ingressClass == nil { - return NewIngressClassNotValid(*tenant.Spec.IngressClasses) + return NewIngressClassNotValid(*tenant.Spec.IngressOptions.IngressClasses) } var valid, matched bool - if len(tenant.Spec.IngressClasses.Exact) > 0 { - valid = tenant.Spec.IngressClasses.ExactMatch(*ingressClass) + if len(tenant.Spec.IngressOptions.IngressClasses.Exact) > 0 { + valid = tenant.Spec.IngressOptions.IngressClasses.ExactMatch(*ingressClass) } - matched = tenant.Spec.IngressClasses.RegexMatch(*ingressClass) + matched = tenant.Spec.IngressOptions.IngressClasses.RegexMatch(*ingressClass) if !valid && !matched { - return NewIngressClassForbidden(*ingressClass, *tenant.Spec.IngressClasses) + return NewIngressClassForbidden(*ingressClass, *tenant.Spec.IngressOptions.IngressClasses) } return nil diff --git a/pkg/webhook/ingress/validate_hostnames.go b/pkg/webhook/ingress/validate_hostnames.go index f6f1a674..5f02b7b0 100644 --- a/pkg/webhook/ingress/validate_hostnames.go +++ b/pkg/webhook/ingress/validate_hostnames.go @@ -14,7 +14,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" - "github.com/clastix/capsule/pkg/configuration" capsulewebhook "github.com/clastix/capsule/pkg/webhook" "github.com/clastix/capsule/pkg/webhook/utils" @@ -105,7 +104,7 @@ func (r *hostnames) OnDelete(client.Client, *admission.Decoder, record.EventReco } func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames []string) error { - if tenant.Spec.IngressHostnames == nil { + if tenant.Spec.IngressOptions == nil || tenant.Spec.IngressOptions.IngressHostnames == nil { return nil } @@ -114,7 +113,7 @@ func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames [] var invalidHostnames []string if len(hostnames) > 0 { for _, currentHostname := range hostnames { - isPresent := HostnamesList(tenant.Spec.IngressHostnames.Exact).IsStringInList(currentHostname) + isPresent := HostnamesList(tenant.Spec.IngressOptions.IngressHostnames.Exact).IsStringInList(currentHostname) if !isPresent { invalidHostnames = append(invalidHostnames, currentHostname) } @@ -125,10 +124,10 @@ func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames [] } var notMatchingHostnames []string - allowedRegex := tenant.Spec.IngressHostnames.Regex + allowedRegex := tenant.Spec.IngressOptions.IngressHostnames.Regex if len(allowedRegex) > 0 { for _, currentHostname := range hostnames { - matched, _ = regexp.MatchString(tenant.Spec.IngressHostnames.Regex, currentHostname) + matched, _ = regexp.MatchString(allowedRegex, currentHostname) if !matched { notMatchingHostnames = append(notMatchingHostnames, currentHostname) } @@ -139,7 +138,7 @@ func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames [] } if !valid && !matched { - return NewIngressHostnamesNotValid(invalidHostnames, notMatchingHostnames, *tenant.Spec.IngressHostnames) + return NewIngressHostnamesNotValid(invalidHostnames, notMatchingHostnames, *tenant.Spec.IngressOptions.IngressHostnames) } return nil diff --git a/pkg/webhook/tenant/hostname_regex.go b/pkg/webhook/tenant/hostname_regex.go index bbb54228..6bd9758f 100644 --- a/pkg/webhook/tenant/hostname_regex.go +++ b/pkg/webhook/tenant/hostname_regex.go @@ -30,8 +30,8 @@ func (h *hostnameRegexHandler) validate(decoder *admission.Decoder, req admissio return utils.ErroredResponse(err) } - if tenant.Spec.IngressHostnames != nil && len(tenant.Spec.IngressHostnames.Regex) > 0 { - if _, err := regexp.Compile(tenant.Spec.IngressHostnames.Regex); err != nil { + if tenant.Spec.IngressOptions.IngressHostnames != nil && len(tenant.Spec.IngressOptions.IngressHostnames.Regex) > 0 { + if _, err := regexp.Compile(tenant.Spec.IngressOptions.IngressHostnames.Regex); err != nil { response := admission.Denied("unable to compile allowedHostnames allowedRegex") return &response diff --git a/pkg/webhook/tenant/hostnames_collision.go b/pkg/webhook/tenant/hostnames_collision.go index 68816309..9d76de7b 100644 --- a/pkg/webhook/tenant/hostnames_collision.go +++ b/pkg/webhook/tenant/hostnames_collision.go @@ -33,8 +33,8 @@ func (h *hostnamesCollisionHandler) validateTenant(ctx context.Context, req admi return utils.ErroredResponse(err) } - if !h.configuration.AllowTenantIngressHostnamesCollision() && tenant.Spec.IngressHostnames != nil && len(tenant.Spec.IngressHostnames.Exact) > 0 { - for _, h := range tenant.Spec.IngressHostnames.Exact { + if !h.configuration.AllowTenantIngressHostnamesCollision() && tenant.Spec.IngressOptions != nil && tenant.Spec.IngressOptions.IngressHostnames != nil && len(tenant.Spec.IngressOptions.IngressHostnames.Exact) > 0 { + for _, h := range tenant.Spec.IngressOptions.IngressHostnames.Exact { tntList := &capsulev1beta1.TenantList{} if err := clt.List(ctx, tntList, client.MatchingFieldsSelector{ Selector: fields.OneTermEqualSelector(".spec.ingressHostnames", h), diff --git a/pkg/webhook/tenant/ingressclass_regex.go b/pkg/webhook/tenant/ingressclass_regex.go index 7fb0e256..9a796de6 100644 --- a/pkg/webhook/tenant/ingressclass_regex.go +++ b/pkg/webhook/tenant/ingressclass_regex.go @@ -30,8 +30,8 @@ func (h *ingressClassRegexHandler) validate(decoder *admission.Decoder, req admi return utils.ErroredResponse(err) } - if tenant.Spec.IngressClasses != nil && len(tenant.Spec.IngressClasses.Regex) > 0 { - if _, err := regexp.Compile(tenant.Spec.IngressClasses.Regex); err != nil { + if tenant.Spec.IngressOptions != nil && tenant.Spec.IngressOptions.IngressClasses != nil && len(tenant.Spec.IngressOptions.IngressClasses.Regex) > 0 { + if _, err := regexp.Compile(tenant.Spec.IngressOptions.IngressClasses.Regex); err != nil { response := admission.Denied("unable to compile ingressClasses allowedRegex") return &response From 5aa3267792c0ed38e46befac249d813c56d4952d Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Fri, 30 Jul 2021 17:15:55 +0200 Subject: [PATCH 02/16] refactor: renaming struct field names for allowed hostnames and classes --- api/v1alpha1/conversion_hub.go | 16 ++++++++-------- api/v1alpha1/conversion_hub_test.go | 4 ++-- api/v1beta1/ingress_options.go | 4 ++-- api/v1beta1/zz_generated.deepcopy.go | 8 ++++---- controllers/tenant/namespaces.go | 10 +++++----- pkg/indexer/tenant/hostnames.go | 4 ++-- pkg/webhook/ingress/validate_class.go | 12 ++++++------ pkg/webhook/ingress/validate_hostnames.go | 8 ++++---- pkg/webhook/tenant/hostname_regex.go | 4 ++-- pkg/webhook/tenant/hostnames_collision.go | 4 ++-- pkg/webhook/tenant/ingressclass_regex.go | 4 ++-- 11 files changed, 39 insertions(+), 39 deletions(-) diff --git a/api/v1alpha1/conversion_hub.go b/api/v1alpha1/conversion_hub.go index 3f0c535c..e9dd23cf 100644 --- a/api/v1alpha1/conversion_hub.go +++ b/api/v1alpha1/conversion_hub.go @@ -174,7 +174,7 @@ func (t *Tenant) ConvertTo(dstRaw conversion.Hub) error { if dst.Spec.IngressOptions == nil { dst.Spec.IngressOptions = &capsulev1beta1.IngressOptions{} } - dst.Spec.IngressOptions.IngressClasses = &capsulev1beta1.AllowedListSpec{ + dst.Spec.IngressOptions.AllowedClasses = &capsulev1beta1.AllowedListSpec{ Exact: t.Spec.IngressClasses.Exact, Regex: t.Spec.IngressClasses.Regex, } @@ -183,7 +183,7 @@ func (t *Tenant) ConvertTo(dstRaw conversion.Hub) error { if dst.Spec.IngressOptions == nil { dst.Spec.IngressOptions = &capsulev1beta1.IngressOptions{} } - dst.Spec.IngressOptions.IngressHostnames = &capsulev1beta1.AllowedListSpec{ + dst.Spec.IngressOptions.AllowedHostnames = &capsulev1beta1.AllowedListSpec{ Exact: t.Spec.IngressHostnames.Exact, Regex: t.Spec.IngressHostnames.Regex, } @@ -459,16 +459,16 @@ func (t *Tenant) ConvertFrom(srcRaw conversion.Hub) error { Regex: src.Spec.StorageClasses.Regex, } } - if src.Spec.IngressOptions != nil && src.Spec.IngressOptions.IngressClasses != nil { + if src.Spec.IngressOptions != nil && src.Spec.IngressOptions.AllowedClasses != nil { t.Spec.IngressClasses = &AllowedListSpec{ - Exact: src.Spec.IngressOptions.IngressClasses.Exact, - Regex: src.Spec.IngressOptions.IngressClasses.Regex, + Exact: src.Spec.IngressOptions.AllowedClasses.Exact, + Regex: src.Spec.IngressOptions.AllowedClasses.Regex, } } - if src.Spec.IngressOptions != nil && src.Spec.IngressOptions.IngressHostnames != nil { + if src.Spec.IngressOptions != nil && src.Spec.IngressOptions.AllowedHostnames != nil { t.Spec.IngressHostnames = &AllowedListSpec{ - Exact: src.Spec.IngressOptions.IngressHostnames.Exact, - Regex: src.Spec.IngressOptions.IngressHostnames.Regex, + Exact: src.Spec.IngressOptions.AllowedHostnames.Exact, + Regex: src.Spec.IngressOptions.AllowedHostnames.Regex, } } if src.Spec.ContainerRegistries != nil { diff --git a/api/v1alpha1/conversion_hub_test.go b/api/v1alpha1/conversion_hub_test.go index 4c87629c..4fa5cb3d 100644 --- a/api/v1alpha1/conversion_hub_test.go +++ b/api/v1alpha1/conversion_hub_test.go @@ -233,8 +233,8 @@ func generateTenantsSpecs() (Tenant, capsulev1beta1.Tenant) { ServiceOptions: v1beta1ServiceOptions, StorageClasses: v1beta1AllowedListSpec, IngressOptions: &capsulev1beta1.IngressOptions{ - IngressClasses: v1beta1AllowedListSpec, - IngressHostnames: v1beta1AllowedListSpec, + AllowedClasses: v1beta1AllowedListSpec, + AllowedHostnames: v1beta1AllowedListSpec, }, ContainerRegistries: v1beta1AllowedListSpec, NodeSelector: nodeSelector, diff --git a/api/v1beta1/ingress_options.go b/api/v1beta1/ingress_options.go index fd87000f..eaa23efe 100644 --- a/api/v1beta1/ingress_options.go +++ b/api/v1beta1/ingress_options.go @@ -5,7 +5,7 @@ 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. - IngressClasses *AllowedListSpec `json:"ingressClasses,omitempty"` + AllowedClasses *AllowedListSpec `json:"allowedClasses,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. - IngressHostnames *AllowedListSpec `json:"ingressHostnames,omitempty"` + AllowedHostnames *AllowedListSpec `json:"allowedHostnames,omitempty"` } diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 4239df7d..69bf367e 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -152,13 +152,13 @@ func (in *ExternalServiceIPsSpec) DeepCopy() *ExternalServiceIPsSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IngressOptions) DeepCopyInto(out *IngressOptions) { *out = *in - if in.IngressClasses != nil { - in, out := &in.IngressClasses, &out.IngressClasses + if in.AllowedClasses != nil { + in, out := &in.AllowedClasses, &out.AllowedClasses *out = new(AllowedListSpec) (*in).DeepCopyInto(*out) } - if in.IngressHostnames != nil { - in, out := &in.IngressHostnames, &out.IngressHostnames + if in.AllowedHostnames != nil { + in, out := &in.AllowedHostnames, &out.AllowedHostnames *out = new(AllowedListSpec) (*in).DeepCopyInto(*out) } diff --git a/controllers/tenant/namespaces.go b/controllers/tenant/namespaces.go index 0246b77e..cfbf9a85 100644 --- a/controllers/tenant/namespaces.go +++ b/controllers/tenant/namespaces.go @@ -64,12 +64,12 @@ 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.IngressClasses != nil { - if len(tnt.Spec.IngressOptions.IngressClasses.Exact) > 0 { - annotations[capsulev1beta1.AvailableIngressClassesAnnotation] = strings.Join(tnt.Spec.IngressOptions.IngressClasses.Exact, ",") + if tnt.Spec.IngressOptions != nil && tnt.Spec.IngressOptions.AllowedClasses != nil { + if len(tnt.Spec.IngressOptions.AllowedClasses.Exact) > 0 { + annotations[capsulev1beta1.AvailableIngressClassesAnnotation] = strings.Join(tnt.Spec.IngressOptions.AllowedClasses.Exact, ",") } - if len(tnt.Spec.IngressOptions.IngressClasses.Regex) > 0 { - annotations[capsulev1beta1.AvailableIngressClassesRegexpAnnotation] = tnt.Spec.IngressOptions.IngressClasses.Regex + if len(tnt.Spec.IngressOptions.AllowedClasses.Regex) > 0 { + annotations[capsulev1beta1.AvailableIngressClassesRegexpAnnotation] = tnt.Spec.IngressOptions.AllowedClasses.Regex } } diff --git a/pkg/indexer/tenant/hostnames.go b/pkg/indexer/tenant/hostnames.go index 4cd8a614..5e69a200 100644 --- a/pkg/indexer/tenant/hostnames.go +++ b/pkg/indexer/tenant/hostnames.go @@ -23,8 +23,8 @@ 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.IngressHostnames != nil { - out = append(out, tenant.Spec.IngressOptions.IngressHostnames.Exact...) + if tenant.Spec.IngressOptions != nil && tenant.Spec.IngressOptions.AllowedHostnames != nil { + out = append(out, tenant.Spec.IngressOptions.AllowedHostnames.Exact...) } return } diff --git a/pkg/webhook/ingress/validate_class.go b/pkg/webhook/ingress/validate_class.go index 3fb73a98..58fd2449 100644 --- a/pkg/webhook/ingress/validate_class.go +++ b/pkg/webhook/ingress/validate_class.go @@ -111,23 +111,23 @@ func (r *class) OnDelete(client.Client, *admission.Decoder, record.EventRecorder } func (r *class) validateClass(tenant capsulev1beta1.Tenant, ingressClass *string) error { - if tenant.Spec.IngressOptions.IngressClasses == nil { + if tenant.Spec.IngressOptions.AllowedClasses == nil { return nil } if ingressClass == nil { - return NewIngressClassNotValid(*tenant.Spec.IngressOptions.IngressClasses) + return NewIngressClassNotValid(*tenant.Spec.IngressOptions.AllowedClasses) } var valid, matched bool - if len(tenant.Spec.IngressOptions.IngressClasses.Exact) > 0 { - valid = tenant.Spec.IngressOptions.IngressClasses.ExactMatch(*ingressClass) + if len(tenant.Spec.IngressOptions.AllowedClasses.Exact) > 0 { + valid = tenant.Spec.IngressOptions.AllowedClasses.ExactMatch(*ingressClass) } - matched = tenant.Spec.IngressOptions.IngressClasses.RegexMatch(*ingressClass) + matched = tenant.Spec.IngressOptions.AllowedClasses.RegexMatch(*ingressClass) if !valid && !matched { - return NewIngressClassForbidden(*ingressClass, *tenant.Spec.IngressOptions.IngressClasses) + return NewIngressClassForbidden(*ingressClass, *tenant.Spec.IngressOptions.AllowedClasses) } return nil diff --git a/pkg/webhook/ingress/validate_hostnames.go b/pkg/webhook/ingress/validate_hostnames.go index 5f02b7b0..31ba829e 100644 --- a/pkg/webhook/ingress/validate_hostnames.go +++ b/pkg/webhook/ingress/validate_hostnames.go @@ -104,7 +104,7 @@ 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.IngressHostnames == nil { + if tenant.Spec.IngressOptions == nil || tenant.Spec.IngressOptions.AllowedHostnames == nil { return nil } @@ -113,7 +113,7 @@ func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames [] var invalidHostnames []string if len(hostnames) > 0 { for _, currentHostname := range hostnames { - isPresent := HostnamesList(tenant.Spec.IngressOptions.IngressHostnames.Exact).IsStringInList(currentHostname) + isPresent := HostnamesList(tenant.Spec.IngressOptions.AllowedHostnames.Exact).IsStringInList(currentHostname) if !isPresent { invalidHostnames = append(invalidHostnames, currentHostname) } @@ -124,7 +124,7 @@ func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames [] } var notMatchingHostnames []string - allowedRegex := tenant.Spec.IngressOptions.IngressHostnames.Regex + allowedRegex := tenant.Spec.IngressOptions.AllowedHostnames.Regex if len(allowedRegex) > 0 { for _, currentHostname := range hostnames { matched, _ = regexp.MatchString(allowedRegex, currentHostname) @@ -138,7 +138,7 @@ func (r *hostnames) validateHostnames(tenant capsulev1beta1.Tenant, hostnames [] } if !valid && !matched { - return NewIngressHostnamesNotValid(invalidHostnames, notMatchingHostnames, *tenant.Spec.IngressOptions.IngressHostnames) + return NewIngressHostnamesNotValid(invalidHostnames, notMatchingHostnames, *tenant.Spec.IngressOptions.AllowedHostnames) } return nil diff --git a/pkg/webhook/tenant/hostname_regex.go b/pkg/webhook/tenant/hostname_regex.go index 6bd9758f..90048fbd 100644 --- a/pkg/webhook/tenant/hostname_regex.go +++ b/pkg/webhook/tenant/hostname_regex.go @@ -30,8 +30,8 @@ func (h *hostnameRegexHandler) validate(decoder *admission.Decoder, req admissio return utils.ErroredResponse(err) } - if tenant.Spec.IngressOptions.IngressHostnames != nil && len(tenant.Spec.IngressOptions.IngressHostnames.Regex) > 0 { - if _, err := regexp.Compile(tenant.Spec.IngressOptions.IngressHostnames.Regex); err != nil { + if tenant.Spec.IngressOptions.AllowedHostnames != nil && len(tenant.Spec.IngressOptions.AllowedHostnames.Regex) > 0 { + if _, err := regexp.Compile(tenant.Spec.IngressOptions.AllowedHostnames.Regex); err != nil { response := admission.Denied("unable to compile allowedHostnames allowedRegex") return &response diff --git a/pkg/webhook/tenant/hostnames_collision.go b/pkg/webhook/tenant/hostnames_collision.go index 9d76de7b..5c3ea3f4 100644 --- a/pkg/webhook/tenant/hostnames_collision.go +++ b/pkg/webhook/tenant/hostnames_collision.go @@ -33,8 +33,8 @@ func (h *hostnamesCollisionHandler) validateTenant(ctx context.Context, req admi return utils.ErroredResponse(err) } - if !h.configuration.AllowTenantIngressHostnamesCollision() && tenant.Spec.IngressOptions != nil && tenant.Spec.IngressOptions.IngressHostnames != nil && len(tenant.Spec.IngressOptions.IngressHostnames.Exact) > 0 { - for _, h := range tenant.Spec.IngressOptions.IngressHostnames.Exact { + if !h.configuration.AllowTenantIngressHostnamesCollision() && tenant.Spec.IngressOptions != nil && 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{ Selector: fields.OneTermEqualSelector(".spec.ingressHostnames", h), diff --git a/pkg/webhook/tenant/ingressclass_regex.go b/pkg/webhook/tenant/ingressclass_regex.go index 9a796de6..e77ab197 100644 --- a/pkg/webhook/tenant/ingressclass_regex.go +++ b/pkg/webhook/tenant/ingressclass_regex.go @@ -30,8 +30,8 @@ func (h *ingressClassRegexHandler) validate(decoder *admission.Decoder, req admi return utils.ErroredResponse(err) } - if tenant.Spec.IngressOptions != nil && tenant.Spec.IngressOptions.IngressClasses != nil && len(tenant.Spec.IngressOptions.IngressClasses.Regex) > 0 { - if _, err := regexp.Compile(tenant.Spec.IngressOptions.IngressClasses.Regex); err != nil { + if tenant.Spec.IngressOptions != nil && 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") return &response From cc52f7765a7fbe6c663d49f61a7b631a7ef23877 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sat, 31 Jul 2021 17:22:35 +0200 Subject: [PATCH 03/16] refactor: avoiding init functions for direct registration --- pkg/indexer/add_ingress.go | 26 -------------------------- pkg/indexer/add_namespace.go | 12 ------------ pkg/indexer/add_tenant.go | 12 ------------ pkg/indexer/indexer.go | 26 +++++++++++++++++++++++--- pkg/indexer/ingress/hostname.go | 11 ++++++----- 5 files changed, 29 insertions(+), 58 deletions(-) delete mode 100644 pkg/indexer/add_ingress.go delete mode 100644 pkg/indexer/add_namespace.go delete mode 100644 pkg/indexer/add_tenant.go diff --git a/pkg/indexer/add_ingress.go b/pkg/indexer/add_ingress.go deleted file mode 100644 index 6248b521..00000000 --- a/pkg/indexer/add_ingress.go +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package indexer - -import ( - extensionsv1beta1 "k8s.io/api/extensions/v1beta1" - networkingv1 "k8s.io/api/networking/v1" - networkingv1beta1 "k8s.io/api/networking/v1beta1" - - "github.com/clastix/capsule/pkg/indexer/ingress" - "github.com/clastix/capsule/pkg/webhook/utils" -) - -func init() { - majorVer, minorVer, _, _ := utils.GetK8sVersion() - - switch { - case majorVer == 1 && minorVer >= 19: - AddToIndexerFuncs = append(AddToIndexerFuncs, ingress.Hostname{Obj: &networkingv1.Ingress{}}) - case majorVer == 1 && (minorVer >= 19 && minorVer < 22): - AddToIndexerFuncs = append(AddToIndexerFuncs, ingress.Hostname{Obj: &networkingv1beta1.Ingress{}}) - case majorVer == 1 && minorVer < 22: - AddToIndexerFuncs = append(AddToIndexerFuncs, ingress.Hostname{Obj: &extensionsv1beta1.Ingress{}}) - } -} diff --git a/pkg/indexer/add_namespace.go b/pkg/indexer/add_namespace.go deleted file mode 100644 index 772fcfff..00000000 --- a/pkg/indexer/add_namespace.go +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package indexer - -import ( - "github.com/clastix/capsule/pkg/indexer/namespace" -) - -func init() { - AddToIndexerFuncs = append(AddToIndexerFuncs, namespace.OwnerReference{}) -} diff --git a/pkg/indexer/add_tenant.go b/pkg/indexer/add_tenant.go deleted file mode 100644 index a269a016..00000000 --- a/pkg/indexer/add_tenant.go +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package indexer - -import "github.com/clastix/capsule/pkg/indexer/tenant" - -func init() { - AddToIndexerFuncs = append(AddToIndexerFuncs, tenant.IngressHostnames{}) - AddToIndexerFuncs = append(AddToIndexerFuncs, tenant.NamespacesReference{}) - AddToIndexerFuncs = append(AddToIndexerFuncs, tenant.OwnerReference{}) -} diff --git a/pkg/indexer/indexer.go b/pkg/indexer/indexer.go index 205c5002..9e6aa36b 100644 --- a/pkg/indexer/indexer.go +++ b/pkg/indexer/indexer.go @@ -6,6 +6,13 @@ 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" ) @@ -16,13 +23,26 @@ type CustomIndexer interface { Func() client.IndexerFunc } -var AddToIndexerFuncs []CustomIndexer - func AddToManager(m manager.Manager) error { - for _, f := range AddToIndexerFuncs { + 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 >= 19 { + indexers = append(indexers, ingress.Hostname{Obj: &networkingv1.Ingress{}}) + } + + for _, f := range indexers { if err := m.GetFieldIndexer().IndexField(context.TODO(), f.Object(), f.Field(), f.Func()); err != nil { return err } } + return nil } diff --git a/pkg/indexer/ingress/hostname.go b/pkg/indexer/ingress/hostname.go index 299ffd89..11bd22c2 100644 --- a/pkg/indexer/ingress/hostname.go +++ b/pkg/indexer/ingress/hostname.go @@ -11,6 +11,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +const ( + HostnameFieldSelector = "host" +) + type Hostname struct { Obj metav1.Object } @@ -20,26 +24,23 @@ func (h Hostname) Object() client.Object { } func (h Hostname) Field() string { - return ".spec.rules[*].host" + return HostnameFieldSelector } func (h Hostname) Func() client.IndexerFunc { return func(object client.Object) (hostnames []string) { - switch h.Obj.(type) { + switch ing := h.Obj.(type) { case *networkingv1.Ingress: - ing := object.(*networkingv1.Ingress) for _, r := range ing.Spec.Rules { hostnames = append(hostnames, r.Host) } return case *networkingv1beta1.Ingress: - ing := object.(*networkingv1beta1.Ingress) for _, r := range ing.Spec.Rules { hostnames = append(hostnames, r.Host) } return case *extensionsv1beta1.Ingress: - ing := object.(*extensionsv1beta1.Ingress) for _, r := range ing.Spec.Rules { hostnames = append(hostnames, r.Host) } From 6c5e372fd3a65c741a17fbee5aaf5629b4a70fab Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sun, 1 Aug 2021 15:26:27 +0200 Subject: [PATCH 04/16] style: no need of nolint here --- api/v1alpha1/capsuleconfiguration_types.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/v1alpha1/capsuleconfiguration_types.go b/api/v1alpha1/capsuleconfiguration_types.go index 2b297d6f..7322d2a3 100644 --- a/api/v1alpha1/capsuleconfiguration_types.go +++ b/api/v1alpha1/capsuleconfiguration_types.go @@ -8,7 +8,6 @@ import ( ) // CapsuleConfigurationSpec defines the Capsule configuration -// nolint:maligned type CapsuleConfigurationSpec struct { // Names of the groups for Capsule users. // +kubebuilder:default={capsule.clastix.io} From 9e296d42fac79d850eeef7a441b25818b64d8931 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sat, 31 Jul 2021 23:08:35 +0200 Subject: [PATCH 05/16] 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 | 170 ++++++++++++++-------- pkg/webhook/ingress/validate_hostnames.go | 34 +++-- pkg/webhook/tenant/hostnames_collision.go | 2 +- pkg/webhook/tenant/ingressclass_regex.go | 2 +- 21 files changed, 424 insertions(+), 188 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 { - return err + 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{} } - switch len(ingressObjList.Items) { - case 0: - break - case 1: - if ingressObj := ingressObjList.Items[0]; ingressObj.GetName() == ingress.Name() && ingressObj.GetNamespace() == ingress.Namespace() { - break + 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 } - fallthrough - default: - return NewIngressHostnameCollision(hostname) + + for _, tenant := range tenantList.Items { + namespaces.Insert(tenant.Status.Namespaces...) + } + case capsulev1beta1.HostnameCollisionScopeNamespace: + namespaces.Insert(ing.Namespace()) } - 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") + + 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() { + ingressList := sets.NewInt() + + switch list := ingressObjList.(type) { + case *extensionsv1beta1.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 *networkingv1.IngressList: + for index, item := range list.Items { + if namespaces.Has(item.GetNamespace()) { + ingressList.Insert(index) + } } - 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() { + 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) + } } - 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) + } } } } 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") From e2d78775be7d894744eeb8c6b837d6151e97586c Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sat, 31 Jul 2021 23:11:25 +0200 Subject: [PATCH 06/16] build(kustomize): Ingress hostname collision scope at Tenant level --- .../crd/bases/capsule.clastix.io_tenants.yaml | 47 ++++++++++++------- config/install.yaml | 47 ++++++++++++------- config/samples/capsule_v1beta1_tenant.yaml | 18 +++---- 3 files changed, 70 insertions(+), 42 deletions(-) diff --git a/config/crd/bases/capsule.clastix.io_tenants.yaml b/config/crd/bases/capsule.clastix.io_tenants.yaml index 02673e29..08d2a338 100644 --- a/config/crd/bases/capsule.clastix.io_tenants.yaml +++ b/config/crd/bases/capsule.clastix.io_tenants.yaml @@ -663,24 +663,37 @@ spec: - IfNotPresent type: string type: array - ingressClasses: - description: 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. + ingressOptions: + description: Specifies options for the Ingress resources, such as allowed hostnames and IngressClass. Optional. properties: - allowed: - items: - type: string - type: array - allowedRegex: - type: string - type: object - ingressHostnames: - description: 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. - properties: - allowed: - items: - type: string - type: array - allowedRegex: + allowedClasses: + description: 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. + properties: + allowed: + items: + type: string + type: array + allowedRegex: + type: string + type: object + allowedHostnames: + description: 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. + properties: + allowed: + items: + type: string + type: array + allowedRegex: + type: string + type: object + hostnameCollisionScope: + default: Disabled + description: "Defines the scope of hostname collision check performed when Tenant Owners create Ingress with allowed hostnames. \n - Cluster: disallow the creation of an Ingress if the pair hostname and path is already used across the Namespaces managed by Capsule. \n - Tenant: disallow the creation of an Ingress if the pair hostname and path is already used across the Namespaces of the Tenant. \n - Namespace: disallow the creation of an Ingress if the pair hostname and path is already used in the Ingress Namespace. \n Optional." + enum: + - Cluster + - Tenant + - Namespace + - Disabled type: string type: object limitRanges: diff --git a/config/install.yaml b/config/install.yaml index cfe6780f..dd939bb5 100644 --- a/config/install.yaml +++ b/config/install.yaml @@ -742,24 +742,37 @@ spec: - IfNotPresent type: string type: array - ingressClasses: - description: 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. + ingressOptions: + description: Specifies options for the Ingress resources, such as allowed hostnames and IngressClass. Optional. properties: - allowed: - items: - type: string - type: array - allowedRegex: - type: string - type: object - ingressHostnames: - description: 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. - properties: - allowed: - items: - type: string - type: array - allowedRegex: + allowedClasses: + description: 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. + properties: + allowed: + items: + type: string + type: array + allowedRegex: + type: string + type: object + allowedHostnames: + description: 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. + properties: + allowed: + items: + type: string + type: array + allowedRegex: + type: string + type: object + hostnameCollisionScope: + default: Disabled + description: "Defines the scope of hostname collision check performed when Tenant Owners create Ingress with allowed hostnames. \n - Cluster: disallow the creation of an Ingress if the pair hostname and path is already used across the Namespaces managed by Capsule. \n - Tenant: disallow the creation of an Ingress if the pair hostname and path is already used across the Namespaces of the Tenant. \n - Namespace: disallow the creation of an Ingress if the pair hostname and path is already used in the Ingress Namespace. \n Optional." + enum: + - Cluster + - Tenant + - Namespace + - Disabled type: string type: object limitRanges: diff --git a/config/samples/capsule_v1beta1_tenant.yaml b/config/samples/capsule_v1beta1_tenant.yaml index f13af856..77fbe24a 100644 --- a/config/samples/capsule_v1beta1_tenant.yaml +++ b/config/samples/capsule_v1beta1_tenant.yaml @@ -31,14 +31,16 @@ spec: - "10.96.42.42" imagePullPolicies: - Always - ingressClasses: - allowed: - - default - allowedRegex: ^\w+-lb$ - ingressHostnames: - allowed: - - gas.acmecorp.com - allowedRegex: ^.*acmecorp.com$ + ingressOptions: + hostnameCollisionScope: Cluster + allowedClasses: + allowed: + - default + allowedRegex: ^\w+-lb$ + allowedHostnames: + allowed: + - gas.acmecorp.com + allowedRegex: ^.*acmecorp.com$ limitRanges: items: - From d47db6696af283216fe020cbd6157e080842972f Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sat, 31 Jul 2021 23:12:34 +0200 Subject: [PATCH 07/16] build(helm): Ingress hostname collision scope at Tenant level --- charts/capsule/crds/tenant-crd.yaml | 47 ++++++++++++++++++----------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/charts/capsule/crds/tenant-crd.yaml b/charts/capsule/crds/tenant-crd.yaml index 45168b26..db7c63a0 100644 --- a/charts/capsule/crds/tenant-crd.yaml +++ b/charts/capsule/crds/tenant-crd.yaml @@ -663,24 +663,37 @@ spec: - IfNotPresent type: string type: array - ingressClasses: - description: 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. + ingressOptions: + description: Specifies options for the Ingress resources, such as allowed hostnames and IngressClass. Optional. properties: - allowed: - items: - type: string - type: array - allowedRegex: - type: string - type: object - ingressHostnames: - description: 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. - properties: - allowed: - items: - type: string - type: array - allowedRegex: + allowedClasses: + description: 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. + properties: + allowed: + items: + type: string + type: array + allowedRegex: + type: string + type: object + allowedHostnames: + description: 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. + properties: + allowed: + items: + type: string + type: array + allowedRegex: + type: string + type: object + hostnameCollisionScope: + default: Disabled + description: "Defines the scope of hostname collision check performed when Tenant Owners create Ingress with allowed hostnames. \n - Cluster: disallow the creation of an Ingress if the pair hostname and path is already used across the Namespaces managed by Capsule. \n - Tenant: disallow the creation of an Ingress if the pair hostname and path is already used across the Namespaces of the Tenant. \n - Namespace: disallow the creation of an Ingress if the pair hostname and path is already used in the Ingress Namespace. \n Optional." + enum: + - Cluster + - Tenant + - Namespace + - Disabled type: string type: object limitRanges: From 56dcfcd0dd5610be15029feea15ece677681f63e Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sat, 31 Jul 2021 23:17:38 +0200 Subject: [PATCH 08/16] refactor: hostname collision is now managed at Tenant level --- api/v1alpha1/capsuleconfiguration_types.go | 9 --- main.go | 4 +- pkg/configuration/client.go | 16 +--- pkg/configuration/configuration.go | 2 - pkg/indexer/indexer.go | 5 +- pkg/indexer/tenant/hostnames.go | 31 -------- pkg/webhook/ingress/validate_collision.go | 4 - pkg/webhook/tenant/hostnames_collision.go | 86 ---------------------- 8 files changed, 7 insertions(+), 150 deletions(-) delete mode 100644 pkg/indexer/tenant/hostnames.go delete mode 100644 pkg/webhook/tenant/hostnames_collision.go diff --git a/api/v1alpha1/capsuleconfiguration_types.go b/api/v1alpha1/capsuleconfiguration_types.go index 7322d2a3..1e9fbacc 100644 --- a/api/v1alpha1/capsuleconfiguration_types.go +++ b/api/v1alpha1/capsuleconfiguration_types.go @@ -18,15 +18,6 @@ type CapsuleConfigurationSpec struct { ForceTenantPrefix bool `json:"forceTenantPrefix,omitempty"` // Disallow creation of namespaces, whose name matches this regexp ProtectedNamespaceRegexpString string `json:"protectedNamespaceRegex,omitempty"` - // When defining the exact match for allowed Ingress hostnames at Tenant level, a collision is not allowed. - // Toggling this, Capsule will not check if a hostname collision is in place, allowing the creation of - // two or more Tenant resources although sharing the same allowed hostname(s). - // - // The JSON path of the resource is: /spec/ingressHostnames/allowed - AllowTenantIngressHostnamesCollision bool `json:"allowTenantIngressHostnamesCollision,omitempty"` - // Allow the collision of Ingress resource hostnames across all the Tenants. - // +kubebuilder:default=true - AllowIngressHostnameCollision bool `json:"allowIngressHostnameCollision,omitempty"` } // +kubebuilder:object:root=true diff --git a/main.go b/main.go index 059aeab2..d58b23ab 100644 --- a/main.go +++ b/main.go @@ -154,7 +154,7 @@ func main() { route.PVC(pvc.Handler()), route.Service(service.Handler()), route.NetworkPolicy(utils.InCapsuleGroups(cfg, networkpolicy.Handler())), - route.Tenant(tenant.NameHandler(), tenant.IngressClassRegexHandler(), tenant.StorageClassRegexHandler(), tenant.ContainerRegistryRegexHandler(), tenant.HostnameRegexHandler(), tenant.HostnamesCollisionHandler(cfg), tenant.FreezedEmitter()), + route.Tenant(tenant.NameHandler(), tenant.IngressClassRegexHandler(), tenant.StorageClassRegexHandler(), tenant.ContainerRegistryRegexHandler(), tenant.HostnameRegexHandler(), tenant.FreezedEmitter()), route.OwnerReference(utils.InCapsuleGroups(cfg, ownerreference.Handler(cfg))), route.Cordoning(tenant.CordoningHandler(cfg)), ) @@ -224,7 +224,7 @@ func main() { ctx := ctrl.SetupSignalHandler() - if err = indexer.AddToManager(manager, ctx); err != nil { + if err = indexer.AddToManager(ctx, manager); err != nil { setupLog.Error(err, "unable to setup indexers") os.Exit(1) } diff --git a/pkg/configuration/client.go b/pkg/configuration/client.go index 7e4b099f..20838a11 100644 --- a/pkg/configuration/client.go +++ b/pkg/configuration/client.go @@ -29,11 +29,9 @@ func NewCapsuleConfiguration(client client.Client, name string) Configuration { if machineryerr.IsNotFound(err) { return &capsulev1alpha1.CapsuleConfiguration{ Spec: capsulev1alpha1.CapsuleConfigurationSpec{ - UserGroups: []string{"capsule.clastix.io"}, - ForceTenantPrefix: false, - ProtectedNamespaceRegexpString: "", - AllowTenantIngressHostnamesCollision: false, - AllowIngressHostnameCollision: true, + UserGroups: []string{"capsule.clastix.io"}, + ForceTenantPrefix: false, + ProtectedNamespaceRegexpString: "", }, } } @@ -44,14 +42,6 @@ func NewCapsuleConfiguration(client client.Client, name string) Configuration { }} } -func (c capsuleConfiguration) AllowIngressHostnameCollision() bool { - return c.retrievalFn().Spec.AllowIngressHostnameCollision -} - -func (c capsuleConfiguration) AllowTenantIngressHostnamesCollision() bool { - return c.retrievalFn().Spec.AllowTenantIngressHostnamesCollision -} - func (c capsuleConfiguration) ProtectedNamespaceRegexp() (*regexp.Regexp, error) { expr := c.retrievalFn().Spec.ProtectedNamespaceRegexpString if len(expr) == 0 { diff --git a/pkg/configuration/configuration.go b/pkg/configuration/configuration.go index e0dafdd3..a481e8e2 100644 --- a/pkg/configuration/configuration.go +++ b/pkg/configuration/configuration.go @@ -8,8 +8,6 @@ import ( ) type Configuration interface { - AllowIngressHostnameCollision() bool - AllowTenantIngressHostnamesCollision() bool ProtectedNamespaceRegexp() (*regexp.Regexp, error) ForceTenantPrefix() bool UserGroups() []string diff --git a/pkg/indexer/indexer.go b/pkg/indexer/indexer.go index 41458e66..acd9f651 100644 --- a/pkg/indexer/indexer.go +++ b/pkg/indexer/indexer.go @@ -24,9 +24,8 @@ type CustomIndexer interface { Func() client.IndexerFunc } -func AddToManager(m manager.Manager, ctx context.Context) error { +func AddToManager(ctx context.Context, mgr manager.Manager) error { indexers := append([]CustomIndexer{}, - tenant.IngressHostnames{}, tenant.NamespacesReference{}, tenant.OwnerReference{}, namespace.OwnerReference{}, @@ -44,7 +43,7 @@ func AddToManager(m manager.Manager, ctx context.Context) error { } for _, f := range indexers { - if err := m.GetFieldIndexer().IndexField(ctx, f.Object(), f.Field(), f.Func()); err != nil { + if err := mgr.GetFieldIndexer().IndexField(ctx, f.Object(), f.Field(), f.Func()); err != nil { return err } } diff --git a/pkg/indexer/tenant/hostnames.go b/pkg/indexer/tenant/hostnames.go deleted file mode 100644 index 2d88a5c9..00000000 --- a/pkg/indexer/tenant/hostnames.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package tenant - -import ( - "sigs.k8s.io/controller-runtime/pkg/client" - - capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" -) - -type IngressHostnames struct { -} - -func (IngressHostnames) Object() client.Object { - return &capsulev1beta1.Tenant{} -} - -func (IngressHostnames) Field() string { - return ".spec.ingressHostnames" -} - -func (IngressHostnames) Func() client.IndexerFunc { - return func(object client.Object) (out []string) { - tenant := object.(*capsulev1beta1.Tenant) - if tenant.Spec.IngressOptions.AllowedHostnames != nil { - out = append(out, tenant.Spec.IngressOptions.AllowedHostnames.Exact...) - } - return - } -} diff --git a/pkg/webhook/ingress/validate_collision.go b/pkg/webhook/ingress/validate_collision.go index f7f6c4a6..92c6a5c8 100644 --- a/pkg/webhook/ingress/validate_collision.go +++ b/pkg/webhook/ingress/validate_collision.go @@ -111,10 +111,6 @@ func (r *collision) OnDelete(client.Client, *admission.Decoder, record.EventReco } func (r *collision) validateCollision(ctx context.Context, clt client.Client, ing Ingress, scope capsulev1beta1.HostnameCollisionScope) error { - if r.configuration.AllowIngressHostnameCollision() { - return nil - } - for hostname, paths := range ing.HostnamePathsPairs() { for path := range paths { var ingressObjList client.ObjectList diff --git a/pkg/webhook/tenant/hostnames_collision.go b/pkg/webhook/tenant/hostnames_collision.go deleted file mode 100644 index db7239e2..00000000 --- a/pkg/webhook/tenant/hostnames_collision.go +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package tenant - -import ( - "context" - "fmt" - "net/http" - - "k8s.io/apimachinery/pkg/fields" - "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/configuration" - capsulewebhook "github.com/clastix/capsule/pkg/webhook" - "github.com/clastix/capsule/pkg/webhook/utils" -) - -type hostnamesCollisionHandler struct { - configuration configuration.Configuration -} - -func HostnamesCollisionHandler(configuration configuration.Configuration) capsulewebhook.Handler { - return &hostnamesCollisionHandler{configuration: configuration} -} - -func (h *hostnamesCollisionHandler) validateTenant(ctx context.Context, req admission.Request, clt client.Client, decoder *admission.Decoder) *admission.Response { - tenant := &capsulev1beta1.Tenant{} - if err := decoder.Decode(req, tenant); err != nil { - return utils.ErroredResponse(err) - } - - 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{ - Selector: fields.OneTermEqualSelector(".spec.ingressHostnames", h), - }); err != nil { - response := admission.Errored(http.StatusInternalServerError, fmt.Errorf("cannot retrieve Tenant list using .spec.ingressHostnames field selector: %w", err)) - - return &response - } - switch { - case len(tntList.Items) == 1 && tntList.Items[0].GetName() == tenant.GetName(): - continue - case len(tntList.Items) > 0: - response := admission.Denied(fmt.Sprintf("the allowed hostname %s is already used by the Tenant %s, cannot proceed", h, tntList.Items[0].GetName())) - - return &response - default: - continue - } - } - } - - return nil -} - -func (h *hostnamesCollisionHandler) OnCreate(client client.Client, decoder *admission.Decoder, _ record.EventRecorder) capsulewebhook.Func { - return func(ctx context.Context, req admission.Request) *admission.Response { - if response := h.validateTenant(ctx, req, client, decoder); response != nil { - return response - } - - return nil - } -} - -func (h *hostnamesCollisionHandler) OnDelete(client.Client, *admission.Decoder, record.EventRecorder) capsulewebhook.Func { - return func(context.Context, admission.Request) *admission.Response { - return nil - } -} - -func (h *hostnamesCollisionHandler) OnUpdate(client client.Client, decoder *admission.Decoder, _ record.EventRecorder) capsulewebhook.Func { - return func(ctx context.Context, req admission.Request) *admission.Response { - if response := h.validateTenant(ctx, req, client, decoder); response != nil { - return response - } - - return nil - } -} From 592eb55c164b2273d8f3d41e02d414f59263aa4b Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sun, 1 Aug 2021 15:08:50 +0200 Subject: [PATCH 09/16] test(e2e): scoped Ingress hostname and path collision --- e2e/ingress_class_extensions_test.go | 16 +- e2e/ingress_class_networking_test.go | 16 +- ...ngress_hostnames_allowed_collision_test.go | 172 -------------- ..._hostnames_collision_cluster_scope_test.go | 221 ++++++++++++++++++ ...gress_hostnames_collision_disabled_test.go | 204 ++++++++++++++++ ...ostnames_collision_namespace_scope_test.go | 206 ++++++++++++++++ ...s_hostnames_collision_tenant_scope_test.go | 192 +++++++++++++++ ...ingress_hostnames_denied_collision_test.go | 125 ---------- e2e/ingress_hostnames_test.go | 12 +- ...ngress_hostnames_collision_allowed_test.go | 130 ----------- ...ngress_hostnames_collision_blocked_test.go | 83 ------- e2e/tenant_resources_test.go | 4 +- 12 files changed, 851 insertions(+), 530 deletions(-) delete mode 100644 e2e/ingress_hostnames_allowed_collision_test.go create mode 100644 e2e/ingress_hostnames_collision_cluster_scope_test.go create mode 100644 e2e/ingress_hostnames_collision_disabled_test.go create mode 100644 e2e/ingress_hostnames_collision_namespace_scope_test.go create mode 100644 e2e/ingress_hostnames_collision_tenant_scope_test.go delete mode 100644 e2e/ingress_hostnames_denied_collision_test.go delete mode 100644 e2e/tenant_ingress_hostnames_collision_allowed_test.go delete mode 100644 e2e/tenant_ingress_hostnames_collision_blocked_test.go diff --git a/e2e/ingress_class_extensions_test.go b/e2e/ingress_class_extensions_test.go index 7e434fb8..bd756073 100644 --- a/e2e/ingress_class_extensions_test.go +++ b/e2e/ingress_class_extensions_test.go @@ -30,12 +30,14 @@ var _ = Describe("when Tenant handles Ingress classes with extensions/v1beta1", Kind: "User", }, }, - IngressClasses: &capsulev1beta1.AllowedListSpec{ - Exact: []string{ - "nginx", - "haproxy", + IngressOptions: capsulev1beta1.IngressOptions{ + AllowedClasses: &capsulev1beta1.AllowedListSpec{ + Exact: []string{ + "nginx", + "haproxy", + }, + Regex: "^oil-.*$", }, - Regex: "^oil-.*$", }, }, } @@ -133,7 +135,7 @@ var _ = Describe("when Tenant handles Ingress classes with extensions/v1beta1", NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName())) - for _, c := range tnt.Spec.IngressClasses.Exact { + for _, c := range tnt.Spec.IngressOptions.AllowedClasses.Exact { Eventually(func() (err error) { i := &extensionsv1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -171,7 +173,7 @@ var _ = Describe("when Tenant handles Ingress classes with extensions/v1beta1", NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName())) - for _, c := range tnt.Spec.IngressClasses.Exact { + for _, c := range tnt.Spec.IngressOptions.AllowedClasses.Exact { Eventually(func() (err error) { i := &extensionsv1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ diff --git a/e2e/ingress_class_networking_test.go b/e2e/ingress_class_networking_test.go index 2d73225b..919e851a 100644 --- a/e2e/ingress_class_networking_test.go +++ b/e2e/ingress_class_networking_test.go @@ -29,12 +29,14 @@ var _ = Describe("when Tenant handles Ingress classes with networking.k8s.io/v1" Kind: "User", }, }, - IngressClasses: &capsulev1beta1.AllowedListSpec{ - Exact: []string{ - "nginx", - "haproxy", + IngressOptions: capsulev1beta1.IngressOptions{ + AllowedClasses: &capsulev1beta1.AllowedListSpec{ + Exact: []string{ + "nginx", + "haproxy", + }, + Regex: "^oil-.*$", }, - Regex: "^oil-.*$", }, }, } @@ -144,7 +146,7 @@ var _ = Describe("when Tenant handles Ingress classes with networking.k8s.io/v1" NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName())) - for _, c := range tnt.Spec.IngressClasses.Exact { + for _, c := range tnt.Spec.IngressOptions.AllowedClasses.Exact { Eventually(func() (err error) { i := &networkingv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -183,7 +185,7 @@ var _ = Describe("when Tenant handles Ingress classes with networking.k8s.io/v1" NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName())) - for _, c := range tnt.Spec.IngressClasses.Exact { + for _, c := range tnt.Spec.IngressOptions.AllowedClasses.Exact { Eventually(func() (err error) { i := &networkingv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ diff --git a/e2e/ingress_hostnames_allowed_collision_test.go b/e2e/ingress_hostnames_allowed_collision_test.go deleted file mode 100644 index 4881d503..00000000 --- a/e2e/ingress_hostnames_allowed_collision_test.go +++ /dev/null @@ -1,172 +0,0 @@ -//+build e2e - -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package e2e - -import ( - "context" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - extensionsv1beta1 "k8s.io/api/extensions/v1beta1" - networkingv1 "k8s.io/api/networking/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - capsulev1alpha1 "github.com/clastix/capsule/api/v1alpha1" - - capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" -) - -var _ = Describe("when handling Ingress hostnames collision", func() { - tnt := &capsulev1beta1.Tenant{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress-hostnames-allowed-collision", - }, - Spec: capsulev1beta1.TenantSpec{ - Owners: capsulev1beta1.OwnerListSpec{ - { - Name: "ingress-allowed", - Kind: "User", - }, - }, - }, - } - - // scaffold a basic networking.k8s.io Ingress with name and host - networkingIngress := func(name, hostname string) *networkingv1.Ingress { - return &networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Spec: networkingv1.IngressSpec{ - Rules: []networkingv1.IngressRule{ - { - Host: hostname, - }, - }, - }, - } - } - // scaffold a basic extensions Ingress with name and host - extensionsIngress := func(name, hostname string) *extensionsv1beta1.Ingress { - return &extensionsv1beta1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Spec: extensionsv1beta1.IngressSpec{ - Rules: []extensionsv1beta1.IngressRule{ - { - Host: hostname, - }, - }, - }, - } - - } - - JustBeforeEach(func() { - EventuallyCreation(func() error { - tnt.ResourceVersion = "" - return k8sClient.Create(context.TODO(), tnt) - }).Should(Succeed()) - - ModifyCapsuleConfigurationOpts(func(configuration *capsulev1alpha1.CapsuleConfiguration) { - configuration.Spec.AllowIngressHostnameCollision = true - }) - }) - - JustAfterEach(func() { - Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) - - ModifyCapsuleConfigurationOpts(func(configuration *capsulev1alpha1.CapsuleConfiguration) { - configuration.Spec.AllowIngressHostnameCollision = false - }) - }) - - It("should not allow creating several Ingress with same hostname", func() { - ModifyCapsuleConfigurationOpts(func(configuration *capsulev1alpha1.CapsuleConfiguration) { - configuration.Spec.AllowIngressHostnameCollision = false - }) - - maj, min, _ := GetKubernetesSemVer() - - ns := NewNamespace("denied-collision") - cs := ownerClient(tnt.Spec.Owners[0]) - - NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) - TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName())) - - if maj == 1 && min > 18 { - By("testing networking.k8s.io", func() { - EventuallyCreation(func() (err error) { - obj := networkingIngress("networking-1", "kubernetes.io") - _, err = cs.NetworkingV1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }).Should(Succeed()) - EventuallyCreation(func() (err error) { - obj := networkingIngress("networking-2", "kubernetes.io") - _, err = cs.NetworkingV1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }).ShouldNot(Succeed()) - }) - } - - if maj == 1 && min < 22 { - By("testing extensions", func() { - EventuallyCreation(func() (err error) { - obj := extensionsIngress("extensions-1", "cncf.io") - _, err = cs.ExtensionsV1beta1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }).Should(Succeed()) - EventuallyCreation(func() (err error) { - obj := extensionsIngress("extensions-2", "cncf.io") - _, err = cs.ExtensionsV1beta1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }).ShouldNot(Succeed()) - }) - } - - }) - - It("should allow creating several Ingress with same hostname", func() { - maj, min, _ := GetKubernetesSemVer() - - ns := NewNamespace("allowed-collision") - cs := ownerClient(tnt.Spec.Owners[0]) - - NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) - TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName())) - - if maj == 1 && min > 18 { - By("testing networking.k8s.io", func() { - EventuallyCreation(func() (err error) { - obj := networkingIngress("networking-1", "kubernetes.io") - _, err = cs.NetworkingV1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }).Should(Succeed()) - EventuallyCreation(func() (err error) { - obj := networkingIngress("networking-2", "kubernetes.io") - _, err = cs.NetworkingV1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }).Should(Succeed()) - }) - } - - if maj == 1 && min < 22 { - By("testing extensions", func() { - EventuallyCreation(func() (err error) { - obj := extensionsIngress("extensions-1", "cncf.io") - _, err = cs.ExtensionsV1beta1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }).Should(Succeed()) - EventuallyCreation(func() (err error) { - obj := extensionsIngress("extensions-2", "cncf.io") - _, err = cs.ExtensionsV1beta1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }).Should(Succeed()) - }) - } - }) -}) diff --git a/e2e/ingress_hostnames_collision_cluster_scope_test.go b/e2e/ingress_hostnames_collision_cluster_scope_test.go new file mode 100644 index 00000000..ed0345a8 --- /dev/null +++ b/e2e/ingress_hostnames_collision_cluster_scope_test.go @@ -0,0 +1,221 @@ +//+build e2e + +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" +) + +var _ = Describe("when handling Cluster scoped Ingress hostnames collision", func() { + tnt1 := &capsulev1beta1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hostnames-collision-cluster-one", + }, + Spec: capsulev1beta1.TenantSpec{ + Owners: capsulev1beta1.OwnerListSpec{ + { + Name: "ingress-tenant-one", + Kind: "User", + }, + }, + IngressOptions: capsulev1beta1.IngressOptions{ + HostnameCollisionScope: capsulev1beta1.HostnameCollisionScopeCluster, + }, + }, + } + tnt2 := &capsulev1beta1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hostnames-collision-cluster-two", + }, + Spec: capsulev1beta1.TenantSpec{ + Owners: capsulev1beta1.OwnerListSpec{ + { + Name: "ingress-tenant-two", + Kind: "User", + }, + }, + IngressOptions: capsulev1beta1.IngressOptions{ + HostnameCollisionScope: capsulev1beta1.HostnameCollisionScopeCluster, + }, + }, + } + // scaffold a basic networking.k8s.io Ingress with name and host + networkingIngress := func(name, hostname, path string) *networkingv1.Ingress { + return &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: hostname, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: path, + PathType: func(v networkingv1.PathType) *networkingv1.PathType { + return &v + }(networkingv1.PathTypeExact), + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "example", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + } + // scaffold a basic extensions Ingress with name and host + extensionsIngress := func(name, hostname, path string) *extensionsv1beta1.Ingress { + return &extensionsv1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: hostname, + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: path, + PathType: func(v extensionsv1beta1.PathType) *extensionsv1beta1.PathType { + return &v + }(extensionsv1beta1.PathTypeExact), + Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "example", + ServicePort: intstr.FromInt(8080), + }, + }, + }, + }, + }, + }, + }, + }, + } + + } + + JustBeforeEach(func() { + EventuallyCreation(func() error { + tnt1.ResourceVersion = "" + + return k8sClient.Create(context.TODO(), tnt1) + }).Should(Succeed()) + + EventuallyCreation(func() error { + tnt2.ResourceVersion = "" + + return k8sClient.Create(context.TODO(), tnt2) + }).Should(Succeed()) + }) + + JustAfterEach(func() { + Expect(k8sClient.Delete(context.TODO(), tnt1)).Should(Succeed()) + + Expect(k8sClient.Delete(context.TODO(), tnt2)).Should(Succeed()) + }) + + It("should ensure Cluster scope for Ingress hostname and path collision", func() { + maj, min, _ := GetKubernetesSemVer() + + ns1 := NewNamespace("tenant-one-ns") + + cs1 := ownerClient(tnt1.Spec.Owners[0]) + + NamespaceCreation(ns1, tnt1.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + + TenantNamespaceList(tnt1, defaultTimeoutInterval).Should(ContainElement(ns1.GetName())) + + ns2 := NewNamespace("tenant-two-ns") + + cs2 := ownerClient(tnt2.Spec.Owners[0]) + + NamespaceCreation(ns2, tnt2.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + + TenantNamespaceList(tnt2, defaultTimeoutInterval).Should(ContainElement(ns2.GetName())) + + if maj == 1 && min > 18 { + By("testing networking.k8s.io", func() { + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-1", "kubernetes.io", "/path") + + _, err = cs1.NetworkingV1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // Creating a second Ingress with same hostname but a different path in a Namespace managed by the same + // Tenant should not trigger a collision... + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-2", "kubernetes.io", "/docs") + + _, err = cs2.NetworkingV1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // ...but it happens if hostname and path collide with the first Ingress, + // although in a different Namespace + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-3", "kubernetes.io", "/path") + + _, err = cs2.NetworkingV1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).ShouldNot(Succeed()) + }) + } + + if maj == 1 && min < 22 { + By("testing extensions", func() { + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-1", "cncf.io", "/foo") + + _, err = cs1.ExtensionsV1beta1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // Creating a second Ingress with same hostname but a different path in a Namespace managed by the same + // Tenant should not trigger a collision... + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-2", "cncf.io", "/bar") + + _, err = cs2.ExtensionsV1beta1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // ...but it happens if hostname and path collide with the first Ingress, + // although in a different Namespace + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-3", "cncf.io", "/foo") + + _, err = cs2.ExtensionsV1beta1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).ShouldNot(Succeed()) + }) + } + }) +}) diff --git a/e2e/ingress_hostnames_collision_disabled_test.go b/e2e/ingress_hostnames_collision_disabled_test.go new file mode 100644 index 00000000..ea45a5c5 --- /dev/null +++ b/e2e/ingress_hostnames_collision_disabled_test.go @@ -0,0 +1,204 @@ +//+build e2e + +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" +) + +var _ = Describe("when disabling Ingress hostnames collision", func() { + tnt := &capsulev1beta1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hostnames-collision-disabled", + }, + Spec: capsulev1beta1.TenantSpec{ + Owners: capsulev1beta1.OwnerListSpec{ + { + Name: "ingress-disabled", + Kind: "User", + }, + }, + IngressOptions: capsulev1beta1.IngressOptions{ + HostnameCollisionScope: capsulev1beta1.HostnameCollisionScopeDisabled, + }, + }, + } + // scaffold a basic networking.k8s.io Ingress with name and host + networkingIngress := func(name, hostname, path string) *networkingv1.Ingress { + return &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: hostname, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: path, + PathType: func(v networkingv1.PathType) *networkingv1.PathType { + return &v + }(networkingv1.PathTypeExact), + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "example", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + } + // scaffold a basic extensions Ingress with name and host + extensionsIngress := func(name, hostname, path string) *extensionsv1beta1.Ingress { + return &extensionsv1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: hostname, + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: path, + PathType: func(v extensionsv1beta1.PathType) *extensionsv1beta1.PathType { + return &v + }(extensionsv1beta1.PathTypeExact), + Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "example", + ServicePort: intstr.FromInt(8080), + }, + }, + }, + }, + }, + }, + }, + }, + } + + } + + JustBeforeEach(func() { + EventuallyCreation(func() error { + tnt.ResourceVersion = "" + + return k8sClient.Create(context.TODO(), tnt) + }).Should(Succeed()) + }) + + JustAfterEach(func() { + Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) + }) + + It("should not check any kind of collision", func() { + maj, min, _ := GetKubernetesSemVer() + + ns1 := NewNamespace("namespace-collision-one") + + ns2 := NewNamespace("namespace-collision-two") + + cs := ownerClient(tnt.Spec.Owners[0]) + + NamespaceCreation(ns1, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + NamespaceCreation(ns2, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns1.GetName())) + TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns2.GetName())) + + if maj == 1 && min > 18 { + By("testing networking.k8s.io", func() { + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-1", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-2", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-3", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-4", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + }) + } + + if maj == 1 && min < 22 { + By("testing extensions", func() { + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-1", "cncf.io", "/docs") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-2", "cncf.io", "/docs") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-3", "cncf.io", "/docs") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-4", "cncf.io", "/docs") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + }) + } + }) +}) diff --git a/e2e/ingress_hostnames_collision_namespace_scope_test.go b/e2e/ingress_hostnames_collision_namespace_scope_test.go new file mode 100644 index 00000000..f764e4f6 --- /dev/null +++ b/e2e/ingress_hostnames_collision_namespace_scope_test.go @@ -0,0 +1,206 @@ +//+build e2e + +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" +) + +var _ = Describe("when handling Namespace scoped Ingress hostnames collision", func() { + tnt := &capsulev1beta1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hostnames-collision-namespace", + }, + Spec: capsulev1beta1.TenantSpec{ + Owners: capsulev1beta1.OwnerListSpec{ + { + Name: "ingress-namespace", + Kind: "User", + }, + }, + IngressOptions: capsulev1beta1.IngressOptions{ + HostnameCollisionScope: capsulev1beta1.HostnameCollisionScopeNamespace, + }, + }, + } + // scaffold a basic networking.k8s.io Ingress with name and host + networkingIngress := func(name, hostname, path string) *networkingv1.Ingress { + return &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: hostname, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: path, + PathType: func(v networkingv1.PathType) *networkingv1.PathType { + return &v + }(networkingv1.PathTypeExact), + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "example", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + } + // scaffold a basic extensions Ingress with name and host + extensionsIngress := func(name, hostname, path string) *extensionsv1beta1.Ingress { + return &extensionsv1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: hostname, + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: path, + PathType: func(v extensionsv1beta1.PathType) *extensionsv1beta1.PathType { + return &v + }(extensionsv1beta1.PathTypeExact), + Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "example", + ServicePort: intstr.FromInt(8080), + }, + }, + }, + }, + }, + }, + }, + }, + } + + } + + JustBeforeEach(func() { + EventuallyCreation(func() error { + tnt.ResourceVersion = "" + + return k8sClient.Create(context.TODO(), tnt) + }).Should(Succeed()) + }) + + JustAfterEach(func() { + Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) + }) + + It("should ensure Namespace scope for Ingress hostname and path collision", func() { + maj, min, _ := GetKubernetesSemVer() + + ns1 := NewNamespace("namespace-collision-one") + + ns2 := NewNamespace("namespace-collision-two") + + cs := ownerClient(tnt.Spec.Owners[0]) + + NamespaceCreation(ns1, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + NamespaceCreation(ns2, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns1.GetName())) + TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns2.GetName())) + + if maj == 1 && min > 18 { + By("testing networking.k8s.io", func() { + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-1", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // A same Ingress with hostname and path pair can be created in a different Namespace, + // although of the same Tenant + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-2", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // ...but a collision occurs if the same pair is created in the same Namespace + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-3", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).ShouldNot(Succeed()) + + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-4", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).ShouldNot(Succeed()) + }) + } + + if maj == 1 && min < 22 { + By("testing extensions", func() { + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-1", "cncf.io", "/docs") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // A same Ingress with hostname and path pair can be created in a different Namespace, + // although of the same Tenant + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-2", "cncf.io", "/docs") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // ...but a collision occurs if the same pair is created in the same Namespace + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-3", "cncf.io", "/docs") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).ShouldNot(Succeed()) + + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-4", "cncf.io", "/docs") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).ShouldNot(Succeed()) + }) + } + }) +}) diff --git a/e2e/ingress_hostnames_collision_tenant_scope_test.go b/e2e/ingress_hostnames_collision_tenant_scope_test.go new file mode 100644 index 00000000..106c61da --- /dev/null +++ b/e2e/ingress_hostnames_collision_tenant_scope_test.go @@ -0,0 +1,192 @@ +//+build e2e + +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" +) + +var _ = Describe("when handling Tenant scoped Ingress hostnames collision", func() { + tnt := &capsulev1beta1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hostnames-collision-tenant", + }, + Spec: capsulev1beta1.TenantSpec{ + Owners: capsulev1beta1.OwnerListSpec{ + { + Name: "ingress-tenant", + Kind: "User", + }, + }, + IngressOptions: capsulev1beta1.IngressOptions{ + HostnameCollisionScope: capsulev1beta1.HostnameCollisionScopeTenant, + }, + }, + } + // scaffold a basic networking.k8s.io Ingress with name and host + networkingIngress := func(name, hostname, path string) *networkingv1.Ingress { + return &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: hostname, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: path, + PathType: func(v networkingv1.PathType) *networkingv1.PathType { + return &v + }(networkingv1.PathTypeExact), + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "example", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + } + // scaffold a basic extensions Ingress with name and host + extensionsIngress := func(name, hostname, path string) *extensionsv1beta1.Ingress { + return &extensionsv1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: hostname, + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: path, + PathType: func(v extensionsv1beta1.PathType) *extensionsv1beta1.PathType { + return &v + }(extensionsv1beta1.PathTypeExact), + Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "example", + ServicePort: intstr.FromInt(8080), + }, + }, + }, + }, + }, + }, + }, + }, + } + + } + + JustBeforeEach(func() { + EventuallyCreation(func() error { + tnt.ResourceVersion = "" + return k8sClient.Create(context.TODO(), tnt) + }).Should(Succeed()) + }) + + JustAfterEach(func() { + Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) + }) + + + It("should ensure Tenant scope for Ingress hostname and path collision", func() { + maj, min, _ := GetKubernetesSemVer() + + ns1 := NewNamespace("cluster-collision-one") + + ns2 := NewNamespace("cluster-collision-two") + + cs := ownerClient(tnt.Spec.Owners[0]) + + NamespaceCreation(ns1, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + NamespaceCreation(ns2, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns1.GetName())) + TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns2.GetName())) + + if maj == 1 && min > 18 { + By("testing networking.k8s.io", func() { + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-1", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // Creating a second Ingress with same hostname but a different path in a Namespace managed by the same + // Tenant should not trigger a collision... + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-2", "kubernetes.io", "/docs") + + _, err = cs.NetworkingV1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // ...but it happens if hostname and path collide with the first Ingress, + // although in a different Namespace + EventuallyCreation(func() (err error) { + obj := networkingIngress("networking-3", "kubernetes.io", "/path") + + _, err = cs.NetworkingV1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).ShouldNot(Succeed()) + }) + } + + if maj == 1 && min < 22 { + By("testing extensions", func() { + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-1", "cncf.io", "/foo") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns1.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // Creating a second Ingress with same hostname but a different path in a Namespace managed by the same + // Tenant should not trigger a collision... + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-2", "cncf.io", "/bar") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).Should(Succeed()) + // ...but it happens if hostname and path collide with the first Ingress, + // although in a different Namespace + EventuallyCreation(func() (err error) { + obj := extensionsIngress("extensions-3", "cncf.io", "/foo") + + _, err = cs.ExtensionsV1beta1().Ingresses(ns2.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) + + return + }).ShouldNot(Succeed()) + }) + } + }) +}) diff --git a/e2e/ingress_hostnames_denied_collision_test.go b/e2e/ingress_hostnames_denied_collision_test.go deleted file mode 100644 index abc362b1..00000000 --- a/e2e/ingress_hostnames_denied_collision_test.go +++ /dev/null @@ -1,125 +0,0 @@ -//+build e2e - -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package e2e - -import ( - "context" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - extensionsv1beta1 "k8s.io/api/extensions/v1beta1" - networkingv1 "k8s.io/api/networking/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - capsulev1alpha1 "github.com/clastix/capsule/api/v1alpha1" - - capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" -) - -var _ = Describe("when handling Ingress hostnames collision", func() { - tnt := &capsulev1beta1.Tenant{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress-hostnames-denied-collision", - }, - Spec: capsulev1beta1.TenantSpec{ - Owners: capsulev1beta1.OwnerListSpec{ - { - Name: "ingress-denied", - Kind: "User", - }, - }, - }, - } - - // scaffold a basic networking.k8s.io Ingress with name and host - networkingIngress := func(name, hostname string) *networkingv1.Ingress { - return &networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Spec: networkingv1.IngressSpec{ - Rules: []networkingv1.IngressRule{ - { - Host: hostname, - }, - }, - }, - } - } - // scaffold a basic extensions Ingress with name and host - extensionsIngress := func(name, hostname string) *extensionsv1beta1.Ingress { - return &extensionsv1beta1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Spec: extensionsv1beta1.IngressSpec{ - Rules: []extensionsv1beta1.IngressRule{ - { - Host: hostname, - }, - }, - }, - } - - } - - JustBeforeEach(func() { - EventuallyCreation(func() error { - return k8sClient.Create(context.TODO(), tnt) - }).Should(Succeed()) - - ModifyCapsuleConfigurationOpts(func(configuration *capsulev1alpha1.CapsuleConfiguration) { - configuration.Spec.AllowIngressHostnameCollision = true - }) - }) - JustAfterEach(func() { - Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) - - ModifyCapsuleConfigurationOpts(func(configuration *capsulev1alpha1.CapsuleConfiguration) { - configuration.Spec.AllowIngressHostnameCollision = false - }) - }) - - It("should not allow creating several Ingress with same hostname", func() { - maj, min, _ := GetKubernetesSemVer() - - ns := NewNamespace("allowed-collision") - cs := ownerClient(tnt.Spec.Owners[0]) - - NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) - TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName())) - - if maj == 1 && min > 18 { - By("testing networking.k8s.io", func() { - Eventually(func() (err error) { - obj := networkingIngress("networking-1", "kubernetes.io") - _, err = cs.NetworkingV1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) - Eventually(func() (err error) { - obj := networkingIngress("networking-2", "kubernetes.io") - _, err = cs.NetworkingV1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }, defaultTimeoutInterval, defaultPollInterval).ShouldNot(Succeed()) - }) - } - - if maj == 1 && min < 22 { - By("testing extensions", func() { - Eventually(func() (err error) { - obj := extensionsIngress("extensions-1", "cncf.io") - _, err = cs.ExtensionsV1beta1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) - Eventually(func() (err error) { - obj := extensionsIngress("extensions-2", "cncf.io") - _, err = cs.ExtensionsV1beta1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) - return - }, defaultTimeoutInterval, defaultPollInterval).ShouldNot(Succeed()) - }) - } - }) -}) diff --git a/e2e/ingress_hostnames_test.go b/e2e/ingress_hostnames_test.go index 11be02b3..05c756ee 100644 --- a/e2e/ingress_hostnames_test.go +++ b/e2e/ingress_hostnames_test.go @@ -31,9 +31,11 @@ var _ = Describe("when Tenant handles Ingress hostnames", func() { Kind: "User", }, }, - IngressHostnames: &capsulev1beta1.AllowedListSpec{ - Exact: []string{"sigs.k8s.io", "operator.sdk", "domain.tld"}, - Regex: `.*\.clastix\.io`, + IngressOptions: capsulev1beta1.IngressOptions{ + AllowedHostnames: &capsulev1beta1.AllowedListSpec{ + Exact: []string{"sigs.k8s.io", "operator.sdk", "domain.tld"}, + Regex: `.*\.clastix\.io`, + }, }, }, } @@ -174,7 +176,7 @@ var _ = Describe("when Tenant handles Ingress hostnames", func() { TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName())) By("testing networking.k8s.io", func() { - for i, h := range tnt.Spec.IngressHostnames.Exact { + for i, h := range tnt.Spec.IngressOptions.AllowedHostnames.Exact { Eventually(func() (err error) { obj := networkingIngress(fmt.Sprintf("allowed-networking-%d", i), h) _, err = cs.NetworkingV1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) @@ -200,7 +202,7 @@ var _ = Describe("when Tenant handles Ingress hostnames", func() { TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName())) By("testing extensions", func() { - for i, h := range tnt.Spec.IngressHostnames.Exact { + for i, h := range tnt.Spec.IngressOptions.AllowedHostnames.Exact { Eventually(func() (err error) { obj := extensionsIngress(fmt.Sprintf("allowed-extensions-%d", i), h) _, err = cs.ExtensionsV1beta1().Ingresses(ns.GetName()).Create(context.TODO(), obj, metav1.CreateOptions{}) diff --git a/e2e/tenant_ingress_hostnames_collision_allowed_test.go b/e2e/tenant_ingress_hostnames_collision_allowed_test.go deleted file mode 100644 index e61e358c..00000000 --- a/e2e/tenant_ingress_hostnames_collision_allowed_test.go +++ /dev/null @@ -1,130 +0,0 @@ -//+build e2e - -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package e2e - -import ( - "context" - "fmt" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - capsulev1alpha1 "github.com/clastix/capsule/api/v1alpha1" - capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" -) - -var _ = Describe("when a second Tenant contains an already declared allowed Ingress hostname", func() { - tnt := &capsulev1beta1.Tenant{ - ObjectMeta: metav1.ObjectMeta{ - Name: "allowed-collision-ingress-hostnames", - }, - Spec: capsulev1beta1.TenantSpec{ - Owners: capsulev1beta1.OwnerListSpec{ - { - Name: "first-user", - Kind: "User", - }, - }, - IngressHostnames: &capsulev1beta1.AllowedListSpec{ - Exact: []string{"capsule.clastix.io", "docs.capsule.k8s", "42.clatix.io"}, - }, - }, - } - - JustBeforeEach(func() { - EventuallyCreation(func() error { - tnt.ResourceVersion = "" - return k8sClient.Create(context.TODO(), tnt) - }).Should(Succeed()) - }) - - JustAfterEach(func() { - Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) - - ModifyCapsuleConfigurationOpts(func(configuration *capsulev1alpha1.CapsuleConfiguration) { - configuration.Spec.AllowTenantIngressHostnamesCollision = false - }) - }) - - It("should block creation if contains collided Ingress hostnames", func() { - var cleanupFuncs []func() - - for i, h := range tnt.Spec.IngressHostnames.Exact { - duplicated := &capsulev1beta1.Tenant{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-%d", tnt.GetName(), i), - }, - Spec: capsulev1beta1.TenantSpec{ - Owners: capsulev1beta1.OwnerListSpec{ - { - Name: "second-user", - Kind: "User", - }, - }, - IngressHostnames: &capsulev1beta1.AllowedListSpec{ - Exact: []string{h}, - }, - }, - } - - EventuallyCreation(func() error { - return k8sClient.Create(context.TODO(), duplicated) - }).ShouldNot(Succeed()) - - cleanupFuncs = append(cleanupFuncs, func() { - duplicatedTnt := *duplicated - - _ = k8sClient.Delete(context.TODO(), &duplicatedTnt) - }) - } - - for _, fn := range cleanupFuncs { - fn() - } - }) - - It("should not block creation if contains collided Ingress hostnames", func() { - var cleanupFuncs []func() - - ModifyCapsuleConfigurationOpts(func(configuration *capsulev1alpha1.CapsuleConfiguration) { - configuration.Spec.AllowTenantIngressHostnamesCollision = true - }) - - for i, h := range tnt.Spec.IngressHostnames.Exact { - duplicated := &capsulev1beta1.Tenant{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-%d", tnt.GetName(), i), - }, - Spec: capsulev1beta1.TenantSpec{ - Owners: capsulev1beta1.OwnerListSpec{ - { - Name: "second-user", - Kind: "User", - }, - }, - IngressHostnames: &capsulev1beta1.AllowedListSpec{ - Exact: []string{h}, - }, - }, - } - - EventuallyCreation(func() error { - return k8sClient.Create(context.TODO(), duplicated) - }).Should(Succeed()) - - cleanupFuncs = append(cleanupFuncs, func() { - duplicatedTnt := *duplicated - - _ = k8sClient.Delete(context.TODO(), &duplicatedTnt) - }) - } - - for _, fn := range cleanupFuncs { - fn() - } - }) -}) diff --git a/e2e/tenant_ingress_hostnames_collision_blocked_test.go b/e2e/tenant_ingress_hostnames_collision_blocked_test.go deleted file mode 100644 index 7cb62523..00000000 --- a/e2e/tenant_ingress_hostnames_collision_blocked_test.go +++ /dev/null @@ -1,83 +0,0 @@ -//+build e2e - -// Copyright 2020-2021 Clastix Labs -// SPDX-License-Identifier: Apache-2.0 - -package e2e - -import ( - "context" - "fmt" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" -) - -var _ = Describe("when a second Tenant contains an already declared allowed Ingress hostname", func() { - tnt := &capsulev1beta1.Tenant{ - ObjectMeta: metav1.ObjectMeta{ - Name: "no-collision-ingress-hostnames", - }, - Spec: capsulev1beta1.TenantSpec{ - Owners: capsulev1beta1.OwnerListSpec{ - { - Name: "first-user", - Kind: "User", - }, - }, - IngressHostnames: &capsulev1beta1.AllowedListSpec{ - Exact: []string{"capsule.clastix.io", "docs.capsule.k8s", "42.clatix.io"}, - }, - }, - } - - JustBeforeEach(func() { - EventuallyCreation(func() error { - tnt.ResourceVersion = "" - return k8sClient.Create(context.TODO(), tnt) - }).Should(Succeed()) - }) - JustAfterEach(func() { - Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) - }) - - It("should block creation if contains collided Ingress hostnames", func() { - var cleanupFuncs []func() - - for i, h := range tnt.Spec.IngressHostnames.Exact { - duplicated := &capsulev1beta1.Tenant{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-%d", tnt.GetName(), i), - }, - Spec: capsulev1beta1.TenantSpec{ - Owners: capsulev1beta1.OwnerListSpec{ - { - Name: "second-user", - Kind: "User", - }, - }, - IngressHostnames: &capsulev1beta1.AllowedListSpec{ - Exact: []string{h}, - }, - }, - } - - EventuallyCreation(func() error { - return k8sClient.Create(context.TODO(), duplicated) - }).ShouldNot(Succeed()) - - cleanupFuncs = append(cleanupFuncs, func() { - duplicatedTenant := *duplicated - - k8sClient.Delete(context.TODO(), &duplicatedTenant) - }) - } - - for _, fn := range cleanupFuncs { - fn() - } - }) -}) diff --git a/e2e/tenant_resources_test.go b/e2e/tenant_resources_test.go index 40fb5721..d1fbbcc0 100644 --- a/e2e/tenant_resources_test.go +++ b/e2e/tenant_resources_test.go @@ -208,7 +208,9 @@ var _ = Describe("creating namespaces within a Tenant with resources", func() { n := fmt.Sprintf("capsule-%s-%d", tnt.GetName(), i) rq := &corev1.ResourceQuota{} - Expect(k8sClient.Get(context.TODO(), types.NamespacedName{Name: n, Namespace: name}, rq)).Should(Succeed()) + if err := k8sClient.Get(context.TODO(), types.NamespacedName{Name: n, Namespace: name}, rq); err != nil { + return corev1.ResourceQuotaSpec{} + } return rq.Spec }, defaultTimeoutInterval, defaultPollInterval).Should(Equal(s)) From 30c963c559b5697c9190a4469350be77047dc8bc Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sat, 31 Jul 2021 23:18:16 +0200 Subject: [PATCH 10/16] build(kustomize): hostname collision is now managed at Tenant level --- .../capsule.clastix.io_capsuleconfigurations.yaml | 9 +-------- config/install.yaml | 11 +---------- config/manager/configuration.yaml | 2 -- .../capsule_v1alpha1_capsuleconfiguration.yaml | 2 -- 4 files changed, 2 insertions(+), 22 deletions(-) diff --git a/config/crd/bases/capsule.clastix.io_capsuleconfigurations.yaml b/config/crd/bases/capsule.clastix.io_capsuleconfigurations.yaml index ce3ac2f3..bcbdb27f 100644 --- a/config/crd/bases/capsule.clastix.io_capsuleconfigurations.yaml +++ b/config/crd/bases/capsule.clastix.io_capsuleconfigurations.yaml @@ -30,15 +30,8 @@ spec: metadata: type: object spec: - description: CapsuleConfigurationSpec defines the Capsule configuration nolint:maligned + description: CapsuleConfigurationSpec defines the Capsule configuration properties: - allowIngressHostnameCollision: - default: true - description: Allow the collision of Ingress resource hostnames across all the Tenants. - type: boolean - allowTenantIngressHostnamesCollision: - description: "When defining the exact match for allowed Ingress hostnames at Tenant level, a collision is not allowed. Toggling this, Capsule will not check if a hostname collision is in place, allowing the creation of two or more Tenant resources although sharing the same allowed hostname(s). \n The JSON path of the resource is: /spec/ingressHostnames/allowed" - type: boolean forceTenantPrefix: default: false description: Enforces the Tenant owner, during Namespace creation, to name it using the selected Tenant name as prefix, separated by a dash. This is useful to avoid Namespace name collision in a public CaaS environment. diff --git a/config/install.yaml b/config/install.yaml index dd939bb5..7c0aae8c 100644 --- a/config/install.yaml +++ b/config/install.yaml @@ -35,15 +35,8 @@ spec: metadata: type: object spec: - description: CapsuleConfigurationSpec defines the Capsule configuration nolint:maligned + description: CapsuleConfigurationSpec defines the Capsule configuration properties: - allowIngressHostnameCollision: - default: true - description: Allow the collision of Ingress resource hostnames across all the Tenants. - type: boolean - allowTenantIngressHostnamesCollision: - description: "When defining the exact match for allowed Ingress hostnames at Tenant level, a collision is not allowed. Toggling this, Capsule will not check if a hostname collision is in place, allowing the creation of two or more Tenant resources although sharing the same allowed hostname(s). \n The JSON path of the resource is: /spec/ingressHostnames/allowed" - type: boolean forceTenantPrefix: default: false description: Enforces the Tenant owner, during Namespace creation, to name it using the selected Tenant name as prefix, separated by a dash. This is useful to avoid Namespace name collision in a public CaaS environment. @@ -1448,8 +1441,6 @@ metadata: name: capsule-default namespace: capsule-system spec: - allowIngressHostnameCollision: false - allowTenantIngressHostnamesCollision: false forceTenantPrefix: false protectedNamespaceRegex: "" userGroups: diff --git a/config/manager/configuration.yaml b/config/manager/configuration.yaml index 7500e83c..950a5d46 100644 --- a/config/manager/configuration.yaml +++ b/config/manager/configuration.yaml @@ -6,5 +6,3 @@ spec: userGroups: ["capsule.clastix.io"] forceTenantPrefix: false protectedNamespaceRegex: "" - allowTenantIngressHostnamesCollision: false - allowIngressHostnameCollision: false diff --git a/config/samples/capsule_v1alpha1_capsuleconfiguration.yaml b/config/samples/capsule_v1alpha1_capsuleconfiguration.yaml index abd5a719..20a480bd 100644 --- a/config/samples/capsule_v1alpha1_capsuleconfiguration.yaml +++ b/config/samples/capsule_v1alpha1_capsuleconfiguration.yaml @@ -7,5 +7,3 @@ spec: userGroups: ["capsule.clastix.io"] forceTenantPrefix: false protectedNamespaceRegex: "" - allowTenantIngressHostnamesCollision: false - allowIngressHostnameCollision: false From a2726f5c1d324f307909705dc25a931b3ab652b1 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sun, 1 Aug 2021 15:39:31 +0200 Subject: [PATCH 11/16] docs(helm): deprecating hostname collision --- charts/capsule/README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/charts/capsule/README.md b/charts/capsule/README.md index acb22cde..12c6e92f 100644 --- a/charts/capsule/README.md +++ b/charts/capsule/README.md @@ -69,8 +69,6 @@ Parameter | Description | Default `manager.options.forceTenantPrefix` | Boolean, enforces the Tenant owner, during Namespace creation, to name it using the selected Tenant name as prefix, separated by a dash | `false` `manager.options.capsuleUserGroup` | Override the Capsule user group | `capsule.clastix.io` `manager.options.protectedNamespaceRegex` | If specified, disallows creation of namespaces matching the passed regexp | `null` -`manager.options.allowIngressHostnameCollision` | Allow the Ingress hostname collision at Ingress resource level across all the Tenants | `true` -`manager.options.allowTenantIngressHostnamesCollision` | Skip the validation check at Tenant level for colliding Ingress hostnames | `false` `manager.image.repository` | Set the image repository of the controller. | `quay.io/clastix/capsule` `manager.image.tag` | Overrides the image tag whose default is the chart. `appVersion` | `null` `manager.image.pullPolicy` | Set the image pull policy. | `IfNotPresent` From 79359cd4fe83ba72745d36fba88cbc45ec77030d Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sun, 1 Aug 2021 15:39:48 +0200 Subject: [PATCH 12/16] build(helm): removing deprecated collision values --- charts/capsule/templates/configuration-default.yaml | 2 -- charts/capsule/values.yaml | 2 -- 2 files changed, 4 deletions(-) diff --git a/charts/capsule/templates/configuration-default.yaml b/charts/capsule/templates/configuration-default.yaml index 96781682..a9614954 100644 --- a/charts/capsule/templates/configuration-default.yaml +++ b/charts/capsule/templates/configuration-default.yaml @@ -9,5 +9,3 @@ spec: - {{ . }} {{- end}} protectedNamespaceRegex: {{ .Values.manager.options.protectedNamespaceRegex | quote }} - allowTenantIngressHostnamesCollision: {{ .Values.manager.options.allowTenantIngressHostnamesCollision }} - allowIngressHostnameCollision: {{ .Values.manager.options.allowIngressHostnameCollision }} diff --git a/charts/capsule/values.yaml b/charts/capsule/values.yaml index 5f146fa0..bec3be3b 100644 --- a/charts/capsule/values.yaml +++ b/charts/capsule/values.yaml @@ -21,8 +21,6 @@ manager: forceTenantPrefix: false capsuleUserGroups: ["capsule.clastix.io"] protectedNamespaceRegex: "" - allowIngressHostnameCollision: true - allowTenantIngressHostnamesCollision: false livenessProbe: httpGet: path: /healthz From 1b97ae6cca3f8ff4ae9e1aa56de4a1e7c3ea6e17 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Sat, 31 Jul 2021 23:18:22 +0200 Subject: [PATCH 13/16] build(helm): hostname collision is now managed at Tenant level --- charts/capsule/crds/capsuleconfiguration-crd.yaml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/charts/capsule/crds/capsuleconfiguration-crd.yaml b/charts/capsule/crds/capsuleconfiguration-crd.yaml index 84269b78..a36d8952 100644 --- a/charts/capsule/crds/capsuleconfiguration-crd.yaml +++ b/charts/capsule/crds/capsuleconfiguration-crd.yaml @@ -30,14 +30,8 @@ spec: spec: description: CapsuleConfigurationSpec defines the Capsule configuration properties: - allowIngressHostnameCollision: - default: true - description: Allow the collision of Ingress resource hostnames across all the Tenants. - type: boolean - allowTenantIngressHostnamesCollision: - description: "When defining the exact match for allowed Ingress hostnames at Tenant level, a collision is not allowed. Toggling this, Capsule will not check if a hostname collision is in place, allowing the creation of two or more Tenant resources although sharing the same allowed hostname(s). \n The JSON path of the resource is: /spec/ingressHostnames/allowed" - type: boolean forceTenantPrefix: + default: false description: Enforces the Tenant owner, during Namespace creation, to name it using the selected Tenant name as prefix, separated by a dash. This is useful to avoid Namespace name collision in a public CaaS environment. type: boolean protectedNamespaceRegex: From 7132f3ac419ef91179d25181517b26fab4302983 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Thu, 12 Aug 2021 10:29:56 +0200 Subject: [PATCH 14/16] docs: hostname collision is now managed at Tenant level --- docs/operator/references.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/operator/references.md b/docs/operator/references.md index ec9330ed..80774ebc 100644 --- a/docs/operator/references.md +++ b/docs/operator/references.md @@ -688,8 +688,6 @@ spec: userGroups: ["capsule.clastix.io"] forceTenantPrefix: false protectedNamespaceRegex: "" - allowTenantIngressHostnamesCollision: false - allowIngressHostnameCollision: false ``` Option | Description | Default @@ -697,8 +695,6 @@ Option | Description | Default `.spec.forceTenantPrefix` | Force the tenant name as prefix for namespaces: `-`. | `false` `.spec.userGroups` | Array of Capsule groups to which all tenant owners must belong. | `[capsule.clastix.io]` `.spec.protectedNamespaceRegex` | Disallows creation of namespaces matching the passed regexp. | `null` -`.spec.allowTenantIngressHostnamesCollision` | By default, Capsule allows Ingress hostname collision: set to `false` to enforce this policy. | `true` -`.spec.allowIngressHostnameCollision` | Toggling this, Capsule will not check if a hostname collision is in place, allowing the creation of two or more Tenant resources although sharing the same allowed hostname(s). | `false` Upon installation using Kustomize or Helm, a `default` resource will be created. The reference to this configuration is managed by the CLI flag `--configuration-name`. From 89aaa7e17bc9d74a4b03345094b6a144a5225968 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Thu, 12 Aug 2021 11:24:15 +0200 Subject: [PATCH 15/16] fix: example was wrong due to missing porting of NamespaceOptions --- config/samples/capsule_v1beta1_tenant.yaml | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/config/samples/capsule_v1beta1_tenant.yaml b/config/samples/capsule_v1beta1_tenant.yaml index 77fbe24a..5093c99b 100644 --- a/config/samples/capsule_v1beta1_tenant.yaml +++ b/config/samples/capsule_v1beta1_tenant.yaml @@ -18,9 +18,9 @@ spec: allowedRegex: ^\w+.gcr.io$ serviceOptions: additionalMetadata: - additionalAnnotations: + annotations: capsule.clastix.io/bgp: "true" - additionalLabels: + labels: capsule.clastix.io/pool: gas allowedServices: nodePort: false @@ -73,12 +73,13 @@ spec: min: storage: 1Gi type: PersistentVolumeClaim - namespaceQuota: 3 - namespacesMetadata: - additionalAnnotations: - capsule.clastix.io/backup: "false" - additionalLabels: - capsule.clastix.io/tenant: gas + namespaceOptions: + quota: 3 + additionalMetadata: + annotations: + capsule.clastix.io/backup: "false" + labels: + capsule.clastix.io/tenant: gas networkPolicies: items: - From bdf5e0b858ed984e465440b1403cedef93ff2669 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Thu, 12 Aug 2021 17:01:54 +0200 Subject: [PATCH 16/16] fix: NewIngressHostnameCollision is returning pointer for error parsing --- pkg/webhook/ingress/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/webhook/ingress/errors.go b/pkg/webhook/ingress/errors.go index ba6c37f3..6c6b18ca 100644 --- a/pkg/webhook/ingress/errors.go +++ b/pkg/webhook/ingress/errors.go @@ -41,7 +41,7 @@ func (i ingressHostnameCollision) Error() string { } func NewIngressHostnameCollision(hostname string) error { - return ingressHostnameCollision{hostname: hostname} + return &ingressHostnameCollision{hostname: hostname} } func NewIngressHostnamesNotValid(invalidHostnames []string, notMatchingHostnames []string, spec capsulev1beta1.AllowedListSpec) error {