Merge pull request #47 from Octops/ingress-class-name

Support ingress class name in ingress spec

Makefile

@@ -36,7 +36,7 @@ OCTOPS_BIN := bin/octops-controller
 
 IMAGE_REPO=octops/gameserver-ingress-controller
 DOCKER_IMAGE_TAG ?= octops/gameserver-ingress-controller:${VERSION}
-RELEASE_TAG=0.2.9
+RELEASE_TAG=0.3.0
 
 default: clean build
 

README.md

@@ -61,7 +61,7 @@ spec:
   template:
     metadata:
       annotations:
-        octops-kubernetes.io/ingress.class: "contour" #required for Contour to handle ingress
+        octops.io/ingress-class-name: "contour" #required for Contour to handle ingress
         octops-projectcontour.io/websocket-routes: "/" #required for Contour to enable websocket
         octops.io/gameserver-ingress-mode: "domain"
         octops.io/gameserver-ingress-domain: "example.com"
@@ -84,7 +84,7 @@ spec:
   template:
     metadata:
       annotations:
-        octops-kubernetes.io/ingress.class: "contour" #required for Contour to handle ingress
+        octops.io/ingress-class-name: "contour" #required for Contour to handle ingress
         octops-projectcontour.io/websocket-routes: "/{{ .Name }}" #required for Contour to enable websocket for exact path. This is a template that the controller will replace by the name of the game server
         octops.io/gameserver-ingress-mode: "path"
         octops.io/gameserver-ingress-fqdn: servers.example.com
@@ -115,7 +115,7 @@ spec:
         cluster: gke-1.24
         region: us-east-1
       annotations:
-        octops-kubernetes.io/ingress.class: "contour" # required for Contour to handle ingress
+        octops.io/ingress-class-name: "contour" # required for Contour to handle ingress
         octops-projectcontour.io/websocket-routes: "/" # required for Contour to enable websocket
         # Required annotation used by the controller
         octops.io/gameserver-ingress-mode: "domain"
@@ -165,6 +165,7 @@ The table below shows how the information from the game server is used to compos
 | annotation: octops.io/issuer-tls-name           |  name of the ClusterIssuer  |
 | annotation: octops-[custom-annotation]          |      custom-annotation      |
 | annotation: octops.io/tls-secret-name           |    custom ingress secret    |
+| annotation: octops.io/ingress-class-name        |   ingressClassName field    |
 
 **Support for Multiple Domains**
 
@@ -264,13 +265,13 @@ https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/
 - **octops.io/gameserver-ingress-fqdn:** full domain name where gameservers will be accessed based on the URL path.
 - **octops.io/terminate-tls:** it determines if the ingress will terminate TLS. If set to "false" it means that TLS will be terminated at the load balancer. In this case there won't be a certificate issued by the local cert-manager.
 - **octops.io/issuer-tls-name:** required if `terminate-tls=true` and certificates are provisioned by CertManager. This is the name of the ClusterIssuer that cert-manager will use when creating the certificate for the ingress.
-- **octops.io/tls-secret-name:** ignore CertManager and sets the secret to be used by the Ingress, requires `terminate-tls=true`. This secret might be provisioned by other means. This is specially useful for wildcard certificates that have been generated or acquired using a different process.
+- **octops.io/ingress-class-name:** Defines the ingress class name to be used e.g ("contour", "nginx", "traefik")
 
 The same configuration works for Fleets and GameServers. Add the following annotations to your manifest:
 ```yaml
 # Fleet annotations using ingress routing mode: domain
 annotations:
-  octops-kubernetes.io/ingress.class: "contour" # required for Contour to handle ingress
+  octops.io/ingress-class-name: "contour" # required for Contour to handle ingress
   octops-projectcontour.io/websocket-routes: "/" # required for Contour to enable websocket
   octops.io/gameserver-ingress-mode: "domain"
   octops.io/gameserver-ingress-domain: "example.com"
@@ -281,7 +282,7 @@ annotations:
 ```yaml
 # Fleet annotations using ingress routing mode: path
 annotations:
-  octops-kubernetes.io/ingress.class: "contour" # required for Contour to handle ingress
+  octops.io/ingress-class-name: "contour" # required for Contour to handle ingress
   octops-projectcontour.io/websocket-routes: "/" # required for Contour to enable websocket
   octops.io/gameserver-ingress-mode: "path"
   octops.io/gameserver-ingress-fqdn: "servers.example.com"

deploy/install.yaml

@@ -69,7 +69,7 @@ spec:
     spec:
       serviceAccountName: octops-ingress-controller
       containers:
-        - image: octops/gameserver-ingress-controller:0.2.9 # Latest release
+        - image: octops/gameserver-ingress-controller:0.3.0 # Latest release
           name: controller
           ports:
             - containerPort: 30235

