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

hashicorp · Aug 23, 2024 · 26079f0 · 26079f0
1 parent 0e47b38
commit 26079f0
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 5 deletions.
diff --git a/agent/config/builder.go b/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)
 			}

diff --git a/agent/consul/config_endpoint.go b/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
 	}

diff --git a/agent/structs/config_entry.go b/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"))

diff --git a/agent/structs/config_entry_test.go b/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)
 }

diff --git a/command/config/write/config_write_test.go b/command/config/write/config_write_test.go
@@ -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) {