Feature Request: Support key-value attributes for services #3881
Conversation
|
Do you think this kind of PR could be merged? |
agent/agent_endpoint.go
Outdated
| @@ -579,6 +579,11 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re | |||
|
|
|||
| // Get the node service. | |||
| ns := args.NodeService() | |||
| if err := structs.ValidateMetadata(ns.ServiceMeta, false); err != nil { | |||
| resp.WriteHeader(http.StatusBadRequest) | |||
| fmt.Fprint(resp, fmt.Errorf("Invalid Meta: %v", err)) | |||
There was a problem hiding this comment.
fmt.Fprint(resp, fmt.Errorf("Invalid Service Meta: %v", err))
| Port int `json:",omitempty"` | ||
| Address string `json:",omitempty"` | ||
| EnableTagOverride bool `json:",omitempty"` | ||
| ServiceMeta map[string]string `json:",omitempty"` | ||
| Check *AgentServiceCheck | ||
| Checks AgentServiceChecks |
There was a problem hiding this comment.
In order to prevent abusing this feature to store arbitarily long metadata values, add some validation.
There is existing logic for node metadata here https://github.com/hashicorp/consul/blob/master/agent/structs/structs.go#L306. To keep things consistent we can use the same rules for service metadata as well. You should validate it in the agent and catalog registration endpoints.
Its fine to allow the reserved consul prefix for service metadata since we don't rely on this for any internal uses.
There was a problem hiding this comment.
I actually used this exact function in agent/agent_endpoint.go:582 but I am adding another check in the catalog as well in next patch and a unit test for Catalog insertion.
Fixed Error Message when ServiceMeta is not valid Added Unit test for adding a Service with badly formatted ServiceMeta
agent/agent_endpoint.go
Outdated
| @@ -579,6 +579,11 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re | |||
|
|
|||
| // Get the node service. | |||
| ns := args.NodeService() | |||
| if err := structs.ValidateMetadata(ns.ServiceMeta, false); err != nil { | |||
| resp.WriteHeader(http.StatusBadRequest) | |||
| fmt.Fprint(resp, fmt.Errorf("Invalid Meta: %v", err)) | |||
| Port int `json:",omitempty"` | ||
| Address string `json:",omitempty"` | ||
| EnableTagOverride bool `json:",omitempty"` | ||
| ServiceMeta map[string]string `json:",omitempty"` | ||
| Check *AgentServiceCheck | ||
| Checks AgentServiceChecks |
There was a problem hiding this comment.
I actually used this exact function in agent/agent_endpoint.go:582 but I am adding another check in the catalog as well in next patch and a unit test for Catalog insertion.
Fixes hashicorp#4045 Was not added by mistake in hashicorp#3881
This is a PR for supporting #1107
It basically allow to add metadata to service instances, so the data is linked to the lifecycle of service and avoid having Hacks such as using the KV (and find a way to cleanup the KV once the service is removed) or using tags to encode data.
It does not support searching for now, but support might be added in the future.
It would be very helpful for lots of new workloads such as adding dynamic provisioning of services in load balancers.
See #1107 (comment) for examples of possible usage