Skip to content

Commit

Permalink
use map instead of list to find out if identity is excluded
Browse files Browse the repository at this point in the history
  • Loading branch information
Anubhav Aeron committed Sep 30, 2022
1 parent e38c47b commit e59c1e7
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 65 deletions.
2 changes: 1 addition & 1 deletion admiral/cmd/admiral/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func GetRootCmd(args []string) *cobra.Command {
"The value of envoy filter is to add additional config to the filter config section")
rootCmd.PersistentFlags().BoolVar(&params.EnableRoutingPolicy, "enable_routing_policy", false,
"If Routing Policy feature needs to be enabled")
rootCmd.PersistentFlags().StringArrayVar(&params.ExcludeIdentityList, "exclude_identity_list", []string{},
rootCmd.PersistentFlags().StringArrayVar(&params.ExcludedIdentityList, "excluded_identity_list", []string{},
"List of identities which should be excluded from getting processed")
return rootCmd
}
Expand Down
4 changes: 2 additions & 2 deletions admiral/pkg/clusters/serviceentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func modifyServiceEntryForNewServiceOrPod(
continue
}
if deployment != nil {
if isAnExcludedIdentity(common.GetDeploymentGlobalIdentifier(deployment), remoteRegistry.ExcludeIdentityList) {
if len(remoteRegistry.ExcludedIdentityMap) > 0 && remoteRegistry.ExcludedIdentityMap[common.GetDeploymentGlobalIdentifier(deployment)] {
log.Infof(LogFormat, event, env, sourceIdentity, clusterId, "Processing skipped as identity is in the exclude list")
return nil
}
Expand All @@ -120,7 +120,7 @@ func modifyServiceEntryForNewServiceOrPod(
sourceDeployments[rc.ClusterID] = deployment
createServiceEntryForDeployment(ctx, event, rc, remoteRegistry.AdmiralCache, localMeshPorts, deployment, serviceEntries)
} else if rollout != nil {
if isAnExcludedIdentity(common.GetRolloutGlobalIdentifier(rollout), remoteRegistry.ExcludeIdentityList) {
if len(remoteRegistry.ExcludedIdentityMap) > 0 && remoteRegistry.ExcludedIdentityMap[common.GetRolloutGlobalIdentifier(rollout)] {
log.Infof(LogFormat, event, env, sourceIdentity, clusterId, "Processing skipped as identity is in the exclude list")
return nil
}
Expand Down
4 changes: 3 additions & 1 deletion admiral/pkg/clusters/serviceentry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,9 @@ func TestModifyServiceEntryForNewServiceOrPodForExcludedIdentity(t *testing.T) {
GlobalTraffic: gtpc,
}
rr1.PutRemoteController(clusterID, rc)
rr1.ExcludeIdentityList = []string{"asset1"}
rr1.ExcludedIdentityMap = map[string]bool{
"asset1": true,
}
rr1.StartTime = time.Now()
rr1.AdmiralCache.ServiceEntryAddressStore = serviceEntryAddressStore

Expand Down
14 changes: 2 additions & 12 deletions admiral/pkg/clusters/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -67,7 +66,7 @@ type RemoteRegistry struct {
ctx context.Context
AdmiralCache *AdmiralCache
StartTime time.Time
ExcludeIdentityList []string
ExcludedIdentityMap map[string]bool
}

func NewRemoteRegistry(ctx context.Context, params common.AdmiralParams) *RemoteRegistry {
Expand Down Expand Up @@ -102,7 +101,7 @@ func NewRemoteRegistry(ctx context.Context, params common.AdmiralParams) *Remote
StartTime: time.Now(),
remoteControllers: make(map[string]*RemoteController),
AdmiralCache: admiralCache,
ExcludeIdentityList: params.ExcludeIdentityList,
ExcludedIdentityMap: mapSliceToBool(params.ExcludedIdentityList, true),
}
}

Expand Down Expand Up @@ -154,15 +153,6 @@ func (r *RemoteRegistry) shutdown() {
}
}

func isAnExcludedIdentity(identityName string, excludedIdentityList []string) bool {
for _, excludedIdentity := range excludedIdentityList {
if strings.EqualFold(identityName, excludedIdentity) {
return true
}
}
return false
}

type ServiceEntryAddressStore struct {
EntryAddresses map[string]string `yaml:"entry-addresses,omitempty"`
Addresses []string `yaml:"addresses,omitempty"` //trading space for efficiency - this will give a quick way to validate that the address is unique
Expand Down
42 changes: 0 additions & 42 deletions admiral/pkg/clusters/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,45 +462,3 @@ func TestRoutingPolicyHandler(t *testing.T) {
assert.Nil(t, registry.AdmiralCache.RoutingPolicyFilterCache.Get("bar4stage"))
assert.Nil(t, registry.AdmiralCache.RoutingPolicyFilterCache.Get("bar3stage"))
}

func TestIsAnExcludedIdentity(t *testing.T) {
testCases := []struct {
name string
identityName string
excludedIdentityList []string
expectedResult bool
}{
{
name: "Given an identity is in the exclude list, " +
"When isAnExcludedIdentity is called, " +
"Then, it should return true",
identityName: "excluded-identity1",
excludedIdentityList: []string{"excluded-identity1"},
expectedResult: true,
},
{
name: "Given an identity is NOT in the exclude list, " +
"When isAnExcludedIdentity is called, " +
"Then, it should return false",
identityName: "not-excluded-identity1",
excludedIdentityList: []string{"excluded-identity1"},
expectedResult: false,
},
{
name: "Given an identity in the exclude list, " +
"When the identity name closely matches an excluded identity, " +
"And is different by one character, " +
"Then, it should return false",
identityName: "e1",
excludedIdentityList: []string{"e2"},
expectedResult: false,
},
}

for _, c := range testCases {
result := isAnExcludedIdentity(c.identityName, c.excludedIdentityList)
if result != c.expectedResult {
t.Fatalf("expected: %v, got: %v", c.expectedResult, result)
}
}
}
21 changes: 15 additions & 6 deletions admiral/pkg/clusters/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ package clusters

import (
"errors"
"strconv"
"strings"
"time"

argo "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
log "github.com/sirupsen/logrus"
"gopkg.in/yaml.v2"
k8sAppsV1 "k8s.io/api/apps/v1"
k8sV1 "k8s.io/api/core/v1"
"strconv"
"strings"
"time"
)

func GetMeshPorts(clusterName string, destService *k8sV1.Service,
Expand All @@ -30,13 +31,13 @@ func GetMeshPortsForRollout(clusterName string, destService *k8sV1.Service,
// Get the service selector to add as workload selector for envoyFilter
func GetServiceSelector(clusterName string, destService *k8sV1.Service) *common.Map {
var selectors = destService.Spec.Selector
if len(selectors) == 0{
if len(selectors) == 0 {
log.Infof(LogFormat, "GetServiceLabels", "no selectors present", destService.Name, clusterName, selectors)
return nil
}
var tempMap = common.NewMap()
for key, value := range selectors {
tempMap.Put(key,value)
tempMap.Put(key, value)
}
log.Infof(LogFormat, "GetServiceLabels", "selectors present", destService.Name, clusterName, selectors)
return tempMap
Expand Down Expand Up @@ -93,7 +94,7 @@ func getMeshPortsHelper(meshPorts string, destService *k8sV1.Service, clusterNam
}
if _, ok := meshPortMap[targetPort]; ok {
var protocol = GetPortProtocol(servicePort.Name)
log.Debugf(LogFormat, "GetMeshPorts", servicePort.Port, destService.Name, clusterName, "Adding mesh port for protocol: " + protocol)
log.Debugf(LogFormat, "GetMeshPorts", servicePort.Port, destService.Name, clusterName, "Adding mesh port for protocol: "+protocol)
ports[protocol] = uint32(servicePort.Port)
break
}
Expand Down Expand Up @@ -147,3 +148,11 @@ func ValidateConfigmapBeforePutting(cm *k8sV1.ConfigMap) error {
func IsCacheWarmupTime(remoteRegistry *RemoteRegistry) bool {
return time.Since(remoteRegistry.StartTime) < common.GetAdmiralParams().CacheRefreshDuration
}

func mapSliceToBool(list []string, value bool) map[string]bool {
m := make(map[string]bool, len(list))
for _, item := range list {
m[item] = value
}
return m
}
2 changes: 1 addition & 1 deletion admiral/pkg/controller/common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type AdmiralParams struct {
EnvoyFilterVersion string
EnvoyFilterAdditionalConfig string
EnableRoutingPolicy bool
ExcludeIdentityList []string
ExcludedIdentityList []string
}

func (b AdmiralParams) String() string {
Expand Down

0 comments on commit e59c1e7

Please sign in to comment.