Skip to content

Commit e48988b

Browse files
authored
feat(trusted documents): Add failure modes for loading trusted docume… (#156)
…nts, to allow for always starting an instance, even with a partial set of documents
1 parent 3608e59 commit e48988b

File tree

14 files changed

+85
-39
lines changed

14 files changed

+85
-39
lines changed

cmd/serve.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ import (
55
"fmt"
66
"github.com/ldebruijn/graphql-protect/internal/app/config"
77
"github.com/ldebruijn/graphql-protect/internal/app/otel"
8-
"github.com/ldebruijn/graphql-protect/internal/business/persistedoperations"
98
"github.com/ldebruijn/graphql-protect/internal/business/protect"
109
"github.com/ldebruijn/graphql-protect/internal/business/rules/block_field_suggestions"
1110
"github.com/ldebruijn/graphql-protect/internal/business/rules/obfuscate_upstream_errors"
1211
"github.com/ldebruijn/graphql-protect/internal/business/schema"
12+
"github.com/ldebruijn/graphql-protect/internal/business/trusteddocuments"
1313
"github.com/ldebruijn/graphql-protect/internal/http/middleware"
1414
"github.com/ldebruijn/graphql-protect/internal/http/proxy"
1515
"github.com/ldebruijn/graphql-protect/internal/http/readiness"
@@ -40,13 +40,13 @@ func httpServer(log *slog.Logger, cfg *config.Config, shutdown chan os.Signal) e
4040
return nil
4141
}
4242

43-
loader, err := persistedoperations.NewLoaderFromConfig(cfg.PersistedOperations, log)
43+
loader, err := trusteddocuments.NewLoaderFromConfig(cfg.PersistedOperations, log)
4444
if err != nil {
4545
log.Error("Error initializing persisted operations loader", "err", err)
4646
return err
4747
}
4848

49-
po, err := persistedoperations.NewPersistedOperations(log, cfg.PersistedOperations, loader)
49+
po, err := trusteddocuments.NewPersistedOperations(log, cfg.PersistedOperations, loader)
5050
if err != nil {
5151
log.Error("Error initializing Persisted Operations", "err", err)
5252
return nil

cmd/validate.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import (
55
"fmt"
66
"github.com/jedib0t/go-pretty/v6/table"
77
"github.com/ldebruijn/graphql-protect/internal/app/config"
8-
"github.com/ldebruijn/graphql-protect/internal/business/persistedoperations"
98
"github.com/ldebruijn/graphql-protect/internal/business/protect"
109
"github.com/ldebruijn/graphql-protect/internal/business/schema"
10+
"github.com/ldebruijn/graphql-protect/internal/business/trusteddocuments"
1111
"github.com/ldebruijn/graphql-protect/internal/business/validation"
1212
"io"
1313
"log/slog"
@@ -17,15 +17,15 @@ import (
1717
var ErrValidationErrorsFound = errors.New("errors found during validation")
1818

1919
func validate(log *slog.Logger, cfg *config.Config, _ chan os.Signal) error {
20-
loader, err := persistedoperations.NewLoaderFromConfig(cfg.PersistedOperations, log)
20+
loader, err := trusteddocuments.NewLoaderFromConfig(cfg.PersistedOperations, log)
2121
if err != nil {
2222
err := fmt.Errorf("store must be defined to have files to validate")
2323
log.Error("Error running validations", "err", err)
2424
return err
2525
}
2626

2727
// Load the persisted operations from the local dir into memory
28-
persistedOperations, err := persistedoperations.NewPersistedOperations(log, cfg.PersistedOperations, loader)
28+
persistedOperations, err := trusteddocuments.NewPersistedOperations(log, cfg.PersistedOperations, loader)
2929
if err != nil {
3030
log.Error("Error initializing Persisted Operations", "err", err)
3131
return nil

internal/app/config/config.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"errors"
55
"github.com/ldebruijn/graphql-protect/internal/app/http"
66
"github.com/ldebruijn/graphql-protect/internal/app/log"
7-
"github.com/ldebruijn/graphql-protect/internal/business/persistedoperations"
87
"github.com/ldebruijn/graphql-protect/internal/business/rules/accesslogging"
98
"github.com/ldebruijn/graphql-protect/internal/business/rules/aliases"
109
"github.com/ldebruijn/graphql-protect/internal/business/rules/batch"
@@ -13,6 +12,7 @@ import (
1312
"github.com/ldebruijn/graphql-protect/internal/business/rules/max_depth"
1413
"github.com/ldebruijn/graphql-protect/internal/business/rules/tokens"
1514
"github.com/ldebruijn/graphql-protect/internal/business/schema"
15+
"github.com/ldebruijn/graphql-protect/internal/business/trusteddocuments"
1616
"github.com/ldebruijn/graphql-protect/internal/http/proxy"
1717
y "gopkg.in/yaml.v3"
1818
"os"
@@ -24,7 +24,7 @@ type Config struct {
2424
Web http.Config `yaml:"web"`
2525
Schema schema.Config `yaml:"schema"`
2626
Target proxy.Config `yaml:"target"`
27-
PersistedOperations persistedoperations.Config `yaml:"persisted_operations"`
27+
PersistedOperations trusteddocuments.Config `yaml:"persisted_operations"`
2828
ObfuscateValidationErrors bool `yaml:"obfuscate_validation_errors"`
2929
ObfuscateUpstreamErrors bool `yaml:"obfuscate_upstream_errors"`
3030
BlockFieldSuggestions block_field_suggestions.Config `yaml:"block_field_suggestions"`
@@ -72,7 +72,7 @@ func defaults() Config {
7272
Web: http.DefaultConfig(),
7373
Schema: schema.DefaultConfig(),
7474
Target: proxy.DefaultConfig(),
75-
PersistedOperations: persistedoperations.DefaultConfig(),
75+
PersistedOperations: trusteddocuments.DefaultConfig(),
7676
ObfuscateValidationErrors: false,
7777
ObfuscateUpstreamErrors: true,
7878
BlockFieldSuggestions: block_field_suggestions.DefaultConfig(),

internal/app/config/config_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package config
33
import (
44
"github.com/ldebruijn/graphql-protect/internal/app/http"
55
"github.com/ldebruijn/graphql-protect/internal/app/log"
6-
"github.com/ldebruijn/graphql-protect/internal/business/persistedoperations"
76
"github.com/ldebruijn/graphql-protect/internal/business/rules/accesslogging"
87
"github.com/ldebruijn/graphql-protect/internal/business/rules/aliases"
98
"github.com/ldebruijn/graphql-protect/internal/business/rules/batch"
@@ -12,6 +11,7 @@ import (
1211
"github.com/ldebruijn/graphql-protect/internal/business/rules/max_depth"
1312
"github.com/ldebruijn/graphql-protect/internal/business/rules/tokens"
1413
"github.com/ldebruijn/graphql-protect/internal/business/schema"
14+
"github.com/ldebruijn/graphql-protect/internal/business/trusteddocuments"
1515
"github.com/ldebruijn/graphql-protect/internal/http/proxy"
1616
"github.com/stretchr/testify/assert"
1717
"gopkg.in/yaml.v3"
@@ -151,9 +151,9 @@ log:
151151
KeepAlive: 1 * time.Second,
152152
Host: "host",
153153
},
154-
PersistedOperations: persistedoperations.Config{
154+
PersistedOperations: trusteddocuments.Config{
155155
Enabled: true,
156-
Loader: persistedoperations.LoaderConfig{
156+
Loader: trusteddocuments.LoaderConfig{
157157
Type: "gcp",
158158
Location: "some-bucket",
159159
Reload: struct {

internal/business/protect/protect.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ import (
55
"errors"
66
"github.com/ldebruijn/graphql-protect/internal/app/config"
77
"github.com/ldebruijn/graphql-protect/internal/business/gql"
8-
"github.com/ldebruijn/graphql-protect/internal/business/persistedoperations"
98
"github.com/ldebruijn/graphql-protect/internal/business/rules/accesslogging"
109
"github.com/ldebruijn/graphql-protect/internal/business/rules/aliases"
1110
"github.com/ldebruijn/graphql-protect/internal/business/rules/batch"
1211
"github.com/ldebruijn/graphql-protect/internal/business/rules/enforce_post"
1312
"github.com/ldebruijn/graphql-protect/internal/business/rules/max_depth"
1413
"github.com/ldebruijn/graphql-protect/internal/business/rules/tokens"
1514
"github.com/ldebruijn/graphql-protect/internal/business/schema"
15+
"github.com/ldebruijn/graphql-protect/internal/business/trusteddocuments"
1616
"github.com/vektah/gqlparser/v2/ast"
1717
"github.com/vektah/gqlparser/v2/gqlerror"
1818
"github.com/vektah/gqlparser/v2/parser"
@@ -39,7 +39,7 @@ type GraphQLProtect struct {
3939
preFilterChain func(handler http.Handler) http.Handler
4040
}
4141

42-
func NewGraphQLProtect(log *slog.Logger, cfg *config.Config, po *persistedoperations.Handler, schema *schema.Provider, upstreamHandler http.Handler) (*GraphQLProtect, error) {
42+
func NewGraphQLProtect(log *slog.Logger, cfg *config.Config, po *trusteddocuments.Handler, schema *schema.Provider, upstreamHandler http.Handler) (*GraphQLProtect, error) {
4343
aliases.NewMaxAliasesRule(cfg.MaxAliases)
4444
max_depth.NewMaxDepthRule(log, cfg.MaxDepth)
4545
maxBatch, err := batch.NewMaxBatch(cfg.MaxBatch)

internal/business/persistedoperations/dir_loader.go renamed to internal/business/trusteddocuments/dir_loader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package persistedoperations // nolint:revive
1+
package trusteddocuments // nolint:revive
22

33
import (
44
"context"

internal/business/persistedoperations/gcp_loader.go renamed to internal/business/trusteddocuments/gcp_loader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package persistedoperations // nolint:revive
1+
package trusteddocuments // nolint:revive
22

33
import (
44
"cloud.google.com/go/storage"

internal/business/persistedoperations/loader.go renamed to internal/business/trusteddocuments/loader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package persistedoperations // nolint:revive
1+
package trusteddocuments // nolint:revive
22

33
import (
44
"context"

internal/business/persistedoperations/memory_loader.go renamed to internal/business/trusteddocuments/memory_loader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package persistedoperations // nolint:revive
1+
package trusteddocuments // nolint:revive
22

33
import (
44
"context"

internal/business/persistedoperations/model.go renamed to internal/business/trusteddocuments/model.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package persistedoperations
1+
package trusteddocuments
22

33
import (
44
"encoding/json"

internal/business/persistedoperations/model_test.go renamed to internal/business/trusteddocuments/model_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package persistedoperations
1+
package trusteddocuments
22

33
import (
44
"github.com/stretchr/testify/assert"

internal/business/persistedoperations/nooploader.go renamed to internal/business/trusteddocuments/nooploader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package persistedoperations
1+
package trusteddocuments
22

33
import "context"
44

internal/business/persistedoperations/persisted_operations.go renamed to internal/business/trusteddocuments/persisted_operations.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package persistedoperations // nolint:revive
1+
package trusteddocuments // nolint:revive
22

33
import (
44
"bytes"
@@ -43,6 +43,11 @@ var (
4343
)
4444
)
4545

46+
type ReloadFailureStrategy string
47+
48+
var ReloadFailureStrategyIgnore ReloadFailureStrategy = "ignore-failure"
49+
var ReloadFailureStrategyReject ReloadFailureStrategy = "reject-on-failure"
50+
4651
type ErrorPayload struct {
4752
Errors gqlerror.List `json:"errors"`
4853
}
@@ -145,7 +150,7 @@ func NewPersistedOperations(log *slog.Logger, cfg Config, loader Loader) (*Handl
145150
refreshLock: sync.Mutex{},
146151
}
147152

148-
err := poh.load()
153+
err := poh.load(ReloadFailureStrategyIgnore)
149154
if err != nil {
150155
return nil, err
151156
}
@@ -257,11 +262,14 @@ func (p *Handler) Validate(validate func(operation string) gqlerror.List) []vali
257262
return errs
258263
}
259264

260-
func (p *Handler) load() error {
265+
func (p *Handler) load(failureStrategy ReloadFailureStrategy) error {
261266
newState, err := p.loader.Load(context.Background())
262267
if err != nil {
263268
loadingResultCounter.WithLabelValues(p.loader.Type(), "failure").Inc()
264-
return err
269+
if failureStrategy == ReloadFailureStrategyReject {
270+
return err
271+
}
272+
// implicit fall through for other failure types
265273
}
266274

267275
p.lock.Lock()
@@ -291,7 +299,7 @@ func (p *Handler) reloadProcessor() {
291299
p.log.Warn("Refresh ticker still running while next tick")
292300
continue
293301
}
294-
err := p.load()
302+
err := p.load(ReloadFailureStrategyReject)
295303
if err != nil {
296304
continue
297305
}

internal/business/persistedoperations/persisted_operations_test.go renamed to internal/business/trusteddocuments/persisted_operations_test.go

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package persistedoperations // nolint:revive
1+
package trusteddocuments // nolint:revive
22

33
import (
44
"bytes"
@@ -265,8 +265,9 @@ func TestNewPersistedOperations(t *testing.T) {
265265

266266
func TestLoader(t *testing.T) {
267267
type args struct {
268-
state map[string]PersistedOperation
269-
loader Loader
268+
state map[string]PersistedOperation
269+
loader Loader
270+
failureStrategy ReloadFailureStrategy
270271
}
271272
tests := []struct {
272273
name string
@@ -288,7 +289,8 @@ func TestLoader(t *testing.T) {
288289

289290
return loader
290291
}(),
291-
state: map[string]PersistedOperation{},
292+
state: map[string]PersistedOperation{},
293+
failureStrategy: ReloadFailureStrategyReject,
292294
},
293295
want: map[string]PersistedOperation{
294296
"123": {
@@ -318,6 +320,7 @@ func TestLoader(t *testing.T) {
318320
Name: "i am a name that doest get deleted",
319321
},
320322
},
323+
failureStrategy: ReloadFailureStrategyReject,
321324
},
322325
want: map[string]PersistedOperation{
323326
"123": {
@@ -328,10 +331,10 @@ func TestLoader(t *testing.T) {
328331
wantErr: nil,
329332
},
330333
{
331-
name: "loader error does not update cache",
334+
name: "loader error does not update cache with failure mode reject",
332335
args: args{
333336
loader: func() Loader {
334-
loader := &errorLoader{
337+
loader := &testLoader{
335338
err: errors.New("this is unexpected"),
336339
willReturnError: false,
337340
}
@@ -344,6 +347,7 @@ func TestLoader(t *testing.T) {
344347
Name: "i am a name that doest not get deleted",
345348
},
346349
},
350+
failureStrategy: ReloadFailureStrategyReject,
347351
},
348352
want: map[string]PersistedOperation{
349353
"456": {
@@ -353,14 +357,47 @@ func TestLoader(t *testing.T) {
353357
},
354358
wantErr: errors.New("this is unexpected"),
355359
},
360+
{
361+
name: "loader error does update cache with failure mode ignore",
362+
args: args{
363+
loader: func() Loader {
364+
loader := &testLoader{
365+
err: errors.New("this is unexpected"),
366+
data: map[string]PersistedOperation{
367+
"123": {
368+
Operation: "i am an operation",
369+
Name: "i am a name",
370+
},
371+
},
372+
willReturnError: false,
373+
}
374+
375+
return loader
376+
}(),
377+
state: map[string]PersistedOperation{
378+
"456": {
379+
Operation: "i am an operation that does not get deleted",
380+
Name: "i am a name that doest not get deleted",
381+
},
382+
},
383+
failureStrategy: ReloadFailureStrategyIgnore,
384+
},
385+
want: map[string]PersistedOperation{
386+
"123": {
387+
Operation: "i am an operation",
388+
Name: "i am a name",
389+
},
390+
},
391+
wantErr: errors.New("this is unexpected"),
392+
},
356393
}
357394
for _, tt := range tests {
358395
t.Run(tt.name, func(t *testing.T) {
359396
log := slog.Default()
360397
po, _ := NewPersistedOperations(log, Config{}, tt.args.loader)
361398
po.cache = tt.args.state
362399

363-
err := po.load()
400+
err := po.load(tt.args.failureStrategy)
364401
if tt.wantErr != nil {
365402
assert.Error(t, tt.wantErr, err)
366403
}
@@ -370,28 +407,29 @@ func TestLoader(t *testing.T) {
370407
}
371408
}
372409

373-
var _ Loader = &errorLoader{}
410+
var _ Loader = &testLoader{}
374411

375412
// ErrorLoader is a loader for testing purposes
376-
type errorLoader struct {
413+
type testLoader struct {
414+
data map[string]PersistedOperation
377415
err error
378416
willReturnError bool
379417
}
380418

381-
func (e *errorLoader) Type() string {
419+
func (e *testLoader) Type() string {
382420
return "error"
383421
}
384422

385423
func newPersistedOperation(query string) PersistedOperation {
386424
return PersistedOperation{query, extractOperationNameFromOperation(query)}
387425
}
388426

389-
func (e *errorLoader) Load(_ context.Context) (map[string]PersistedOperation, error) {
427+
func (e *testLoader) Load(_ context.Context) (map[string]PersistedOperation, error) {
390428
if e.willReturnError {
391-
return nil, e.err
429+
return e.data, e.err
392430
}
393431
// return error after the first call
394432
e.willReturnError = true
395433

396-
return map[string]PersistedOperation{}, nil
434+
return e.data, nil
397435
}

0 commit comments

Comments
 (0)