From 31b7783c26879de6f170e40777c799dfd797213b Mon Sep 17 00:00:00 2001 From: Nejc Habjan Date: Tue, 7 Jun 2022 13:43:12 +0200 Subject: [PATCH 1/8] Make _redirects limits configurable Changelog: added --- internal/config/config.go | 16 ++++++++++++++++ internal/config/flags.go | 5 +++++ internal/redirects/matching.go | 2 +- internal/redirects/redirects.go | 22 ++++++++-------------- internal/redirects/redirects_test.go | 4 ++-- internal/redirects/validations.go | 2 +- 6 files changed, 33 insertions(+), 18 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 18fe33ebf..fcaa1d5e7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -21,6 +21,7 @@ type Config struct { Authentication Auth GitLab GitLab Log Log + Redirects Redirects Sentry Sentry Server Server TLS TLS @@ -119,6 +120,13 @@ type Log struct { Verbose bool } +// Redirects groups settings related to configuring _redirects limits +type Redirects struct { + MaxConfigSize int + MaxPathSegments int + MaxRuleCount int +} + // Sentry groups settings related to configuring Sentry type Sentry struct { DSN string @@ -287,6 +295,11 @@ func loadConfig() (*Config, error) { Format: *logFormat, Verbose: *logVerbose, }, + Redirects: Redirects{ + MaxConfigSize: *redirectsMaxConfigSize, + MaxPathSegments: *redirectsMaxPathSegments, + MaxRuleCount: *redirectsMaxRuleCount, + }, Sentry: Sentry{ DSN: *sentryDSN, Environment: *sentryEnvironment, @@ -401,6 +414,9 @@ func LogConfig(config *Config) { "rate-limit-tls-source-ip-burst": config.RateLimit.TLSSourceIPBurst, "rate-limit-tls-domain": config.RateLimit.TLSDomainLimitPerSecond, "rate-limit-tls-domain-burst": config.RateLimit.TLSDomainBurst, + "redirects-max-config-size": config.Redirects.MaxConfigSize, + "redirects-max-path-segments": config.Redirects.MaxPathSegments, + "redirects-max-rule-count": config.Redirects.MaxRuleCount, "server-read-timeout": config.Server.ReadTimeout, "server-read-header-timeout": config.Server.ReadHeaderTimeout, "server-write-timeout": config.Server.WriteTimeout, diff --git a/internal/config/flags.go b/internal/config/flags.go index 88ebbb155..9d79f0bcd 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -61,6 +61,11 @@ var ( gitlabRetrievalInterval = flag.Duration("gitlab-retrieval-interval", time.Second, "The interval to wait before retrying to resolve a domain's configuration via the GitLab API") gitlabRetrievalRetries = flag.Int("gitlab-retrieval-retries", 3, "The maximum number of times to retry to resolve a domain's configuration via the API") + // Check https://gitlab.com/gitlab-org/gitlab-pages/-/issues/472 before increasing default redirectsMaxConfigSize value + redirectsMaxConfigSize = flag.Int("redirects-max-config-size", 64*1024, "The maximum size of the _redirects file, in bytes") + redirectsMaxPathSegments = flag.Int("redirects-max-path-segments", 25, "The maximum number of path segments allowed in _redirects rules URLs") + redirectsMaxRuleCount = flag.Int("redirects-max-rule-count", 1000, "The maximum number of rules allowed in _redirects") + enableDisk = flag.Bool("enable-disk", true, "Enable disk access, shall be disabled in environments where shared disk storage isn't available") clientID = flag.String("auth-client-id", "", "GitLab application Client ID") diff --git a/internal/redirects/matching.go b/internal/redirects/matching.go index a75255bbe..0b4d26e06 100644 --- a/internal/redirects/matching.go +++ b/internal/redirects/matching.go @@ -103,7 +103,7 @@ func matchesRule(rule *netlifyRedirects.Rule, path string) (bool, string) { // If no rule matches, this function returns `nil` and an empty string func (r *Redirects) match(path string) (*netlifyRedirects.Rule, string) { for i := range r.rules { - if i >= maxRuleCount { + if i >= cfg.Redirects.MaxRuleCount { // do not process any more rules return nil, "" } diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go index f3c4e491a..3c2140731 100644 --- a/internal/redirects/redirects.go +++ b/internal/redirects/redirects.go @@ -13,6 +13,7 @@ import ( netlifyRedirects "github.com/tj/go-redirects" "gitlab.com/gitlab-org/labkit/log" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" ) @@ -22,18 +23,11 @@ const ( // - https://docs.netlify.com/routing/redirects/ // - https://docs.netlify.com/routing/redirects/redirect-options/ ConfigFile = "_redirects" - - // Check https://gitlab.com/gitlab-org/gitlab-pages/-/issues/472 before increasing this value - maxConfigSize = 64 * 1024 - - // maxPathSegments is used to limit the number of path segments allowed in rules URLs - maxPathSegments = 25 - - // maxRuleCount is used to limit the total number of rules allowed in _redirects - maxRuleCount = 1000 ) var ( + // hack, todo: properly pass config context to internals + cfg, _ = config.LoadConfig() // ErrNoRedirect is the error thrown when a no redirect rule matches while trying to Rewrite URL. // This means that no redirect applies to the URL and you can fallback to serving actual content instead. ErrNoRedirect = errors.New("no redirect found") @@ -50,7 +44,7 @@ var ( errNoParams = errors.New("params not supported") errUnsupportedStatus = errors.New("status not supported") errNoForce = errors.New("force! not supported") - errTooManyPathSegments = fmt.Errorf("url path cannot contain more than %d forward slashes", maxPathSegments) + errTooManyPathSegments = fmt.Errorf("url path cannot contain more than %d forward slashes", cfg.Redirects.MaxConfigSize) regexpPlaceholder = regexp.MustCompile(`(?i)/:[a-z]+`) ) @@ -69,13 +63,13 @@ func (r *Redirects) Status() string { messages = append(messages, fmt.Sprintf("%d rules", len(r.rules))) for i, rule := range r.rules { - if i >= maxRuleCount { + if i >= cfg.Redirects.MaxConfigSize { messages = append([]string{ fmt.Sprintf( "The _redirects file contains (%d) rules, more than the maximum of %d rules. Only the first %d rules will be processed.", len(r.rules), - maxRuleCount, - maxRuleCount, + cfg.Redirects.MaxRuleCount, + cfg.Redirects.MaxRuleCount, )}, messages..., ) @@ -126,7 +120,7 @@ func ParseRedirects(ctx context.Context, root vfs.Root) *Redirects { return &Redirects{error: errNeedRegularFile} } - if fi.Size() > maxConfigSize { + if int(fi.Size()) > cfg.Redirects.MaxConfigSize { return &Redirects{error: errFileTooLarge} } diff --git a/internal/redirects/redirects_test.go b/internal/redirects/redirects_test.go index 66c5d7339..c0df43c02 100644 --- a/internal/redirects/redirects_test.go +++ b/internal/redirects/redirects_test.go @@ -165,7 +165,7 @@ func TestRedirectsParseRedirects(t *testing.T) { }, { name: "Config file too big", - redirectsFile: strings.Repeat("a", 2*maxConfigSize), + redirectsFile: strings.Repeat("a", 2*cfg.Redirects.MaxConfigSize), expectedRules: 0, expectedErr: errFileTooLarge, }, @@ -197,7 +197,7 @@ func TestRedirectsParseRedirects(t *testing.T) { func TestMaxRuleCount(t *testing.T) { root, tmpDir := testhelpers.TmpDir(t) - err := os.WriteFile(path.Join(tmpDir, ConfigFile), []byte(strings.Repeat("/goto.html /target.html 301\n", maxRuleCount-1)+ + err := os.WriteFile(path.Join(tmpDir, ConfigFile), []byte(strings.Repeat("/goto.html /target.html 301\n", cfg.Redirects.MaxRuleCount-1)+ "/1000.html /target1000 301\n"+ "/1001.html /target1001 301\n", ), 0600) diff --git a/internal/redirects/validations.go b/internal/redirects/validations.go index ed022f527..4e5eb86ae 100644 --- a/internal/redirects/validations.go +++ b/internal/redirects/validations.go @@ -43,7 +43,7 @@ func validateURL(urlText string) error { // Limit the number of path segments a rule can contain. // This prevents the matching logic from generating regular // expressions that are too large/complex. - if strings.Count(url.Path, "/") > maxPathSegments { + if strings.Count(url.Path, "/") > cfg.Redirects.MaxPathSegments { return errTooManyPathSegments } } else { -- GitLab From 39b7e2c35b2f609623ae483ee3f607b3da42808e Mon Sep 17 00:00:00 2001 From: Nejc Habjan Date: Thu, 9 Jun 2022 15:38:28 +0200 Subject: [PATCH 2/8] Apply review suggestions --- app.go | 3 +++ internal/redirects/matching.go | 2 +- internal/redirects/redirects.go | 17 ++++++++++------- internal/redirects/validations.go | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app.go b/app.go index c08dfb297..7a22a3fd5 100644 --- a/app.go +++ b/app.go @@ -32,6 +32,7 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" "gitlab.com/gitlab-org/gitlab-pages/internal/netutil" + "gitlab.com/gitlab-org/gitlab-pages/internal/redirects" "gitlab.com/gitlab-org/gitlab-pages/internal/rejectmethods" "gitlab.com/gitlab-org/gitlab-pages/internal/request" "gitlab.com/gitlab-org/gitlab-pages/internal/routing" @@ -349,6 +350,8 @@ func (a *theApp) listenMetrics(eg *errgroup.Group, config cfg.Metrics) *http.Ser } func runApp(config *cfg.Config) error { + redirects.SetConfig(config.Redirects) + source, err := gitlab.New(&config.GitLab) if err != nil { return fmt.Errorf("could not create domains config source: %w", err) diff --git a/internal/redirects/matching.go b/internal/redirects/matching.go index 0b4d26e06..25e2ce2e3 100644 --- a/internal/redirects/matching.go +++ b/internal/redirects/matching.go @@ -103,7 +103,7 @@ func matchesRule(rule *netlifyRedirects.Rule, path string) (bool, string) { // If no rule matches, this function returns `nil` and an empty string func (r *Redirects) match(path string) (*netlifyRedirects.Rule, string) { for i := range r.rules { - if i >= cfg.Redirects.MaxRuleCount { + if i >= cfg.MaxRuleCount { // do not process any more rules return nil, "" } diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go index 3c2140731..04815404c 100644 --- a/internal/redirects/redirects.go +++ b/internal/redirects/redirects.go @@ -26,8 +26,7 @@ const ( ) var ( - // hack, todo: properly pass config context to internals - cfg, _ = config.LoadConfig() + cfg = config.Redirects{} // ErrNoRedirect is the error thrown when a no redirect rule matches while trying to Rewrite URL. // This means that no redirect applies to the URL and you can fallback to serving actual content instead. ErrNoRedirect = errors.New("no redirect found") @@ -44,10 +43,14 @@ var ( errNoParams = errors.New("params not supported") errUnsupportedStatus = errors.New("status not supported") errNoForce = errors.New("force! not supported") - errTooManyPathSegments = fmt.Errorf("url path cannot contain more than %d forward slashes", cfg.Redirects.MaxConfigSize) + errTooManyPathSegments = fmt.Errorf("url path cannot contain more than %d forward slashes", cfg.MaxConfigSize) regexpPlaceholder = regexp.MustCompile(`(?i)/:[a-z]+`) ) +func SetConfig(redirectsConfig config.Redirects) { + cfg = redirectsConfig +} + type Redirects struct { rules []netlifyRedirects.Rule error error @@ -63,13 +66,13 @@ func (r *Redirects) Status() string { messages = append(messages, fmt.Sprintf("%d rules", len(r.rules))) for i, rule := range r.rules { - if i >= cfg.Redirects.MaxConfigSize { + if i >= cfg.MaxConfigSize { messages = append([]string{ fmt.Sprintf( "The _redirects file contains (%d) rules, more than the maximum of %d rules. Only the first %d rules will be processed.", len(r.rules), - cfg.Redirects.MaxRuleCount, - cfg.Redirects.MaxRuleCount, + cfg.MaxRuleCount, + cfg.MaxRuleCount, )}, messages..., ) @@ -120,7 +123,7 @@ func ParseRedirects(ctx context.Context, root vfs.Root) *Redirects { return &Redirects{error: errNeedRegularFile} } - if int(fi.Size()) > cfg.Redirects.MaxConfigSize { + if int(fi.Size()) > cfg.MaxConfigSize { return &Redirects{error: errFileTooLarge} } diff --git a/internal/redirects/validations.go b/internal/redirects/validations.go index 4e5eb86ae..c37b79ec3 100644 --- a/internal/redirects/validations.go +++ b/internal/redirects/validations.go @@ -43,7 +43,7 @@ func validateURL(urlText string) error { // Limit the number of path segments a rule can contain. // This prevents the matching logic from generating regular // expressions that are too large/complex. - if strings.Count(url.Path, "/") > cfg.Redirects.MaxPathSegments { + if strings.Count(url.Path, "/") > cfg.MaxPathSegments { return errTooManyPathSegments } } else { -- GitLab From 12ca83b790fdddbbfcf7c6b08aa9da79a3ab8f6d Mon Sep 17 00:00:00 2001 From: Nejc Habjan Date: Thu, 9 Jun 2022 15:39:27 +0200 Subject: [PATCH 3/8] Set up redirects config for unit tests --- internal/redirects/redirects_test.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/internal/redirects/redirects_test.go b/internal/redirects/redirects_test.go index c0df43c02..210e8fe0f 100644 --- a/internal/redirects/redirects_test.go +++ b/internal/redirects/redirects_test.go @@ -12,12 +12,20 @@ import ( "github.com/stretchr/testify/require" netlifyRedirects "github.com/tj/go-redirects" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/feature" "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" ) +const ( + defaultMaxConfigSize = 64 * 1024 + defaultMaxPathSegments = 25 + defaultMaxRuleCount = 1000 +) + func TestRedirectsRewrite(t *testing.T) { t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") + setupRedirectsConfig() tests := []struct { name string @@ -138,6 +146,7 @@ func TestRedirectsRewrite(t *testing.T) { func TestRedirectsParseRedirects(t *testing.T) { ctx := context.Background() + setupRedirectsConfig() root, tmpDir := testhelpers.TmpDir(t) @@ -165,7 +174,7 @@ func TestRedirectsParseRedirects(t *testing.T) { }, { name: "Config file too big", - redirectsFile: strings.Repeat("a", 2*cfg.Redirects.MaxConfigSize), + redirectsFile: strings.Repeat("a", 2*cfg.MaxConfigSize), expectedRules: 0, expectedErr: errFileTooLarge, }, @@ -196,8 +205,9 @@ func TestRedirectsParseRedirects(t *testing.T) { func TestMaxRuleCount(t *testing.T) { root, tmpDir := testhelpers.TmpDir(t) + setupRedirectsConfig() - err := os.WriteFile(path.Join(tmpDir, ConfigFile), []byte(strings.Repeat("/goto.html /target.html 301\n", cfg.Redirects.MaxRuleCount-1)+ + err := os.WriteFile(path.Join(tmpDir, ConfigFile), []byte(strings.Repeat("/goto.html /target.html 301\n", cfg.MaxRuleCount-1)+ "/1000.html /target1000 301\n"+ "/1001.html /target1001 301\n", ), 0600) @@ -226,3 +236,11 @@ func TestMaxRuleCount(t *testing.T) { t.Run("maxRuleCount matches", testFn("/1000.html", "/target1000", http.StatusMovedPermanently, nil)) t.Run("maxRuleCount+1 does not match", testFn("/1001.html", "", 0, ErrNoRedirect)) } + +func setupRedirectsConfig() { + SetConfig(config.Redirects{ + MaxConfigSize: defaultMaxConfigSize, + MaxPathSegments: defaultMaxPathSegments, + MaxRuleCount: defaultMaxRuleCount, + }) +} -- GitLab From 9d39341fe0a5dad0080bd3d472c1b8fb757ceeb6 Mon Sep 17 00:00:00 2001 From: Nejc Habjan Date: Fri, 10 Jun 2022 00:11:19 +0200 Subject: [PATCH 4/8] Swap int casts for MaxConfigSize --- internal/redirects/redirects.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go index 04815404c..5d9015054 100644 --- a/internal/redirects/redirects.go +++ b/internal/redirects/redirects.go @@ -123,7 +123,7 @@ func ParseRedirects(ctx context.Context, root vfs.Root) *Redirects { return &Redirects{error: errNeedRegularFile} } - if int(fi.Size()) > cfg.MaxConfigSize { + if fi.Size() > int64(cfg.MaxConfigSize) { return &Redirects{error: errFileTooLarge} } -- GitLab From 770aba26fddb728107d278e906a5c1669671db0c Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Fri, 10 Jun 2022 05:25:04 +0000 Subject: [PATCH 5/8] Apply review suggestion --- internal/redirects/redirects.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go index 5d9015054..f92a58069 100644 --- a/internal/redirects/redirects.go +++ b/internal/redirects/redirects.go @@ -43,7 +43,7 @@ var ( errNoParams = errors.New("params not supported") errUnsupportedStatus = errors.New("status not supported") errNoForce = errors.New("force! not supported") - errTooManyPathSegments = fmt.Errorf("url path cannot contain more than %d forward slashes", cfg.MaxConfigSize) + errTooManyPathSegments = errors.New("url path contains more forward slashes than the configured maximum") regexpPlaceholder = regexp.MustCompile(`(?i)/:[a-z]+`) ) -- GitLab From 89dfffbca7d5a21f002354909edb394b5001604a Mon Sep 17 00:00:00 2001 From: Nejc Habjan Date: Wed, 22 Jun 2022 19:27:04 +0200 Subject: [PATCH 6/8] Apply maintainer review suggestions --- internal/redirects/redirects.go | 12 ++++++++++-- internal/redirects/redirects_test.go | 6 ------ internal/redirects/validations.go | 3 ++- internal/redirects/validations_test.go | 11 +++++++++-- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go index f92a58069..ed3bc4b10 100644 --- a/internal/redirects/redirects.go +++ b/internal/redirects/redirects.go @@ -23,6 +23,15 @@ const ( // - https://docs.netlify.com/routing/redirects/ // - https://docs.netlify.com/routing/redirects/redirect-options/ ConfigFile = "_redirects" + + // Check https://gitlab.com/gitlab-org/gitlab-pages/-/issues/472 before increasing this value + defaultMaxConfigSize = 64 * 1024 + + // maxPathSegments is used to limit the number of path segments allowed in rules URLs + defaultMaxPathSegments = 25 + + // maxRuleCount is used to limit the total number of rules allowed in _redirects + defaultMaxRuleCount = 1000 ) var ( @@ -43,7 +52,6 @@ var ( errNoParams = errors.New("params not supported") errUnsupportedStatus = errors.New("status not supported") errNoForce = errors.New("force! not supported") - errTooManyPathSegments = errors.New("url path contains more forward slashes than the configured maximum") regexpPlaceholder = regexp.MustCompile(`(?i)/:[a-z]+`) ) @@ -66,7 +74,7 @@ func (r *Redirects) Status() string { messages = append(messages, fmt.Sprintf("%d rules", len(r.rules))) for i, rule := range r.rules { - if i >= cfg.MaxConfigSize { + if i >= cfg.MaxRuleCount { messages = append([]string{ fmt.Sprintf( "The _redirects file contains (%d) rules, more than the maximum of %d rules. Only the first %d rules will be processed.", diff --git a/internal/redirects/redirects_test.go b/internal/redirects/redirects_test.go index 210e8fe0f..a9891b908 100644 --- a/internal/redirects/redirects_test.go +++ b/internal/redirects/redirects_test.go @@ -17,12 +17,6 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" ) -const ( - defaultMaxConfigSize = 64 * 1024 - defaultMaxPathSegments = 25 - defaultMaxRuleCount = 1000 -) - func TestRedirectsRewrite(t *testing.T) { t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") setupRedirectsConfig() diff --git a/internal/redirects/validations.go b/internal/redirects/validations.go index c37b79ec3..86fb02128 100644 --- a/internal/redirects/validations.go +++ b/internal/redirects/validations.go @@ -1,6 +1,7 @@ package redirects import ( + "fmt" "net/http" "net/url" "regexp" @@ -44,7 +45,7 @@ func validateURL(urlText string) error { // This prevents the matching logic from generating regular // expressions that are too large/complex. if strings.Count(url.Path, "/") > cfg.MaxPathSegments { - return errTooManyPathSegments + return fmt.Errorf("url path cannot contain more than %d forward slashes", cfg.MaxPathSegments) } } else { // No support for splats, https://docs.netlify.com/routing/redirects/redirect-options/#splats diff --git a/internal/redirects/validations_test.go b/internal/redirects/validations_test.go index 296be5118..906845a11 100644 --- a/internal/redirects/validations_test.go +++ b/internal/redirects/validations_test.go @@ -1,6 +1,7 @@ package redirects import ( + "fmt" "strings" "testing" @@ -42,7 +43,7 @@ func TestRedirectsValidateUrl(t *testing.T) { }, "too_many_slashes": { url: strings.Repeat("/a", 26), - expectedErr: errTooManyPathSegments, + expectedErr: fmt.Errorf("url path cannot contain more than %d forward slashes", defaultMaxPathSegments), }, "placeholders": { url: "/news/:year/:month/:date/:slug", @@ -58,7 +59,13 @@ func TestRedirectsValidateUrl(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { err := validateURL(tt.url) - require.ErrorIs(t, err, tt.expectedErr) + if tt.expectedErr != nil { + require.Error(t, err) + require.Equal(t, err, tt.expectedErr) + return + } + + require.NoError(t, err) }) } } -- GitLab From 5c43f84ea593cfdccfec4993d5baa300676fa946 Mon Sep 17 00:00:00 2001 From: Nejc Habjan Date: Mon, 27 Jun 2022 10:42:58 +0200 Subject: [PATCH 7/8] Initialize redirects config with default values --- internal/redirects/redirects.go | 7 ++++++- internal/redirects/redirects_test.go | 12 ------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go index ed3bc4b10..4add6537d 100644 --- a/internal/redirects/redirects.go +++ b/internal/redirects/redirects.go @@ -35,7 +35,12 @@ const ( ) var ( - cfg = config.Redirects{} + cfg = config.Redirects{ + MaxConfigSize: defaultMaxConfigSize, + MaxPathSegments: defaultMaxPathSegments, + MaxRuleCount: defaultMaxRuleCount, + } + // ErrNoRedirect is the error thrown when a no redirect rule matches while trying to Rewrite URL. // This means that no redirect applies to the URL and you can fallback to serving actual content instead. ErrNoRedirect = errors.New("no redirect found") diff --git a/internal/redirects/redirects_test.go b/internal/redirects/redirects_test.go index a9891b908..d40b065b8 100644 --- a/internal/redirects/redirects_test.go +++ b/internal/redirects/redirects_test.go @@ -12,14 +12,12 @@ import ( "github.com/stretchr/testify/require" netlifyRedirects "github.com/tj/go-redirects" - "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/feature" "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" ) func TestRedirectsRewrite(t *testing.T) { t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") - setupRedirectsConfig() tests := []struct { name string @@ -140,7 +138,6 @@ func TestRedirectsRewrite(t *testing.T) { func TestRedirectsParseRedirects(t *testing.T) { ctx := context.Background() - setupRedirectsConfig() root, tmpDir := testhelpers.TmpDir(t) @@ -199,7 +196,6 @@ func TestRedirectsParseRedirects(t *testing.T) { func TestMaxRuleCount(t *testing.T) { root, tmpDir := testhelpers.TmpDir(t) - setupRedirectsConfig() err := os.WriteFile(path.Join(tmpDir, ConfigFile), []byte(strings.Repeat("/goto.html /target.html 301\n", cfg.MaxRuleCount-1)+ "/1000.html /target1000 301\n"+ @@ -230,11 +226,3 @@ func TestMaxRuleCount(t *testing.T) { t.Run("maxRuleCount matches", testFn("/1000.html", "/target1000", http.StatusMovedPermanently, nil)) t.Run("maxRuleCount+1 does not match", testFn("/1001.html", "", 0, ErrNoRedirect)) } - -func setupRedirectsConfig() { - SetConfig(config.Redirects{ - MaxConfigSize: defaultMaxConfigSize, - MaxPathSegments: defaultMaxPathSegments, - MaxRuleCount: defaultMaxRuleCount, - }) -} -- GitLab From 520fa6df5119d8fb2ff7ec5ac7da521a3e102820 Mon Sep 17 00:00:00 2001 From: Nejc Habjan Date: Mon, 27 Jun 2022 10:43:27 +0200 Subject: [PATCH 8/8] Simplify validation test checks --- internal/redirects/validations_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/redirects/validations_test.go b/internal/redirects/validations_test.go index 906845a11..0d6df3ecf 100644 --- a/internal/redirects/validations_test.go +++ b/internal/redirects/validations_test.go @@ -60,8 +60,7 @@ func TestRedirectsValidateUrl(t *testing.T) { t.Run(name, func(t *testing.T) { err := validateURL(tt.url) if tt.expectedErr != nil { - require.Error(t, err) - require.Equal(t, err, tt.expectedErr) + require.EqualError(t, err, tt.expectedErr.Error()) return } -- GitLab