storage: make static storages query real storages for some actions #855
Conversation
ericchiang
force-pushed
the
static-storage-fallthrough
branch
3 times, most recently
from
a6caa69
to
2bbf996
Compare
| } | ||
|
|
||
| func (s staticClientsStorage) CreateClient(c Client) error { | ||
| return errors.New("static clients: read-only cannot create client") | ||
| if s.isStatic(c.ID) { | ||
| return errors.New("static clients: read-only cannot create client") |
There was a problem hiding this comment.
Should we show a different error message that implies that the client already exists as a static client? Similar comment for CreatePassword
There was a problem hiding this comment.
I don't like adding yet another sentinel error :( Maybe we could introduce a storage.Error interface instead? Something like
type ErrKind int
const (
ErrAlreadyExists ErrKind = iota
ErrNotFound
ErrNotModifiable
ErrUnknown
)
type Error interface {
Kind() ErrKind
}Then use that to do all of this custom detection?
There was a problem hiding this comment.
Nice article on these kind of errors here: https://dave.cheney.net/2014/12/24/inspecting-errors
There was a problem hiding this comment.
Yep that would be nice.
There was a problem hiding this comment.
Something we want to address in this PR or later? I'm leaning towards later.
There was a problem hiding this comment.
ok we could address it separately
| s2 := storage.WithStaticClients(s, []storage.Client{c2}) | ||
| c3 := storage.Client{ID: "spam", Secret: "spam_secret"} | ||
|
|
||
| backing.CreateClient(c1) |
There was a problem hiding this comment.
Do we want to add a similar test for Passwords?
If dex is configured with static passwords or clients, let the API still add or modify objects in the backing storage, so long as their IDs don't conflict with the static ones. List options now aggregate resources from the static list and backing storage.
ericchiang
force-pushed
the
static-storage-fallthrough
branch
from
2bbf996
to
4c39bc2
Compare
storage: make static storages query real storages for some actions
If dex is configured with static passwords or clients, let the API
still add or modify objects in the backing storage, so long as
their IDs don't conflict with the static ones. List options now
aggregate resources from the static list and backing storage.
Closes #735
Still needs unit tests.