Add kubernetes registry in front of ServiceEntry #51003

hzxuzhonghu · 2024-05-13T04:07:29Z

Please provide a description of this PR:

Without this we can get proxy labels from WLE when the wle's address = pod ip

howardjohn · 2024-05-13T18:25:50Z

pilot/pkg/serviceregistry/aggregate/controller.go

+		for i, r := range c.registries {
+			if r.Provider() != provider.Kubernetes {
+				// insert the registry in the position of the first non kubernetes registry
+				newRegistries := make([]*registryEntry, len(c.registries)+1)


nit: can we use slices.Insert to avoid this complex code

good to know that function.

costinm · 2024-05-14T13:53:58Z

Any impact on backwards compatibility ? Should we prevent WE from using the same IP as a pod instead ? Any security implications ?

…

On Sun, May 12, 2024 at 9:07 PM hzxuzhonghu ***@***.***> wrote: *Please provide a description of this PR:* Fix #50968 <#50968> Without this we can get proxy labels from WLE when the wle's address = pod ip ------------------------------ You can view, comment on, or merge this pull request online at: #51003 Commit Summary - 777917f <777917f> add kubernetes registry infront of serviceentry - 6f4d577 <6f4d577> add test - 01185dd <01185dd> add release note File Changes (3 files <https://github.com/istio/istio/pull/51003/files>) - *M* pilot/pkg/serviceregistry/aggregate/controller.go <https://github.com/istio/istio/pull/51003/files#diff-4ab8bb2188afa1dc914f11b5eaab7e1a544454457d25ef3841fdb6f2c23b1d7b> (19) - *M* pilot/pkg/serviceregistry/aggregate/controller_test.go <https://github.com/istio/istio/pull/51003/files#diff-dc650829e52b22a3c617acbdae1a4056d9b30786e674aeaf6a8d4deb6a9aa159> (19) - *A* releasenotes/notes/serviceregistry-order.yaml <https://github.com/istio/istio/pull/51003/files#diff-d969d95e5d54d68d378c17658dfcfc4441a6c3512d6f6f10b78f25d7540a0483> (8) Patch Links: - https://github.com/istio/istio/pull/51003.patch - https://github.com/istio/istio/pull/51003.diff — Reply to this email directly, view it on GitHub <#51003>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAUR2TR6HQGL5GTVVIR5XDZCA4BNAVCNFSM6AAAAABHTNN2FCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI4TCNZVHEZTQMY> . You are receiving this because your review was requested.Message ID: ***@***.***>

hzxuzhonghu · 2024-05-15T02:38:06Z

Should we prevent WE from using the same IP as a pod instead ? Any security
implications ?

That can not be 100% possible.

Any impact on backwards compatibility ?

@costinm Maybe no, and in services ordering we have already sort kubernetes service before serviceentry

costinm · 2024-05-15T02:44:28Z

On Tue, May 14, 2024 at 7:38 PM hzxuzhonghu ***@***.***> wrote: Should we prevent WE from using the same IP as a pod instead ? Any security implications ? That can not be 100% possible.

We do build a map from IP to 'workload info' - so it should not be impossible to detect. Or giving users a mechanism to set CIDR ranges allowed for WE ( assigned to their VMs and separate from the Pod/Services CIDR). To be fair - Endpoints and EndpointSlices don't seem to have any protection either. I don't remember if we still consume them (for the 'custom' EndpointSlices and selector-less cases ).

Any impact on backwards compatibility ? @costinm <https://github.com/costinm> Maybe no, and in services ordering we have already sort kubernetes service before serviceentry

I see, so this will sort Pods before WE ? I think it's reasonable - we don't make guarantees and it's probably something wrong if users set pod IPs in WE. +1 from me.

hzxuzhonghu · 2024-05-15T02:51:48Z

We do build a map from IP to 'workload info' - so it should not be
impossible to detect.

OK, i know your point. We can do that in istiod. I meant we cannot prevent it from creating with validating webhook

hzxuzhonghu · 2024-05-15T02:54:09Z

It seems out certificate used for test has expired again

{Failed      utils_test.go:312: failed at updateMutatingWebhookConfig: failed to verify the certificate chain: x509: certificate has expired or is not yet valid: current time 2024-05-15T02:46:41Z is after 2024-05-14T23:11:26Z}

zirain · 2024-05-15T04:08:13Z

/retest-required

hzxuzhonghu · 2024-05-15T04:22:27Z

/test integ-pilot-istiodremote-mc

hzxuzhonghu · 2024-05-15T06:11:08Z

/test integ-pilot-istiodremote-mc

istio-testing · 2024-05-15T06:36:26Z

In response to a cherrypick label: #51003 failed to apply on top of branch "release-1.21":

Applying: add kubernetes registry infront of serviceentry
Applying: add test
Applying: add release note
Applying: address comments
Using index info to reconstruct a base tree...
M	pilot/pkg/serviceregistry/aggregate/controller.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/serviceregistry/aggregate/controller.go
CONFLICT (content): Merge conflict in pilot/pkg/serviceregistry/aggregate/controller.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 address comments
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

istio-testing · 2024-05-15T06:36:27Z

In response to a cherrypick label: new issue created for failed cherrypick: #51055

istio-testing · 2024-05-15T06:36:28Z

In response to a cherrypick label: #51003 failed to apply on top of branch "release-1.20":

Applying: add kubernetes registry infront of serviceentry
Applying: add test
Applying: add release note
Applying: address comments
Using index info to reconstruct a base tree...
M	pilot/pkg/serviceregistry/aggregate/controller.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/serviceregistry/aggregate/controller.go
CONFLICT (content): Merge conflict in pilot/pkg/serviceregistry/aggregate/controller.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 address comments
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

istio-testing · 2024-05-15T06:36:29Z

In response to a cherrypick label: new issue created for failed cherrypick: #51056

istio-testing · 2024-05-15T06:37:05Z

In response to a cherrypick label: new pull request created: #51057

istio-testing · 2024-05-15T06:37:06Z

In response to a cherrypick label: #51003 failed to apply on top of branch "release-1.21":

Applying: add kubernetes registry infront of serviceentry
Applying: add test
Applying: add release note
Applying: address comments
Using index info to reconstruct a base tree...
M	pilot/pkg/serviceregistry/aggregate/controller.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/serviceregistry/aggregate/controller.go
CONFLICT (content): Merge conflict in pilot/pkg/serviceregistry/aggregate/controller.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 address comments
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

istio-testing · 2024-05-15T06:37:07Z

In response to a cherrypick label: new issue created for failed cherrypick: #51058

istio-testing · 2024-05-15T06:37:08Z

In response to a cherrypick label: #51003 failed to apply on top of branch "release-1.20":

Applying: add kubernetes registry infront of serviceentry
Applying: add test
Applying: add release note
Applying: address comments
Using index info to reconstruct a base tree...
M	pilot/pkg/serviceregistry/aggregate/controller.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/serviceregistry/aggregate/controller.go
CONFLICT (content): Merge conflict in pilot/pkg/serviceregistry/aggregate/controller.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 address comments
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

istio-testing · 2024-05-15T06:37:10Z

In response to a cherrypick label: new issue created for failed cherrypick: #51059

istio-testing · 2024-05-15T06:37:46Z

In response to a cherrypick label: new pull request could not be created: failed to create pull request against istio/istio#release-1.22 from head istio-testing:cherry-pick-51003-to-release-1.22: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for istio-testing:cherry-pick-51003-to-release-1.22."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request"}

istio-testing · 2024-05-15T06:37:47Z

In response to a cherrypick label: #51003 failed to apply on top of branch "release-1.20":

Applying: add kubernetes registry infront of serviceentry
Applying: add test
Applying: add release note
Applying: address comments
Using index info to reconstruct a base tree...
M	pilot/pkg/serviceregistry/aggregate/controller.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/serviceregistry/aggregate/controller.go
CONFLICT (content): Merge conflict in pilot/pkg/serviceregistry/aggregate/controller.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 address comments
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

istio-testing · 2024-05-15T06:37:48Z

In response to a cherrypick label: new issue created for failed cherrypick: #51060

istio-testing · 2024-05-15T06:37:49Z

In response to a cherrypick label: #51003 failed to apply on top of branch "release-1.21":

Applying: add kubernetes registry infront of serviceentry
Applying: add test
Applying: add release note
Applying: address comments
Using index info to reconstruct a base tree...
M	pilot/pkg/serviceregistry/aggregate/controller.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/serviceregistry/aggregate/controller.go
CONFLICT (content): Merge conflict in pilot/pkg/serviceregistry/aggregate/controller.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 address comments
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

istio-testing · 2024-05-15T06:37:50Z

In response to a cherrypick label: new issue created for failed cherrypick: #51061

istio-testing · 2024-05-15T06:38:28Z

In response to a cherrypick label: new pull request could not be created: failed to create pull request against istio/istio#release-1.22 from head istio-testing:cherry-pick-51003-to-release-1.22: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for istio-testing:cherry-pick-51003-to-release-1.22."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request"}

hzxuzhonghu added 3 commits May 13, 2024 12:07

add kubernetes registry infront of serviceentry

777917f

add test

6f4d577

add release note

01185dd

hzxuzhonghu requested a review from a team as a code owner May 13, 2024 04:07

hzxuzhonghu force-pushed the addregistry branch from 138cc4b to 01185dd Compare May 13, 2024 04:07

istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 13, 2024

howardjohn reviewed May 13, 2024

View reviewed changes

hzxuzhonghu added 2 commits May 14, 2024 14:38

address comments

59ded53

fix lint

fef4fe6

hzxuzhonghu requested a review from a team as a code owner May 14, 2024 07:37

ramaraochavali approved these changes May 14, 2024

View reviewed changes

howardjohn approved these changes May 14, 2024

View reviewed changes

fix lint

4a9fa5e

istio-testing merged commit c62ebac into istio:master May 15, 2024
28 checks passed

istio-testing mentioned this pull request May 15, 2024

[release-1.21] Add kubernetes registry in front of ServiceEntry #51055

Open

istio-testing mentioned this pull request May 15, 2024

[release-1.20] Add kubernetes registry in front of ServiceEntry #51056

Closed

istio-testing mentioned this pull request May 15, 2024

[release-1.22] Add kubernetes registry in front of ServiceEntry #51057

Merged

istio-testing mentioned this pull request May 15, 2024

[release-1.21] Add kubernetes registry in front of ServiceEntry #51058

Closed

istio-testing mentioned this pull request May 15, 2024

[release-1.20] Add kubernetes registry in front of ServiceEntry #51059

Closed

istio-testing mentioned this pull request May 15, 2024

[release-1.20] Add kubernetes registry in front of ServiceEntry #51060

Open

istio-testing mentioned this pull request May 15, 2024

[release-1.21] Add kubernetes registry in front of ServiceEntry #51061

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add kubernetes registry in front of ServiceEntry #51003

Add kubernetes registry in front of ServiceEntry #51003

hzxuzhonghu commented May 13, 2024 •

edited by istio-policy-bot

howardjohn May 13, 2024

hzxuzhonghu May 14, 2024

costinm commented May 14, 2024 via email

hzxuzhonghu commented May 15, 2024

costinm commented May 15, 2024 via email

hzxuzhonghu commented May 15, 2024

hzxuzhonghu commented May 15, 2024

zirain commented May 15, 2024

hzxuzhonghu commented May 15, 2024

hzxuzhonghu commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

Add kubernetes registry in front of ServiceEntry #51003

Add kubernetes registry in front of ServiceEntry #51003

Conversation

hzxuzhonghu commented May 13, 2024 • edited by istio-policy-bot

howardjohn May 13, 2024

Choose a reason for hiding this comment

hzxuzhonghu May 14, 2024

Choose a reason for hiding this comment

costinm commented May 14, 2024 via email

hzxuzhonghu commented May 15, 2024

costinm commented May 15, 2024 via email

hzxuzhonghu commented May 15, 2024

hzxuzhonghu commented May 15, 2024

zirain commented May 15, 2024

hzxuzhonghu commented May 15, 2024

hzxuzhonghu commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

istio-testing commented May 15, 2024

hzxuzhonghu commented May 13, 2024 •

edited by istio-policy-bot