On 01/15/2015 03:57 PM, Pavel Reichl wrote:
On 01/15/2015 11:21 AM, Pavel Březina wrote:
On 01/09/2015 04:56 PM, Pavel Reichl wrote:
On 01/08/2015 08:36 PM, Pavel Březina wrote:
On 01/07/2015 03:28 PM, Pavel Reichl wrote:
Please see attached patches.
Thanks.
Hi,
I know there is a design page I did not comment on but seeing the
patch now, does the option really have to be names ad_site_override?
Wouldn't simple ad_site be a better name?
I'd change the description to something like:
"Specify AD site client should try to connect to. If this option is
not set, the AD site will be auto-discovered."
We are using talloc_zero so the following code:
@@ -591,6 +593,14 @@ ad_srv_plugin_ctx_init(TALLOC_CTX *mem_ctx,
goto fail;
}
+ if (ad_site_override == NULL) {
+ ctx->ad_site_override = NULL;
+ } else {
+ ctx->ad_site_override = talloc_strdup(ctx,
ad_site_override);
+ if (ctx->ad_site_override == NULL) {
+ goto fail;
+ }
+ }
return ctx;
can be simplified to:
if (ad_site_override != NULL) {
ctx->ad_site_override = talloc_strdup(ctx, ad_site_override);
if (ctx->ad_site_override == NULL) {
goto fail;
}
}
> (please add an empty line here)
return ctx;
@@ -756,6 +766,19 @@ static void ad_srv_plugin_site_done(struct
tevent_req *subreq)
ret = ad_get_client_site_recv(state, subreq, &state->site,
&state->forest);
talloc_zfree(subreq);
+ /* Ignore AD site found by dns discovery if specific site is
set in
+ * configuration file. */
+ if (state->ctx->ad_site_override != NULL) {
+ DEBUG(SSSDBG_TRACE_INTERNAL,
+ "Ignoring AD site found by DNS discovery: '%s', "
+ "using configured value: '%s' instead.\n",
+ state->site, state->ctx->ad_site_override);
+ state->site = talloc_strdup(state,
state->ctx->ad_site_override);
+ if (state->site == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+ }
There is no need to strdup the site name if you make state->site
const.
man page:
Specify AD site client should try to connect to.
Specify AD site to which client should try to connect.
Which one sounds better?
I don't care much, maybe the second one. Should I prepare a new patch
or will you Jakub amend the patch while pushing it? Thanks.
Attached patch updates man page with Pavel's second suggestion.
Can I get the final ack? I would like to prepare scratch build for
customer ASAP.
Thanks!
Ack otherwise.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>From 6b319c66f08a4458c7de1bc152cc4646f047c0a3 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <prei...@redhat.com>
Date: Wed, 7 Jan 2015 09:40:45 +0000
Subject: [PATCH 1/2] AD: add new option ad_site
This option overrides a result of the automatic site discovery.
Resolves:
https://fedorahosted.org/sssd/ticket/2486
---
src/config/SSSDConfig/__init__.py.in | 2 ++
src/config/etc/sssd.api.d/sssd-ad.conf | 1 +
src/man/sssd-ad.5.xml | 14 ++++++++++++++
src/providers/ad/ad_common.h | 1 +
src/providers/ad/ad_opts.h | 1 +
5 files changed, 19 insertions(+)
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index 500bd717fec7abcaafd5153ccca7847b91e208ad..ae00a2b7f9130725a6a766a4cbbba0a53f86dd7a 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -188,6 +188,8 @@ option_strings = {
'ad_gpo_map_permit' : _('PAM service names for which GPO-based access is always granted'),
'ad_gpo_map_deny' : _('PAM service names for which GPO-based access is always denied'),
'ad_gpo_default_right' : _('Default logon right (or permit/deny) to use for unmapped PAM service names'),
+ 'ad_site' : _('a particular site to be used by the client'),
+
# [provider/krb5]
'krb5_kdcip' : _('Kerberos server address'),
'krb5_server' : _('Kerberos server address'),
diff --git a/src/config/etc/sssd.api.d/sssd-ad.conf b/src/config/etc/sssd.api.d/sssd-ad.conf
index 3496fb4006697d380f7c9729ed9997272cbce2ea..5a5ea0c36b07d7497c1caa4208c7270d6de6dcc9 100644
--- a/src/config/etc/sssd.api.d/sssd-ad.conf
+++ b/src/config/etc/sssd.api.d/sssd-ad.conf
@@ -16,6 +16,7 @@ ad_gpo_map_service = str, None, false
ad_gpo_map_permit = str, None, false
ad_gpo_map_deny = str, None, false
ad_gpo_default_right = str, None, false
+ad_site = str, None, false
ldap_uri = str, None, false
ldap_backup_uri = str, None, false
ldap_search_base = str, None, false
diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index 1b33f28ab601871fbd52c47a6a829a24b9b67302..461390fe3a82ed7744da3c8d375b6a01079368fe 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -228,6 +228,20 @@ FOREST:EXAMPLE.COM:(memberOf=cn=admins,ou=groups,dc=example,dc=com)
</varlistentry>
<varlistentry>
+ <term>ad_site (string)</term>
+ <listitem>
+ <para>
+ Specify AD site to which client should try to
+ connect to. If this option is not set, the AD site
+ will be auto-discovered.
+ </para>
+ <para>
+ Default: Not set
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>ad_enable_gc (boolean)</term>
<listitem>
<para>
diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h
index b39ade40cd00ad5fccdb5d4bf4df8790eb634a51..dcd70bf94c682fde8b7d17a323ccc3947c659d73 100644
--- a/src/providers/ad/ad_common.h
+++ b/src/providers/ad/ad_common.h
@@ -60,6 +60,7 @@ enum ad_basic_opt {
AD_GPO_MAP_PERMIT,
AD_GPO_MAP_DENY,
AD_GPO_DEFAULT_RIGHT,
+ AD_SITE,
AD_KRB5_CONFD_PATH,
AD_OPTS_BASIC /* opts counter */
diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h
index d9405e5020ca724a0f7caa752ac10fb07d8aa397..f4c1c523bdc57a824105dfd781eb90a88e068908 100644
--- a/src/providers/ad/ad_opts.h
+++ b/src/providers/ad/ad_opts.h
@@ -48,6 +48,7 @@ struct dp_option ad_basic_opts[] = {
{ "ad_gpo_map_permit", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ad_gpo_map_deny", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ad_gpo_default_right", DP_OPT_STRING, NULL_STRING, NULL_STRING },
+ { "ad_site", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "krb5_confd_path", DP_OPT_STRING, { KRB5_MAPPING_DIR }, NULL_STRING },
DP_OPTION_TERMINATOR
};
--
2.1.0
>From 17f44c7f3270de22364a9407106a9799a6db944b Mon Sep 17 00:00:00 2001
From: Pavel Reichl <prei...@redhat.com>
Date: Wed, 7 Jan 2015 11:02:44 +0000
Subject: [PATCH 2/2] AD: support for AD site override
Override AD site found during DNS discovery.
Resolves:
https://fedorahosted.org/sssd/ticket/2486
---
src/providers/ad/ad_init.c | 6 +++++-
src/providers/ad/ad_srv.c | 24 +++++++++++++++++++++---
src/providers/ad/ad_srv.h | 3 ++-
src/providers/ad/ad_subdomains.c | 5 ++++-
src/providers/ipa/ipa_subdomains.c | 6 +++++-
5 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c
index cba792726374fb7a949ee4077d799a737b2f6681..2de7e0a44015750c7a4c7980d4ebeb26736c4f02 100644
--- a/src/providers/ad/ad_init.c
+++ b/src/providers/ad/ad_init.c
@@ -159,6 +159,7 @@ sssm_ad_id_init(struct be_ctx *bectx,
struct ad_id_ctx *ad_ctx;
const char *hostname;
const char *ad_domain;
+ const char *ad_site_override;
struct ad_srv_plugin_ctx *srv_ctx;
if (!ad_options) {
@@ -234,9 +235,12 @@ sssm_ad_id_init(struct be_ctx *bectx,
if (dp_opt_get_bool(ad_options->basic, AD_ENABLE_DNS_SITES)) {
/* use AD plugin */
ad_domain = dp_opt_get_string(ad_options->basic, AD_DOMAIN);
+ ad_site_override = dp_opt_get_string(ad_options->basic, AD_SITE);
+
srv_ctx = ad_srv_plugin_ctx_init(bectx, bectx->be_res,
default_host_dbs, ad_options->id,
- hostname, ad_domain);
+ hostname, ad_domain,
+ ad_site_override);
if (srv_ctx == NULL) {
DEBUG(SSSDBG_FATAL_FAILURE, "Out of memory?\n");
ret = ENOMEM;
diff --git a/src/providers/ad/ad_srv.c b/src/providers/ad/ad_srv.c
index 53d8a87701705654318845eaa4e4eac9ff6a5951..ac9dfa187213e6cdb9cdc3440f013e5b73e11fc4 100644
--- a/src/providers/ad/ad_srv.c
+++ b/src/providers/ad/ad_srv.c
@@ -540,7 +540,7 @@ done:
int ad_get_client_site_recv(TALLOC_CTX *mem_ctx,
struct tevent_req *req,
- char **_site,
+ const char **_site,
char **_forest)
{
struct ad_get_client_site_state *state = NULL;
@@ -560,6 +560,7 @@ struct ad_srv_plugin_ctx {
struct sdap_options *opts;
const char *hostname;
const char *ad_domain;
+ const char *ad_site_override;
};
struct ad_srv_plugin_ctx *
@@ -568,7 +569,8 @@ ad_srv_plugin_ctx_init(TALLOC_CTX *mem_ctx,
enum host_database *host_dbs,
struct sdap_options *opts,
const char *hostname,
- const char *ad_domain)
+ const char *ad_domain,
+ const char *ad_site_override)
{
struct ad_srv_plugin_ctx *ctx = NULL;
@@ -591,6 +593,13 @@ ad_srv_plugin_ctx_init(TALLOC_CTX *mem_ctx,
goto fail;
}
+ if (ad_site_override != NULL) {
+ ctx->ad_site_override = talloc_strdup(ctx, ad_site_override);
+ if (ctx->ad_site_override == NULL) {
+ goto fail;
+ }
+ }
+
return ctx;
fail:
@@ -605,7 +614,7 @@ struct ad_srv_plugin_state {
const char *protocol;
const char *discovery_domain;
- char *site;
+ const char *site;
char *dns_domain;
char *forest;
struct fo_server_info *primary_servers;
@@ -756,6 +765,15 @@ static void ad_srv_plugin_site_done(struct tevent_req *subreq)
ret = ad_get_client_site_recv(state, subreq, &state->site, &state->forest);
talloc_zfree(subreq);
+ /* Ignore AD site found by dns discovery if specific site is set in
+ * configuration file. */
+ if (state->ctx->ad_site_override != NULL) {
+ DEBUG(SSSDBG_TRACE_INTERNAL,
+ "Ignoring AD site found by DNS discovery: '%s', "
+ "using configured value: '%s' instead.\n",
+ state->site, state->ctx->ad_site_override);
+ state->site = state->ctx->ad_site_override;
+ }
if (ret == EOK) {
if (strcmp(state->service, "gc") == 0) {
primary_domain = talloc_asprintf(state, AD_SITE_DOMAIN_FMT,
diff --git a/src/providers/ad/ad_srv.h b/src/providers/ad/ad_srv.h
index 7522ecae4d15397efc0bd3c6cd711a3024eb1771..be3ac2826186008a4db6333e5d27e408aaed2e51 100644
--- a/src/providers/ad/ad_srv.h
+++ b/src/providers/ad/ad_srv.h
@@ -29,7 +29,8 @@ ad_srv_plugin_ctx_init(TALLOC_CTX *mem_ctx,
enum host_database *host_dbs,
struct sdap_options *opts,
const char *hostname,
- const char *ad_domain);
+ const char *ad_domain,
+ const char *ad_site_override);
struct tevent_req *ad_srv_plugin_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 3c61d13522c7c773171ea8645dddb417e610745c..b3821f8d07855e431e69cdc00c225bcd7e4db1eb 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -102,6 +102,7 @@ ad_subdom_ad_ctx_new(struct be_ctx *be_ctx,
const char *gc_service_name;
struct ad_srv_plugin_ctx *srv_ctx;
char *ad_domain;
+ char *ad_site_override;
struct sdap_domain *sdom;
errno_t ret;
const char *realm;
@@ -122,6 +123,7 @@ ad_subdom_ad_ctx_new(struct be_ctx *be_ctx,
}
ad_domain = subdom->name;
+ ad_site_override = dp_opt_get_string(ad_options->basic, AD_SITE);
ret = dp_opt_set_string(ad_options->basic, AD_DOMAIN, ad_domain);
if (ret != EOK) {
@@ -158,7 +160,8 @@ ad_subdom_ad_ctx_new(struct be_ctx *be_ctx,
default_host_dbs,
ad_id_ctx->ad_options->id,
hostname,
- ad_domain);
+ ad_domain,
+ ad_site_override);
if (srv_ctx == NULL) {
DEBUG(SSSDBG_FATAL_FAILURE, "Out of memory?\n");
return ENOMEM;
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 3148389f71135b6c64e62d1cf7f4064dd183f595..d0f02badee52afc93eb9bf316e654aa0e789937a 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -116,6 +116,7 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx,
const char *gc_service_name;
struct ad_srv_plugin_ctx *srv_ctx;
char *ad_domain;
+ const char *ad_site_override;
struct sdap_domain *sdom;
errno_t ret;
const char *extra_attrs;
@@ -201,12 +202,15 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx,
ad_id_ctx->sdap_id_ctx->opts = ad_options->id;
ad_options->id_ctx = ad_id_ctx;
+ ad_site_override = dp_opt_get_string(ad_options->basic, AD_SITE);
+
/* use AD plugin */
srv_ctx = ad_srv_plugin_ctx_init(be_ctx, be_ctx->be_res,
default_host_dbs,
ad_id_ctx->ad_options->id,
id_ctx->server_mode->hostname,
- ad_domain);
+ ad_domain,
+ ad_site_override);
if (srv_ctx == NULL) {
DEBUG(SSSDBG_FATAL_FAILURE, "Out of memory?\n");
return ENOMEM;
--
2.1.0
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
