Fixing NGINX Ingress reverse proxy (#185)

* Fixing NGINX Ingress reverse proxy

Signed-off-by: Ricardo Zanini <[email protected]>

* Typos

Signed-off-by: Ricardo Zanini <[email protected]>

controllers/nexus/resource/deployment/deployment.go

@@ -78,7 +78,7 @@ func newDeployment(nexus *v1alpha1.Nexus) *appsv1.Deployment {
 							Ports: []corev1.ContainerPort{
 								{
 									Name:          NexusPortName,
-									ContainerPort: NexusServicePort,
+									ContainerPort: nexusContainerPort,
 									Protocol:      corev1.ProtocolTCP,
 								},
 							},
@@ -112,7 +112,7 @@ func addProbes(nexus *v1alpha1.Nexus, deployment *appsv1.Deployment) {
 			HTTPGet: &corev1.HTTPGetAction{
 				Path: "/service/rest/v1/status",
 				Port: intstr.IntOrString{
-					IntVal: NexusServicePort,
+					IntVal: nexusContainerPort,
 				},
 				Scheme: corev1.URISchemeHTTP,
 			},
@@ -129,7 +129,7 @@ func addProbes(nexus *v1alpha1.Nexus, deployment *appsv1.Deployment) {
 			HTTPGet: &corev1.HTTPGetAction{
 				Path: "/service/rest/v1/status",
 				Port: intstr.IntOrString{
-					IntVal: NexusServicePort,
+					IntVal: nexusContainerPort,
 				},
 				Scheme: corev1.URISchemeHTTP,
 			},

controllers/nexus/resource/deployment/deployment_test.go

@@ -54,8 +54,8 @@ func Test_newDeployment_WithoutPersistence(t *testing.T) {
 	assert.Equal(t, validation.NexusCommunityImage, deployment.Spec.Template.Spec.Containers[0].Image)
 	assert.Equal(t, int32(1), *deployment.Spec.Replicas)
 
-	assert.Equal(t, int32(NexusServicePort), deployment.Spec.Template.Spec.Containers[0].LivenessProbe.HTTPGet.Port.IntVal)
-	assert.Equal(t, int32(NexusServicePort), deployment.Spec.Template.Spec.Containers[0].ReadinessProbe.HTTPGet.Port.IntVal)
+	assert.Equal(t, int32(nexusContainerPort), deployment.Spec.Template.Spec.Containers[0].LivenessProbe.HTTPGet.Port.IntVal)
+	assert.Equal(t, int32(nexusContainerPort), deployment.Spec.Template.Spec.Containers[0].ReadinessProbe.HTTPGet.Port.IntVal)
 
 	assert.Len(t, deployment.Spec.Template.Spec.Containers[0].VolumeMounts, 0)
 	assert.Len(t, deployment.Spec.Template.Spec.Volumes, 0)

controllers/nexus/resource/deployment/service.go

@@ -23,9 +23,9 @@ import (
 )
 
 const (
-	NexusPortName    = "http"
-	NexusServicePort = 8081
-	DefaultHTTPPort  = 80
+	NexusPortName      = "http"
+	nexusContainerPort = 8081
+	DefaultHTTPPort    = 80
 )
 
 func newService(nexus *v1alpha1.Nexus) *corev1.Service {
@@ -38,7 +38,7 @@ func newService(nexus *v1alpha1.Nexus) *corev1.Service {
 					Protocol: corev1.ProtocolTCP,
 					Port:     DefaultHTTPPort,
 					TargetPort: intstr.IntOrString{
-						IntVal: NexusServicePort,
+						IntVal: nexusContainerPort,
 					},
 				},
 			},

controllers/nexus/resource/networking/ingress.go

@@ -15,17 +15,24 @@
 package networking
 
 import (
+	"github.com/RHsyseng/operator-utils/pkg/resource"
 	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   = "/?(.*)"
+	ingressClassKey   = "kubernetes.io/ingress.class"
+	ingressClassNginx = "nginx"
+	nginxRewriteKey   = "nginx.ingress.kubernetes.io/rewrite-target"
+	nginxRewriteValue = "/$1"
+)
 
 // hack to take the address of v1.PathExactType
-var pathTypeExact = v1.PathTypeExact
+var pathTypePrefix = v1.PathTypePrefix
 
 type ingressBuilder struct {
 	*v1.Ingress
@@ -43,12 +50,12 @@ func newIngressBuilder(nexus *v1alpha1.Nexus) *ingressBuilder {
 						HTTP: &v1.HTTPIngressRuleValue{
 							Paths: []v1.HTTPIngressPath{
 								{
-									PathType: &pathTypeExact,
+									PathType: &pathTypePrefix,
 									Path:     ingressBasePath,
 									Backend: v1.IngressBackend{
 										Service: &v1.IngressServiceBackend{
 											Name: nexus.Name,
-											Port: v1.ServiceBackendPort{Number: deployment.NexusServicePort},
+											Port: v1.ServiceBackendPort{Number: deployment.DefaultHTTPPort},
 										},
 									},
 								},
@@ -59,7 +66,7 @@ func newIngressBuilder(nexus *v1alpha1.Nexus) *ingressBuilder {
 			},
 		},
 	}
-
+	addNginxAnnotations(ingress)
 	return &ingressBuilder{Ingress: ingress, nexus: nexus}
 }
 
@@ -84,3 +91,10 @@ func hosts(rules []v1.IngressRule) []string {
 	}
 	return hosts
 }
+
+func addNginxAnnotations(resource resource.KubernetesResource) {
+	resource.SetAnnotations(map[string]string{
+		ingressClassKey: ingressClassNginx,
+		nginxRewriteKey: nginxRewriteValue,
+	})
+}

controllers/nexus/resource/networking/ingress_test.go

@@ -26,10 +26,10 @@ import (
 )
 
 var (
-	ingressNexus = &v1alpha1.Nexus{
+	nexusIngress = &v1alpha1.Nexus{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "nexus3",
-			Namespace: "nexus",
+			Namespace: "nexusIngress",
 		},
 		Spec: v1alpha1.NexusSpec{
 			Networking: v1alpha1.NexusNetworking{
@@ -68,26 +68,26 @@ func TestHosts(t *testing.T) {
 }
 
 func TestNewIngress(t *testing.T) {
-	ingress := newIngressBuilder(ingressNexus).build()
+	ingress := newIngressBuilder(nexusIngress).build()
 	assertIngressBasic(t, ingress)
 }
 
 func TestNewIngressWithSecretName(t *testing.T) {
-	ingress := newIngressBuilder(ingressNexus).withCustomTLS().build()
+	ingress := newIngressBuilder(nexusIngress).withCustomTLS().build()
 	assertIngressBasic(t, ingress)
 	assertIngressSecretName(t, ingress)
 }
 
 func assertIngressBasic(t *testing.T, ingress *v1.Ingress) {
-	assert.Equal(t, ingressNexus.Name, ingress.Name)
-	assert.Equal(t, ingressNexus.Namespace, ingress.Namespace)
+	assert.Equal(t, nexusIngress.Name, ingress.Name)
+	assert.Equal(t, nexusIngress.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.Equal(t, nexusIngress.Spec.Networking.Host, rule.Host)
 	assert.NotNil(t, rule.IngressRuleValue)
 	assert.NotNil(t, rule.IngressRuleValue.HTTP)
 
@@ -96,14 +96,16 @@ func assertIngressBasic(t *testing.T, ingress *v1.Ingress) {
 
 	assert.Equal(t, ingressBasePath, path.Path)
 	assert.NotNil(t, path.Backend)
-	assert.Equal(t, int32(deployment.NexusServicePort), path.Backend.Service.Port.Number)
-	assert.Equal(t, ingressNexus.Name, path.Backend.Service.Name)
+	assert.Equal(t, int32(deployment.DefaultHTTPPort), path.Backend.Service.Port.Number)
+	assert.Equal(t, nexusIngress.Name, path.Backend.Service.Name)
+	assert.NotEmpty(t, ingress.Annotations[nginxRewriteKey])
+	assert.Equal(t, ingressClassNginx, ingress.Annotations[ingressClassKey])
 }
 
 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)
+	assert.Equal(t, nexusIngress.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])
+	assert.Equal(t, nexusIngress.Spec.Networking.Host, ingress.Spec.TLS[0].Hosts[0])
 }

controllers/nexus/resource/networking/legacy_ingress.go

@@ -42,7 +42,7 @@ func newLegacyIngressBuilder(nexus *v1alpha1.Nexus) *legacyIngressBuilder {
 									Path: ingressBasePath,
 									Backend: v1beta1.IngressBackend{
 										ServiceName: nexus.Name,
-										ServicePort: intstr.FromInt(deployment.NexusServicePort),
+										ServicePort: intstr.FromInt(deployment.DefaultHTTPPort),
 									},
 								},
 							},
@@ -52,7 +52,7 @@ func newLegacyIngressBuilder(nexus *v1alpha1.Nexus) *legacyIngressBuilder {
 			},
 		},
 	}
-
+	addNginxAnnotations(ingress)
 	return &legacyIngressBuilder{Ingress: ingress, nexus: nexus}
 }
 

controllers/nexus/resource/networking/legacy_ingress_test.go

@@ -48,26 +48,26 @@ func TestLegacyHosts(t *testing.T) {
 }
 
 func TestNewLegacyIngress(t *testing.T) {
-	ingress := newLegacyIngressBuilder(ingressNexus).build()
+	ingress := newLegacyIngressBuilder(nexusIngress).build()
 	assertLegacyIngressBasic(t, ingress)
 }
 
 func TestNewLegacyIngressWithSecretName(t *testing.T) {
-	ingress := newLegacyIngressBuilder(ingressNexus).withCustomTLS().build()
+	ingress := newLegacyIngressBuilder(nexusIngress).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.Equal(t, nexusIngress.Name, ingress.Name)
+	assert.Equal(t, nexusIngress.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.Equal(t, nexusIngress.Spec.Networking.Host, rule.Host)
 	assert.NotNil(t, rule.IngressRuleValue)
 	assert.NotNil(t, rule.IngressRuleValue.HTTP)
 
@@ -76,14 +76,16 @@ func assertLegacyIngressBasic(t *testing.T, ingress *v1beta1.Ingress) {
 
 	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)
+	assert.Equal(t, intstr.FromInt(deployment.DefaultHTTPPort), path.Backend.ServicePort)
+	assert.Equal(t, nexusIngress.Name, path.Backend.ServiceName)
+	assert.NotEmpty(t, ingress.Annotations[nginxRewriteKey])
+	assert.Equal(t, ingressClassNginx, ingress.Annotations[ingressClassKey])
 }
 
 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.Equal(t, nexusIngress.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])
+	assert.Equal(t, nexusIngress.Spec.Networking.Host, ingress.Spec.TLS[0].Hosts[0])
 }

controllers/nexus/resource/networking/manager_test.go

@@ -168,9 +168,9 @@ func TestManager_GetRequiredResources(t *testing.T) {
 
 	// now an ingress
 	mgr = &Manager{
-		nexus:            ingressNexus,
+		nexus:            nexusIngress,
 		client:           test.NewFakeClientBuilder().WithLegacyIngress().Build(),
-		log:              logger.GetLoggerWithResource("test", ingressNexus),
+		log:              logger.GetLoggerWithResource("test", nexusIngress),
 		ingressAvailable: true,
 	}
 	resources, err = mgr.GetRequiredResources()
@@ -180,8 +180,8 @@ func TestManager_GetRequiredResources(t *testing.T) {
 
 	// still an ingress, but in a cluster without ingresses
 	mgr = &Manager{
-		nexus:  ingressNexus,
-		log:    logger.GetLoggerWithResource("test", ingressNexus),
+		nexus:  nexusIngress,
+		log:    logger.GetLoggerWithResource("test", nexusIngress),
 		client: test.NewFakeClientBuilder().Build(),
 	}
 	resources, err = mgr.GetRequiredResources()
@@ -311,7 +311,7 @@ func Test_legacyIngressEqual(t *testing.T) {
 									Path: ingressBasePath,
 									Backend: networkingv1beta1.IngressBackend{
 										ServiceName: "test",
-										ServicePort: intstr.FromInt(deployment.NexusServicePort),
+										ServicePort: intstr.FromInt(deployment.DefaultHTTPPort),
 									},
 								},
 							},
@@ -387,12 +387,12 @@ func Test_ingressEqual(t *testing.T) {
 						HTTP: &networkingv1.HTTPIngressRuleValue{
 							Paths: []networkingv1.HTTPIngressPath{
 								{
-									PathType: &pathTypeExact,
+									PathType: &pathTypePrefix,
 									Path:     ingressBasePath,
 									Backend: networkingv1.IngressBackend{
 										Service: &networkingv1.IngressServiceBackend{
 											Name: "test",
-											Port: networkingv1.ServiceBackendPort{Number: deployment.NexusServicePort},
+											Port: networkingv1.ServiceBackendPort{Number: deployment.DefaultHTTPPort},
 										},
 									},
 								},

controllers/nexus/resource/networking/route.go

@@ -39,7 +39,7 @@ func newRouteBuilder(nexus *v1alpha1.Nexus) *routeBuilder {
 				Name: nexus.Name,
 			},
 			Port: &v1.RoutePort{
-				TargetPort: intstr.FromInt(deployment.NexusServicePort),
+				TargetPort: intstr.FromString(deployment.NexusPortName),
 			},
 		},
 	}

controllers/nexus/resource/networking/route_test.go

@@ -53,7 +53,7 @@ var (
 		},
 		Spec: corev1.ServiceSpec{
 			Ports: []corev1.ServicePort{
-				{TargetPort: intstr.FromInt(deployment.NexusServicePort)},
+				{TargetPort: intstr.FromString(deployment.NexusPortName)},
 			},
 		},
 	}
@@ -74,7 +74,7 @@ func assertRouteBasic(t *testing.T, route *v1.Route) {
 	assert.Equal(t, routeNexus.Name, route.Name)
 	assert.Equal(t, routeNexus.Namespace, route.Namespace)
 	assert.Len(t, route.Labels, 1)
-	assert.Equal(t, ingressNexus.Name, route.Labels[meta.AppLabel])
+	assert.Equal(t, nexusIngress.Name, route.Labels[meta.AppLabel])
 
 	assert.NotNil(t, route.Spec)
 

main.go

@@ -18,7 +18,6 @@ package main
 
 import (
 	"flag"
-	"fmt"
 	"os"
 	"strings"
 
@@ -46,7 +45,7 @@ var (
 
 func init() {
 	// adding routev1
-	utilruntime.Must(routev1.AddToScheme(scheme))
+	utilruntime.Must(routev1.Install(scheme))
 	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
 	utilruntime.Must(appsv1alpha1.AddToScheme(scheme))
 	// +kubebuilder:scaffold:scheme
@@ -63,11 +62,8 @@ func main() {
 
 	ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
 
-	watchNamespace, err := getWatchNamespace()
-	if err != nil {
-		setupLog.Error(err, "unable to get WatchNamespace, "+
-			"the manager will watch and manage resources in all namespaces")
-	}
+	watchNamespace := getWatchNamespace()
+
 	options := ctrl.Options{
 		Scheme:             scheme,
 		MetricsBindAddress: metricsAddr,
@@ -111,15 +107,18 @@ func main() {
 }
 
 // getWatchNamespace returns the Namespace the operator should be watching for changes
-func getWatchNamespace() (string, error) {
+func getWatchNamespace() string {
 	// WatchNamespaceEnvVar is the constant for env variable WATCH_NAMESPACE
 	// which specifies the Namespace to watch.
 	// An empty value means the operator is running with cluster scope.
 	var watchNamespaceEnvVar = "WATCH_NAMESPACE"
 
 	ns, found := os.LookupEnv(watchNamespaceEnvVar)
 	if !found {
-		return "", fmt.Errorf("%s must be set", watchNamespaceEnvVar)
+		setupLog.Info("unable to get WatchNamespace, "+
+			"the manager will watch and manage resources in all namespaces",
+			"Env Var lookup", watchNamespaceEnvVar)
+		return ""
 	}
-	return ns, nil
+	return ns
 }