Feature/auth docker setup docs#12459
Conversation
- Fix auth failures across ldap-direct, ldap-sync, and Keycloak login flows - Update Jenkins build flow to support the LDAP_MODE parameter - Align LDAP LDIFs, datadir config, sample env files, and compose settings - Simplify LDAP and Keycloak healthchecks - Replace sample secrets with disposable placeholder credentials - Update docs with new requirements and setup procedure
mahesh-wor
force-pushed
the
feature/auth-docker-setup-docs
branch
from
7114fa7 to
9cd7319
Compare
mahesh-wor
force-pushed
the
feature/auth-docker-setup-docs
branch
from
1799b41 to
cdf04bc
Compare
|
hi @offtherailz i have fixed the lint issue, along with some docs and sample conf updates. It's ready to review now. |
offtherailz
left a comment
offtherailz
left a comment
There was a problem hiding this comment.
See comments inline, there are some issues.
I see a sparse management of LDAP with 3 different implementation and a default in default override --> this is a problem, remove from it
Instead we should use only one LDAP image, properly active and maintained, with only one setup.
Of couse these changes have to be properly reflected in documentation produced for configuration
I can suggest to:
- remove acme-ldap DockerFile
- remove ldif/02-users.ldpf on gitignore and commit it directly, removing ldif.sample
- replace osixia with bitnami/openldap, activiely maintained
- remove changes on docker-compose-override , use a separate override / profile for that
- update web/src/main/resources/ldap.properties from port 10389 a 389 (standard)
| ldap: | ||
| build: | ||
| context: ./tests/acme-ldap/ | ||
| image: mapstore-test/acme-ldap | ||
| profiles: | ||
| - base | ||
| healthcheck: | ||
| test: ["CMD-SHELL", "bash -c '</dev/tcp/localhost/10389'"] | ||
| interval: 5s | ||
| timeout: 5s | ||
| retries: 30 | ||
| start_period: 5s | ||
| networks: | ||
| - mapstore-network | ||
|
|
There was a problem hiding this comment.
This is the stanard override file one. Start always mapstore with LDAP. This is not the base scenario.
Base is mapstore with DB. Then an override allows ldap and another overrided allows keycloak
This and the service_healthy on ldap imposes that the ldap is present on every installation. Please add an override file to apply in cascade, with another name, and remove these changes
There was a problem hiding this comment.
introduced tests/docker-compose-tests.yml. So docker-compose-override.yml is untouched for now. If the docker-compose-override.yml is only used for tests we can move it and later use it for meaningful use orther then just running test.
we don't know rn so just overriding currently.
Endpoint tests adds in new compose and will work this way now
docker compose --profile test -f docker-compose.yml -f docker-compose-override.yml -f tests/docker-compose-tests.yml up --build --exit-code-from sut
| interval: 5s | ||
| timeout: 10s | ||
| retries: 120 | ||
| platform: linux/amd64 |
There was a problem hiding this comment.
This fixes problems probably on mac, but can be a problem, should be removed
| ldap: | ||
| condition: service_healthy |
| condition: service_healthy | ||
| volumes: | ||
| - ./tests/geostore-datasource-ovr.properties:/mapstore/datadir/geostore-datasource-ovr.properties:rw | ||
| - ./tests/ldap.properties:/mapstore/datadir/ldap.properties:rw |
| @@ -0,0 +1,4 @@ | |||
| FROM osixia/openldap:1.5.0 | |||
There was a problem hiding this comment.
osixia/openldap is unmaintained
Alternatives:
- bitnami/openldap similar API
- gh.io/nitnelave/lldap light, web UI
We are using 3 different LDAP alternatives...
- acme
- osixia/openldap
- geosolutions-it/openldap
I'd suggest to use only one implemntation, probably based to a maintained one.
There was a problem hiding this comment.
bitnami/openldap is private only, there seems an option to use bitnamilegacy/openldap but it's again legacy and no better than osixia/openldap. osixia/openldap does have v2 but it's in beta.
Writing a custom Dockerfile and init script for building image from scratch might be too much here. Maybe a seperate geosolutions-it/openldap would be nice, if we plan to use dockerized ldap setups.
For now i kept is the same.
|
Please @mahesh-wor look at my comments |
|
Thank you @offtherailz i will check it and update you. |
mahesh-wor
force-pushed
the
feature/auth-docker-setup-docs
branch
from
f8a1dcf to
648fadb
Compare
3 participants
Description
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
What is the current behavior?
Currently we only have setup instructions and docker compose for mapstore/postgres/proxy services.
https://github.com/geosolutions-it/DevOps/issues/1878
What is the new behavior?
This PR introduces optional
docker/docker-compose.auth.ymlthat dev's and users can leverage to have working authentication integration for LDAP/Keycloak. Also includes sample environments and configurations.Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)
Other useful information