feat: add new flags to allow migration of OwnerID

[troll-os]

Description

This pull requests aims to complete the work initiated in these PR : #2466 and #3631

Most credits go to the creators of the initial work, as I mostly just rebased their work on the current state of master

Initial PR description:

When changing --txt-owner-id on an existing external-dns resource, it does not update the existing TXT records it owns, therefore losing ownership. Meaning that we have to manually delete the records in order to have external-dns take ownership again.
To solve this problem, I added the ability to update the original txt-owner by setting -- migrate-txt-owner to overwrite the old txt-owner. I have successfully modified thousands of pieces of data using this code, so far without any bugs

As for me I've successfully managed records without any issue using the newly added flags on the OVH provider
If anyone is willing to try these changes on other providers I'd be happy to troubleshoot their issues

Regarding the documentation I'm not sure either in which category these changes are relevant, so taking any recommendations on that

Note: this is my first contribution to such a project and first time using Go aswell, I've went through the contribution guidelines but if I missed something please let me know so that I can fix it

Fixes #2036

Checklist

• Unit tests updated
• End user documentation updated

[k8s-ci-robot]

Welcome @troll-os!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @troll-os. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[mloiseleur]

Thanks @troll-os. It looks good.
Would you please add some documentation on it ?

[troll-os]

@mloiseleur Sure !

Where do you think it would fit the best? TXT registry ?

[mloiseleur]

With this PR, it works only with TXT registry, so yes.

[mloiseleur]

/ok-to-test

[mloiseleur]

/lgtm

/assign @Raffo

[Raffo]

Can you explain the purpose of those lines?

[troll-os]

We want to apply the changes which should include the deletion of the old entries in the registry, otherwise we're left we unmanaged and unwanted records

[Raffo]

@troll-os answered to your comments. Could you please also provide a way to test this manually with manifests and kubectl commands? I would be eager to verify myself that the migration works and then keep an end to end test around.

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from mloiseleur. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[troll-os]

So I had to make a change for the OVH provider to ensure record targets are properly formatted, I ran another batch of test to migrate from the default owner and for some reason, records returned by the OVH API don't have their targets formatted like ExternalDNS expects

Meaning that we couldn't map the record ID and the deletion would fail

I don't know why this happens on a specific zones, the other zones covered seem fine... When I make direct calls to the OVH API the formatting is good...

I didn't forget your query for manifests to tests the changes @Raffo, I'll make them ASAP

[troll-os]

So I did a little of homework and made this work with Gandi with a domain I own for personal use and minikube

Steps used to reproduce on another setup than my work dev and prod environments :

1. make build on the branch of this PR (eventually push to a repo you own)
2. Setup Minikube with Docker
3. minikube enable addons ingress
4. Use following manifests (update variables to ensure your tests cases):

apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - image: nginx
        name: nginx
        ports:
        - containerPort: 80
---
apiVersion: v1
kind: Service
metadata:
  name: nginx
  annotations:
    external-dns.alpha.kubernetes.io/hostname: helloworld.whatever.tld
spec:
  selector:
    app: nginx
  type: LoadBalancer
  ports:
    - protocol: TCP
      port: 80
      targetPort: 80
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: nginx
spec:
  ingressClassName: nginx
  rules:
  - host: helloworld.whatever.tld
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
           name: nginx
           port:
             number: 80

apiVersion: apps/v1
kind: Deployment
metadata:
  name: external-dns
spec:
  replicas: 1
  selector:
    matchLabels:
      app: external-dns
  strategy:
    type: Recreate
  template:
    metadata:
      labels:
        app: external-dns
    spec:
      containers:
      - name: external-dns
        image: yourRegistry.com/external-dns:v-someBuildNumber  # <<<<<<<<<<<<
        args:
        - "--txt-prefix=%{record_type}-"
        - "--txt-cache-interval=15m"
        - "--log-level=info"
        - "--log-format=text"
        - "--txt-owner-id=old-owner"
        - "--policy=sync"
        - "--provider=youDecide" # <<<<<<<<<<<<<<<<
        - "--registry=txt"
        - "--interval=1m"
        - "--domain-filter=whatever.tld"
        - "--source=ingress"
        env:
        - name: whatever_env_var_provider_needs
          value: "hello"

5. Check your records, the A and TXT should be there, owned by "old-owner"
6. Apply following manifest to make migration :

apiVersion: apps/v1
kind: Deployment
metadata:
  name: external-dns
spec:
  replicas: 1
  selector:
    matchLabels:
      app: external-dns
  strategy:
    type: Recreate
  template:
    metadata:
      labels:
        app: external-dns
    spec:
      containers:
      - name: external-dns
        image: yourRegistry.com/external-dns:v-someBuildNumber  # <<<<<<<<<<<<
        args:
        - "--txt-prefix=%{record_type}-"
        - "--txt-cache-interval=15m"
        - "--log-level=info"
        - "--log-format=text"
        - "--txt-owner-id=new-owner"
        - "--from-txt-owner=old-owner"
        - "--migrate-txt-owner"
        - "--policy=sync"
        - "--provider=youDecide" # <<<<<<<<<<<<
        - "--registry=txt"
        - "--interval=1m"
        - "--domain-filter=whatever.tld"
        - "--source=ingress"
        env:
        - name: whatever_env_var_provider_needs
          value: "hello"

7. Check TXT record again, everything should be up to date

[troll-os]

Also I tried to follow the CoreDNS setup from the external-dns docs to have a non "external" provider dependent way to tests but didn't manage to make it work for some reason and I didn't want to spend too much time on it (probably a minikube issue) knowing that it worked with a classic provider