feat(source): optional expose of nodes internal ipv6 #5192

hjoshi123 · 2025-03-17T14:43:34Z

Description

This PR introduces a flag expose-internal-ipv6 according to the proposal IPv6 proposal which states that we are going to implement a flag to allow users to determine if they want to expose their internal IPv6. For the next release, this flag is going to be defaulted as true which means internal IPv6s are exposed (this is how we treat IPv6s currently too). The next minor release will flip the flag and still provide users the option to go back if needed.

Relates #4566
Related pull request #4574
Implement proposal IPv6 proposal

Checklist

Unit tests updated
End user documentation updated

…expose-internal-ipv6

k8s-ci-robot · 2025-03-17T14:43:44Z

Hi @hjoshi123. 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.

hjoshi123 · 2025-03-17T14:43:54Z

Will update the documentation soon. Raising PR for visibility.

mloiseleur · 2025-03-17T14:57:04Z

/ok-to-test
/retitle feat(source): optional expose of nodes internal ipv6

mloiseleur · 2025-03-17T14:57:38Z

Thanks @hjoshi123 !

ivankatliarchuk · 2025-03-17T16:20:11Z

Could you add a warning message with the tests, to inform users that in follow-up release we are going to flip the flag/behaviour?

source/node.go

source/node_test.go

ivankatliarchuk · 2025-03-19T07:15:05Z

The Warning message is missing, with description that if users would like to expose private Node IPs, they should configure the flag explicitly. Otherwise with next release, when we flip the flag, external-dns will silently update records for them.

With explicit test, we have a helper for that

external-dns/internal/testutils/log.go

Line 36 in ecd57c8

func LogsToBuffer(level log.Level, t *testing.T) *bytes.Buffer {

docs/sources/nodes.md

pkg/apis/externaldns/types.go

source/node_test.go

hjoshi123 · 2025-03-20T04:22:50Z

/retest

ivankatliarchuk · 2025-03-20T07:31:19Z

source/node_test.go

+		require.NoError(t, err)
+
+		if tc.exposeInternalIPv6 {
+			buf := testutils.LogsToBuffer(log.DebugLevel, t)


should be log.WarningLevel https://sematext.com/blog/logging-levels/

WARNING is not required as well, as

yup will change that

mloiseleur · 2025-03-24T08:01:43Z

This PR LGTM.
@ivankatliarchuk Anything left from your PoV ?
@jonasbadstuebner Do you think you can review this PR ?

jonasbadstuebner

I request to change the behavior back to the original one, when adding the feature flag - I added my suggestion(s).

Sorry for some nits I added. If they are invalid or unnecessary, please let me know.

docs/sources/nodes.md

source/node_test.go

pkg/apis/externaldns/types.go

source/node.go

source/node_test.go

docs/sources/nodes.md

source/node_test.go

k8s-ci-robot · 2025-03-24T09:32:23Z

@jonasbadstuebner: changing LGTM is restricted to collaborators

In response to this:

I request to change the behavior back to the original one, when adding the feature flag - I added my suggestion(s).

Sorry for some nits I added. If they are invalid or unnecessary, please let me know.

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.

jonasbadstuebner · 2025-03-24T09:34:56Z

Thank you @hjoshi123, for taking the time to open this PR, it's greatly appreciated from my side! Even if I added a lot of comments, they are not meant to devalue your work!

source/node.go

k8s-ci-robot · 2025-03-25T18:36:23Z

@jonasbadstuebner: changing LGTM is restricted to collaborators

In response to this:

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.

ivankatliarchuk · 2025-03-25T18:38:23Z

This PR LGTM. @ivankatliarchuk Anything left from your PoV ? @jonasbadstuebner Do you think you can review this PR ?

All good from my side, will approve when other comments resolved

jonasbadstuebner · 2025-03-26T18:41:19Z

It's good to go from my side 🚀 Thank you again, @hjoshi123!

ivankatliarchuk

/lgtm

jonasbadstuebner · 2025-03-26T19:57:11Z

/approve
I guess? :D

jonasbadstuebner · 2025-03-26T19:58:03Z

Ah I misunderstood - I thought I have to approve because there are pending requests by me shown.

ivankatliarchuk · 2025-03-26T20:28:18Z

Everything ls correct. Thanks for reviewing.

We review & approve.

mloiseleur · 2025-03-26T22:25:21Z

@jonasbadstuebner I'm unsure, maybe you will need to approve from Git Hub review tab, in order to remove the requested change.

I'll try and we'll see :)

/approve

k8s-ci-robot · 2025-03-26T22:25:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ivankatliarchuk, jonasbadstuebner, mloiseleur

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mloiseleur]

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

hjoshi123 added 2 commits March 17, 2025 08:37

feat: added expose internal ipv6 flag

87e3c4b

Merge branch 'master' of github.com:hjoshi123/external-dns into feat-…

1fbcb57

