Skip to content

Conversation

giorio94
Copy link
Member

The GetEndpoints() method already returns a complete DeepCopy of the underlying structures, except when two separate epslices contain information about the same address. In this case, the additional ports are not DeepCopied, but plainly assigned. While I wouldn't expect this to create problems at the moment, let's get it fixed, especially considering that the result is not DeepCopied a second time anymore [1].

While being there, let's also get rid of the unnecessary DeepCopy of the mutated object, as already guaranteed to be a DeepCopy.

[1]: 3b59c26 ("service-cache: improve performance of correlateEndpoints")

The GetEndpoints() method already returns a complete DeepCopy of the
underlying structures, except when two separate epslices contain
information about the same address. In this case, the additional ports
are not DeepCopied, but plainly assigned. While I wouldn't expect this
to create problems at the moment, let's get it fixed, especially
considering that the result is not DeepCopied a second time anymore [1].

While being there, let's also get rid of the unnecessary DeepCopy of the
mutated object, as already guaranteed to be a DeepCopy.

[1]: 3b59c26 ("service-cache: improve performance of correlateEndpoints")

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94 giorio94 added the release-note/misc This PR makes changes that have no direct user impact. label Nov 15, 2024
@giorio94 giorio94 requested a review from a team as a code owner November 15, 2024 16:56
@giorio94 giorio94 requested a review from joamaki November 15, 2024 16:56
@giorio94
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 20, 2024
@joamaki joamaki added this pull request to the merge queue Nov 20, 2024
Merged via the queue into cilium:main with commit 2aee68b Nov 20, 2024
70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants