From ede169f351da5bda241d0fffa74dbea0cd99de81 Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Thu, 12 May 2022 18:01:49 +0100 Subject: [PATCH 1/5] feat: Resource list enhancements --- go.mod | 1 + go.sum | 1 + pkg/config/provider.go | 8 +++++++ pkg/core/fetch.go | 52 +++++++++++++++++++++++++++++++++--------- pkg/core/fetch_test.go | 41 +++++++++++++++++++++++++++++---- 5 files changed, 88 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index 848c7e1defeb6e..edb26c9596ca65 100644 --- a/go.mod +++ b/go.mod @@ -51,6 +51,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 github.com/olekukonko/tablewriter v0.0.5 github.com/rudderlabs/analytics-go v3.3.2+incompatible + github.com/ryanuber/go-glob v1.0.0 github.com/spf13/cast v1.4.1 github.com/spf13/pflag v1.0.5 github.com/tidwall/gjson v1.11.0 diff --git a/go.sum b/go.sum index b1ad15a6b166b0..23a67fd99619bc 100644 --- a/go.sum +++ b/go.sum @@ -1406,6 +1406,7 @@ github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= +github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc= github.com/ryszard/goskiplist v0.0.0-20150312221310-2dfbae5fcf46 h1:GHRpF1pTW19a8tTFrMLUcfWwyC0pnifVo2ClaLq+hP8= github.com/ryszard/goskiplist v0.0.0-20150312221310-2dfbae5fcf46/go.mod h1:uAQ5PCi+MFsC7HjREoAz1BU+Mq60+05gifQSsHSDG/8= diff --git a/pkg/config/provider.go b/pkg/config/provider.go index ad310b3aff4f8a..60f9297565898e 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -14,6 +14,7 @@ type Provider struct { Name string `hcl:"name,label"` Alias string `hcl:"alias,optional"` Resources []string `hcl:"resources,optional"` + SkipResources []string `hcl:"skip_resources,optional"` Env []string `hcl:"env,optional"` Configuration []byte MaxParallelResourceFetchLimit uint64 `hcl:"max_parallel_resource_fetch_limit"` @@ -71,6 +72,10 @@ func decodeProviderBlock(block *hcl.Block, ctx *hcl.EvalContext, existingProvide valDiags := gohcl.DecodeExpression(attr.Expr, ctx, &provider.Resources) diags = append(diags, valDiags...) } + if attr, exists := content.Attributes["skip_resources"]; exists { + valDiags := gohcl.DecodeExpression(attr.Expr, ctx, &provider.SkipResources) + diags = append(diags, valDiags...) + } if attr, exists := content.Attributes["env"]; exists { valDiags := gohcl.DecodeExpression(attr.Expr, ctx, &provider.Env) diags = append(diags, valDiags...) @@ -118,6 +123,9 @@ var providerBlockSchema = &hcl.BodySchema{ { Name: "resources", }, + { + Name: "skip_resources", + }, { Name: "alias", }, diff --git a/pkg/core/fetch.go b/pkg/core/fetch.go index 557c94171e0eb1..12d390709f5d1f 100644 --- a/pkg/core/fetch.go +++ b/pkg/core/fetch.go @@ -6,26 +6,28 @@ import ( "fmt" "io" "math" - "sort" "sync" "time" "github.com/cloudquery/cloudquery/internal/logging" + cqsort "github.com/cloudquery/cloudquery/internal/sort" "github.com/cloudquery/cloudquery/pkg/config" "github.com/cloudquery/cloudquery/pkg/core/database" "github.com/cloudquery/cloudquery/pkg/core/history" "github.com/cloudquery/cloudquery/pkg/core/state" "github.com/cloudquery/cloudquery/pkg/plugin" "github.com/cloudquery/cloudquery/pkg/plugin/registry" - "github.com/cloudquery/cq-provider-sdk/cqproto" sdkdb "github.com/cloudquery/cq-provider-sdk/database" "github.com/cloudquery/cq-provider-sdk/database/dsn" "github.com/cloudquery/cq-provider-sdk/provider/diag" "github.com/cloudquery/cq-provider-sdk/provider/schema" + "github.com/google/uuid" "github.com/rs/zerolog" "github.com/rs/zerolog/log" + "github.com/ryanuber/go-glob" + "github.com/thoas/go-funk" gcodes "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -337,7 +339,7 @@ func executeFetch(ctx context.Context, pLog zerolog.Logger, plugin plugin.Plugin }() var resources []string - resources, diags = normalizeResources(ctx, plugin, info.Config.Resources) + resources, diags = normalizeResources(ctx, plugin, info.Config.Resources, info.Config.SkipResources) if diags.HasErrors() { summary.Status = FetchFailed return summary, diags @@ -418,27 +420,42 @@ func executeFetch(ctx context.Context, pLog zerolog.Logger, plugin plugin.Plugin // * wildcard expansion // * verify no unknown resources // * verify no duplicate resources -func normalizeResources(ctx context.Context, provider plugin.Plugin, resources []string) ([]string, diag.Diagnostics) { +func normalizeResources(ctx context.Context, provider plugin.Plugin, resources, skip []string) ([]string, diag.Diagnostics) { s, err := provider.Provider().GetProviderSchema(ctx, &cqproto.GetProviderSchemaRequest{}) if err != nil { return nil, diag.FromError(err, diag.INTERNAL) } - return doNormalizeResources(resources, s.ResourceTables) + + return doNormalizeResources(resources, skip, s.ResourceTables) } -// doNormalizeResources returns a canonical list of resources given a list of requested and all known resources. -// It replaces wildcard resource with all resources. Error is returned if: +// doNormalizeResources matches the given two resource lists to all provider resources and returns the requested resources (excluding skip resources) as another list. +func doNormalizeResources(resources, skip []string, all map[string]*schema.Table) ([]string, diag.Diagnostics) { + useRes, diags := doGlobResources(resources, false, all) + skipRes, dd := doGlobResources(skip, true, all) + return funk.Subtract(useRes, skipRes).([]string), diags.Add(dd) +} + +// doGlobResources returns a canonical list of resources given a list of requested and all known resources. +// It replaces wildcard resource with all resources in non-wild mode. Error is returned if: // // * wildcard is present and other explicit resource is requested; // * one of explicitly requested resources is not present in all known; // * some resource is specified more than once (duplicate). -func doNormalizeResources(requested []string, all map[string]*schema.Table) ([]string, diag.Diagnostics) { - if len(requested) == 1 && requested[0] == "*" { +func doGlobResources(requested []string, allowWild bool, all map[string]*schema.Table) ([]string, diag.Diagnostics) { + if allowWild { + for _, s := range requested { + if s == "*" { + return nil, diag.FromError(fmt.Errorf("wildcard resource can only be in the requested resources list"), diag.USER, diag.WithDetails("you can only use * in the resources part of the configuration")) + } + } + } else if len(requested) == 1 && requested[0] == "*" { requested = make([]string, 0, len(all)) for k := range all { requested = append(requested, k) } } + result := make([]string, 0, len(requested)) seen := make(map[string]struct{}) for _, r := range requested { @@ -446,16 +463,29 @@ func doNormalizeResources(requested []string, all map[string]*schema.Table) ([]s return nil, diag.FromError(fmt.Errorf("resource %q is duplicate", r), diag.USER, diag.WithDetails("configuration has duplicate resources")) } seen[r] = struct{}{} + if _, ok := all[r]; !ok { if r == "*" { return nil, diag.FromError(fmt.Errorf("wildcard resource must be the only one in the list"), diag.USER, diag.WithDetails("you can only use * or a list of resources in configuration, but not both")) } + + // do glob match + found := false + for k := range all { + if glob.Glob(r, k) { + result = append(result, k) + found = true + } + } + if found { + continue + } return nil, diag.FromError(fmt.Errorf("resource %q does not exist", r), diag.USER, diag.WithDetails("configuration refers to a non-existing resource. Maybe you recently downgraded the provider but kept the config, or a typo perhaps?")) } result = append(result, r) } - sort.Strings(result) - return result, nil + + return cqsort.Unique(result), nil } func parseDSN(storage database.Storage, cfg *history.Config) (string, error) { diff --git a/pkg/core/fetch_test.go b/pkg/core/fetch_test.go index 47b2e03d758d0d..0d383a14f9df0e 100644 --- a/pkg/core/fetch_test.go +++ b/pkg/core/fetch_test.go @@ -2,7 +2,6 @@ package core import ( "context" - "reflect" "testing" "time" @@ -261,6 +260,7 @@ func Test_doNormalizeResources(t *testing.T) { tests := []struct { name string requested []string + skip []string all map[string]*schema.Table want []string wantErr bool @@ -268,6 +268,7 @@ func Test_doNormalizeResources(t *testing.T) { { "wilcard", []string{"*"}, + nil, map[string]*schema.Table{"3": nil, "2": nil, "1": nil}, []string{"1", "2", "3"}, false, @@ -275,6 +276,7 @@ func Test_doNormalizeResources(t *testing.T) { { "wilcard with explicit", []string{"*", "1"}, + nil, map[string]*schema.Table{"3": nil, "2": nil, "1": nil}, nil, true, @@ -282,6 +284,7 @@ func Test_doNormalizeResources(t *testing.T) { { "unknown resource", []string{"1", "2", "x"}, + nil, map[string]*schema.Table{"3": nil, "2": nil, "1": nil}, nil, true, @@ -289,6 +292,7 @@ func Test_doNormalizeResources(t *testing.T) { { "duplicate resource", []string{"1", "2", "1"}, + nil, map[string]*schema.Table{"3": nil, "2": nil, "1": nil}, nil, true, @@ -296,21 +300,50 @@ func Test_doNormalizeResources(t *testing.T) { { "ok, all explicit", []string{"2", "1"}, + nil, map[string]*schema.Table{"3": nil, "2": nil, "1": nil}, []string{"1", "2"}, false, }, + { + "ok, all explicit with ignores", + []string{"2", "1", "3"}, + []string{"1"}, + map[string]*schema.Table{"3": nil, "2": nil, "1": nil}, + []string{"2", "3"}, + false, + }, + { + "ok, some globs", + []string{"c1.*", "c2.res4"}, + nil, + map[string]*schema.Table{"c1.res1": nil, "c1.res2": nil, "c2.res3": nil, "c2.res4": nil}, + []string{"c1.res1", "c1.res2", "c2.res4"}, + false, + }, + { + "ok, some globs with skips", + []string{"c1.*", "c2.res4"}, + []string{"c1.res1"}, + map[string]*schema.Table{"c1.res1": nil, "c1.res2": nil, "c2.res3": nil, "c2.res4": nil}, + []string{"c1.res2", "c2.res4"}, + false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := doNormalizeResources(tt.requested, tt.all) + got, err := doNormalizeResources(tt.requested, tt.skip, tt.all) if (err != nil) != tt.wantErr { t.Errorf("doInterpolate() error = %v, wantErr %v", err, tt.wantErr) return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("doInterpolate() = %v, want %v", got, tt.want) + if tt.want == nil { + tt.want = []string{} + } + if got == nil { + got = []string{} } + assert.EqualValues(t, tt.want, got) }) } } From d7e16f5d7b3cf0541ef7ae9f9d57c183180127ea Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Thu, 12 May 2022 21:41:55 +0100 Subject: [PATCH 2/5] Match only .* globs --- go.mod | 1 - go.sum | 1 - pkg/core/fetch.go | 37 +++++++++++++++++++++++++------------ pkg/core/fetch_test.go | 20 ++++++++++++++++++-- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index edb26c9596ca65..848c7e1defeb6e 100644 --- a/go.mod +++ b/go.mod @@ -51,7 +51,6 @@ require ( github.com/modern-go/reflect2 v1.0.2 github.com/olekukonko/tablewriter v0.0.5 github.com/rudderlabs/analytics-go v3.3.2+incompatible - github.com/ryanuber/go-glob v1.0.0 github.com/spf13/cast v1.4.1 github.com/spf13/pflag v1.0.5 github.com/tidwall/gjson v1.11.0 diff --git a/go.sum b/go.sum index 23a67fd99619bc..b1ad15a6b166b0 100644 --- a/go.sum +++ b/go.sum @@ -1406,7 +1406,6 @@ github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= -github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc= github.com/ryszard/goskiplist v0.0.0-20150312221310-2dfbae5fcf46 h1:GHRpF1pTW19a8tTFrMLUcfWwyC0pnifVo2ClaLq+hP8= github.com/ryszard/goskiplist v0.0.0-20150312221310-2dfbae5fcf46/go.mod h1:uAQ5PCi+MFsC7HjREoAz1BU+Mq60+05gifQSsHSDG/8= diff --git a/pkg/core/fetch.go b/pkg/core/fetch.go index 12d390709f5d1f..3a03955241a566 100644 --- a/pkg/core/fetch.go +++ b/pkg/core/fetch.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "math" + "strings" "sync" "time" @@ -26,7 +27,6 @@ import ( "github.com/google/uuid" "github.com/rs/zerolog" "github.com/rs/zerolog/log" - "github.com/ryanuber/go-glob" "github.com/thoas/go-funk" gcodes "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -459,30 +459,43 @@ func doGlobResources(requested []string, allowWild bool, all map[string]*schema. result := make([]string, 0, len(requested)) seen := make(map[string]struct{}) for _, r := range requested { + if r == "" { + return nil, diag.FromError(errors.New("invalid resource"), diag.USER, diag.WithDetails("empty resource names are not allowed")) + } + if _, ok := seen[r]; ok { return nil, diag.FromError(fmt.Errorf("resource %q is duplicate", r), diag.USER, diag.WithDetails("configuration has duplicate resources")) } seen[r] = struct{}{} - if _, ok := all[r]; !ok { - if r == "*" { - return nil, diag.FromError(fmt.Errorf("wildcard resource must be the only one in the list"), diag.USER, diag.WithDetails("you can only use * or a list of resources in configuration, but not both")) - } + if _, ok := all[r]; ok { + result = append(result, r) + continue + } - // do glob match - found := false + if r == "*" { + return nil, diag.FromError(fmt.Errorf("wildcard resource must be the only one in the list"), diag.USER, diag.WithDetails("you can only use * or a list of resources in configuration, but not both")) + } + + // do globish match: end with ".*" + found := false + if wildPos := strings.Index(r, ".*"); wildPos > 0 { + if wildPos != len(r)-2 { // make sure it ends with .* + return nil, diag.FromError(errors.New("invalid wildcard syntax"), diag.USER, diag.WithDetails("resource match should end with `.*`")) + } for k := range all { - if glob.Glob(r, k) { + if strings.HasPrefix(k, r[:wildPos+1]) { // include the "." in the match result = append(result, k) found = true } } - if found { - continue - } + } else if wildPos == 0 || strings.Index(r, "*") > -1 { + return nil, diag.FromError(errors.New("invalid wildcard syntax"), diag.USER, diag.WithDetails("you can only use `*` or `resource.*` or full resource name")) + } + + if !found { return nil, diag.FromError(fmt.Errorf("resource %q does not exist", r), diag.USER, diag.WithDetails("configuration refers to a non-existing resource. Maybe you recently downgraded the provider but kept the config, or a typo perhaps?")) } - result = append(result, r) } return cqsort.Unique(result), nil diff --git a/pkg/core/fetch_test.go b/pkg/core/fetch_test.go index 0d383a14f9df0e..ba8f2e5357a4df 100644 --- a/pkg/core/fetch_test.go +++ b/pkg/core/fetch_test.go @@ -317,7 +317,7 @@ func Test_doNormalizeResources(t *testing.T) { "ok, some globs", []string{"c1.*", "c2.res4"}, nil, - map[string]*schema.Table{"c1.res1": nil, "c1.res2": nil, "c2.res3": nil, "c2.res4": nil}, + map[string]*schema.Table{"c1.res1": nil, "c1.res2": nil, "c2.res3": nil, "c2.res4": nil, "c1a.res5": nil}, []string{"c1.res1", "c1.res2", "c2.res4"}, false, }, @@ -325,10 +325,26 @@ func Test_doNormalizeResources(t *testing.T) { "ok, some globs with skips", []string{"c1.*", "c2.res4"}, []string{"c1.res1"}, - map[string]*schema.Table{"c1.res1": nil, "c1.res2": nil, "c2.res3": nil, "c2.res4": nil}, + map[string]*schema.Table{"c1.res1": nil, "c1.res2": nil, "c2.res3": nil, "c2.res4": nil, "c1a.res5": nil}, []string{"c1.res2", "c2.res4"}, false, }, + { + "invalid glob 1", + []string{"c1*res1"}, + nil, + map[string]*schema.Table{"c1.res1": nil, "c1.res2": nil, "c2.res3": nil, "c2.res4": nil}, + nil, + true, + }, + { + "invalid glob 2", + []string{"c1.*res1"}, + nil, + map[string]*schema.Table{"c1.res1": nil, "c1.res2": nil, "c2.res3": nil, "c2.res4": nil}, + nil, + true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 357eb76d3ed483501de01fb814e804fc262a5d40 Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Thu, 12 May 2022 22:25:31 +0100 Subject: [PATCH 3/5] CR test case --- pkg/core/fetch_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/core/fetch_test.go b/pkg/core/fetch_test.go index ba8f2e5357a4df..6be5ec34350505 100644 --- a/pkg/core/fetch_test.go +++ b/pkg/core/fetch_test.go @@ -345,6 +345,14 @@ func Test_doNormalizeResources(t *testing.T) { nil, true, }, + { + "invalid glob 3", + []string{"c1.res*"}, + nil, + map[string]*schema.Table{"c1.res1": nil, "c1.res2": nil, "c2.res3": nil, "c2.res4": nil}, + nil, + true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 7270a44f95fb50177c5d9c103fbc4665fe18f7c8 Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Thu, 12 May 2022 22:26:13 +0100 Subject: [PATCH 4/5] lint --- pkg/core/fetch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/core/fetch.go b/pkg/core/fetch.go index 3a03955241a566..9d024d912375b4 100644 --- a/pkg/core/fetch.go +++ b/pkg/core/fetch.go @@ -489,7 +489,7 @@ func doGlobResources(requested []string, allowWild bool, all map[string]*schema. found = true } } - } else if wildPos == 0 || strings.Index(r, "*") > -1 { + } else if wildPos == 0 || strings.Contains(r, "*") { return nil, diag.FromError(errors.New("invalid wildcard syntax"), diag.USER, diag.WithDetails("you can only use `*` or `resource.*` or full resource name")) } From b59f6c25d0f0a298fc5d88b0fe6fb35bbec00fd8 Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Fri, 13 May 2022 21:39:56 +0100 Subject: [PATCH 5/5] CR --- pkg/core/fetch.go | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/pkg/core/fetch.go b/pkg/core/fetch.go index 9d024d912375b4..31634d24086562 100644 --- a/pkg/core/fetch.go +++ b/pkg/core/fetch.go @@ -477,30 +477,41 @@ func doGlobResources(requested []string, allowWild bool, all map[string]*schema. return nil, diag.FromError(fmt.Errorf("wildcard resource must be the only one in the list"), diag.USER, diag.WithDetails("you can only use * or a list of resources in configuration, but not both")) } - // do globish match: end with ".*" - found := false - if wildPos := strings.Index(r, ".*"); wildPos > 0 { - if wildPos != len(r)-2 { // make sure it ends with .* - return nil, diag.FromError(errors.New("invalid wildcard syntax"), diag.USER, diag.WithDetails("resource match should end with `.*`")) - } - for k := range all { - if strings.HasPrefix(k, r[:wildPos+1]) { // include the "." in the match - result = append(result, k) - found = true - } - } - } else if wildPos == 0 || strings.Contains(r, "*") { - return nil, diag.FromError(errors.New("invalid wildcard syntax"), diag.USER, diag.WithDetails("you can only use `*` or `resource.*` or full resource name")) - } - - if !found { + switch globMatches, diags := matchResourceGlob(r, all); { + case diags.HasDiags(): + return nil, diags + case len(globMatches) == 0: return nil, diag.FromError(fmt.Errorf("resource %q does not exist", r), diag.USER, diag.WithDetails("configuration refers to a non-existing resource. Maybe you recently downgraded the provider but kept the config, or a typo perhaps?")) + default: + result = append(result, globMatches...) } } return cqsort.Unique(result), nil } +// matchResourceGlob matches pattern to the given resources, returns matched resources or diags +// pattern should end with .*, exact matches are not handled. +func matchResourceGlob(pattern string, all map[string]*schema.Table) ([]string, diag.Diagnostics) { + var result []string + wildPos := strings.Index(pattern, ".*") + + if wildPos > 0 { + if wildPos != len(pattern)-2 { // make sure it ends with .* + return nil, diag.FromError(errors.New("invalid wildcard syntax"), diag.USER, diag.WithDetails("resource match should end with `.*`")) + } + for k := range all { + if strings.HasPrefix(k, pattern[:wildPos+1]) { // include the "." in the match + result = append(result, k) + } + } + } else if wildPos == 0 || strings.Contains(pattern, "*") { + return nil, diag.FromError(errors.New("invalid wildcard syntax"), diag.USER, diag.WithDetails("you can only use `*` or `resource.*` or full resource name")) + } + + return result, nil +} + func parseDSN(storage database.Storage, cfg *history.Config) (string, error) { if cfg == nil { parsed, err := dsn.ParseConnectionString(storage.DSN())