…expose-internal-ipv6

k8s-ci-robot requested review from ivankatliarchuk and mloiseleur March 17, 2025 14:43

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

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 17, 2025

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 17, 2025

k8s-ci-robot changed the title ~~Feat expose internal ipv6~~ feat(source): optional expose of nodes internal ipv6 Mar 17, 2025

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 17, 2025

ivankatliarchuk suggested changes Mar 17, 2025

View reviewed changes

source/node.go Outdated Show resolved Hide resolved

source/node_test.go Outdated Show resolved Hide resolved

source/node_test.go Outdated Show resolved Hide resolved

source/node_test.go Outdated Show resolved Hide resolved

k8s-ci-robot assigned ivankatliarchuk Mar 17, 2025

hjoshi123 added 3 commits March 17, 2025 20:33

fix: removing fmt.Printf

ed2419a

fix: fixing ci lint

794cdd2

docs: added documentation in node source

1d61159

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 Mar 18, 2025

ivankatliarchuk suggested changes Mar 19, 2025

View reviewed changes

docs/sources/nodes.md Outdated Show resolved Hide resolved

pkg/apis/externaldns/types.go Outdated Show resolved Hide resolved

source/node_test.go Outdated Show resolved Hide resolved

edited docs and made new test

1bf31da

ivankatliarchuk suggested changes Mar 20, 2025

View reviewed changes

added warn log

bf43ddf

hjoshi123 requested a review from ivankatliarchuk March 21, 2025 20:45

jonasbadstuebner suggested changes Mar 24, 2025

View reviewed changes

detailed documentation with no-expose

df517cf

jonasbadstuebner suggested changes Mar 25, 2025

View reviewed changes

source/node.go Outdated Show resolved Hide resolved

hjoshi123 added 3 commits March 25, 2025 20:45

added new tests to handle edge case

af3a7df

renaming variable

04f1661

removing reduntant code

719cc78

ivankatliarchuk approved these changes Mar 26, 2025

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 26, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 26, 2025

k8s-ci-robot merged commit a2f0f2e into kubernetes-sigs:master Mar 26, 2025
13 checks passed

ivankatliarchuk mentioned this pull request Mar 27, 2025

fix(node): logger test fixed #5232

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(source): optional expose of nodes internal ipv6 #5192

feat(source): optional expose of nodes internal ipv6 #5192

hjoshi123 commented Mar 17, 2025 •

edited

Loading

k8s-ci-robot commented Mar 17, 2025

hjoshi123 commented Mar 17, 2025

mloiseleur commented Mar 17, 2025

mloiseleur commented Mar 17, 2025

ivankatliarchuk commented Mar 17, 2025

ivankatliarchuk commented Mar 19, 2025

hjoshi123 commented Mar 20, 2025

ivankatliarchuk Mar 20, 2025

hjoshi123 Mar 20, 2025

mloiseleur commented Mar 24, 2025

jonasbadstuebner left a comment

k8s-ci-robot commented Mar 24, 2025

jonasbadstuebner commented Mar 24, 2025

k8s-ci-robot commented Mar 25, 2025

ivankatliarchuk commented Mar 25, 2025

jonasbadstuebner commented Mar 26, 2025

ivankatliarchuk left a comment

jonasbadstuebner commented Mar 26, 2025

jonasbadstuebner commented Mar 26, 2025

ivankatliarchuk commented Mar 26, 2025

mloiseleur commented Mar 26, 2025

k8s-ci-robot commented Mar 26, 2025

feat(source): optional expose of nodes internal ipv6 #5192

feat(source): optional expose of nodes internal ipv6 #5192

Conversation

hjoshi123 commented Mar 17, 2025 • edited Loading

k8s-ci-robot commented Mar 17, 2025

hjoshi123 commented Mar 17, 2025

mloiseleur commented Mar 17, 2025

mloiseleur commented Mar 17, 2025

ivankatliarchuk commented Mar 17, 2025

ivankatliarchuk commented Mar 19, 2025

hjoshi123 commented Mar 20, 2025

ivankatliarchuk Mar 20, 2025

Choose a reason for hiding this comment

hjoshi123 Mar 20, 2025

Choose a reason for hiding this comment

mloiseleur commented Mar 24, 2025

jonasbadstuebner left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 24, 2025

jonasbadstuebner commented Mar 24, 2025

k8s-ci-robot commented Mar 25, 2025

ivankatliarchuk commented Mar 25, 2025

jonasbadstuebner commented Mar 26, 2025

ivankatliarchuk left a comment

Choose a reason for hiding this comment

jonasbadstuebner commented Mar 26, 2025

jonasbadstuebner commented Mar 26, 2025

ivankatliarchuk commented Mar 26, 2025

mloiseleur commented Mar 26, 2025

k8s-ci-robot commented Mar 26, 2025

hjoshi123 commented Mar 17, 2025 •

edited

Loading