Skip to content

Commit

Permalink
changes for review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@slahirucd7
slahirucd7 committed Apr 29, 2024
1 parent 44bbe72 commit 4ba9169
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 29 deletions.
49 changes: 31 additions & 18 deletions adapter/internal/discovery/xds/rate_limiter_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ import (

var rlsPolicyCache *rateLimitPolicyCache

const subscriptionPolicyType = "subscription"
const (
subscriptionPolicyType = "subscription"
organization = "organization"
)

func getRateLimitUnit(name string) (rls_config.RateLimitUnit, error) {
switch strings.ToUpper(name) {
Expand Down Expand Up @@ -77,7 +80,7 @@ type rateLimitPolicyCache struct {
apiLevelRateLimitPolicies map[string]map[string]map[string][]*rls_config.RateLimitDescriptor
// metadataBasedPolicies is used to store the rate limit policies which are based on dynamic metadata.
// rate limit type (eg: subscription) -> policy name (eg: Gold, Silver) -> Rate Limit Config
metadataBasedPolicies map[string]map[string][]*rls_config.RateLimitDescriptor
metadataBasedPolicies map[string]map[string]map[string]*rls_config.RateLimitDescriptor
// mutex for API level
apiLevelMu sync.RWMutex
}
Expand Down Expand Up @@ -247,18 +250,26 @@ func (r *rateLimitPolicyCache) generateRateLimitConfig(label string) *rls_config
}

//Iterate through the subscription policies and append it to the orgDescriptors
for metadataType, metadataPolicyMap := range r.metadataBasedPolicies {
var subscriptionDescriptors []*rls_config.RateLimitDescriptor
var metadataDescriptor *rls_config.RateLimitDescriptor
for organization, subscriptionPolicyDescriptorList := range metadataPolicyMap {
subscriptionDescriptors = append(subscriptionDescriptors, subscriptionPolicyDescriptorList...)
//organization -> subscription -> policyName -> RateLimitDescriptor
if subscriptionPoliciesList, ok := r.metadataBasedPolicies[subscriptionPolicyType]; ok {
for orgName := range subscriptionPoliciesList {
var metadataDescriptor *rls_config.RateLimitDescriptor
var policyDescriptors []*rls_config.RateLimitDescriptor
metadataDescriptor = &rls_config.RateLimitDescriptor{
Key: metadataType,
Value: organization,
Descriptors: subscriptionDescriptors,
Key: organization,
Value: orgName,
}
subscriptionIDDescriptor := &rls_config.RateLimitDescriptor{
Key: subscriptionPolicyType,
}
for policyName := range subscriptionPoliciesList[orgName] {
policyDescriptors = append(policyDescriptors, subscriptionPoliciesList[orgName][policyName])
}
subscriptionIDDescriptor.Descriptors = policyDescriptors
metadataDescriptor.Descriptors = append(metadataDescriptor.Descriptors, subscriptionIDDescriptor)

metadataDescriptors = append(metadataDescriptors, metadataDescriptor)
}
metadataDescriptors = append(metadataDescriptors, metadataDescriptor)
}

allDescriptors := append(orgDescriptors, metadataDescriptors...)
Expand Down Expand Up @@ -299,10 +310,6 @@ func (r *rateLimitPolicyCache) updateXdsCache(label string) bool {
// AddSubscriptionLevelRateLimitPolicy adds a subscription level rate limit policy to the cache. This method is called
// only during the startup
func AddSubscriptionLevelRateLimitPolicy(policyList *types.SubscriptionPolicyList) error {
// Check if rlsPolicyCache.metadataBasedPolicies[Subscription] exists and create a new map if not
if _, ok := rlsPolicyCache.metadataBasedPolicies[subscriptionPolicyType]; !ok {
rlsPolicyCache.metadataBasedPolicies[subscriptionPolicyType] = make(map[string][]*rls_config.RateLimitDescriptor)
}
for _, policy := range policyList.List {
// Needs to skip on async policies.
if policy.DefaultLimit == nil || policy.DefaultLimit.QuotaType != "requestCount" || policy.DefaultLimit.RequestCount == nil {
Expand All @@ -328,8 +335,14 @@ func AddSubscriptionLevelRateLimitPolicy(policyList *types.SubscriptionPolicyLis
Value: policy.Name,
RateLimit: &rlPolicyConfig,
}
rlsPolicyCache.metadataBasedPolicies[subscriptionPolicyType][policy.Organization] =
append(rlsPolicyCache.metadataBasedPolicies[subscriptionPolicyType][policy.Organization], descriptor)
// Check if rlsPolicyCache.metadataBasedPolicies[Subscription] exists and create a new map if not
if _, ok := rlsPolicyCache.metadataBasedPolicies[subscriptionPolicyType]; !ok {
rlsPolicyCache.metadataBasedPolicies[subscriptionPolicyType] = make(map[string]map[string]*rls_config.RateLimitDescriptor)
}
if _, ok := rlsPolicyCache.metadataBasedPolicies[subscriptionPolicyType][policy.Organization]; !ok {
rlsPolicyCache.metadataBasedPolicies[subscriptionPolicyType][policy.Organization] = make(map[string]*rls_config.RateLimitDescriptor)
}
rlsPolicyCache.metadataBasedPolicies[subscriptionPolicyType][policy.Organization][policy.Name] = descriptor
loggers.LoggerXds.Infof("Custom subscription policy: %s is added to the cache map for organization: %s", policy.Name, policy.Organization)
}
return nil
Expand All @@ -339,6 +352,6 @@ func init() {
rlsPolicyCache = &rateLimitPolicyCache{
xdsCache: gcp_cache.NewSnapshotCache(false, IDHash{}, nil),
apiLevelRateLimitPolicies: make(map[string]map[string]map[string][]*rls_config.RateLimitDescriptor),
metadataBasedPolicies: make(map[string]map[string][]*rls_config.RateLimitDescriptor),
metadataBasedPolicies: make(map[string]map[string]map[string]*rls_config.RateLimitDescriptor),
}
}
24 changes: 13 additions & 11 deletions adapter/internal/discovery/xds/rate_limiter_cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,53 +587,57 @@ func TestAddSubscriptionLevelRateLimitPolicy(t *testing.T) {
QuotaType: "requestCount",
RequestCount: &types.SubscriptionRequestCount{
RequestCount: 100,
TimeUnit: "SECOND",
TimeUnit: "sec",
},
},
Organization: "org1",
},
{
Name: "Policy2",
DefaultLimit: &types.SubscriptionDefaultLimit{
QuotaType: "requestCount",
RequestCount: &types.SubscriptionRequestCount{
RequestCount: 200,
TimeUnit: "MINUTE",
TimeUnit: "min",
},
},
Organization: "org1",
},
{
Name: "Unauthenticated",
DefaultLimit: &types.SubscriptionDefaultLimit{
QuotaType: "requestCount",
RequestCount: &types.SubscriptionRequestCount{
RequestCount: 300,
TimeUnit: "HOUR",
TimeUnit: "hours",
},
},
Organization: "org1",
},
{
Name: "AsyncPolicy1",
DefaultLimit: &types.SubscriptionDefaultLimit{
QuotaType: "eventCount",
RequestCount: &types.SubscriptionRequestCount{
RequestCount: 300,
TimeUnit: "HOUR",
TimeUnit: "hours",
},
},
Organization: "org1",
},
},
}

// Initialize rlsPolicyCache.metadataBasedPolicies
rlsPolicyCache.metadataBasedPolicies = make(map[string]map[string][]*rls_config.RateLimitDescriptor)
rlsPolicyCache.metadataBasedPolicies = make(map[string]map[string]map[string]*rls_config.RateLimitDescriptor)

err := AddSubscriptionLevelRateLimitPolicy(policyList)
assert.NoError(t, err)

expectedPolicies := map[string]map[string][]*rls_config.RateLimitDescriptor{
expectedPolicies := map[string]map[string]map[string]*rls_config.RateLimitDescriptor{
subscriptionPolicyType: {
"Policy1": {
{
"org1": {
"Policy1": {
Key: "policy",
Value: "Policy1",
RateLimit: &rls_config.RateLimitPolicy{
Expand All @@ -642,9 +646,7 @@ func TestAddSubscriptionLevelRateLimitPolicy(t *testing.T) {
Name: "Policy1",
},
},
},
"Policy2": {
{
"Policy2": {
Key: "policy",
Value: "Policy2",
RateLimit: &rls_config.RateLimitPolicy{
Expand Down
19 changes: 19 additions & 0 deletions adapter/internal/oasparser/envoyconf/envoyconf_internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,25 @@ func TestCreateRoute(t *testing.T) {
RateLimits: []*routev3.RateLimit{
{
Actions: []*routev3.RateLimit_Action{
{
ActionSpecifier: &routev3.RateLimit_Action_Metadata{
Metadata: &routev3.RateLimit_Action_MetaData{
DescriptorKey: "organization",
MetadataKey: &metadatav3.MetadataKey{
Key: extAuthzFilterName,
Path: []*metadatav3.MetadataKey_PathSegment{
{
Segment: &metadatav3.MetadataKey_PathSegment_Key{
Key: "ratelimit:organization",
},
},
},
},
Source: routev3.RateLimit_Action_MetaData_DYNAMIC,
SkipIfAbsent: true,
},
},
},
{
ActionSpecifier: &routev3.RateLimit_Action_Metadata{
Metadata: &routev3.RateLimit_Action_MetaData{
Expand Down

0 comments on commit 4ba9169

Please sign in to comment.