On 08/30/2016 03:54 AM, Jakub Hrozek wrote:
On Sat, Aug 27, 2016 at 12:54:53PM -0400, Justin Stephenson wrote:
Hello,
The attached patches resolve https://fedorahosted.org/sssd/ticket/3142
However, I am having difficult with the man page addition to
'src/man/sssd.conf.5.xml' for this new option. I have stared at the open and
close xml tags(for far too long) and it looks correct but when I build sssd
I never see the sssd.conf man page inclusion. Could anyone tell me what I am
missing here?
If you feel there is better wording for the description please let me know.
Kind regards,
Justin Stephenson
From 0552c199dd37c7e280304b9bc92ff44a8a1a6d57 Mon Sep 17 00:00:00 2001
From: Justin Stephenson <jstep...@redhat.com>
Date: Fri, 26 Aug 2016 15:15:32 -0400
Subject: [PATCH 1/2] MONITOR: Remove --disable-netlink command-line option
I'm not sure I like removing the netlink option w/o letting admins who
use it at least know what happened. Could we keep the option in the popt
option list, but use the HIDDEN argument so that it doesn't show up in
--help output and print a loud warning that the option was removed in
favor of a sssd.conf option?
I already know of two people from sssd-users list who might be using
this feature. On the other hand, it was just introduced in the last
version and not in any enterprise distro, so just printing a warning and
removing even that warning in the next version would be fine for me..
Agreed, please see updated patches also with Petr's corrections. Once
this fix is pushed I can respond to the email and at least let these
users know.
I am still having trouble with the man page addition to sssd.conf not
showing, any ideas why?
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index
ae291e0fc8f2f9afabcdf32f18a5ec12252bbbbf..6f231b8ab8fc078d83331bb7ef5b980528a30bd6
100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -482,6 +482,24 @@
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>disable_netlink (boolean)</term>
+ <listitem>
+ <para>
+ SSSD hooks into the netlink interface to
+ monitor changes to routes, addresses, links
+ and trigger certain actions.
+ </para>
+ <para>
+ The SSSD state changes caused by netlink
+ events may be undesirable and can be
disabled
+ by setting this option to 'true'
+ </para>
+ <para>
+ Default: false (netlink changes are
detected)
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
</refsect2>
Kind regards,
Justin Stephenson
From c52c0c1a520cdf8509baaaaac00fa3c7bec0dd73 Mon Sep 17 00:00:00 2001
From: Justin Stephenson <jstep...@redhat.com>
Date: Fri, 26 Aug 2016 17:43:25 -0400
Subject: [PATCH 2/2] MONITOR: Add disable_netlink option
LGTM, untested, though.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>From 9282b9dc9f935bd632ace452dd6e07b31ea25ede Mon Sep 17 00:00:00 2001
From: Justin Stephenson <jstep...@redhat.com>
Date: Fri, 26 Aug 2016 17:43:25 -0400
Subject: [PATCH 2/3] MONITOR: Add disable_netlink option
Adding a new monitor boolean option to disable netlink support.
This will give users more control over sssd state changes without
having to modify systemd unit files.
Resolves:
https://fedorahosted.org/sssd/ticket/3142
---
src/confdb/confdb.h | 1 +
src/config/SSSDConfig/__init__.py.in | 1 +
src/config/SSSDConfigTest.py | 3 ++-
src/config/cfg_rules.ini | 1 +
src/config/etc/sssd.api.conf | 1 +
src/man/sssd.conf.5.xml | 18 ++++++++++++++++++
src/monitor/monitor.c | 21 ++++++++++++++++++---
7 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 401e5fbf7ed6bb9e8d7158dfab378c8159aa03db..2d650900170d5f2214aa56f00fc749980e53f516 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -73,6 +73,7 @@
#define CONFDB_MONITOR_OVERRIDE_SPACE "override_space"
#define CONFDB_MONITOR_USER_RUNAS "user"
#define CONFDB_MONITOR_CERT_VERIFICATION "certificate_verification"
+#define CONFDB_MONITOR_DISABLE_NETLINK "disable_netlink"
/* Both monitor and domains */
#define CONFDB_NAME_REGEX "re_expression"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index 0191920f93ab9016508e08785c25dd043c180c0b..2027028f7b4e972c7bc0dd5156fd85157ae192f4 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -62,6 +62,7 @@ option_strings = {
'user' : _('The user to drop privileges to'),
'certificate_verification' : _('Tune certificate verification'),
'override_space': _('All spaces in group or user names will be replaced with this character'),
+ 'disable_netlink' : _('Tune sssd to honor or ignore netlink state changes'),
# [nss]
'enum_cache_timeout' : _('Enumeration cache timeout length (seconds)'),
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 6a0fdf0ea5215103b48dc8521a43ae945342c0e2..8a64a257ab978b81ae4b26918c683b25a30fe7c1 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -310,7 +310,8 @@ class SSSDConfigTestSSSDService(unittest.TestCase):
'client_idle_timeout',
'description',
'certificate_verification',
- 'override_space']
+ 'override_space',
+ 'disable_netlink']
self.assertTrue(type(options) == dict,
"Options should be a dictionary")
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 5e248066bd554d2a654a764f406f6b33c4d66733..93c10e2b7892027f0ee7a7af096814fb7cac333a 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -38,6 +38,7 @@ option = default_domain_suffix
option = certificate_verification
option = override_space
option = config_file_version
+option = disable_netlink
[rule/allowed_nss_options]
validator = ini_allowed_options
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 525f939cd204f4d484caa7b490d85b0d50de00ef..9e4bf2f6e5d536099af75a82126bc577e10386b4 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -28,6 +28,7 @@ user = str, None, false
default_domain_suffix = str, None, false
certificate_verification = str, None, false
override_space = str, None, false
+disable_netlink = bool, None, false
[nss]
# Name service
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index ae291e0fc8f2f9afabcdf32f18a5ec12252bbbbf..6f231b8ab8fc078d83331bb7ef5b980528a30bd6 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -482,6 +482,24 @@
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>disable_netlink (boolean)</term>
+ <listitem>
+ <para>
+ SSSD hooks into the netlink interface to
+ monitor changes to routes, addresses, links
+ and trigger certain actions.
+ </para>
+ <para>
+ The SSSD state changes caused by netlink
+ events may be undesirable and can be disabled
+ by setting this option to 'true'
+ </para>
+ <para>
+ Default: false (netlink changes are detected)
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
</refsect2>
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 3df28669ded86461a008ef8fd681436c01ba7281..51efce564dbd5e08ba9d3e906402797af3a8903f 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2052,6 +2052,7 @@ static int monitor_process_init(struct mt_ctx *ctx,
int num_providers;
int ret;
int error;
+ bool disable_netlink;
struct sysdb_upgrade_ctx db_up_ctx;
/* Set up the environment variable for the Kerberos Replay Cache */
@@ -2172,14 +2173,28 @@ static int monitor_process_init(struct mt_ctx *ctx,
return ret;
}
- ret = setup_netlink(ctx, ctx->ev, network_status_change_cb,
- ctx, &ctx->nlctx);
+ ret = confdb_get_bool(ctx->cdb,
+ CONFDB_MONITOR_CONF_ENTRY,
+ CONFDB_MONITOR_DISABLE_NETLINK,
+ false, &disable_netlink);
+
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
- "Cannot set up listening for network notifications\n");
+ "Failed to read disable_netlink from confdb: [%d] %s\n",
+ ret, sss_strerror(ret));
return ret;
}
+ if (disable_netlink == false) {
+ ret = setup_netlink(ctx, ctx->ev, network_status_change_cb,
+ ctx, &ctx->nlctx);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Cannot set up listening for network notifications\n");
+ return ret;
+ }
+ }
+
/* start providers */
num_providers = 0;
for (dom = ctx->domains; dom; dom = get_next_domain(dom, 0)) {
--
2.7.4
>From f647e732c2a5b8727376dded962766fb03bb5ea8 Mon Sep 17 00:00:00 2001
From: Justin Stephenson <jstep...@redhat.com>
Date: Fri, 26 Aug 2016 15:15:32 -0400
Subject: [PATCH 1/3] MONITOR: Remove --disable-netlink command-line option
Removing monitor command-line option, to be superceded by
sssd.conf option
---
src/man/sssd.8.xml | 11 -----------
src/monitor/monitor.c | 31 ++++++++++++++++++-------------
2 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/src/man/sssd.8.xml b/src/man/sssd.8.xml
index ca8444d31ebca3d65a3baf83e20d458226ed5cd4..923da6824907f0d2d140d9ca83f87338e7664f83 100644
--- a/src/man/sssd.8.xml
+++ b/src/man/sssd.8.xml
@@ -114,17 +114,6 @@
</varlistentry>
<varlistentry>
<term>
- <option>--disable-netlink</option>
- </term>
- <listitem>
- <para>
- sssd will ignore Netlink changes when making decisions
- about resetting online and offline operational status.
- </para>
- </listitem>
- </varlistentry>
- <varlistentry>
- <term>
<option>-c</option>,<option>--config</option>
</term>
<listitem>
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 1f89c5a79feab8a921ce2f9132763b37ab506596..3df28669ded86461a008ef8fd681436c01ba7281 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2041,8 +2041,7 @@ static void missing_resolv_conf(struct tevent_context *ev,
}
static int monitor_process_init(struct mt_ctx *ctx,
- const char *config_file,
- bool opt_netlinkoff)
+ const char *config_file)
{
TALLOC_CTX *tmp_ctx;
struct tevent_signal *tes;
@@ -2173,14 +2172,12 @@ static int monitor_process_init(struct mt_ctx *ctx,
return ret;
}
- if (opt_netlinkoff == false) {
- ret = setup_netlink(ctx, ctx->ev, network_status_change_cb,
- ctx, &ctx->nlctx);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE,
- "Cannot set up listening for network notifications\n");
- return ret;
- }
+ ret = setup_netlink(ctx, ctx->ev, network_status_change_cb,
+ ctx, &ctx->nlctx);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Cannot set up listening for network notifications\n");
+ return ret;
}
/* start providers */
@@ -2488,7 +2485,8 @@ int main(int argc, const char *argv[])
_("Become a daemon (default)"), NULL }, \
{"interactive", 'i', POPT_ARG_NONE, &opt_interactive, 0, \
_("Run interactive (not a daemon)"), NULL}, \
- {"disable-netlink", '\0', POPT_ARG_NONE, &opt_netlinkoff, 0, \
+ {"disable-netlink", '\0', POPT_ARG_NONE | POPT_ARGFLAG_DOC_HIDDEN,
+ &opt_netlinkoff, 0, \
_("Disable netlink interface"), NULL}, \
{"config", 'c', POPT_ARG_STRING, &opt_config_file, 0, \
_("Specify a non-default config file"), NULL}, \
@@ -2540,6 +2538,13 @@ int main(int argc, const char *argv[])
return 1;
}
+ if (opt_netlinkoff) {
+ fprintf(stderr, "Option --disable-netlink has been removed and "
+ "replaced as a Monitor option in sssd.conf\n");
+ poptPrintUsage(pc, stderr, 0);
+ return 1;
+ }
+
if (!opt_daemon && !opt_interactive && !opt_genconf) {
opt_daemon = 1;
}
@@ -2692,8 +2697,8 @@ int main(int argc, const char *argv[])
monitor->ev = main_ctx->event_ctx;
talloc_steal(main_ctx, monitor);
- ret = monitor_process_init(monitor, config_file,
- opt_netlinkoff);
+ ret = monitor_process_init(monitor, config_file);
+
if (ret != EOK) return 3;
talloc_free(tmp_ctx);
--
2.7.4
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
