Skip to content
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

SAML Support for docker #463

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

LoveSkylark
Copy link

Added a new variable that allows the docker to install plugins that are needed for SAML.

"rebased"

@dot-mike
Copy link

dot-mike commented Dec 4, 2024

I have one issue with this... INSTALL_PLUGINS is set-up so installing plugins only happens after container is started because it's a service in root-fs. That means running lnms plugin:add will require internet to work. Not everyone has internet access after an image is built and will be running internally ;)

A better way would be to install the plugins in Dockerfile during image build, all though this requires more steps to ensure things are done in the right order.

@LoveSkylark
Copy link
Author

This is by design - installing third-party plugins in the Docker image has two major drawbacks:

  1. You'd need to maintain and regularly update all included plugins within the image
  2. You'd need to predict and include all possible plugins users might need

Note: If you don't have Internet access, you likely won't be using SAML auth anyway.

This approach is a compromise - plugins are installed during container startup if needed, rather than being pre-installed. While this adds some startup time, it keeps the image lean and flexible, allowing you to use plugins only when required. The installation only occurs when the "INSTALL_PLUGINS" variable is set, so it only affects those willing to accept that startup cost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants