feat(registry): support all "managed" record types #4342

haslersn · 2024-03-27T19:34:12Z

Previously, the registry only supported certain hard-coded record types (A, AAAA, CNAME, NS). With this commit, we instead take the list of types given via --managed-record-types. That means the default now is A, AAAA, CNAME (i.e., NS is no longer included), but I don't think this is a breaking change, because any changes to records that aren't "managed" are filtered out at a later point anyway.

Now other record types (such as TXT) can also be handled by the TXT registry.

I have tested this by creating a DNSEndpoint with TXT targets and then deleting the DNSEndpoint again. The record and heritage record are correctly created and deleted.

Description

Fixes #ISSUE

Checklist

Unit tests updated
End user documentation updated

k8s-ci-robot · 2024-03-27T19:34:21Z

Welcome @haslersn!

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 · 2024-03-27T19:34:21Z

Hi @haslersn. 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/test-infra repository.

mloiseleur · 2024-04-05T08:15:15Z

Thanks for this PR @haslersn !
Wdyt about adding some documentation on registries about this ?

mloiseleur · 2024-04-05T08:15:22Z

/ok-to-test

k8s-triage-robot · 2024-07-04T09:32:34Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

haslersn · 2024-07-29T08:30:15Z

@mloiseleur

Wdyt about adding some documentation on registries about this ?

I don't really know what to write there. The --managed-record-types option in the first place has nothing to do with the registry. It specifies the record types which external DNS can create.

It is expected that the registry supports all records that are created by external-dns, but currently this is not the case. Seen from this angle, this PR is simply a bug fix that establishes expected behavior.

In the registry documentation I could add something like: “The registry persists metadata for DNS records of those types specified by --managed-record-types”. But that could be confusing and could lead people to think that they now have to specify --managed-record-types, even though the default works for most people.

Therefore, I now just added „A registry persists metadata for all DNS records created by external-dns“, which is an equivalent statement, because external-dns will not create records of types not specified in --managed-record-types, anyway.

k8s-ci-robot · 2024-08-15T07:19:42Z

[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 assign raffo for approval. 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

haslersn · 2024-08-16T15:52:28Z

@mloiseleur PTAL

registry/txt.go

mloiseleur · 2024-08-19T07:24:36Z

registry/txt.go

 }

 // extractRecordTypeDefaultPosition extracts record type from the default position
 // when not using '%{record_type}' in the prefix/suffix
