Bug 19760 - Die instead of warn if koha-conf is not accessible
Summary: Die instead of warn if koha-conf is not accessible
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-06 16:58 UTC by Jonathan Druart
Modified: 2018-06-04 20:15 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19760: Die if koha-conf is not accessible (862 bytes, patch)
2017-12-06 16:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19760: Die if koha-conf is not accessible (1.13 KB, patch)
2017-12-06 17:59 UTC, Jon Knight
Details | Diff | Splinter Review
Bug 19760: Change wording (791 bytes, patch)
2017-12-21 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19760: Die if koha-conf is not accessible (1.22 KB, patch)
2017-12-22 11:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19760: Change wording (881 bytes, patch)
2017-12-22 11:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19760: Fix Config.t test (996 bytes, patch)
2017-12-22 17:05 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-12-06 16:58:00 UTC
We got a lot of support questions because scripts are executed without the correct environment/user.
We could die and provide more useful information
Comment 1 Jonathan Druart 2017-12-06 16:58:49 UTC
Created attachment 69568 [details] [review]
Bug 19760: Die if koha-conf is not accessible

We got a lot of support questions because scripts are executed without
the correct environment/user.
We could die and provide more useful information
Comment 2 Jon Knight 2017-12-06 17:59:57 UTC
Created attachment 69571 [details] [review]
Bug 19760: Die if koha-conf is not accessible

We got a lot of support questions because scripts are executed without
the correct environment/user.
We could die and provide more useful information

Tested on my kohadevbox VM by:

1) sudo chmod a-rx /etc/koha/sites/kohadev/koha-conf.xml
2) run help.pl from the command line to produce the desired die message
3) sudo chmod a+rx /etc/koha/sites/kohadev/koha-conf.xml
4) rerun help.pl successfully

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>
Comment 3 Julian Maurice 2017-12-15 15:19:40 UTC
I'm not sure about the message, should we recommend a tool that is only available on Debian package installs ?

Also, there is a strange behaviour (not caused by this patch), when KOHA_CONF is empty or does not contain a valid/existing filename, it is just defaulted to /etc/koha/koha-conf.xml (if it exists).
So, on my dev install, I can run `KOHA_CONF=/not/existing perl help.pl` and it uses /etc/koha/koha-conf.xml without warning
Comment 4 Julian Maurice 2017-12-15 15:21:39 UTC
Changing status to In Discussion to get answers to comment 3
Comment 5 Jonathan Druart 2017-12-18 13:55:31 UTC
(In reply to Julian Maurice from comment #3)
> I'm not sure about the message, should we recommend a tool that is only
> available on Debian package installs ?

It is the common mistake people does when they use kohadevbox, so they will have access to koha-shell.
What do you suggest?

> Also, there is a strange behaviour (not caused by this patch), when
> KOHA_CONF is empty or does not contain a valid/existing filename, it is just
> defaulted to /etc/koha/koha-conf.xml (if it exists).
> So, on my dev install, I can run `KOHA_CONF=/not/existing perl help.pl` and
> it uses /etc/koha/koha-conf.xml without warning

Yes we should add a warning instead.
Comment 6 Julian Maurice 2017-12-18 14:45:30 UTC
(In reply to Jonathan Druart from comment #5)
> It is the common mistake people does when they use kohadevbox, so they will
> have access to koha-shell.
> What do you suggest?
I'm not sure, I just don't like how Koha is being more and more coupled with Debian.
I won't have time to think about another solution for the next two weeks and I don't want to block this bug, so I'm resetting the status to Signed Off. @QAers, feel free to take this one.
Comment 7 Jonathan Druart 2017-12-18 15:01:28 UTC
We could just add something like: If you are using a debian-based system you can use the koha-shell command otherwise you should check your permissions.
Comment 8 Nick Clemens 2017-12-21 13:08:43 UTC
(In reply to Jonathan Druart from comment #7)
> We could just add something like: If you are using a debian-based system you
> can use the koha-shell command otherwise you should check your permissions.

Maybe "Try running this again as the koha instance user (or use the koha-shell command in debian)"
Comment 9 Jonathan Druart 2017-12-21 13:25:55 UTC
Created attachment 70014 [details] [review]
Bug 19760: Change wording
Comment 10 Marcel de Rooy 2017-12-22 11:13:43 UTC
(In reply to Jon Knight from comment #2)
> 3) sudo chmod a+rx /etc/koha/sites/kohadev/koha-conf.xml

chmod 640 might be better ?
Comment 11 Marcel de Rooy 2017-12-22 11:15:26 UTC
Created attachment 70116 [details] [review]
Bug 19760: Die if koha-conf is not accessible

We got a lot of support questions because scripts are executed without
the correct environment/user.
We could die and provide more useful information

Tested on my kohadevbox VM by:

1) sudo chmod a-rx /etc/koha/sites/kohadev/koha-conf.xml
2) run help.pl from the command line to produce the desired die message
3) sudo chmod a+rx /etc/koha/sites/kohadev/koha-conf.xml
4) rerun help.pl successfully

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2017-12-22 11:15:31 UTC
Created attachment 70117 [details] [review]
Bug 19760: Change wording

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Jonathan Druart 2017-12-22 16:17:19 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 14 Jonathan Druart 2017-12-22 17:05:29 UTC
Created attachment 70135 [details] [review]
Bug 19760: Fix Config.t test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2017-12-22 17:06:07 UTC
Last patch pushed to master
Comment 16 Nick Clemens 2018-01-15 16:43:51 UTC
Awesome work all, pushed to stable for 17.11.02
Comment 17 Fridolin SOMERS 2018-01-19 09:10:06 UTC
Pushed to 17.05.x for v17.05.08