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.

@nirvanagit nirvanagit requested a review from aattuluri December 12, 2022 18:42
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{""},
}

@nirvanagit nirvanagit force-pushed the ignore-identity-interface branch from c8c0048 to 5c19170 Compare December 12, 2022 19:10
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
kpharasi pushed a commit that referenced this pull request Dec 19, 2024
kpharasi pushed a commit that referenced this pull request Dec 19, 2024
add interface for ignoring identity through command line (#267)
kpharasi pushed a commit that referenced this pull request Dec 19, 2024
* Adding codecov/patch config

* Adding 90% patch restriction for coverage

* Fixing codecov config
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