Unstructured Source by akutz · Pull Request #3490 · kubernetes-sigs/external-dns

akutz · 2023-03-21T18:49:49Z

Description

This patch introduces a new source for reading APIs using the unstructured package. Users can specify the property path for the target of the endpoint, leveraging the well-known annotations for the host names, TTL, and record type. This source allows a wider-range of resources to participate in ExternalDNS without having to maintain a secondary resource for DNSNames. This also decreases the need to create sources based on alpha APIs before they are fully baked.

Fixes NA

Checklist

Unit tests updated
End user documentation updated

docs/contributing/unstructured-source.md

aruneshpa

LGTM.

docs/contributing/unstructured-source.md

source/unstructured_source.go

k8s-ci-robot · 2023-03-22T04:57:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details

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

akutz · 2023-03-22T15:02:52Z

Please note that I have updated the property path to use JSON Path for a more standard means of evaluating the target of the DNS entry. This also supports index fields in slices and not just object notation.

This patch introduces a new source for reading APIs using the unstructured package. Users can specify the property path for the target of the endpoint, leveraging the well-known annotations for the host names, TTL, and record type. This source allows a wider-range of resources to participate in ExternalDNS without having to maintain a secondary resource for DNSNames. This also decreases the need to create sources based on alpha APIs before they are fully baked.

szuecs · 2023-03-27T08:37:20Z

source/source_test.go

+			expectedSlice: []string{"fu.bar"},
+		},
+
+		// two hostsnames and whitespace


hostsnames -> hostnames

szuecs · 2023-03-27T08:40:34Z

source/unstructured_source.go

