Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add interface for ignoring identity through command line #267

Merged
merged 3 commits into from
Dec 12, 2022

Conversation

nirvanagit
Copy link
Collaborator

Changes:

Add interface for endpoint generation suspension so that other implementations can be easily swapped with the current command line implementation.

Copy link
Contributor

@aattuluri aattuluri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, a small change requested and a question.

@@ -0,0 +1,5 @@
package clusters

type EndpointSuspender interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor this to EndpointUpdateSuspender?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about ServiceEntryUpdateSuspender ?

if des.ignoredIdentityCache.EnvironmentsByIdentity[identity] != nil {
identityEnvironments := des.ignoredIdentityCache.EnvironmentsByIdentity[identity]
if len(identityEnvironments) == 0 || (len(identityEnvironments) == 1 && identityEnvironments[0] == "") {
log.Printf("%s, identity: %s", alertMsgSuspensionForIdentityInAllEnvironments, identity)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(len(identityEnvironments) == 1 && identityEnvironments[0] == "") is this a special case or just a safety check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the case when the entry is like:

identityByEnvironment: map[string][]string{
  "identity": []string{""},
}

Anubhav Aeron added 3 commits December 12, 2022 11:09
Signed-off-by: Anubhav Aeron <anubhav_aeron@intuit.com>
Signed-off-by: Anubhav Aeron <anubhav_aeron@intuit.com>
Signed-off-by: Anubhav Aeron <anubhav_aeron@intuit.com>
Copy link
Contributor

@aattuluri aattuluri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nirvanagit nirvanagit merged commit c8fa298 into master Dec 12, 2022
@nirvanagit nirvanagit deleted the ignore-identity-interface branch December 12, 2022 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants