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

Service tags containing a comma #719

Closed
AndyL4N opened this issue Sep 7, 2021 · 8 comments · Fixed by #983
Closed

Service tags containing a comma #719

AndyL4N opened this issue Sep 7, 2021 · 8 comments · Fixed by #983

Comments

@AndyL4N
Copy link

AndyL4N commented Sep 7, 2021

Consul uses comma-separated service tags notation.
But using traefik proxy with consul catalog I found some complications to setup multiple middlewares or entrypoints, required comma in value. For example:

'consul.hashicorp.com/service-tags':  |- 
      traefik.http.routers.router1.middlewares=middleware-redirect-https,middleware-ipwhitelist

Here Consul will split the second middleware to another tag
Of course, in consul terminology, this does not match the architecture. But the flexibility of the traefik-consul-kubernetes stack is very efficient. Maybe there is some workаround?

@jkirschner-hashicorp
Copy link
Contributor

jkirschner-hashicorp commented Sep 7, 2021

Hi @AndyL4N : can you provide any relevant log messages that occur when you try to register a service with a comma in a service tag? [EDIT]: no longer needed, see comment below.

I'm not yet familiar with using consul-k8s to register services, but I know with pure consul that a service can be registered with a tag that contains commas. For example, the below HCL can successfully register a service with tags "a" and "b,c" (just tested with 1.10):

service = {
	id = "rabbitmq"
	name = "rabbitmq"
	port = 5672
	tags = ["a", "b,c"]
	meta = {
		environment = "prod"
	}
	connect = {
		sidecar_service {

		}
	}
}

@jkirschner-hashicorp
Copy link
Contributor

@david-yu: bringing to your attention as I believe this is a limitation on the consul-k8s side.

The consul.hashicorp.com/service-tags annotation expects a comma separated list of tags. I'm not sure if there's a way to escape the comma.

// annotationTags is a list of tags to register with the service
// this is specified as a comma separated list e.g. abc,123
annotationTags = "consul.hashicorp.com/service-tags"

var tags []string
if raw, ok := pod.Annotations[annotationTags]; ok && raw != "" {
tags = strings.Split(raw, ",")
}

@david-yu david-yu transferred this issue from hashicorp/consul Sep 10, 2021
@david-yu
Copy link
Contributor

Hi @AndyL4N Could you tell us more about your use case? Have you enabled Connect or are you using mainly Catalog Sync? I believe likely do expect commas to separate tags, but I wonder if you attempted to escape that whole tag somehow with with a single or double quote?

@AndyL4N
Copy link
Author

AndyL4N commented Sep 10, 2021

Hi @david-yu. Currently we are only using Service Sync without Connect. Sure, we tried many options to escape commas and tags with quotes, double quotes, braces and brackets, backslashes and even replacing with HTML code symbols. Same for escaping only tag values and whole tag line. As I see in listing above it has no sense because whole annotation tag string splits only by commas without any other logic.

For example this annotations:

     annotations:
    'consul.hashicorp.com/service-sync': 'true'
    'consul.hashicorp.com/service-tags':  |- 
     env=prod,
      solution=SupperApp,
      traefik.enable=true,
      traefik.http.middlewares.app-ipwhitelist.ipwhitelist.sourcerange=192.168.1.0/24, 
      traefik.http.middlewares.app-redirect-https.redirectscheme.scheme=https,
      traefik.http.routers.app-http.middlewares=app-redirect-https,app-ipwhitelist,

will be splitted to something like this:

consul catalog services -tags | grep app
my-service-in-consul      app-ipwhitelist,env=prod,solution=SupperApp,traefik.enable=true,
                          traefik.http.middlewares.app-ipwhitelist.ipwhitelist.sourcerange=192.168.1.1/24,
                          traefik.http.middlewares.app-redirect-https.redirectscheme.scheme=https,
                          traefik.http.routers.app-http.middlewares=app-redirect-https

Trying to escape whole tag with double quotes doing a similar thing:

my-service-in-consul   "traefik.http.routers.app-http.middlewares=app-redirect-https, env=prod,   
                          app-ipwhitelist",solution=SupperApp,traefik.enable=true,
                          traefik.http.middlewares.app-ipwhitelist.ipwhitelist.sourcerange=192.168.1.1/24,
                          traefik.http.middlewares.app-redirect-https.redirectscheme.scheme=https,
                          

Or only tag value, for example with single quotes:

my-service-in-consul   traefik.http.routers.app-http.middlewares='app-redirect-https, env=prod,   
                          app-ipwhitelist',solution=SupperApp,traefik.enable=true,
                          traefik.http.middlewares.app-ipwhitelist.ipwhitelist.sourcerange=192.168.1.1/24,
                          traefik.http.middlewares.app-redirect-https.redirectscheme.scheme=https,
                          

If it requires, I can send you more variants, but result will be the same.

And the case is to register service with Traefik-specific parameters for configuration discovery. Usually these parameters can have many tags contain commas-separated values like headers, endpoints, middlewares, rules and even Regexp expressions.

@lkysow
Copy link
Member

lkysow commented Sep 13, 2021

Using , as our separator was obviously the wrong decision there unfortunately given that tags supports commas. Maybe now in order to preserve backwards compatibility we could supports \, being converted into a comma?

@ivanmordor
Copy link

Good afternoon. Any news on this issue? We can make a fix and use a special separator for a non-sign ","

@ivanmordor
Copy link

ivanmordor commented Jan 19, 2022

@lkysow can we release a fix for this problem as a fix-release???

@lkysow
Copy link
Member

lkysow commented Jan 24, 2022

Will be in our next release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants