Skip to content

Commit 61765a7

Browse files
authored
fix: Bring back plugin validation (#1108)
Fixes #1106 Plugin validation was removed in https://github.com/cloudquery/plugin-sdk/pull/984/files#diff-95f04007d983a3bfdb080b338b9de5a8bbd73a3d65910fbe664f97788a9021b3. As a result we don't validate tables and columns anymore, see resulted bug in cloudquery/cloudquery#12428. This PR brings it back and per a better option puts the validation in the `Init` method. Other options: 1. Have each plugin call `.validate` on the plugin side, probably after transformation [here](https://github.com/cloudquery/cloudquery/blob/03fae6d803a4e2ac00ff83d9e90a4169b247c346/plugins/source/aws/resources/plugin/tables.go#L565). Downside that each plugin needs to implement the call 2. Have the validation done as a part of `schema.TransformTables` [here](https://github.com/cloudquery/plugin-sdk/blob/8a6a2cc87e303d66c810e629a497f9acd3c4982c/transformers/tables.go#L30). Downside it will happen before any custom transformation that are executed on tables like [`titleTransformer`](https://github.com/cloudquery/cloudquery/blob/03fae6d803a4e2ac00ff83d9e90a4169b247c346/plugins/source/aws/resources/plugin/tables.go#L567) > Please note that before it was removed `validate` was called during `NewPlugin` which means it errored right after calling the `serve` command, meaning the plugin would not start. With this fix the plugin will error out during `Init`, which means you need to run `sync/migrate` to see the error ---
1 parent e1ea578 commit 61765a7

File tree

4 files changed

+80
-5
lines changed

4 files changed

+80
-5
lines changed

plugin/plugin.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,11 @@ func (p *Plugin) Init(ctx context.Context, spec []byte, options NewClientOptions
150150
if err != nil {
151151
return fmt.Errorf("failed to initialize client: %w", err)
152152
}
153+
154+
if err := p.validate(ctx); err != nil {
155+
return fmt.Errorf("failed to validate tables: %w", err)
156+
}
157+
153158
p.spec = spec
154159

155160
return nil

plugin/validate.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package plugin
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
8+
"github.com/cloudquery/plugin-sdk/v4/schema"
9+
)
10+
11+
func validateTables(tables schema.Tables) error {
12+
if err := tables.ValidateDuplicateTables(); err != nil {
13+
return fmt.Errorf("found duplicate tables in plugin: %w", err)
14+
}
15+
16+
if err := tables.ValidateTableNames(); err != nil {
17+
return fmt.Errorf("found table with invalid name in plugin: %w", err)
18+
}
19+
20+
if err := tables.ValidateDuplicateColumns(); err != nil {
21+
return fmt.Errorf("found duplicate columns in plugin: %w", err)
22+
}
23+
24+
if err := tables.ValidateColumnNames(); err != nil {
25+
return fmt.Errorf("found column with invalid name in plugin: %w", err)
26+
}
27+
28+
return nil
29+
}
30+
31+
func (p *Plugin) validate(ctx context.Context) error {
32+
tables, err := p.client.Tables(ctx, TableOptions{Tables: []string{"*"}})
33+
// ErrNotImplemented means it's a destination only plugin
34+
if err != nil && !errors.Is(err, ErrNotImplemented) {
35+
return fmt.Errorf("failed to get tables: %w", err)
36+
}
37+
38+
return validateTables(tables)
39+
}

schema/table.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -342,12 +342,13 @@ func (tt Tables) ValidateDuplicateColumns() error {
342342
}
343343

344344
func (tt Tables) ValidateDuplicateTables() error {
345+
tableNames := tt.TableNames()
345346
tables := make(map[string]bool, len(tt))
346-
for _, t := range tt {
347-
if _, ok := tables[t.Name]; ok {
348-
return fmt.Errorf("duplicate table %s", t.Name)
347+
for _, t := range tableNames {
348+
if _, ok := tables[t]; ok {
349+
return fmt.Errorf("duplicate table %s", t)
349350
}
350-
tables[t.Name] = true
351+
tables[t] = true
351352
}
352353
return nil
353354
}
@@ -405,7 +406,7 @@ func (t *Table) ValidateName() error {
405406
if !ok {
406407
return fmt.Errorf("table name %q is not valid: table names must contain only lower-case letters, numbers and underscores, and must start with a lower-case letter or underscore", t.Name)
407408
}
408-
return nil
409+
return ValidateTable(t)
409410
}
410411

411412
func (t *Table) PrimaryKeysIndexes() []int {

schema/table_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,3 +390,33 @@ func TestTableToAndFromArrow(t *testing.T) {
390390
t.Errorf("diff (+got, -want): %v", diff)
391391
}
392392
}
393+
394+
func TestValidateDuplicateTables(t *testing.T) {
395+
tests := []struct {
396+
name string
397+
tables Tables
398+
err string
399+
}{
400+
{
401+
name: "should return error when duplicate tables are found",
402+
tables: Tables{{Name: "table1"}, {Name: "table1"}},
403+
err: "duplicate table table1",
404+
},
405+
{
406+
name: "should return error when duplicate relational tables are found",
407+
tables: Tables{{Name: "table1", Relations: []*Table{{Name: "table2"}, {Name: "table2"}}}},
408+
err: "duplicate table table2",
409+
},
410+
}
411+
412+
for _, tc := range tests {
413+
t.Run(tc.name, func(t *testing.T) {
414+
err := tc.tables.ValidateDuplicateTables()
415+
if tc.err != "" {
416+
require.ErrorContains(t, err, tc.err)
417+
} else {
418+
require.NoError(t, err)
419+
}
420+
})
421+
}
422+
}

0 commit comments

Comments
 (0)