You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/10/25 15:20:33 UTC
[GitHub] [couchdb] big-r81 opened a new pull request, #4240: Couch password server
big-r81 opened a new pull request, #4240:
URL: https://github.com/apache/couchdb/pull/4240
First draft PR to create a `gen_server` process, which hashes the admin passwords from the configuration file.
It gets started in `couch_sup`.
PR needs some more polish.
In the current state, this PR already fixes #4236.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005200415
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
I wonder if another problem here is that we'll be rehashing the password N times, once for each couch_server? For instance on a 96 cpu server, we'll rehash the password 96 times in parallel, it seems.
An ideal way to handle it might be to just remove the password rehashing from here to the new password hasher gen_server, which we have one of. So it would have the `handle_config_change` callback and so on. A quicker hack for now could be to only handle admin rehashing for `N=1`. Also do the same for httpd changes, we don't need to stop/restart the httpd backend N times.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004657982
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,86 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ terminate/2,
+ code_change/3
+]).
+
+-export([sync_hash/0, async_hash/0]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+sync_hash() ->
+ gen_server:call(?MODULE, hash_passwords).
+async_hash() ->
+ gen_server:cast(?MODULE, hash_passwords).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call(hash_passwords, _From, _State) ->
+ {reply, hash_admin_passwords(), _State};
+handle_call(_Request, _From, State = #state{}) ->
+ {reply, ok, State}.
Review Comment:
So we would have a `{stop, invalid_call, State}`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005903736
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
While at it, let's do the same 1 = N for the all the httpd:stop() clauses below as well. Those have the same bug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004650173
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,86 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ terminate/2,
+ code_change/3
+]).
+
+-export([sync_hash/0, async_hash/0]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+sync_hash() ->
+ gen_server:call(?MODULE, hash_passwords).
+async_hash() ->
+ gen_server:cast(?MODULE, hash_passwords).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call(hash_passwords, _From, _State) ->
+ {reply, hash_admin_passwords(), _State};
+handle_call(_Request, _From, State = #state{}) ->
+ {reply, ok, State}.
+
+handle_cast(hash_passwords, _State) ->
+ hash_admin_passwords(),
+ {noreply, _State};
+handle_cast(_Request, State) ->
+ {noreply, State}.
+
+handle_info(_Info, State = #state{}) ->
+ {noreply, State}.
+
+terminate(_Reason, _State = #state{}) ->
+ ok.
Review Comment:
I think these two callbacks might be optional these days. See if you remove them and everything still work as expected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004651698
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,86 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ terminate/2,
+ code_change/3
+]).
+
+-export([sync_hash/0, async_hash/0]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+sync_hash() ->
+ gen_server:call(?MODULE, hash_passwords).
+async_hash() ->
Review Comment:
Tiny nit: leave an empty line between functions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005836320
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,80 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean()) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ couch_log:info("Password Hasher Process is starting.~n", []),
Review Comment:
Let's skip the log line. If it doesn't start we'll get an error in the logs and otherwise we'll assume it started.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004649559
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,86 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ terminate/2,
+ code_change/3
+]).
+
+-export([sync_hash/0, async_hash/0]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+sync_hash() ->
+ gen_server:call(?MODULE, hash_passwords).
+async_hash() ->
+ gen_server:cast(?MODULE, hash_passwords).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call(hash_passwords, _From, _State) ->
+ {reply, hash_admin_passwords(), _State};
+handle_call(_Request, _From, State = #state{}) ->
+ {reply, ok, State}.
+
+handle_cast(hash_passwords, _State) ->
+ hash_admin_passwords(),
+ {noreply, _State};
+handle_cast(_Request, State) ->
+ {noreply, State}.
Review Comment:
Let's stop and crash as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 merged pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 merged PR #4240:
URL: https://github.com/apache/couchdb/pull/4240
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004649559
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,86 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ terminate/2,
+ code_change/3
+]).
+
+-export([sync_hash/0, async_hash/0]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+sync_hash() ->
+ gen_server:call(?MODULE, hash_passwords).
+async_hash() ->
+ gen_server:cast(?MODULE, hash_passwords).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call(hash_passwords, _From, _State) ->
+ {reply, hash_admin_passwords(), _State};
+handle_call(_Request, _From, State = #state{}) ->
+ {reply, ok, State}.
+
+handle_cast(hash_passwords, _State) ->
+ hash_admin_passwords(),
+ {noreply, _State};
+handle_cast(_Request, State) ->
+ {noreply, State}.
Review Comment:
Let's stop and crash as well by returning `{stop, {invalid_cast, Msg}, State}.`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005316605
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,79 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, _State) ->
+ {reply, hash_admin_passwords(Persist), _State};
+handle_call(Msg, _From, State = #state{}) ->
+ {stop, {invalid_call, Request}, {invalid_call, Request}, State}.
+
+handle_cast({hash_passwords, Persist}, _State) ->
+ hash_admin_passwords(Persist),
+ {noreply, _State};
+handle_cast(Msg, State) ->
+ {stop, {invalid_call, Msg}, State}.
Review Comment:
Ah yes, good find
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1006101336
##########
src/couch/src/couch_password_hasher.erl:
##########
@@ -0,0 +1,75 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_hasher).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/1]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(Persist :: boolean()) -> Reply :: term().
+hash(Persist) ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist}).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ hash_admin_passwords(true),
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, State) ->
Review Comment:
I think we don't need this call anymore?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1006105957
##########
src/couch/src/couch_password_hasher.erl:
##########
@@ -0,0 +1,75 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_hasher).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/1]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(Persist :: boolean()) -> Reply :: term().
+hash(Persist) ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist}).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ hash_admin_passwords(true),
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, State) ->
Review Comment:
yeah, good idea!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005835215
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
`N` is the couch_server process number `1 ... $(number_of_schedulers)`. So in handle_config_change ... we'd replace `N` with `1`. So the first couch_server would do the hashing or stopping of httpd and the rest would ignore it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004649092
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,86 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ terminate/2,
+ code_change/3
+]).
+
+-export([sync_hash/0, async_hash/0]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+sync_hash() ->
+ gen_server:call(?MODULE, hash_passwords).
+async_hash() ->
+ gen_server:cast(?MODULE, hash_passwords).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call(hash_passwords, _From, _State) ->
+ {reply, hash_admin_passwords(), _State};
+handle_call(_Request, _From, State = #state{}) ->
+ {reply, ok, State}.
Review Comment:
Let's stop and crash with an invalid_call message here. This service shouldn't be getting unexpected calls.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005187681
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,80 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean()) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ couch_log:info("Password Hasher Process is starting.~n", []),
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, _State) ->
+ {reply, hash_admin_passwords(Persist), _State};
+handle_call(Msg, _From, State = #state{}) ->
+ {stop, {invalid_call, Msg}, {invalid_call, Msg}, State}.
+
+handle_cast({hash_passwords, Persist}, _State) ->
+ hash_admin_passwords(Persist),
+ {noreply, _State};
+handle_cast(Msg, State) ->
+ {stop, {invalid_cast, Msg}, State}.
+
+code_change(_OldVsn, State = #state{}, _Extra) ->
Review Comment:
as above, `#state{} = State` is a bit more common
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005902807
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
For others it can just be a fall-through and nothing will happen:
This case basically:
```erlang
handle_config_change(_, _, _, _, N) ->
{ok, N}.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005896492
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
So, I pattern-matched the call for the server with Number 1, do we need to handle the "admin" for other servers too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004707314
##########
src/couch/src/couch_sup.erl:
##########
@@ -168,3 +169,8 @@ write_file(FileName, Contents) ->
couch_log:error("Failed ot write ~s :: ~s", Args),
throw({error, Reason})
end.
+
+start_password_server() ->
+ couch_log:info("Password Server Process is starting.~n", []),
+ couch_password_server:start_link(),
+ ok.
Review Comment:
So, put it into Children of `couch_primary_services`, or where to add?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005329182
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
Is N the number of Nodes in the cluster or the Number of processors?
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
Is N the number of Nodes in the cluster or the number of processors?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005866271
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,80 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean()) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ couch_log:info("Password Hasher Process is starting.~n", []),
Review Comment:
Ok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005841191
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
To debug this try to log N and the rest of the state and then in remsh do something like `config:set("admins", "adm5", "pass5").`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005311757
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,80 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean()) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ couch_log:info("Password Hasher Process is starting.~n", []),
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, _State) ->
+ {reply, hash_admin_passwords(Persist), _State};
+handle_call(Msg, _From, State = #state{}) ->
Review Comment:
Same as above, will change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005187189
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,80 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean()) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ couch_log:info("Password Hasher Process is starting.~n", []),
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, _State) ->
+ {reply, hash_admin_passwords(Persist), _State};
+handle_call(Msg, _From, State = #state{}) ->
Review Comment:
`#state{} = State` is a bit more common I think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004661466
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,86 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ terminate/2,
+ code_change/3
+]).
+
+-export([sync_hash/0, async_hash/0]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+sync_hash() ->
+ gen_server:call(?MODULE, hash_passwords).
+async_hash() ->
+ gen_server:cast(?MODULE, hash_passwords).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call(hash_passwords, _From, _State) ->
+ {reply, hash_admin_passwords(), _State};
+handle_call(_Request, _From, State = #state{}) ->
+ {reply, ok, State}.
Review Comment:
Maybe `{stop, {invalid_call, Msg}, {invalid_call, Msg}, State}.` just so the caller also gets a reply about it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005316630
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,80 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean()) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ couch_log:info("Password Hasher Process is starting.~n", []),
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, _State) ->
Review Comment:
The idea is that we're not making a var equal to State, but asserting that it matches an existing value, the reverse order sort of highlights that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005339768
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
Or can we use the `State` record to call to number of unhashed passwords and only start the hashing if it's greater than zero?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004661466
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,86 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ terminate/2,
+ code_change/3
+]).
+
+-export([sync_hash/0, async_hash/0]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+sync_hash() ->
+ gen_server:call(?MODULE, hash_passwords).
+async_hash() ->
+ gen_server:cast(?MODULE, hash_passwords).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call(hash_passwords, _From, _State) ->
+ {reply, hash_admin_passwords(), _State};
+handle_call(_Request, _From, State = #state{}) ->
+ {reply, ok, State}.
Review Comment:
Maybe `{stop, {invalid_call, Request}, {invalid_call, Request}, State}.` just so the caller also gets a reply about it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005954709
##########
src/couch/src/couch_server.erl:
##########
@@ -381,7 +369,7 @@ handle_config_change("couchdb_engines", _, _, _, N) ->
{ok, N};
handle_config_change("admins", _, _, Persist, N) ->
% spawn here so couch event manager doesn't deadlock
- spawn(fun() -> hash_admin_passwords(Persist) end),
+ couch_password_server:hash(true, Persist),
Review Comment:
Ok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004707084
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,79 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, _State) ->
+ {reply, hash_admin_passwords(Persist), _State};
+handle_call(Msg, _From, State = #state{}) ->
+ {stop, {invalid_call, Request}, {invalid_call, Request}, State}.
+
+handle_cast({hash_passwords, Persist}, _State) ->
+ hash_admin_passwords(Persist),
+ {noreply, _State};
+handle_cast(Msg, State) ->
+ {stop, {invalid_call, Msg}, State}.
Review Comment:
`invalid_cast` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005186848
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,80 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean()) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ couch_log:info("Password Hasher Process is starting.~n", []),
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, _State) ->
+ {reply, hash_admin_passwords(Persist), _State};
+handle_call(Msg, _From, State = #state{}) ->
+ {stop, {invalid_call, Msg}, {invalid_call, Msg}, State}.
+
+handle_cast({hash_passwords, Persist}, _State) ->
Review Comment:
`_State` is used, it should not have an underscore. As above, let's use `#state{} = State`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005186481
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,80 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean()) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ couch_log:info("Password Hasher Process is starting.~n", []),
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, _State) ->
Review Comment:
If `_State` is used it should be `State`. Let's match it like below as well just in case `#state{} =State`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004661466
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,86 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ terminate/2,
+ code_change/3
+]).
+
+-export([sync_hash/0, async_hash/0]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+sync_hash() ->
+ gen_server:call(?MODULE, hash_passwords).
+async_hash() ->
+ gen_server:cast(?MODULE, hash_passwords).
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call(hash_passwords, _From, _State) ->
+ {reply, hash_admin_passwords(), _State};
+handle_call(_Request, _From, State = #state{}) ->
+ {reply, ok, State}.
Review Comment:
Maybe `{stop, {invalid_call, Msg}, {invalid_call, Msg}, State}.` just so the caller also gets a reply about it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1004682996
##########
src/couch/src/couch_sup.erl:
##########
@@ -168,3 +169,8 @@ write_file(FileName, Contents) ->
couch_log:error("Failed ot write ~s :: ~s", Args),
throw({error, Reason})
end.
+
+start_password_server() ->
+ couch_log:info("Password Server Process is starting.~n", []),
+ couch_password_server:start_link(),
+ ok.
Review Comment:
If we intend this to be properly supervised we should add it to the supervisor as a child. `couch_primary_services` might be a better place for it, right before the couch_server instances are added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] big-r81 commented on a diff in pull request #4240: Couch password server process
Posted by GitBox <gi...@apache.org>.
big-r81 commented on code in PR #4240:
URL: https://github.com/apache/couchdb/pull/4240#discussion_r1005310159
##########
src/couch/src/couch_password_server.erl:
##########
@@ -0,0 +1,80 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_password_server).
+
+-behaviour(gen_server).
+
+-include_lib("couch/include/couch_db.hrl").
+
+-export([start_link/0]).
+-export([
+ init/1,
+ handle_call/3,
+ handle_cast/2,
+ code_change/3
+]).
+
+-export([hash/2]).
+
+-record(state, {}).
+
+%%%===================================================================
+%%% Public functions
+%%%===================================================================
+
+-spec hash(IsAsyncCall :: boolean(), Persist :: boolean()) -> Reply :: term().
+hash(IsAsyncCall, Persist) ->
+ case IsAsyncCall of
+ true ->
+ gen_server:cast(?MODULE, {hash_passwords, Persist});
+ false ->
+ gen_server:call(?MODULE, {hash_passwords, Persist})
+ end.
+
+%%%===================================================================
+%%% Spawning and gen_server implementation
+%%%===================================================================
+
+start_link() ->
+ couch_log:info("Password Hasher Process is starting.~n", []),
+ gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
+
+init(_Args) ->
+ {ok, #state{}}.
+
+handle_call({hash_passwords, Persist}, _From, _State) ->
Review Comment:
> If `_State` is used it should be `State`.
Yes, that makes sense.
> Let's match it like below as well just in case `#state{} = State`
Btw, I personally find this spelling unnatural, maybe i'm too much "left-to-right" indoctrinated ;-). But it seems common practice and I will change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org