docs(annotations): scope the access annotation to the Service source - #6624
docs(annotations): scope the access annotation to the Service source#6624oscrx wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
the
needs-ok-to-test
|
|
|
Hi @oscrx. 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 Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
kubernetes-prow
Bot
added
cncf-cla: no
`external-dns.kubernetes.io/access` is read in exactly one place:
`getAccessFromAnnotations` in source/source.go, called only from
`extractNodePortTargets` in source/service.go. Every other source
ignores it.
The FAQ entry describing it is written source-agnostically, and its
stated default ("the public IP will be used by default") does not hold
outside the Service source. The Ingress source takes its targets from
`status.loadBalancer.ingress`, so when the controller publishes both a
public and a private address, both are published rather than the public
one being preferred.
Scope the FAQ entry, document the alternatives available for Ingress,
and add `access` and `endpoints-type` columns to the annotation support
matrix, where both were previously absent.
Reported in kubernetes-sigs#5266, kubernetes-sigs#3270 and kubernetes-sigs#1841.
Signed-off-by: Oscar Wieman <oscar@oscarr.nl>
08b4670 to
ba440c5
Compare
kubernetes-prow
Bot
added
cncf-cla: yes
|
/ok-to-test |
kubernetes-prow
Bot
added
ok-to-test
Coverage Report for CI Build 31446894312Coverage remained the same at 81.772%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
/lgtm |
kubernetes-prow
Bot
added
the
lgtm
| | Service | Yes | Yes[^1] | Yes[^1][^2] | Yes[^3] | Yes | Yes | | ||
| | Skipper | Yes | Yes[^1] | | Yes | Yes | Yes | | ||
| | Traefik | | Yes[^1] | | Yes[^6] | Yes | Yes | | ||
| | Source | access | controller | endpoints-type | hostname | internal-hostname | target | ttl | (provider-specific) | |
There was a problem hiding this comment.
That looks a bit weird to add a column in this table for an annotation used only by one source.
Would you please restore the original table?
| If this annotation is not set, and the node has both public and private IP addresses, then the public IP will be used by default. | ||
|
|
||
| This applies to the [Service source](sources/service.md) only, and only to `Service`s of type `NodePort`. | ||
| Every other source ignores the annotation, and the default described above does not apply to them either. |
There was a problem hiding this comment.
| Every other source ignores the annotation, and the default described above does not apply to them either. | |
| Every other source ignores the annotation. |
AFAIR, even though the annotation cannot be used, the default applies for node source.
| and a private address, both are published. To control which addresses end up in DNS for an `Ingress`: | ||
|
|
||
| - configure the ingress controller to publish the addresses you want in `status.loadBalancer.ingress` | ||
| (`ingress-nginx`, for example, has `--report-node-internal-ip-address`); |
There was a problem hiding this comment.
| (`ingress-nginx`, for example, has `--report-node-internal-ip-address`); |
Please avoid citing specific soft here, especially when this one is now no more maintained.
4 participants
Description
external-dns.kubernetes.io/accessis read in exactly one place —getAccessFromAnnotations, called only fromextractNodePortTargetsinsource/service.go. Every other source ignores it silently.Two docs problems follow from that:
The FAQ entry is written source-agnostically and states a default that is not true outside the Service source. It says "If this annotation is not set, and the node has both public and private IP addresses, then the public IP will be used by default." The Ingress source takes its targets from
status.loadBalancer.ingress, so when the ingress controller publishes both a public and a private address, both are published — there is no preference for the public one. This was pointed out in Target detection with NodePort ingress #3270 in December 2023 and never addressed.The annotation support matrix has no
accesscolumn at all (norendpoints-type). Both are documented only in prose further down the page, so the table a user consults to check per-source support does not mention them.This PR is docs-only:
Serviceof typeNodePort, states that other sources ignore the annotation and that the documented default does not apply to them;Ingress(ingress controller configuration, thetargetannotation,--target-net-filter/--exclude-target-net);--target-net-filter/--exclude-target-netare process-wide and that an endpoint whose targets are all filtered out is dropped;accessandendpoints-typecolumns to the support matrix, with footnotes for theNodePort-only and headless-only restrictions;No behaviour change is proposed here. Whether the Ingress source should honour
accessis a separate question — it would need a node informer andnodesRBAC in that source, and a definition of which nodes an arbitraryIngressrefers to. That belongs in a proposal rather than this PR.Fixes #5266
Related: #3270, #1841 — the same report, in 2023 and 2020, both bot-closed without a docs fix.
Checklist
mkdocs build --strictpasses locally🤖 Generated with Claude Code