pkg/gameserver/gameserver.go

@@ -19,6 +19,8 @@ const (
 	OctopsAnnotationCustomPrefix           = "octops-"
 	OctopsAnnotationCustomServicePrefix    = "octops.service-"
 	OctopsAnnotationGameServerIngressReady = "octops.io/ingress-ready"
+	OctopsAnnotationIngressClassName       = "octops.io/ingress-class-name"
+	OctopsAnnotationIngressClassNameLegacy = "octops-kubernetes.io/ingress.class"
 
 	CertManagerAnnotationIssuer = "cert-manager.io/cluster-issuer"
 	AgonesGameServerNameLabel   = "agones.dev/gameserver"
@@ -102,3 +104,15 @@ func GetTLSCertIssuer(gs *agonesv1.GameServer) string {
 
 	return ""
 }
+
+func GetIngressClassName(gs *agonesv1.GameServer) string {
+	if className, ok := HasAnnotation(gs, OctopsAnnotationIngressClassName); ok {
+		return className
+	}
+
+	if className, ok := HasAnnotation(gs, OctopsAnnotationIngressClassNameLegacy); ok {
+		return className
+	}
+
+	return ""
+}

pkg/reconcilers/gameserver_reconciler.go

@@ -1,15 +1,18 @@
 package reconcilers
 
 import (
-	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
 	"context"
 	"fmt"
+	"strconv"
+	"strings"
+
+	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
+	"github.com/Octops/gameserver-ingress-controller/internal/runtime"
 	"github.com/Octops/gameserver-ingress-controller/pkg/gameserver"
 	"github.com/Octops/gameserver-ingress-controller/pkg/k8sutil"
 	"github.com/Octops/gameserver-ingress-controller/pkg/record"
 	"github.com/pkg/errors"
 	"k8s.io/client-go/util/retry"
-	"strconv"
 )
 
 type GameServerStore interface {
@@ -64,9 +67,21 @@ func (r *GameServerReconciler) reconcile(ctx context.Context, gs *agonesv1.GameS
 	}
 
 	r.recorder.RecordEvent(result, fmt.Sprintf("GameServer annotated with %s", gameserver.OctopsAnnotationGameServerIngressReady))
+	r.recordDeprecatedAnnotations(result)
 	return result, nil
 }
 
+func (r *GameServerReconciler) recordDeprecatedAnnotations(gs *agonesv1.GameServer) {
+	if _, ok := gs.Annotations[gameserver.OctopsAnnotationIngressClassNameLegacy]; ok {
+
+		msg := fmt.Sprintf("Annotation %s deprecated in favor of %s, future versions won't support this annotation",
+			gameserver.OctopsAnnotationIngressClassNameLegacy, gameserver.OctopsAnnotationIngressClassName)
+
+		r.recorder.RecordEvent(gs, msg)
+		runtime.Logger().Warn(strings.ToLower(msg))
+	}
+}
+
 func (r *GameServerReconciler) MustReconcile(gs *agonesv1.GameServer) (bool, error) {
 	if value, ok := gameserver.HasAnnotation(gs, gameserver.OctopsAnnotationGameServerIngressReady); ok && len(value) > 0 {
 		isReady, err := strconv.ParseBool(value)

pkg/reconcilers/ingress_options.go

@@ -237,6 +237,20 @@ func WithTLSCertIssuer(issuerName string) IngressOption {
 	}
 }
 
+func WithIngressClassName(className string) IngressOption {
+	return func(gs *agonesv1.GameServer, ingress *networkingv1.Ingress) error {
+		if className == "" {
+			return errors.Errorf("annotation %s for %s must not be empty, check your Fleet or GameServer manifest.",
+				gameserver.OctopsAnnotationIngressClassName, gs.Name)
+		}
+
+		//TODO: The order that Options functions run matters, an Ingress can't have the annotation and the Class set in the spec
+		delete(ingress.Annotations, "kubernetes.io/ingress.class")
+		ingress.Spec.IngressClassName = &className
+		return nil
+	}
+}
+
 func newIngressRule(host, path, serviceName string, port int32) networkingv1.IngressRule {
 	return networkingv1.IngressRule{
 		Host: strings.TrimSpace(host),

pkg/reconcilers/ingress_options_test.go

@@ -565,6 +565,141 @@ func Test_WithIngressRule(t *testing.T) {
 	}
 }
 
+func TestWithIngressClassName(t *testing.T) {
+	testCases := []struct {
+		name                  string
+		gsName                string
+		annotations           map[string]string
+		expected              string
+		wantErr               bool
+		wantEmptyIngressClass bool
+	}{
+		{
+			name:   "with octops ingress class annotation set to contour",
+			gsName: "simple-gameserver",
+			annotations: map[string]string{
+				gameserver.OctopsAnnotationIngressClassName: "contour",
+			},
+			expected:              "contour",
+			wantErr:               false,
+			wantEmptyIngressClass: false,
+		},
+		{
+			name:   "with legacy ingress class annotation set to contour",
+			gsName: "simple-gameserver",
+			annotations: map[string]string{
+				gameserver.OctopsAnnotationIngressClassNameLegacy: "contour",
+			},
+			expected:              "contour",
+			wantErr:               false,
+			wantEmptyIngressClass: false,
+		},
+		{
+			name:   "with legacy and standard ingress class annotations",
+			gsName: "simple-gameserver",
+			annotations: map[string]string{
+				gameserver.OctopsAnnotationIngressClassName:       "contour",
+				gameserver.OctopsAnnotationIngressClassNameLegacy: "nginx",
+			},
+			expected:              "contour",
+			wantErr:               false,
+			wantEmptyIngressClass: false,
+		},
+		{
+			name:                  "with no ingress class annotation",
+			gsName:                "simple-gameserver",
+			annotations:           map[string]string{},
+			expected:              "",
+			wantErr:               true,
+			wantEmptyIngressClass: true,
+		},
+		{
+			name:   "with empty ingress class annotation",
+			gsName: "simple-gameserver",
+			annotations: map[string]string{
+				gameserver.OctopsAnnotationIngressClassName: "",
+			},
+			expected:              "",
+			wantErr:               true,
+			wantEmptyIngressClass: true,
+		},
+		{
+			name:   "with empty legacy ingress class annotation",
+			gsName: "simple-gameserver",
+			annotations: map[string]string{
+				gameserver.OctopsAnnotationIngressClassNameLegacy: "",
+			},
+			expected:              "",
+			wantErr:               true,
+			wantEmptyIngressClass: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			gs := newGameServer(tc.gsName, "default", tc.annotations)
+
+			className := gameserver.GetIngressClassName(gs)
+			if tc.wantEmptyIngressClass {
+				require.Empty(t, className)
+			}
+
+			ingress, err := newIngress(gs, WithIngressClassName(className))
+			if tc.wantErr {
+				require.Error(t, err)
+				require.Nil(t, ingress)
+				require.Equal(t, errors.Errorf("annotation %s for %s must not be empty, check your Fleet or GameServer manifest.", gameserver.OctopsAnnotationIngressClassName, gs.Name).Error(), err.Error())
+			} else {
+				require.NoError(t, err)
+				require.NotNil(t, ingress)
+				require.Equal(t, tc.expected, *ingress.Spec.IngressClassName)
+			}
+		})
+	}
+}
+
+func TestIngressClassAnnotationNotPresent(t *testing.T) {
+	testCases := []struct {
+		name        string
+		gsName      string
+		annotations map[string]string
+		expected    map[string]string
+		err         bool
+	}{
+		{
+			name:   "kubernetes.io/class-domain is dropped",
+			gsName: "simple-gameserver",
+			annotations: map[string]string{
+				gameserver.OctopsAnnotationIngressClassName: "contour",
+			},
+			expected: map[string]string{},
+		},
+		{
+			name:   "kubernetes.io/class-domain is dropped when custom annotation is set",
+			gsName: "simple-gameserver",
+			annotations: map[string]string{
+				gameserver.OctopsAnnotationIngressClassNameLegacy: "contour",
+			},
+			expected: map[string]string{},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			gs := newGameServer(tc.gsName, "default", tc.annotations)
+
+			className := gameserver.GetIngressClassName(gs)
+			ingress, err := newIngress(gs, WithCustomAnnotations(), WithIngressClassName(className))
+			if tc.err {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, ingress.Annotations, tc.expected)
+			}
+		})
+	}
+}
+
 func newIngressRules(hosts, path, svcName string, port int32) []networkingv1.IngressRule {
 	var rules []networkingv1.IngressRule
 

pkg/reconcilers/ingress_reconciler.go

@@ -49,12 +49,14 @@ func (r *IngressReconciler) reconcileNotFound(ctx context.Context, gs *agonesv1.
 
 	mode := gameserver.GetIngressRoutingMode(gs)
 	issuer := gameserver.GetTLSCertIssuer(gs)
+	className := gameserver.GetIngressClassName(gs)
 
 	opts := []IngressOption{
 		WithCustomAnnotations(),
 		WithCustomAnnotationsTemplate(),
 		WithIngressRule(mode),
 		WithTLS(mode),
+		WithIngressClassName(className),
 	}
 
 	if issuer != "" {