8000 Proceed when config path is missing for environment by Firstyear · Pull Request #3630 · kanidm/kanidm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Proceed when config path is missing for environment #3630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 13, 2025

Conversation

Firstyear
Copy link
Member

Change summary

When a configuration path is specified then the server will force it to be used, and will error if it can't be found. The container previously manually specified this path on the command line which forced the configuration to need to exist.

Remove the configuration from the docker cli, the default config path test will discover if the file exists or not and will correctly handle the flow when the file is absent and environment only configuration is used.

Fixes #3617

Checklist

  • This PR contains no AI generated code
  • book chapter included (if relevant)
  • design document included (if relevant)

@Firstyear
Copy link
Member Author

For reference, the important lines are here:

https://github.com/kanidm/kanidm/blob/master/server/daemon/src/main.rs#L565

If the command line requests a configuration path with opt.config_path, then we check that it must exist. If no path is requested, we fall through and check if the default config path exists first, and only if it does, then we attempt to force it's use.

In this situation, there is no config file present, so this will cause the None case and we skip this.

@Firstyear Firstyear marked this pull request as ready for review May 13, 2025 03:49
Copy link
Member
@yaleman yaleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ezpz

@github-project-automation github-project-automation bot moved this from 🆕 New to 🔖 Ready in Organising Everything May 13, 2025
@yaleman yaleman merged commit 97952d5 into kanidm:master May 13, 2025
25 checks passed
@github-project-automation github-project-automation bot moved this from 🔖 Ready to ✅ Done in Organising Everything May 13, 2025
Firstyear added a commit that referenced this pull request May 14, 2025
@Doridian
Copy link

@Firstyear Sorry to respond to a closed/merged PR, but reading this change randomly and looking into the Dockerfile.
The HEALTHCHECK line here: https://github.com/kanidm/kanidm/blob/master/server/Dockerfile#L101-L107
also specifies a config path. So that would mean if the config file does not exist, as described here, the health check would always fail still?

@Firstyear
Copy link
Member Author

True, it would. I'll fix the PR.

@Firstyear
Copy link
Member Author

@Doridian Good catch btw, #3656

@Firstyear Firstyear deleted the 20250513-3617-env-config branch May 26, 2025 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

1.6.0: Issue upgrading with env var only config
3 participants
0