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

Upper-case CNAME target causing constantly creation and deletion on Cloudflare #1906

Closed
Gnnng opened this issue Dec 24, 2020 · 3 comments · Fixed by #1960
Closed

Upper-case CNAME target causing constantly creation and deletion on Cloudflare #1906

Gnnng opened this issue Dec 24, 2020 · 3 comments · Fixed by #1960
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@Gnnng
Copy link

Gnnng commented Dec 24, 2020

What happened:

When setting an upper-case CNAME target in ingress object through the annotation, ExternalDNS constantly creates and deletes the CNAME record every 1 minute (same as the specified update interval).

external-dns.alpha.kubernetes.io/target: "UpperCase.domain.com"

This can be confirmed from Cloudflare audit log as well as ExternalDNS log as below.

time="2020-12-24T06:52:58Z" level=info msg="Changing record." action=CREATE record=test-external-dns.foo.bar ttl=1 type=CNAME zone=19bc*************e036c
time="2020-12-24T06:52:58Z" level=error msg="failed to create record: error from makeRequest: HTTP status 400: content \"{\\\"result\\\":null,\\\"success\\\":false,\\\"errors\\\":[{\\\"code\\\":81053,\\\"message\\\":\\\"An A, AAAA or CNAME record already exists with that host.\\\"}],\\\"messages\\\":[]}\"" action=CREATE record=test-external-dns.foo.bar ttl=1 type=CNAME zone=19bc*************e036c
time="2020-12-24T06:52:58Z" level=info msg="Changing record." action=DELETE record=test-external-dns.foo.bar ttl=1 type=CNAME zone=19bc*************e036c
time="2020-12-24T06:52:58Z" level=info msg="Changing record." action=UPDATE record=test-external-dns.foo.bar ttl=1 type=TXT zone=19bc*************e036c
time="2020-12-24T06:53:58Z" level=debug msg="zoneIDFilter configured. only looking up zone IDs defined"
time="2020-12-24T06:53:58Z" level=debug msg="looking up zone 19bc*************e036c"
time="2020-12-24T06:53:58Z" level=debug msg="adding zone for consideration" zoneID=19bc*************e036c zoneName=foo.bar
time="2020-12-24T06:53:59Z" level=debug msg="Endpoints generated from ingress: test-external-dns/test-external-dns-nginx: [test-external-dns.foo.bar 0 IN CNAME  UpperCase.domain.com []]"
time="2020-12-24T06:53:59Z" level=debug msg="zoneIDFilter configured. only looking up zone IDs defined"
time="2020-12-24T06:53:59Z" level=debug msg="looking up zone 19bc*************e036c"
time="2020-12-24T06:53:59Z" level=debug msg="adding zone for consideration" zoneID=19bc*************e036c zoneName=foo.bar
time="2020-12-24T06:53:59Z" level=info msg="Changing record." action=CREATE record=test-external-dns.foo.bar ttl=1 type=CNAME zone=19bc*************e036c
time="2020-12-24T06:53:59Z" level=info msg="Changing record." action=CREATE record=test-external-dns.foo.bar ttl=1 type=TXT zone=19bc*************e036c
time="2020-12-24T06:54:00Z" level=error msg="failed to create record: error from makeRequest: HTTP status 400: content \"{\\\"result\\\":null,\\\"success\\\":false,\\\"errors\\\":[{\\\"code\\\":81057,\\\"message\\\":\\\"The record already exists.\\\"}],\\\"messages\\\":[]}\"" action=CREATE record=test-external-dns.foo.bar ttl=1 type=TXT zone=19bc*************e036c
time="2020-12-24T06:54:58Z" level=debug msg="zoneIDFilter configured. only looking up zone IDs defined"
time="2020-12-24T06:54:58Z" level=debug msg="looking up zone 19bc*************e036c"
time="2020-12-24T06:54:58Z" level=debug msg="adding zone for consideration" zoneID=19bc*************e036c zoneName=foo.bar
time="2020-12-24T06:54:58Z" level=debug msg="Endpoints generated from ingress: test-external-dns/test-external-dns-nginx: [test-external-dns.foo.bar 0 IN CNAME  UpperCase.domain.com []]"
time="2020-12-24T06:54:58Z" level=debug msg="zoneIDFilter configured. only looking up zone IDs defined"
time="2020-12-24T06:54:58Z" level=debug msg="looking up zone 19bc*************e036c"
time="2020-12-24T06:54:59Z" level=debug msg="adding zone for consideration" zoneID=19bc*************e036c zoneName=foo.bar
time="2020-12-24T06:54:59Z" level=info msg="Changing record." action=CREATE record=test-external-dns.foo.bar ttl=1 type=CNAME zone=19bc*************e036c
time="2020-12-24T06:54:59Z" level=error msg="failed to create record: error from makeRequest: HTTP status 400: content \"{\\\"result\\\":null,\\\"success\\\":false,\\\"errors\\\":[{\\\"code\\\":81053,\\\"message\\\":\\\"An A, AAAA or CNAME record already exists with that host.\\\"}],\\\"messages\\\":[]}\"" action=CREATE record=test-external-dns.foo.bar ttl=1 type=CNAME zone=19bc*************e036c
time="2020-12-24T06:54:59Z" level=info msg="Changing record." action=DELETE record=test-external-dns.foo.bar ttl=1 type=CNAME zone=19bc*************e036c
time="2020-12-24T06:54:59Z" level=info msg="Changing record." action=UPDATE record=test-external-dns.foo.bar ttl=1 type=TXT zone=19bc*************e036c
time="2020-12-24T06:55:58Z" level=debug msg="zoneIDFilter configured. only looking up zone IDs defined"
time="2020-12-24T06:55:58Z" level=debug msg="looking up zone 19bc*************e036c"
time="2020-12-24T06:55:58Z" level=debug msg="adding zone for consideration" zoneID=19bc*************e036c zoneName=foo.bar
time="2020-12-24T06:55:58Z" level=debug msg="Endpoints generated from ingress: test-external-dns/test-external-dns-nginx: [test-external-dns.foo.bar 0 IN CNAME  UpperCase.domain.com []]"
time="2020-12-24T06:55:58Z" level=debug msg="zoneIDFilter configured. only looking up zone IDs defined"
time="2020-12-24T06:55:58Z" level=debug msg="looking up zone 19bc*************e036c"
time="2020-12-24T06:55:59Z" level=debug msg="adding zone for consideration" zoneID=19bc*************e036c zoneName=foo.bar
time="2020-12-24T06:55:59Z" level=info msg="Changing record." action=CREATE record=test-external-dns.foo.bar ttl=1 type=CNAME zone=19bc*************e036c
time="2020-12-24T06:55:59Z" level=info msg="Changing record." action=CREATE record=test-external-dns.foo.bar ttl=1 type=TXT zone=19bc*************e036c
time="2020-12-24T06:56:00Z" level=error msg="failed to create record: error from makeRequest: HTTP status 400: content \"{\\\"result\\\":null,\\\"success\\\":false,\\\"errors\\\":[{\\\"code\\\":81057,\\\"message\\\":\\\"The record already exists.\\\"}],\\\"messages\\\":[]}\"" action=CREATE record=test-external-dns.foo.bar ttl=1 type=TXT zone=19bc*************e036c

