With this patch all the backend providers can share the same offline status. This means composite backends like what AD or IPA will be that use a mix of ldap and krb can share the offline status for both protocols.
A further extension might allow to have per protocol "online/offline" status, that will be done eventually when we integrate also the DNS discovery options.
Tested with ldap_id+ldap_auth and ldap_id+krb5_auth
Simo.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 09/13/2009 10:31 AM, Simo Sorce wrote:
With this patch all the backend providers can share the same offline status. This means composite backends like what AD or IPA will be that use a mix of ldap and krb can share the offline status for both protocols.
A further extension might allow to have per protocol "online/offline" status, that will be done eventually when we integrate also the DNS discovery options.
Tested with ldap_id+ldap_auth and ldap_id+krb5_auth
Simo.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Just a nitpick, but why did you replace sbus_conn_send_reply() in be_check_online with sbus_get_connection and dbus_connection_send()? They are functionally identical. (except that sbus_conn_send_reply() can get the connection in one fewer stack frame, since it can access the sbus_connection object directly)
Assuming I'm reading this correctly, we're talking about considering a single backend process as being online or offline as a whole. Why is this dependent only on the ID provider for the backend? Shouldn't we consider that if the authentication module or password change modules are offline that we are offline?
Furthermore, even if the ID provider is offline, if we have cached user information that allows us to initiate a connection to a still-live authentication provider, isn't that perfectly reasonable?
I'd argue that if any ONE of the modules was the ultimate determinator of online/offline status, it should be authentication rather than identification.
Code itself is sensible, so this is a Nack until you can convince me that the approach itself is right.
- -- Stephen Gallagher RHCE 804006346421761
Looking to carve out IT costs? www.redhat.com/carveoutcosts/
On Mon, 2009-09-14 at 07:48 -0400, Stephen Gallagher wrote:
Just a nitpick, but why did you replace sbus_conn_send_reply() in be_check_online with sbus_get_connection and dbus_connection_send()? They are functionally identical. (except that sbus_conn_send_reply() can get the connection in one fewer stack frame, since it can access the sbus_connection object directly)
I merged together what previously was an async request. It was only asking the id provider about offline status. Since now the offline status is directly accessible by the dp_provider_be.c there is no need to make a request to any provider, we directly return the answer to dp. (this call is not used by anything anyway so far but we were planning to use it to force a backend to go offline so it will come handy later on).
Assuming I'm reading this correctly, we're talking about considering a single backend process as being online or offline as a whole. Why is this dependent only on the ID provider for the backend?
It isn't, attached new patch that add be_mark_offline() calls also to the auth backends.
Shouldn't we consider that if the authentication module or password change modules are offline that we are offline?
yes, should be fixed in the new patch, now.
Furthermore, even if the ID provider is offline, if we have cached user information that allows us to initiate a connection to a still-live authentication provider, isn't that perfectly reasonable?
No, also because this is just an initial patch to start building infrastructure. If you remember we discussed the idea or allowing the monitor to put a backend forcibly offline, in that case all providers must respect this. Further more during auth we want to always refresh users, so if the id part is not available we can as well auth from the cached password (if caching passwords is allowed).
This infrastructure is also need for DNS discovery later, where we want a central place to tell a specific server is unreachable.
In any case I would rather put the infrastructure in place now, and tweak specific behaviors later.
I'd argue that if any ONE of the modules was the ultimate determinator of online/offline status, it should be authentication rather than identification.
Nope the auth module contacts the servers more rarely than the id backend (auths are rare compared to requests to get id information), so normally the ID backend is more qualified. In any case I am not going to make any provider king, all of them should be able to signal that servers are unreachable.
Code itself is sensible, so this is a Nack until you can convince me that the approach itself is right.
Let's see if the new patch and explanations are enough :)
Simo.
On Mon, Sep 14, 2009 at 11:30:44AM -0400, Simo Sorce wrote:
On Mon, 2009-09-14 at 07:48 -0400, Stephen Gallagher wrote:
Just a nitpick, but why did you replace sbus_conn_send_reply() in be_check_online with sbus_get_connection and dbus_connection_send()? They are functionally identical. (except that sbus_conn_send_reply() can get the connection in one fewer stack frame, since it can access the sbus_connection object directly)
I merged together what previously was an async request. It was only asking the id provider about offline status. Since now the offline status is directly accessible by the dp_provider_be.c there is no need to make a request to any provider, we directly return the answer to dp. (this call is not used by anything anyway so far but we were planning to use it to force a backend to go offline so it will come handy later on).
Assuming I'm reading this correctly, we're talking about considering a single backend process as being online or offline as a whole. Why is this dependent only on the ID provider for the backend?
It isn't, attached new patch that add be_mark_offline() calls also to the auth backends.
Shouldn't we consider that if the authentication module or password change modules are offline that we are offline?
yes, should be fixed in the new patch, now.
Furthermore, even if the ID provider is offline, if we have cached user information that allows us to initiate a connection to a still-live authentication provider, isn't that perfectly reasonable?
No, also because this is just an initial patch to start building infrastructure. If you remember we discussed the idea or allowing the monitor to put a backend forcibly offline, in that case all providers must respect this. Further more during auth we want to always refresh users, so if the id part is not available we can as well auth from the cached password (if caching passwords is allowed).
This infrastructure is also need for DNS discovery later, where we want a central place to tell a specific server is unreachable.
In any case I would rather put the infrastructure in place now, and tweak specific behaviors later.
I'd argue that if any ONE of the modules was the ultimate determinator of online/offline status, it should be authentication rather than identification.
Nope the auth module contacts the servers more rarely than the id backend (auths are rare compared to requests to get id information), so normally the ID backend is more qualified. In any case I am not going to make any provider king, all of them should be able to signal that servers are unreachable.
Code itself is sensible, so this is a Nack until you can convince me that the approach itself is right.
Let's see if the new patch and explanations are enough :)
Simo.
I agree, this patch is a good starting point and we can add fine tuning later.
ACK.
bye, Sumit
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 09/14/2009 12:10 PM, Sumit Bose wrote:
On Mon, Sep 14, 2009 at 11:30:44AM -0400, Simo Sorce wrote:
On Mon, 2009-09-14 at 07:48 -0400, Stephen Gallagher wrote:
Just a nitpick, but why did you replace sbus_conn_send_reply() in be_check_online with sbus_get_connection and dbus_connection_send()? They are functionally identical. (except that sbus_conn_send_reply() can get the connection in one fewer stack frame, since it can access the sbus_connection object directly)
I merged together what previously was an async request. It was only asking the id provider about offline status. Since now the offline status is directly accessible by the dp_provider_be.c there is no need to make a request to any provider, we directly return the answer to dp. (this call is not used by anything anyway so far but we were planning to use it to force a backend to go offline so it will come handy later on).
Assuming I'm reading this correctly, we're talking about considering a single backend process as being online or offline as a whole. Why is this dependent only on the ID provider for the backend?
It isn't, attached new patch that add be_mark_offline() calls also to the auth backends.
Shouldn't we consider that if the authentication module or password change modules are offline that we are offline?
yes, should be fixed in the new patch, now.
Furthermore, even if the ID provider is offline, if we have cached user information that allows us to initiate a connection to a still-live authentication provider, isn't that perfectly reasonable?
No, also because this is just an initial patch to start building infrastructure. If you remember we discussed the idea or allowing the monitor to put a backend forcibly offline, in that case all providers must respect this. Further more during auth we want to always refresh users, so if the id part is not available we can as well auth from the cached password (if caching passwords is allowed).
This infrastructure is also need for DNS discovery later, where we want a central place to tell a specific server is unreachable.
In any case I would rather put the infrastructure in place now, and tweak specific behaviors later.
I'd argue that if any ONE of the modules was the ultimate determinator of online/offline status, it should be authentication rather than identification.
Nope the auth module contacts the servers more rarely than the id backend (auths are rare compared to requests to get id information), so normally the ID backend is more qualified. In any case I am not going to make any provider king, all of them should be able to signal that servers are unreachable.
Code itself is sensible, so this is a Nack until you can convince me that the approach itself is right.
Let's see if the new patch and explanations are enough :)
Simo.
I agree, this patch is a good starting point and we can add fine tuning later.
ACK.
bye, Sumit _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
For the record, I concur.
- -- Stephen Gallagher RHCE 804006346421761
Looking to carve out IT costs? www.redhat.com/carveoutcosts/
sssd-devel@lists.fedorahosted.org
-
Simo Sorce -
Stephen Gallagher -
Sumit Bose