Fix: Use exact service name match when searching container labels

[damfleu]

There is an issue with findContainerByServiceName where it incorrectly matches containers with similar names, specifically when one service name is a prefix of the other.

The check that the label contains the string traefik.<svcType>.services.<svcName> will match for both the service name <svcName> but also services such as <svcName>-suffix. If the docker container for the longer service is looked up first, then it will incorrectly retrieve the port binding from that container.

The labels should be in the format traefik.<svcType>.services.<svcName>.<attribute> so just add a dot at the end of the needle and use HasPrefix() to ensure an exact match. The added test verifies the fix for this issue where hello would end up with "http://192.168.100.100:5566" instead of 5555. Unfortunately, since it depends in which order the containers will be iterated over, it doesn't fail (without the fix) all the time.

Here is the output of a non-passing run:

[...]
time="2024-07-07T12:13:53+02:00" level=debug msg="found container 'hello-test' (hello-test) for service 'hello'"
time="2024-07-07T12:13:53+02:00" level=debug msg="overriding service port from container host-port: using 5566 (was 5555) for hello@docker" service=hello@docker service-type=http
time="2024-07-07T12:13:53+02:00" level=info msg="publishing http://192.168.100.100:5566" service=hello@docker service-type=http
traefik/http/routers/hello/service: hello
traefik/http/routers/hello/tls/certResolver: default
traefik/http/services/hello-test/loadBalancer/servers/0/url: http://192.168.100.100:5566
traefik/http/services/hello-test/loadBalancer/passHostHeader: true
traefik/http/services/hello/loadBalancer/servers/0/url: http://192.168.100.100:5566
traefik/http/routers/hello-test/service: hello-test
traefik/http/routers/hello-test/rule: Host(`hello-test.local`)
traefik/http/routers/hello-test/tls/certResolver: default
traefik/http/routers/hello/rule: Host(`hello.local`)
traefik/http/services/hello/loadBalancer/passHostHeader: true
    traefik-kop/docker_helpers_test.go:320:
                Error Trace:    traefik-kop/docker_helpers_test.go:320
                                                        traefik-kop/docker_test.go:133
                Error:          Not equal:
                                expected: "http://192.168.100.100:5555"
                                actual  : "http://192.168.100.100:5566"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -http://192.168.100.100:5555
                                +http://192.168.100.100:5566
                Test:           Test_samePrefix
                Messages:       service has wrong IP at key: traefik/http/services/hello/loadBalancer/servers/0/url
--- FAIL: Test_samePrefix (0.01s)
FAIL
FAIL    github.com/jittering/traefik-kop        0.842s

[chetan]

I'm not sure why I used Contains originally instead of HasPrefix, as that would have been more appropriate. Nonetheless, rather than use a regex, why not simply append a dot to the needle? Wouldn't that have the same effect here or are there cases the regex would more completely cover?

The goal is to find a label like traefik.http.routers.hello.[attribute]=[value] without looking for a specific attribute which may not be there in all cases. I don't think we'd ever have the case of a bare label like traefik.http.routers.hello=[value], but even in this case we could also check for an exact match (without the dot).

[damfleu]

I'm not sure why I used Contains originally instead of HasPrefix, as that would have been more appropriate.

Though in this case HasPrefix would have had the same issue!

Nonetheless, rather than use a regex, why not simply append a dot to the needle? Wouldn't that have the same effect here or are there cases the regex would more completely cover?

The goal is to find a label like traefik.http.routers.hello.[attribute]=[value] without looking for a specific attribute which may not be there in all cases. I don't think we'd ever have the case of a bare label like traefik.http.routers.hello=[value], but even in this case we could also check for an exact match (without the dot).

Right, I went with that at first and then used a regex to handle exactly the case where the label ends there. I just didn't want to assume that wasn't be a valid case. Using HasPrefix() and adding a dot to the needle sounds good, it should cover all cases where an attribute is expected.

[chetan]

Though in this case HasPrefix would have had the same issue!

Yup, you are totally correct, I was just commenting on my odd choice there 😖. Started out as a quick hack. It worked, I shipped!

[damfleu]

Thanks for the comments, code has been updated, let me know what you think!