Skip to content

Commit cb888ed

Browse files
committed
enhance(upstream): proxy target parsing from multiple configs
1 parent 44246c9 commit cb888ed

File tree

2 files changed

+196
-5
lines changed

2 files changed

+196
-5
lines changed

internal/upstream/proxy_parser.go renamed to internal/upstream/upstream_parser.go

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,42 @@ type UpstreamContext struct {
2525
}
2626

2727
// ParseProxyTargetsFromRawContent parses proxy targets from raw nginx configuration content
28+
// It performs two passes to handle cases where proxy_pass/grpc_pass directives appear before upstream definitions
29+
//
30+
// Problem: In nginx configuration files, the order of directives can vary. A proxy_pass directive
31+
// might reference an upstream block that is defined later in the file. Without two-pass parsing,
32+
// the parser would incorrectly treat such proxy_pass directives as direct targets instead of
33+
// upstream references.
34+
//
35+
// Solution:
36+
// 1. First pass: Scan the entire content to collect all upstream names
37+
// 2. Second pass: Parse upstream blocks and proxy_pass/grpc_pass directives with full knowledge
38+
// of all upstream names, allowing proper identification of upstream references regardless of order
2839
func ParseProxyTargetsFromRawContent(content string) []ProxyTarget {
2940
var targets []ProxyTarget
3041

31-
// First, collect all upstream names and their contexts
42+
// First pass: collect all upstream names to handle forward references
43+
// This ensures we know about all upstreams before processing proxy_pass directives
3244
upstreamNames := make(map[string]bool)
33-
upstreamContexts := make(map[string]*UpstreamContext)
34-
upstreamRegex := regexp.MustCompile(`(?s)upstream\s+([^\s]+)\s*\{([^}]+)\}`)
45+
upstreamRegex := regexp.MustCompile(`(?s)upstream\s+([^\s]+)\s*\{`)
3546
upstreamMatches := upstreamRegex.FindAllStringSubmatch(content, -1)
3647

37-
// Parse upstream blocks and collect upstream names
3848
for _, match := range upstreamMatches {
39-
if len(match) >= 3 {
49+
if len(match) >= 2 {
4050
upstreamName := match[1]
4151
upstreamNames[upstreamName] = true
52+
}
53+
}
54+
55+
// Second pass: parse upstream blocks with full context and collect upstream names
56+
upstreamContexts := make(map[string]*UpstreamContext)
57+
upstreamFullRegex := regexp.MustCompile(`(?s)upstream\s+([^\s]+)\s*\{([^}]+)\}`)
58+
upstreamFullMatches := upstreamFullRegex.FindAllStringSubmatch(content, -1)
59+
60+
// Parse upstream blocks and collect upstream names
61+
for _, match := range upstreamFullMatches {
62+
if len(match) >= 3 {
63+
upstreamName := match[1]
4264
upstreamContent := match[2]
4365

4466
// Create upstream context

internal/upstream/proxy_parser_test.go renamed to internal/upstream/upstream_parser_test.go

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,3 +753,172 @@ func TestGrpcPassPortDefaults(t *testing.T) {
753753
})
754754
}
755755
}
756+
757+
func TestParseProxyTargetsWithForwardReferences(t *testing.T) {
758+
// Test case where proxy_pass appears before upstream definition
759+
config := `server {
760+
listen 80;
761+
server_name example.com;
762+
763+
# This proxy_pass appears BEFORE the upstream definition
764+
location /api/ {
765+
proxy_pass http://api-backend/;
766+
}
767+
768+
location /grpc/ {
769+
grpc_pass grpc://grpc-backend;
770+
}
771+
772+
# Direct proxy_pass that should be included
773+
location /external/ {
774+
proxy_pass http://external.example.com:8080/;
775+
}
776+
}
777+
778+
# Upstream definitions appear AFTER the proxy_pass directives
779+
upstream api-backend {
780+
server 127.0.0.1:9000;
781+
server 127.0.0.1:9001;
782+
}
783+
784+
upstream grpc-backend {
785+
server 127.0.0.1:9090;
786+
server 127.0.0.1:9091;
787+
}`
788+
789+
targets := ParseProxyTargetsFromRawContent(config)
790+
791+
// Print actual results for debugging
792+
t.Logf("Found %d targets:", len(targets))
793+
for i, target := range targets {
794+
t.Logf("Target %d: Host=%s, Port=%s, Type=%s", i+1, target.Host, target.Port, target.Type)
795+
}
796+
797+
// Expected targets:
798+
// - 4 upstream servers (2 from api-backend, 2 from grpc-backend)
799+
// - 1 direct proxy_pass target (external.example.com:8080)
800+
// - proxy_pass to api-backend and grpc_pass to grpc-backend should be ignored since they reference upstreams
801+
expectedTargets := []ProxyTarget{
802+
{Host: "127.0.0.1", Port: "9000", Type: "upstream"},
803+
{Host: "127.0.0.1", Port: "9001", Type: "upstream"},
804+
{Host: "127.0.0.1", Port: "9090", Type: "upstream"},
805+
{Host: "127.0.0.1", Port: "9091", Type: "upstream"},
806+
{Host: "external.example.com", Port: "8080", Type: "proxy_pass"},
807+
}
808+
809+
if len(targets) != len(expectedTargets) {
810+
t.Errorf("Expected %d targets, got %d", len(expectedTargets), len(targets))
811+
return
812+
}
813+
814+
// Create a map for easier comparison
815+
targetMap := make(map[string]ProxyTarget)
816+
for _, target := range targets {
817+
key := target.Host + ":" + target.Port + ":" + target.Type
818+
targetMap[key] = target
819+
}
820+
821+
for _, expected := range expectedTargets {
822+
key := expected.Host + ":" + expected.Port + ":" + expected.Type
823+
if _, found := targetMap[key]; !found {
824+
t.Errorf("Expected target not found: %+v", expected)
825+
}
826+
}
827+
}
828+
829+
func TestParseProxyTargetsWithMixedOrderReferences(t *testing.T) {
830+
// Test case with mixed order: some proxy_pass before upstream, some after
831+
config := `# First server block with forward references
832+
server {
833+
listen 80;
834+
server_name app1.example.com;
835+
836+
# These proxy_pass directives appear BEFORE upstream definitions
837+
location /api/ {
838+
proxy_pass http://api-backend/;
839+
}
840+
841+
location /grpc/ {
842+
grpc_pass grpc://grpc-backend;
843+
}
844+
}
845+
846+
# First upstream definition
847+
upstream api-backend {
848+
server 127.0.0.1:9000;
849+
server 127.0.0.1:9001;
850+
}
851+
852+
# Second server block with backward references
853+
server {
854+
listen 443 ssl;
855+
server_name app2.example.com;
856+
857+
# These proxy_pass directives appear AFTER the first upstream definition
858+
# but BEFORE the second upstream definition
859+
location /api/ {
860+
proxy_pass https://api-backend/; # References already defined upstream
861+
}
862+
863+
location /cache/ {
864+
proxy_pass http://cache-backend/; # Forward reference to upcoming upstream
865+
}
866+
867+
# Direct proxy_pass that should be included
868+
location /external/ {
869+
proxy_pass https://external.example.com:8443/;
870+
}
871+
}
872+
873+
# Second upstream definition
874+
upstream cache-backend {
875+
server 127.0.0.1:6379;
876+
server 127.0.0.1:6380;
877+
}
878+
879+
upstream grpc-backend {
880+
server 127.0.0.1:9090;
881+
server 127.0.0.1:9091;
882+
}`
883+
884+
targets := ParseProxyTargetsFromRawContent(config)
885+
886+
// Print actual results for debugging
887+
t.Logf("Found %d targets:", len(targets))
888+
for i, target := range targets {
889+
t.Logf("Target %d: Host=%s, Port=%s, Type=%s", i+1, target.Host, target.Port, target.Type)
890+
}
891+
892+
// Expected targets:
893+
// - 6 upstream servers (2 from api-backend, 2 from cache-backend, 2 from grpc-backend)
894+
// - 1 direct proxy_pass target (external.example.com:8443)
895+
// - All proxy_pass/grpc_pass to upstreams should be ignored
896+
expectedTargets := []ProxyTarget{
897+
{Host: "127.0.0.1", Port: "9000", Type: "upstream"},
898+
{Host: "127.0.0.1", Port: "9001", Type: "upstream"},
899+
{Host: "127.0.0.1", Port: "6379", Type: "upstream"},
900+
{Host: "127.0.0.1", Port: "6380", Type: "upstream"},
901+
{Host: "127.0.0.1", Port: "9090", Type: "upstream"},
902+
{Host: "127.0.0.1", Port: "9091", Type: "upstream"},
903+
{Host: "external.example.com", Port: "8443", Type: "proxy_pass"},
904+
}
905+
906+
if len(targets) != len(expectedTargets) {
907+
t.Errorf("Expected %d targets, got %d", len(expectedTargets), len(targets))
908+
return
909+
}
910+
911+
// Create a map for easier comparison
912+
targetMap := make(map[string]ProxyTarget)
913+
for _, target := range targets {
914+
key := target.Host + ":" + target.Port + ":" + target.Type
915+
targetMap[key] = target
916+
}
917+
918+
for _, expected := range expectedTargets {
919+
key := expected.Host + ":" + expected.Port + ":" + expected.Type
920+
if _, found := targetMap[key]; !found {
921+
t.Errorf("Expected target not found: %+v", expected)
922+
}
923+
}
924+
}

0 commit comments

Comments
 (0)