hashicorp / hashicorp/consul

Connect: support enabling proxies via `service-defaults` ConfigEntry

Open
#5,856 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

theme/connect type/enhancement
Dominant language
Go
Stars
30.1k
Forks
4.6k
Avg merge
1d 18h
Merged PRs (30d)
39

Description

As originally designed, we should allow a simple boolean in the `service-defaults` ConfigEntry for any service (inside a connect block for future expansion:

```hcl
kind = "service-defaults"
name = "web"
protocol = "http"
connect {
enable_sidecar_proxy = true
}
```

This was planned for 1.5.0 but ran into some issues in the way we resolve these in the client agent's `ServiceManager`.

Rough steps needed from memory are:
- Enable resolving for all registrations again (provided config enables it) [not just proxies](https://github.com/hashicorp/consul/blob/45cdc8076703212b6ff3e31b50f226fbbef5ef7c/agent/service_manager.go#L39-L44)
- If the service has sidecars enabled centrally, then then create one and validate and register: https://github.com/hashicorp/consul/blob/45cdc8076703212b6ff3e31b50f226fbbef5ef7c/agent/agent.go#L3053-L3072

Notes:
- we need to register the sidecar explicitly with `a.AddService` in the same way as above link - `addServiceInternal` won't do it for us and we need to do it in such a way that it goes back through the `ServiceManager` which will pick up it's config properly.
- edge cases we need to account for but not totally sure how:
1. the service definition already had `sidecar_service = {}`. This means that the config loader linked above already registered the sidecar for it and we must not override that as it may have had local customizations`. I guess we could just not have a flag to `AddService` that will only add it if there is no ID collision or something?
- complication: think through the lifecycle here. For example, if the user has a custom proxy then we don't override it, but if they then remove their proxy on a service reload we should probably go back to setting up the default one - I _think_ this would just work because the service reload would also re-register the service and so re-trigger the logic, but what above if they remove the current proxy via API? I don't think it's the end of the world if we don't handle that perfectly right away but worth considering.
2. a sidecar was manually registered for the service already (not by sidecar service)
- If the sidecar's ID is the same as the sidecar_service default on (e.g. `web_sidecar_proxy`) then this is effectively the same case as 1 above and the existing reg should override etc.
- if the manual sidecar has a different ID then I guess we should still add the default one - it might be a separate sidecar is being run for reasons and the registration mostly doesn't hurt even if it's not actually consumed by a sidecar. the one gross thing is that `-sidecar-for` won't work as there will be too but we can just document "if you want to manually override a centrally configured proxy, ensure you use the same ID or you will end up with both registered" etc.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with agent/service_manager.go and the AddService flow referenced in agent/agent.go, then trace how service-defaults are resolved through ServiceManager. Define the behavior for centrally enabled sidecars, existing sidecar_service registrations, manual sidecars, and reloads before implementing it. Done means the default proxy is registered through ServiceManager without overriding custom registrations and the relevant lifecycle edge cases are covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, networking
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.