Validate port on mesh service registration #12881
Conversation
| result = multierror.Append(result, fmt.Errorf( | ||
| "Connect native service must have non-zero port")) | ||
| } | ||
|
|
||
| return result | ||
| } |
There was a problem hiding this comment.
Note: This Validate() helper is used by both the catalog and agent endpoints.
| @@ -1333,9 +1333,8 @@ func (s *NodeService) Validate() error { | |||
| "services")) | |||
| } | |||
|
|
|||
| if s.Port == 0 && s.SocketPath == "" { | |||
There was a problem hiding this comment.
This makes me think that we should require one of port or socket path everywhere.
There was a problem hiding this comment.
Added. Let me know if it is not the case for gateway types - in which case I will remove the sockpath validation below.
There was a problem hiding this comment.
Removed for gateway types per our discussion.
agent/consul/catalog_endpoint.go
Outdated
| @@ -204,6 +204,10 @@ func servicePreApply(service *structs.NodeService, authz ACLResolveResult, authz | |||
| } | |||
| } | |||
|
|
|||
| if service.Connect.SidecarService != nil && service.Connect.SidecarService.Port == 0 { | |||
There was a problem hiding this comment.
This will generally never be hit because SidecarService will always be nil.
consul/agent/agent_endpoint_test.go
Lines 3649 to 3652 in a668c36
So I think the better validation would be to check the top level Port (or I guess SocketPath) to ensure one of those is filled in for services with kind connect-proxy.
There was a problem hiding this comment.
Removed this check.
riddhi89
force-pushed
the
validate-port-service-mesh
branch
from
3439325
to
9ee5ed6
Compare
Without a port in the registration, the mesh is not functional. For agentless, since there will not be any client agents for auto port assignment, require a port to be specified on registration of a mesh service (connect proxy, gateway types, sidecar definitions, connect native services).
Move the check for the side car service having a port specified only for the catalog endpoint.
riddhi89
force-pushed
the
validate-port-service-mesh
branch
from
9ee5ed6
to
3e7aa17
Compare
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/662637. |
Description
Without a port in the registration, the mesh is not functional. For Agentless, there will not be any client agent for auto port assignment. Agentless phase 1 is for k8s workloads which are already able to specify a port on mesh service registration.
This PR change is to require a port to be specified on registration of a mesh service (connect proxy, all gateway types (except ingress), sidecar definitions, connect native services) - this is mostly the case already except the validation was missing for connect native services.
Centralized port allocation by servers would be done in the future to support other workloads (example: VM based) that depend on port assignment.