-func extractRecordTypeDefaultPosition(name string) (baseName, recordType string) {
+func (pr affixNameMapper) extractRecordTypeDefaultPosition(name string) (baseName, recordType string) {


Suggested change

func (pr affixNameMapper) extractRecordTypeDefaultPosition(name string) (baseName, recordType string) {

func (anm affixNameMapper) extractRecordTypeDefaultPosition(name string) (baseName, recordType string) {

Well, affixNameMapper already has a lot of methods and all of them use the identifier pr.

That's true. It does not make sense too. It's like im for TXTRegistry.

Shall I change it everywhere (in a separate commit)?

That would be great, yes 👍

mloiseleur · 2024-08-19T07:54:00Z

I get your point for the documentation. It makes sense.

Previously, the registry only supported certain hard-coded record types (A, AAAA, CNAME, NS). With this commit, we instead take the list of types given via --managed-record-types. That means the default now is A, AAAA, CNAME (i.e., NS is no longer included), but I don't think this is a breaking change, because any changes to records that aren't "managed" are filtered out at a later point anyway. Now other record types (such as TXT) can also be handled by the TXT registry. I have tested this by creating a DNSEndpoint with TXT targets and then deleting the DNSEndpoint again. The record and heritage record are correctly created and deleted. Signed-off-by: Sebastian Hasler <[email protected]>

Signed-off-by: Sebastian Hasler <[email protected]>

mloiseleur · 2024-08-19T15:07:19Z

/lgtm

haslersn · 2024-08-25T16:40:30Z

@johngmyers according to the robot, you are requested as a second reviewer. Can you take a look, please?

k8s-triage-robot · 2024-09-24T17:17:24Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle rotten
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

haslersn · 2024-09-24T23:35:03Z

This should be merged, since it fixes a bug.

@johngmyers @mloiseleur

haslersn · 2024-10-08T20:10:59Z

@szuecs can you take a look at this one? (Pinging you, because you have reviewed external-dns PRs in the past.)

k8s-triage-robot · 2024-11-07T21:01:57Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen
Mark this PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2024-11-07T21:02:03Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen

Mark this PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 27, 2024

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 27, 2024

k8s-ci-robot requested review from johngmyers and mloiseleur March 27, 2024 19:34

haslersn force-pushed the txt-support-managed-record-types branch from 13c98b7 to 3e1eb24 Compare March 27, 2024 19:56

haslersn mentioned this pull request Mar 28, 2024

Endpoints with templated prefix are not deleted #3031

Closed

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 5, 2024

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 4, 2024

haslersn force-pushed the txt-support-managed-record-types branch from 3e1eb24 to 04701a3 Compare July 29, 2024 08:29

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 29, 2024

haslersn force-pushed the txt-support-managed-record-types branch 5 times, most recently from 9033122 to b119ea1 Compare July 31, 2024 20:59

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 31, 2024

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 15, 2024

haslersn force-pushed the txt-support-managed-record-types branch from b119ea1 to e637fe1 Compare August 15, 2024 07:19

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 15, 2024

haslersn force-pushed the txt-support-managed-record-types branch 3 times, most recently from a2e82c1 to 3721e75 Compare August 15, 2024 14:08

mloiseleur reviewed Aug 19, 2024

View reviewed changes

registry/txt.go Outdated Show resolved Hide resolved

mloiseleur reviewed Aug 19, 2024

View reviewed changes

haslersn force-pushed the txt-support-managed-record-types branch from 3721e75 to 84cc10e Compare August 19, 2024 13:10

refactor(registry): rename identifier for affixNameMapper

684addd

Signed-off-by: Sebastian Hasler <[email protected]>

haslersn force-pushed the txt-support-managed-record-types branch from d429855 to 684addd Compare August 19, 2024 14:10

k8s-ci-robot assigned mloiseleur Aug 19, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 19, 2024

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 24, 2024

haslersn mentioned this pull request Oct 8, 2024

feat(txt-registry): add support for TXT records #3916

Closed

1 task

k8s-ci-robot closed this Nov 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(registry): support all "managed" record types #4342

feat(registry): support all "managed" record types #4342

haslersn commented Mar 27, 2024 •

edited

Loading

k8s-ci-robot commented Mar 27, 2024

k8s-ci-robot commented Mar 27, 2024

mloiseleur commented Apr 5, 2024

mloiseleur commented Apr 5, 2024

k8s-triage-robot commented Jul 4, 2024

haslersn commented Jul 29, 2024

k8s-ci-robot commented Aug 15, 2024

haslersn commented Aug 16, 2024

mloiseleur Aug 19, 2024

haslersn Aug 19, 2024

mloiseleur Aug 19, 2024

haslersn Aug 19, 2024 •

edited

Loading

mloiseleur Aug 19, 2024

haslersn Aug 19, 2024

mloiseleur commented Aug 19, 2024

mloiseleur commented Aug 19, 2024

haslersn commented Aug 25, 2024

k8s-triage-robot commented Sep 24, 2024

haslersn commented Sep 24, 2024

haslersn commented Oct 8, 2024

k8s-triage-robot commented Nov 7, 2024

k8s-ci-robot commented Nov 7, 2024

	func (pr affixNameMapper) extractRecordTypeDefaultPosition(name string) (baseName, recordType string) {
	func (anm affixNameMapper) extractRecordTypeDefaultPosition(name string) (baseName, recordType string) {

feat(registry): support all "managed" record types #4342

feat(registry): support all "managed" record types #4342

Conversation

haslersn commented Mar 27, 2024 • edited Loading

k8s-ci-robot commented Mar 27, 2024

k8s-ci-robot commented Mar 27, 2024

mloiseleur commented Apr 5, 2024

mloiseleur commented Apr 5, 2024

k8s-triage-robot commented Jul 4, 2024

haslersn commented Jul 29, 2024

k8s-ci-robot commented Aug 15, 2024

haslersn commented Aug 16, 2024

mloiseleur Aug 19, 2024

Choose a reason for hiding this comment

haslersn Aug 19, 2024

Choose a reason for hiding this comment

mloiseleur Aug 19, 2024

Choose a reason for hiding this comment

haslersn Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

mloiseleur Aug 19, 2024

Choose a reason for hiding this comment

haslersn Aug 19, 2024

Choose a reason for hiding this comment

mloiseleur commented Aug 19, 2024

mloiseleur commented Aug 19, 2024

haslersn commented Aug 25, 2024

k8s-triage-robot commented Sep 24, 2024

haslersn commented Sep 24, 2024

haslersn commented Oct 8, 2024

k8s-triage-robot commented Nov 7, 2024

k8s-ci-robot commented Nov 7, 2024

haslersn commented Mar 27, 2024 •

edited

Loading

haslersn Aug 19, 2024 •

edited

Loading