Discussion:
[Freeipa-devel] [freeipa PR#655][opened] httpinstance.disable_system_trust: Don't fail if module 'Root Certs' …
dkupka
2017-03-27 08:06:25 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Author: dkupka
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 

Action: opened

PR body:
"""

is not available

Server installation failed when attmpting to disable module 'Root Certs' and
the module was not available in HTTP_ALIAS_DIR. When the module is not
available there's no need to disable it and the error may be treated as
success.

https://pagure.io/freeipa/issue/6803
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/655/head:pr655
git checkout pr655
stlaz
2017-03-27 10:22:26 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


stlaz commented:
"""
Hm, I believe the `-list` operation was there just to check whether the module is there. If `modutil` fails like this no matter the situation, it'd be better to just try to disable it whatever happens and go away with "meh, so what" if the disable fails.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289413533
stlaz
2017-03-27 10:22:59 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


stlaz commented:
"""
Hm, I believe the `-list` operation was there just to check whether the module is there. If `modutil` fails like this no matter the situation, it'd be better to just try to disable it whatever happens and go away with "meh, so what" if the disable fails.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289413533
stlaz
2017-03-27 10:24:20 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


stlaz commented:
"""
For the record:
```bash
[***@vm-066 ~]$ sudo modutil -dbdir nssdb/ -disable 'Root Certs' -force
ERROR: Module "Root Certs" not found in database.
[***@machine ~]$ echo $?
29
[***@machine ~]$
```
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289413925
stlaz
2017-03-27 10:24:31 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


stlaz commented:
"""
For the record:
```bash
[***@vm-066 ~]$ sudo modutil -dbdir nssdb/ -disable 'Root Certs' -force
ERROR: Module "Root Certs" not found in database.
[***@machine ~]$ echo $?
29
[***@machine ~]$
```
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289413925
tiran
2017-03-27 10:32:15 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


tiran commented:
"""
@stlaz The broad except also catches and ignores typos in the command line or missing ```modutil``` binary.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289415627
stlaz
2017-03-27 10:39:44 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


stlaz commented:
"""
@tiran I of course agree on narrowing the broad except down, my point is we should rather remove the whole `-list` part and just try to do `-disable`.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289416128
stlaz
2017-03-27 10:34:40 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


stlaz commented:
"""
@tiran I of course agree on narrowing the broad except down, my point is we should rather remove the whole `-list` part and just try to do `-disable`.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289416128
dkupka
2017-03-27 10:53:52 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Author: dkupka
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 

Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/655/head:pr655
git checkout pr655
dkupka
2017-03-27 11:08:41 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


dkupka commented:
"""
@tiran @stlaz Makes sense. I will update it accordingly. Thanks for suggestions.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289422724
HonzaCholasta
2017-03-27 11:18:03 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


HonzaCholasta commented:
"""
@stlaz, you can't do just `-disable`, as that would break upgrade (note that the `-list` is there because `-disable` always reports success).
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289424502
stlaz
2017-03-27 11:38:10 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


stlaz commented:
"""
@HonzaCholasta You're right, I completely forgot about that one.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289428384
dkupka
2017-03-27 11:52:54 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Author: dkupka
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 

Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/655/head:pr655
git checkout pr655
stlaz
2017-03-28 10:33:05 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


stlaz commented:
"""
This fixes the mentioned issue. I did not test whether the actual disable works but I should hope so as I don't see how this could break it.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289729611
stlaz
2017-03-28 10:33:10 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


Label: +ack
tomaskrizek
2017-03-28 15:11:40 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Author: dkupka
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 

Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/655/head:pr655
git checkout pr655
tomaskrizek
2017-03-28 15:11:33 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


Label: +pushed
tomaskrizek
2017-03-28 15:11:36 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/655
Title: #655: httpinstance.disable_system_trust: Don't fail if module 'Root Certs' 


tomaskrizek commented:
"""
ipa-4-5:

* 2a499551ca5ddf2596cc19a77f47c34e9f5c10c5 httpinstance.disable_system_trust: Don't fail if module 'Root Certs' is not available
master:

* 0128e805e591bc8ca5cea99739ad4cd7478df0b4 httpinstance.disable_system_trust: Don't fail if module 'Root Certs' is not available
"""

See the full comment at https://github.com/freeipa/freeipa/pull/655#issuecomment-289802039
Loading...