ClusterMesh: improve validation of remote endpoints and identities #32785
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/test |
cc38a40 to
3542a55
Compare
|
Rebased onto main to fix conflict. |
|
/test |
Ensure that the unmarshaled data matches the kvstore key it was retrieved from, to ensure consistency and prevent the propagation of corrupted data. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
3542a55 to
dbffa40
Compare
|
Rebased onto main to fix conflict. |
|
/test |
nebril
approved these changes
May 31, 2024
asauber
approved these changes
May 31, 2024
pippolo84
approved these changes
May 31, 2024
Take the clusterID as parameter, rather than retrieving it from the global option. This allows to reuse the same methods also for clusterIDs different from the one configured locally. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Fix the handling of CiliumIdentity events received from the k8s informer to correctly compare the old and new object, rather than the new one with itself, and propagate modification events as expected. Although identities are never expected to be modified (only added and removed), this ensures that the corresponding warnings are emitted as expected. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
In preparation for the subsequent commits, and to simplify the overall logic, let's remove the difference between Add and Modify events, in favor of a single Upsert one. Identities are never expected to be modified (only added and removed), hence this change is effectively a no-op. However, differentiating add and modify events is typically fragile and can lead to possible inconsistencies (e.g., if incomplete or invalid Add events are filtered out, while subsequent updates are propagated). Additionally, the downstream logic is already designed to correctly handle (and ignore, when appropriate) updates. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
In preparation for the subsequent commit, let's introduce the possibility of configuring additional validators to filter out invalid identity events received from CRD or kvstore. Upsert events marked as invalid are directly skipped. Deletions, instead, are propagated for previously known identities only, to avoid leaving stale identities behind. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Explicitly validate that the identities retrieved from a remote cluster match the expected range based on the corresponding clusterID, to ensure improved consistency and prevent the propagation of corrupted data. Additionally, ensure that said identities include the cluster label matching the expected cluster name for the given cluster. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
dbffa40 to
0a40f14
Compare
|
/test |
This was referenced Jun 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explicitly validate that the identities retrieved from a remote cluster match the expected range based on the corresponding clusterID, to ensure improved consistency and prevent the propagation of corrupted data. Additionally, ensure that said identities include the cluster label matching the expected cluster name for the given cluster. Similarly, ensure that the unmarshaled endpoint data matches the kvstore key it was retrieved from.
Please review commit by commit, and refer to the individual descriptions for additional details.
Related: #29602, #32749