From 10a4999a87783338beb0f3b628dcdc7ad60c14c5 Mon Sep 17 00:00:00 2001
From: Sarah Pratt <sarah.pratt@hashicorp.com>
Date: Tue, 26 Jul 2022 13:31:06 -0500
Subject: [PATCH] Separate port and socket path requirement in case of local
 agent assignment

---
 agent/agent_endpoint.go       |  6 +++---
 agent/sidecar_service_test.go |  2 +-
 agent/structs/structs.go      | 25 +++++++++++++++++++++----
 agent/structs/structs_test.go | 10 ++++++++++
 4 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go
index 11ec5f9ca7..b2d68e3044 100644
--- a/agent/agent_endpoint.go
+++ b/agent/agent_endpoint.go
@@ -1123,8 +1123,8 @@ func (s *HTTPHandlers) AgentRegisterService(resp http.ResponseWriter, req *http.
 		return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: fmt.Sprintf("Invalid Service Meta: %v", err)}
 	}
 
-	// Run validation. This is the same validation that would happen on
-	// the catalog endpoint so it helps ensure the sync will work properly.
+	// Run validation. This same validation would happen on the catalog endpoint,
+	// so it helps ensure the sync will work properly.
 	if err := ns.Validate(); err != nil {
 		return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: fmt.Sprintf("Validation failed: %v", err.Error())}
 	}
@@ -1164,7 +1164,7 @@ func (s *HTTPHandlers) AgentRegisterService(resp http.ResponseWriter, req *http.
 		return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: fmt.Sprintf("Invalid SidecarService: %s", err)}
 	}
 	if sidecar != nil {
-		if err := sidecar.Validate(); err != nil {
+		if err := sidecar.ValidateForAgent(); err != nil {
 			return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: fmt.Sprintf("Failed Validation: %v", err.Error())}
 		}
 		// Make sure we are allowed to register the sidecar using the token
diff --git a/agent/sidecar_service_test.go b/agent/sidecar_service_test.go
index a2ffe9af49..7ced9720a7 100644
--- a/agent/sidecar_service_test.go
+++ b/agent/sidecar_service_test.go
@@ -339,7 +339,7 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) {
 			}
 
 			ns := tt.sd.NodeService()
-			err := ns.Validate()
+			err := ns.ValidateForAgent()
 			require.NoError(t, err, "Invalid test case - NodeService must validate")
 
 			gotNS, gotChecks, gotToken, err := a.sidecarServiceFromNodeService(ns, tt.token)
diff --git a/agent/structs/structs.go b/agent/structs/structs.go
index 275bf4c18b..7a7bd93f5c 100644
--- a/agent/structs/structs.go
+++ b/agent/structs/structs.go
@@ -1411,6 +1411,27 @@ func (s *NodeService) IsGateway() bool {
 func (s *NodeService) Validate() error {
 	var result error
 
+	if s.Kind == ServiceKindConnectProxy {
+		if s.Port == 0 && s.SocketPath == "" {
+			result = multierror.Append(result, fmt.Errorf("Port or SocketPath must be set for a %s", s.Kind))
+		}
+	}
+
+	commonValidation := s.ValidateForAgent()
+	if commonValidation != nil {
+		result = multierror.Append(result, commonValidation)
+	}
+
+	return result
+}
+
+// ValidateForAgent does a subset validation, with the assumption that a local agent can assist with missing values.
+//
+// I.e. in the catalog case, a local agent cannot be assumed to facilitate auto-assignment of port or socket path,
+// so additional checks are needed.
+func (s *NodeService) ValidateForAgent() error {
+	var result error
+
 	// TODO(partitions): remember to double check that this doesn't cross partition boundaries
 
 	// ConnectProxy validation
@@ -1426,10 +1447,6 @@ func (s *NodeService) Validate() error {
 					"services"))
 		}
 
-		if s.Port == 0 && s.SocketPath == "" {
-			result = multierror.Append(result, fmt.Errorf("Port or SocketPath must be set for a %s", s.Kind))
-		}
-
 		if s.Connect.Native {
 			result = multierror.Append(result, fmt.Errorf(
 				"A Proxy cannot also be Connect Native, only typical services"))
diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go
index 0b6efb3309..87402a2d4f 100644
--- a/agent/structs/structs_test.go
+++ b/agent/structs/structs_test.go
@@ -1157,6 +1157,16 @@ func TestStructs_NodeService_ValidateConnectProxy(t *testing.T) {
 	}
 }
 
+func TestStructs_NodeService_ValidateConnectProxyWithAgentAutoAssign(t *testing.T) {
+	t.Run("connect-proxy: no port set", func(t *testing.T) {
+		ns := TestNodeServiceProxy(t)
+		ns.Port = 0
+
+		err := ns.ValidateForAgent()
+		assert.NoError(t, err)
+	})
+}
+
 func TestStructs_NodeService_ValidateConnectProxy_In_Partition(t *testing.T) {
 	cases := []struct {
 		Name   string