feat(example): Allow configuring some parameters with env variables #663
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #663 +/- ##
==========================================
+ Coverage 60.06% 61.02% +0.96%
==========================================
Files 80 81 +1
Lines 6998 7418 +420
==========================================
+ Hits 4203 4527 +324
- Misses 2498 2581 +83
- Partials 297 310 +13 ☔ View full report in Codecov by Sentry. |
| // If there is no such variable defined, then use default values. | ||
| func FromEnvVars(defaults *Config) *Config { | ||
| if defaults == nil { | ||
| defaults = &Config{} |
There was a problem hiding this comment.
Shouldn't DefaultIssuerPort be set to the Config here?
There was a problem hiding this comment.
That works as planned - if the defaults is nil, then user didn't want to have any defaults and expects everything to be set in the env vars.
example/server/config/config.go
Outdated
| if value, ok := os.LookupEnv("USERS_FILE"); ok { | ||
| cfg.UsersFile = value | ||
| } | ||
| if value, ok := os.LookupEnv("REDIRECT_URIS"); ok { |
There was a problem hiding this comment.
Please keep the environment variable name REDIRECT_URI, so we don't break existing uses of this variables.
| if value, ok := os.LookupEnv("REDIRECT_URIS"); ok { | |
| if value, ok := os.LookupEnv("REDIRECT_URI"); ok { |
|
Hi @lanseg I noticed you pushed another commit with additional changes after the initial review was done, not related to the review comments. Although I do not have a problem with the additional change, this is not a nice netiquette. If you intent to continue work on a PR, it should be marked as "draft" and not "ready for review". Please keep that in mind for future contributions to any github project. You also have introduced a build error that needs to be fixed. I'll mark this as draft for the moment and please mark it ready for review once you are done. |
lanseg
force-pushed
the
main
branch
2 times, most recently
from
58c9abb
to
514c1de
Compare
I'm very sorry for causing a mess, github workflow confused me a bit better. |
This change allows to load users from the json file.
That should help to create better test scenarios when using example server as fake Zitadel instance for testing. For example, having several users with different permissions or presence in the internal base (known user, unknown user, blocked user, etc).
We don't need dynamic server for testing, but I could update it to make env variables unified if needed.
This is an extended version of another pull request: #656, which was deleted by mistake.