-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[doc/example] Make LDAP example functional again by running OpenLDAP with docker-compose #1762
Conversation
aea5bfb
to
b4d22bf
Compare
…ocker Signed-off-by: Martin Heide <martin.heide@faro.com>
Signed-off-by: Martin Heide <martin.heide@faro.com>
@@ -11,7 +11,7 @@ connectors: | |||
name: OpenLDAP | |||
id: ldap | |||
config: | |||
host: localhost:10389 | |||
host: localhost:389 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's a strong opinion to use the nonstandard port, I can revert it.
But then the port would be different here (outside the container) and when using "docker exec" (inside the container).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the rationale behind using a non-standard port is avoiding potential collision with an existing service. But all the other examples (mysql, etc) use standard ports, so it should be okay.
Thanks @heidemn-faro . Personally, I'd much rather see a docker-compose based solution, without adding back that script. I believe the OpenLDAP image is able to add definitions automatically on startup. |
d52d592
to
521954a
Compare
Signed-off-by: Martin Heide <martin.heide@faro.com>
Signed-off-by: Martin Heide <martin.heide@faro.com>
@sagikazarmark as requested, I've reworked this to use docker-compose. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks @heidemn-faro !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@sagikazarmark @bonifaido thanks for reviewing. I don't have permissions to merge, so you would have to do that. |
The official docker release for this release can be pulled from dexidp/dex:v2.25.0 **Features:** - Move the API package to a separate module (dexidp#1741, @sagikazarmark) - OAuth2 Device Authorization Grant (dexidp#1706, @justin-slowik) - Support username, email and groups claim in OIDC connector (dexidp#1634, @xtremerui) **Bugfixes:** - Add offline_access scope in microsoft connector, if required (dexidp#1441, @jimmythedog) - Allow the google connector to work without a service account (dexidp#1720, @candlerb) **Minor changes:** - Remove vendor (finally) (dexidp#1745, @sagikazarmark) - Fix the LDAP example (dexidp#1762, @heidemn-faro) - Relocate the example app (dexidp#1764, @sagikazarmark)
The official docker release for this release can be pulled from dexidp/dex:v2.25.0 **Features:** - Move the API package to a separate module (dexidp#1741, @sagikazarmark) - OAuth2 Device Authorization Grant (dexidp#1706, @justin-slowik) - Support username, email and groups claim in OIDC connector (dexidp#1634, @xtremerui) **Bugfixes:** - Add offline_access scope in microsoft connector, if required (dexidp#1441, @jimmythedog) - Allow the google connector to work without a service account (dexidp#1720, @candlerb) **Minor changes:** - Remove vendor (finally) (dexidp#1745, @sagikazarmark) - Fix the LDAP example (dexidp#1762, @heidemn-faro) - Relocate the example app (dexidp#1764, @sagikazarmark)
Fixes #1654