Allow passing directories as the config sources #257

lemenkov · 2014-07-04T03:30:51Z

We should allow passing the entire directories as the config sources. In
this case CouchDB will traverse the given directories and load every
*.ini file found.

E.g.

"... -couch_ini /etc/couchdb/default.d/ /etc/couchdb/local.d/ /path/to/extra/couchdb/default.ini /path/to/extra/couchdb/local.ini ..."

Signed-off-by: Peter Lemenkov lemenkov@gmail.com

We should allow passing the entire directories as the config sources. In this case CouchDB will traverse the given directories and load every *.ini file found. E.g. "... -couch_ini /etc/couchdb/default.d/ /etc/couchdb/local.d/ /path/to/extra/couchdb/default.ini /path/to/extra/couchdb/local.ini ..." Signed-off-by: Peter Lemenkov <lemenkov@gmail.com>

kxepal · 2014-07-04T04:05:03Z

LGFM. +1. It was fun to consume php configs on start (:
The only notice is indention: we prefer 4 spaces over tabs - this makes your changes looks a bit alien in module.

kxepal · 2014-07-04T04:12:16Z

Ah, I was a bit in rush: if directory is specified with trailing slash it will not be handled correctly: the couchdb -A /etc/couchdb/local.d/ call will look for /etc/couchdb/local.d//*.ini files and obliviously will find nothing. And since there would be no error this could make someone life a bit harder.

Better replace: filelib:wildcard(V ++ "/*.ini")
with: filelib:wildcard(filename:join([V,"*.ini"]))

lemenkov · 2014-07-04T05:00:11Z

@kxepal quite the contrary - it wil be handled correctly in any way. However I agree - that will be a source of confusion for further CouchDB code divers.

Signed-off-by: Peter Lemenkov <lemenkov@gmail.com>

rnewson · 2014-07-04T09:07:27Z

I'm confused. couchdb already loads all .ini files in the default.d and local.d directories. What is this adding?

rnewson · 2014-07-04T09:07:50Z

src/couchdb/couch_app.erl

+        lists:foldl(
+		fun (V, AccIn) ->
+			  case file:read_file_info(V) of
+				  {ok, #file_info{type = regular}} -> AccIn ++ [V];


should be [V | AccIn]

(reverse at the end if the order mattered)

Order matters since otherwise we'll write all the changes to the first config in chain and it is default.ini.

sure, just saying it's conventional to prepend then lists:reverse at the end.

Alternatively, use lists:foldr/3 and just do [V | AccIn] without the reverse.

rnewson · 2014-07-04T09:08:34Z

finally, this area of code changes quite a bit after the 1843-feature-bigcouch merge.

rnewson · 2014-07-04T11:02:09Z

moved to: https://git-wip-us.apache.org/repos/asf?p=couchdb-config.git;a=blob;f=src/config_app.erl;h=5c5515af36e92b243a6f4c9d76890f3abaa280fa;hb=master

rnewson · 2014-07-06T21:48:09Z

src/couchdb/couch_app.erl

+		fun (V, AccIn) ->
+			  case file:read_file_info(V) of
+				  {ok, #file_info{type = regular}} -> AccIn ++ [V];
+				  {ok, #file_info{type = directory}} -> AccIn ++ filelib:wildcard(filename:join([V,"*.ini"]));


As noted in IRC, the ordering of filelib:wildcard is not specified, so this is unpredictable.

To fix this, let's have [V | AccIn] for the earlier clause, preserve this clause, but add a final lists:sort(Values) for the function's result.

+1 to adding a sort.

rnewson · 2014-07-06T23:13:20Z

my thoughts captured here: https://github.com/rnewson/couchdb/compare/erlang-expand?expand=1

wtogami · 2014-07-07T04:12:33Z

@rnewson
Somehow both your and Peter's patch result in default.ini being opened for write.

diff --git a/src/couchdb/couch_app.erl b/src/couchdb/couch_app.erl
index d6d8c0c..9e5d7b5 100644
--- a/src/couchdb/couch_app.erl
+++ b/src/couchdb/couch_app.erl
@@ -38,7 +38,8 @@ get_ini_files(Default) ->
     {ok, [[]]} ->
         Default;
     {ok, [Values]} ->
-        lists:flatmap(fun(V) ->
+        io:fwrite("INIT CHAIN: ~p~n", [Values]),
+        R = lists:flatmap(fun(V) ->
             case file:read_file_info(V) of
        {ok, #file_info{type = regular}} ->
            [V];
@@ -47,7 +48,9 @@ get_ini_files(Default) ->
        {error, enoent} ->
            []
        end
-   end, Values)
+   end, Values),
+   io:fwrite("RESULT CHAIN: ~p~n", [R]),
+   R
     end.

 start_apps([]) ->

The above patch applied on top of your patch confirms that default.ini is first.

RESULT CHAIN: ["/etc/couchdb/default.ini","/etc/couchdb/default.d/1.ini",
"/etc/couchdb/default.d/2.ini","/etc/couchdb/default.d/3.ini",
"/etc/couchdb/local.d/1.ini","/etc/couchdb/local.d/2.ini",
"/etc/couchdb/local.d/3.ini","/etc/couchdb/local.ini"]

Yet default.ini is somehow the file that is written to.

type=AVC msg=audit(1404705722.154:5821): avc:  denied  { write } for  pid=23681 comm="beam.smp" name="default.ini" dev="dm-0" ino=677154 scontext=system_u:system_r:couchdb_t:s0 tcontext=system_u:object_r:couchdb_conf_t:s0 tclass=file

wtogami · 2014-07-07T05:43:53Z

I was partially wrong ...

type=AVC msg=audit(1404711494.997:6093): avc:  denied  { write } for  pid=19794 comm="beam.smp" name="default.ini" dev="dm-0" ino=672839 scontext=system_u:system_r:couchdb_t:s0 tcontext=system_u:object_r:couchdb_conf_t:s0 tclass=file

couchdb does try to write to the first ini during startup.

type=AVC msg=audit(1404711578.067:6096): avc:  denied  { write } for  pid=19793 comm="beam.smp" name="local.ini" dev="dm-0" ino=677149 scontext=system_u:system_r:couchdb_t:s0 tcontext=system_u:object_r:couchdb_conf_t:s0 tclass=file

When you try to save a configuration via REST it does write to the last ini, so @rnewson's patch actually does work.

Any idea where/why it opens the first ini file for writing during startup?

davisp · 2015-03-23T23:54:52Z

Also, this should be ported to the couchdb-config app since this won't merge to master. Should be a trivial move though.

lemenkov · 2016-10-19T15:00:29Z

This PR was reworked, and applied upstream as apache/couchdb-config#9. Time to close this one.

Thanks everyone!

Use filename:join/1 to avoid confusion

8cdeaee

Signed-off-by: Peter Lemenkov <lemenkov@gmail.com>

rnewson reviewed Jul 4, 2014
View reviewed changes

rnewson reviewed Jul 6, 2014
View reviewed changes

lemenkov closed this Oct 19, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow passing directories as the config sources #257

Allow passing directories as the config sources #257

lemenkov commented Jul 4, 2014

kxepal commented Jul 4, 2014

kxepal commented Jul 4, 2014

lemenkov commented Jul 4, 2014

rnewson commented Jul 4, 2014

rnewson Jul 4, 2014

rnewson Jul 4, 2014

kxepal Jul 6, 2014

rnewson Jul 6, 2014

davisp Mar 23, 2015

rnewson commented Jul 4, 2014

rnewson commented Jul 4, 2014

rnewson Jul 6, 2014

davisp Mar 23, 2015

rnewson commented Jul 6, 2014

wtogami commented Jul 7, 2014

wtogami commented Jul 7, 2014

davisp commented Mar 23, 2015

lemenkov commented Oct 19, 2016

Allow passing directories as the config sources #257

Allow passing directories as the config sources #257

Conversation

lemenkov commented Jul 4, 2014

kxepal commented Jul 4, 2014

kxepal commented Jul 4, 2014

lemenkov commented Jul 4, 2014

rnewson commented Jul 4, 2014

rnewson Jul 4, 2014

Choose a reason for hiding this comment

rnewson Jul 4, 2014

Choose a reason for hiding this comment

kxepal Jul 6, 2014

Choose a reason for hiding this comment

rnewson Jul 6, 2014

Choose a reason for hiding this comment

davisp Mar 23, 2015

Choose a reason for hiding this comment

rnewson commented Jul 4, 2014

rnewson commented Jul 4, 2014

rnewson Jul 6, 2014

Choose a reason for hiding this comment

davisp Mar 23, 2015

Choose a reason for hiding this comment

rnewson commented Jul 6, 2014

wtogami commented Jul 7, 2014

wtogami commented Jul 7, 2014

davisp commented Mar 23, 2015

lemenkov commented Oct 19, 2016