connect: ingress gateway validation for http hosts and wildcards (#15749

)

* connect: fix non-"tcp" ingress gateway validation

changes apply to http, http2, and grpc:
* if "hosts" is excluded, consul will use its default domain
  e.g. <service-name>.ingress.dc1.consul
* can't set hosts with "*" service name
* test http2 and grpc too

.changelog/15749.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+connect: ingress http/2/grpc listeners may exclude hosts
+```

nomad/structs/services.go

@@ -1874,13 +1874,13 @@ func (s *ConsulIngressService) Validate(protocol string) error {
 		return nil
 	}
 
+	// pre-validate service Name and Hosts before passing along to consul:
+	// https://developer.hashicorp.com/consul/docs/connect/config-entries/ingress-gateway#services
+
 	if s.Name == "" {
 		return errors.New("Consul Ingress Service requires a name")
 	}
 
-	// Validation of wildcard service name and hosts varies depending on the
-	// protocol for the gateway.
-	// https://www.consul.io/docs/connect/config-entries/ingress-gateway#hosts
 	switch protocol {
 	case "tcp":
 		if s.Name == "*" {
@@ -1891,12 +1891,8 @@ func (s *ConsulIngressService) Validate(protocol string) error {
 			return errors.New(`Consul Ingress Service doesn't support associating hosts to a service for the "tcp" protocol`)
 		}
 	default:
-		if s.Name == "*" {
-			return nil
-		}
-
-		if len(s.Hosts) == 0 {
-			return fmt.Errorf("Consul Ingress Service requires one or more hosts when using %q protocol", protocol)
+		if s.Name == "*" && len(s.Hosts) != 0 {
+			return errors.New(`Consul Ingress Service with a wildcard "*" service name can not also specify hosts`)
 		}
 	}
 

nomad/structs/services_test.go

@@ -1325,52 +1325,63 @@ func TestConsulIngressService_Validate(t *testing.T) {
 		err := (&ConsulIngressService{
 			Name: "",
 		}).Validate("http")
-		require.EqualError(t, err, "Consul Ingress Service requires a name")
-	})
-
-	t.Run("http missing hosts", func(t *testing.T) {
-		err := (&ConsulIngressService{
-			Name: "service1",
-		}).Validate("http")
-		require.EqualError(t, err, `Consul Ingress Service requires one or more hosts when using "http" protocol`)
+		must.EqError(t, err, "Consul Ingress Service requires a name")
 	})
 
 	t.Run("tcp extraneous hosts", func(t *testing.T) {
 		err := (&ConsulIngressService{
 			Name:  "service1",
 			Hosts: []string{"host1"},
 		}).Validate("tcp")
-		require.EqualError(t, err, `Consul Ingress Service doesn't support associating hosts to a service for the "tcp" protocol`)
+		must.EqError(t, err, `Consul Ingress Service doesn't support associating hosts to a service for the "tcp" protocol`)
 	})
 
-	t.Run("ok tcp", func(t *testing.T) {
+	t.Run("tcp ok", func(t *testing.T) {
 		err := (&ConsulIngressService{
 			Name: "service1",
 		}).Validate("tcp")
-		require.NoError(t, err)
-	})
-
-	t.Run("ok http", func(t *testing.T) {
-		err := (&ConsulIngressService{
-			Name:  "service1",
-			Hosts: []string{"host1"},
-		}).Validate("http")
-		require.NoError(t, err)
-	})
-
-	t.Run("http with wildcard service", func(t *testing.T) {
-		err := (&ConsulIngressService{
-			Name: "*",
-		}).Validate("http")
-		require.NoError(t, err)
+		must.NoError(t, err)
 	})
 
 	t.Run("tcp with wildcard service", func(t *testing.T) {
 		err := (&ConsulIngressService{
 			Name: "*",
 		}).Validate("tcp")
-		require.EqualError(t, err, `Consul Ingress Service doesn't support wildcard name for "tcp" protocol`)
+		must.EqError(t, err, `Consul Ingress Service doesn't support wildcard name for "tcp" protocol`)
 	})
+
+	// non-"tcp" protocols should be all treated the same.
+	for _, proto := range []string{"http", "http2", "grpc"} {
+		t.Run(proto+" ok", func(t *testing.T) {
+			err := (&ConsulIngressService{
+				Name:  "service1",
+				Hosts: []string{"host1"},
+			}).Validate(proto)
+			must.NoError(t, err)
+		})
+
+		t.Run(proto+" without hosts", func(t *testing.T) {
+			err := (&ConsulIngressService{
+				Name: "service1",
+			}).Validate(proto)
+			must.NoError(t, err, must.Sprintf(`"%s" protocol should not require hosts`, proto))
+		})
+
+		t.Run(proto+" wildcard service", func(t *testing.T) {
+			err := (&ConsulIngressService{
+				Name: "*",
+			}).Validate(proto)
+			must.NoError(t, err, must.Sprintf(`"%s" protocol should allow wildcard service`, proto))
+		})
+
+		t.Run(proto+" wildcard service and host", func(t *testing.T) {
+			err := (&ConsulIngressService{
+				Name:  "*",
+				Hosts: []string{"any"},
+			}).Validate(proto)
+			must.EqError(t, err, `Consul Ingress Service with a wildcard "*" service name can not also specify hosts`)
+		})
+	}
 }
 
 func TestConsulIngressListener_Validate(t *testing.T) {