Skip to content

Commit

Permalink
Separate port and socket path requirement in case of local agent assi…
Browse files Browse the repository at this point in the history
…gnment
  • Loading branch information
skpratt committed Jul 29, 2022
1 parent af04851 commit 10a4999
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 8 deletions.
6 changes: 3 additions & 3 deletions agent/agent_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())}
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion agent/sidecar_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 21 additions & 4 deletions agent/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"))
Expand Down
10 changes: 10 additions & 0 deletions agent/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 10a4999

Please sign in to comment.