Ui Service Dump Gateway Enhancements #8411
Conversation
|
Overall, I do like this better than the way I dealt with it in my PR! 👍 |
agent/consul/internal_endpoint.go
Outdated
|
|
||
| reply.Index, reply.Nodes = index, nodes | ||
| if err := m.srv.filterACL(args.Token, reply); err != nil { | ||
| if err := m.srv.filterACL(args.Token, reply.Nodes); err != nil { |
There was a problem hiding this comment.
Something weird is happening with the ACL filtering, causing a test to fail? https://app.circleci.com/pipelines/github/hashicorp/consul/11574/workflows/99efb4ba-e98f-4889-919e-26d98efab9a2/jobs/216919
There was a problem hiding this comment.
Yep, it needed a pointer: https://github.com/hashicorp/consul/blob/master/agent/consul/acl.go#L1843
|
Should this line be updated to consul/agent/proxycfg/state.go Line 759 in de0b574 I see tests passing locally, but I would have expected something to fail here. 🤔 |
|
@crhino that was certainly making integration tests fail, fixed. |
|
Ah, merging #8704 caused conflicts with this, should be easy to rebase on top of |
agent/ui_endpoint.go
Outdated
| @@ -199,18 +200,22 @@ RPC: | |||
| return nil, err | |||
| } | |||
|
|
|||
| return summarizeServices(out.Dump, s.agent.config), nil | |||
There was a problem hiding this comment.
The more /v1/internal/ui/gateway-services-nodes/<name> and /v1/internal/ui/services diverge, the more awkward it feels using summarizeServices to account for both. I got confused because I was accidentally testing out /v1/internal/ui/gateway-services-nodes/gateway-name and ConnectedWithGateway/ConnectedWithProxy were false for services that did have proxies and gateways. Eventually I realized that endpoint does not pass in a list of gateways here, which accounts for that difference. Definitely feels a bit weird though.
Perhaps we can put a TODO here about refactoring this to make the two endpoints more separate? Or somehow cleaning up the confusion of the output?
There was a problem hiding this comment.
I'm about to add a 3rd summary use-case in another PR 🙈 .
I'll address it there.
This PR adds some enhancements needed for additional UI functionality related to gateways:
ConnectedWithProxyandConnectedWithGatewaytrack whether a service is proxied by a gateway or Connect proxy.GatewayConfig.AssociatedServiceCounttracks the number of service names associated with a gatewayThe main change is that
Internal.ServiceDumpnow returns a dump of thegateway-servicestable, and we join the information from both of those on the client side.Backwards compatibility of the changes made to the return type of
Internal.ServiceDumphave been tested.Remaining TODO:
TestHTTPAPI_MethodNotAllowed_OSSInternal.ServiceDumpRPC endpoint tests