Skip to content

Conversation

eduardosan
Copy link
Contributor

Hi,

I have a very specific use case where data volumes are mounted from a sshfs partition and other network partitions, and in this case one can't change permissions on remote disks.

I just added an environment var to disable this when it's necessary.

@BertrandGouny
Copy link
Member

Hello,
thanks for the pull request.

Can you please adapt it to the existing formatting :

  • use 2 spaces as indentation
  • close if statements with fi

Will merge that in the 1.2.3 coming release.
Thanks

@kumy
Copy link

kumy commented Mar 21, 2019

@eduardosan Could you please fix the formatting, so we could all benefit from your fix :)

@eduardosan
Copy link
Contributor Author

Hello,

The fix is provided in above commit.

Copy link

@kumy kumy left a comment

Choose a reason for hiding this comment

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

@eduardosan still some fix needed, no?

@@ -125,7 +127,8 @@ EOF
mv /tmp/schema/cn=config/cn=schema/* /etc/ldap/slapd.d/cn=config/cn=schema
rm -r /tmp/schema

chown -R openldap:openldap /etc/ldap/slapd.d/cn=config/cn=schema
if [ -z "$DISABLE_CHOWN" ]; then
chown -R openldap:openldap /etc/ldap/slapd.d/cn=config/cn=schema
Copy link

Choose a reason for hiding this comment

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

missing fi?

chown -R openldap:openldap ${CONTAINER_SERVICE_DIR}/slapd
if [ -z "$DISABLE_CHOWN" ]; then
chmod 600 ${LDAP_TLS_DH_PARAM_PATH}
chown -R openldap:openldap ${CONTAINER_SERVICE_DIR}/slapd
Copy link

Choose a reason for hiding this comment

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

missing fi?

chown openldap:openldap $PREVIOUS_LDAP_TLS_CRT_PATH $PREVIOUS_LDAP_TLS_KEY_PATH $PREVIOUS_LDAP_TLS_CA_CRT_PATH $PREVIOUS_LDAP_TLS_DH_PARAM_PATH
if [ -z "$DISABLE_CHOWN" ]; then
chmod 600 ${PREVIOUS_LDAP_TLS_DH_PARAM_PATH}
chown openldap:openldap $PREVIOUS_LDAP_TLS_CRT_PATH $PREVIOUS_LDAP_TLS_KEY_PATH $PREVIOUS_LDAP_TLS_CA_CRT_PATH $PREVIOUS_LDAP_TLS_DH_PARAM_PATH
Copy link

Choose a reason for hiding this comment

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

indent + fi

@eduardosan
Copy link
Contributor Author

Now I thing they are all included

@kumy
Copy link

kumy commented Mar 21, 2019

@eduardosan Thanks 👍

@BertrandGouny Do you think it can be merged? Thanks.

BertrandGouny added a commit that referenced this pull request Jul 7, 2019
@BertrandGouny
Copy link
Member

Thanks all,
merge in next release

@BertrandGouny BertrandGouny merged commit 9aec10a into osixia:stable Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants