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

[doc/example] Make LDAP example functional again by running OpenLDAP with docker-compose #1762

Merged
merged 4 commits into from
Jul 15, 2020

Conversation

heidemn-faro
Copy link
Contributor

@heidemn-faro heidemn-faro commented Jul 13, 2020

Fixes #1654

@heidemn-faro heidemn-faro changed the title Add missing slapd.sh script from LDAP docs, and convert it to using Docker Add missing slapd.sh script from LDAP guide, and convert it to run OpenLDAP in Docker Jul 13, 2020
…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
Copy link
Contributor Author

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).

Copy link
Member

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.

@heidemn-faro heidemn-faro changed the title Add missing slapd.sh script from LDAP guide, and convert it to run OpenLDAP in Docker [doc/example] Add missing slapd.sh script from LDAP guide, and convert it to run OpenLDAP in Docker Jul 13, 2020
@sagikazarmark
Copy link
Member

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.

@heidemn-faro heidemn-faro force-pushed the doc/ldap-example branch 3 times, most recently from d52d592 to 521954a Compare July 15, 2020 09:49
Signed-off-by: Martin Heide <martin.heide@faro.com>
Signed-off-by: Martin Heide <martin.heide@faro.com>
@heidemn-faro heidemn-faro changed the title [doc/example] Add missing slapd.sh script from LDAP guide, and convert it to run OpenLDAP in Docker [doc/example] Make LDAP example functional again by running OpenLDAP with docker-compose Jul 15, 2020
@heidemn-faro
Copy link
Contributor Author

@sagikazarmark as requested, I've reworked this to use docker-compose.
Any further changes required?

Copy link
Member

@sagikazarmark sagikazarmark left a 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 !

Copy link
Member

@bonifaido bonifaido left a comment

Choose a reason for hiding this comment

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

👍

@heidemn-faro
Copy link
Contributor Author

@sagikazarmark @bonifaido thanks for reviewing.
The PR is ready from my side. I've verified that the example works.

I don't have permissions to merge, so you would have to do that.

@sagikazarmark sagikazarmark merged commit cb46a28 into dexidp:master Jul 15, 2020
xtremerui pushed a commit to concourse/dex that referenced this pull request Oct 5, 2020
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)
xtremerui pushed a commit to concourse/dex that referenced this pull request Nov 5, 2020
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)
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.

missing ./scripts/slapd.sh
3 participants