diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index 11ec5f9ca..b2d68e304 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 a2ffe9af4..7ced9720a 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 275bf4c18..7a7bd93f5 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 0b6efb330..87402a2d4 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