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

Freenas/11.1 stable ticket 40720 #1643

Merged
merged 12 commits into from Aug 6, 2018

Conversation

jhixson74
Copy link

John Hixson added 5 commits August 3, 2018 15:25
Ticket: #23392
(cherry picked from commit 51b4222)

(11.1-stable)
Ticket: #40720
(cherry picked from commit b65131b)

(11.1-stable)
Ticket: #40720
(cherry picked from commit de60a61)

(11.1-stable)
Ticket: #40720
(cherry picked from commit efa2e99)

(11.1-stable)
Ticket: #40692
Ticket: #23392
(cherry picked from commit 97fa419)

(11.1-stable)
Ticket: #40720
@ghost ghost assigned jhixson74 Aug 3, 2018
@ghost ghost added the review label Aug 3, 2018
if nslcd_running
then
nslcd_stop
ldapctl_cmd ${service} ix-nslcd start
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find ix-nslcd in the master, does it exist or it should be nslcd here?

Copy link
Author

Choose a reason for hiding this comment

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

Good question. I don't know! I'll look at the patch and see what I meant to do here.

John Hixson added 5 commits August 4, 2018 10:19
(cherry picked from commit 336062b)
Ticket: #37138
(cherry picked from commit 53a9743)
(cherry picked from commit ad78bca)
if ldap['ldap_certificate']:
cert = safe_call('certificateauthority.query', [('id', '=', ldap['ldap_certificate']['id'])], {'get': True})
if cert:
capath = cert['cert_certificate_path']
Copy link
Contributor

@b-a-t b-a-t Aug 4, 2018

Choose a reason for hiding this comment

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

I have no idea what I'm talking about but in the next file there is an update:

-                    capath = cert['cert_certificate_path']
+                    capath = cert['certificate_path']

So one of them must be wrong, I guess.

Copy link
Author

Choose a reason for hiding this comment

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

So apparently cert_certificate_path is the correct key. This was changed at some point, which is why you see a bunch of cert_certificate_path to certificate_path changes. Now that I see that cert_certificate_path is the correct key, I need to revisit all of these PR's where that key change is made. Good find.


ldap_enabled = safe_call('notifier.common', 'system', 'ldap_enabled')

ldap_uri = "%s://%s" % ("ldaps" if ldap['ldap_ssl'] == "on" else "ldap", ldap['ldap_hostname'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Potentially, ldap can be set to None here, so querying ldap['ldap_hostname'] will give us an exception here?

@@ -13,7 +13,7 @@
if ldap['ldap_certificate']:
cert = safe_call('certificateauthority.query', [('id', '=', ldap['ldap_certificate']['id'])], {'get': True})
if cert:
capath = cert['cert_certificate_path']
Copy link
Contributor

Choose a reason for hiding this comment

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

This change vs. file above.

Copy link
Contributor

@b-a-t b-a-t left a comment

Choose a reason for hiding this comment

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

LGTM

@jhixson74 jhixson74 merged commit 1db7cc7 into freenas/11.1-stable Aug 6, 2018
@ghost ghost removed the review label Aug 6, 2018
@jhixson74 jhixson74 deleted the freenas/11.1-stable-ticket-40720 branch August 6, 2018 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants