Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 07/17/2013 01:48 PM, Jan Cholasta wrote: > On 17.7.2013 13:13, Tomas Babej wrote: >> > + class AdviceLogger(object): >> >> > >> >> > Please don't use nested classes. If you want AdviceLogger to be >> >> > private-ish, you can rename it to _AdviceLogger. >> >> > >> >> > Also I think AdviceLogger is a little bit misleading name, I would >> >> > prefer AdviceOutput or something like that. >> >> > >> >> Fixed. > > Thanks. > >> >> > Functionally the patch is OK, but I have some second thoughts about the >> >> > design. I'm not sure if using API plugins is truly the right thing to >> >> > do, as advises seem to be pretty much orthogonal to the rest of our API. >> >> > There are some negative side effects, such as initializing the API every >> >> > time ipa-advise is run, for each and every advice, which takes some >> >> > time, so there is a short but noticable delay. >> >> What do you mean by that API is initialized for each and every advice? > > For example when you run "ipa-advise config-fedora-authconfig", all of the > ipalib and advise plugins are initialized. Seems like an overkill just to > print > 6 lines of text. In this case, it at least uses options from IPA server configuration (api.env) - which I think will be the case for most avices of this kind. > >> >> AFAIK, the advice plugins are all imported at once, the the API is >> initialized. >> >> They are imported only in the API 'advise' context, so no performance >> decrease >> >> for the rest of the framework. >> >> > What are the benefits of >> >> > using API plugins for this, besides code reuse? (I'm not saying this >> >> > must be changed, just give it some thought, using something simpler >> >> > might be better.) >> >> Code reuse is one thing. Also, ability to call the IPA commands from >> >> within the plugins is the second factor. To allow that we would have to >> >> inicialize the API anyway. > > ... which could be done on-demand when it is actually needed. > >> >> Also some important constants which can be leveraged by the plugins are >> >> contained in api.env namespace. >> >> Taking into consideration that running ipa-advise is more of a >> >> one-time thing, I am willing to sacrifice a bit of delay in >> >> favour of these advantages. > > OK. > > I still think that it's rather strange to pretend that advices are part of our > API when they don't actually contribute anything to the API, but that's more > of > a structural problem, not a problem with your patch. > >> >> Updated patch attached. > > ACK. > > Honza > Pushed to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 17.7.2013 13:13, Tomas Babej wrote: > + class AdviceLogger(object): > > Please don't use nested classes. If you want AdviceLogger to be > private-ish, you can rename it to _AdviceLogger. > > Also I think AdviceLogger is a little bit misleading name, I would > prefer AdviceOutput or something like that. > Fixed. Thanks. > Functionally the patch is OK, but I have some second thoughts about the > design. I'm not sure if using API plugins is truly the right thing to > do, as advises seem to be pretty much orthogonal to the rest of our API. > There are some negative side effects, such as initializing the API every > time ipa-advise is run, for each and every advice, which takes some > time, so there is a short but noticable delay. What do you mean by that API is initialized for each and every advice? For example when you run "ipa-advise config-fedora-authconfig", all of the ipalib and advise plugins are initialized. Seems like an overkill just to print 6 lines of text. AFAIK, the advice plugins are all imported at once, the the API is initialized. They are imported only in the API 'advise' context, so no performance decrease for the rest of the framework. > What are the benefits of > using API plugins for this, besides code reuse? (I'm not saying this > must be changed, just give it some thought, using something simpler > might be better.) Code reuse is one thing. Also, ability to call the IPA commands from within the plugins is the second factor. To allow that we would have to inicialize the API anyway. ... which could be done on-demand when it is actually needed. Also some important constants which can be leveraged by the plugins are contained in api.env namespace. Taking into consideration that running ipa-advise is more of a one-time thing, I am willing to sacrifice a bit of delay in favour of these advantages. OK. I still think that it's rather strange to pretend that advices are part of our API when they don't actually contribute anything to the API, but that's more of a structural problem, not a problem with your patch. Updated patch attached. ACK. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On Tuesday 16 of July 2013 14:10:44 Jan Cholasta wrote:
> On 21.6.2013 11:45, Tomas Babej wrote:
> > Newly added features:
> >
> > - options propagated to plugins
> > - made plugin content creation more comfortable, now 3 classes of
> > output are
> > available (debug, comment, command)
> >
> > Now pretty much everything that comes into my mind is addressed, so
> > please have a look
> > at the current implementation.
>
> The patch needs a rebase.
>
> +class AdviceLogger(object):
>
> Please don't use nested classes. If you want AdviceLogger to be
> private-ish, you can rename it to _AdviceLogger.
>
> Also I think AdviceLogger is a little bit misleading name, I would
> prefer AdviceOutput or something like that.
>
Fixed.
> Functionally the patch is OK, but I have some second thoughts about the
> design. I'm not sure if using API plugins is truly the right thing to
> do, as advises seem to be pretty much orthogonal to the rest of our API.
> There are some negative side effects, such as initializing the API every
> time ipa-advise is run, for each and every advice, which takes some
> time, so there is a short but noticable delay.
What do you mean by that API is initialized for each and every advice?
AFAIK, the advice plugins are all imported at once, the the API is initialized.
They are imported only in the API 'advise' context, so no performance decrease
for the rest of the framework.
> What are the benefits of
> using API plugins for this, besides code reuse? (I'm not saying this
> must be changed, just give it some thought, using something simpler
> might be better.)
Code reuse is one thing. Also, ability to call the IPA commands from
within the plugins is the second factor. To allow that we would have to
inicialize the API anyway.
Also some important constants which can be leveraged by the plugins are
contained in api.env namespace.
Taking into consideration that running ipa-advise is more of a
one-time thing, I am willing to sacrifice a bit of delay in
favour of these advantages.
Updated patch attached.
Tomas>From d4384a2fd9991bec0aa6082046d1a87d5645add8 Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Mon, 10 Jun 2013 14:43:24 +0200
Subject: [PATCH] Provide ipa-advise tool
Provides a pluggable framework for generating configuration
scriptlets and instructions for various machine setups and use
cases.
Creates a new ipa-advise command, available to root user
on the IPA server.
Also provides an example configuration plugin,
config-fedora-authconfig.
https://fedorahosted.org/freeipa/ticket/3670
---
freeipa.spec.in | 4 +
install/tools/Makefile.am | 1 +
install/tools/ipa-advise | 23
install/tools/man/Makefile.am | 1 +
install/tools/man/ipa-advise.1| 44 +++
ipalib/__init__.py| 7 +-
ipalib/frontend.py| 45 +++
ipalib/plugable.py| 2 +
ipaserver/advise/__init__.py | 22
ipaserver/advise/base.py | 169 ++
ipaserver/advise/plugins/__init__.py | 22
ipaserver/advise/plugins/fedora_authconfig.py | 41 +++
make-lint | 2 +-
setup.py | 2 +
14 files changed, 382 insertions(+), 3 deletions(-)
create mode 100755 install/tools/ipa-advise
create mode 100644 install/tools/man/ipa-advise.1
create mode 100644 ipaserver/advise/__init__.py
create mode 100644 ipaserver/advise/base.py
create mode 100644 ipaserver/advise/plugins/__init__.py
create mode 100644 ipaserver/advise/plugins/fedora_authconfig.py
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 86de29ffc16a199e382d2f6a6ad230a76758a55c..2f241b22c3bf2fb52aef04f8d2287565190d7870 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -600,6 +600,7 @@ fi
%{_sbindir}/ipa-managed-entries
%{_sbindir}/ipactl
%{_sbindir}/ipa-upgradeconfig
+%{_sbindir}/ipa-advise
%{_libexecdir}/certmonger/dogtag-ipa-retrieve-agent-submit
%{_libexecdir}/ipa-otpd
%config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
@@ -615,6 +616,8 @@ fi
%dir %{python_sitelib}/ipaserver
%dir %{python_sitelib}/ipaserver/install
%dir %{python_sitelib}/ipaserver/install/plugins
+%dir %{python_sitelib}/ipaserver/advise
+%dir %{python_sitelib}/ipaserver/advise/plugins
%dir %{python_sitelib}/ipaserver/plugins
%dir %{_libdir}/ipa/certmonger
%attr(755,root,root) %{_libdir}/ipa/certmonger/*
@@ -730,6 +733,7 @@ fi
%{_mandir}/man8/ipa-upgradeconfig.8.gz
%{_mandir}/man1/ipa-backup.1.gz
%{_mandir}/man1/ipa-restore.1.gz
+%{_mandir}/man1/ipa-advise.1.gz
%files server-trust-ad
%{_sbindir}/ipa-adtrust-install
diff --git a/install/tools/Makefile.am b/install/tools/Makefile.am
index 659ce0a87a65715b5829384f939c05e7026d763f..2cf66c6dfc1c272bb423253902e7339e7d159567 100644
--- a/inst
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 21.6.2013 11:45, Tomas Babej wrote: Newly added features: - options propagated to plugins - made plugin content creation more comfortable, now 3 classes of output are available (debug, comment, command) Now pretty much everything that comes into my mind is addressed, so please have a look at the current implementation. The patch needs a rebase. +class AdviceLogger(object): Please don't use nested classes. If you want AdviceLogger to be private-ish, you can rename it to _AdviceLogger. Also I think AdviceLogger is a little bit misleading name, I would prefer AdviceOutput or something like that. Functionally the patch is OK, but I have some second thoughts about the design. I'm not sure if using API plugins is truly the right thing to do, as advises seem to be pretty much orthogonal to the rest of our API. There are some negative side effects, such as initializing the API every time ipa-advise is run, for each and every advice, which takes some time, so there is a short but noticable delay. What are the benefits of using API plugins for this, besides code reuse? (I'm not saying this must be changed, just give it some thought, using something simpler might be better.) Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On Mon, 15 Jul 2013, Martin Kosek wrote: On 07/11/2013 08:59 AM, Tomas Babej wrote: On Wednesday 26 of June 2013 10:12:48 Petr Spacek wrote: [snip] Appropriate error handling = Return 'Permission denied' if particular operation requires higher privileges. IMHO 'cryptic' error message is bad in any case, so the right way how to fix 'cryptic' error messages is to fix the places where errors are thrown. I don't think that additional checks in 'advisor' to hide 'cryptic' errors are the right approach. -- Petr^2 Spacek To wrap-up, after an offline discussion Petr: We came to an conclusion that since 'require_root' attribute is optional (Petr was not aware of that, and that fact was what caused his concern), with default value False, the attribute should not pose any additional burden for the plugin developer. Tomas Agreed, I think that current approach is fine. Alexander, are you OK with the ipa-advise tool as is? We can add more bells and whistles or options/API for plugins when we have actually more ipa-advise plugins and see the real needs of such plugins. Yes, I'm OK. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 07/11/2013 08:59 AM, Tomas Babej wrote: > On Wednesday 26 of June 2013 10:12:48 Petr Spacek wrote: > > > > [snip] > > > >> > >> Appropriate error handling = Return 'Permission denied' if particular > >> operation requires higher privileges. > >> > >> IMHO 'cryptic' error message is bad in any case, so the right way how to fix > >> 'cryptic' error messages is to fix the places where errors are thrown. > >> > >> I don't think that additional checks in 'advisor' to hide 'cryptic' errors >> are > >> the right approach. > >> > >> -- > >> Petr^2 Spacek > > > > To wrap-up, after an offline discussion Petr: > > > > We came to an conclusion that since 'require_root' attribute is optional (Petr > was not aware of that, and that fact was what caused his concern), with > default > value False, the attribute should not pose any additional burden for > > the plugin developer. > > > > Tomas > Agreed, I think that current approach is fine. Alexander, are you OK with the ipa-advise tool as is? We can add more bells and whistles or options/API for plugins when we have actually more ipa-advise plugins and see the real needs of such plugins. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On Wednesday 26 of June 2013 10:12:48 Petr Spacek wrote: [snip] > > Appropriate error handling = Return 'Permission denied' if particular > operation requires higher privileges. > > IMHO 'cryptic' error message is bad in any case, so the right way how to fix > 'cryptic' error messages is to fix the places where errors are thrown. > > I don't think that additional checks in 'advisor' to hide 'cryptic' errors are > the right approach. > > -- > Petr^2 Spacek To wrap-up, after an offline discussion Petr: We came to an conclusion that since 'require_root' attribute is optional (Petr was not aware of that, and that fact was what caused his concern), with default value False, the attribute should not pose any additional burden for the plugin developer. Tomas___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 24.6.2013 14:55, Tomas Babej wrote: On 06/24/2013 09:35 AM, Petr Spacek wrote: What would happen if require_root = False, UID = 1234 but the plugin requires root access? (I.e. there is an error in the require_root value.) The calling of particular external command that requires root access for its execution will fail. I don't like this boolean, because plugin author has to test the plugin and maintain the boolean after each change in the plugin. From my (naive) point of view it is error prone and unnecessary. Why? From my point of view, it simplifies the work for the plugin author, since he can set the boolean if he knows that plugin will need root access to require information needed. I see the opposite part: "Root" requirement can change over time (during plugin and IPA development), so the plugin maintainer has to maintain this boolean. Without it: - If he wanted to stay user-friendly he would have to implement the check for effective UID in every plugin. - If he did not, he would be having his command fail with (often) cryptic errors. Proper error handling seems like 'the right way'™ to me. What kind of proper error handling? The errors are now properly handled via AdminTool's framework. Appropriate error handling = Return 'Permission denied' if particular operation requires higher privileges. IMHO 'cryptic' error message is bad in any case, so the right way how to fix 'cryptic' error messages is to fix the places where errors are thrown. I don't think that additional checks in 'advisor' to hide 'cryptic' errors are the right approach. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/24/2013 09:35 AM, Petr Spacek wrote: What would happen if require_root = False, UID = 1234 but the plugin requires root access? (I.e. there is an error in the require_root value.) The calling of particular external command that requires root access for its execution will fail. I don't like this boolean, because plugin author has to test the plugin and maintain the boolean after each change in the plugin. From my (naive) point of view it is error prone and unnecessary. Why? From my point of view, it simplifies the work for the plugin author, since he can set the boolean if he knows that plugin will need root access to require information needed. Without it: - If he wanted to stay user-friendly he would have to implement the check for effective UID in every plugin. - If he did not, he would be having his command fail with (often) cryptic errors. Proper error handling seems like 'the right way'™ to me. What kind of proper error handling? The errors are now properly handled via AdminTool's framework. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 21.6.2013 11:45, Tomas Babej wrote: On 06/21/2013 09:32 AM, Jan Cholasta wrote: On 21.6.2013 09:16, Tomas Babej wrote: I'm also thinking about propagating the --verbose, etc. options provided by default by AdminTool down to plugin level so that plugin authors can make use of them. What do you think? +1 Newly added features: - options propagated to plugins - made plugin content creation more comfortable, now 3 classes of output are available (debug, comment, command) Now pretty much everything that comes into my mind is addressed, so please have a look at the current implementation. Any suggestions welcome. New version: - provides require_root setting for plugins What would happen if require_root = False, UID = 1234 but the plugin requires root access? (I.e. there is an error in the require_root value.) I don't like this boolean, because plugin author has to test the plugin and maintain the boolean after each change in the plugin. From my (naive) point of view it is error prone and unnecessary. Proper error handling seems like 'the right way'™ to me. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/21/2013 09:32 AM, Jan Cholasta wrote:
On 21.6.2013 09:16, Tomas Babej wrote:
I'm also thinking about propagating the --verbose, etc. options provided
by default by AdminTool down to plugin level so that plugin authors can
make use of them. What do you think?
+1
Newly added features:
- options propagated to plugins
- made plugin content creation more comfortable, now 3 classes of
output are
available (debug, comment, command)
Now pretty much everything that comes into my mind is addressed, so
please have a look
at the current implementation.
Any suggestions welcome.
Tomas
From 757a82bf6b028c3cf3774267f9d4723d5fb9d578 Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Mon, 10 Jun 2013 14:43:24 +0200
Subject: [PATCH] Provide ipa-advise tool
Provides a pluggable framework for generating configuration
scriptlets and instructions for various machine setups and use
cases.
Creates a new ipa-advise command, available to root user
on the IPA server.
Also provides an example configuration plugin,
config-fedora-authconfig.
https://fedorahosted.org/freeipa/ticket/3670
---
freeipa.spec.in | 4 +
install/tools/Makefile.am | 1 +
install/tools/ipa-advise | 23
install/tools/man/Makefile.am | 1 +
install/tools/man/ipa-advise.1| 44 +++
ipalib/__init__.py| 7 +-
ipalib/frontend.py| 44 +++
ipalib/plugable.py| 2 +
ipaserver/advise/__init__.py | 22
ipaserver/advise/base.py | 169 ++
ipaserver/advise/plugins/__init__.py | 22
ipaserver/advise/plugins/fedora_authconfig.py | 41 +++
make-lint | 2 +-
setup.py | 2 +
14 files changed, 381 insertions(+), 3 deletions(-)
create mode 100755 install/tools/ipa-advise
create mode 100644 install/tools/man/ipa-advise.1
create mode 100644 ipaserver/advise/__init__.py
create mode 100644 ipaserver/advise/base.py
create mode 100644 ipaserver/advise/plugins/__init__.py
create mode 100644 ipaserver/advise/plugins/fedora_authconfig.py
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 239811ac26aa84e1928cefb9c3adac58326ad9a7..2bd2ca5558d189e7f64f3e575bae53ba64614a5a 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -598,6 +598,7 @@ fi
%{_sbindir}/ipactl
%{_sbindir}/ipa-upgradeconfig
%{_sbindir}/ipa-compliance
+%{_sbindir}/ipa-advise
%{_libexecdir}/certmonger/dogtag-ipa-retrieve-agent-submit
%{_libexecdir}/ipa-otpd
%{_sysconfdir}/cron.d/ipa-compliance
@@ -614,6 +615,8 @@ fi
%dir %{python_sitelib}/ipaserver
%dir %{python_sitelib}/ipaserver/install
%dir %{python_sitelib}/ipaserver/install/plugins
+%dir %{python_sitelib}/ipaserver/advise
+%dir %{python_sitelib}/ipaserver/advise/plugins
%dir %{python_sitelib}/ipaserver/plugins
%dir %{_libdir}/ipa/certmonger
%attr(755,root,root) %{_libdir}/ipa/certmonger/*
@@ -730,6 +733,7 @@ fi
%{_mandir}/man1/ipa-compliance.1.gz
%{_mandir}/man1/ipa-backup.1.gz
%{_mandir}/man1/ipa-restore.1.gz
+%{_mandir}/man1/ipa-advise.1.gz
%files server-trust-ad
%{_sbindir}/ipa-adtrust-install
diff --git a/install/tools/Makefile.am b/install/tools/Makefile.am
index 7c553116cdfdce69e594f9d9a36e6bebcab5f692..045874a352f6d006c11b3557d1bf002e37d18587 100644
--- a/install/tools/Makefile.am
+++ b/install/tools/Makefile.am
@@ -24,6 +24,7 @@ sbin_SCRIPTS = \
ipa-compliance \
ipa-backup \
ipa-restore \
+ ipa-advise \
$(NULL)
EXTRA_DIST = \
diff --git a/install/tools/ipa-advise b/install/tools/ipa-advise
new file mode 100755
index ..4ec3c48c7d8b24893ed39e33d3a14ae632483bfa
--- /dev/null
+++ b/install/tools/ipa-advise
@@ -0,0 +1,23 @@
+#! /usr/bin/python -E
+# Authors: Tomas Babej
+#
+# Copyright (C) 2013 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 21.6.2013 09:16, Tomas Babej wrote: I'm also thinking about propagating the --verbose, etc. options provided by default by AdminTool down to plugin level so that plugin authors can make use of them. What do you think? +1 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/20/2013 12:52 PM, Jan Cholasta wrote:
On 20.6.2013 12:28, Tomas Babej wrote:
Providing new version:
- no longer requires root access defaultly
- headers are printed out as comments
Tomas
You still have reference(s) to previous names of the script in the patch:
+"""
+Base class for advices, plugins for ipa-config-advice.
+"""
Fixed.
Is the --list option absolutely necessary? If I read your code
correctly, the list of advices is also returned when you run
ipa-advise without arguments.
Honza
New version:
- provides require_root setting for plugins
- options --list removed, man pages altered accordingly
I'm also thinking about propagating the --verbose, etc. options provided
by default by AdminTool down to plugin level so that plugin authors can
make use of them. What do you think?
Tomas
From 2542291baaa022d3be673ee11b5d31d3594be236 Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Mon, 10 Jun 2013 14:43:24 +0200
Subject: [PATCH] Provide ipa-advise tool
Provides a pluggable framework for generating configuration
scriptlets and instructions for various machine setups and use
cases.
Creates a new ipa-advise command, available to root user
on the IPA server.
Also provides an example configuration plugin,
config-fedora-authconfig.
https://fedorahosted.org/freeipa/ticket/3670
---
freeipa.spec.in | 4 +
install/tools/Makefile.am | 1 +
install/tools/ipa-advise | 23
install/tools/man/Makefile.am | 1 +
install/tools/man/ipa-advise.1| 44
ipalib/__init__.py| 7 +-
ipalib/frontend.py| 21
ipalib/plugable.py| 2 +
ipaserver/advise/__init__.py | 22
ipaserver/advise/base.py | 151 ++
ipaserver/advise/plugins/__init__.py | 22
ipaserver/advise/plugins/fedora_authconfig.py | 40 +++
make-lint | 2 +-
setup.py | 2 +
14 files changed, 339 insertions(+), 3 deletions(-)
create mode 100755 install/tools/ipa-advise
create mode 100644 install/tools/man/ipa-advise.1
create mode 100644 ipaserver/advise/__init__.py
create mode 100644 ipaserver/advise/base.py
create mode 100644 ipaserver/advise/plugins/__init__.py
create mode 100644 ipaserver/advise/plugins/fedora_authconfig.py
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 239811ac26aa84e1928cefb9c3adac58326ad9a7..2bd2ca5558d189e7f64f3e575bae53ba64614a5a 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -598,6 +598,7 @@ fi
%{_sbindir}/ipactl
%{_sbindir}/ipa-upgradeconfig
%{_sbindir}/ipa-compliance
+%{_sbindir}/ipa-advise
%{_libexecdir}/certmonger/dogtag-ipa-retrieve-agent-submit
%{_libexecdir}/ipa-otpd
%{_sysconfdir}/cron.d/ipa-compliance
@@ -614,6 +615,8 @@ fi
%dir %{python_sitelib}/ipaserver
%dir %{python_sitelib}/ipaserver/install
%dir %{python_sitelib}/ipaserver/install/plugins
+%dir %{python_sitelib}/ipaserver/advise
+%dir %{python_sitelib}/ipaserver/advise/plugins
%dir %{python_sitelib}/ipaserver/plugins
%dir %{_libdir}/ipa/certmonger
%attr(755,root,root) %{_libdir}/ipa/certmonger/*
@@ -730,6 +733,7 @@ fi
%{_mandir}/man1/ipa-compliance.1.gz
%{_mandir}/man1/ipa-backup.1.gz
%{_mandir}/man1/ipa-restore.1.gz
+%{_mandir}/man1/ipa-advise.1.gz
%files server-trust-ad
%{_sbindir}/ipa-adtrust-install
diff --git a/install/tools/Makefile.am b/install/tools/Makefile.am
index 7c553116cdfdce69e594f9d9a36e6bebcab5f692..045874a352f6d006c11b3557d1bf002e37d18587 100644
--- a/install/tools/Makefile.am
+++ b/install/tools/Makefile.am
@@ -24,6 +24,7 @@ sbin_SCRIPTS = \
ipa-compliance \
ipa-backup \
ipa-restore \
+ ipa-advise \
$(NULL)
EXTRA_DIST = \
diff --git a/install/tools/ipa-advise b/install/tools/ipa-advise
new file mode 100755
index ..4ec3c48c7d8b24893ed39e33d3a14ae632483bfa
--- /dev/null
+++ b/install/tools/ipa-advise
@@ -0,0 +1,23 @@
+#! /usr/bin/python -E
+# Authors: Tomas Babej
+#
+# Copyright (C) 2013 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
Martin Kosek wrote: On 06/20/2013 09:29 AM, Petr Spacek wrote: On 19.6.2013 20:56, Alexander Bokovoy wrote: On Wed, 19 Jun 2013, Rob Crittenden wrote: Tomas Babej wrote: [big snip] Providing new version which should address mentioned issues: - advice plugins now inherit directly from Plugin, initial approach via Method class was abandoned - new Namespace api.Advice collects all the advice plugins - tool renamed to ipa-advise to express a more general use case Additional improvements: - keywords are now generated out of Advice class's name, where underscores are replaced by hyphens - rewritten the example plugin in the docs, and provided more information there - instead of --setup option to provide configuration, ipa-advise takes one positional argument - renamed to ipa-advise Concerns: - man page might need more improvements I'll craft a design page for plugin authors, might be useful, even if the info is in the package docs. --- Here's a little preview: [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. /sbin/authconfig --enableldap --ldapserver=vm-001.idm.com --enablerfc2307bis --enablekrb5 [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig4 invalid 'setup': No instructions are available for 'fedora_authconfig4'. See the list of available configuration advices using the --list option. [tbabej@vm-001 ~]$ sudo ipa-advise - List of available advices - fedora-authconfig : Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. If it's just providing advise why does it need root access? Or is it expected to provide advise based on current configuration? Exactly. Getting ranges, configured trusts, etc. Not all of that information may be available under non-privileged account, especially if somebody would decide to plug in advices for backup or CA handling/configuration of advanced features. I think that ipa-advise should not require root access *implicitly*. It would prevent lower-level admins from ipa-advise tool. IMHO plugins should try to get required information and print an 'Insufficient access rights, try it again as root/admin' error when appropriate. As a result, basic 'advices' (like recommended client configuration) will be accessible anybody and special 'advices' (something related to AD trusts etc.) will be accessible only to admins. +1 I think the reason why Tomas did it as root was that he can that autobind to the DS. But he could easily operate in 2 modes, similarly to ipa-ldap-updater and simply just auth wuth GSSAPI when he is not logged as a root. Alternatively, add a requires_root on the plugin level so that some plugins require root, others do not. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 20.6.2013 12:28, Tomas Babej wrote: Providing new version: - no longer requires root access defaultly - headers are printed out as comments Tomas You still have reference(s) to previous names of the script in the patch: +""" +Base class for advices, plugins for ipa-config-advice. +""" Is the --list option absolutely necessary? If I read your code correctly, the list of advices is also returned when you run ipa-advise without arguments. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
[snip]
On 06/19/2013 08:58 PM, Alexander Bokovoy wrote:
As the output is almost usable for cut&paste to run on client
machines, may be prefix the description/instructions with #?
[snip]
+1
I think the reason why Tomas did it as root was that he can that autobind to
the DS. But he could easily operate in 2 modes, similarly to ipa-ldap-updater
and simply just auth wuth GSSAPI when he is not logged as a root.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Providing new version:
- no longer requires root access defaultly
- headers are printed out as comments
Tomas
From d145dd7c26c20335248892507ca0364d8d205c4d Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Mon, 10 Jun 2013 14:43:24 +0200
Subject: [PATCH] Provide ipa-advise tool
Provides a pluggable framework for generating configuration
scriptlets and instructions for various machine setups and use
cases.
Creates a new ipa-advise command, available to root user
on the IPA server.
Also provides an example configuration plugin,
config-fedora-authconfig.
https://fedorahosted.org/freeipa/ticket/3670
---
freeipa.spec.in | 4 +
install/tools/Makefile.am | 1 +
install/tools/ipa-advise | 23
install/tools/man/Makefile.am | 1 +
install/tools/man/ipa-advise.1| 45
ipalib/__init__.py| 7 +-
ipalib/frontend.py| 17 +++
ipalib/plugable.py| 2 +
ipaserver/advise/__init__.py | 22
ipaserver/advise/base.py | 144 ++
ipaserver/advise/plugins/__init__.py | 22
ipaserver/advise/plugins/fedora_authconfig.py | 40 +++
make-lint | 2 +-
setup.py | 2 +
14 files changed, 329 insertions(+), 3 deletions(-)
create mode 100755 install/tools/ipa-advise
create mode 100644 install/tools/man/ipa-advise.1
create mode 100644 ipaserver/advise/__init__.py
create mode 100644 ipaserver/advise/base.py
create mode 100644 ipaserver/advise/plugins/__init__.py
create mode 100644 ipaserver/advise/plugins/fedora_authconfig.py
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 239811ac26aa84e1928cefb9c3adac58326ad9a7..2bd2ca5558d189e7f64f3e575bae53ba64614a5a 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -598,6 +598,7 @@ fi
%{_sbindir}/ipactl
%{_sbindir}/ipa-upgradeconfig
%{_sbindir}/ipa-compliance
+%{_sbindir}/ipa-advise
%{_libexecdir}/certmonger/dogtag-ipa-retrieve-agent-submit
%{_libexecdir}/ipa-otpd
%{_sysconfdir}/cron.d/ipa-compliance
@@ -614,6 +615,8 @@ fi
%dir %{python_sitelib}/ipaserver
%dir %{python_sitelib}/ipaserver/install
%dir %{python_sitelib}/ipaserver/install/plugins
+%dir %{python_sitelib}/ipaserver/advise
+%dir %{python_sitelib}/ipaserver/advise/plugins
%dir %{python_sitelib}/ipaserver/plugins
%dir %{_libdir}/ipa/certmonger
%attr(755,root,root) %{_libdir}/ipa/certmonger/*
@@ -730,6 +733,7 @@ fi
%{_mandir}/man1/ipa-compliance.1.gz
%{_mandir}/man1/ipa-backup.1.gz
%{_mandir}/man1/ipa-restore.1.gz
+%{_mandir}/man1/ipa-advise.1.gz
%files server-trust-ad
%{_sbindir}/ipa-adtrust-install
diff --git a/install/tools/Makefile.am b/install/tools/Makefile.am
index 7c553116cdfdce69e594f9d9a36e6bebcab5f692..13bf1f78ab159b2f57e72a21cc3a9a036826ba27 100644
--- a/install/tools/Makefile.am
+++ b/install/tools/Makefile.am
@@ -24,6 +24,7 @@ sbin_SCRIPTS = \
ipa-compliance \
ipa-backup \
ipa-restore \
+ ipa-advise \
$(NULL)
EXTRA_DIST = \
diff --git a/install/tools/ipa-advise b/install/tools/ipa-advise
new file mode 100755
index ..28a1ce101a5d6581395ad56347946fd6cc36e195
--- /dev/null
+++ b/install/tools/ipa-advise
@@ -0,0 +1,23 @@
+#! /usr/bin/python -E
+# Authors: Tomas Babej
+#
+# Copyright (C) 2013 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/20/2013 09:29 AM, Petr Spacek wrote: > On 19.6.2013 20:56, Alexander Bokovoy wrote: >> On Wed, 19 Jun 2013, Rob Crittenden wrote: >>> Tomas Babej wrote: [big snip] Providing new version which should address mentioned issues: - advice plugins now inherit directly from Plugin, initial approach via Method class was abandoned - new Namespace api.Advice collects all the advice plugins - tool renamed to ipa-advise to express a more general use case Additional improvements: - keywords are now generated out of Advice class's name, where underscores are replaced by hyphens - rewritten the example plugin in the docs, and provided more information there - instead of --setup option to provide configuration, ipa-advise takes one positional argument - renamed to ipa-advise Concerns: - man page might need more improvements I'll craft a design page for plugin authors, might be useful, even if the info is in the package docs. --- Here's a little preview: [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. /sbin/authconfig --enableldap --ldapserver=vm-001.idm.com --enablerfc2307bis --enablekrb5 [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig4 invalid 'setup': No instructions are available for 'fedora_authconfig4'. See the list of available configuration advices using the --list option. [tbabej@vm-001 ~]$ sudo ipa-advise - List of available advices - fedora-authconfig : Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. >>> >>> If it's just providing advise why does it need root access? Or is it >>> expected to provide advise based on current configuration? >> Exactly. Getting ranges, configured trusts, etc. Not all of that >> information may be available under non-privileged account, especially if >> somebody would decide to plug in advices for backup or CA >> handling/configuration of advanced features. > > I think that ipa-advise should not require root access *implicitly*. It would > prevent lower-level admins from ipa-advise tool. > > IMHO plugins should try to get required information and print an 'Insufficient > access rights, try it again as root/admin' error when appropriate. > > As a result, basic 'advices' (like recommended client configuration) will be > accessible anybody and special 'advices' (something related to AD trusts etc.) > will be accessible only to admins. +1 I think the reason why Tomas did it as root was that he can that autobind to the DS. But he could easily operate in 2 modes, similarly to ipa-ldap-updater and simply just auth wuth GSSAPI when he is not logged as a root. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 19.6.2013 20:56, Alexander Bokovoy wrote: On Wed, 19 Jun 2013, Rob Crittenden wrote: Tomas Babej wrote: [big snip] Providing new version which should address mentioned issues: - advice plugins now inherit directly from Plugin, initial approach via Method class was abandoned - new Namespace api.Advice collects all the advice plugins - tool renamed to ipa-advise to express a more general use case Additional improvements: - keywords are now generated out of Advice class's name, where underscores are replaced by hyphens - rewritten the example plugin in the docs, and provided more information there - instead of --setup option to provide configuration, ipa-advise takes one positional argument - renamed to ipa-advise Concerns: - man page might need more improvements I'll craft a design page for plugin authors, might be useful, even if the info is in the package docs. --- Here's a little preview: [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. /sbin/authconfig --enableldap --ldapserver=vm-001.idm.com --enablerfc2307bis --enablekrb5 [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig4 invalid 'setup': No instructions are available for 'fedora_authconfig4'. See the list of available configuration advices using the --list option. [tbabej@vm-001 ~]$ sudo ipa-advise - List of available advices - fedora-authconfig : Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. If it's just providing advise why does it need root access? Or is it expected to provide advise based on current configuration? Exactly. Getting ranges, configured trusts, etc. Not all of that information may be available under non-privileged account, especially if somebody would decide to plug in advices for backup or CA handling/configuration of advanced features. I think that ipa-advise should not require root access *implicitly*. It would prevent lower-level admins from ipa-advise tool. IMHO plugins should try to get required information and print an 'Insufficient access rights, try it again as root/admin' error when appropriate. As a result, basic 'advices' (like recommended client configuration) will be accessible anybody and special 'advices' (something related to AD trusts etc.) will be accessible only to admins. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On Wed, 19 Jun 2013, Tomas Babej wrote: [snip] Here's a little preview: [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. /sbin/authconfig --enableldap --ldapserver=vm-001.idm.com --enablerfc2307bis --enablekrb5 As the output is almost usable for cut&paste to run on client machines, may be prefix the description/instructions with #? Sure, that's a good idea. Then you could simply do [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig > script.sh [tbabej@vm-001 ~]$ scp script.sh vm-002:script.sh [tbabej@vm-002 ~]$ ./script.sh I'll include that change in the next revision Also, adding this to the wiki page for plugin authors as an convention wouldn't hurt. On the second thought, we run the risk of people mindlessly using the generated scripts without even looking at them though. Yep, that could happend. We can make those statements echoed on execution rather than put as comments: #!/bin/sh cat << "ADVISE_DESCRIPTION" Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. ADVISE_DESCRIPTION -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
[snip] Here's a little preview: [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. /sbin/authconfig --enableldap --ldapserver=vm-001.idm.com --enablerfc2307bis --enablekrb5 As the output is almost usable for cut&paste to run on client machines, may be prefix the description/instructions with #? Sure, that's a good idea. Then you could simply do [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig > script.sh [tbabej@vm-001 ~]$ scp script.sh vm-002:script.sh [tbabej@vm-002 ~]$ ./script.sh I'll include that change in the next revision Also, adding this to the wiki page for plugin authors as an convention wouldn't hurt. On the second thought, we run the risk of people mindlessly using the generated scripts without even looking at them though. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On Wed, 19 Jun 2013, Tomas Babej wrote: [big snip] Providing new version which should address mentioned issues: - advice plugins now inherit directly from Plugin, initial approach via Method class was abandoned - new Namespace api.Advice collects all the advice plugins - tool renamed to ipa-advise to express a more general use case Additional improvements: - keywords are now generated out of Advice class's name, where underscores are replaced by hyphens - rewritten the example plugin in the docs, and provided more information there - instead of --setup option to provide configuration, ipa-advise takes one positional argument - renamed to ipa-advise Concerns: - man page might need more improvements I'll craft a design page for plugin authors, might be useful, even if the info is in the package docs. --- Here's a little preview: [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. /sbin/authconfig --enableldap --ldapserver=vm-001.idm.com --enablerfc2307bis --enablekrb5 As the output is almost usable for cut&paste to run on client machines, may be prefix the description/instructions with #? -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On Wed, 19 Jun 2013, Rob Crittenden wrote: Tomas Babej wrote: [big snip] Providing new version which should address mentioned issues: - advice plugins now inherit directly from Plugin, initial approach via Method class was abandoned - new Namespace api.Advice collects all the advice plugins - tool renamed to ipa-advise to express a more general use case Additional improvements: - keywords are now generated out of Advice class's name, where underscores are replaced by hyphens - rewritten the example plugin in the docs, and provided more information there - instead of --setup option to provide configuration, ipa-advise takes one positional argument - renamed to ipa-advise Concerns: - man page might need more improvements I'll craft a design page for plugin authors, might be useful, even if the info is in the package docs. --- Here's a little preview: [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. /sbin/authconfig --enableldap --ldapserver=vm-001.idm.com --enablerfc2307bis --enablekrb5 [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig4 invalid 'setup': No instructions are available for 'fedora_authconfig4'. See the list of available configuration advices using the --list option. [tbabej@vm-001 ~]$ sudo ipa-advise - List of available advices - fedora-authconfig : Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. If it's just providing advise why does it need root access? Or is it expected to provide advise based on current configuration? Exactly. Getting ranges, configured trusts, etc. Not all of that information may be available under non-privileged account, especially if somebody would decide to plug in advices for backup or CA handling/configuration of advanced features. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 08:30 PM, Rob Crittenden wrote: Tomas Babej wrote: [big snip] Providing new version which should address mentioned issues: - advice plugins now inherit directly from Plugin, initial approach via Method class was abandoned - new Namespace api.Advice collects all the advice plugins - tool renamed to ipa-advise to express a more general use case Additional improvements: - keywords are now generated out of Advice class's name, where underscores are replaced by hyphens - rewritten the example plugin in the docs, and provided more information there - instead of --setup option to provide configuration, ipa-advise takes one positional argument - renamed to ipa-advise Concerns: - man page might need more improvements I'll craft a design page for plugin authors, might be useful, even if the info is in the package docs. --- Here's a little preview: [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. /sbin/authconfig --enableldap --ldapserver=vm-001.idm.com --enablerfc2307bis --enablekrb5 [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig4 invalid 'setup': No instructions are available for 'fedora_authconfig4'. See the list of available configuration advices using the --list option. [tbabej@vm-001 ~]$ sudo ipa-advise - List of available advices - fedora-authconfig : Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. If it's just providing advise why does it need root access? Or is it expected to provide advise based on current configuration? rob Original purpose I had in mind was to provide an option for plugin authors to connect via autobind to the LDAP. Now there's also a option of using our api commands, e.g. to read trust-related information out of the tree. However some parts of the tree are not exposed, so if some plugin needs to access information, about replica topology for example, I guess they would need to use this approach. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
Tomas Babej wrote: [big snip] Providing new version which should address mentioned issues: - advice plugins now inherit directly from Plugin, initial approach via Method class was abandoned - new Namespace api.Advice collects all the advice plugins - tool renamed to ipa-advise to express a more general use case Additional improvements: - keywords are now generated out of Advice class's name, where underscores are replaced by hyphens - rewritten the example plugin in the docs, and provided more information there - instead of --setup option to provide configuration, ipa-advise takes one positional argument - renamed to ipa-advise Concerns: - man page might need more improvements I'll craft a design page for plugin authors, might be useful, even if the info is in the package docs. --- Here's a little preview: [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. /sbin/authconfig --enableldap --ldapserver=vm-001.idm.com --enablerfc2307bis --enablekrb5 [tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig4 invalid 'setup': No instructions are available for 'fedora_authconfig4'. See the list of available configuration advices using the --list option. [tbabej@vm-001 ~]$ sudo ipa-advise - List of available advices - fedora-authconfig : Authconfig instructions for configuring Fedora 18/19 client with IPA server without use of SSSD. If it's just providing advise why does it need root access? Or is it expected to provide advise based on current configuration? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
[big snip]
Providing new version which should address mentioned issues:
- advice plugins now inherit directly from Plugin, initial approach
via Method class was abandoned
- new Namespace api.Advice collects all the advice plugins
- tool renamed to ipa-advise to express a more general use case
Additional improvements:
- keywords are now generated out of Advice class's name, where
underscores are replaced by hyphens
- rewritten the example plugin in the docs, and provided more
information there
- instead of --setup option to provide configuration, ipa-advise
takes one positional argument
- renamed to ipa-advise
Concerns:
- man page might need more improvements
I'll craft a design page for plugin authors, might be useful, even if
the info is in the package docs.
---
Here's a little preview:
[tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig
Authconfig instructions for configuring Fedora 18/19 client with IPA
server without use of SSSD.
/sbin/authconfig --enableldap --ldapserver=vm-001.idm.com
--enablerfc2307bis --enablekrb5
[tbabej@vm-001 ~]$ sudo ipa-advise fedora-authconfig4
invalid 'setup': No instructions are available for 'fedora_authconfig4'.
See the list of available configuration advices using the --list option.
[tbabej@vm-001 ~]$ sudo ipa-advise
-
List of available advices
-
fedora-authconfig : Authconfig instructions for configuring Fedora
18/19 client with IPA server without use of SSSD.
Tomas
Tomas
From 52e3afa05b98caa770534969d2ca5e7b9c2fd584 Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Mon, 10 Jun 2013 14:43:24 +0200
Subject: [PATCH] Provide ipa-advise tool
Provides a pluggable framework for generating configuration
scriptlets and instructions for various machine setups and use
cases.
Creates a new ipa-advise command, available to root user
on the IPA server.
Also provides an example configuration plugin,
config-fedora-authconfig.
https://fedorahosted.org/freeipa/ticket/3670
---
freeipa.spec.in | 4 +
install/tools/Makefile.am | 1 +
install/tools/ipa-advise | 23 +
install/tools/man/Makefile.am | 1 +
install/tools/man/ipa-advise.1| 45
ipalib/__init__.py| 7 +-
ipalib/frontend.py| 17 +++
ipalib/plugable.py| 2 +
ipaserver/advise/__init__.py | 22
ipaserver/advise/base.py | 142 ++
ipaserver/advise/plugins/__init__.py | 22
ipaserver/advise/plugins/fedora_authconfig.py | 40
make-lint | 2 +-
setup.py | 2 +
14 files changed, 327 insertions(+), 3 deletions(-)
create mode 100755 install/tools/ipa-advise
create mode 100644 install/tools/man/ipa-advise.1
create mode 100644 ipaserver/advise/__init__.py
create mode 100644 ipaserver/advise/base.py
create mode 100644 ipaserver/advise/plugins/__init__.py
create mode 100644 ipaserver/advise/plugins/fedora_authconfig.py
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 239811ac26aa84e1928cefb9c3adac58326ad9a7..2bd2ca5558d189e7f64f3e575bae53ba64614a5a 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -598,6 +598,7 @@ fi
%{_sbindir}/ipactl
%{_sbindir}/ipa-upgradeconfig
%{_sbindir}/ipa-compliance
+%{_sbindir}/ipa-advise
%{_libexecdir}/certmonger/dogtag-ipa-retrieve-agent-submit
%{_libexecdir}/ipa-otpd
%{_sysconfdir}/cron.d/ipa-compliance
@@ -614,6 +615,8 @@ fi
%dir %{python_sitelib}/ipaserver
%dir %{python_sitelib}/ipaserver/install
%dir %{python_sitelib}/ipaserver/install/plugins
+%dir %{python_sitelib}/ipaserver/advise
+%dir %{python_sitelib}/ipaserver/advise/plugins
%dir %{python_sitelib}/ipaserver/plugins
%dir %{_libdir}/ipa/certmonger
%attr(755,root,root) %{_libdir}/ipa/certmonger/*
@@ -730,6 +733,7 @@ fi
%{_mandir}/man1/ipa-compliance.1.gz
%{_mandir}/man1/ipa-backup.1.gz
%{_mandir}/man1/ipa-restore.1.gz
+%{_mandir}/man1/ipa-advise.1.gz
%files server-trust-ad
%{_sbindir}/ipa-adtrust-install
diff --git a/install/tools/Makefile.am b/install/tools/Makefile.am
index 7c553116cdfdce69e594f9d9a36e6bebcab5f692..13bf1f78ab159b2f57e72a21cc3a9a036826ba27 100644
--- a/install/tools/Makefile.am
+++ b/install/tools/Makefile.am
@@ -24,6 +24,7 @@ sbin_SCRIPTS = \
ipa-compliance \
ipa-backup \
ipa-restore \
+ ipa-advise \
$(NULL)
EXTRA_DIST = \
diff --git a/install/tools/ipa-advise b/install/tools/ipa-advise
new file mode 100755
index ..28a1ce101a5d6581395ad563479
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On Wed, 19 Jun 2013, Dmitri Pal wrote: On 06/19/2013 12:46 PM, Alexander Bokovoy wrote: (please trim conversations) On Wed, 19 Jun 2013, Tomas Babej wrote: So if I want an advise about Solaris 11 client configuration would it look like this? ipa-advise config --client --distro=solaris --version=11 or ipa-advise client-config-solrais-11 The latter. Tomas My point is that if the topics would be long and there will be many of them we should have a naming convention for them. Sure, but I am not so certain whether we can come up with anything reasonable, that can capture all the use cases and be simple enough at the same time. Making plugins to provide their activities named as -- will make it easy to group and use: ipa-advise config-solaris11-padl config-freebsd7-padl config-aix63-native list help setup-ipa-trust2ad setup-ipa-dnsdelegation and so on. ipa-advise list Yes this is exactly what I was talking about. Should be a part of design page in the naming convention section IMO. I've added it there: http://www.freeipa.org/page/V3/Serving_legacy_clients_for_trusts#Major_configuration_options_and_enablement -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 12:46 PM, Alexander Bokovoy wrote: > (please trim conversations) > > On Wed, 19 Jun 2013, Tomas Babej wrote: > So if I want an advise about Solaris 11 client configuration would it > look like this? > > ipa-advise config --client --distro=solaris --version=11 > > or > > ipa-advise client-config-solrais-11 > > The latter. Tomas >>> My point is that if the topics would be long and there will be many of >>> them we should have a naming convention for them. >>> >> >> Sure, but I am not so certain whether we can come up with anything >> reasonable, that can capture all the use >> cases and be simple enough at the same time. > Making plugins to provide their activities named as > > -- > > will make it easy to group and use: > >ipa-advise config-solaris11-padl > config-freebsd7-padl > config-aix63-native > list > help > setup-ipa-trust2ad > setup-ipa-dnsdelegation > > and so on. > >ipa-advise list Yes this is exactly what I was talking about. Should be a part of design page in the naming convention section IMO. > > would show all plugins (filtering itself, i.e. list and help plugin) > with their short descriptions. > >> E.g., somebody might provide a plugin to generate records for DNS >> zone delegation (using Petr's idea here). Such a plugin does not >> really fit into client|server-os-version schema. >> >> We can probably start naming plugins in a fairly systematic way, and >> ensure that we will not create mess in the future via review process. > Sure. > > -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 12:29 PM, Tomas Babej wrote: > On 06/19/2013 06:19 PM, Dmitri Pal wrote: >> On 06/19/2013 12:18 PM, Tomas Babej wrote: >>> On 06/19/2013 06:13 PM, Dmitri Pal wrote: On 06/19/2013 10:46 AM, Simo Sorce wrote: > - Original Message - >> On 06/19/2013 03:03 PM, Alexander Bokovoy wrote: >>> On Wed, 19 Jun 2013, Jan Cholasta wrote: On 19.6.2013 14:47, Dmitri Pal wrote: > On 06/19/2013 08:02 AM, Tomas Babej wrote: >> Do you have something particular in mind? >> >> Tomas >> >> ___ >> Freeipa-devel mailing list >> Freeipa-devel@redhat.com >> https://www.redhat.com/mailman/listinfo/freeipa-devel > ipa-config-advisor ? > IMO we should stick to a verb in the name, so ipa-config-advise. >>> Then it is better to be simpler, ipa-advise is a nice name. >> Isn't that too simple? Are you trying to create an all knowing >> Siri-like >> advisor for IPA? If I am a user, I would really not know what >> "ipa-advise" >> means and what advise could it give to me. >> >> # ipa-advise "what pair of socks should I take for today?" >> >> ipa-config-advise was better IMHO. > then as soon as you need to 'advise' on something that is not config > related it becomes akward, also ipa-config-advise is much longer to > type and 'config' doesn't really add much. > > As for the user 'man ipa-advise' will neatly explain what it will > advise about, I think that is sufficient. > Nobody will expect 'ipa'-advise to provide info about non-ipa > related stuff anyway. > > As for the actual command line options I do wonder as well why you > need a --setup or --about option at all. > > ipa-advise 'topic' is sufficient imo. > > options that may make sense are things like --verbose so that you > can have a small excerpt with the short form and a much longer text > with --verbose if necessary. Although maybe we should just reference > man pages for longer text and not try to create a new manpage > substitute, we certainly should always provide all the content in > man pages first. > > Simo. > So if I want an advise about Solaris 11 client configuration would it look like this? ipa-advise config --client --distro=solaris --version=11 or ipa-advise client-config-solrais-11 >>> The latter. >>> >>> Tomas >> My point is that if the topics would be long and there will be many of >> them we should have a naming convention for them. >> > > Sure, but I am not so certain whether we can come up with anything > reasonable, that can capture all the use > cases and be simple enough at the same time. > > E.g., somebody might provide a plugin to generate records for DNS zone > delegation (using Petr's idea here). Such a plugin does not really fit > into client|server-os-version schema. > > We can probably start naming plugins in a fairly systematic way, and > ensure that we will not create mess in the future via review process. > > Tomas No I am talking about naming conventions regarding : spaces, dashes, capitalization, verb use, noun use etc. So that we do not have "client-config-solrais-11" and "ConfiguringHPUX_11.23withKerberos" -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
(please trim conversations) On Wed, 19 Jun 2013, Tomas Babej wrote: So if I want an advise about Solaris 11 client configuration would it look like this? ipa-advise config --client --distro=solaris --version=11 or ipa-advise client-config-solrais-11 The latter. Tomas My point is that if the topics would be long and there will be many of them we should have a naming convention for them. Sure, but I am not so certain whether we can come up with anything reasonable, that can capture all the use cases and be simple enough at the same time. Making plugins to provide their activities named as -- will make it easy to group and use: ipa-advise config-solaris11-padl config-freebsd7-padl config-aix63-native list help setup-ipa-trust2ad setup-ipa-dnsdelegation and so on. ipa-advise list would show all plugins (filtering itself, i.e. list and help plugin) with their short descriptions. E.g., somebody might provide a plugin to generate records for DNS zone delegation (using Petr's idea here). Such a plugin does not really fit into client|server-os-version schema. We can probably start naming plugins in a fairly systematic way, and ensure that we will not create mess in the future via review process. Sure. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 06:19 PM, Dmitri Pal wrote: On 06/19/2013 12:18 PM, Tomas Babej wrote: On 06/19/2013 06:13 PM, Dmitri Pal wrote: On 06/19/2013 10:46 AM, Simo Sorce wrote: - Original Message - On 06/19/2013 03:03 PM, Alexander Bokovoy wrote: On Wed, 19 Jun 2013, Jan Cholasta wrote: On 19.6.2013 14:47, Dmitri Pal wrote: On 06/19/2013 08:02 AM, Tomas Babej wrote: Do you have something particular in mind? Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ipa-config-advisor ? IMO we should stick to a verb in the name, so ipa-config-advise. Then it is better to be simpler, ipa-advise is a nice name. Isn't that too simple? Are you trying to create an all knowing Siri-like advisor for IPA? If I am a user, I would really not know what "ipa-advise" means and what advise could it give to me. # ipa-advise "what pair of socks should I take for today?" ipa-config-advise was better IMHO. then as soon as you need to 'advise' on something that is not config related it becomes akward, also ipa-config-advise is much longer to type and 'config' doesn't really add much. As for the user 'man ipa-advise' will neatly explain what it will advise about, I think that is sufficient. Nobody will expect 'ipa'-advise to provide info about non-ipa related stuff anyway. As for the actual command line options I do wonder as well why you need a --setup or --about option at all. ipa-advise 'topic' is sufficient imo. options that may make sense are things like --verbose so that you can have a small excerpt with the short form and a much longer text with --verbose if necessary. Although maybe we should just reference man pages for longer text and not try to create a new manpage substitute, we certainly should always provide all the content in man pages first. Simo. So if I want an advise about Solaris 11 client configuration would it look like this? ipa-advise config --client --distro=solaris --version=11 or ipa-advise client-config-solrais-11 The latter. Tomas My point is that if the topics would be long and there will be many of them we should have a naming convention for them. Sure, but I am not so certain whether we can come up with anything reasonable, that can capture all the use cases and be simple enough at the same time. E.g., somebody might provide a plugin to generate records for DNS zone delegation (using Petr's idea here). Such a plugin does not really fit into client|server-os-version schema. We can probably start naming plugins in a fairly systematic way, and ensure that we will not create mess in the future via review process. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 12:18 PM, Tomas Babej wrote: > On 06/19/2013 06:13 PM, Dmitri Pal wrote: >> On 06/19/2013 10:46 AM, Simo Sorce wrote: >>> - Original Message - On 06/19/2013 03:03 PM, Alexander Bokovoy wrote: > On Wed, 19 Jun 2013, Jan Cholasta wrote: >> On 19.6.2013 14:47, Dmitri Pal wrote: >>> On 06/19/2013 08:02 AM, Tomas Babej wrote: Do you have something particular in mind? Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel >>> ipa-config-advisor ? >>> >> IMO we should stick to a verb in the name, so ipa-config-advise. > Then it is better to be simpler, ipa-advise is a nice name. Isn't that too simple? Are you trying to create an all knowing Siri-like advisor for IPA? If I am a user, I would really not know what "ipa-advise" means and what advise could it give to me. # ipa-advise "what pair of socks should I take for today?" ipa-config-advise was better IMHO. >>> then as soon as you need to 'advise' on something that is not config >>> related it becomes akward, also ipa-config-advise is much longer to >>> type and 'config' doesn't really add much. >>> >>> As for the user 'man ipa-advise' will neatly explain what it will >>> advise about, I think that is sufficient. >>> Nobody will expect 'ipa'-advise to provide info about non-ipa >>> related stuff anyway. >>> >>> As for the actual command line options I do wonder as well why you >>> need a --setup or --about option at all. >>> >>> ipa-advise 'topic' is sufficient imo. >>> >>> options that may make sense are things like --verbose so that you >>> can have a small excerpt with the short form and a much longer text >>> with --verbose if necessary. Although maybe we should just reference >>> man pages for longer text and not try to create a new manpage >>> substitute, we certainly should always provide all the content in >>> man pages first. >>> >>> Simo. >>> >> So if I want an advise about Solaris 11 client configuration would it >> look like this? >> >> ipa-advise config --client --distro=solaris --version=11 >> >> or >> >> ipa-advise client-config-solrais-11 >> >> > > The latter. > > Tomas My point is that if the topics would be long and there will be many of them we should have a naming convention for them. -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 06:13 PM, Dmitri Pal wrote: On 06/19/2013 10:46 AM, Simo Sorce wrote: - Original Message - On 06/19/2013 03:03 PM, Alexander Bokovoy wrote: On Wed, 19 Jun 2013, Jan Cholasta wrote: On 19.6.2013 14:47, Dmitri Pal wrote: On 06/19/2013 08:02 AM, Tomas Babej wrote: Do you have something particular in mind? Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ipa-config-advisor ? IMO we should stick to a verb in the name, so ipa-config-advise. Then it is better to be simpler, ipa-advise is a nice name. Isn't that too simple? Are you trying to create an all knowing Siri-like advisor for IPA? If I am a user, I would really not know what "ipa-advise" means and what advise could it give to me. # ipa-advise "what pair of socks should I take for today?" ipa-config-advise was better IMHO. then as soon as you need to 'advise' on something that is not config related it becomes akward, also ipa-config-advise is much longer to type and 'config' doesn't really add much. As for the user 'man ipa-advise' will neatly explain what it will advise about, I think that is sufficient. Nobody will expect 'ipa'-advise to provide info about non-ipa related stuff anyway. As for the actual command line options I do wonder as well why you need a --setup or --about option at all. ipa-advise 'topic' is sufficient imo. options that may make sense are things like --verbose so that you can have a small excerpt with the short form and a much longer text with --verbose if necessary. Although maybe we should just reference man pages for longer text and not try to create a new manpage substitute, we certainly should always provide all the content in man pages first. Simo. So if I want an advise about Solaris 11 client configuration would it look like this? ipa-advise config --client --distro=solaris --version=11 or ipa-advise client-config-solrais-11 The latter. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 10:46 AM, Simo Sorce wrote: > - Original Message - >> On 06/19/2013 03:03 PM, Alexander Bokovoy wrote: >>> On Wed, 19 Jun 2013, Jan Cholasta wrote: On 19.6.2013 14:47, Dmitri Pal wrote: > On 06/19/2013 08:02 AM, Tomas Babej wrote: >> Do you have something particular in mind? >> >> Tomas >> >> ___ >> Freeipa-devel mailing list >> Freeipa-devel@redhat.com >> https://www.redhat.com/mailman/listinfo/freeipa-devel > > ipa-config-advisor ? > IMO we should stick to a verb in the name, so ipa-config-advise. >>> Then it is better to be simpler, ipa-advise is a nice name. >> Isn't that too simple? Are you trying to create an all knowing Siri-like >> advisor for IPA? If I am a user, I would really not know what "ipa-advise" >> means and what advise could it give to me. >> >> # ipa-advise "what pair of socks should I take for today?" >> >> ipa-config-advise was better IMHO. > then as soon as you need to 'advise' on something that is not config related > it becomes akward, also ipa-config-advise is much longer to type and 'config' > doesn't really add much. > > As for the user 'man ipa-advise' will neatly explain what it will advise > about, I think that is sufficient. > Nobody will expect 'ipa'-advise to provide info about non-ipa related stuff > anyway. > > As for the actual command line options I do wonder as well why you need a > --setup or --about option at all. > > ipa-advise 'topic' is sufficient imo. > > options that may make sense are things like --verbose so that you can have a > small excerpt with the short form and a much longer text with --verbose if > necessary. Although maybe we should just reference man pages for longer text > and not try to create a new manpage substitute, we certainly should always > provide all the content in man pages first. > > Simo. > So if I want an advise about Solaris 11 client configuration would it look like this? ipa-advise config --client --distro=solaris --version=11 or ipa-advise client-config-solrais-11 -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
- Original Message - > On 06/19/2013 03:03 PM, Alexander Bokovoy wrote: > > On Wed, 19 Jun 2013, Jan Cholasta wrote: > >> On 19.6.2013 14:47, Dmitri Pal wrote: > >>> On 06/19/2013 08:02 AM, Tomas Babej wrote: > Do you have something particular in mind? > > Tomas > > ___ > Freeipa-devel mailing list > Freeipa-devel@redhat.com > https://www.redhat.com/mailman/listinfo/freeipa-devel > >>> > >>> > >>> ipa-config-advisor ? > >>> > >> > >> IMO we should stick to a verb in the name, so ipa-config-advise. > > Then it is better to be simpler, ipa-advise is a nice name. > > Isn't that too simple? Are you trying to create an all knowing Siri-like > advisor for IPA? If I am a user, I would really not know what "ipa-advise" > means and what advise could it give to me. > > # ipa-advise "what pair of socks should I take for today?" > > ipa-config-advise was better IMHO. then as soon as you need to 'advise' on something that is not config related it becomes akward, also ipa-config-advise is much longer to type and 'config' doesn't really add much. As for the user 'man ipa-advise' will neatly explain what it will advise about, I think that is sufficient. Nobody will expect 'ipa'-advise to provide info about non-ipa related stuff anyway. As for the actual command line options I do wonder as well why you need a --setup or --about option at all. ipa-advise 'topic' is sufficient imo. options that may make sense are things like --verbose so that you can have a small excerpt with the short form and a much longer text with --verbose if necessary. Although maybe we should just reference man pages for longer text and not try to create a new manpage substitute, we certainly should always provide all the content in man pages first. Simo. -- Simo Sorce * Red Hat, Inc. * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 19.6.2013 16:09, Tomas Babej wrote: On 06/19/2013 03:03 PM, Alexander Bokovoy wrote: On Wed, 19 Jun 2013, Jan Cholasta wrote: On 19.6.2013 14:47, Dmitri Pal wrote: On 06/19/2013 08:02 AM, Tomas Babej wrote: Do you have something particular in mind? Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ipa-config-advisor ? IMO we should stick to a verb in the name, so ipa-config-advise. Then it is better to be simpler, ipa-advise is a nice name. In the work I have in progress right now, I changed the --setup option to --about, so now it is: # ipa-advise --about fedora-authconfig Why is the option necessary? Why not make it just "ipa-advise fedora-authconfig"? -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 03:03 PM, Alexander Bokovoy wrote: > On Wed, 19 Jun 2013, Jan Cholasta wrote: >> On 19.6.2013 14:47, Dmitri Pal wrote: >>> On 06/19/2013 08:02 AM, Tomas Babej wrote: Do you have something particular in mind? Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel >>> >>> >>> ipa-config-advisor ? >>> >> >> IMO we should stick to a verb in the name, so ipa-config-advise. > Then it is better to be simpler, ipa-advise is a nice name. Isn't that too simple? Are you trying to create an all knowing Siri-like advisor for IPA? If I am a user, I would really not know what "ipa-advise" means and what advise could it give to me. # ipa-advise "what pair of socks should I take for today?" ipa-config-advise was better IMHO. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 03:03 PM, Alexander Bokovoy wrote: On Wed, 19 Jun 2013, Jan Cholasta wrote: On 19.6.2013 14:47, Dmitri Pal wrote: On 06/19/2013 08:02 AM, Tomas Babej wrote: Do you have something particular in mind? Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ipa-config-advisor ? IMO we should stick to a verb in the name, so ipa-config-advise. Then it is better to be simpler, ipa-advise is a nice name. In the work I have in progress right now, I changed the --setup option to --about, so now it is: # ipa-advise --about fedora-authconfig Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On Wed, 19 Jun 2013, Jan Cholasta wrote: On 19.6.2013 14:47, Dmitri Pal wrote: On 06/19/2013 08:02 AM, Tomas Babej wrote: Do you have something particular in mind? Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ipa-config-advisor ? IMO we should stick to a verb in the name, so ipa-config-advise. Then it is better to be simpler, ipa-advise is a nice name. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 19.6.2013 14:47, Dmitri Pal wrote: On 06/19/2013 08:02 AM, Tomas Babej wrote: Do you have something particular in mind? Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ipa-config-advisor ? IMO we should stick to a verb in the name, so ipa-config-advise. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 08:02 AM, Tomas Babej wrote: > On 06/19/2013 01:31 PM, Petr Spacek wrote: >> On 19.6.2013 09:48, Tomas Babej wrote: >>> Hi, >>> >>> Provides a pluggable framework for generating configuration >>> scriptlets and instructions for various machine setups. >>> >>> Creates a new ipa-client-advise command, available to root user >>> on the IPA server. >>> >>> Also provides an example configuration plugin. >>> >>> https://fedorahosted.org/freeipa/ticket/3670 >> >> BTW, shouldn't we rename the tool to 'ipa-advise'? It is pluggable >> and I can imagine that it will be extended to generate/produce >> various recommendations for clients & servers ... >> > > I do not see any objections. > > Indeed the current name may be somewhat misleading given that it is > meant to be run on the server. > > Do you have something particular in mind? > > Tomas > > ___ > Freeipa-devel mailing list > Freeipa-devel@redhat.com > https://www.redhat.com/mailman/listinfo/freeipa-devel ipa-config-advisor ? -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 06/19/2013 01:31 PM, Petr Spacek wrote: On 19.6.2013 09:48, Tomas Babej wrote: Hi, Provides a pluggable framework for generating configuration scriptlets and instructions for various machine setups. Creates a new ipa-client-advise command, available to root user on the IPA server. Also provides an example configuration plugin. https://fedorahosted.org/freeipa/ticket/3670 BTW, shouldn't we rename the tool to 'ipa-advise'? It is pluggable and I can imagine that it will be extended to generate/produce various recommendations for clients & servers ... I do not see any objections. Indeed the current name may be somewhat misleading given that it is meant to be run on the server. Do you have something particular in mind? Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On 19.6.2013 09:48, Tomas Babej wrote: Hi, Provides a pluggable framework for generating configuration scriptlets and instructions for various machine setups. Creates a new ipa-client-advise command, available to root user on the IPA server. Also provides an example configuration plugin. https://fedorahosted.org/freeipa/ticket/3670 BTW, shouldn't we rename the tool to 'ipa-advise'? It is pluggable and I can imagine that it will be extended to generate/produce various recommendations for clients & servers ... -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
Hi, On 19.6.2013 09:48, Tomas Babej wrote: Hi, Provides a pluggable framework for generating configuration scriptlets and instructions for various machine setups. Creates a new ipa-client-advise command, available to root user on the IPA server. Also provides an example configuration plugin. I don't like how you abuse our object model in this patch. For example, why does Configuration inherit from Method? It does not represent method of any object, it doesn't even represent a runnable command. I see you added an artificial advise object, which uses the ldap2 backend, but doesn't actually use LDAP, this is also ugly. Please inherit from Plugin directly and create a new API namespace for advises instead. And don't call the class Configuration, it's misleading (Advise or Advisory is better IMHO). Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
On Wed, 19 Jun 2013, Tomas Babej wrote: Hi, Provides a pluggable framework for generating configuration scriptlets and instructions for various machine setups. Creates a new ipa-client-advise command, available to root user on the IPA server. Also provides an example configuration plugin. https://fedorahosted.org/freeipa/ticket/3670 In general looks fine. Manual page is copied from ipa-restore and has its content irrelevant to ipa-client-advise. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0072] Provide ipa-client-advise tool
Hi,
Provides a pluggable framework for generating configuration
scriptlets and instructions for various machine setups.
Creates a new ipa-client-advise command, available to root user
on the IPA server.
Also provides an example configuration plugin.
https://fedorahosted.org/freeipa/ticket/3670
Tomas
From 6cb034b8facf98e45ccad54142ff3e0f9ffb08cb Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Mon, 10 Jun 2013 14:43:24 +0200
Subject: [PATCH] Provide ipa-client-advise tool
Provides a pluggable framework for generating configuration
scriptlets and instructions for various machine setups.
Creates a new ipa-client-advise command, available to root user
on the IPA server.
Also provides an example configuration plugin.
https://fedorahosted.org/freeipa/ticket/3670
---
freeipa.spec.in | 4 +
install/tools/Makefile.am | 1 +
install/tools/ipa-client-advise | 23 +
install/tools/man/Makefile.am | 1 +
install/tools/man/ipa-client-advise.1 | 72 ++
ipalib/frontend.py| 19
ipalib/plugable.py| 2 +
ipaserver/advise/__init__.py | 22 +
ipaserver/advise/base.py | 132 ++
ipaserver/advise/plugins/__init__.py | 22 +
ipaserver/advise/plugins/fedora_authconfig.py | 39
setup.py | 2 +
12 files changed, 339 insertions(+)
create mode 100755 install/tools/ipa-client-advise
create mode 100644 install/tools/man/ipa-client-advise.1
create mode 100644 ipaserver/advise/__init__.py
create mode 100644 ipaserver/advise/base.py
create mode 100644 ipaserver/advise/plugins/__init__.py
create mode 100644 ipaserver/advise/plugins/fedora_authconfig.py
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 4a38e8785903faa5f38295534432a8f679671136..4e37f7f2c4a78cfe295c799efda80342efdc7ba4 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -636,6 +636,7 @@ fi
%{_sbindir}/ipactl
%{_sbindir}/ipa-upgradeconfig
%{_sbindir}/ipa-compliance
+%{_sbindir}/ipa-client-advise
%{_libexecdir}/certmonger/dogtag-ipa-retrieve-agent-submit
%{_libexecdir}/ipa-otpd
%{_sysconfdir}/cron.d/ipa-compliance
@@ -652,6 +653,8 @@ fi
%dir %{python_sitelib}/ipaserver
%dir %{python_sitelib}/ipaserver/install
%dir %{python_sitelib}/ipaserver/install/plugins
+%dir %{python_sitelib}/ipaserver/advise
+%dir %{python_sitelib}/ipaserver/advise/plugins
%dir %{python_sitelib}/ipaserver/plugins
%dir %{_libdir}/ipa/certmonger
%attr(755,root,root) %{_libdir}/ipa/certmonger/*
@@ -770,6 +773,7 @@ fi
%{_mandir}/man1/ipa-compliance.1.gz
%{_mandir}/man1/ipa-backup.1.gz
%{_mandir}/man1/ipa-restore.1.gz
+%{_mandir}/man1/ipa-client-advise.1.gz
%files server-selinux
%defattr(-,root,root,-)
diff --git a/install/tools/Makefile.am b/install/tools/Makefile.am
index 7c553116cdfdce69e594f9d9a36e6bebcab5f692..90d5a5382c23893d55b0297a73e5f01ad312f8d4 100644
--- a/install/tools/Makefile.am
+++ b/install/tools/Makefile.am
@@ -24,6 +24,7 @@ sbin_SCRIPTS = \
ipa-compliance \
ipa-backup \
ipa-restore \
+ ipa-client-advise \
$(NULL)
EXTRA_DIST = \
diff --git a/install/tools/ipa-client-advise b/install/tools/ipa-client-advise
new file mode 100755
index ..6440503243bd124584eca55cf286b101ec861c16
--- /dev/null
+++ b/install/tools/ipa-client-advise
@@ -0,0 +1,23 @@
+#! /usr/bin/python -E
+# Authors: Tomas Babej
+#
+# Copyright (C) 2013 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