+
+		ttl, err := us.getTTL(item)
+		if err != nil {
+			log.Warnf("failed to get TTL for resource %q that has hostnames %s and targets %s: %s", item.GetName(), hostnames, targets, err)


warning only for deprecations, use Errorf instead, please check also rest of the logs above in the same file.

szuecs · 2023-03-27T08:40:56Z

source/unstructured_source.go

+
+		recordType := us.getRecordType(item)
+
+		log.Infof("resource=%q, hostnames=%s, targets=%s, ttl=%v, recordType=%q\n", item.GetName(), hostnames, targets, ttl, recordType)


no need to add \n

szuecs · 2023-03-27T08:58:58Z

docs/contributing/unstructured-source.md

+      * [DNS A-record](#dns-a-record-a-cluster-scoped-crd)
+  * [Cleanup](#cleanup-steps)
+
+## Details


Because it seems very different than the envisioned purpose of external-dns, can you please add more about architecture, when it makes sense to use when it does not?
There is a need for more context here. I basically don't understand who would use it and why one would use this instead of the current available sources. What do you do better or enable (use cases?), than the current offer?

szuecs

Code looks fines, some tiny comments but all in all lgtm
I just wonder about use case and why someone would like to use it. Basically what does it offer more than the current solution?

k8s-ci-robot · 2023-04-11T19:06:30Z

PR needs rebase.

Details

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.

k8s-triage-robot · 2023-07-10T19:29:47Z

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

TomyLobo · 2023-07-13T14:54:52Z

@szuecs was asking for a use case.
I've been successfully using this PR's changes to create hostnames for rancher nodes since march.

In the PR description, @akutz's motivation was forward compatibility and compatibility with narrow use cases, and I guess my use case falls into that latter category.
Basically, with this PR, you can create DNS entries based on any attribute of any Kubernetes object and I think the flexibility that introduces is itself a good enough reason to merge it.

aruneshpa · 2023-07-14T19:07:39Z

I just wonder about use case and why someone would like to use it. Basically what does it offer more than the current solution?

@szuecs , consider a custom resource that specifies hostnames and targets, which should be consumed external-dns to add entries with the configured DNS provider. As I understand, there are two ways to do that today:

Using the CRD source. This requires that the CRD includes a slice ofEndpoints in the Spec. This isn't ideal if the usecase is relatively simple (e.g., consider a resource that only supports specifying a single hostname and target with a fixed record type).
Adding a bespoke source for the new type. This is fine if the resource's API version has matured (e.g., beyond alpha). Otherwise we always run a risk of breaking external-dns if the API version of the type changes.

With the above in mind, I see value in the unstructured source which can allow users to use external-dns with their configured provider without heavily changing their API type, or introducing a brand new source. Let me know what you think.

mloiseleur · 2023-10-11T16:04:35Z

@akutz Do you think you can rebase this PR ?

k8s-ci-robot · 2024-01-03T18:24:43Z

@akutz: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-external-dns-licensecheck	`e3df86f`	link	true	`/test pull-external-dns-licensecheck`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

mloiseleur · 2024-01-04T07:50:20Z

I'm closing this PR, since there are no answer from the author.
Feel free to open a new one based on this if you need it.
/close

k8s-ci-robot · 2024-01-04T07:50:24Z

@mloiseleur: Closed this PR.

Details

In response to this:

I'm closing this PR, since there are no answer from the author.
Feel free to open a new one based on this if you need it.
/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/test-infra repository.

TomyLobo · 2024-01-11T13:58:29Z

@mloiseleur will external-dns add support for nonstandard and niche sources in any other way?
@akutz was kind enough to provide me with a build of his fork, which I've been using ever since, in order to create DNS entries based on Rancher's node resources.
If this version ever breaks, I'm back to square one where I don't have a way to do this automatically and centrally on the rancher manager cluster.

mloiseleur · 2024-01-11T14:55:43Z

@TomyLobo Feel free to open a new PR based on this if you need it.

TomyLobo · 2024-01-12T11:35:29Z

I would, but my Go skills are severely lacking.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 21, 2023

k8s-ci-robot requested review from njuettner and szuecs March 21, 2023 18:50

akutz force-pushed the feature/unstructured-source branch 5 times, most recently from 9b25ea7 to 552db48 Compare March 21, 2023 20:55

aruneshpa reviewed Mar 22, 2023

View reviewed changes

akutz force-pushed the feature/unstructured-source branch 3 times, most recently from ae40443 to 3797dce Compare March 22, 2023 03:57

aruneshpa approved these changes Mar 22, 2023

View reviewed changes

docs/contributing/unstructured-source.md Outdated Show resolved Hide resolved

source/unstructured_source.go Outdated Show resolved Hide resolved

akutz force-pushed the feature/unstructured-source branch 7 times, most recently from 9ebd3fe to 0fc3b55 Compare March 22, 2023 15:02

akutz force-pushed the feature/unstructured-source branch 5 times, most recently from a317f84 to 501710c Compare March 26, 2023 20:10

akutz force-pushed the feature/unstructured-source branch from 501710c to e3df86f Compare March 26, 2023 20:12

szuecs reviewed Mar 27, 2023

View reviewed changes

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

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

k8s-ci-robot closed this Jan 4, 2024

ivankatliarchuk mentioned this pull request Feb 7, 2026

feat(source): add unstructured source #6172

Merged

3 tasks


		recordType := us.getRecordType(item)

		log.Infof("resource=%q, hostnames=%s, targets=%s, ttl=%v, recordType=%q\n", item.GetName(), hostnames, targets, ttl, recordType)

Conversation

akutz commented Mar 21, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aruneshpa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

k8s-ci-robot commented Mar 22, 2023

Uh oh!

akutz commented Mar 22, 2023

Uh oh!

szuecs Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

szuecs Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

szuecs Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

szuecs Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

szuecs left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Apr 11, 2023

Uh oh!

k8s-triage-robot commented Jul 10, 2023

Uh oh!

TomyLobo commented Jul 13, 2023

Uh oh!

aruneshpa commented Jul 14, 2023

Uh oh!

mloiseleur commented Oct 11, 2023

Uh oh!

k8s-ci-robot commented Jan 3, 2024

Uh oh!

mloiseleur commented Jan 4, 2024

Uh oh!

k8s-ci-robot commented Jan 4, 2024

Uh oh!

TomyLobo commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mloiseleur commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomyLobo commented Jan 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

TomyLobo commented Jan 11, 2024 •

edited

Loading

mloiseleur commented Jan 11, 2024 •

edited

Loading