chore: Convert deprecate host field to hosts field

Signed-off-by: Jocelyn Thode <jocelyn@thode.email>

CHANGELOG.md

@@ -15,11 +15,12 @@ This changelog keeps track of work items that have been completed and are ready
 
 ### Breaking Changes
 
-- **General**: Add multi-host support to `HTTPScaledObject` ([#552](https://github.com/kedacore/http-add-on/issues/552))
-
+- **General**: TODO ([#TODO](https://github.com/kedacore/http-add-on/issues/TODO))
 
 ### New
 
+- **General**: Add multi-host support to `HTTPScaledObject` ([#552](https://github.com/kedacore/http-add-on/issues/552))
+
 ### Improvements
 
 - **General**: Automatically tag Docker image with commit SHA ([#567](https://github.com/kedacore/http-add-on/issues/567))
@@ -37,7 +38,7 @@ You can find all deprecations in [this overview](https://github.com/kedacore/htt
 
 New deprecation(s):
 
-- TODO
+- **General**: `host` field deprecated in favor of `hosts` in `HTTPScaledObject` ([#552](https://github.com/kedacore/http-add-on/issues/552))
 
 Previously announced deprecation(s):
 

config/crd/bases/http.keda.sh_httpscaledobjects.yaml

@@ -59,10 +59,17 @@ spec:
           spec:
             description: HTTPScaledObjectSpec defines the desired state of HTTPScaledObject
             properties:
+              host:
+                description: (optional) (deprecated) The host to route. All requests
+                  with these hosts in the "Host" header will be routed to the Service
+                  and Port specified in the scaleTargetRef. The host field is mutually
+                  exclusive of the hosts field
+                type: string
               hosts:
-                description: The host to route. All requests with this host in the
-                  "Host" header will be routed to the Service and Port specified in
-                  the scaleTargetRef
+                description: (optional) The hosts to route. All requests with these
+                  hosts in the "Host" header will be routed to the Service and Port
+                  specified in the scaleTargetRef. The hosts field is mutually exclusive
+                  of the host field.
                 items:
                   type: string
                 type: array
@@ -109,7 +116,6 @@ spec:
                 format: int32
                 type: integer
             required:
-            - hosts
             - scaleTargetRef
             type: object
           status:

operator/apis/http/v1alpha1/httpscaledobject_types.go

@@ -40,9 +40,14 @@ type ReplicaStruct struct {
 
 // HTTPScaledObjectSpec defines the desired state of HTTPScaledObject
 type HTTPScaledObjectSpec struct {
-	// The hosts to route. All requests with these hosts in the "Host" header will
-	// be routed to the Service and Port specified in the scaleTargetRef
-	Hosts []string `json:"hosts"`
+	// (optional) (deprecated) The host to route. All requests with these hosts in the "Host" header will
+	// be routed to the Service and Port specified in the scaleTargetRef. The host field is mutually exclusive of the hosts field
+	// +optional
+	Host *string `json:"host,omitempty"`
+	// (optional) The hosts to route. All requests with these hosts in the "Host" header will
+	// be routed to the Service and Port specified in the scaleTargetRef. The hosts field is mutually exclusive of the host field.
+	// +optional
+	Hosts []string `json:"hosts,omitempty"`
 	// The name of the deployment to route HTTP requests to (and to autoscale).
 	// Either this or Image must be set
 	ScaleTargetRef *ScaleTargetRef `json:"scaleTargetRef"`

operator/controllers/http/app.go

@@ -80,18 +80,14 @@ func removeApplicationResources(
 		v1alpha1.AppScaledObjectTerminated,
 	))
 
-	if err := removeAndUpdateRoutingTable(
+	return removeAndUpdateRoutingTable(
 		ctx,
 		logger,
 		cl,
 		routingTable,
 		httpso.Spec.Hosts,
 		baseConfig.CurrentNamespace,
-	); err != nil {
-		return err
-	}
-
-	return nil
+	)
 }
 
 func createOrUpdateApplicationResources(
@@ -144,7 +140,7 @@ func createOrUpdateApplicationResources(
 		targetPendingReqs = *tpr
 	}
 
-	if err := addAndUpdateRoutingTable(
+	return addAndUpdateRoutingTable(
 		ctx,
 		logger,
 		cl,
@@ -158,8 +154,5 @@ func createOrUpdateApplicationResources(
 			targetPendingReqs,
 		),
 		baseConfig.CurrentNamespace,
-	); err != nil {
-		return err
-	}
-	return nil
+	)
 }

operator/controllers/http/httpscaledobject_controller.go

@@ -18,9 +18,11 @@ package http
 
 import (
 	"context"
+	"errors"
 	"time"
 
-	"k8s.io/apimachinery/pkg/api/errors"
+	"github.com/go-logr/logr"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -61,7 +63,7 @@ func (r *HTTPScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 	httpso := &httpv1alpha1.HTTPScaledObject{}
 	if err := r.Client.Get(ctx, req.NamespacedName, httpso); err != nil {
-		if errors.IsNotFound(err) {
+		if k8serrors.IsNotFound(err) {
 			// If the HTTPScaledObject wasn't found, it might have
 			// been deleted between the reconcile and the get.
 			// It'll automatically get garbage collected, so don't
@@ -108,6 +110,12 @@ func (r *HTTPScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, err
 	}
 
+	// ensure only host or hosts is set and if host is set that
+	// it is converted to hosts
+	if err := sanitizeHosts(logger, httpso); err != nil {
+		return ctrl.Result{}, err
+	}
+
 	// httpso is updated now
 	logger.Info(
 		"Reconciling HTTPScaledObject",
@@ -171,3 +179,28 @@ func (r *HTTPScaledObjectReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		For(&httpv1alpha1.HTTPScaledObject{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
 		Complete(r)
 }
+
+// sanitize hosts by converting the host definition to hosts are erroring
+// when both fields are set
+func sanitizeHosts(
+	logger logr.Logger,
+	httpso *httpv1alpha1.HTTPScaledObject,
+) error {
+	switch {
+	case httpso.Spec.Hosts != nil && httpso.Spec.Host != nil:
+		err := errors.New("mutually exclusive fields Error")
+		logger.Error(err, "Only one of 'hosts' or 'host' field can be defined")
+		return err
+	case httpso.Spec.Hosts == nil && httpso.Spec.Host == nil:
+		err := errors.New("no host specified Error")
+		logger.Error(err, "At least one of 'hosts' or 'host' field must be defined")
+		return err
+	case httpso.Spec.Hosts == nil:
+		httpso.Spec.Hosts = []string{*httpso.Spec.Host}
+		httpso.Spec.Host = nil
+		logger.Info("Using the 'host' field is deprecated. Please consider switching to the 'hosts' field")
+		return nil
+	default:
+		return nil
+	}
+}

operator/controllers/http/httpscaledobject_test.go

@@ -0,0 +1,65 @@
+package http
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSanitizeHostsWithOnlyHosts(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newCommonTestInfra("testns", "testapp")
+	spec := testInfra.httpso.Spec
+
+	r.NoError(sanitizeHosts(
+		testInfra.logger,
+		&testInfra.httpso,
+	))
+
+	r.Equal(spec.Hosts, testInfra.httpso.Spec.Hosts)
+	r.Nil(testInfra.httpso.Spec.Host)
+}
+
+func TestSanitizeHostsWithBothHostAndHosts(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newBrokenTestInfra("testns", "testapp")
+
+	err := sanitizeHosts(
+		testInfra.logger,
+		&testInfra.httpso,
+	)
+	r.Error(err)
+}
+
+func TestSanitizeHostsWithOnlyHost(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newDeprecatedTestInfra("testns", "testapp")
+	spec := testInfra.httpso.Spec
+
+	r.NoError(sanitizeHosts(
+		testInfra.logger,
+		&testInfra.httpso,
+	))
+
+	r.NotEqual(spec.Hosts, testInfra.httpso.Spec.Hosts)
+	r.NotEqual(spec.Host, testInfra.httpso.Spec.Host)
+	r.Nil(testInfra.httpso.Spec.Host)
+	r.Equal([]string{*spec.Host}, testInfra.httpso.Spec.Hosts)
+}
+
+func TestSanitizeHostsWithNoHostOrHosts(t *testing.T) {
+	r := require.New(t)
+
+	testInfra := newEmptyHostTestInfra("testns", "testapp")
+
+	err := sanitizeHosts(
+		testInfra.logger,
+		&testInfra.httpso,
+	)
+	r.Error(err)
+	r.Nil(testInfra.httpso.Spec.Host)
+	r.Nil(testInfra.httpso.Spec.Hosts)
+}

operator/controllers/http/suite_test.go

@@ -126,3 +126,112 @@ func newCommonTestInfra(namespace, appName string) *commonTestInfra {
 		httpso:  httpso,
 	}
 }
+
+func newBrokenTestInfra(namespace, appName string) *commonTestInfra {
+	localScheme := runtime.NewScheme()
+	utilruntime.Must(clientgoscheme.AddToScheme(localScheme))
+	utilruntime.Must(httpv1alpha1.AddToScheme(localScheme))
+	utilruntime.Must(kedav1alpha1.AddToScheme(localScheme))
+
+	ctx := context.Background()
+	cl := fake.NewClientBuilder().WithScheme(localScheme).Build()
+	logger := logr.Discard()
+
+	host := "myhost1.com"
+
+	httpso := httpv1alpha1.HTTPScaledObject{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      appName,
+		},
+		Spec: httpv1alpha1.HTTPScaledObjectSpec{
+			ScaleTargetRef: &httpv1alpha1.ScaleTargetRef{
+				Deployment: appName,
+				Service:    appName,
+				Port:       8081,
+			},
+			Hosts: []string{"myhost1.com", "myhost2.com"},
+			Host:  &host,
+		},
+	}
+
+	return &commonTestInfra{
+		ns:      namespace,
+		appName: appName,
+		ctx:     ctx,
+		cl:      cl,
+		logger:  logger,
+		httpso:  httpso,
+	}
+}
+
+func newDeprecatedTestInfra(namespace, appName string) *commonTestInfra {
+	localScheme := runtime.NewScheme()
+	utilruntime.Must(clientgoscheme.AddToScheme(localScheme))
+	utilruntime.Must(httpv1alpha1.AddToScheme(localScheme))
+	utilruntime.Must(kedav1alpha1.AddToScheme(localScheme))
+
+	ctx := context.Background()
+	cl := fake.NewClientBuilder().WithScheme(localScheme).Build()
+	logger := logr.Discard()
+
+	host := "myhost1.com"
+
+	httpso := httpv1alpha1.HTTPScaledObject{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      appName,
+		},
+		Spec: httpv1alpha1.HTTPScaledObjectSpec{
+			ScaleTargetRef: &httpv1alpha1.ScaleTargetRef{
+				Deployment: appName,
+				Service:    appName,
+				Port:       8081,
+			},
+			Host: &host,
+		},
+	}
+
+	return &commonTestInfra{
+		ns:      namespace,
+		appName: appName,
+		ctx:     ctx,
+		cl:      cl,
+		logger:  logger,
+		httpso:  httpso,
+	}
+}
+
+func newEmptyHostTestInfra(namespace, appName string) *commonTestInfra {
+	localScheme := runtime.NewScheme()
+	utilruntime.Must(clientgoscheme.AddToScheme(localScheme))
+	utilruntime.Must(httpv1alpha1.AddToScheme(localScheme))
+	utilruntime.Must(kedav1alpha1.AddToScheme(localScheme))
+
+	ctx := context.Background()
+	cl := fake.NewClientBuilder().WithScheme(localScheme).Build()
+	logger := logr.Discard()
+
+	httpso := httpv1alpha1.HTTPScaledObject{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      appName,
+		},
+		Spec: httpv1alpha1.HTTPScaledObjectSpec{
+			ScaleTargetRef: &httpv1alpha1.ScaleTargetRef{
+				Deployment: appName,
+				Service:    appName,
+				Port:       8081,
+			},
+		},
+	}
+
+	return &commonTestInfra{
+		ns:      namespace,
+		appName: appName,
+		ctx:     ctx,
+		cl:      cl,
+		logger:  logger,
+		httpso:  httpso,
+	}
+}

scaler/handlers.go

@@ -195,15 +195,14 @@ func (e *impl) GetMetrics(
 			e.routingTable,
 		)
 		if !ok {
-			if host == interceptor {
-				hostCount = e.pinger.aggregate()
-				metricName = interceptor
-			} else {
+			if host != interceptor {
 				err := fmt.Errorf("host '%s' not found in counts", host)
 				allCounts := e.pinger.mergeCountsWithRoutingTable(e.routingTable)
 				lggr.Error(err, "allCounts", allCounts)
 				return nil, err
 			}
+			hostCount = e.pinger.aggregate()
+			metricName = interceptor
 		}
 		totalCount += int64(hostCount)
 	}