Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hope Service.Meta to contain a special characters such as dot(.) #8127

Open
Ayden-Franklin opened this issue Jun 17, 2020 · 7 comments
Open
Labels
theme/service-metadata Anything related to management/tracking of service metadata type/enhancement Proposed improvement or new feature

Comments

@Ayden-Franklin
Copy link

Ayden-Franklin commented Jun 17, 2020

Feature Description

I encountered an exception OperationException(statusCode=400, statusMessage='Bad Request', statusContent='Invalid Service Meta: Couldn't load metadata pair ('user.name', 'spring_boot'): Key contains invalid characters') that was related to that issue #4422. I actually want to use Service.Meta to pass come configurations containing a special characters . such as user.name or user.password. Could you relax the restriction about Service.Meta?

Use Case(s)

I use spring boot admin in which BasicAuthHttpHeaderProvider requires the metadata(user.name and user.password) that can not be supported by consul. This project support many discovery client such as eureka, kubernetes service that all support to use user.name for metadata. In SpringFramework's perspective, metadata won't be related with DNS so I suggest that Service.Meta should keep relaxing.

@Ayden-Franklin
Copy link
Author

I think Service.Meta like a dictionary to pass some K-V data except the service name. Service name is another significant term that not be related with metadata.

@pierresouchay
Copy link
Contributor

I tend to agree that let users use the dot would be a good thing. In our case, we had to escape several times meta and replace with underscores, but it might create clashes. I imagine that's because dots are in use in various tools (consul-template, filters in queries...) to navigate in results of objects, but the biggest issue is probably DNS as dots have a special meaning here (dots cannot be used within DNS labels) and meta can be returned by DNS lookups. So if dots could be used, dots would have to be escaped in DNS responses as well.

Note that this limitation is the same for node.meta as well.

Here is a discussion in original PR #3881 (comment)

@jsosulska jsosulska added theme/service-metadata Anything related to management/tracking of service metadata type/enhancement Proposed improvement or new feature labels Jun 18, 2020
@HaojunRen
Copy link

Hello guys,

I also met this issue. For Spring Cloud 2020, it refactorys metadata logics. And we register some config as metadata to Consul, like spring.boot.version. For Spring config, it often uses dot, so the issue will be

Caused by: com.ecwid.consul.v1.OperationException: OperationException(statusCode=400, statusMessage='Bad Request', statusContent='Invalid Service Meta: Couldn't load metadata pair ('spring.boot.version', '2.4.1'): Key contains invalid characters')
	at com.ecwid.consul.v1.agent.AgentConsulClient.agentServiceRegister(AgentConsulClient.java:278)
	at com.ecwid.consul.v1.ConsulClient.agentServiceRegister(ConsulClient.java:310)

@pierresouchay
Copy link
Contributor

@banks It is true that for services meta, many people want to push ., mostly in the Java world because it is the common way to define properties. We had to patch in many places our SDKs/injections systems to replace those with _ for instance.
Maybe allowing it in Service meta would not be such a big problem for Consul and would definitely help many people, but I see your point regarding the differences between Node.Meta and Service.Meta. However, there is a big difference: Node.Meta is mostly injected by Infrastructure (eg: chef), so it is possible easily to enforce some rules regarding the naming in 1 single place, while service.Meta might be injected by various systems (Java, Ruby, python, C# and friends) with many libraries that consider service.Meta like a commodity and do not take such limitations, so, for the the sake of UX, I would tend to say that it would be a good move to be a bit more permissive for Service.Meta

@HaojunRen
Copy link

@pierresouchay thanks for your answer. BTW, the spring cloud version before v2020.0.0, it use tags as metadata, so no limitation for dot. And if you want to refactory this logic, when it will release in 1.9.2 version?

@pierresouchay
Copy link
Contributor

@HaojunRen I am not working for Hashicorp, so I cannot decide, the patch would not be very complicated to do, but it really depends on Hashicorp's decision

@banks
Copy link
Member

banks commented Jan 15, 2021

I have always assumed this was a DNS limitation too since all Node meta is presented in TXT records:

$ dig @127.0.0.1 -p 8600 foo.node.consul ANY

; <<>> DiG 9.8.3-P1 <<>> @127.0.0.1 -p 8600 foo.node.consul ANY
; (1 server found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 24355
;; flags: qr aa rd; QUERY: 1, ANSWER: 1, AUTHORITY: 1, ADDITIONAL: 0
;; WARNING: recursion requested but not available

;; QUESTION SECTION:
;foo.node.consul.       IN  ANY

;; ANSWER SECTION:
foo.node.consul.    0   IN  A   10.1.10.12
foo.node.consul.    0   IN  TXT "meta_key=meta_value"
foo.node.consul.    0   IN  TXT "value only"


;; AUTHORITY SECTION:
consul.         0   IN  SOA ns.consul. postmaster.consul. 1392836399 3600 600 86400 0

(From https://www.consul.io/docs/discovery/dns#node-lookups)

But it turns out the contents of a TXT record per RFC1035 may contain dots and even the attribute syntax we use as specified in RFC1464 permits "any printable character" in an attribute name including = (assuming it is escaped).

So I think this was really just a case of starting off being conservative to not create potential issues later.

Only Node meta is used in DNS anyway, for Service Meta we just choose to keep the rules the same for consistency reasons.

So unless I've missed some other place we rely on Node meta, I don't see an issue with allowing periods in keys and values in both Node and Service meta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/service-metadata Anything related to management/tracking of service metadata type/enhancement Proposed improvement or new feature
Projects
None yet
Development

No branches or pull requests

5 participants