From 41b9b045a6794f79239786c03e096609227364ce Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Thu, 29 Oct 2020 16:20:34 -0300 Subject: [PATCH] Use v1 Ingresses when available (#182) --- Makefile | 4 +- .../nexus/resource/deployment/manager.go | 9 +- .../nexus/resource/networking/ingress.go | 45 ++-- .../nexus/resource/networking/ingress_test.go | 34 +-- .../resource/networking/legacy_ingress.go | 79 +++++++ .../networking/legacy_ingress_test.go | 89 +++++++ .../nexus/resource/networking/manager.go | 120 ++++++---- .../nexus/resource/networking/manager_test.go | 219 +++++++++++++----- .../nexus/resource/persistence/manager.go | 5 +- .../nexus/resource/security/manager.go | 9 +- .../nexus/resource/security/manager_test.go | 5 +- .../nexus/resource/validation/validation.go | 13 +- .../resource/validation/validation_test.go | 11 +- controllers/nexus/server/users.go | 5 +- pkg/cluster/discovery/discovery.go | 19 ++ pkg/cluster/discovery/kubernetes.go | 14 +- pkg/cluster/discovery/kubernetes_test.go | 12 + pkg/cluster/discovery/openshift.go | 4 +- pkg/framework/fetcher.go | 20 ++ pkg/framework/fetcher_test.go | 46 +++- pkg/framework/{ => kind}/kinds.go | 2 +- pkg/test/client.go | 23 +- pkg/test/client_test.go | 41 ++++ 23 files changed, 649 insertions(+), 179 deletions(-) create mode 100644 controllers/nexus/resource/networking/legacy_ingress.go create mode 100644 controllers/nexus/resource/networking/legacy_ingress_test.go rename pkg/framework/{ => kind}/kinds.go (97%) diff --git a/Makefile b/Makefile index ac2a5788..4b648426 100644 --- a/Makefile +++ b/Makefile @@ -34,12 +34,14 @@ all: manager # Run tests ENVTEST_ASSETS_DIR=$(shell pwd)/testbin +# Needed to support k8s.io/api/networking/v1 Ingress +K8S_VERSION=1.19.0 test: generate-installer fmt vet bundle mkdir -p ${ENVTEST_ASSETS_DIR} test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/master/hack/setup-envtest.sh sed -i "s,#\!.*,#\!\/bin\/bash,g" ${ENVTEST_ASSETS_DIR}/setup-envtest.sh sed -i "/pipefail/d" ${ENVTEST_ASSETS_DIR}/setup-envtest.sh - source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out + source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; ENVTEST_K8S_VERSION=$(K8S_VERSION) fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out generate-installer: generate manifests kustomize cd config/manager && $(KUSTOMIZE) edit set image controller=$(OPERATOR_IMG) diff --git a/controllers/nexus/resource/deployment/manager.go b/controllers/nexus/resource/deployment/manager.go index 5051501f..d96fe06d 100644 --- a/controllers/nexus/resource/deployment/manager.go +++ b/controllers/nexus/resource/deployment/manager.go @@ -28,12 +28,13 @@ import ( "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/pkg/framework" + "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" ) var managedObjectsRef = map[string]resource.KubernetesResource{ - framework.DeploymentKind: &appsv1.Deployment{}, - framework.ServiceKind: &corev1.Service{}, + kind.DeploymentKind: &appsv1.Deployment{}, + kind.ServiceKind: &corev1.Service{}, } // Manager is responsible for creating deployment-related resources, fetching deployed ones and comparing them @@ -56,8 +57,8 @@ func NewManager(nexus *v1alpha1.Nexus, client client.Client) *Manager { // GetRequiredResources returns the resources initialized by the manager func (m *Manager) GetRequiredResources() ([]resource.KubernetesResource, error) { - m.log.Debug("Generating required resource", "kind", framework.DeploymentKind) - m.log.Debug("Generating required resource", "kind", framework.ServiceKind) + m.log.Debug("Generating required resource", "kind", kind.DeploymentKind) + m.log.Debug("Generating required resource", "kind", kind.ServiceKind) return []resource.KubernetesResource{newDeployment(m.nexus), newService(m.nexus)}, nil } diff --git a/controllers/nexus/resource/networking/ingress.go b/controllers/nexus/resource/networking/ingress.go index 2997e8a0..f8b3bb71 100644 --- a/controllers/nexus/resource/networking/ingress.go +++ b/controllers/nexus/resource/networking/ingress.go @@ -15,41 +15,46 @@ package networking import ( - "k8s.io/api/networking/v1beta1" - "k8s.io/apimachinery/pkg/util/intstr" + v1 "k8s.io/api/networking/v1" "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/controllers/nexus/resource/deployment" "github.com/m88i/nexus-operator/controllers/nexus/resource/meta" ) -const ( - ingressBasePath = "/" -) +const ingressBasePath = "/" + +// hack to take the address of v1.PathExactType +var pathTypeExact = v1.PathTypeExact type ingressBuilder struct { - *v1beta1.Ingress + *v1.Ingress nexus *v1alpha1.Nexus } func newIngressBuilder(nexus *v1alpha1.Nexus) *ingressBuilder { - ingress := &v1beta1.Ingress{ + ingress := &v1.Ingress{ ObjectMeta: meta.DefaultObjectMeta(nexus), - Spec: v1beta1.IngressSpec{ - Rules: []v1beta1.IngressRule{ + Spec: v1.IngressSpec{ + Rules: []v1.IngressRule{ { Host: nexus.Spec.Networking.Host, - IngressRuleValue: v1beta1.IngressRuleValue{HTTP: &v1beta1.HTTPIngressRuleValue{ - Paths: []v1beta1.HTTPIngressPath{ - { - Path: ingressBasePath, - Backend: v1beta1.IngressBackend{ - ServiceName: nexus.Name, - ServicePort: intstr.FromInt(deployment.NexusServicePort), + IngressRuleValue: v1.IngressRuleValue{ + HTTP: &v1.HTTPIngressRuleValue{ + Paths: []v1.HTTPIngressPath{ + { + PathType: &pathTypeExact, + Path: ingressBasePath, + Backend: v1.IngressBackend{ + Service: &v1.IngressServiceBackend{ + Name: nexus.Name, + Port: v1.ServiceBackendPort{Number: deployment.NexusServicePort}, + }, + }, }, }, }, - }}, + }, }, }, }, @@ -59,7 +64,7 @@ func newIngressBuilder(nexus *v1alpha1.Nexus) *ingressBuilder { } func (i *ingressBuilder) withCustomTLS() *ingressBuilder { - i.Spec.TLS = []v1beta1.IngressTLS{ + i.Spec.TLS = []v1.IngressTLS{ { Hosts: hosts(i.Spec.Rules), SecretName: i.nexus.Spec.Networking.TLS.SecretName, @@ -68,11 +73,11 @@ func (i *ingressBuilder) withCustomTLS() *ingressBuilder { return i } -func (i *ingressBuilder) build() *v1beta1.Ingress { +func (i *ingressBuilder) build() *v1.Ingress { return i.Ingress } -func hosts(rules []v1beta1.IngressRule) []string { +func hosts(rules []v1.IngressRule) []string { var hosts []string for _, rule := range rules { hosts = append(hosts, rule.Host) diff --git a/controllers/nexus/resource/networking/ingress_test.go b/controllers/nexus/resource/networking/ingress_test.go index 987270a4..4949e109 100644 --- a/controllers/nexus/resource/networking/ingress_test.go +++ b/controllers/nexus/resource/networking/ingress_test.go @@ -18,10 +18,8 @@ import ( "testing" "github.com/stretchr/testify/assert" - v1 "k8s.io/api/core/v1" - "k8s.io/api/networking/v1beta1" + v1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/controllers/nexus/resource/deployment" @@ -44,24 +42,12 @@ var ( }, }, } - - ingressService = &v1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "nexus3", - Namespace: "nexus", - }, - Spec: v1.ServiceSpec{ - Ports: []v1.ServicePort{ - {TargetPort: intstr.FromInt(deployment.NexusServicePort)}, - }, - }, - } ) func TestHosts(t *testing.T) { - ingress := &v1beta1.Ingress{ - Spec: v1beta1.IngressSpec{ - Rules: []v1beta1.IngressRule{}, + ingress := &v1.Ingress{ + Spec: v1.IngressSpec{ + Rules: []v1.IngressRule{}, }, } @@ -69,13 +55,13 @@ func TestHosts(t *testing.T) { assert.Len(t, h, 0) host := "a" - ingress.Spec.Rules = append(ingress.Spec.Rules, v1beta1.IngressRule{Host: host}) + ingress.Spec.Rules = append(ingress.Spec.Rules, v1.IngressRule{Host: host}) h = hosts(ingress.Spec.Rules) assert.Len(t, h, 1) assert.Equal(t, h[0], host) host = "b" - ingress.Spec.Rules = append(ingress.Spec.Rules, v1beta1.IngressRule{Host: host}) + ingress.Spec.Rules = append(ingress.Spec.Rules, v1.IngressRule{Host: host}) h = hosts(ingress.Spec.Rules) assert.Len(t, h, 2) assert.Equal(t, h[1], host) @@ -92,7 +78,7 @@ func TestNewIngressWithSecretName(t *testing.T) { assertIngressSecretName(t, ingress) } -func assertIngressBasic(t *testing.T, ingress *v1beta1.Ingress) { +func assertIngressBasic(t *testing.T, ingress *v1.Ingress) { assert.Equal(t, ingressNexus.Name, ingress.Name) assert.Equal(t, ingressNexus.Namespace, ingress.Namespace) @@ -110,11 +96,11 @@ func assertIngressBasic(t *testing.T, ingress *v1beta1.Ingress) { assert.Equal(t, ingressBasePath, path.Path) assert.NotNil(t, path.Backend) - assert.Equal(t, ingressService.Spec.Ports[0].TargetPort, path.Backend.ServicePort) - assert.Equal(t, ingressService.Name, path.Backend.ServiceName) + assert.Equal(t, int32(deployment.NexusServicePort), path.Backend.Service.Port.Number) + assert.Equal(t, ingressNexus.Name, path.Backend.Service.Name) } -func assertIngressSecretName(t *testing.T, ingress *v1beta1.Ingress) { +func assertIngressSecretName(t *testing.T, ingress *v1.Ingress) { assert.Len(t, ingress.Spec.TLS, 1) assert.Equal(t, ingressNexus.Spec.Networking.TLS.SecretName, ingress.Spec.TLS[0].SecretName) diff --git a/controllers/nexus/resource/networking/legacy_ingress.go b/controllers/nexus/resource/networking/legacy_ingress.go new file mode 100644 index 00000000..514e446d --- /dev/null +++ b/controllers/nexus/resource/networking/legacy_ingress.go @@ -0,0 +1,79 @@ +// Copyright 2020 Nexus Operator and/or its authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package networking + +import ( + "k8s.io/api/networking/v1beta1" + "k8s.io/apimachinery/pkg/util/intstr" + + "github.com/m88i/nexus-operator/api/v1alpha1" + "github.com/m88i/nexus-operator/controllers/nexus/resource/deployment" + "github.com/m88i/nexus-operator/controllers/nexus/resource/meta" +) + +type legacyIngressBuilder struct { + *v1beta1.Ingress + nexus *v1alpha1.Nexus +} + +func newLegacyIngressBuilder(nexus *v1alpha1.Nexus) *legacyIngressBuilder { + ingress := &v1beta1.Ingress{ + ObjectMeta: meta.DefaultObjectMeta(nexus), + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: nexus.Spec.Networking.Host, + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Path: ingressBasePath, + Backend: v1beta1.IngressBackend{ + ServiceName: nexus.Name, + ServicePort: intstr.FromInt(deployment.NexusServicePort), + }, + }, + }, + }, + }, + }, + }, + }, + } + + return &legacyIngressBuilder{Ingress: ingress, nexus: nexus} +} + +func (i *legacyIngressBuilder) withCustomTLS() *legacyIngressBuilder { + i.Spec.TLS = []v1beta1.IngressTLS{ + { + Hosts: legacyHosts(i.Spec.Rules), + SecretName: i.nexus.Spec.Networking.TLS.SecretName, + }, + } + return i +} + +func (i *legacyIngressBuilder) build() *v1beta1.Ingress { + return i.Ingress +} + +func legacyHosts(rules []v1beta1.IngressRule) []string { + var hosts []string + for _, rule := range rules { + hosts = append(hosts, rule.Host) + } + return hosts +} diff --git a/controllers/nexus/resource/networking/legacy_ingress_test.go b/controllers/nexus/resource/networking/legacy_ingress_test.go new file mode 100644 index 00000000..943b91a5 --- /dev/null +++ b/controllers/nexus/resource/networking/legacy_ingress_test.go @@ -0,0 +1,89 @@ +// Copyright 2020 Nexus Operator and/or its authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package networking + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/api/networking/v1beta1" + "k8s.io/apimachinery/pkg/util/intstr" + + "github.com/m88i/nexus-operator/controllers/nexus/resource/deployment" +) + +func TestLegacyHosts(t *testing.T) { + ingress := &v1beta1.Ingress{ + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{}, + }, + } + + h := legacyHosts(ingress.Spec.Rules) + assert.Len(t, h, 0) + + host := "a" + ingress.Spec.Rules = append(ingress.Spec.Rules, v1beta1.IngressRule{Host: host}) + h = legacyHosts(ingress.Spec.Rules) + assert.Len(t, h, 1) + assert.Equal(t, h[0], host) + + host = "b" + ingress.Spec.Rules = append(ingress.Spec.Rules, v1beta1.IngressRule{Host: host}) + h = legacyHosts(ingress.Spec.Rules) + assert.Len(t, h, 2) + assert.Equal(t, h[1], host) +} + +func TestNewLegacyIngress(t *testing.T) { + ingress := newLegacyIngressBuilder(ingressNexus).build() + assertLegacyIngressBasic(t, ingress) +} + +func TestNewLegacyIngressWithSecretName(t *testing.T) { + ingress := newLegacyIngressBuilder(ingressNexus).withCustomTLS().build() + assertLegacyIngressBasic(t, ingress) + assertLegacyIngressSecretName(t, ingress) +} + +func assertLegacyIngressBasic(t *testing.T, ingress *v1beta1.Ingress) { + assert.Equal(t, ingressNexus.Name, ingress.Name) + assert.Equal(t, ingressNexus.Namespace, ingress.Namespace) + + assert.NotNil(t, ingress.Spec) + + assert.Len(t, ingress.Spec.Rules, 1) + rule := ingress.Spec.Rules[0] + + assert.Equal(t, ingressNexus.Spec.Networking.Host, rule.Host) + assert.NotNil(t, rule.IngressRuleValue) + assert.NotNil(t, rule.IngressRuleValue.HTTP) + + assert.Len(t, rule.IngressRuleValue.HTTP.Paths, 1) + path := rule.IngressRuleValue.HTTP.Paths[0] + + assert.Equal(t, ingressBasePath, path.Path) + assert.NotNil(t, path.Backend) + assert.Equal(t, intstr.FromInt(deployment.NexusServicePort), path.Backend.ServicePort) + assert.Equal(t, ingressNexus.Name, path.Backend.ServiceName) +} + +func assertLegacyIngressSecretName(t *testing.T, ingress *v1beta1.Ingress) { + assert.Len(t, ingress.Spec.TLS, 1) + assert.Equal(t, ingressNexus.Spec.Networking.TLS.SecretName, ingress.Spec.TLS[0].SecretName) + + assert.Len(t, ingress.Spec.TLS[0].Hosts, 1) + assert.Equal(t, ingressNexus.Spec.Networking.Host, ingress.Spec.TLS[0].Hosts[0]) +} diff --git a/controllers/nexus/resource/networking/manager.go b/controllers/nexus/resource/networking/manager.go index 288202a2..718efe6e 100644 --- a/controllers/nexus/resource/networking/manager.go +++ b/controllers/nexus/resource/networking/manager.go @@ -21,34 +21,48 @@ import ( "github.com/RHsyseng/operator-utils/pkg/resource" "github.com/RHsyseng/operator-utils/pkg/resource/compare" routev1 "github.com/openshift/api/route/v1" + networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" - "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/pkg/cluster/discovery" "github.com/m88i/nexus-operator/pkg/framework" + "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" ) const ( - discOCPFailureFormat = "unable to determine if cluster is Openshift: %v" discFailureFormat = "unable to determine if %s are available: %v" // resource type, error resUnavailableFormat = "%s are not available in this cluster" // resource type ) +var ( + legacyIngressType = reflect.TypeOf(&networkingv1beta1.Ingress{}) + ingressType = reflect.TypeOf(&networkingv1.Ingress{}) +) + // Manager is responsible for creating networking resources, fetching deployed ones and comparing them // Use with zero values will result in a panic. Use the NewManager function to get a properly initialized manager type Manager struct { - nexus *v1alpha1.Nexus - client client.Client - log logger.Logger - routeAvailable, ingressAvailable, ocp bool + nexus *v1alpha1.Nexus + client client.Client + log logger.Logger + managedObjectsRef map[string]resource.KubernetesResource + + routeAvailable, ingressAvailable, legacyIngressAvailable bool } // NewManager creates a networking resources manager // It is expected that the Nexus has been previously validated. func NewManager(nexus *v1alpha1.Nexus, client client.Client) (*Manager, error) { + mgr := &Manager{ + nexus: nexus, + client: client, + log: logger.GetLoggerWithResource("networking_manager", nexus), + managedObjectsRef: make(map[string]resource.KubernetesResource), + } + routeAvailable, err := discovery.IsRouteAvailable() if err != nil { return nil, fmt.Errorf(discFailureFormat, "routes", err) @@ -59,27 +73,25 @@ func NewManager(nexus *v1alpha1.Nexus, client client.Client) (*Manager, error) { return nil, fmt.Errorf(discFailureFormat, "ingresses", err) } - ocp, err := discovery.IsOpenShift() + legacyIngressAvailable, err := discovery.IsLegacyIngressAvailable() if err != nil { - return nil, fmt.Errorf(discOCPFailureFormat, err) + return nil, fmt.Errorf(discFailureFormat, "v1beta1 ingresses", err) } - return &Manager{ - nexus: nexus, - client: client, - routeAvailable: routeAvailable, - ingressAvailable: ingressAvailable, - ocp: ocp, - log: logger.GetLoggerWithResource("networking_manager", nexus), - }, nil -} + if ingressAvailable { + mgr.ingressAvailable = true + mgr.managedObjectsRef[kind.IngressKind] = &networkingv1.Ingress{} + } else if legacyIngressAvailable { + mgr.legacyIngressAvailable = true + mgr.managedObjectsRef[kind.IngressKind] = &networkingv1beta1.Ingress{} + } -func (m *Manager) IngressAvailable() bool { - return m.ingressAvailable -} + if routeAvailable { + mgr.routeAvailable = true + mgr.managedObjectsRef[kind.RouteKind] = &routev1.Route{} + } -func (m *Manager) RouteAvailable() bool { - return m.routeAvailable + return mgr, nil } // GetRequiredResources returns the resources initialized by the manager @@ -95,16 +107,16 @@ func (m *Manager) GetRequiredResources() ([]resource.KubernetesResource, error) return nil, fmt.Errorf(resUnavailableFormat, "routes") } - m.log.Debug("Generating required resource", "kind", framework.RouteKind) + m.log.Debug("Generating required resource", "kind", kind.RouteKind) route := m.createRoute() resources = append(resources, route) case v1alpha1.IngressExposeType: - if !m.ingressAvailable { + if !m.ingressAvailable && !m.legacyIngressAvailable { return nil, fmt.Errorf(resUnavailableFormat, "ingresses") } - m.log.Debug("Generating required resource", "kind", framework.IngressKind) + m.log.Debug("Generating required resource", "kind", kind.IngressKind) ingress := m.createIngress() resources = append(resources, ingress) } @@ -119,7 +131,15 @@ func (m *Manager) createRoute() *routev1.Route { return builder.build() } -func (m *Manager) createIngress() *networkingv1beta1.Ingress { +func (m *Manager) createIngress() resource.KubernetesResource { + // we're only here if either ingress is available, no need to check legacy is + if !m.ingressAvailable { + builder := newLegacyIngressBuilder(m.nexus) + if len(m.nexus.Spec.Networking.TLS.SecretName) > 0 { + builder = builder.withCustomTLS() + } + return builder.build() + } builder := newIngressBuilder(m.nexus) if len(m.nexus.Spec.Networking.TLS.SecretName) > 0 { builder = builder.withCustomTLS() @@ -129,45 +149,32 @@ func (m *Manager) createIngress() *networkingv1beta1.Ingress { // GetDeployedResources returns the networking resources deployed on the cluster func (m *Manager) GetDeployedResources() ([]resource.KubernetesResource, error) { - var resources []resource.KubernetesResource - if m.routeAvailable { - route := &routev1.Route{} - if err := framework.Fetch(m.client, framework.Key(m.nexus), route, framework.RouteKind); err == nil { - resources = append(resources, route) - } else if !errors.IsNotFound(err) { - return nil, fmt.Errorf("could not fetch %s (%s/%s): %v", framework.RouteKind, m.nexus.Namespace, m.nexus.Name, err) - } - } - if m.ingressAvailable { - ingress := &networkingv1beta1.Ingress{} - if err := framework.Fetch(m.client, framework.Key(m.nexus), ingress, framework.IngressKind); err == nil { - resources = append(resources, ingress) - } else if !errors.IsNotFound(err) { - return nil, fmt.Errorf("could not fetch %s (%s/%s): %v", framework.IngressKind, m.nexus.Namespace, m.nexus.Name, err) - } - } - return resources, nil + return framework.FetchDeployedResources(m.managedObjectsRef, m.nexus, m.client) } // GetCustomComparator returns the custom comp function used to compare a networking resource. // Returns nil if there is none func (m *Manager) GetCustomComparator(t reflect.Type) func(deployed resource.KubernetesResource, requested resource.KubernetesResource) bool { - if t == reflect.TypeOf(&networkingv1beta1.Ingress{}) { + switch t { + case legacyIngressType: + return legacyIngressEqual + case ingressType: return ingressEqual + default: + return nil } - return nil } // GetCustomComparators returns all custom comp functions in a map indexed by the resource type // Returns nil if there are none func (m *Manager) GetCustomComparators() map[reflect.Type]func(deployed resource.KubernetesResource, requested resource.KubernetesResource) bool { - ingressType := reflect.TypeOf(networkingv1beta1.Ingress{}) return map[reflect.Type]func(deployed resource.KubernetesResource, requested resource.KubernetesResource) bool{ - ingressType: ingressEqual, + legacyIngressType: legacyIngressEqual, + ingressType: ingressEqual, } } -func ingressEqual(deployed resource.KubernetesResource, requested resource.KubernetesResource) bool { +func legacyIngressEqual(deployed resource.KubernetesResource, requested resource.KubernetesResource) bool { ingress1 := deployed.(*networkingv1beta1.Ingress) ingress2 := requested.(*networkingv1beta1.Ingress) var pairs [][2]interface{} @@ -181,3 +188,18 @@ func ingressEqual(deployed resource.KubernetesResource, requested resource.Kuber } return equal } + +func ingressEqual(deployed resource.KubernetesResource, requested resource.KubernetesResource) bool { + ingress1 := deployed.(*networkingv1.Ingress) + ingress2 := requested.(*networkingv1.Ingress) + var pairs [][2]interface{} + pairs = append(pairs, [2]interface{}{ingress1.Name, ingress2.Name}) + pairs = append(pairs, [2]interface{}{ingress1.Namespace, ingress2.Namespace}) + pairs = append(pairs, [2]interface{}{ingress1.Spec, ingress2.Spec}) + + equal := compare.EqualPairs(pairs) + if !equal { + logger.GetLogger("networking_manager").Info("Resources are not equal", "deployed", deployed, "requested", requested) + } + return equal +} diff --git a/controllers/nexus/resource/networking/manager_test.go b/controllers/nexus/resource/networking/manager_test.go index b0542515..fc4638f9 100644 --- a/controllers/nexus/resource/networking/manager_test.go +++ b/controllers/nexus/resource/networking/manager_test.go @@ -20,8 +20,10 @@ import ( "reflect" "testing" + "github.com/RHsyseng/operator-utils/pkg/resource" routev1 "github.com/openshift/api/route/v1" "github.com/stretchr/testify/assert" + networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -30,6 +32,7 @@ import ( "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/controllers/nexus/resource/deployment" "github.com/m88i/nexus-operator/pkg/cluster/discovery" + "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" "github.com/m88i/nexus-operator/pkg/test" ) @@ -41,30 +44,12 @@ var nodePortNexus = &v1alpha1.Nexus{ }, } -func TestManager_IngressAvailable(t *testing.T) { - client := test.NewFakeClientBuilder().Build() - discovery.SetClient(client) - nexus := &v1alpha1.Nexus{} - mgr, err := NewManager(nexus, client) - assert.Nil(t, err) - - assert.Equal(t, mgr.ingressAvailable, mgr.IngressAvailable()) -} - -func TestManager_RouteAvailable(t *testing.T) { - client := test.NewFakeClientBuilder().Build() - discovery.SetClient(client) - nexus := &v1alpha1.Nexus{} - mgr, err := NewManager(nexus, client) - assert.Nil(t, err) - - assert.Equal(t, mgr.ingressAvailable, mgr.IngressAvailable()) -} - func TestNewManager(t *testing.T) { k8sClient := test.NewFakeClientBuilder().Build() k8sClientWithIngress := test.NewFakeClientBuilder().WithIngress().Build() + k8sClientWithLegacyIngress := test.NewFakeClientBuilder().WithLegacyIngress().Build() ocpClient := test.NewFakeClientBuilder().OnOpenshift().Build() + //default-setting logic is tested elsewhere //so here we just check if the resulting manager took in the arguments correctly tests := []struct { @@ -73,35 +58,52 @@ func TestNewManager(t *testing.T) { wantClient *test.FakeClient }{ { - "On Kubernetes with Ingresses available", + "On Kubernetes with v1 Ingresses available", &Manager{ - nexus: nodePortNexus, - client: test.NewFakeClientBuilder().WithIngress().Build(), - routeAvailable: false, - ingressAvailable: true, - ocp: false, + nexus: nodePortNexus, + routeAvailable: false, + ingressAvailable: true, + legacyIngressAvailable: false, + managedObjectsRef: map[string]resource.KubernetesResource{ + kind.IngressKind: &networkingv1.Ingress{}, + }, }, k8sClientWithIngress, }, + { + "On Kubernetes with v1beta1 Ingresses available", + &Manager{ + nexus: nodePortNexus, + routeAvailable: false, + ingressAvailable: false, + legacyIngressAvailable: true, + managedObjectsRef: map[string]resource.KubernetesResource{ + kind.IngressKind: &networkingv1beta1.Ingress{}, + }, + }, + k8sClientWithLegacyIngress, + }, { "On Kubernetes without Ingresses", &Manager{ - nexus: nodePortNexus, - client: test.NewFakeClientBuilder().Build(), - routeAvailable: false, - ingressAvailable: false, - ocp: false, + nexus: nodePortNexus, + routeAvailable: false, + ingressAvailable: false, + legacyIngressAvailable: false, + managedObjectsRef: make(map[string]resource.KubernetesResource), }, k8sClient, }, { "On Openshift", &Manager{ - nexus: nodePortNexus, - client: test.NewFakeClientBuilder().OnOpenshift().Build(), - routeAvailable: true, - ingressAvailable: false, - ocp: true, + nexus: nodePortNexus, + routeAvailable: true, + ingressAvailable: false, + legacyIngressAvailable: false, + managedObjectsRef: map[string]resource.KubernetesResource{ + kind.RouteKind: &routev1.Route{}, + }, }, ocpClient, }, @@ -111,11 +113,12 @@ func TestNewManager(t *testing.T) { discovery.SetClient(tt.wantClient) got, err := NewManager(nodePortNexus, tt.wantClient) assert.NoError(t, err) - assert.NotNil(t, got.client) - assert.NotNil(t, got.nexus) + assert.Equal(t, tt.wantClient, got.client) + assert.Equal(t, tt.want.nexus, got.nexus) assert.Equal(t, tt.want.routeAvailable, got.routeAvailable) assert.Equal(t, tt.want.ingressAvailable, got.ingressAvailable) - assert.Equal(t, tt.want.ocp, got.ocp) + assert.Equal(t, tt.want.legacyIngressAvailable, got.legacyIngressAvailable) + assert.Equal(t, tt.want.managedObjectsRef, got.managedObjectsRef) } // simulate discovery 500 response, expect error @@ -147,7 +150,6 @@ func TestManager_GetRequiredResources(t *testing.T) { client: test.NewFakeClientBuilder().OnOpenshift().Build(), log: logger.GetLoggerWithResource("test", routeNexus), routeAvailable: true, - ocp: true, } resources, err = mgr.GetRequiredResources() assert.Nil(t, err) @@ -167,14 +169,14 @@ func TestManager_GetRequiredResources(t *testing.T) { // now an ingress mgr = &Manager{ nexus: ingressNexus, - client: test.NewFakeClientBuilder().WithIngress().Build(), + client: test.NewFakeClientBuilder().WithLegacyIngress().Build(), log: logger.GetLoggerWithResource("test", ingressNexus), ingressAvailable: true, } resources, err = mgr.GetRequiredResources() assert.Nil(t, err) assert.Len(t, resources, 1) - assert.True(t, test.ContainsType(resources, reflect.TypeOf(&networkingv1beta1.Ingress{}))) + assert.True(t, test.ContainsType(resources, reflect.TypeOf(&networkingv1.Ingress{}))) // still an ingress, but in a cluster without ingresses mgr = &Manager{ @@ -203,13 +205,29 @@ func TestManager_createRoute(t *testing.T) { func TestManager_createIngress(t *testing.T) { mgr := &Manager{nexus: &v1alpha1.Nexus{Spec: v1alpha1.NexusSpec{Networking: v1alpha1.NexusNetworking{TLS: v1alpha1.NexusNetworkingTLS{}}}}} + // Let's test out everything with v1beta1 ingresses + mgr.legacyIngressAvailable = true + + // first without TLS + legacyIngress := mgr.createIngress().(*networkingv1beta1.Ingress) + assert.Empty(t, legacyIngress.Spec.TLS) + + // now with TLS + mgr.nexus.Spec.Networking.TLS.SecretName = "test-secret" + legacyIngress = mgr.createIngress().(*networkingv1beta1.Ingress) + assert.Len(t, legacyIngress.Spec.TLS, 1) + + // Now repeat, but with networkingv1 ingresses + mgr.nexus.Spec.Networking.TLS = v1alpha1.NexusNetworkingTLS{} + mgr.ingressAvailable = true + // first without TLS - ingress := mgr.createIngress() + ingress := mgr.createIngress().(*networkingv1.Ingress) assert.Empty(t, ingress.Spec.TLS) // now with TLS mgr.nexus.Spec.Networking.TLS.SecretName = "test-secret" - ingress = mgr.createIngress() + ingress = mgr.createIngress().(*networkingv1.Ingress) assert.Len(t, ingress.Spec.TLS, 1) } @@ -221,7 +239,10 @@ func TestManager_GetDeployedResources(t *testing.T) { client: fakeClient, ingressAvailable: true, routeAvailable: true, - ocp: true, + managedObjectsRef: map[string]resource.KubernetesResource{ + kind.RouteKind: &routev1.Route{}, + kind.IngressKind: &networkingv1.Ingress{}, + }, } resources, err := mgr.GetDeployedResources() assert.Nil(t, resources) @@ -232,14 +253,14 @@ func TestManager_GetDeployedResources(t *testing.T) { route := &routev1.Route{ObjectMeta: metav1.ObjectMeta{Name: mgr.nexus.Name, Namespace: mgr.nexus.Namespace}} assert.NoError(t, mgr.client.Create(ctx.TODO(), route)) - ingress := &networkingv1beta1.Ingress{ObjectMeta: metav1.ObjectMeta{Name: mgr.nexus.Name, Namespace: mgr.nexus.Namespace}} + ingress := &networkingv1.Ingress{ObjectMeta: metav1.ObjectMeta{Name: mgr.nexus.Name, Namespace: mgr.nexus.Namespace}} assert.NoError(t, mgr.client.Create(ctx.TODO(), ingress)) resources, err = mgr.GetDeployedResources() assert.Nil(t, err) assert.Len(t, resources, 2) assert.True(t, test.ContainsType(resources, reflect.TypeOf(&routev1.Route{}))) - assert.True(t, test.ContainsType(resources, reflect.TypeOf(&networkingv1beta1.Ingress{}))) + assert.True(t, test.ContainsType(resources, reflect.TypeOf(&networkingv1.Ingress{}))) // make the client return a mocked 500 response to test errors other than NotFound mockErrorMsg := "mock 500" @@ -257,8 +278,11 @@ func TestManager_GetCustomComparator(t *testing.T) { // there is no custom comparator function for routes routeComp := mgr.GetCustomComparator(reflect.TypeOf(&routev1.Route{})) assert.Nil(t, routeComp) - // there is a custom comparator function for ingresses - ingressComp := mgr.GetCustomComparator(reflect.TypeOf(&networkingv1beta1.Ingress{})) + // there is a custom comparator function for v1beta1 ingresses + legacyIngressComp := mgr.GetCustomComparator(reflect.TypeOf(&networkingv1beta1.Ingress{})) + assert.NotNil(t, legacyIngressComp) + // there is a custom comparator function for v1 ingresses + ingressComp := mgr.GetCustomComparator(reflect.TypeOf(&networkingv1.Ingress{})) assert.NotNil(t, ingressComp) } @@ -267,12 +291,12 @@ func TestManager_GetCustomComparators(t *testing.T) { // comparator functions offered by the manager mgr := &Manager{} - // there is one custom comparator (ingresses) + // there are two custom comparators (v1 and v1beta1 ingresses) comparators := mgr.GetCustomComparators() - assert.Len(t, comparators, 1) + assert.Len(t, comparators, 2) } -func TestIngressEqual(t *testing.T) { +func Test_legacyIngressEqual(t *testing.T) { // base ingress which will be used in all comparisons baseIngress := &networkingv1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "test", UID: "base UID"}, @@ -342,6 +366,97 @@ func TestIngressEqual(t *testing.T) { }, } + for _, testCase := range testCases { + if legacyIngressEqual(baseIngress, testCase.modifiedIngress) != testCase.wantEqual { + assert.Failf(t, "%s\nbase: %+v\nmodified: %+v\nwantedEqual: %v", testCase.name, baseIngress, testCase.modifiedIngress, testCase.wantEqual) + } + } +} + +func Test_ingressEqual(t *testing.T) { + var bogusPathType networkingv1.PathType = "not a real path type" + + // base ingress which will be used in all comparisons + baseIngress := &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "test", UID: "base UID"}, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: "test.example.com", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + PathType: &pathTypeExact, + Path: ingressBasePath, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test", + Port: networkingv1.ServiceBackendPort{Number: deployment.NexusServicePort}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + testCases := []struct { + name string + modifiedIngress *networkingv1.Ingress + wantEqual bool + }{ + { + "Two ingresses that are equal where it matters and different where it doesn't", + func() *networkingv1.Ingress { + ingress := baseIngress.DeepCopy() + // simulates a field that would be different in runtime + ingress.UID = "a different UID" + return ingress + }(), + true, + }, + { + "All equal except name", + func() *networkingv1.Ingress { + ingress := baseIngress.DeepCopy() + ingress.Name = "a different name" + return ingress + }(), + false, + }, + { + "All equal except namespace", + func() *networkingv1.Ingress { + ingress := baseIngress.DeepCopy() + ingress.Namespace = "a different namespace" + return ingress + }(), + false, + }, + { + "All equal except the service name", + func() *networkingv1.Ingress { + ingress := baseIngress.DeepCopy() + ingress.Spec.Rules[0].IngressRuleValue.HTTP.Paths[0].Backend.Service.Name = "a different service" + return ingress + }(), + false, + }, + { + "All equal except the Path Type", + func() *networkingv1.Ingress { + ingress := baseIngress.DeepCopy() + ingress.Spec.Rules[0].IngressRuleValue.HTTP.Paths[0].PathType = &bogusPathType + return ingress + }(), + false, + }, + } + for _, testCase := range testCases { if ingressEqual(baseIngress, testCase.modifiedIngress) != testCase.wantEqual { assert.Failf(t, "%s\nbase: %+v\nmodified: %+v\nwantedEqual: %v", testCase.name, baseIngress, testCase.modifiedIngress, testCase.wantEqual) diff --git a/controllers/nexus/resource/persistence/manager.go b/controllers/nexus/resource/persistence/manager.go index 64d7d5a3..1b879627 100644 --- a/controllers/nexus/resource/persistence/manager.go +++ b/controllers/nexus/resource/persistence/manager.go @@ -25,11 +25,12 @@ import ( "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/pkg/framework" + "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" ) var managedObjectsRef = map[string]resource.KubernetesResource{ - framework.PVCKind: &corev1.PersistentVolumeClaim{}, + kind.PVCKind: &corev1.PersistentVolumeClaim{}, } // Manager is responsible for creating persistence resources, fetching deployed ones and comparing them @@ -57,7 +58,7 @@ func (m *Manager) GetRequiredResources() ([]resource.KubernetesResource, error) return resources, nil } - m.log.Debug("Generating required resource", "kind", framework.PVCKind) + m.log.Debug("Generating required resource", "kind", kind.PVCKind) pvc := newPVC(m.nexus) resources = append(resources, pvc) diff --git a/controllers/nexus/resource/security/manager.go b/controllers/nexus/resource/security/manager.go index bb71f931..dd0217ff 100644 --- a/controllers/nexus/resource/security/manager.go +++ b/controllers/nexus/resource/security/manager.go @@ -25,12 +25,13 @@ import ( "github.com/m88i/nexus-operator/api/v1alpha1" "github.com/m88i/nexus-operator/pkg/framework" + "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" ) var managedObjectsRef = map[string]resource.KubernetesResource{ - framework.SecretKind: &core.Secret{}, - framework.SvcAccountKind: &core.ServiceAccount{}, + kind.SecretKind: &core.Secret{}, + kind.SvcAccountKind: &core.ServiceAccount{}, } // Manager is responsible for creating security resources, fetching deployed ones and comparing them @@ -52,8 +53,8 @@ func NewManager(nexus *v1alpha1.Nexus, client client.Client) *Manager { // GetRequiredResources returns the resources initialized by the Manager func (m *Manager) GetRequiredResources() ([]resource.KubernetesResource, error) { - m.log.Debug("Generating required resource", "kind", framework.SvcAccountKind) - m.log.Debug("Generating required resource", "kind", framework.SecretKind) + m.log.Debug("Generating required resource", "kind", kind.SvcAccountKind) + m.log.Debug("Generating required resource", "kind", kind.SecretKind) return []resource.KubernetesResource{defaultServiceAccount(m.nexus), defaultSecret(m.nexus)}, nil } diff --git a/controllers/nexus/resource/security/manager_test.go b/controllers/nexus/resource/security/manager_test.go index 08087dd5..1a70b321 100644 --- a/controllers/nexus/resource/security/manager_test.go +++ b/controllers/nexus/resource/security/manager_test.go @@ -20,6 +20,7 @@ import ( "reflect" "testing" + "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/logger" "github.com/stretchr/testify/assert" @@ -102,13 +103,13 @@ func TestManager_getDeployedSvcAccnt(t *testing.T) { } // first, test without creating the svcAccnt - err := framework.Fetch(mgr.client, framework.Key(mgr.nexus), managedObjectsRef[framework.SvcAccountKind], framework.SvcAccountKind) + err := framework.Fetch(mgr.client, framework.Key(mgr.nexus), managedObjectsRef[kind.SvcAccountKind], kind.SvcAccountKind) assert.True(t, errors.IsNotFound(err)) // now test after creating the svcAccnt svcAccnt := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: mgr.nexus.Name, Namespace: mgr.nexus.Namespace}} assert.NoError(t, mgr.client.Create(ctx.TODO(), svcAccnt)) - err = framework.Fetch(mgr.client, framework.Key(svcAccnt), svcAccnt, framework.SvcAccountKind) + err = framework.Fetch(mgr.client, framework.Key(svcAccnt), svcAccnt, kind.SvcAccountKind) assert.NotNil(t, svcAccnt) assert.NoError(t, err) } diff --git a/controllers/nexus/resource/validation/validation.go b/controllers/nexus/resource/validation/validation.go index d32e39c9..3ad5500a 100644 --- a/controllers/nexus/resource/validation/validation.go +++ b/controllers/nexus/resource/validation/validation.go @@ -53,6 +53,11 @@ func NewValidator(client client.Client, scheme *runtime.Scheme) (*Validator, err return nil, fmt.Errorf(discFailureFormat, "ingresses", err) } + legacyIngressAvailable, err := discovery.IsLegacyIngressAvailable() + if err != nil { + return nil, fmt.Errorf(discFailureFormat, "ingresses", err) + } + ocp, err := discovery.IsOpenShift() if err != nil { return nil, fmt.Errorf(discOCPFailureFormat, err) @@ -62,7 +67,7 @@ func NewValidator(client client.Client, scheme *runtime.Scheme) (*Validator, err client: client, scheme: scheme, routeAvailable: routeAvailable, - ingressAvailable: ingressAvailable, + ingressAvailable: ingressAvailable || legacyIngressAvailable, ocp: ocp, }, nil } @@ -236,8 +241,10 @@ func (v *Validator) setUpdateDefaults(nexus *v1alpha1.Nexus) { tag, _ = update.GetLatestMicro(minor) } newImage := fmt.Sprintf("%s:%s", image, tag) - v.log.Debug("Replacing 'spec.image'", "OldImage", nexus.Spec.Image, "NewImage", newImage) - nexus.Spec.Image = newImage + if newImage != nexus.Spec.Image { + v.log.Debug("Replacing 'spec.image'", "OldImage", nexus.Spec.Image, "NewImage", newImage) + nexus.Spec.Image = newImage + } } func (v *Validator) setNetworkingDefaults(nexus *v1alpha1.Nexus) { diff --git a/controllers/nexus/resource/validation/validation_test.go b/controllers/nexus/resource/validation/validation_test.go index 542c84c9..b7aa9400 100644 --- a/controllers/nexus/resource/validation/validation_test.go +++ b/controllers/nexus/resource/validation/validation_test.go @@ -54,7 +54,16 @@ func TestNewValidator(t *testing.T) { }, }, { - "On K8s with ingress", + "On K8s with v1beta1 ingress", + test.NewFakeClientBuilder().WithLegacyIngress().Build(), + &Validator{ + routeAvailable: false, + ingressAvailable: true, + ocp: false, + }, + }, + { + "On K8s with v1 ingress", test.NewFakeClientBuilder().WithIngress().Build(), &Validator{ routeAvailable: false, diff --git a/controllers/nexus/server/users.go b/controllers/nexus/server/users.go index db4a5d36..0b1e4ad0 100644 --- a/controllers/nexus/server/users.go +++ b/controllers/nexus/server/users.go @@ -22,6 +22,7 @@ import ( corev1 "k8s.io/api/core/v1" "github.com/m88i/nexus-operator/pkg/framework" + "github.com/m88i/nexus-operator/pkg/framework/kind" ) const ( @@ -107,7 +108,7 @@ func (u *userOperation) createOperatorUserIfNotExists() (*nexus.User, error) { func (u *userOperation) storeOperatorUserCredentials(user *nexus.User) error { secret := &corev1.Secret{} log.Debug("Attempt to store operator user credentials into Secret") - if err := framework.Fetch(u.k8sclient, framework.Key(u.nexus), secret, framework.SecretKind); err != nil { + if err := framework.Fetch(u.k8sclient, framework.Key(u.nexus), secret, kind.SecretKind); err != nil { return err } if secret.StringData == nil { @@ -124,7 +125,7 @@ func (u *userOperation) storeOperatorUserCredentials(user *nexus.User) error { func (u *userOperation) getOperatorUserCredentials() (user, password string, err error) { secret := &corev1.Secret{} - if err := framework.Fetch(u.k8sclient, framework.Key(u.nexus), secret, framework.SecretKind); err != nil { + if err := framework.Fetch(u.k8sclient, framework.Key(u.nexus), secret, kind.SecretKind); err != nil { return "", "", err } return string(secret.Data[SecretKeyUsername]), string(secret.Data[SecretKeyPassword]), nil diff --git a/pkg/cluster/discovery/discovery.go b/pkg/cluster/discovery/discovery.go index be9a5feb..3d20ebe9 100644 --- a/pkg/cluster/discovery/discovery.go +++ b/pkg/cluster/discovery/discovery.go @@ -15,6 +15,7 @@ package discovery import ( + "fmt" "strings" "k8s.io/client-go/discovery" @@ -62,3 +63,21 @@ func hasGroupVersion(group, version string) (bool, error) { } return false, nil } + +// hasGroupVersionKind checks if the given group name, version and kind is available in the cluster +func hasGroupVersionKind(group, version, kind string) (bool, error) { + if hasGroupVersion, err := hasGroupVersion(group, version); err != nil || !hasGroupVersion { + return false, err + } + + resources, err := cli.ServerResourcesForGroupVersion(fmt.Sprintf("%s/%s", group, version)) + if err != nil { + return false, err + } + for _, resource := range resources.APIResources { + if resource.Kind == kind { + return true, nil + } + } + return false, nil +} diff --git a/pkg/cluster/discovery/kubernetes.go b/pkg/cluster/discovery/kubernetes.go index f951c8f7..a36d571f 100644 --- a/pkg/cluster/discovery/kubernetes.go +++ b/pkg/cluster/discovery/kubernetes.go @@ -15,10 +15,18 @@ package discovery import ( - networking "k8s.io/api/networking/v1beta1" + networkingv1 "k8s.io/api/networking/v1" + networkingv1beta1 "k8s.io/api/networking/v1beta1" + + "github.com/m88i/nexus-operator/pkg/framework/kind" ) -// IsIngressAvailable checks if th cluster supports Ingresses from k8s.io/api/networking/v1beta1 +// IsIngressAvailable checks if the cluster supports Ingresses from k8s.io/api/networking/v1 func IsIngressAvailable() (bool, error) { - return hasGroupVersion(networking.GroupName, networking.SchemeGroupVersion.Version) + return hasGroupVersionKind(networkingv1.SchemeGroupVersion.Group, networkingv1.SchemeGroupVersion.Version, kind.IngressKind) +} + +// IsLegacyIngressAvailable checks if the cluster supports Ingresses from k8s.io/api/networking/v1beta1 +func IsLegacyIngressAvailable() (bool, error) { + return hasGroupVersionKind(networkingv1beta1.SchemeGroupVersion.Group, networkingv1beta1.SchemeGroupVersion.Version, kind.IngressKind) } diff --git a/pkg/cluster/discovery/kubernetes_test.go b/pkg/cluster/discovery/kubernetes_test.go index 878c9ca7..b67d9b68 100644 --- a/pkg/cluster/discovery/kubernetes_test.go +++ b/pkg/cluster/discovery/kubernetes_test.go @@ -33,3 +33,15 @@ func TestIsIngressAvailable(t *testing.T) { assert.Nil(t, err) assert.True(t, ingressAvailable) } + +func TestIsLegacyIngressAvailable(t *testing.T) { + cli = test.NewFakeClientBuilder().Build() + ingressAvailable, err := IsLegacyIngressAvailable() + assert.Nil(t, err) + assert.False(t, ingressAvailable) + + cli = test.NewFakeClientBuilder().WithLegacyIngress().Build() + ingressAvailable, err = IsLegacyIngressAvailable() + assert.Nil(t, err) + assert.True(t, ingressAvailable) +} diff --git a/pkg/cluster/discovery/openshift.go b/pkg/cluster/discovery/openshift.go index 96cbd7c8..4168562b 100644 --- a/pkg/cluster/discovery/openshift.go +++ b/pkg/cluster/discovery/openshift.go @@ -16,6 +16,8 @@ package discovery import ( routev1 "github.com/openshift/api/route/v1" + + "github.com/m88i/nexus-operator/pkg/framework/kind" ) const ( @@ -29,5 +31,5 @@ func IsOpenShift() (bool, error) { // IsRouteAvailable verifies if the current cluster has the Route API from OpenShift available func IsRouteAvailable() (bool, error) { - return hasGroupVersion(routev1.GroupName, routev1.GroupVersion.Version) + return hasGroupVersionKind(routev1.GroupVersion.Group, routev1.GroupVersion.Version, kind.RouteKind) } diff --git a/pkg/framework/fetcher.go b/pkg/framework/fetcher.go index 58e4b421..87c8b45b 100644 --- a/pkg/framework/fetcher.go +++ b/pkg/framework/fetcher.go @@ -16,13 +16,33 @@ package framework import ( ctx "context" + "fmt" "github.com/RHsyseng/operator-utils/pkg/resource" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/m88i/nexus-operator/api/v1alpha1" ) +// FetchDeployedResources fetches deployed resources whose Kind is present in "managedObjectsRef" +func FetchDeployedResources(managedObjectsRef map[string]resource.KubernetesResource, nexus *v1alpha1.Nexus, cli client.Client) ([]resource.KubernetesResource, error) { + var resources []resource.KubernetesResource + for resKind, resRef := range managedObjectsRef { + key := Key(nexus) + if err := Fetch(cli, key, resRef, resKind); err == nil { + resources = append(resources, resRef) + } else if !errors.IsNotFound(err) { + return nil, fmt.Errorf("could not fetch %s (%s/%s): %v", resKind, nexus.Namespace, nexus.Name, err) + } else { + log.Debug("Unable to find resource", "kind", resKind, "namespacedName", key) + } + } + return resources, nil +} + +// Fetch fetches a single deployed resource and stores it in "instance" func Fetch(client client.Client, key types.NamespacedName, instance resource.KubernetesResource, kind string) error { log.Info("Attempting to fetch deployed resource", "kind", kind, "namespacedName", key) if err := client.Get(ctx.TODO(), key, instance); err != nil { diff --git a/pkg/framework/fetcher_test.go b/pkg/framework/fetcher_test.go index cb3cadfc..f5fe3e90 100644 --- a/pkg/framework/fetcher_test.go +++ b/pkg/framework/fetcher_test.go @@ -15,27 +15,63 @@ package framework import ( + goerrors "errors" "testing" + "github.com/RHsyseng/operator-utils/pkg/resource" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/m88i/nexus-operator/api/v1alpha1" + "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/test" ) +func TestFetchDeployedResources(t *testing.T) { + nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus-test", Namespace: t.Name()}} + deployment := &appsv1.Deployment{ObjectMeta: nexus.ObjectMeta} + service := &corev1.Service{ObjectMeta: nexus.ObjectMeta} + managedObjectsRef := map[string]resource.KubernetesResource{ + kind.ServiceKind: &corev1.Service{}, + kind.DeploymentKind: &appsv1.Deployment{}, + // we won't have a SA, but this is useful to test no error is triggered when a resource isn't found + kind.SvcAccountKind: &corev1.ServiceAccount{}, + } + cli := test.NewFakeClientBuilder(deployment, service).Build() + + gotResources, err := FetchDeployedResources(managedObjectsRef, nexus, cli) + + assert.Nil(t, err) + assert.Len(t, gotResources, 2) +} + +func TestFetchDeployedResourcesFailure(t *testing.T) { + nexus := &v1alpha1.Nexus{ObjectMeta: metav1.ObjectMeta{Name: "nexus-test", Namespace: t.Name()}} + // managedObjectsRef cannot be empty in order to raise error, the content is irrelevant though + managedObjectsRef := map[string]resource.KubernetesResource{kind.DeploymentKind: &appsv1.Deployment{}} + cli := test.NewFakeClientBuilder().Build() + mockErrorMsg := "mock error" + + cli.SetMockError(goerrors.New(mockErrorMsg)) + _, err := FetchDeployedResources(managedObjectsRef, nexus, cli) + + assert.Contains(t, err.Error(), mockErrorMsg) +} + func TestFetch(t *testing.T) { - deployment := &appsv1.Deployment{ObjectMeta: v1.ObjectMeta{Name: "deployment", Namespace: t.Name()}} + deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "deployment", Namespace: t.Name()}} cli := test.NewFakeClientBuilder(deployment).Build() - err := Fetch(cli, Key(deployment), deployment, DeploymentKind) + err := Fetch(cli, Key(deployment), deployment, kind.DeploymentKind) assert.NoError(t, err) } func TestNotFoundFetch(t *testing.T) { - deployment := &appsv1.Deployment{ObjectMeta: v1.ObjectMeta{Name: "deployment", Namespace: t.Name()}} + deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "deployment", Namespace: t.Name()}} cli := test.NewFakeClientBuilder().Build() - err := Fetch(cli, Key(deployment), deployment, DeploymentKind) + err := Fetch(cli, Key(deployment), deployment, kind.DeploymentKind) assert.Error(t, err) assert.True(t, errors.IsNotFound(err)) } diff --git a/pkg/framework/kinds.go b/pkg/framework/kind/kinds.go similarity index 97% rename from pkg/framework/kinds.go rename to pkg/framework/kind/kinds.go index d3b4bc10..63d95c16 100644 --- a/pkg/framework/kinds.go +++ b/pkg/framework/kind/kinds.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package framework +package kind const ( DeploymentKind = "Deployment" diff --git a/pkg/test/client.go b/pkg/test/client.go index 8f660518..e1632c66 100644 --- a/pkg/test/client.go +++ b/pkg/test/client.go @@ -19,7 +19,8 @@ import ( openapi_v2 "github.com/googleapis/gnostic/openapiv2" routev1 "github.com/openshift/api/route/v1" - "k8s.io/api/networking/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/runtime" "k8s.io/apimachinery/pkg/version" @@ -32,6 +33,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/m88i/nexus-operator/api/v1alpha1" + "github.com/m88i/nexus-operator/pkg/framework/kind" "github.com/m88i/nexus-operator/pkg/util" ) @@ -66,14 +68,21 @@ func (b *FakeClientBuilder) OnOpenshift() *FakeClientBuilder { util.Must(schemeBuilderOnOCP().AddToScheme(b.scheme)) b.resources = append(b.resources, &metav1.APIResourceList{GroupVersion: openshiftGroupVersion}, - &metav1.APIResourceList{GroupVersion: routev1.GroupVersion.String()}) + &metav1.APIResourceList{GroupVersion: routev1.GroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.RouteKind}}}) return b } -// WithIngress makes the fake client aware of Ingresses +// WithIngress makes the fake client aware of v1 Ingresses func (b *FakeClientBuilder) WithIngress() *FakeClientBuilder { util.Must(schemeBuilderWithIngress().AddToScheme(b.scheme)) - b.resources = append(b.resources, &metav1.APIResourceList{GroupVersion: v1beta1.SchemeGroupVersion.String()}) + b.resources = append(b.resources, &metav1.APIResourceList{GroupVersion: networkingv1.SchemeGroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.IngressKind}}}) + return b +} + +// WithLegacyIngress makes the fake client aware of v1beta1 Ingresses +func (b *FakeClientBuilder) WithLegacyIngress() *FakeClientBuilder { + util.Must(schemeBuilderWithLegacyIngress().AddToScheme(b.scheme)) + b.resources = append(b.resources, &metav1.APIResourceList{GroupVersion: networkingv1beta1.SchemeGroupVersion.String(), APIResources: []metav1.APIResource{{Kind: kind.IngressKind}}}) return b } @@ -99,7 +108,11 @@ func schemeBuilderOnOCP() *runtime.SchemeBuilder { } func schemeBuilderWithIngress() *runtime.SchemeBuilder { - return &runtime.SchemeBuilder{v1beta1.AddToScheme} + return &runtime.SchemeBuilder{networkingv1.AddToScheme} +} + +func schemeBuilderWithLegacyIngress() *runtime.SchemeBuilder { + return &runtime.SchemeBuilder{networkingv1beta1.AddToScheme} } // FakeClient wraps an API fake client to allow mocked error responses diff --git a/pkg/test/client_test.go b/pkg/test/client_test.go index ad7aa2b1..6877c559 100644 --- a/pkg/test/client_test.go +++ b/pkg/test/client_test.go @@ -23,6 +23,7 @@ import ( routev1 "github.com/openshift/api/route/v1" "github.com/stretchr/testify/assert" + 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" @@ -66,6 +67,18 @@ func TestFakeClientBuilder_OnOpenshift(t *testing.T) { func TestFakeClientBuilder_WithIngress(t *testing.T) { b := NewFakeClientBuilder().WithIngress() + // client.Client + assert.Len(t, b.scheme.KnownTypes(networkingv1.SchemeGroupVersion), 14) + assert.Contains(t, b.scheme.KnownTypes(networkingv1.SchemeGroupVersion), strings.Split(reflect.TypeOf(&networkingv1.Ingress{}).String(), ".")[1]) + assert.Contains(t, b.scheme.KnownTypes(networkingv1.SchemeGroupVersion), strings.Split(reflect.TypeOf(&networkingv1.IngressList{}).String(), ".")[1]) + + // discovery.DiscoveryInterface + assert.True(t, resourceListsContainsGroupVersion(b.resources, networkingv1.SchemeGroupVersion.String())) +} + +func TestFakeClientBuilder_WithLegacyIngress(t *testing.T) { + b := NewFakeClientBuilder().WithLegacyIngress() + // client.Client assert.Len(t, b.scheme.KnownTypes(networkingv1beta1.SchemeGroupVersion), 12) assert.Contains(t, b.scheme.KnownTypes(networkingv1beta1.SchemeGroupVersion), strings.Split(reflect.TypeOf(&networkingv1beta1.Ingress{}).String(), ".")[1]) @@ -95,6 +108,8 @@ func TestFakeClientBuilder_Build(t *testing.T) { assert.False(t, withRoute) withIngress, _ := discovery.IsIngressAvailable() assert.False(t, withIngress) + withLegacyIngress, _ := discovery.IsLegacyIngressAvailable() + assert.False(t, withLegacyIngress) // on Openshift b = NewFakeClientBuilder(nexus, route).OnOpenshift() @@ -110,6 +125,8 @@ func TestFakeClientBuilder_Build(t *testing.T) { assert.True(t, withRoute) withIngress, _ = discovery.IsIngressAvailable() assert.False(t, withIngress) + withLegacyIngress, _ = discovery.IsLegacyIngressAvailable() + assert.False(t, withLegacyIngress) // with Ingress b = NewFakeClientBuilder(nexus, ingress).WithIngress() @@ -125,6 +142,25 @@ func TestFakeClientBuilder_Build(t *testing.T) { assert.False(t, withRoute) withIngress, _ = discovery.IsIngressAvailable() assert.True(t, withIngress) + withLegacyIngress, _ = discovery.IsLegacyIngressAvailable() + assert.False(t, withLegacyIngress) + + // with v1beta1 Ingress + b = NewFakeClientBuilder(nexus, ingress).WithLegacyIngress() + c = b.Build() + discovery.SetClient(c) + assert.NoError(t, c.client.Get(ctx.TODO(), client.ObjectKey{ + Namespace: ingress.Namespace, + Name: ingress.Name, + }, ingress)) + ocp, _ = discovery.IsOpenShift() + assert.False(t, ocp) + withRoute, _ = discovery.IsRouteAvailable() + assert.False(t, withRoute) + withIngress, _ = discovery.IsIngressAvailable() + assert.False(t, withIngress) + withLegacyIngress, _ = discovery.IsLegacyIngressAvailable() + assert.True(t, withLegacyIngress) } func resourceListsContainsGroupVersion(lists []*metav1.APIResourceList, gv string) bool { @@ -380,3 +416,8 @@ func TestFakeClient_Status(t *testing.T) { c := NewFakeClientBuilder().Build() assert.Equal(t, c.client.Status(), c.Status()) } + +func TestFakeClient_Scheme(t *testing.T) { + c := NewFakeClientBuilder().Build() + assert.Equal(t, c.scheme, c.Scheme()) +}