From b754c1d693018844874d82fead9565fa2a713ab1 Mon Sep 17 00:00:00 2001 From: Valyria McFarland Date: Tue, 18 Feb 2025 09:39:36 -0800 Subject: [PATCH 1/2] NLB-8186: identify directives as parameters in map blocks Crossplane parses map-like directives by adding the parameters in their bodies to the payload as Directives. However, the parameters within the map blocks are not valid nginx directives; and therefore, it is useful to discern which Directive types that crossplane generates are nginx directives or map parameters. --- build_test.go | 3 +- parse.go | 1 + parse_test.go | 153 ++++++++++++++++++++++++++++---------------------- types.go | 5 ++ 4 files changed, 93 insertions(+), 69 deletions(-) diff --git a/build_test.go b/build_test.go index e75f83bc..2daf6973 100644 --- a/build_test.go +++ b/build_test.go @@ -595,7 +595,8 @@ func equalDirectives(d1, d2 *Directive) bool { !equalIncludes(d1.Includes, d2.Includes) || !equalBlocks(d1.Block, d2.Block) || (d1.Comment == nil) != (d2.Comment == nil) || - (d1.Comment != nil && *d1.Comment != *d2.Comment) { + (d1.Comment != nil && *d1.Comment != *d2.Comment) || + d1.IsMapBlockParameter != d2.IsMapBlockParameter { return false } diff --git a/parse.go b/parse.go index 2f3126f7..58302d31 100644 --- a/parse.go +++ b/parse.go @@ -314,6 +314,7 @@ func (p *parser) parse(parsing *Config, tokens <-chan NgxToken, ctx blockCtx, co } continue } + stmt.IsMapBlockParameter = true parsed = append(parsed, stmt) continue } diff --git a/parse_test.go b/parse_test.go index a245d052..efc4af5d 100644 --- a/parse_test.go +++ b/parse_test.go @@ -1048,16 +1048,18 @@ var parseFixtures = []parseFixture{ Line: 4, Block: Directives{ { - Directive: "default", - Args: []string{"0"}, - Line: 5, - Block: Directives{}, + Directive: "default", + Args: []string{"0"}, + Line: 5, + Block: Directives{}, + IsMapBlockParameter: true, }, { - Directive: "~Opera Mini", - Args: []string{"1"}, - Line: 6, - Block: Directives{}, + Directive: "~Opera Mini", + Args: []string{"1"}, + Line: 6, + Block: Directives{}, + IsMapBlockParameter: true, }, }, }, @@ -1067,16 +1069,18 @@ var parseFixtures = []parseFixture{ Line: 9, Block: Directives{ { - Directive: "C0", - Args: []string{"D18E"}, - Line: 10, - Block: Directives{}, + Directive: "C0", + Args: []string{"D18E"}, + Line: 10, + Block: Directives{}, + IsMapBlockParameter: true, }, { - Directive: "C1", - Args: []string{"D0B0"}, - Line: 11, - Block: Directives{}, + Directive: "C1", + Args: []string{"D0B0"}, + Line: 11, + Block: Directives{}, + IsMapBlockParameter: true, }, }, }, @@ -1183,16 +1187,18 @@ var parseFixtures = []parseFixture{ Line: 4, Block: Directives{ { - Directive: "default", - Args: []string{"0"}, - Line: 5, - Block: Directives{}, + Directive: "default", + Args: []string{"0"}, + Line: 5, + Block: Directives{}, + IsMapBlockParameter: true, }, { - Directive: "~Opera Mini", - Args: []string{"1"}, - Line: 6, - Block: Directives{}, + Directive: "~Opera Mini", + Args: []string{"1"}, + Line: 6, + Block: Directives{}, + IsMapBlockParameter: true, }, }, }, @@ -1238,28 +1244,32 @@ var parseFixtures = []parseFixture{ Line: 6, Block: Directives{ { - Directive: "ranges", - Args: []string{}, - Line: 7, - Block: Directives{}, + Directive: "ranges", + Args: []string{}, + Line: 7, + Block: Directives{}, + IsMapBlockParameter: true, }, { - Directive: "default", - Args: []string{"0"}, - Line: 8, - Block: Directives{}, + Directive: "default", + Args: []string{"0"}, + Line: 8, + Block: Directives{}, + IsMapBlockParameter: true, }, { - Directive: "192.168.1.0/24", - Args: []string{"1"}, - Line: 9, - Block: Directives{}, + Directive: "192.168.1.0/24", + Args: []string{"1"}, + Line: 9, + Block: Directives{}, + IsMapBlockParameter: true, }, { - Directive: "127.0.0.1", - Args: []string{"2"}, - Line: 10, - Block: Directives{}, + Directive: "127.0.0.1", + Args: []string{"2"}, + Line: 10, + Block: Directives{}, + IsMapBlockParameter: true, }, }, }, @@ -1325,16 +1335,18 @@ var parseFixtures = []parseFixture{ Line: 2, Block: Directives{ { - Directive: "text/html", - Args: []string{"html", "htm", "shtml"}, - Line: 3, - Block: Directives{}, + Directive: "text/html", + Args: []string{"html", "htm", "shtml"}, + Line: 3, + Block: Directives{}, + IsMapBlockParameter: true, }, { - Directive: "text/css", - Args: []string{"css"}, - Line: 4, - Block: Directives{}, + Directive: "text/css", + Args: []string{"css"}, + Line: 4, + Block: Directives{}, + IsMapBlockParameter: true, }, }, }, @@ -2131,16 +2143,18 @@ var parseFixtures = []parseFixture{ Args: []string{"/etc/Geo/GeoLite2-City.mmdb"}, Block: Directives{ { - Directive: "auto_reload", - Args: []string{"5s"}, - Line: 3, - Block: Directives{}, + Directive: "auto_reload", + Args: []string{"5s"}, + Line: 3, + Block: Directives{}, + IsMapBlockParameter: true, }, { - Directive: "$geoip2_city_name", - Args: []string{"city", "names", "en"}, - Line: 4, - Block: Directives{}, + Directive: "$geoip2_city_name", + Args: []string{"city", "names", "en"}, + Line: 4, + Block: Directives{}, + IsMapBlockParameter: true, }, }, }, @@ -2203,10 +2217,11 @@ var parseFixtures = []parseFixture{ Args: []string{"/etc/Geo/GeoLite2-Country.mmdb"}, Block: Directives{ { - Directive: "$geoip2_country_name", - Args: []string{"country", "names", "en"}, - Line: 20, - Block: Directives{}, + Directive: "$geoip2_country_name", + Args: []string{"country", "names", "en"}, + Line: 20, + Block: Directives{}, + IsMapBlockParameter: true, }, }, }, @@ -2216,16 +2231,18 @@ var parseFixtures = []parseFixture{ Args: []string{"$geoip2_country_name", "$backend"}, Block: Directives{ { - Directive: "United States", - Args: []string{"us_backend"}, - Line: 24, - Block: Directives{}, + Directive: "United States", + Args: []string{"us_backend"}, + Line: 24, + Block: Directives{}, + IsMapBlockParameter: true, }, { - Directive: "default", - Args: []string{"default_backend"}, - Line: 25, - Block: Directives{}, + Directive: "default", + Args: []string{"default_backend"}, + Line: 25, + Block: Directives{}, + IsMapBlockParameter: true, }, }, }, diff --git a/types.go b/types.go index 334216de..5d6dc64c 100644 --- a/types.go +++ b/types.go @@ -45,7 +45,10 @@ type Directive struct { Includes []int `json:"includes,omitempty"` Block Directives `json:"block,omitempty"` Comment *string `json:"comment,omitempty"` + // IsMapBlockParameter is true if the directive represents a parameter in the body of a "map-like" directive. + IsMapBlockParameter bool `json:"mapBlockParameter,omitempty"` } + type Directives []*Directive // IsBlock returns true if this is a block directive. @@ -110,6 +113,8 @@ func (d *Directive) Equal(a *Directive) bool { return false case a.File != d.File: return false + case a.IsMapBlockParameter != d.IsMapBlockParameter: + return false } for i, inc := range a.Includes { if inc != d.Includes[i] { From 722cbd1ba01b6c0becf1df5b6f72e041331cdeff Mon Sep 17 00:00:00 2001 From: Valyria McFarland Date: Tue, 18 Feb 2025 10:27:09 -0800 Subject: [PATCH 2/2] fix: pin golangci-lint version to v1 From golangci-lint's changelog: "[v1.64.2] is the last minor release of golangci-lint v1. The next release will be golangci-lint v2." The repo's current golangci.yaml configuration contains deprecated fields which break when using v2. This pins the golangci-lint version until we can update the configuration to be compatable with v2. --- .github/workflows/cicd.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/cicd.yml b/.github/workflows/cicd.yml index fa08467d..5f1eb033 100644 --- a/.github/workflows/cicd.yml +++ b/.github/workflows/cicd.yml @@ -28,7 +28,9 @@ jobs: - name: Lint Code uses: golangci/golangci-lint-action@v6 with: + version: v1.64.2 only-new-issues: true + verify: false unit-tests: name: Unit Tests