NET-5912/service-defaults protocol validation (#21593)

* fix: add validation for protocol field on service-defaults config entry

* test: update test cases with correct protocol

agent/config/builder.go

@@ -22,12 +22,13 @@ import (
 	"time"
 
 	"github.com/armon/go-metrics/prometheus"
+	"golang.org/x/time/rate"
+
 	"github.com/hashicorp/go-bexpr"
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/go-sockaddr/template"
 	"github.com/hashicorp/memberlist"
-	"golang.org/x/time/rate"
 
 	"github.com/hashicorp/consul/agent/cache"
 	"github.com/hashicorp/consul/agent/checks"
@@ -774,6 +775,7 @@ func (b *builder) build() (rt RuntimeConfig, err error) {
 			if err != nil {
 				return RuntimeConfig{}, fmt.Errorf("config_entries.bootstrap[%d]: %s", i, err)
 			}
+			// Ensure Normalize is called before Validate for accurate validation
 			if err := entry.Normalize(); err != nil {
 				return RuntimeConfig{}, fmt.Errorf("config_entries.bootstrap[%d]: %s", i, err)
 			}

agent/config_endpoint_test.go

@@ -612,7 +612,7 @@ func TestConfig_Apply_CAS(t *testing.T) {
 	{
 		"Kind": "service-defaults",
 		"Name": "foo",
-		"Protocol": "udp"
+		"Protocol": "http"
 	}
 	`))
 	req, _ = http.NewRequest("PUT", "/v1/config?cas=0", body)
@@ -628,7 +628,7 @@ func TestConfig_Apply_CAS(t *testing.T) {
 	{
 		"Kind": "service-defaults",
 		"Name": "foo",
-		"Protocol": "udp"
+		"Protocol": "http"
 	}
 	`))
 	req, _ = http.NewRequest("PUT", fmt.Sprintf("/v1/config?cas=%d", entry.GetRaftIndex().ModifyIndex), body)

agent/consul/config_endpoint.go

@@ -10,10 +10,11 @@ import (
 
 	metrics "github.com/armon/go-metrics"
 	"github.com/armon/go-metrics/prometheus"
+	hashstructure_v2 "github.com/mitchellh/hashstructure/v2"
+
 	"github.com/hashicorp/go-bexpr"
 	"github.com/hashicorp/go-hclog"
 	memdb "github.com/hashicorp/go-memdb"
-	hashstructure_v2 "github.com/mitchellh/hashstructure/v2"
 
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/configentry"
@@ -85,6 +86,7 @@ func (c *ConfigEntry) Apply(args *structs.ConfigEntryRequest, reply *bool) error
 	}
 
 	// Normalize and validate the incoming config entry as if it came from a user.
+	// Ensure Normalize is called before Validate for accurate validation
 	if err := args.Entry.Normalize(); err != nil {
 		return err
 	}

agent/consul/config_replication_test.go

@@ -6,11 +6,11 @@ package consul
 import (
 	"context"
 	"fmt"
-	"github.com/oklog/ulid/v2"
-	"github.com/stretchr/testify/assert"
 	"os"
 	"testing"
 
+	"github.com/oklog/ulid/v2"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/agent/structs"
@@ -129,7 +129,7 @@ func TestReplication_ConfigEntries(t *testing.T) {
 			Entry: &structs.ServiceConfigEntry{
 				Kind:     structs.ServiceDefaults,
 				Name:     fmt.Sprintf("svc-%d", i),
-				Protocol: "udp",
+				Protocol: "tcp",
 			},
 		}
 

agent/structs/config_entry.go

@@ -12,12 +12,11 @@ import (
 	"time"
 
 	"github.com/miekg/dns"
-
-	"github.com/hashicorp/go-multierror"
 	"github.com/mitchellh/hashstructure"
 	"github.com/mitchellh/mapstructure"
 
 	"github.com/hashicorp/consul-net-rpc/go-msgpack/codec"
+	"github.com/hashicorp/go-multierror"
 
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/cache"
@@ -269,6 +268,12 @@ func (e *ServiceConfigEntry) Validate() error {
 		validationErr = multierror.Append(validationErr, fmt.Errorf("invalid value for balance_inbound_connections: %v", e.BalanceInboundConnections))
 	}
 
+	switch e.Protocol {
+	case "", "http", "http2", "grpc", "tcp":
+	default:
+		validationErr = multierror.Append(validationErr, fmt.Errorf("invalid value for protocol: %v", e.Protocol))
+	}
+
 	// External endpoints are invalid with an existing service's upstream configuration
 	if e.UpstreamConfig != nil && e.Destination != nil {
 		validationErr = multierror.Append(validationErr, errors.New("UpstreamConfig and Destination are mutually exclusive for service defaults"))

agent/structs/config_entry_test.go

@@ -10,12 +10,12 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/hashicorp/hcl"
 	"github.com/mitchellh/copystructure"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul-net-rpc/go-msgpack/codec"
+	"github.com/hashicorp/hcl"
 
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/cache"
@@ -3225,6 +3225,14 @@ func TestServiceConfigEntry(t *testing.T) {
 			},
 			validateErr: `Invalid MutualTLSMode "invalid-mtls-mode". Must be one of "", "strict", or "permissive".`,
 		},
+		"validate: invalid Protocol in service-defaults": {
+			entry: &ServiceConfigEntry{
+				Kind:     ServiceDefaults,
+				Name:     "web",
+				Protocol: "blah",
+			},
+			validateErr: `invalid value for protocol: blah`,
+		},
 	}
 	testConfigEntryNormalizeAndValidate(t, cases)
 }

api/config_entry_test.go

@@ -104,7 +104,7 @@ func TestAPI_ConfigEntries(t *testing.T) {
 		service := &ServiceConfigEntry{
 			Kind:          ServiceDefaults,
 			Name:          "foo",
-			Protocol:      "udp",
+			Protocol:      "http",
 			MutualTLSMode: MutualTLSModeStrict,
 			Meta: map[string]string{
 				"foo": "bar",
@@ -124,7 +124,7 @@ func TestAPI_ConfigEntries(t *testing.T) {
 		service2 := &ServiceConfigEntry{
 			Kind:        ServiceDefaults,
 			Name:        "bar",
-			Protocol:    "tcp",
+			Protocol:    "http",
 			Destination: dest,
 		}
 
@@ -176,7 +176,7 @@ func TestAPI_ConfigEntries(t *testing.T) {
 		require.True(t, written)
 
 		// update no cas
-		service.Protocol = "http"
+		service.Protocol = "tcp"
 
 		_, wm, err = config_entries.Set(service, nil)
 		require.NoError(t, err)

command/config/write/config_write_test.go

@@ -43,7 +43,7 @@ func TestConfigWrite(t *testing.T) {
 		_, err := f.WriteString(`
       Kind = "service-defaults"
       Name = "web"
-      Protocol = "udp"
+      Protocol = "tcp"
       `)
 
 		require.NoError(t, err)
@@ -65,7 +65,7 @@ func TestConfigWrite(t *testing.T) {
 		require.True(t, ok)
 		require.Equal(t, api.ServiceDefaults, svc.Kind)
 		require.Equal(t, "web", svc.Name)
-		require.Equal(t, "udp", svc.Protocol)
+		require.Equal(t, "tcp", svc.Protocol)
 	})
 
 	t.Run("Stdin", func(t *testing.T) {
@@ -170,6 +170,27 @@ kind = "proxy-defaults"
 			`Config entry written: proxy-defaults/global`)
 		require.Equal(t, 0, code)
 	})
+
+	// Test that protocol field is first normalized and then validated
+	// before writing the config entry
+	t.Run("service defaults config entry mixed case in protocol field", func(t *testing.T) {
+		stdin := new(bytes.Buffer)
+		stdin.WriteString(`
+		Kind = "service-defaults"
+		Name = "web"
+		Protocol = "TcP"
+`)
+
+		ui := cli.NewMockUi()
+		c := New(ui)
+		c.testStdin = stdin
+
+		code := c.Run([]string{"-http-addr=" + a.HTTPAddr(), "-"})
+		require.Empty(t, ui.ErrorWriter.String())
+		require.Contains(t, ui.OutputWriter.String(),
+			`Config entry written: service-defaults/web`)
+		require.Equal(t, 0, code)
+	})
 }
 
 func TestConfigWrite_Warning(t *testing.T) {