From 7811524c229ff464ddce91f85496a3da436b3377 Mon Sep 17 00:00:00 2001 From: Chad Kunde Date: Tue, 5 Feb 2019 00:04:09 -0800 Subject: [PATCH] Version filtering (#1050) * extend filtering logic and configuration to include version lists Module filtering is very useful, but many deployments will need to satisfy even more granular constrainsts. Enterprises may need approved lists specific down to the minor (or patch) version element. Version filtering logic is similar to the module filtering, in that it's a prefix match of the version requested against each entry in the version filter list. Closes #1045 * include version filtering in documentation with example * allow filtering when version is missing Endpoints that do not specify a version, such as "@v/list", only need to be filtered by module rules. --- docs/content/configuration/filter.md | 19 ++++++- pkg/middleware/filter.go | 6 ++- pkg/middleware/middleware_test.go | 6 +-- pkg/module/filter.go | 37 ++++++++++--- pkg/module/filter_rule.go | 1 + pkg/module/filter_test.go | 77 ++++++++++++++++++---------- 6 files changed, 106 insertions(+), 40 deletions(-) diff --git a/docs/content/configuration/filter.md b/docs/content/configuration/filter.md index 9d37ca3a..9cdb20b7 100644 --- a/docs/content/configuration/filter.md +++ b/docs/content/configuration/filter.md @@ -36,7 +36,7 @@ It allows for `#` to add comments and new lines are skipped. Anything else would D golang.org/x/tools -In the above example, `golang.org/x/tools` is fetched directly from the upstream proxy. All the modules from from `github.com/azure` are excluded except `github.com/azure/azure-sdk-for-go` +In the above example, `golang.org/x/tools` is fetched directly from the upstream proxy. All the modules from `github.com/azure` are excluded except `github.com/azure/azure-sdk-for-go` ### Adding a default mode @@ -51,3 +51,20 @@ D In the above example, all the modules are fetched directly from the source. `github.com/manugupt1/athens` is excluded and `github.com/gomods/athens` is stored in the proxy storage. + +### Adding versions to the filter + +Using an "approved list" is a common practice that requires each minor or patch version to be approved before it is allowed in the codebase. This is accomplished by adding a list of version patterns to the rule. These version patterns are comma-separated and prefix-matching, so `v2` and `v2.3.*` both match the requested version `2.3.5`. + +An example version filter is + +
+-
+# use internal github enterprise server directly
+D enterprise.github.com/company
+
+# external dependency approved list
++ github.com/gomods/athens v0.1,v0.2,v0.4.1
+
+ +In the above example, any module not in the rules will be excluded. All modules from `enterprise.github.com/company` are fetched directly from the source. The `github.com/gomods/athens` module will be stored in the proxy storage, but only for version `v0.4.1` and any patch versions under `v0.1` and `v0.2` minor versions. diff --git a/pkg/middleware/filter.go b/pkg/middleware/filter.go index d67fb426..ea88b7c0 100644 --- a/pkg/middleware/filter.go +++ b/pkg/middleware/filter.go @@ -23,7 +23,11 @@ func NewFilterMiddleware(mf *module.Filter, upstreamEndpoint string) mux.Middlew h.ServeHTTP(w, r) return } - rule := mf.Rule(mod) + ver, err := paths.GetVersion(r) + if err != nil { + ver = "" + } + rule := mf.Rule(mod, ver) switch rule { case module.Exclude: // Exclude: ignore request for this module diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 1c15ff94..7d631d3f 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -49,9 +49,9 @@ func newTestFilter(filterFile string) (*module.Filter, error) { if err != nil { return nil, err } - f.AddRule("github.com/gomods/athens/", module.Direct) - f.AddRule("github.com/athens-artifacts/no-tags", module.Exclude) - f.AddRule("github.com/athens-artifacts", module.Include) + f.AddRule("github.com/gomods/athens/", nil, module.Direct) + f.AddRule("github.com/athens-artifacts/no-tags", nil, module.Exclude) + f.AddRule("github.com/athens-artifacts", nil, module.Include) return f, nil } diff --git a/pkg/module/filter.go b/pkg/module/filter.go index cee8c81f..63005c09 100644 --- a/pkg/module/filter.go +++ b/pkg/module/filter.go @@ -42,7 +42,7 @@ func NewFilter(filterFilePath string) (*Filter, error) { } // AddRule adds rule for specified path -func (f *Filter) AddRule(path string, rule FilterRule) { +func (f *Filter) AddRule(path string, versions []string, rule FilterRule) { f.ensurePath(path) segments := getPathSegments(path) @@ -62,13 +62,14 @@ func (f *Filter) AddRule(path string, rule FilterRule) { last := segments[len(segments)-1] rn := latest.next[last] rn.rule = rule + rn.vers = versions latest.next[last] = rn } // Rule returns the filter rule to be applied to the given path -func (f *Filter) Rule(path string) FilterRule { +func (f *Filter) Rule(path, version string) FilterRule { segs := getPathSegments(path) - rule := f.getAssociatedRule(segs...) + rule := f.getAssociatedRule(version, segs...) if rule == Default { rule = Include } @@ -88,7 +89,7 @@ func (f *Filter) ensurePath(path string) { } } -func (f *Filter) getAssociatedRule(path ...string) FilterRule { +func (f *Filter) getAssociatedRule(version string, path ...string) FilterRule { if len(path) == 0 { return f.root.rule } @@ -100,7 +101,17 @@ func (f *Filter) getAssociatedRule(path ...string) FilterRule { break } rn = rn.next[p] - rules = append(rules, rn.rule) + // default to true if no version filter, false otherwise + match := len(rn.vers) == 0 + for _, ver := range rn.vers { + if strings.HasPrefix(version, ver) { + match = true + break + } + } + if match || version == "" { + rules = append(rules, rn.rule) + } } if len(rules) == 0 { @@ -140,7 +151,7 @@ func initFromConfig(filePath string) (*Filter, error) { } split := strings.Split(line, " ") - if len(split) > 2 { + if len(split) > 3 { return nil, errors.E(op, "Invalid configuration found in filter file at the line "+strconv.Itoa(idx+1)) } @@ -158,12 +169,22 @@ func initFromConfig(filePath string) (*Filter, error) { } // is root config if len(split) == 1 { - f.AddRule("", rule) + f.AddRule("", nil, rule) continue } + var vers []string + if len(split) == 3 { + vers = strings.Split(split[2], ",") + for i := range vers { + vers[i] = strings.TrimRight(vers[i], "*") + if vers[i][len(vers[i])-1] != '.' && strings.Count(vers[i], ".") < 2 { + vers[i] += "." + } + } + } path := strings.TrimSpace(split[1]) - f.AddRule(path, rule) + f.AddRule(path, vers, rule) } return f, nil } diff --git a/pkg/module/filter_rule.go b/pkg/module/filter_rule.go index 952086eb..b2210f1c 100644 --- a/pkg/module/filter_rule.go +++ b/pkg/module/filter_rule.go @@ -3,4 +3,5 @@ package module type ruleNode struct { next map[string]ruleNode rule FilterRule + vers []string } diff --git a/pkg/module/filter_test.go b/pkg/module/filter_test.go index 1deadcb9..26d363aa 100644 --- a/pkg/module/filter_test.go +++ b/pkg/module/filter_test.go @@ -52,13 +52,13 @@ func (t *FilterTests) Test_IgnoreSimple() { f, err := NewFilter(filter) r.NoError(err) - f.AddRule("github.com/a/b", Exclude) + f.AddRule("github.com/a/b", nil, Exclude) - r.Equal(Include, f.Rule("github.com/a")) - r.Equal(Exclude, f.Rule("github.com/a/b")) - r.Equal(Exclude, f.Rule("github.com/a/b/c")) - r.Equal(Include, f.Rule("github.com/d")) - r.Equal(Include, f.Rule("bitbucket.com/a/b")) + r.Equal(Include, f.Rule("github.com/a", "")) + r.Equal(Exclude, f.Rule("github.com/a/b", "")) + r.Equal(Exclude, f.Rule("github.com/a/b/c", "")) + r.Equal(Include, f.Rule("github.com/d", "")) + r.Equal(Include, f.Rule("bitbucket.com/a/b", "")) } func (t *FilterTests) Test_IgnoreParentAllowChildren() { @@ -69,14 +69,14 @@ func (t *FilterTests) Test_IgnoreParentAllowChildren() { f, err := NewFilter(filter) r.NoError(err) - f.AddRule("github.com/a/b", Exclude) - f.AddRule("github.com/a/b/c", Include) + f.AddRule("github.com/a/b", nil, Exclude) + f.AddRule("github.com/a/b/c", nil, Include) - r.Equal(Include, f.Rule("github.com/a")) - r.Equal(Exclude, f.Rule("github.com/a/b")) - r.Equal(Include, f.Rule("github.com/a/b/c")) - r.Equal(Include, f.Rule("github.com/d")) - r.Equal(Include, f.Rule("bitbucket.com/a/b")) + r.Equal(Include, f.Rule("github.com/a", "")) + r.Equal(Exclude, f.Rule("github.com/a/b", "")) + r.Equal(Include, f.Rule("github.com/a/b/c", "")) + r.Equal(Include, f.Rule("github.com/d", "")) + r.Equal(Include, f.Rule("bitbucket.com/a/b", "")) } func (t *FilterTests) Test_OnlyAllowed() { @@ -87,14 +87,14 @@ func (t *FilterTests) Test_OnlyAllowed() { f, err := NewFilter(filter) r.NoError(err) - f.AddRule("github.com/a/b", Include) - f.AddRule("", Exclude) + f.AddRule("github.com/a/b", nil, Include) + f.AddRule("", nil, Exclude) - r.Equal(Exclude, f.Rule("github.com/a")) - r.Equal(Include, f.Rule("github.com/a/b")) - r.Equal(Include, f.Rule("github.com/a/b/c")) - r.Equal(Exclude, f.Rule("github.com/d")) - r.Equal(Exclude, f.Rule("bitbucket.com/a/b")) + r.Equal(Exclude, f.Rule("github.com/a", "")) + r.Equal(Include, f.Rule("github.com/a/b", "")) + r.Equal(Include, f.Rule("github.com/a/b/c", "")) + r.Equal(Exclude, f.Rule("github.com/d", "")) + r.Equal(Exclude, f.Rule("bitbucket.com/a/b", "")) } func (t *FilterTests) Test_Direct() { @@ -105,14 +105,32 @@ func (t *FilterTests) Test_Direct() { f, err := NewFilter(filter) r.NoError(err) - f.AddRule("github.com/a/b/c", Exclude) - f.AddRule("github.com/a/b", Direct) - f.AddRule("github.com/a", Include) - f.AddRule("", Exclude) + f.AddRule("github.com/a/b/c", nil, Exclude) + f.AddRule("github.com/a/b", nil, Direct) + f.AddRule("github.com/a", nil, Include) + f.AddRule("", nil, Exclude) - r.Equal(Include, f.Rule("github.com/a")) - r.Equal(Direct, f.Rule("github.com/a/b")) - r.Equal(Exclude, f.Rule("github.com/a/b/c/d")) + r.Equal(Include, f.Rule("github.com/a", "")) + r.Equal(Direct, f.Rule("github.com/a/b", "")) + r.Equal(Exclude, f.Rule("github.com/a/b/c/d", "")) +} + +func (t *FilterTests) Test_versionFilter() { + r := t.Require() + filter := tempFilterFile(t.T()) + defer os.Remove(filter) + + f, err := NewFilter(filter) + r.NoError(err) + f.AddRule("", nil, Exclude) + f.AddRule("github.com/a/b", []string{"v1."}, Include) + f.AddRule("github.com/a/b/c", []string{"v1.2.", "v0.8."}, Direct) + + r.Equal(Exclude, f.Rule("github.com/d/e", "v1.2.0")) + r.Equal(Exclude, f.Rule("github.com/a/b", "v10.0.0")) + r.Equal(Include, f.Rule("github.com/a/b", "v1.5.0")) + r.Equal(Direct, f.Rule("github.com/a/b/c/d", "v1.2.3")) + r.Equal(Include, f.Rule("github.com/a/b/c/d", "v1.3.4")) } func (t *FilterTests) Test_initFromConfig() { @@ -133,6 +151,11 @@ func (t *FilterTests) Test_initFromConfig() { r.Nil(f) r.Error(err) + versionInput := []byte("+ github.com/a/b\n\n# some comment\n\n- github.com/c/d v1,v2.3.4,v3.2.*\n\nD github.com/x\n") + ioutil.WriteFile(filterFile, versionInput, 0644) + f, err = initFromConfig(filterFile) + r.NotNil(f) + r.NoError(err) } func tempFilterFile(t *testing.T) (path string) {