From 9f1e7866c55c31e0dd5f67628bf213efd8807b1a Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Fri, 27 Aug 2021 10:46:54 -0300 Subject: [PATCH 1/5] Support v1beta1 and v1 ingresses Signed-off-by: Lucas Caparelli --- README.md | 2 +- controllers/cloudfront.go | 33 +-- controllers/cloudfront_test.go | 212 +++++------------- controllers/ingress_dto.go | 92 ++++++++ controllers/ingress_reconciler.go | 65 ++++++ controllers/ingress_v1_controller.go | 79 +++++++ ...oller.go => ingress_v1beta1_controller.go} | 42 +--- main.go | 30 ++- 8 files changed, 339 insertions(+), 216 deletions(-) create mode 100644 controllers/ingress_dto.go create mode 100644 controllers/ingress_reconciler.go create mode 100644 controllers/ingress_v1_controller.go rename controllers/{cdn_origin_controller.go => ingress_v1beta1_controller.go} (64%) diff --git a/README.md b/README.md index c6d8faa..2c928e3 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Currently, the controller only supports adding origins to AWS CloudFront. Other Requirements: - - Kubernetes with Ingresses on networking.k8s.io/v1beta1 (< v1.22) + - Kubernetes with Ingress support for networking.k8s.io/v1 or networking.k8s.io/v1beta1 # AWS CloudFront diff --git a/controllers/cloudfront.go b/controllers/cloudfront.go index fe34436..c930422 100644 --- a/controllers/cloudfront.go +++ b/controllers/cloudfront.go @@ -27,10 +27,11 @@ import ( "github.com/Gympass/cdn-origin-controller/internal/cloudfront" ) -func newOrigin(rules []networkingv1beta1.IngressRule, status networkingv1beta1.IngressStatus) cloudfront.Origin { - h := originHost(status) - builder := cloudfront.NewOriginBuilder(h) - patterns := pathPatterns(rules) +const prefixPathType = string(networkingv1beta1.PathTypePrefix) + +func newOrigin(dto ingressDTO) cloudfront.Origin { + builder := cloudfront.NewOriginBuilder(dto.host) + patterns := pathPatterns(dto.paths) for _, p := range patterns { builder = builder.WithBehavior(p) } @@ -38,29 +39,19 @@ func newOrigin(rules []networkingv1beta1.IngressRule, status networkingv1beta1.I return builder.Build() } -func originHost(status networkingv1beta1.IngressStatus) string { - return status.LoadBalancer.Ingress[0].Hostname -} - -func pathPatterns(rules []networkingv1beta1.IngressRule) []string { +func pathPatterns(paths []path) []string { var patterns []string - for _, r := range rules { - patterns = append(patterns, pathPatternsForRule(r)...) + for _, r := range paths { + patterns = append(patterns, pathPatternsForPath(r)...) } return patterns } -func pathPatternsForRule(rule networkingv1beta1.IngressRule) []string { - var paths []string - for _, p := range rule.HTTP.Paths { - pattern := p.Path - if *p.PathType == networkingv1beta1.PathTypePrefix { - paths = append(paths, buildPatternsForPrefix(pattern)...) - continue - } - paths = append(paths, pattern) +func pathPatternsForPath(p path) []string { + if p.pathType == prefixPathType { + return buildPatternsForPrefix(p.pathPattern) } - return paths + return []string{p.pathPattern} } // https://github.com/kubernetes-sigs/aws-load-balancer-controller/pull/1772/files#diff-ab931de004b4ee78d1a27f20f37cc9acaf851ab150094ec8baa1fdbcf5ca67f1R163-R174 diff --git a/controllers/cloudfront_test.go b/controllers/cloudfront_test.go index c2df932..3257550 100644 --- a/controllers/cloudfront_test.go +++ b/controllers/cloudfront_test.go @@ -23,7 +23,6 @@ import ( "testing" "github.com/stretchr/testify/suite" - corev1 "k8s.io/api/core/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" ) @@ -37,119 +36,67 @@ type IngressConverterSuite struct { } func (s *IngressConverterSuite) TestNewOrigin_SingleBehaviorAndRule() { - rules := []networkingv1beta1.IngressRule{ - { - IngressRuleValue: networkingv1beta1.IngressRuleValue{ - HTTP: &networkingv1beta1.HTTPIngressRuleValue{ - Paths: []networkingv1beta1.HTTPIngressPath{ - { - Path: "/", - PathType: pathTypePointer(networkingv1beta1.PathTypeExact), - }, - }, - }, + dto := ingressDTO{ + host: "origin1", + paths: []path{ + { + pathPattern: "/", + pathType: string(networkingv1beta1.PathTypeExact), }, }, } - status := networkingv1beta1.IngressStatus{ - LoadBalancer: corev1.LoadBalancerStatus{ - Ingress: []corev1.LoadBalancerIngress{ - { - Hostname: "origin1", - }, - }, - }, - } - - origin := newOrigin(rules, status) + origin := newOrigin(dto) s.Equal("origin1", origin.Host) s.Len(origin.Behaviors, 1) s.Equal("/", origin.Behaviors[0].PathPattern) } func (s *IngressConverterSuite) TestNewOrigins_MultipleBehaviorsSingleRule() { - rules := []networkingv1beta1.IngressRule{ - { - IngressRuleValue: networkingv1beta1.IngressRuleValue{ - HTTP: &networkingv1beta1.HTTPIngressRuleValue{ - Paths: []networkingv1beta1.HTTPIngressPath{ - { - Path: "/", - PathType: pathTypePointer(networkingv1beta1.PathTypeExact), - }, - { - Path: "/foo", - PathType: pathTypePointer(networkingv1beta1.PathTypeExact), - }, - }, - }, + dto := ingressDTO{ + host: "origin1", + paths: []path{ + { + pathPattern: "/", + pathType: string(networkingv1beta1.PathTypeExact), }, - }, - } - - status := networkingv1beta1.IngressStatus{ - LoadBalancer: corev1.LoadBalancerStatus{ - Ingress: []corev1.LoadBalancerIngress{ - { - Hostname: "origin1", - }, + { + pathPattern: "/foo", + pathType: string(networkingv1beta1.PathTypeExact), }, }, } - origin := newOrigin(rules, status) + origin := newOrigin(dto) s.Equal("origin1", origin.Host) s.Len(origin.Behaviors, 2) s.Equal("/", origin.Behaviors[0].PathPattern) s.Equal("/foo", origin.Behaviors[1].PathPattern) } func (s *IngressConverterSuite) TestNewOrigins_MultipleBehaviorsMultipleRules() { - rule1 := networkingv1beta1.IngressRule{ - IngressRuleValue: networkingv1beta1.IngressRuleValue{ - HTTP: &networkingv1beta1.HTTPIngressRuleValue{ - Paths: []networkingv1beta1.HTTPIngressPath{ - { - Path: "/", - PathType: pathTypePointer(networkingv1beta1.PathTypeExact), - }, - { - Path: "/foo", - PathType: pathTypePointer(networkingv1beta1.PathTypeExact), - }, - }, + dto := ingressDTO{ + host: "origin1", + paths: []path{ + { + pathPattern: "/", + pathType: string(networkingv1beta1.PathTypeExact), }, - }, - } - rule2 := networkingv1beta1.IngressRule{ - IngressRuleValue: networkingv1beta1.IngressRuleValue{ - HTTP: &networkingv1beta1.HTTPIngressRuleValue{ - Paths: []networkingv1beta1.HTTPIngressPath{ - { - Path: "/foo/bar", - PathType: pathTypePointer(networkingv1beta1.PathTypeExact), - }, - { - Path: "/bar", - PathType: pathTypePointer(networkingv1beta1.PathTypeExact), - }, - }, + { + pathPattern: "/foo", + pathType: string(networkingv1beta1.PathTypeExact), }, - }, - } - rules := []networkingv1beta1.IngressRule{rule1, rule2} - - status := networkingv1beta1.IngressStatus{ - LoadBalancer: corev1.LoadBalancerStatus{ - Ingress: []corev1.LoadBalancerIngress{ - { - Hostname: "origin1", - }, + { + pathPattern: "/foo/bar", + pathType: string(networkingv1beta1.PathTypeExact), + }, + { + pathPattern: "/bar", + pathType: string(networkingv1beta1.PathTypeExact), }, }, } - origin := newOrigin(rules, status) + origin := newOrigin(dto) s.Equal("origin1", origin.Host) s.Len(origin.Behaviors, 4) s.Equal("/", origin.Behaviors[0].PathPattern) @@ -160,32 +107,17 @@ func (s *IngressConverterSuite) TestNewOrigins_MultipleBehaviorsMultipleRules() // https://kubernetes.io/docs/concepts/services-networking/ingress/#examples func (s *IngressConverterSuite) TestNewCloudFrontOrigins_PrefixPathType_SingleSlashSpecialCase() { - rules := []networkingv1beta1.IngressRule{ - { - IngressRuleValue: networkingv1beta1.IngressRuleValue{ - HTTP: &networkingv1beta1.HTTPIngressRuleValue{ - Paths: []networkingv1beta1.HTTPIngressPath{ - { - Path: "/", - PathType: pathTypePointer(networkingv1beta1.PathTypePrefix), - }, - }, - }, + dto := ingressDTO{ + host: "origin1", + paths: []path{ + { + pathPattern: "/", + pathType: string(networkingv1beta1.PathTypePrefix), }, }, } - status := networkingv1beta1.IngressStatus{ - LoadBalancer: corev1.LoadBalancerStatus{ - Ingress: []corev1.LoadBalancerIngress{ - { - Hostname: "origin1", - }, - }, - }, - } - - origin := newOrigin(rules, status) + origin := newOrigin(dto) s.Equal("origin1", origin.Host) s.Len(origin.Behaviors, 1) s.Equal("/*", origin.Behaviors[0].PathPattern) @@ -193,32 +125,17 @@ func (s *IngressConverterSuite) TestNewCloudFrontOrigins_PrefixPathType_SingleSl // https://kubernetes.io/docs/concepts/services-networking/ingress/#examples func (s *IngressConverterSuite) TestNewCloudFrontOrigins_PrefixPathType_EndsWithSlash() { - rules := []networkingv1beta1.IngressRule{ - { - IngressRuleValue: networkingv1beta1.IngressRuleValue{ - HTTP: &networkingv1beta1.HTTPIngressRuleValue{ - Paths: []networkingv1beta1.HTTPIngressPath{ - { - Path: "/foo/", - PathType: pathTypePointer(networkingv1beta1.PathTypePrefix), - }, - }, - }, - }, - }, - } - - status := networkingv1beta1.IngressStatus{ - LoadBalancer: corev1.LoadBalancerStatus{ - Ingress: []corev1.LoadBalancerIngress{ - { - Hostname: "origin1", - }, + dto := ingressDTO{ + host: "origin1", + paths: []path{ + { + pathPattern: "/foo/", + pathType: string(networkingv1beta1.PathTypePrefix), }, }, } - origin := newOrigin(rules, status) + origin := newOrigin(dto) s.Equal("origin1", origin.Host) s.Len(origin.Behaviors, 2) s.Equal("/foo", origin.Behaviors[0].PathPattern) @@ -227,38 +144,19 @@ func (s *IngressConverterSuite) TestNewCloudFrontOrigins_PrefixPathType_EndsWith // https://kubernetes.io/docs/concepts/services-networking/ingress/#examples func (s *IngressConverterSuite) TestNewCloudFrontOrigins_PrefixPathType_DoesNotEndWithSlash() { - rules := []networkingv1beta1.IngressRule{ - { - IngressRuleValue: networkingv1beta1.IngressRuleValue{ - HTTP: &networkingv1beta1.HTTPIngressRuleValue{ - Paths: []networkingv1beta1.HTTPIngressPath{ - { - Path: "/foo", - PathType: pathTypePointer(networkingv1beta1.PathTypePrefix), - }, - }, - }, + dto := ingressDTO{ + host: "origin1", + paths: []path{ + { + pathPattern: "/foo", + pathType: string(networkingv1beta1.PathTypePrefix), }, }, } - status := networkingv1beta1.IngressStatus{ - LoadBalancer: corev1.LoadBalancerStatus{ - Ingress: []corev1.LoadBalancerIngress{ - { - Hostname: "origin1", - }, - }, - }, - } - - origin := newOrigin(rules, status) + origin := newOrigin(dto) s.Equal("origin1", origin.Host) s.Len(origin.Behaviors, 2) s.Equal("/foo", origin.Behaviors[0].PathPattern) s.Equal("/foo/*", origin.Behaviors[1].PathPattern) } - -func pathTypePointer(pt networkingv1beta1.PathType) *networkingv1beta1.PathType { - return &pt -} diff --git a/controllers/ingress_dto.go b/controllers/ingress_dto.go new file mode 100644 index 0000000..8ddcb7a --- /dev/null +++ b/controllers/ingress_dto.go @@ -0,0 +1,92 @@ +// Copyright (c) 2021 GPBR Participacoes LTDA. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of +// this software and associated documentation files (the "Software"), to deal in +// the Software without restriction, including without limitation the rights to +// use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software is furnished to do so, +// subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS +// FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR +// COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +// IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +// CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +package controllers + +import ( + "errors" + + networkingv1 "k8s.io/api/networking/v1" + networkingv1beta1 "k8s.io/api/networking/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var errUnsupportedKind = errors.New("unsupported kind") + +type path struct { + pathPattern string + pathType string +} + +type ingressDTO struct { + host string + paths []path +} + +func newIngressDTO(obj client.Object) (ingressDTO, error) { + switch obj.(type) { + case *networkingv1beta1.Ingress: + return newIngressDTOV1beta1(obj.(*networkingv1beta1.Ingress)), nil + case *networkingv1.Ingress: + return newIngressDTOV1(obj.(*networkingv1.Ingress)), nil + } + return ingressDTO{}, errUnsupportedKind +} + +func newIngressDTOV1beta1(ing *networkingv1beta1.Ingress) ingressDTO { + return ingressDTO{ + host: ing.Status.LoadBalancer.Ingress[0].Hostname, + paths: pathsV1beta1(ing.Spec.Rules), + } +} + +func pathsV1beta1(rules []networkingv1beta1.IngressRule) []path { + var paths []path + for _, rule := range rules { + for _, p := range rule.HTTP.Paths { + newPath := path{ + pathPattern: p.Path, + pathType: string(*p.PathType), + } + paths = append(paths, newPath) + } + } + return paths +} + +func newIngressDTOV1(ing *networkingv1.Ingress) ingressDTO { + return ingressDTO{ + host: ing.Status.LoadBalancer.Ingress[0].Hostname, + paths: pathsV1(ing.Spec.Rules), + } +} + +func pathsV1(rules []networkingv1.IngressRule) []path { + var paths []path + for _, rule := range rules { + for _, p := range rule.HTTP.Paths { + newPath := path{ + pathPattern: p.Path, + pathType: string(*p.PathType), + } + paths = append(paths, newPath) + } + } + return paths +} diff --git a/controllers/ingress_reconciler.go b/controllers/ingress_reconciler.go new file mode 100644 index 0000000..57f8636 --- /dev/null +++ b/controllers/ingress_reconciler.go @@ -0,0 +1,65 @@ +// Copyright (c) 2021 GPBR Participacoes LTDA. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of +// this software and associated documentation files (the "Software"), to deal in +// the Software without restriction, including without limitation the rights to +// use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software is furnished to do so, +// subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS +// FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR +// COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +// IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +// CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +package controllers + +import ( + "errors" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/Gympass/cdn-origin-controller/internal/cloudfront" +) + +const cdnIDAnnotation = "cdn-origin-controller.gympass.com/cdn.id" + +const ( + attachOriginFailedReason = "FailedToAttach" + attachOriginSuccessReason = "SuccessfullyAttached" +) + +var errNoAnnotation = errors.New(cdnIDAnnotation + " annotation not present") + +type IngressReconciler struct { + Recorder record.EventRecorder + Repo cloudfront.OriginRepository +} + +func (r *IngressReconciler) Reconcile(obj client.Object) error { + cdnID, ok := obj.GetAnnotations()[cdnIDAnnotation] + if !ok { + return errNoAnnotation + } + + dto, err := newIngressDTO(obj) + if err != nil { + return err + } + + if err := r.Repo.Save(cdnID, newOrigin(dto)); err != nil { + r.Recorder.Eventf(obj, corev1.EventTypeWarning, attachOriginFailedReason, "Unable to attach origin to CDN: saving origin: %v", err) + return fmt.Errorf("saving origin: %v", err) + } + + r.Recorder.Event(obj, corev1.EventTypeNormal, attachOriginSuccessReason, "Successfully attached origin to CDN") + return nil +} diff --git a/controllers/ingress_v1_controller.go b/controllers/ingress_v1_controller.go new file mode 100644 index 0000000..63e29b0 --- /dev/null +++ b/controllers/ingress_v1_controller.go @@ -0,0 +1,79 @@ +// Copyright (c) 2021 GPBR Participacoes LTDA. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of +// this software and associated documentation files (the "Software"), to deal in +// the Software without restriction, including without limitation the rights to +// use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software is furnished to do so, +// subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS +// FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR +// COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +// IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +// CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +package controllers + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// V1Reconciler reconciles v1 Ingress resources +type V1Reconciler struct { + client.Client + + OriginalLog logr.Logger + Scheme *runtime.Scheme + IngressReconciler IngressReconciler + + log logr.Logger +} + +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch +// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch + +//Reconcile a v1 Ingress resource +func (r *V1Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + r.log = r.OriginalLog.WithValues("Ingress", req.NamespacedName) + + ingress := &networkingv1.Ingress{} + err := r.Client.Get(ctx, req.NamespacedName, ingress) + if err != nil { + if errors.IsNotFound(err) { + r.log.Info("Ignoring not found Ingress.") + return reconcile.Result{}, nil + } + + return reconcile.Result{}, fmt.Errorf("could not fetch Ingress: %+v", err) + } + + err = r.IngressReconciler.Reconcile(ingress) + if err == errNoAnnotation { + r.log.Error(err, "Ignoring reconciliation request") + return ctrl.Result{}, nil + } + + return ctrl.Result{}, err +} + +// SetupWithManager ... +func (r *V1Reconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + WithEventFilter(&hasCdnAnnotationPredicate{}). + For(&networkingv1.Ingress{}). + Complete(r) +} diff --git a/controllers/cdn_origin_controller.go b/controllers/ingress_v1beta1_controller.go similarity index 64% rename from controllers/cdn_origin_controller.go rename to controllers/ingress_v1beta1_controller.go index 6baa9a2..7b6d5dd 100644 --- a/controllers/cdn_origin_controller.go +++ b/controllers/ingress_v1beta1_controller.go @@ -24,33 +24,21 @@ import ( "fmt" "github.com/go-logr/logr" - corev1 "k8s.io/api/core/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "github.com/Gympass/cdn-origin-controller/internal/cloudfront" -) - -const cdnIDAnnotation = "cdn-origin-controller.gympass.com/cdn.id" - -const ( - attachOriginFailedReason = "FailedToAttach" - attachOriginSuccessReason = "SuccessfullyAttached" ) -// Reconciler ... -type Reconciler struct { +// V1Reconciler reconciles v1beta1 Ingress resources +type V1beta1Reconciler struct { client.Client - OriginalLog logr.Logger - Scheme *runtime.Scheme - Recorder record.EventRecorder - Repo cloudfront.OriginRepository + OriginalLog logr.Logger + Scheme *runtime.Scheme + IngressReconciler IngressReconciler log logr.Logger } @@ -58,8 +46,8 @@ type Reconciler struct { // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch // +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch -//Reconcile ... -func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +//Reconcile a v1beta1 Ingress resource +func (r *V1beta1Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { r.log = r.OriginalLog.WithValues("Ingress", req.NamespacedName) ingress := &networkingv1beta1.Ingress{} @@ -73,23 +61,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return reconcile.Result{}, fmt.Errorf("could not fetch Ingress: %+v", err) } - cfID, ok := ingress.ObjectMeta.Annotations[cdnIDAnnotation] - if !ok { - r.log.Info(cdnIDAnnotation + " annotation not present. Ignoring reconciliation request.") + err = r.IngressReconciler.Reconcile(ingress) + if err == errNoAnnotation { + r.log.Error(err, "Ignoring reconciliation request") return ctrl.Result{}, nil } - if err := r.Repo.Save(cfID, newOrigin(ingress.Spec.Rules, ingress.Status)); err != nil { - r.Recorder.Eventf(ingress, corev1.EventTypeWarning, attachOriginFailedReason, "Unable to attach origin to CDN: saving origin: %v", err) - return ctrl.Result{}, fmt.Errorf("saving origin: %v", err) - } - - r.Recorder.Event(ingress, corev1.EventTypeNormal, attachOriginSuccessReason, "Successfully attached origin to CDN") - return ctrl.Result{}, nil + return ctrl.Result{}, err } // SetupWithManager ... -func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { +func (r *V1beta1Reconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). WithEventFilter(&hasCdnAnnotationPredicate{}). For(&networkingv1beta1.Ingress{}). diff --git a/main.go b/main.go index 8681443..1e29829 100644 --- a/main.go +++ b/main.go @@ -103,15 +103,31 @@ func main() { os.Exit(1) } - reconciler := controllers.Reconciler{ - Client: mgr.GetClient(), - OriginalLog: ctrl.Log.WithName("controllers").WithName("CDNOrigin"), - Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("cdn-origin-controller"), - Repo: cloudfront.NewOriginRepository(mustGetCloudFrontClient()), + ingressReconciler := controllers.IngressReconciler{ + Recorder: mgr.GetEventRecorderFor("cdn-origin-controller"), + Repo: cloudfront.NewOriginRepository(mustGetCloudFrontClient()), } - if err := reconciler.SetupWithManager(mgr); err != nil { + v1beta1Reconciler := controllers.V1beta1Reconciler{ + Client: mgr.GetClient(), + OriginalLog: ctrl.Log.WithName("controllers").WithName("ingressv1beta1"), + Scheme: mgr.GetScheme(), + IngressReconciler: ingressReconciler, + } + + if err := v1beta1Reconciler.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to set up reconciler") + os.Exit(1) + } + + v1Reconciler := controllers.V1Reconciler{ + Client: mgr.GetClient(), + OriginalLog: ctrl.Log.WithName("controllers").WithName("ingressv1"), + Scheme: mgr.GetScheme(), + IngressReconciler: ingressReconciler, + } + + if err := v1Reconciler.SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to set up reconciler") os.Exit(1) } From cdacce09f44387e5cf47f0b47ca1df5a8409068e Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Fri, 27 Aug 2021 16:54:05 -0300 Subject: [PATCH 2/5] Simplify type switch Signed-off-by: Lucas Caparelli --- controllers/ingress_dto.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/controllers/ingress_dto.go b/controllers/ingress_dto.go index 8ddcb7a..32b4e74 100644 --- a/controllers/ingress_dto.go +++ b/controllers/ingress_dto.go @@ -40,11 +40,11 @@ type ingressDTO struct { } func newIngressDTO(obj client.Object) (ingressDTO, error) { - switch obj.(type) { + switch obj := obj.(type) { case *networkingv1beta1.Ingress: - return newIngressDTOV1beta1(obj.(*networkingv1beta1.Ingress)), nil + return newIngressDTOV1beta1(obj), nil case *networkingv1.Ingress: - return newIngressDTOV1(obj.(*networkingv1.Ingress)), nil + return newIngressDTOV1(obj), nil } return ingressDTO{}, errUnsupportedKind } From 9fed93c8a7aefa91dbdd026e0c8563e4f3cb84cb Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Fri, 27 Aug 2021 16:58:48 -0300 Subject: [PATCH 3/5] Add comments for IngressReconciler Signed-off-by: Lucas Caparelli --- controllers/ingress_reconciler.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/controllers/ingress_reconciler.go b/controllers/ingress_reconciler.go index 57f8636..f63e78c 100644 --- a/controllers/ingress_reconciler.go +++ b/controllers/ingress_reconciler.go @@ -39,11 +39,13 @@ const ( var errNoAnnotation = errors.New(cdnIDAnnotation + " annotation not present") +// IngressReconciler reconciles Ingress resources of any version type IngressReconciler struct { Recorder record.EventRecorder Repo cloudfront.OriginRepository } +// Reconcile an Ingress resource of any version func (r *IngressReconciler) Reconcile(obj client.Object) error { cdnID, ok := obj.GetAnnotations()[cdnIDAnnotation] if !ok { From 1307b4c200686811e63531e0e32c75c33abac65b Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Fri, 27 Aug 2021 17:29:06 -0300 Subject: [PATCH 4/5] Fix comment of V1beta1Reconciler Signed-off-by: Lucas Caparelli --- controllers/ingress_v1beta1_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/ingress_v1beta1_controller.go b/controllers/ingress_v1beta1_controller.go index 7b6d5dd..d0618f8 100644 --- a/controllers/ingress_v1beta1_controller.go +++ b/controllers/ingress_v1beta1_controller.go @@ -32,7 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) -// V1Reconciler reconciles v1beta1 Ingress resources +// V1beta1Reconciler reconciles v1beta1 Ingress resources type V1beta1Reconciler struct { client.Client From c8f95ecbad88e6e3a44477b03c3e3184b903c77f Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Fri, 3 Sep 2021 10:17:22 -0300 Subject: [PATCH 5/5] Make error handling more generic By using errors.Is instead of directly comparing err value. Signed-off-by: Lucas Caparelli --- controllers/cloudfront.go | 4 ++-- controllers/ingress_v1_controller.go | 7 ++++--- controllers/ingress_v1beta1_controller.go | 7 ++++--- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/controllers/cloudfront.go b/controllers/cloudfront.go index c930422..aa76945 100644 --- a/controllers/cloudfront.go +++ b/controllers/cloudfront.go @@ -41,8 +41,8 @@ func newOrigin(dto ingressDTO) cloudfront.Origin { func pathPatterns(paths []path) []string { var patterns []string - for _, r := range paths { - patterns = append(patterns, pathPatternsForPath(r)...) + for _, p := range paths { + patterns = append(patterns, pathPatternsForPath(p)...) } return patterns } diff --git a/controllers/ingress_v1_controller.go b/controllers/ingress_v1_controller.go index 63e29b0..9b37ff0 100644 --- a/controllers/ingress_v1_controller.go +++ b/controllers/ingress_v1_controller.go @@ -21,11 +21,12 @@ package controllers import ( "context" + "errors" "fmt" "github.com/go-logr/logr" networkingv1 "k8s.io/api/networking/v1" - "k8s.io/apimachinery/pkg/api/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -53,7 +54,7 @@ func (r *V1Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re ingress := &networkingv1.Ingress{} err := r.Client.Get(ctx, req.NamespacedName, ingress) if err != nil { - if errors.IsNotFound(err) { + if k8serrors.IsNotFound(err) { r.log.Info("Ignoring not found Ingress.") return reconcile.Result{}, nil } @@ -62,7 +63,7 @@ func (r *V1Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re } err = r.IngressReconciler.Reconcile(ingress) - if err == errNoAnnotation { + if errors.Is(err, errNoAnnotation) { r.log.Error(err, "Ignoring reconciliation request") return ctrl.Result{}, nil } diff --git a/controllers/ingress_v1beta1_controller.go b/controllers/ingress_v1beta1_controller.go index d0618f8..7495eb0 100644 --- a/controllers/ingress_v1beta1_controller.go +++ b/controllers/ingress_v1beta1_controller.go @@ -21,11 +21,12 @@ package controllers import ( "context" + "errors" "fmt" "github.com/go-logr/logr" networkingv1beta1 "k8s.io/api/networking/v1beta1" - "k8s.io/apimachinery/pkg/api/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -53,7 +54,7 @@ func (r *V1beta1Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct ingress := &networkingv1beta1.Ingress{} err := r.Client.Get(ctx, req.NamespacedName, ingress) if err != nil { - if errors.IsNotFound(err) { + if k8serrors.IsNotFound(err) { r.log.Info("Ignoring not found Ingress.") return reconcile.Result{}, nil } @@ -62,7 +63,7 @@ func (r *V1beta1Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } err = r.IngressReconciler.Reconcile(ingress) - if err == errNoAnnotation { + if errors.Is(err, errNoAnnotation) { r.log.Error(err, "Ignoring reconciliation request") return ctrl.Result{}, nil }