Repository: Kong/kubernetes-ingress-controller
Pull request #5399 (2024-01-04 19:59:23)

fix(annotations) allow WebSocket protocols
**What this PR does / why we need it**:

Accept the `wss` and `ws` protocols in the protocol annotation value filter. Enterprise added these at some point (see FT-2127 internally) for additional plugin capabilities (`https` and `http` accept WebSocket traffic also, but can't inspect the stream after the initial Upgrade request and 101 response) but we never updated the filter.

**Which issue this PR fixes**:

Reported by field teams in chat. Attempting to use `ws` or `wss` instead gets you a route with the default `http,https` in 3.0 and older.

**Special notes for your reviewer**:

Test tools for WebSockets seem annoyingly uncommon. There's not really an httpbin equivalent that I could find, so we'd probably need to add functionality to our echo server to do integration tests. Unit tests are probably fine for the filter itself, but it'd be nice to test WebSocket functionality in general. For manual testing, I built an image and created 
[wss-example.yaml.txt](https://github.com/Kong/kubernetes-ingress-controller/files/13834849/wss-example.yaml.txt) (https://websocket.org/tools/websocket-echo-server is a hosted echo service) and used https://www.npmjs.com/package/wscat to connect to that ExternalName Service.

As noted in the override comment, the way we handle route object parents, parent info in kongstate doesn't include the original object and thus can't use the failures system. Validating protocols is easy enough earlier in the translators, but it'd be nice to do it all in one place. I only validated Ingress and HTTPRoute there, since WebSockets are the odd protocol out that doesn't have a dedicated GWAPI resource type.

**PR Readiness Checklist**:

Complete these before marking the PR as `ready to review`:

- [x] the `CHANGELOG.md` release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) Report
Attention: `14 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`558dd93`)](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/commit/558dd93e5f4bb45d843001a81e738cfc8e98252e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) 69.5% compared to head [(`8076307`)](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) 67.2%.
> Report is 2 commits behind head on main.

> :exclamation: Current head 8076307 differs from pull request most recent head b246646. Consider uploading reports for the commit b246646 to get more accurate results

| [Files](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | Patch % | Lines |
|---|---|---|
| [...rnal/dataplane/translator/subtranslator/ingress.go](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#diff-aW50ZXJuYWwvZGF0YXBsYW5lL3RyYW5zbGF0b3Ivc3VidHJhbnNsYXRvci9pbmdyZXNzLmdv) | 14.2% | [5 Missing and 1 partial :warning: ](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) |
| [internal/dataplane/kongstate/kongstate.go](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#diff-aW50ZXJuYWwvZGF0YXBsYW5lL2tvbmdzdGF0ZS9rb25nc3RhdGUuZ28=) | 66.6% | [3 Missing and 1 partial :warning: ](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) |
| [...ternal/dataplane/translator/translate\_httproute.go](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#diff-aW50ZXJuYWwvZGF0YXBsYW5lL3RyYW5zbGF0b3IvdHJhbnNsYXRlX2h0dHByb3V0ZS5nbw==) | 0.0% | [3 Missing and 1 partial :warning: ](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) |

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##            main   #5399     +/-   ##
=======================================
- Coverage   69.5%   67.2%   -2.4%     
=======================================
  Files        174     174             
  Lines      21934   21955     +21     
=======================================
- Hits       15258   14756    -502     
- Misses      5780    6292    +512     
- Partials     896     907     +11     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/Kong/kubernetes-ingress-controller/pull/5399?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong).

rainest: > Could we add some tests on the translation failures?

Yes! And also no, because I refactored the whole route source part to not validate in there.

Handling these in the translators was somewhat annoying to begin with, and got more annoying trying to test them. The tests were already kinda messy due to the split traditional/expressions paths, and while https://github.com/rainest/kubernetes-ingress-controller/commits/fix/allow-ws/ works, but it's a lot of additional code for a single test case.

We don't need to transform or cross-reference anything to validate the protocols--they're already present in their final and complete form at  resource creation time--so admission probably works better for this, and it's much simpler to implement. We already validate Ingress and HTTPRoute anyway, so adding the check there is pretty low impact.

Services would require an additional kind validator and we can't filter relevant ones up front (though we could maybe assume anything with one of our vendor annotations is fair game), so those remain in the translator.

> Also please add integration tests for websocket usage (possibly in another PR).

I avoided this for lack of a readily available local test server and because we don't really have any WebSocket-specific translation. All we're doing is passing the `konghq.com/protocols` value to the proxy verbatim; everything after is the proxy's responsibility. Integration tests on our end would be more pressing if GWAPI had a WebSocketRoute or whatever, but I wouldn't expect them to add one--AFAIK there's not really any additional routing configuration you could add atop the HTTP base like gRPC does. Absent that, it'd be nice to have. Fixing the filter and making the feedback not silent is a more immediate need.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 570dd36e7..5d33ffa79 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -153,6 +153,8 @@ Adding a new version? You'll need three changes:
 - Allow configuring a GRPCRoute without hostnames and matches that catch all
   requests.
   [#5303](https://github.com/Kong/kubernetes-ingress-controller/pull/5303)
+- Allow the `ws` and `wss` Enterprise protocol values for protocol annotations.
+  [#5399](https://github.com/Kong/kubernetes-ingress-controller/pull/5399)
 
 ### Changed
 
diff --git a/internal/admission/validation/gateway/httproute.go b/internal/admission/validation/gateway/httproute.go
index dde0fa8f5..5830228b8 100644
--- a/internal/admission/validation/gateway/httproute.go
+++ b/internal/admission/validation/gateway/httproute.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/kong/go-kong/kong"
 
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/admission/validation"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator/subtranslator"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
@@ -37,6 +38,10 @@ func ValidateHTTPRoute(
 		return false, fmt.Sprintf("HTTPRoute spec did not pass validation: %s", err), nil
 	}
 
+	if err := validation.ValidateRouteSourceAnnotations(httproute); err != nil {
+		return false, fmt.Sprintf("HTTPRoute has invalid Kong annotations: %s", err), nil
+	}
+
 	// perform Gateway validations for the HTTPRoute (e.g. listener validation, namespace validation, e.t.c.)
 	for _, gateway := range attachedGateways {
 		// TODO: validate that the namespace is supported by the linked Gateway objects
diff --git a/internal/admission/validation/ingress/ingress.go b/internal/admission/validation/ingress/ingress.go
index 61d355f1e..9b7a8a3d4 100644
--- a/internal/admission/validation/ingress/ingress.go
+++ b/internal/admission/validation/ingress/ingress.go
@@ -9,6 +9,7 @@ import (
 	"github.com/kong/go-kong/kong"
 	netv1 "k8s.io/api/networking/v1"
 
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/admission/validation"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/failures"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator/subtranslator"
@@ -32,6 +33,11 @@ func ValidateIngress(
 		errMsgs           []string
 		failuresCollector = failures.NewResourceFailuresCollector(logger)
 	)
+
+	if err := validation.ValidateRouteSourceAnnotations(ingress); err != nil {
+		return false, fmt.Sprintf("Ingress has invalid Kong annotations: %s", err), nil
+	}
+
 	for _, kg := range ingressToKongRoutesForValidation(translatorFeatures, ingress, failuresCollector, storer) {
 		kg := kg
 		// Validate by using feature of Kong Gateway.
diff --git a/internal/admission/validation/routelike.go b/internal/admission/validation/routelike.go
new file mode 100644
index 000000000..a0fffffcf
--- /dev/null
+++ b/internal/admission/validation/routelike.go
@@ -0,0 +1,20 @@
+package validation
+
+import (
+	"fmt"
+
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
+)
+
+func ValidateRouteSourceAnnotations(obj client.Object) error {
+	protocols := annotations.ExtractProtocolNames(obj.GetAnnotations())
+	for _, protocol := range protocols {
+		if !util.ValidateProtocol(protocol) {
+			return fmt.Errorf("invalid %s value: %s", annotations.AnnotationPrefix+annotations.ProtocolsKey, protocol)
+		}
+	}
+	return nil
+}
diff --git a/internal/dataplane/kongstate/kongstate.go b/internal/dataplane/kongstate/kongstate.go
index 18290319b..64a8ad0a3 100644
--- a/internal/dataplane/kongstate/kongstate.go
+++ b/internal/dataplane/kongstate/kongstate.go
@@ -193,6 +193,13 @@ func (ks *KongState) FillConsumerGroups(_ logr.Logger, s store.Storer) {
 	}
 }
 
+// servicesAsObjects returns a corev1.Service as a client.Object. It's used as a helper with lo.Map to return something
+// acceptable to functions that accept multiple client.Objects, since simply expanding the Service slice results in a
+// compiler error.
+func servicesAsObjects(svc *corev1.Service, _ int) client.Object {
+	return svc
+}
+
 func (ks *KongState) FillOverrides(
 	logger logr.Logger,
 	s store.Storer,
@@ -200,10 +207,20 @@ func (ks *KongState) FillOverrides(
 ) {
 	for i := 0; i < len(ks.Services); i++ {
 		// Services
-		ks.Services[i].override()
+		if err := ks.Services[i].override(); err != nil {
+			servicesGroup := lo.Values(ks.Services[i].K8sServices)
+			failuresCollector.PushResourceFailure(err.Error(), lo.Map(servicesGroup, servicesAsObjects)...)
+		}
 
 		// Routes
 		for j := 0; j < len(ks.Services[i].Routes); j++ {
+			// Routes, opposed to Services, are not validated in their override method therefore we have no error check here.
+			// Routes nested under Services here do not include their original parent object info in kongstate. Translators
+			// convert this into a util.K8sObjectInfo, which includes name/namespace/GVK as strings. Unfortunately we can't
+			// really convert this into a client.Object for use with the failures collector, and plumbing the original object
+			// down into the kongstate.Route copy looked a bit annoying. Protocol validation for routes instead lives in the
+			// HTTPRoute and Ingress translators (these may override to ws/wss, whereas the others are expected to derive
+			// their protcol from the resource type alone).
 			ks.Services[i].Routes[j].override(logger)
 		}
 	}
@@ -218,7 +235,6 @@ func (ks *KongState) FillUpstreamOverrides(
 ) {
 	for i := 0; i < len(ks.Upstreams); i++ {
 		servicesGroup := lo.Values(ks.Upstreams[i].Service.K8sServices)
-		servicesAsObjects := func(svc *corev1.Service, _ int) client.Object { return svc }
 
 		// In case `konghq.com/override` annotation is set on any of the services, we should log a deprecation error.
 		maybeLogKongIngressDeprecationError(logger, servicesGroup)
diff --git a/internal/dataplane/kongstate/service.go b/internal/dataplane/kongstate/service.go
index ae74b493c..de13c34e6 100644
--- a/internal/dataplane/kongstate/service.go
+++ b/internal/dataplane/kongstate/service.go
@@ -1,6 +1,7 @@
 package kongstate
 
 import (
+	"fmt"
 	"sort"
 	"strconv"
 	"strings"
@@ -160,9 +161,9 @@ func (s *Service) overrideByAnnotation(anns map[string]string) {
 }
 
 // override sets Service fields using Kubernetes Service annotations.
-func (s *Service) override() {
+func (s *Service) override() error {
 	if s == nil {
-		return
+		return nil
 	}
 
 	// Apply overrides from Kubernetes Service annotations. As we keep them in a map, let's first sort its keys to ensure
@@ -172,10 +173,15 @@ func (s *Service) override() {
 	for _, serviceName := range servicesNames {
 		svc := s.K8sServices[serviceName]
 		s.overrideByAnnotation(svc.Annotations)
+		protocol := annotations.ExtractProtocolName(svc.Annotations)
+		if !util.ValidateProtocol(protocol) {
+			return fmt.Errorf("%s annotation has invalid value: %s", annotations.AnnotationPrefix+annotations.ProtocolKey, protocol)
+		}
 	}
 
 	if s.Protocol != nil && (*s.Protocol == "grpc" || *s.Protocol == "grpcs") {
 		// grpc(s) doesn't accept a path
 		s.Path = nil
 	}
+	return nil
 }
diff --git a/internal/util/protocol.go b/internal/util/protocol.go
index 37c5dbded..8c65047a7 100644
--- a/internal/util/protocol.go
+++ b/internal/util/protocol.go
@@ -4,8 +4,11 @@ import "regexp"
 
 // ValidateProtocol returns a bool of whether string is a valid protocol.
 func ValidateProtocol(protocol string) bool {
+	if protocol == "" {
+		return true
+	}
 	match := validProtocols.MatchString(protocol)
 	return match
 }
 
-var validProtocols = regexp.MustCompile(`\Ahttps$|\Ahttp$|\Agrpc$|\Agrpcs|\Atcp|\Atls|\Atls_passthrough$`)
+var validProtocols = regexp.MustCompile(`\Ahttps$|\Ahttp$|\Agrpc$|\Agrpcs|\Aws$|\Awss|\Atcp|\Atls|\Atls_passthrough$`)


Test patch:
diff --git a/internal/admission/validation/gateway/httproute_test.go b/internal/admission/validation/gateway/httproute_test.go
index 7ef0ebd87..3dfcd2541 100644
--- a/internal/admission/validation/gateway/httproute_test.go
+++ b/internal/admission/validation/gateway/httproute_test.go
@@ -10,6 +10,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
 )
@@ -539,6 +540,47 @@ func TestValidateHTTPRoute(t *testing.T) {
 			valid:         false,
 			validationMsg: "HTTPRoute spec did not pass validation: rules[0].backendRefs[0]: filters in backendRef is unsupported",
 		},
+		{
+			msg: "invalid protocols",
+			route: &gatewayapi.HTTPRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: corev1.NamespaceDefault,
+					Name:      "testing-httproute",
+					Annotations: map[string]string{
+						annotations.AnnotationPrefix + annotations.ProtocolsKey: "ohno",
+					},
+				},
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: gatewayapi.CommonRouteSpec{
+						ParentRefs: []gatewayapi.ParentReference{{
+							Name: "testing-gateway",
+						}},
+					},
+					Rules: []gatewayapi.HTTPRouteRule{},
+				},
+			},
+			gateways: []*gatewayapi.Gateway{{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: corev1.NamespaceDefault,
+					Name:      "testing-gateway",
+				},
+				Spec: gatewayapi.GatewaySpec{
+					Listeners: []gatewayapi.Listener{{
+						Name:     "http",
+						Port:     80,
+						Protocol: (gatewayapi.HTTPProtocolType),
+						AllowedRoutes: &gatewayapi.AllowedRoutes{
+							Kinds: []gatewayapi.RouteGroupKind{{
+								Group: &group,
+								Kind:  "HTTPRoute",
+							}},
+						},
+					}},
+				},
+			}},
+			valid:         false,
+			validationMsg: "HTTPRoute has invalid Kong annotations: invalid konghq.com/protocols value: ohno",
+		},
 	} {
 		t.Run(tt.msg, func(t *testing.T) {
 			// Passed routesValidator is irrelevant for the above test cases.
diff --git a/internal/admission/validation/ingress/ingress_test.go b/internal/admission/validation/ingress/ingress_test.go
new file mode 100644
index 000000000..a6986eba1
--- /dev/null
+++ b/internal/admission/validation/ingress/ingress_test.go
@@ -0,0 +1,71 @@
+package ingress
+
+import (
+	"context"
+	"testing"
+
+	"github.com/go-logr/zapr"
+	"github.com/kong/go-kong/kong"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+	corev1 "k8s.io/api/core/v1"
+	netv1 "k8s.io/api/networking/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
+)
+
+func TestValidateIngress(t *testing.T) {
+	for _, tt := range []struct {
+		msg           string
+		ingress       *netv1.Ingress
+		valid         bool
+		validationMsg string
+		err           error
+	}{
+		{
+			msg: "invalid protocols",
+			ingress: &netv1.Ingress{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: corev1.NamespaceDefault,
+					Name:      "testing",
+					Annotations: map[string]string{
+						annotations.AnnotationPrefix + annotations.ProtocolsKey: "ohno",
+					},
+				},
+			},
+			valid:         false,
+			validationMsg: "Ingress has invalid Kong annotations: invalid konghq.com/protocols value: ohno",
+		},
+	} {
+		t.Run(tt.msg, func(t *testing.T) {
+			logger := zapr.NewLogger(zap.NewNop())
+			fakestore, err := store.NewFakeStore(store.FakeObjects{
+				IngressesV1: []*netv1.Ingress{
+					tt.ingress,
+				},
+			})
+			require.NoError(t, err)
+			valid, validMsg, err := ValidateIngress(
+				context.Background(),
+				mockRoutesValidator{},
+				translator.FeatureFlags{},
+				tt.ingress,
+				logger,
+				fakestore,
+			)
+			assert.Equal(t, tt.valid, valid, tt.msg)
+			assert.Equal(t, tt.validationMsg, validMsg, tt.msg)
+			assert.Equal(t, tt.err, err, tt.msg)
+		})
+	}
+}
+
+type mockRoutesValidator struct{}
+
+func (mockRoutesValidator) Validate(_ context.Context, _ *kong.Route) (bool, string, error) {
+	return true, "", nil
+}
diff --git a/internal/dataplane/kongstate/kongstate_test.go b/internal/dataplane/kongstate/kongstate_test.go
index bf2a7b2c9..ac5ef261a 100644
--- a/internal/dataplane/kongstate/kongstate_test.go
+++ b/internal/dataplane/kongstate/kongstate_test.go
@@ -35,6 +35,11 @@ var kongConsumerTypeMeta = metav1.TypeMeta{
 	Kind:       "KongConsumer",
 }
 
+var serviceTypeMeta = metav1.TypeMeta{
+	APIVersion: "v1",
+	Kind:       "Service",
+}
+
 func TestKongState_SanitizedCopy(t *testing.T) {
 	testedFields := sets.New[string]()
 	for _, tt := range []struct {
@@ -779,10 +784,10 @@ func TestFillConsumersAndCredentials(t *testing.T) {
 				KongConsumers: tc.k8sConsumers,
 			})
 			logger := zapr.NewLogger(zap.NewNop())
-			failureCollector := failures.NewResourceFailuresCollector(logger)
+			failuresCollector := failures.NewResourceFailuresCollector(logger)
 
 			state := KongState{}
-			state.FillConsumersAndCredentials(logger, store, failureCollector)
+			state.FillConsumersAndCredentials(logger, store, failuresCollector)
 			// compare translated consumers.
 			require.Len(t, state.Consumers, len(tc.expectedKongStateConsumers))
 			// compare fields. Since we only test for translating a single consumer, we only compare the first one if exists.
@@ -796,7 +801,7 @@ func TestFillConsumersAndCredentials(t *testing.T) {
 			}
 			// check for expected translation failures.
 			if len(tc.expectedTranslationFailureMessages) > 0 {
-				translationFailures := failureCollector.PopResourceFailures()
+				translationFailures := failuresCollector.PopResourceFailures()
 				for nsName, expectedMessage := range tc.expectedTranslationFailureMessages {
 					relatedFailures := lo.Filter(translationFailures, func(f failures.ResourceFailure, _ int) bool {
 						for _, obj := range f.CausingObjects() {
@@ -1307,3 +1312,97 @@ func TestFillVaults(t *testing.T) {
 		})
 	}
 }
+
+func TestFillOverrides_ServiceFailures(t *testing.T) {
+	tests := []struct {
+		name                               string
+		state                              *KongState
+		want                               Service
+		expectedTranslationFailureMessages map[k8stypes.NamespacedName]string
+	}{
+		{
+			name: "service protocol set to valid value",
+			state: &KongState{
+				Services: []Service{
+					{
+						Namespace: "default",
+						K8sServices: map[string]*corev1.Service{
+							"test": {
+								TypeMeta: serviceTypeMeta,
+								ObjectMeta: metav1.ObjectMeta{
+									Name:      "test",
+									Namespace: "default",
+									Annotations: map[string]string{
+										"konghq.com/protocol": "wss",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			want: Service{
+				Service: kong.Service{
+					Protocol: kong.String("wss"),
+				},
+			},
+		},
+		{
+			name: "service protocol set to invalid value",
+			state: &KongState{
+				Services: []Service{
+					{
+						Namespace: "default",
+						K8sServices: map[string]*corev1.Service{
+							"test": {
+								TypeMeta: serviceTypeMeta,
+								ObjectMeta: metav1.ObjectMeta{
+									Name:      "test",
+									Namespace: "default",
+									Annotations: map[string]string{
+										"konghq.com/protocol": "djnfkgjfgn",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			want: Service{
+				Service: kong.Service{
+					Protocol: kong.String("http"),
+				},
+			},
+			expectedTranslationFailureMessages: map[k8stypes.NamespacedName]string{
+				{Namespace: "default", Name: "test"}: "konghq.com/protocol annotation has invalid value: djnfkgjfgn",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			store, err := store.NewFakeStore(store.FakeObjects{})
+			require.NoError(t, err)
+			logger := zapr.NewLogger(zap.NewNop())
+			failuresCollector := failures.NewResourceFailuresCollector(logger)
+			tt.state.FillOverrides(logger, store, failuresCollector)
+			if len(tt.expectedTranslationFailureMessages) > 0 {
+				translationFailures := failuresCollector.PopResourceFailures()
+				for nsName, expectedMessage := range tt.expectedTranslationFailureMessages {
+					relatedFailures := lo.Filter(translationFailures, func(f failures.ResourceFailure, _ int) bool {
+						for _, obj := range f.CausingObjects() {
+							if obj.GetNamespace() == nsName.Namespace && obj.GetName() == nsName.Name {
+								return true
+							}
+						}
+						return false
+					})
+
+					assert.Truef(t, lo.ContainsBy(relatedFailures, func(f failures.ResourceFailure) bool {
+						return strings.Contains(f.Message(), expectedMessage)
+					}), "should find expected translation failure caused by Service %s: should contain '%s'",
+						nsName.String(), expectedMessage)
+				}
+			}
+		})
+	}
+}
diff --git a/internal/dataplane/kongstate/service_test.go b/internal/dataplane/kongstate/service_test.go
index dbe03dfdb..bbb246405 100644
--- a/internal/dataplane/kongstate/service_test.go
+++ b/internal/dataplane/kongstate/service_test.go
@@ -186,7 +186,7 @@ func TestOverrideService(t *testing.T) {
 func TestNilServiceOverrideDoesntPanic(t *testing.T) {
 	require.NotPanics(t, func() {
 		var nilService *Service
-		nilService.override()
+		nilService.override() //nolint:errcheck
 	})
 }
 
@@ -527,6 +527,6 @@ func TestServiceOverride_DeterministicOrderWhenMoreThan1KubernetesService(t *tes
 
 	// We expect default/service-3 to be the last one to be processed effectively overriding the previous annotations.
 	const expectedRetries = 3
-	service.override()
+	require.NoError(t, service.override())
 	require.Equal(t, expectedRetries, *service.Service.Retries)
 }
diff --git a/internal/util/protocol_test.go b/internal/util/protocol_test.go
index 5edc34a2a..86ddfb154 100644
--- a/internal/util/protocol_test.go
+++ b/internal/util/protocol_test.go
@@ -12,10 +12,13 @@ func TestValidateProtocol(t *testing.T) {
 		input  string
 		result bool
 	}{
+		{"", true},
 		{"http", true},
 		{"https", true},
 		{"grpc", true},
 		{"grpcs", true},
+		{"ws", true},
+		{"wss", true},
 		{"tls", true},
 		{"tcp", true},
 		{"tls_passthrough", true},