From 82fc344c77550e05d58d61fe7c4f6512a891653d Mon Sep 17 00:00:00 2001 From: dhurley Date: Tue, 1 Aug 2023 16:06:51 +0100 Subject: [PATCH] Update how the sdk parses nginx api location and server directives --- sdk/config_helpers.go | 53 ++++-- sdk/config_helpers_test.go | 179 ++++++++++-------- .../nginx/agent/sdk/v2/config_helpers.go | 53 ++++-- .../nginx/agent/sdk/v2/config_helpers.go | 53 ++++-- .../nginx/agent/sdk/v2/config_helpers.go | 53 ++++-- 5 files changed, 237 insertions(+), 154 deletions(-) diff --git a/sdk/config_helpers.go b/sdk/config_helpers.go index fe85ae6c1..e83593b6e 100644 --- a/sdk/config_helpers.go +++ b/sdk/config_helpers.go @@ -662,46 +662,61 @@ func parseStatusAPIEndpoints(parent *crossplane.Directive, current *crossplane.D if locChild.Directive != plusAPIDirective && locChild.Directive != ossAPIDirective { continue } - host := parseServerHost(parent) - path := parseLocationPath(current) - switch locChild.Directive { - case plusAPIDirective: - plusUrls = append(plusUrls, fmt.Sprintf(apiFormat, host, path)) - case ossAPIDirective: - ossUrls = append(ossUrls, fmt.Sprintf(apiFormat, host, path)) + + addresses := parseAddressesFromServerDirective(parent) + + for _, address := range addresses { + path := parsePathFromLocationDirective(current) + + switch locChild.Directive { + case plusAPIDirective: + plusUrls = append(plusUrls, fmt.Sprintf(apiFormat, address, path)) + case ossAPIDirective: + ossUrls = append(ossUrls, fmt.Sprintf(apiFormat, address, path)) + } } } return plusUrls, ossUrls } -func parseServerHost(parent *crossplane.Directive) string { - listenPort := "80" - serverName := "localhost" +func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { + addresses := []string{} + port := "80" + for _, dir := range parent.Block { + address := "127.0.0.1" + switch dir.Directive { case "listen": - host, port, err := net.SplitHostPort(dir.Args[0]) + host, listenPort, err := net.SplitHostPort(dir.Args[0]) if err == nil { - if host != "*" && host != "::" && host != "" { - serverName = host + if host == "*" || host == "" { + address = "127.0.0.1" + } else if host == "::" || host == "::1" { + address = "[::1]" + } else { + address = host } - listenPort = port + port = listenPort } else { if isPort(dir.Args[0]) { - listenPort = dir.Args[0] + port = dir.Args[0] } else { - serverName = dir.Args[0] + address = dir.Args[0] } } + addresses = append(addresses, fmt.Sprintf("%s:%s", address, port)) case "server_name": if dir.Args[0] == "_" { // default server continue } - serverName = dir.Args[0] + address = dir.Args[0] + addresses = append(addresses, fmt.Sprintf("%s:%s", address, port)) } } - return fmt.Sprintf("%s:%s", serverName, listenPort) + + return addresses } func isPort(value string) bool { @@ -709,7 +724,7 @@ func isPort(value string) bool { return err == nil && port >= 1 && port <= 65535 } -func parseLocationPath(location *crossplane.Directive) string { +func parsePathFromLocationDirective(location *crossplane.Directive) string { path := "/" if len(location.Args) > 0 { if location.Args[0] != "=" { diff --git a/sdk/config_helpers_test.go b/sdk/config_helpers_test.go index a9f57bbb9..5601b79de 100644 --- a/sdk/config_helpers_test.go +++ b/sdk/config_helpers_test.go @@ -141,8 +141,8 @@ var tests = []struct { ssl_certificate /tmp/testdata/nginx/ca.crt; server { - server_name localhost; listen 127.0.0.1:80; + server_name localhost; error_page 500 502 503 504 /50x.html; # ssl_certificate /usr/local/nginx/conf/cert.pem; @@ -298,7 +298,7 @@ var tests = []struct { }, Zconfig: &proto.ZippedFile{ Contents: []uint8{31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 1, 0, 0, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0}, - Checksum: "5da60539dbedfe08011646f96b964af9be68dcd3bdb7b6cc2d64c06723bba659", + Checksum: "9e05884e8c5d1db866d1b4a55fbef7f8b54b88b7a2e0a17bc7a11e6ff5b5b500", RootDirectory: "/tmp/testdata/nginx", }, }, @@ -340,8 +340,8 @@ var tests = []struct { ssl_certificate /tmp/testdata/nginx/ca.crt; server { - server_name localhost; listen 127.0.0.1:80; + server_name localhost; error_page 500 502 503 504 /50x.html; # ssl_certificate /usr/local/nginx/conf/cert.pem; @@ -443,7 +443,7 @@ var tests = []struct { }, Zconfig: &proto.ZippedFile{ Contents: []uint8{31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 1, 0, 0, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0}, - Checksum: "1b6422f8a17527b2e9f255b7362ab7c320cd4a2efea7bff3e402438e5877f00e", + Checksum: "433837974aaab32148c9bb034f1c59ab878482af506a13d9a834a8a99384efbd", RootDirectory: "/tmp/testdata/nginx", }, }, @@ -795,6 +795,7 @@ func TestParseStatusAPIEndpoints(t *testing.T) { }{ { plus: []string{ + "http://127.0.0.1:80/api/", "http://localhost:80/api/", }, conf: ` @@ -811,7 +812,7 @@ server { }, { plus: []string{ - "http://localhost:80/api/", + "http://127.0.0.1:80/api/", }, conf: ` server { @@ -826,7 +827,7 @@ server { }, { plus: []string{ - "http://localhost:80/api/", + "http://127.0.0.1:80/api/", }, conf: ` server { @@ -838,6 +839,23 @@ server { deny all; } } +`, + }, + { + plus: []string{ + "http://127.0.0.1:8888/api/", + "http://status.internal.com:8888/api/", + }, + conf: ` +server { + listen 8888 default_server; + server_name status.internal.com; + location /api/ { + api write=on; + allow 127.0.0.1; + deny all; + } +} `, }, { @@ -845,19 +863,20 @@ server { "http://127.0.0.1:8080/privateapi", }, conf: ` - server { - listen 127.0.0.1:8080; - location /privateapi { - api write=on; - allow 127.0.0.1; - deny all; - } - } + server { + listen 127.0.0.1:8080; + location /privateapi { + api write=on; + allow 127.0.0.1; + deny all; + } + } `, }, { plus: []string{ - "http://localhost:80/api/", + "http://127.0.0.1:80/api/", + "http://[::1]:80/api/", }, conf: ` server { @@ -906,7 +925,7 @@ server { }, { plus: []string{ - "http://localhost:80/api/", + "http://127.0.0.1:80/api/", }, conf: ` server { @@ -922,7 +941,7 @@ server { }, { plus: []string{ - "http://localhost:80/api/", + "http://127.0.0.1:80/api/", }, conf: ` server { @@ -970,7 +989,7 @@ server { }, { plus: []string{ - "http://localhost:8000/api/", + "http://[::1]:8000/api/", }, conf: ` server { @@ -986,90 +1005,94 @@ server { }, { oss: []string{ + "http://localhost:80/stub_status", "http://127.0.0.1:80/stub_status", }, conf: ` - server { - server_name localhost; - listen 127.0.0.1:80; - - error_page 500 502 503 504 /50x.html; - # ssl_certificate /usr/local/nginx/conf/cert.pem; - - location / { - root /tmp/testdata/foo; - } - - location /stub_status { - stub_status; - } - } +server { + server_name localhost; + listen 127.0.0.1:80; + + error_page 500 502 503 504 /50x.html; + # ssl_certificate /usr/local/nginx/conf/cert.pem; + + location / { + root /tmp/testdata/foo; + } + + location /stub_status { + stub_status; + } +} `, }, { oss: []string{ "http://localhost:80/stub_status", + "http://127.0.0.1:80/stub_status", }, conf: ` - server { - server_name localhost; - listen :80; - - error_page 500 502 503 504 /50x.html; - # ssl_certificate /usr/local/nginx/conf/cert.pem; - - location / { - root /tmp/testdata/foo; - } - - location /stub_status { - stub_status; - } - } +server { + server_name localhost; + listen :80; + + error_page 500 502 503 504 /50x.html; + # ssl_certificate /usr/local/nginx/conf/cert.pem; + + location / { + root /tmp/testdata/foo; + } + + location /stub_status { + stub_status; + } +} `, }, { oss: []string{ "http://localhost:80/stub_status", + "http://127.0.0.1:80/stub_status", }, conf: ` - server { - server_name localhost; - listen 80; - - error_page 500 502 503 504 /50x.html; - # ssl_certificate /usr/local/nginx/conf/cert.pem; - - location / { - root /tmp/testdata/foo; - } - - location /stub_status { - stub_status; - } - } +server { + server_name localhost; + listen 80; + + error_page 500 502 503 504 /50x.html; + # ssl_certificate /usr/local/nginx/conf/cert.pem; + + location / { + root /tmp/testdata/foo; + } + + location /stub_status { + stub_status; + } +} `, }, { oss: []string{ "http://localhost:80/stub_status", + "http://127.0.0.1:80/stub_status", }, conf: ` - server { - server_name localhost; - listen 80; - - error_page 500 502 503 504 /50x.html; - # ssl_certificate /usr/local/nginx/conf/cert.pem; - - location / { - root /tmp/testdata/foo; - } - - location = /stub_status { - stub_status; - } - } +server { + server_name localhost; + listen 80; + + error_page 500 502 503 504 /50x.html; + # ssl_certificate /usr/local/nginx/conf/cert.pem; + + location / { + root /tmp/testdata/foo; + } + + location = /stub_status { + stub_status; + } +} `, }, } { diff --git a/test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go b/test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go index fe85ae6c1..e83593b6e 100644 --- a/test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go +++ b/test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go @@ -662,46 +662,61 @@ func parseStatusAPIEndpoints(parent *crossplane.Directive, current *crossplane.D if locChild.Directive != plusAPIDirective && locChild.Directive != ossAPIDirective { continue } - host := parseServerHost(parent) - path := parseLocationPath(current) - switch locChild.Directive { - case plusAPIDirective: - plusUrls = append(plusUrls, fmt.Sprintf(apiFormat, host, path)) - case ossAPIDirective: - ossUrls = append(ossUrls, fmt.Sprintf(apiFormat, host, path)) + + addresses := parseAddressesFromServerDirective(parent) + + for _, address := range addresses { + path := parsePathFromLocationDirective(current) + + switch locChild.Directive { + case plusAPIDirective: + plusUrls = append(plusUrls, fmt.Sprintf(apiFormat, address, path)) + case ossAPIDirective: + ossUrls = append(ossUrls, fmt.Sprintf(apiFormat, address, path)) + } } } return plusUrls, ossUrls } -func parseServerHost(parent *crossplane.Directive) string { - listenPort := "80" - serverName := "localhost" +func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { + addresses := []string{} + port := "80" + for _, dir := range parent.Block { + address := "127.0.0.1" + switch dir.Directive { case "listen": - host, port, err := net.SplitHostPort(dir.Args[0]) + host, listenPort, err := net.SplitHostPort(dir.Args[0]) if err == nil { - if host != "*" && host != "::" && host != "" { - serverName = host + if host == "*" || host == "" { + address = "127.0.0.1" + } else if host == "::" || host == "::1" { + address = "[::1]" + } else { + address = host } - listenPort = port + port = listenPort } else { if isPort(dir.Args[0]) { - listenPort = dir.Args[0] + port = dir.Args[0] } else { - serverName = dir.Args[0] + address = dir.Args[0] } } + addresses = append(addresses, fmt.Sprintf("%s:%s", address, port)) case "server_name": if dir.Args[0] == "_" { // default server continue } - serverName = dir.Args[0] + address = dir.Args[0] + addresses = append(addresses, fmt.Sprintf("%s:%s", address, port)) } } - return fmt.Sprintf("%s:%s", serverName, listenPort) + + return addresses } func isPort(value string) bool { @@ -709,7 +724,7 @@ func isPort(value string) bool { return err == nil && port >= 1 && port <= 65535 } -func parseLocationPath(location *crossplane.Directive) string { +func parsePathFromLocationDirective(location *crossplane.Directive) string { path := "/" if len(location.Args) > 0 { if location.Args[0] != "=" { diff --git a/test/performance/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go b/test/performance/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go index fe85ae6c1..e83593b6e 100644 --- a/test/performance/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go +++ b/test/performance/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go @@ -662,46 +662,61 @@ func parseStatusAPIEndpoints(parent *crossplane.Directive, current *crossplane.D if locChild.Directive != plusAPIDirective && locChild.Directive != ossAPIDirective { continue } - host := parseServerHost(parent) - path := parseLocationPath(current) - switch locChild.Directive { - case plusAPIDirective: - plusUrls = append(plusUrls, fmt.Sprintf(apiFormat, host, path)) - case ossAPIDirective: - ossUrls = append(ossUrls, fmt.Sprintf(apiFormat, host, path)) + + addresses := parseAddressesFromServerDirective(parent) + + for _, address := range addresses { + path := parsePathFromLocationDirective(current) + + switch locChild.Directive { + case plusAPIDirective: + plusUrls = append(plusUrls, fmt.Sprintf(apiFormat, address, path)) + case ossAPIDirective: + ossUrls = append(ossUrls, fmt.Sprintf(apiFormat, address, path)) + } } } return plusUrls, ossUrls } -func parseServerHost(parent *crossplane.Directive) string { - listenPort := "80" - serverName := "localhost" +func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { + addresses := []string{} + port := "80" + for _, dir := range parent.Block { + address := "127.0.0.1" + switch dir.Directive { case "listen": - host, port, err := net.SplitHostPort(dir.Args[0]) + host, listenPort, err := net.SplitHostPort(dir.Args[0]) if err == nil { - if host != "*" && host != "::" && host != "" { - serverName = host + if host == "*" || host == "" { + address = "127.0.0.1" + } else if host == "::" || host == "::1" { + address = "[::1]" + } else { + address = host } - listenPort = port + port = listenPort } else { if isPort(dir.Args[0]) { - listenPort = dir.Args[0] + port = dir.Args[0] } else { - serverName = dir.Args[0] + address = dir.Args[0] } } + addresses = append(addresses, fmt.Sprintf("%s:%s", address, port)) case "server_name": if dir.Args[0] == "_" { // default server continue } - serverName = dir.Args[0] + address = dir.Args[0] + addresses = append(addresses, fmt.Sprintf("%s:%s", address, port)) } } - return fmt.Sprintf("%s:%s", serverName, listenPort) + + return addresses } func isPort(value string) bool { @@ -709,7 +724,7 @@ func isPort(value string) bool { return err == nil && port >= 1 && port <= 65535 } -func parseLocationPath(location *crossplane.Directive) string { +func parsePathFromLocationDirective(location *crossplane.Directive) string { path := "/" if len(location.Args) > 0 { if location.Args[0] != "=" { diff --git a/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go b/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go index fe85ae6c1..e83593b6e 100644 --- a/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go +++ b/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go @@ -662,46 +662,61 @@ func parseStatusAPIEndpoints(parent *crossplane.Directive, current *crossplane.D if locChild.Directive != plusAPIDirective && locChild.Directive != ossAPIDirective { continue } - host := parseServerHost(parent) - path := parseLocationPath(current) - switch locChild.Directive { - case plusAPIDirective: - plusUrls = append(plusUrls, fmt.Sprintf(apiFormat, host, path)) - case ossAPIDirective: - ossUrls = append(ossUrls, fmt.Sprintf(apiFormat, host, path)) + + addresses := parseAddressesFromServerDirective(parent) + + for _, address := range addresses { + path := parsePathFromLocationDirective(current) + + switch locChild.Directive { + case plusAPIDirective: + plusUrls = append(plusUrls, fmt.Sprintf(apiFormat, address, path)) + case ossAPIDirective: + ossUrls = append(ossUrls, fmt.Sprintf(apiFormat, address, path)) + } } } return plusUrls, ossUrls } -func parseServerHost(parent *crossplane.Directive) string { - listenPort := "80" - serverName := "localhost" +func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { + addresses := []string{} + port := "80" + for _, dir := range parent.Block { + address := "127.0.0.1" + switch dir.Directive { case "listen": - host, port, err := net.SplitHostPort(dir.Args[0]) + host, listenPort, err := net.SplitHostPort(dir.Args[0]) if err == nil { - if host != "*" && host != "::" && host != "" { - serverName = host + if host == "*" || host == "" { + address = "127.0.0.1" + } else if host == "::" || host == "::1" { + address = "[::1]" + } else { + address = host } - listenPort = port + port = listenPort } else { if isPort(dir.Args[0]) { - listenPort = dir.Args[0] + port = dir.Args[0] } else { - serverName = dir.Args[0] + address = dir.Args[0] } } + addresses = append(addresses, fmt.Sprintf("%s:%s", address, port)) case "server_name": if dir.Args[0] == "_" { // default server continue } - serverName = dir.Args[0] + address = dir.Args[0] + addresses = append(addresses, fmt.Sprintf("%s:%s", address, port)) } } - return fmt.Sprintf("%s:%s", serverName, listenPort) + + return addresses } func isPort(value string) bool { @@ -709,7 +724,7 @@ func isPort(value string) bool { return err == nil && port >= 1 && port <= 65535 } -func parseLocationPath(location *crossplane.Directive) string { +func parsePathFromLocationDirective(location *crossplane.Directive) string { path := "/" if len(location.Args) > 0 { if location.Args[0] != "=" {