feat(dex): use postgresql as backend #839
Conversation
pkg/features/features.go
Outdated
| "github.com/cloudoperators/greenhouse/pkg/clientutil" | ||
| ) | ||
|
|
||
| func GetOFClient(appName string) *openfeature.Client { |
There was a problem hiding this comment.
It would be helpful to have some context to what this client does. Maybe something like this?
| func GetOFClient(appName string) *openfeature.Client { | |
| // NewClient returns a open-feature client loading feature flags from a ConfigMap | |
| func NewClient(appName string) *openfeature.Client { |
There was a problem hiding this comment.
sure, we are not finished fully with this PR, will do some meaningful commenting into the new functions
There was a problem hiding this comment.
added some comments to the code and renamed the func to better correlate in 4baeee0
pkg/features/features.go
Outdated
| } | ||
|
|
||
| goFeatureFlagConfig := &goffclient.Config{ | ||
| PollingInterval: 30 * time.Second, |
There was a problem hiding this comment.
Is the polling continuously? In that case we could set the value very high, as this is only evaluated during startup.
There was a problem hiding this comment.
30 * time.Minutes sounds better ?
There was a problem hiding this comment.
increased in 42393b6
cmd/idproxy/main.go
Outdated
| evalCtx := openfeature.NewEvaluationContext( | ||
| "backend", | ||
| map[string]interface{}{}, | ||
| ) | ||
| postgresBackend, err := ofClient.BooleanValue(context.TODO(), "dex-sql-backend", false, evalCtx) | ||
| if err != nil { | ||
| log.Fatalf("Failed to initialize kubernetes storage: %s", err) | ||
| log.Fatalf("Failed to get dex-sql-backend value details: %s", err) | ||
| } | ||
|
|
||
| k8sBackend, err := ofClient.BooleanValue(context.TODO(), "dex-kubernetes-backend", false, evalCtx) | ||
| if err != nil { | ||
| log.Fatalf("Failed to get dex-kubernetes-backend value details: %s", err) | ||
| } | ||
|
|
||
| switch { | ||
| case postgresBackend: | ||
| dexStorage, err = idproxy.NewPostgresStorage(sql.SSL{Mode: "disable"}, postgresDB, logger.With("component", "storage")) | ||
| if err != nil { | ||
| log.Fatalf("Failed to initialize postgres storage: %s", err) | ||
| } | ||
| case k8sBackend: | ||
| dexStorage, err = idproxy.NewKubernetesStorage(kubeconfig, kubecontext, kubenamespace, logger.With("component", "storage")) | ||
| if err != nil { | ||
| log.Fatalf("Failed to initialize kubernetes storage: %s", err) | ||
| } | ||
| default: | ||
| log.Fatalf("Exactly one of dex-sql-backend or dex-kubernetes-backend must be true") |
There was a problem hiding this comment.
We have two flags for the same feature but only one may be toggled. Would it be possible to combine them into one flag with the value for either k8s or postgres?
There was a problem hiding this comment.
Not easily with this feature-flag
pkg/controllers/organization/dex.go
Outdated
| if oidcConnector, err = r.dexStorage.GetConnector(org.Name); err != nil { | ||
| if err = r.dexStorage.CreateConnector(ctx, storage.Connector{ |
There was a problem hiding this comment.
The error from line 95 must not always be a NotFound error. Directly trying to create a new connector could then lead to AlreadyExists error but obscurring the underlying issue.
pkg/controllers/organization/dex.go
Outdated
| if oAuthClient, err = r.dexStorage.GetClient(org.Name); err != nil { | ||
| if err = r.dexStorage.CreateClient(ctx, storage.Client{ |
There was a problem hiding this comment.
Same as above w.r.t. obscuring the error condition
Description
What type of PR is this? (check all applicable)
Related Tickets & Documents
Added tests?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Added to documentation?
Checklist