From 0a95caef6efaedc46d4739ef4916f972c86ecd86 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Tue, 26 Oct 2021 16:14:49 +0800 Subject: [PATCH 1/5] chore: rename module --- cmd/lagoon-linter/validate.go | 2 +- go.mod | 2 +- internal/lagoonyml/lint_test.go | 2 +- internal/lagoonyml/routeannotation_test.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/lagoon-linter/validate.go b/cmd/lagoon-linter/validate.go index 8fa5324..0f062a2 100644 --- a/cmd/lagoon-linter/validate.go +++ b/cmd/lagoon-linter/validate.go @@ -1,6 +1,6 @@ package main -import "github.com/amazeeio/lagoon-linter/internal/lagoonyml" +import "github.com/uselagoon/lagoon-linter/internal/lagoonyml" // ValidateCmd represents the validate command. type ValidateCmd struct { diff --git a/go.mod b/go.mod index 53d5f16..e1c7406 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/amazeeio/lagoon-linter +module github.com/uselagoon/lagoon-linter go 1.17 diff --git a/internal/lagoonyml/lint_test.go b/internal/lagoonyml/lint_test.go index 2e4571d..d4a1af6 100644 --- a/internal/lagoonyml/lint_test.go +++ b/internal/lagoonyml/lint_test.go @@ -3,7 +3,7 @@ package lagoonyml_test import ( "testing" - "github.com/amazeeio/lagoon-linter/internal/lagoonyml" + "github.com/uselagoon/lagoon-linter/internal/lagoonyml" ) func TestLint(t *testing.T) { diff --git a/internal/lagoonyml/routeannotation_test.go b/internal/lagoonyml/routeannotation_test.go index 1d6f7b8..170a0d7 100644 --- a/internal/lagoonyml/routeannotation_test.go +++ b/internal/lagoonyml/routeannotation_test.go @@ -3,7 +3,7 @@ package lagoonyml_test import ( "testing" - "github.com/amazeeio/lagoon-linter/internal/lagoonyml" + "github.com/uselagoon/lagoon-linter/internal/lagoonyml" ) func TestRouteAnnotation(t *testing.T) { From 015a2d874466abe74a920abd0543c0f5c7c290db Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Tue, 26 Oct 2021 16:15:35 +0800 Subject: [PATCH 2/5] fix: actually use the commandline --lagoon-yaml value --- cmd/lagoon-linter/validate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/lagoon-linter/validate.go b/cmd/lagoon-linter/validate.go index 0f062a2..cf48201 100644 --- a/cmd/lagoon-linter/validate.go +++ b/cmd/lagoon-linter/validate.go @@ -9,5 +9,5 @@ type ValidateCmd struct { // Run the validation of the Lagoon YAML. func (cmd *ValidateCmd) Run() error { - return lagoonyml.Lint(`.lagoon.yml`, lagoonyml.RouteAnnotation()) + return lagoonyml.Lint(cmd.LagoonYAML, lagoonyml.RouteAnnotation()) } From 33667a32b08f6706724b9998a268a27157e7ba47 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Tue, 26 Oct 2021 16:56:56 +0800 Subject: [PATCH 3/5] fix: improve validation of server-snippet --- internal/lagoonyml/routeannotation.go | 41 ++++++++++++---------- internal/lagoonyml/routeannotation_test.go | 6 +++- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/internal/lagoonyml/routeannotation.go b/internal/lagoonyml/routeannotation.go index 4235c7d..e5aa922 100644 --- a/internal/lagoonyml/routeannotation.go +++ b/internal/lagoonyml/routeannotation.go @@ -3,16 +3,30 @@ package lagoonyml import ( "fmt" "regexp" + "strings" ) // ServerSnippet is the annotation for server snippets with ingress-nginx. const ServerSnippet = "nginx.ingress.kubernetes.io/server-snippet" // validSnippets is the allow-list of snippets that Lagoon will accept. -var validSnippets []*regexp.Regexp = []*regexp.Regexp{ - regexp.MustCompile(`^(rewrite +[^; ]+ +[^; ]+( (last|break|redirect|permanent))?;\n?)+$`), - regexp.MustCompile(`^(add_header +[^; ]+ +[^;]+;\n?)+$`), - regexp.MustCompile(`^(set_real_ip_from +[^; ]+;\n?)+$`), +var validServerSnippets = regexp.MustCompile( + `^(rewrite +[^; ]+ +[^; ]+( (last|break|redirect|permanent))?;|` + + `add_header +[^; ]+ +[^;]+;|` + + `set_real_ip_from +[^; ]+;|` + + ` )+$`) + +// validate returns true if the annotations are valid, and false otherwise. +func validate(annotations map[string]string, r *regexp.Regexp, annotation string) (string, bool) { + if ss, ok := annotations[annotation]; ok { + for _, line := range strings.Split(ss, "\n") { + line = strings.TrimSpace(line) + if len(line) > 0 && !r.MatchString(line) { + return line, false + } + } + } + return "", true } // RouteAnnotation checks for valid annotations on defined routes. @@ -22,20 +36,11 @@ func RouteAnnotation() Linter { for _, routeMap := range e.Routes { for rName, lagoonRoutes := range routeMap { for _, lagoonRoute := range lagoonRoutes { - for iName, route := range lagoonRoute.Ingresses { - if ss, ok := route.Annotations[ServerSnippet]; ok { - valid := false - for _, v := range validSnippets { - if v.MatchString(ss) { - valid = true - break - } - } - if !valid { - return fmt.Errorf( - "invalid %s annotation on environment %s, route %s, ingress %s: %s", - ServerSnippet, eName, rName, iName, ss) - } + for iName, ingress := range lagoonRoute.Ingresses { + if annotation, ok := validate(ingress.Annotations, validServerSnippets, ServerSnippet); !ok { + return fmt.Errorf( + "invalid %s annotation on environment %s, route %s, ingress %s: %s", + ServerSnippet, eName, rName, iName, annotation) } } } diff --git a/internal/lagoonyml/routeannotation_test.go b/internal/lagoonyml/routeannotation_test.go index 170a0d7..f4102d7 100644 --- a/internal/lagoonyml/routeannotation_test.go +++ b/internal/lagoonyml/routeannotation_test.go @@ -39,8 +39,12 @@ func TestRouteAnnotation(t *testing.T) { input: "add_header X-branch \"#main\";\n", valid: true, }, - "invalid double add_header": { + "valid double add_header": { input: "add_header X-Robots-Tag \"noindex, nofollow\"; add_header X-Robots-Tag \"noindex, nofollow\";", + valid: true, + }, + "invalid more_set_header": { + input: "more_set_headers \"Strict-Transport-Security: max-age=31536000\";\n", valid: false, }, "valid set_real_ip_from": { From 7f1652325ef7fef204f6f34bb396a58081d9f0fb Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Tue, 26 Oct 2021 17:15:57 +0800 Subject: [PATCH 4/5] feat: update route annotation linter * disallow these annotations: * nginx.ingress.kubernetes.io/auth-snippet * nginx.ingress.kubernetes.io/configuration-snippet * nginx.ingress.kubernetes.io/modsecurity-snippet * allow server-snippet to contain multiple allowed directives in a single annotation --- internal/lagoonyml/routeannotation.go | 38 ++++++++-- internal/lagoonyml/routeannotation_test.go | 77 +++++++++++++++++--- internal/lagoonyml/testdata/valid.lagoon.yml | 5 ++ 3 files changed, 105 insertions(+), 15 deletions(-) diff --git a/internal/lagoonyml/routeannotation.go b/internal/lagoonyml/routeannotation.go index e5aa922..dafb464 100644 --- a/internal/lagoonyml/routeannotation.go +++ b/internal/lagoonyml/routeannotation.go @@ -6,8 +6,12 @@ import ( "strings" ) -// ServerSnippet is the annotation for server snippets with ingress-nginx. -const ServerSnippet = "nginx.ingress.kubernetes.io/server-snippet" +const ( + authSnippet = "nginx.ingress.kubernetes.io/auth-snippet" + configurationSnippet = "nginx.ingress.kubernetes.io/configuration-snippet" + modsecuritySnippet = "nginx.ingress.kubernetes.io/modsecurity-snippet" + serverSnippet = "nginx.ingress.kubernetes.io/server-snippet" +) // validSnippets is the allow-list of snippets that Lagoon will accept. var validServerSnippets = regexp.MustCompile( @@ -17,7 +21,8 @@ var validServerSnippets = regexp.MustCompile( ` )+$`) // validate returns true if the annotations are valid, and false otherwise. -func validate(annotations map[string]string, r *regexp.Regexp, annotation string) (string, bool) { +func validate(annotations map[string]string, r *regexp.Regexp, + annotation string) (string, bool) { if ss, ok := annotations[annotation]; ok { for _, line := range strings.Split(ss, "\n") { line = strings.TrimSpace(line) @@ -37,10 +42,33 @@ func RouteAnnotation() Linter { for rName, lagoonRoutes := range routeMap { for _, lagoonRoute := range lagoonRoutes { for iName, ingress := range lagoonRoute.Ingresses { - if annotation, ok := validate(ingress.Annotations, validServerSnippets, ServerSnippet); !ok { + // auth-snippet + if _, ok := ingress.Annotations[authSnippet]; ok { + return fmt.Errorf( + "invalid %s annotation on environment %s, route %s, ingress %s: %s", + authSnippet, eName, rName, iName, + "this annotation is restricted") + } + // configuration-snippet + if _, ok := ingress.Annotations[configurationSnippet]; ok { + return fmt.Errorf( + "invalid %s annotation on environment %s, route %s, ingress %s: %s", + configurationSnippet, eName, rName, iName, + "this annotation is restricted") + } + // modsecurity-snippet + if _, ok := ingress.Annotations[modsecuritySnippet]; ok { + return fmt.Errorf( + "invalid %s annotation on environment %s, route %s, ingress %s: %s", + modsecuritySnippet, eName, rName, iName, + "this annotation is restricted") + } + // server-snippet + if annotation, ok := validate(ingress.Annotations, + validServerSnippets, serverSnippet); !ok { return fmt.Errorf( "invalid %s annotation on environment %s, route %s, ingress %s: %s", - ServerSnippet, eName, rName, iName, annotation) + serverSnippet, eName, rName, iName, annotation) } } } diff --git a/internal/lagoonyml/routeannotation_test.go b/internal/lagoonyml/routeannotation_test.go index f4102d7..0297363 100644 --- a/internal/lagoonyml/routeannotation_test.go +++ b/internal/lagoonyml/routeannotation_test.go @@ -1,12 +1,10 @@ -package lagoonyml_test +package lagoonyml import ( "testing" - - "github.com/uselagoon/lagoon-linter/internal/lagoonyml" ) -func TestRouteAnnotation(t *testing.T) { +func TestServerSnippet(t *testing.T) { var testCases = map[string]struct { input string valid bool @@ -58,17 +56,76 @@ func TestRouteAnnotation(t *testing.T) { } for name, tc := range testCases { t.Run(name, func(tt *testing.T) { - l := lagoonyml.Lagoon{ - Environments: map[string]lagoonyml.Environment{ + l := Lagoon{ + Environments: map[string]Environment{ + "testenv": { + Routes: []map[string][]LagoonRoute{ + { + "nginx": { + { + Ingresses: map[string]Ingress{ + "www.example.com": { + Annotations: map[string]string{ + serverSnippet: tc.input, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + err := RouteAnnotation()(&l) + if tc.valid { + if err != nil { + tt.Fatalf("unexpected error %v", err) + } + } else { + if err == nil { + tt.Fatalf("expected error, but got nil") + } + } + }) + } +} + +func TestRestrictedSnippets(t *testing.T) { + var testCases = map[string]struct { + input string + valid bool + }{ + "restrict configuration-snippet": { + input: "nginx.ingress.kubernetes.io/configuration-snippet", + valid: false, + }, + "restrict modsecurity-snippet": { + input: "nginx.ingress.kubernetes.io/modsecurity-snippet", + valid: false, + }, + "restrict auth-snippet": { + input: "nginx.ingress.kubernetes.io/auth-snippet", + valid: false, + }, + "allow whitelist-source-range": { + input: "nginx.ingress.kubernetes.io/whitelist-source-range", + valid: true, + }, + } + for name, tc := range testCases { + t.Run(name, func(tt *testing.T) { + l := Lagoon{ + Environments: map[string]Environment{ "testenv": { - Routes: []map[string][]lagoonyml.LagoonRoute{ + Routes: []map[string][]LagoonRoute{ { "nginx": { { - Ingresses: map[string]lagoonyml.Ingress{ + Ingresses: map[string]Ingress{ "www.example.com": { Annotations: map[string]string{ - lagoonyml.ServerSnippet: tc.input, + tc.input: "any value", }, }, }, @@ -79,7 +136,7 @@ func TestRouteAnnotation(t *testing.T) { }, }, } - err := lagoonyml.RouteAnnotation()(&l) + err := RouteAnnotation()(&l) if tc.valid { if err != nil { tt.Fatalf("unexpected error %v", err) diff --git a/internal/lagoonyml/testdata/valid.lagoon.yml b/internal/lagoonyml/testdata/valid.lagoon.yml index a731274..a375078 100644 --- a/internal/lagoonyml/testdata/valid.lagoon.yml +++ b/internal/lagoonyml/testdata/valid.lagoon.yml @@ -14,3 +14,8 @@ environments: annotations: nginx.ingress.kubernetes.io/server-snippet: | set_real_ip_from 1.2.3.4/32; + - "dev.example.com": + annotations: + nginx.ingress.kubernetes.io/server-snippet: | + set_real_ip_from 1.2.3.4/32; + add_header Content-type text/plain; From 09c1417260cc000e07ead184d1c0dd3a9ca6146d Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Tue, 26 Oct 2021 17:22:04 +0800 Subject: [PATCH 5/5] chore: add another invalid .lagoon.yml to test suite --- internal/lagoonyml/lint_test.go | 8 ++++++-- ...invalid.lagoon.yml => invalid.0.lagoon.yml} | 0 .../lagoonyml/testdata/invalid.1.lagoon.yml | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) rename internal/lagoonyml/testdata/{invalid.lagoon.yml => invalid.0.lagoon.yml} (100%) create mode 100644 internal/lagoonyml/testdata/invalid.1.lagoon.yml diff --git a/internal/lagoonyml/lint_test.go b/internal/lagoonyml/lint_test.go index d4a1af6..d8b1401 100644 --- a/internal/lagoonyml/lint_test.go +++ b/internal/lagoonyml/lint_test.go @@ -15,8 +15,12 @@ func TestLint(t *testing.T) { input: "testdata/valid.lagoon.yml", valid: true, }, - "invalid .lagoon.yml": { - input: "testdata/invalid.lagoon.yml", + "invalid.0.lagoon.yml": { + input: "testdata/invalid.0.lagoon.yml", + valid: false, + }, + "invalid.1.lagoon.yml": { + input: "testdata/invalid.1.lagoon.yml", valid: false, }, } diff --git a/internal/lagoonyml/testdata/invalid.lagoon.yml b/internal/lagoonyml/testdata/invalid.0.lagoon.yml similarity index 100% rename from internal/lagoonyml/testdata/invalid.lagoon.yml rename to internal/lagoonyml/testdata/invalid.0.lagoon.yml diff --git a/internal/lagoonyml/testdata/invalid.1.lagoon.yml b/internal/lagoonyml/testdata/invalid.1.lagoon.yml new file mode 100644 index 0000000..d2c8c22 --- /dev/null +++ b/internal/lagoonyml/testdata/invalid.1.lagoon.yml @@ -0,0 +1,18 @@ +environments: + main: + monitoring_urls: + - "https://www.example.com" + - "https://www.example.com/special_page" + routes: + - nginx: + - example.com + - "www.example.com": + tls-acme: 'true' + insecure: Redirect + hsts: max-age=31536000 + - "example.com": + annotations: + nginx.ingress.kubernetes.io/server-snippet: | + rewrite ^/redirect-test(.*) https://www.example.com/redirect-test$1 permanent; + nginx.ingress.kubernetes.io/configuration-snippet: | + more_set_headers "Strict-Transport-Security: max-age=31536000; includeSubDomains";