fix cleaning up workloadentry with same ip and network

[stevenctl]

fixes #43950

Could also use the autoregisteredWorkloadEntryName with less validation.

This keys the resource to be cleaned up as well as the input.

If there were already multiple same network/IP workloadentries, that's it's own problem. This just keeps it from being worse with stuck resources.

[istio-policy-bot]

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

[linux-foundation-easycla]

• ❌ - login: @stevenctl / name: Steven Landow . The commit (d605f4b, 7946e7d) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[dhawton]

Release note looks good

[dhawton]

/test integ-security

[howardjohn]

Seems like maybe if we have:

wg1-1.2.3.4

and we get a connection for

wg2-1.2.3.4

we should just remove wg1-1.2.3.4 immediately? Its not valid to have overlapping IP in the same network, violating that may lead to strange behavior

[stevenctl]

we should just remove wg1-1.2.3.4 immediately? Its not valid to have overlapping IP in the same network, violating that may lead to strange behavior

Should we remove the existing one assuming that the new one is replacing it, and we're just waiting for the old one to go away? Or should we ignore the new one so we can't have something new kick an existing workload?

The former case sounds more realistic I guess

[howardjohn]

That (old one is stale and should be removed) would be my guess, but I am not sure I fully understand the cases that lead to this. I do know we make a strong assumption that "IP is unique within network" in a variety of places though

[hzxuzhonghu]

We donot have such case, first #43950 experiment is not valid, we do not allow running multi sidecar/ztunnel in a VM.

For VM workloadentry autoregister, we have gracefully handle that reconnect.

[linsun]

Agreed with @hzxuzhonghu - what is the user case of running multiple docker contains and each has its own ztunnel?

[stevenctl]

The docker experiment was a way for me to test running zTunnel on "vms" or in "dedicated mode" locally.
The fact that they had the same IP was a mistake I made.

Regardless of the fact that it's not a usecase to have multiple things with the same IP, we shouldn't have invalid workloadentries automatically created by istiod that never get cleaned up.

[stevenctl]

That (old one is stale and should be removed) would be my guess, but I am not sure I fully understand the cases that lead to this. I do know we make a strong assumption that "IP is unique within network" in a variety of places though

The case I ran into would only occur if a proxy with the same network/ip connects but asks for a different auto-register group. So maybe the same IP ends up getting re-used to be part of a different app or something.

1. We do our check to see if there is already a workloadentry (this name includes the WorkloadGroup and network)

istio/pilot/pkg/autoregistration/controller.go

Line 233 in b034571

if wle != nil {

2. That check misses, so now we add the new check that lists all WorkloadEntries with the same IP/network. If they have the same workloadgroup, we should have seen it in the first check. Anything that exists here should be cleaned up since the IP has "moved" to a new workloadgroup.

Regardless of all that logic, I still think this PR makes things more robust against misconfiguration. A user installing things on VMs for the first time could make the same mistake I did. Having a manual step of cleaning up resources that supposedly are managed my istiod doesn't seem right.

One question I have is for the new check, should we consider only same-namespace WorkloadEntries as duplicates?

[hzxuzhonghu]

I donot see any bad effect with more meta data in key

[stevenctl]

Yeah I think the key should map 1:1 with the WLE. It could just be the WLE name. Even if we have some logic to prevent duplicates, there can still be races when the old instance and new instance with the same IP are on different istiod instances.

The most common case that could cause something like this is editing the metadata to move the VM onto a different workload group and restarting the proxy.