Note that the deletion happens even when --policy=upsert-only is set, meaning the policy is not respected.

Good news is everything works fine if the CNAME target is converted to lower-case first

external-dns.alpha.kubernetes.io/target: "lowercase.domain.com"

What you expected to happen:

  1. When setting an upper-case CNAME target, ExternalDNS should properly handle it or throw an error if it's not able to.
  2. ExternalDNS should respect --policy=upsert-only and not delete the CNAME record in this case.

How to reproduce it (as minimally and precisely as possible):

The cli arguments.

        - name: external-dns
          image: "docker.io/bitnami/external-dns:0.7.5-debian-10-r0"
          imagePullPolicy: "IfNotPresent"
          args:
            # Generic arguments
            - --log-level=debug
            - --log-format=text
            - --zone-id-filter=19b******************036c
            - --policy=upsert-only
            - --provider=cloudflare
            - --registry=txt
            - --interval=1m
            - --txt-owner-id=some-owner
            - --source=service
            - --source=ingress
            # Cloudflare arguments
            - --cloudflare-proxied

And the ingress.

apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: test-external-dns-nginx
  labels:
     # -- snip --
  annotations:
    external-dns.alpha.kubernetes.io/target: UpperCase.domain.com
spec:
  rules:
    - host: test-external-dns.foo.bar
      http:
        paths:
          - path: /
            backend:
              serviceName: test-external-dns-nginx
              servicePort: http

Anything else we need to know?:

Environment:

  • External-DNS version (use external-dns --version): v0.7.5
  • DNS provider: Cloudflare
  • Others:
@Gnnng Gnnng added the kind/bug Categorizes issue or PR as related to a bug. label Dec 24, 2020
@robselway
Copy link
Contributor

I've had a dig into this.

Cloudflare will store CNAME records as lowercase - the plan logic (specifically targetChanged()) is case-sensitive, meaning your UpperCase.domain.com target will always be returned as a change.

Logic in the provider (in this case Cloudflare), which calls Difference() in provider.go to generate add/leave/remove arrays, does not expect current and desired states that only differ in casing, so the record is returned in both the add and remove arrays, causing the cloudflare provider to attempt both CREATE and DELETE actions. It looks like the cloudflare provider is the only one using this function, however others might behave in another unexpected way (perhaps just trying to continuously update the record).

The upsert-only policy is evaluated, however because the downstream logic is case sensitive, we end up in the curious state where the record is to be both created and deleted.

I think there are a couple of bugs here:

  1. Cloudflare provider is deciding to delete records unnecessarily (essentially circumventing the upsert policy)
  2. Casing differences are considered a change when updating a CNAME record

Problem 1 looks quite straightforward to fix, but there are few ways we could fix problem 2:

  • Transform the host to lowercase in the source, so downstream logic that is case sensitive behaves as expected
  • Update targetChanged() so the plan does not consider targets with different casing a change
  • Check for case-only changes in the cloudflare provider, and perhaps log a warning if the change won't be made (i.e. we won't respect the plan)

I'm new to this project so unsure what the best approach would be. Perhaps one of the maintainers could point us in the right direction - I'm happy to raise a PR.

@jgrumboe
Copy link
Contributor

@robselway
Thanks for looking into this. I'm not using cloudflare but I think you touched a "global" problem.
I've looked into the DNS RFC (https://tools.ietf.org/html/rfc1035) and on page 8 it states that DNS won't differ on the case.
So I would suggest to solve the upper/lower-case problem very centrally and convert everything to lowercase.

@szuecs
Copy link
Contributor

szuecs commented Jan 10, 2021

Yeah just convert to lowercase makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants