Skip to content

Commit e4b94bc

Browse files
committed
Add pathRuleIdx to epp location
1 parent 6127cbf commit e4b94bc

File tree

7 files changed

+393
-263
lines changed

7 files changed

+393
-263
lines changed

internal/controller/nginx/config/servers.go

Lines changed: 70 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference {
319319
}
320320
321321
location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 {
322+
internal;
322323
set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference;
323324
js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference
324325
}
@@ -329,6 +330,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference {
329330
}
330331
331332
location /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0 {
333+
internal;
332334
set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference;
333335
js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference
334336
}
@@ -388,6 +390,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend0-inference {
388390
}
389391
390392
location /_ngf-internal-upstreamNamePrimary-routeNS-routeName-rule0 {
393+
internal;
391394
set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend0-inference;
392395
js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend0-inference
393396
}
@@ -398,6 +401,7 @@ location /_ngf-internal-proxy-pass-rule0-route0-backend1-inference {
398401
}
399402
400403
location /_ngf-internal-upstreamNameSecondary-routeNS-routeName-rule0 {
404+
internal;
401405
set $epp_internal_path /_ngf-internal-proxy-pass-rule0-route0-backend1-inference;
402406
js_content epp.getEndpoint; // gets endpoint and redirects to /_ngf-internal-proxy-pass-rule0-route0-backend1-inference
403407
}
@@ -536,9 +540,16 @@ func createInternalLocationsForRule(
536540

537541
internalLocations := make([]http.Location, 0, capacity)
538542
matches := make([]routeMatch, 0, len(rule.MatchRules))
543+
544+
// If there are multiple matches on a single route rule, they will share the same intEPPLocation and
545+
// intProxyPassLocation. To avoid creating duplicates, we track the unique names here.
546+
uniqueEPPNameMap := make(map[string]struct{})
547+
539548
for matchRuleIdx, r := range rule.MatchRules {
540549
var intLocation http.Location
541550
var match routeMatch
551+
skipMatch := false
552+
542553
if !rule.HasInferenceBackends {
543554
intLocation, match = initializeInternalMatchLocation(pathRuleIdx, matchRuleIdx, r.Match, rule.GRPC)
544555
intLocation.Includes = createIncludesFromPolicyGenerateResult(
@@ -572,23 +583,7 @@ func createInternalLocationsForRule(
572583
//
573584
// The match needs to point to the internal inference location which calls the EPP NJS module (intEPPLocation)
574585

575-
if len(r.BackendGroup.Backends) > 1 {
576-
intSplitClientsLocation := initializeInternalInferenceSplitClientsLocation(pathRuleIdx, matchRuleIdx)
577-
intSplitClientsLocation.Includes = createIncludesFromPolicyGenerateResult(
578-
generator.GenerateForInternalLocation(rule.Policies),
579-
)
580-
581-
splitClientsVariableName := createInferenceSplitClientsVariableName(
582-
convertStringToSafeVariableName(r.BackendGroup.Name()),
583-
)
584-
intSplitClientsLocation.Rewrites = append(intSplitClientsLocation.Rewrites,
585-
fmt.Sprintf("^ $%s last", splitClientsVariableName))
586-
587-
internalLocations = append(internalLocations, intSplitClientsLocation)
588-
589-
match = createRouteMatch(r.Match, intSplitClientsLocation.Path)
590-
}
591-
586+
var intEPPLocation http.Location
592587
for backendIdx, b := range r.BackendGroup.Backends {
593588
intProxyPassLocation := initializeInternalInferenceProxyPassLocation(pathRuleIdx, matchRuleIdx, backendIdx)
594589
intProxyPassLocation.Includes = createIncludesFromPolicyGenerateResult(
@@ -609,22 +604,61 @@ func createInternalLocationsForRule(
609604
keepAliveCheck,
610605
mirrorPercentage,
611606
)
612-
internalLocations = append(internalLocations, intProxyPassLocation)
613607

614-
intEPPLocation := initializeInternalInferenceEPPLocation(b, r.BackendGroup.Source, r.BackendGroup.RuleIdx)
608+
intEPPLocation = initializeInternalInferenceEPPLocation(
609+
b,
610+
r.BackendGroup.Source,
611+
r.BackendGroup.RuleIdx,
612+
r.BackendGroup.PathRuleIdx,
613+
)
615614
intEPPLocation.Includes = createIncludesFromPolicyGenerateResult(
616615
generator.GenerateForInternalLocation(rule.Policies),
617616
)
618617

619-
eppHost, portNum := extractEPPConfig(b)
620-
intEPPLocation = setLocationEPPConfig(intEPPLocation, intProxyPassLocation.Path, eppHost, portNum)
621-
internalLocations = append(internalLocations, intEPPLocation)
618+
mapKey := intEPPLocation.Path // Use this as the key to detect duplicates
622619

623-
if len(r.BackendGroup.Backends) == 1 {
624-
match = createRouteMatch(r.Match, intEPPLocation.Path)
620+
// The only time this happens is on a single route rule with multiple matches with the same path.
621+
// In this case, we only need to create one set of internal locations, and can skip the duplicate matches.
622+
if _, exists := uniqueEPPNameMap[mapKey]; exists {
623+
skipMatch = true
624+
break
625+
}
626+
uniqueEPPNameMap[mapKey] = struct{}{}
627+
// we only append intEPPLocation and intProxyPassLocation once per unique intEPPLocation name
628+
internalLocations = append(internalLocations, intProxyPassLocation)
629+
630+
if b.EndpointPickerConfig != nil && b.EndpointPickerConfig.EndpointPickerRef != nil {
631+
eppHost, portNum := extractEPPConfig(b)
632+
intEPPLocation = setLocationEPPConfig(intEPPLocation, intProxyPassLocation.Path, eppHost, portNum)
633+
internalLocations = append(internalLocations, intEPPLocation)
625634
}
626635
}
636+
637+
// skip adding match and creating split clients location if its a duplicate intEPPLocation.Path
638+
if skipMatch {
639+
continue
640+
}
641+
642+
if len(r.BackendGroup.Backends) > 1 {
643+
intSplitClientsLocation := initializeInternalInferenceSplitClientsLocation(pathRuleIdx, matchRuleIdx)
644+
intSplitClientsLocation.Includes = createIncludesFromPolicyGenerateResult(
645+
generator.GenerateForInternalLocation(rule.Policies),
646+
)
647+
648+
splitClientsVariableName := createInferenceSplitClientsVariableName(
649+
convertStringToSafeVariableName(r.BackendGroup.Name()),
650+
)
651+
intSplitClientsLocation.Rewrites = append(intSplitClientsLocation.Rewrites,
652+
fmt.Sprintf("^ $%s last", splitClientsVariableName))
653+
654+
internalLocations = append(internalLocations, intSplitClientsLocation)
655+
656+
match = createRouteMatch(r.Match, intSplitClientsLocation.Path)
657+
} else {
658+
match = createRouteMatch(r.Match, intEPPLocation.Path)
659+
}
627660
}
661+
628662
matches = append(matches, match)
629663
}
630664

@@ -657,6 +691,8 @@ func createInferenceLocationsForRule(
657691

658692
locs := make([]http.Location, 0, capacity)
659693

694+
// There will only be one rule.MatchRules, since if there are multiple, createInternalLocationsForRule
695+
// would have been called instead.
660696
for matchRuleIdx, r := range rule.MatchRules {
661697
// If there are multiple inference backends, we need 3 locations:
662698
// 1. (external location) external location which rewrites to the EPP internal location based on a
@@ -706,7 +742,12 @@ func createInferenceLocationsForRule(
706742
eppHost, portNum := extractEPPConfig(b)
707743

708744
if len(r.BackendGroup.Backends) > 1 {
709-
intEPPLocation := initializeInternalInferenceEPPLocation(b, r.BackendGroup.Source, r.BackendGroup.RuleIdx)
745+
intEPPLocation := initializeInternalInferenceEPPLocation(
746+
b,
747+
r.BackendGroup.Source,
748+
r.BackendGroup.RuleIdx,
749+
r.BackendGroup.PathRuleIdx,
750+
)
710751
intEPPLocation.Includes = createIncludesFromPolicyGenerateResult(
711752
generator.GenerateForInternalLocation(rule.Policies),
712753
)
@@ -918,17 +959,19 @@ func initializeInternalMatchLocation(
918959
func initializeInternalInferenceEPPLocation(
919960
b dataplane.Backend,
920961
source types.NamespacedName,
921-
ruleIdx int,
962+
ruleIdx,
963+
pathruleIdx int,
922964
) http.Location {
923965
return http.Location{
924966
// This path needs to be recreated in the split_clients directive generation to match correctly.
925967
Path: fmt.Sprintf(
926-
"%s-%s-%s-%s-rule%d",
968+
"%s-%s-%s-%s-routeRule%d-pathRule%d",
927969
http.InternalRoutePathPrefix,
928970
b.UpstreamName,
929971
source.Namespace,
930972
source.Name,
931973
ruleIdx,
974+
pathruleIdx,
932975
),
933976
Type: http.InferenceInternalLocationType,
934977
}

0 commit comments

Comments
 (0)