[Freeipa-devel] [freeipa PR#112][synchronized] The first jab at fixing https://fedorahosted.org/freeipa/ticket/5809

[martbab]

   URL: https://github.com/freeipa/freeipa/pull/112
Author: martbab
 Title: #112: The first jab at fixing 
https://fedorahosted.org/freeipa/ticket/5809
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/112/head:pr112
git checkout pr112
From 6db1f860dd13d90b039e71a08804bdd1f7f5a8fd Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 23 Sep 2016 15:53:41 +0200
Subject: [PATCH 1/2] Move character escaping function to ipautil

Functions `escape_seq` and `unescape_seq` have a generic use-case so it makes
sense to move them from `kerberos` to ipautil module so that other modules can
reuse them more readily.

https://fedorahosted.org/freeipa/ticket/5809
---
 ipapython/ipautil.py  | 27 +++
 ipapython/kerberos.py | 29 ++---
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 62d029d..fac76d1 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1484,3 +1484,30 @@ def is_fips_enabled():
 # Consider that the host is not fips-enabled if the file does not exist
 pass
 return False
+
+
+def unescape_seq(seq, *args):
+"""
+unescape (remove '\\') all occurences of sequence in input strings.
+
+:param seq: sequence to unescape
+:param args: input string to process
+
+:returns: tuple of strings with unescaped sequences
+"""
+unescape_re = re.compile(r'\\{}'.format(seq))
+
+return tuple(re.sub(unescape_re, seq, a) for a in args)
+
+
+def escape_seq(seq, *args):
+"""
+escape (prepend '\\') all occurences of sequence in input strings
+
+:param seq: sequence to escape
+:param args: input string to process
+
+:returns: tuple of strings with escaped sequences
+"""
+
+return tuple(a.replace(seq, u'\\{}'.format(seq)) for a in args)
diff --git a/ipapython/kerberos.py b/ipapython/kerberos.py
index 298dbf1..a8ebc04 100644
--- a/ipapython/kerberos.py
+++ b/ipapython/kerberos.py
@@ -8,6 +8,8 @@
 import re
 import six
 
+from ipapython.ipautil import escape_seq, unescape_seq
+
 if six.PY3:
 unicode = str
 
@@ -58,33 +60,6 @@ def split_principal_name(principal_name):
 return tuple(COMPONENT_SPLIT_RE.split(principal_name))
 
 
-def unescape_seq(seq, *args):
-"""
-unescape (remove '\\') all occurences of sequence in input strings.
-
-:param seq: sequence to unescape
-:param args: input string to process
-
-:returns: tuple of strings with unescaped sequences
-"""
-unescape_re = re.compile(r'\\{}'.format(seq))
-
-return tuple(re.sub(unescape_re, seq, a) for a in args)
-
-
-def escape_seq(seq, *args):
-"""
-escape (prepend '\\') all occurences of sequence in input strings
-
-:param seq: sequence to escape
-:param args: input string to process
-
-:returns: tuple of strings with escaped sequences
-"""
-
-return tuple(a.replace(seq, u'\\{}'.format(seq)) for a in args)
-
-
 @six.python_2_unicode_compatible
 class Principal(object):
 """

From 49a3535e0eff2a9ed2c3cbc36adff03c96730f69 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 23 Sep 2016 15:56:46 +0200
Subject: [PATCH 2/2] mod_nss: use more robust quoting of NSSNickname directive

The code which handles configuration of mod_nss module must be more robust
when handling NSS nicknames generated from subject names containing quoted RDN
values.

https://fedorahosted.org/freeipa/ticket/5809
---
 ipaserver/install/httpinstance.py |  3 ++-
 ipaserver/install/installutils.py | 42 +--
 2 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 00f8901..7914f4c 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -263,7 +263,8 @@ def __set_mod_nss_port(self):
 print("Updating port in %s failed." % paths.HTTPD_NSS_CONF)
 
 def __set_mod_nss_nickname(self, nickname):
-installutils.set_directive(paths.HTTPD_NSS_CONF, 'NSSNickname', nickname)
+installutils.set_directive(
+paths.HTTPD_NSS_CONF, 'NSSNickname', nickname, quote_char="'")
 
 def set_mod_nss_protocol(self):
 installutils.set_directive(paths.HTTPD_NSS_CONF, 'NSSProtocol', 'TLSv1.0,TLSv1.1,TLSv1.2', False)
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index bf179a2..2e4fc58 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -376,13 +376,35 @@ def update_file(filename, orig, subst):
 print("File %s doesn't exist." % filename)
 return 1
 
-def set_directive(filename, directive, value, quotes=True, separator=' '):
+
+def set_directive(filename, directive, value, quotes=True, separator=' ',
+  quote_char='\"'):
 """Set a 

[Freeipa-devel] [freeipa PR#112][opened] The first jab at fixing https://fedorahosted.org/freeipa/ticket/5809

[martbab]

   URL: https://github.com/freeipa/freeipa/pull/112
Author: martbab
 Title: #112: The first jab at fixing 
https://fedorahosted.org/freeipa/ticket/5809
Action: opened

PR body:
"""
There are two ways to fix the issue reported in the ticket:

1.) Make certificate handling code to generate nicknames that do not break
existing implementation of `installutils.set_directive`

2.) Extend the quoting abilities of the function so that it is less fragile
when encoding more funky values such as quoted RDNs

This PR opts for option 2.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/112/head:pr112
git checkout pr112
From 6db1f860dd13d90b039e71a08804bdd1f7f5a8fd Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 23 Sep 2016 15:53:41 +0200
Subject: [PATCH 1/2] Move character escaping function to ipautil

Functions `escape_seq` and `unescape_seq` have a generic use-case so it makes
sense to move them from `kerberos` to ipautil module so that other modules can
reuse them more readily.

https://fedorahosted.org/freeipa/ticket/5809
---
 ipapython/ipautil.py  | 27 +++
 ipapython/kerberos.py | 29 ++---
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 62d029d..fac76d1 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1484,3 +1484,30 @@ def is_fips_enabled():
 # Consider that the host is not fips-enabled if the file does not exist
 pass
 return False
+
+
+def unescape_seq(seq, *args):
+"""
+unescape (remove '\\') all occurences of sequence in input strings.
+
+:param seq: sequence to unescape
+:param args: input string to process
+
+:returns: tuple of strings with unescaped sequences
+"""
+unescape_re = re.compile(r'\\{}'.format(seq))
+
+return tuple(re.sub(unescape_re, seq, a) for a in args)
+
+
+def escape_seq(seq, *args):
+"""
+escape (prepend '\\') all occurences of sequence in input strings
+
+:param seq: sequence to escape
+:param args: input string to process
+
+:returns: tuple of strings with escaped sequences
+"""
+
+return tuple(a.replace(seq, u'\\{}'.format(seq)) for a in args)
diff --git a/ipapython/kerberos.py b/ipapython/kerberos.py
index 298dbf1..a8ebc04 100644
--- a/ipapython/kerberos.py
+++ b/ipapython/kerberos.py
@@ -8,6 +8,8 @@
 import re
 import six
 
+from ipapython.ipautil import escape_seq, unescape_seq
+
 if six.PY3:
 unicode = str
 
@@ -58,33 +60,6 @@ def split_principal_name(principal_name):
 return tuple(COMPONENT_SPLIT_RE.split(principal_name))
 
 
-def unescape_seq(seq, *args):
-"""
-unescape (remove '\\') all occurences of sequence in input strings.
-
-:param seq: sequence to unescape
-:param args: input string to process
-
-:returns: tuple of strings with unescaped sequences
-"""
-unescape_re = re.compile(r'\\{}'.format(seq))
-
-return tuple(re.sub(unescape_re, seq, a) for a in args)
-
-
-def escape_seq(seq, *args):
-"""
-escape (prepend '\\') all occurences of sequence in input strings
-
-:param seq: sequence to escape
-:param args: input string to process
-
-:returns: tuple of strings with escaped sequences
-"""
-
-return tuple(a.replace(seq, u'\\{}'.format(seq)) for a in args)
-
-
 @six.python_2_unicode_compatible
 class Principal(object):
 """

From 685e48ef2fca9e0bccacb789d8c15ce367f9b846 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 23 Sep 2016 15:56:46 +0200
Subject: [PATCH 2/2] mod_nss: use more robust quoting of NSSNickname directive

The code which handles configuration of mod_nss module must be more robust
when handling NSS nicknames generated from subject names containing quoted RDN
values.

https://fedorahosted.org/freeipa/ticket/5809
---
 ipaserver/install/httpinstance.py |  3 ++-
 ipaserver/install/installutils.py | 41 ---
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 00f8901..7914f4c 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -263,7 +263,8 @@ def __set_mod_nss_port(self):
 print("Updating port in %s failed." % paths.HTTPD_NSS_CONF)
 
 def __set_mod_nss_nickname(self, nickname):
-installutils.set_directive(paths.HTTPD_NSS_CONF, 'NSSNickname', nickname)
+installutils.set_directive(
+paths.HTTPD_NSS_CONF, 'NSSNickname', nickname, quote_char="'")
 
 def set_mod_nss_protocol(self):
 installutils.set_directive(paths.HTTPD_NSS_CONF, 'NSSProtocol', 'TLSv1.0,TLSv1.1,TLSv1.2', False)
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index bf179a2..bbf4cc4 100644
--- a/ipaserver/install/installutils.py
+++ 

Re: [Freeipa-devel] FedoraHosted.org sunset

[Petr Vobornik]

On 09/23/2016 02:09 PM, Martin Basti wrote:
> 
> 
> On 23.09.2016 09:54, Jakub Hrozek wrote:
>> On Thu, Sep 22, 2016 at 06:09:43PM +0200, Petr Vobornik wrote:
>>> Hi all,
>>>
>>> As you know, FedoraHosted.org will be decommissioned.
>>>  
>>> https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
>>>
>>> We use Trac instance there. Let's discuss where we should migrate and
>>> what are our requirements. Then put results on one place. For that I've
>>> created:
>>>http://www.freeipa.org/page/FedoraHosted_Migration
>> That you for writing this up, there are some good points I didn't think
>> about, like migrating the ticket numbers. Did you already file an issue
>> that tracks this in Pagure (or asked if this is already possible)?
>>
> 
> Do we need review by field? It is recorded in commit and for ongoing
> reviews we are assigning ourselves to pull requests, so everybody knows
> if somebody is reviewing a PR.
> 
> Martin^2
> 

Assigning to PR solves the issue for which "review by" was meant. So we
may eventually drop it. Ideally when patch backlog on devel list is
cleansed.

In general, I'd not say that each individual field is a requirement,
e.g. I can imagine that: keywords, source, component, maybe even a
milestone and a priority can be tags/labels

What would be nice is some reporting/filtering capability so that I
don't have to script each view separately.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#107][closed] Update man/help for --server option

[mbasti-rh]

   URL: https://github.com/freeipa/freeipa/pull/107
Author: tomaskrizek
 Title: #107: Update man/help for --server option
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/107/head:pr107
git checkout pr107
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#107][comment] Update man/help for --server option

[mbasti-rh]

  URL: https://github.com/freeipa/freeipa/pull/107
Title: #107: Update man/help for --server option

mbasti-rh commented:
"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/07ff1f619c001181563886b5a0b5f1886b6638a1
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/107#issuecomment-249186962
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#109][+pushed] sudorule: add SELinux transition examples to plugin doc

[mbasti-rh]

  URL: https://github.com/freeipa/freeipa/pull/109
Title: #109: sudorule: add SELinux transition examples to plugin doc

Label: +pushed
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#109][closed] sudorule: add SELinux transition examples to plugin doc

[mbasti-rh]

   URL: https://github.com/freeipa/freeipa/pull/109
Author: frasertweedale
 Title: #109: sudorule: add SELinux transition examples to plugin doc
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/109/head:pr109
git checkout pr109
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#109][comment] sudorule: add SELinux transition examples to plugin doc

[mbasti-rh]

  URL: https://github.com/freeipa/freeipa/pull/109
Title: #109: sudorule: add SELinux transition examples to plugin doc

mbasti-rh commented:
"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/ff490b6c403f9fe14fcc2d1558c43dae5b80f493
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/109#issuecomment-249185580
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] pylint: remove unused variables

[Standa Laznicka]

On 09/23/2016 02:11 PM, Martin Basti wrote:



On 23.09.2016 14:12, Jan Cholasta wrote:

On 23.9.2016 13:23, Standa Laznicka wrote:

On 09/23/2016 07:28 AM, Jan Cholasta wrote:

On 22.9.2016 16:39, Martin Basti wrote:

Hello all,

In 4.5, I would like to remove all unused variables from code and 
enable

pylint check. Due to big amount of unused variables in the code this
will be longterm effort.

Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug


It is clear what to do with unused assignments, but I need an 
agreement

what to do with unpacking or iteration with unused variables


For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused


Pylint will detect surname as unused variable and we have to agree 
on a

way how to tell pylint that this variable is unused on purpose:


1)

(

   name,

   surname,  # pylint: disable=unused-variable

   gender

) = user['mbasti']


I dont like this approach


+1




2)

Use defined keyword: 'dummy' is default in pylint, we can set our 
own,

like ignored, unused

name, dummy, gender = user['mbasti']


-1, not visible enough.




3)

use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


This. We have already been using it in new code for quite some time,
and it's common in other Python projects too. Don't reinvent the 
wheel.





4)

we can combine all :)


For me the best is to have prefix '_' and 'dummy' keyword


Use '_dummy', it's both :-)


+1. I would rather use _meh as it's easier to type but perhaps not that
self-explanatory and not established at all, so _dummy is just fine :)


What I'm actually suggesting is that any local variable with "_" 
prefix should be considered unused, so _meh would be fine as well.




+1 regexp '_.+' works for me


Wonderful, I'm all in.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] pylint: remove unused variables

[Martin Basti]

On 23.09.2016 14:12, Jan Cholasta wrote:

On 23.9.2016 13:23, Standa Laznicka wrote:

On 09/23/2016 07:28 AM, Jan Cholasta wrote:

On 22.9.2016 16:39, Martin Basti wrote:

Hello all,

In 4.5, I would like to remove all unused variables from code and 
enable

pylint check. Due to big amount of unused variables in the code this
will be longterm effort.

Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug


It is clear what to do with unused assignments, but I need an 
agreement

what to do with unpacking or iteration with unused variables


For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused


Pylint will detect surname as unused variable and we have to agree 
on a

way how to tell pylint that this variable is unused on purpose:


1)

(

   name,

   surname,  # pylint: disable=unused-variable

   gender

) = user['mbasti']


I dont like this approach


+1




2)

Use defined keyword: 'dummy' is default in pylint, we can set our own,
like ignored, unused

name, dummy, gender = user['mbasti']


-1, not visible enough.




3)

use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


This. We have already been using it in new code for quite some time,
and it's common in other Python projects too. Don't reinvent the wheel.




4)

we can combine all :)


For me the best is to have prefix '_' and 'dummy' keyword


Use '_dummy', it's both :-)


+1. I would rather use _meh as it's easier to type but perhaps not that
self-explanatory and not established at all, so _dummy is just fine :)


What I'm actually suggesting is that any local variable with "_" 
prefix should be considered unused, so _meh would be fine as well.




+1 regexp '_.+' works for me

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#109][+ack] sudorule: add SELinux transition examples to plugin doc

[tomaskrizek]

  URL: https://github.com/freeipa/freeipa/pull/109
Title: #109: sudorule: add SELinux transition examples to plugin doc

Label: +ack
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] FedoraHosted.org sunset

[Martin Basti]

On 23.09.2016 09:54, Jakub Hrozek wrote:

On Thu, Sep 22, 2016 at 06:09:43PM +0200, Petr Vobornik wrote:

Hi all,

As you know, FedoraHosted.org will be decommissioned.
  https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/

We use Trac instance there. Let's discuss where we should migrate and
what are our requirements. Then put results on one place. For that I've
created:
   http://www.freeipa.org/page/FedoraHosted_Migration

That you for writing this up, there are some good points I didn't think
about, like migrating the ticket numbers. Did you already file an issue
that tracks this in Pagure (or asked if this is already possible)?



Do we need review by field? It is recorded in commit and for ongoing 
reviews we are assigning ourselves to pull requests, so everybody knows 
if somebody is reviewing a PR.


Martin^2

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] pylint: remove unused variables

[Jan Cholasta]

On 23.9.2016 13:23, Standa Laznicka wrote:

On 09/23/2016 07:28 AM, Jan Cholasta wrote:

On 22.9.2016 16:39, Martin Basti wrote:

Hello all,

In 4.5, I would like to remove all unused variables from code and enable
pylint check. Due to big amount of unused variables in the code this
will be longterm effort.

Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug


It is clear what to do with unused assignments, but I need an agreement
what to do with unpacking or iteration with unused variables


For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused


Pylint will detect surname as unused variable and we have to agree on a
way how to tell pylint that this variable is unused on purpose:


1)

(

   name,

   surname,  # pylint: disable=unused-variable

   gender

) = user['mbasti']


I dont like this approach


+1




2)

Use defined keyword: 'dummy' is default in pylint, we can set our own,
like ignored, unused

name, dummy, gender = user['mbasti']


-1, not visible enough.




3)

use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


This. We have already been using it in new code for quite some time,
and it's common in other Python projects too. Don't reinvent the wheel.




4)

we can combine all :)


For me the best is to have prefix '_' and 'dummy' keyword


Use '_dummy', it's both :-)


+1. I would rather use _meh as it's easier to type but perhaps not that
self-explanatory and not established at all, so _dummy is just fine :)


What I'm actually suggesting is that any local variable with "_" prefix 
should be considered unused, so _meh would be fine as well.


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#111][opened] Prompt for forwarder in dnsforwardzone-add

[tomaskrizek]

   URL: https://github.com/freeipa/freeipa/pull/111
Author: tomaskrizek
 Title: #111: Prompt for forwarder in dnsforwardzone-add
Action: opened

PR body:
"""
When the command ipa dnsforwardzone-add is invoked without
specifying the forwarder as an argument and the forward
policy is not set to none, prompt for DNS forwarder.

https://fedorahosted.org/freeipa/ticket/6169
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/111/head:pr111
git checkout pr111
From ed8f86f40b31c62b5636557ded9bfbe82d643137 Mon Sep 17 00:00:00 2001
From: Tomas Krizek 
Date: Fri, 23 Sep 2016 13:26:40 +0200
Subject: [PATCH] Prompt for forwarder in dnsforwardzone-add

When the command ipa dnsforwardzone-add is invoked without
specifying the forwarder as an argument and the forward
policy is not set to none, prompt for DNS forwarder.

https://fedorahosted.org/freeipa/ticket/6169
---
 ipaclient/plugins/dns.py | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ipaclient/plugins/dns.py b/ipaclient/plugins/dns.py
index b9ab709..42ccd3d 100644
--- a/ipaclient/plugins/dns.py
+++ b/ipaclient/plugins/dns.py
@@ -389,6 +389,11 @@ def interactive_prompt_callback(self, kw):
 @register(override=True, no_fail=True)
 class dnsforwardzone_add(MethodOverride):
 def interactive_prompt_callback(self, kw):
+if ('idnsforwarders' not in kw and
+kw.get('idnsforwardpolicy') != u'none'):
+kw['idnsforwarders'] = self.Backend.textui.prompt(
+_(u'DNS forwarder'))
+
 # show informative message on client side
 # server cannot send messages asynchronous
 if kw.get('idnsforwarders', False):
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] pylint: remove unused variables

[Standa Laznicka]

On 09/23/2016 07:28 AM, Jan Cholasta wrote:

On 22.9.2016 16:39, Martin Basti wrote:

Hello all,

In 4.5, I would like to remove all unused variables from code and enable
pylint check. Due to big amount of unused variables in the code this
will be longterm effort.

Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug


It is clear what to do with unused assignments, but I need an agreement
what to do with unpacking or iteration with unused variables


For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused


Pylint will detect surname as unused variable and we have to agree on a
way how to tell pylint that this variable is unused on purpose:


1)

(

   name,

   surname,  # pylint: disable=unused-variable

   gender

) = user['mbasti']


I dont like this approach


+1




2)

Use defined keyword: 'dummy' is default in pylint, we can set our own,
like ignored, unused

name, dummy, gender = user['mbasti']


-1, not visible enough.




3)

use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


This. We have already been using it in new code for quite some time, 
and it's common in other Python projects too. Don't reinvent the wheel.





4)

we can combine all :)


For me the best is to have prefix '_' and 'dummy' keyword


Use '_dummy', it's both :-)

+1. I would rather use _meh as it's easier to type but perhaps not that 
self-explanatory and not established at all, so _dummy is just fine :)



As first step I'll enable pylint check and disable it locally per module
where unused variables are, to avoid new regressions. Then I will fix it
module by module.


I'm open to suggestions

Martin^2






--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] pylint: remove unused variables

[Jan Cholasta]

On 23.9.2016 10:40, Petr Spacek wrote:

On 23.9.2016 07:28, Jan Cholasta wrote:

On 22.9.2016 16:39, Martin Basti wrote:

Hello all,

In 4.5, I would like to remove all unused variables from code and enable
pylint check. Due to big amount of unused variables in the code this
will be longterm effort.

Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug


It is clear what to do with unused assignments, but I need an agreement
what to do with unpacking or iteration with unused variables


For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused


Pylint will detect surname as unused variable and we have to agree on a
way how to tell pylint that this variable is unused on purpose:


1)

(

   name,

   surname,  # pylint: disable=unused-variable

   gender

) = user['mbasti']


I dont like this approach


+1




2)

Use defined keyword: 'dummy' is default in pylint, we can set our own,
like ignored, unused

name, dummy, gender = user['mbasti']


-1, not visible enough.




3)

use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


This. We have already been using it in new code for quite some time, and it's
common in other Python projects too. Don't reinvent the wheel.




4)

we can combine all :)


For me the best is to have prefix '_' and 'dummy' keyword


Use '_dummy', it's both :-)


I like "__".  If it is not acceptable for rest of the team, I would be okay
with _dummy.


I'm not a fan of "__" (because it's as "brilliant" as "___" or ""), 
but if any local variable with "_" prefix is considered unused (i.e. 
what I'm proposing), it would be perfectly legal.



I would even use _dummy multiple times:

name, _dummy, _dummy = user['mbasti']

so namespace is not polluted and garbage collector can do the right thing.


This is a pretty misguided argument if I may say so. First, I don't see 
how locals namespace pollution could ever cause any issues, and even if 
it did, the proper way to avoid it is to not write long functions with 
many local variables. Second, removing a local variable does not 
guarantee that the garbage collector will do anything (because garbage 
collection is not always deterministic), and even if it did, you should 
be explicit about it and use the del statement.




Petr^2 Spacek



As first step I'll enable pylint check and disable it locally per module
where unused variables are, to avoid new regressions. Then I will fix it
module by module.


I'm open to suggestions

Martin^2





--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#110][comment] test_text: add test ipa.pot file for tests

[mbasti-rh]

  URL: https://github.com/freeipa/freeipa/pull/110
Title: #110: test_text: add test ipa.pot file for tests

mbasti-rh commented:
"""
I hope I didn't changed the aim of test, but having test packaged in separate 
module which requires to have cloned repo and working only from project 
directory is quite weird for me. So I create testing 'ipa.pot' file in test 
directory (in test package as well)
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/110#issuecomment-249137555
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#110][opened] test_text: add test ipa.pot file for tests

[mbasti-rh]

   URL: https://github.com/freeipa/freeipa/pull/110
Author: mbasti-rh
 Title: #110: test_text: add test ipa.pot file for tests
Action: opened

PR body:
"""
Input data should be packaged into freeipa-test module to be able run
test from RPM (outoftree)

https://fedorahosted.org/freeipa/ticket/6333
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/110/head:pr110
git checkout pr110
From 22bf6e7ac35172916ac2a4c53e08a13b3cc89d9c Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 23 Sep 2016 09:51:41 +0200
Subject: [PATCH] test_text: add test ipa.pot file for tests

Input data should be packaged into freeipa-test module to be able run
test from RPM (outoftree)

https://fedorahosted.org/freeipa/ticket/6333
---
 ipatests/setup.py.in  |  1 +
 ipatests/test_ipalib/data/ipa.pot | 47 +++
 ipatests/test_ipalib/test_text.py | 11 -
 3 files changed, 54 insertions(+), 5 deletions(-)
 create mode 100644 ipatests/test_ipalib/data/ipa.pot

diff --git a/ipatests/setup.py.in b/ipatests/setup.py.in
index cb5f722..18483b0 100644
--- a/ipatests/setup.py.in
+++ b/ipatests/setup.py.in
@@ -82,6 +82,7 @@ def setup_package():
 'ipatests': ['pytest.ini'],
 'ipatests.test_install': ['*.update'],
 'ipatests.test_integration': ['scripts/*'],
+'ipatests.test_ipalib': ['data/*'],
 'ipatests.test_pkcs10': ['*.csr'],
 "ipatests.test_ipaserver": ['data/*'],
 'ipatests.test_xmlrpc': ['data/*'],
diff --git a/ipatests/test_ipalib/data/ipa.pot b/ipatests/test_ipalib/data/ipa.pot
new file mode 100644
index 000..523d3e2
--- /dev/null
+++ b/ipatests/test_ipalib/data/ipa.pot
@@ -0,0 +1,47 @@
+# SOME DESCRIPTIVE TITLE.
+# Copyright (C) YEAR Red Hat
+# This file is distributed under the same license as the ipa package.
+# FIRST AUTHOR , YEAR.
+#
+#, fuzzy
+msgid ""
+msgstr ""
+"Project-Id-Version: ipa\n"
+"Report-Msgid-Bugs-To: https://fedorahosted.org/freeipa/newticket\n;
+"POT-Creation-Date: 2016-08-29 10:39+0200\n"
+"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
+"Last-Translator: FULL NAME \n"
+"Language-Team: LANGUAGE \n"
+"Language: \n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n"
+
+#: ipaclient/frontend.py:28 ipaclient/frontend.py:85
+#: ipaserver/plugins/baseldap.py:52
+msgid "Failed members"
+msgstr ""
+
+#: ipaclient/frontend.py:32 ipaserver/plugins/baseldap.py:169
+msgid "Failed source hosts/hostgroups"
+msgstr ""
+
+#: ipaclient/frontend.py:36 ipaserver/plugins/baseldap.py:172
+msgid "Failed hosts/hostgroups"
+msgstr ""
+
+#: ipaclient/frontend.py:40 ipaserver/plugins/baseldap.py:175
+msgid "Failed users/groups"
+msgstr ""
+
+#: ipaclient/plugins/dns.py:249
+#, python-format
+msgid ""
+"%(count)d %(type)s record skipped. Only one value per DNS record type can be "
+"modified at one time."
+msgid_plural ""
+"%(count)d %(type)s records skipped. Only one value per DNS record type can "
+"be modified at one time."
+msgstr[0] ""
+msgstr[1] ""
diff --git a/ipatests/test_ipalib/test_text.py b/ipatests/test_ipalib/test_text.py
index bdc7623..d510646 100644
--- a/ipatests/test_ipalib/test_text.py
+++ b/ipatests/test_ipalib/test_text.py
@@ -59,8 +59,6 @@ def setup(self):
 self.lang = 'xh_ZA'
 self.domain = 'ipa'
 
-self.ipa_i18n_dir = os.path.join(os.path.dirname(__file__), '../../install/po')
-
 self.pot_basename = '%s.pot' % self.domain
 self.po_basename = '%s.po' % self.lang
 self.mo_basename = '%s.mo' % self.domain
@@ -74,7 +72,8 @@ def setup(self):
 if not os.path.exists(self.msg_dir):
 os.makedirs(self.msg_dir)
 
-self.pot_file = os.path.join(self.ipa_i18n_dir, self.pot_basename)
+self.pot_file = os.path.join(
+os.path.dirname(__file__), 'data', self.pot_basename)
 self.mo_file = os.path.join(self.msg_dir, self.mo_basename)
 self.po_file = os.path.join(self.tmp_dir, self.po_basename)
 
@@ -84,10 +83,12 @@ def setup(self):
 (self.po_file, self.mo_file, self.pot_file))
 
 if not file_exists(self.po_file):
-raise nose.SkipTest('Test po file unavailable, run "make test" in install/po')
+raise nose.SkipTest(
+'Test po file unavailable: {}'.format(self.po_file))
 
 if not file_exists(self.mo_file):
-raise nose.SkipTest('Test mo file unavailable, run "make test" in install/po')
+raise nose.SkipTest(
+'Test mo file unavailable: {}'.format(self.mo_file))
 
 self.po_file_iterate = po_file_iterate
 
-- 
Manage your subscription for the Freeipa-devel mailing list:

[Freeipa-devel] [freeipa PR#107][+ack] Update man/help for --server option

[pspacek]

  URL: https://github.com/freeipa/freeipa/pull/107
Title: #107: Update man/help for --server option

Label: +ack
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] pylint: remove unused variables

[Petr Spacek]

On 23.9.2016 07:28, Jan Cholasta wrote:
> On 22.9.2016 16:39, Martin Basti wrote:
>> Hello all,
>>
>> In 4.5, I would like to remove all unused variables from code and enable
>> pylint check. Due to big amount of unused variables in the code this
>> will be longterm effort.
>>
>> Why this?:
>>
>> * better code readability
>>
>> * removing dead code
>>
>> * unused variable may uncover potential bug
>>
>>
>> It is clear what to do with unused assignments, but I need an agreement
>> what to do with unpacking or iteration with unused variables
>>
>>
>> For example:
>>
>> for name, surname, gender in (('Martin', 'Basti', 'M'), ):
>>
>> name, surname, gender = user['mbasti']
>>
>> Where 'surname' is unused
>>
>>
>> Pylint will detect surname as unused variable and we have to agree on a
>> way how to tell pylint that this variable is unused on purpose:
>>
>>
>> 1)
>>
>> (
>>
>>name,
>>
>>surname,  # pylint: disable=unused-variable
>>
>>gender
>>
>> ) = user['mbasti']
>>
>>
>> I dont like this approach
> 
> +1
> 
>>
>>
>> 2)
>>
>> Use defined keyword: 'dummy' is default in pylint, we can set our own,
>> like ignored, unused
>>
>> name, dummy, gender = user['mbasti']
> 
> -1, not visible enough.
> 
>>
>>
>> 3)
>>
>> use a prefix for unused variables: '_' or 'ignore_'
>>
>> name, _surname, gender = user['mbasti']
> 
> This. We have already been using it in new code for quite some time, and it's
> common in other Python projects too. Don't reinvent the wheel.
> 
>>
>>
>> 4)
>>
>> we can combine all :)
>>
>>
>> For me the best is to have prefix '_' and 'dummy' keyword
> 
> Use '_dummy', it's both :-)

I like "__". If it is not acceptable for rest of the team, I would be okay
with _dummy. I would even use _dummy multiple times:
> name, _dummy, _dummy = user['mbasti']
so namespace is not polluted and garbage collector can do the right thing.

Petr^2 Spacek


>> As first step I'll enable pylint check and disable it locally per module
>> where unused variables are, to avoid new regressions. Then I will fix it
>> module by module.
>>
>>
>> I'm open to suggestions
>>
>> Martin^2

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] FedoraHosted.org sunset

[Jakub Hrozek]

On Thu, Sep 22, 2016 at 06:09:43PM +0200, Petr Vobornik wrote:
> Hi all,
> 
> As you know, FedoraHosted.org will be decommissioned.
>  https://communityblog.fedoraproject.org/fedorahosted-sunset-2017-02-28/
> 
> We use Trac instance there. Let's discuss where we should migrate and
> what are our requirements. Then put results on one place. For that I've
> created:
>   http://www.freeipa.org/page/FedoraHosted_Migration

That you for writing this up, there are some good points I didn't think
about, like migrating the ticket numbers. Did you already file an issue
that tracks this in Pagure (or asked if this is already possible)?

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#106][+pushed] Pylint: enable additional checks

[mbasti-rh]

  URL: https://github.com/freeipa/freeipa/pull/106
Title: #106: Pylint: enable additional checks

Label: +pushed
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#106][closed] Pylint: enable additional checks

[mbasti-rh]

   URL: https://github.com/freeipa/freeipa/pull/106
Author: mbasti-rh
 Title: #106: Pylint: enable additional checks
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/106/head:pr106
git checkout pr106
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#106][comment] Pylint: enable additional checks

[mbasti-rh]

  URL: https://github.com/freeipa/freeipa/pull/106
Title: #106: Pylint: enable additional checks

mbasti-rh commented:
"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/4d7d53c9664c9e68d7c6cda1a65cae7551423df7
https://fedorahosted.org/freeipa/changeset/9b68d2a1f858854bc3cf2d6024f3fd3d79c2f696
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/106#issuecomment-249121222
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0091 Allow full customisability of CA subject name

[Jan Cholasta]

On 23.9.2016 09:15, Fraser Tweedale wrote:

On Fri, Sep 23, 2016 at 08:51:02AM +0200, Jan Cholasta wrote:

On 25.8.2016 12:08, Jan Cholasta wrote:

On 22.8.2016 07:00, Fraser Tweedale wrote:

On Fri, Aug 19, 2016 at 08:09:33PM +1000, Fraser Tweedale wrote:

On Mon, Aug 15, 2016 at 10:54:25PM +1000, Fraser Tweedale wrote:

On Mon, Aug 15, 2016 at 02:08:54PM +0200, Jan Cholasta wrote:

On 19.7.2016 12:05, Jan Cholasta wrote:

On 19.7.2016 11:54, Fraser Tweedale wrote:

On Tue, Jul 19, 2016 at 09:36:17AM +0200, Jan Cholasta wrote:

Hi,

On 15.7.2016 07:05, Fraser Tweedale wrote:

On Fri, Jul 15, 2016 at 03:04:48PM +1000, Fraser Tweedale wrote:

The attached patch is a work in progress for
https://fedorahosted.org/freeipa/ticket/2614 (BZ 828866).

I am sharing now to make the approach clear and solicit feedback.

It has been tested for server install, replica install (with and
without CA) and CA-replica install (all hosts running
master+patch).

Migration from earlier versions and server/replica/CA install
on a
CA-less deployment are not yet tested; these will be tested over
coming days and patch will be tweaked as necessary.

Commit message has a fair bit to say so I won't repeat here
but let
me know your questions and comments.

Thanks,
Fraser


It does help to attach the patch, of course ^_^


IMO explicit is better than implicit, so instead of introducing
additional
magic around --subject, I would rather add a new separate option
for
specifying the CA subject name (I think --ca-subject, for
consistency
with
--ca-signing-algorithm).


The current situation - the --subject argument which specifies the
not the subject but the subject base, is confusing enough (to say
nothing of the limitations that give rise to the RFE).

Retaining --subject for specifying the subject base and adding
--ca-subject for specifying the *actual* subject DN gets us over the
line in terms of the RFE, but does not make the installer less
confusing.  This is why I made --subject accept the full subject DN,
with provisions to retain existing behaviour.

IMO if we want to have separate arguments for subject DN and subject
base (I am not against it), let's bite the bullet and name arguments
accordingly.  --subject should be used to specify full Subject DN,
--subject-base (or similar) for specifying subject base.


IMHO --ca-subject is better than --subject, because it is more
explicit
whose subject name that is (the CA's). I agree that --subject
should be
deprecated and replaced with --subject-base.



(I intentionally defer discussion of specific behaviour if one, none
or both are specified; let's resolve the question or renaming /
changing meaning of arguments first).



By specifying the option you would override the default
"CN=Certificate
Authority,$SUBJECT_BASE" subject name. If --external-ca was not
used,
additional validation would be done to make sure the subject
name meets
Dogtag's expectations. Actually, it might make sense to always
do the
additional validation, to be able to print a warning that if a
Dogtag-incompatible subject name is used, it won't be possible to
change the
CA cert chaining from externally signed to self-signed later.

Honza


Bump, any update on this?


I have an updated patch that fixes some issues Sebastian encountered
in testing, but I've not yet tackled the main change requested by
Honza (in brief: adding --ca-subject for specifying that, adding
--subject-base for specifying that, and deprecating --subject;
Sebastian, see discussion above and feel free to give your
thoughts).  I expect I'll get back onto this work within the next
few days.


Update: I've got an updated version of patch almost ready for
review, but I'm still ironing out some wrinkles in replica
installation.

Expect to be able to send it Monday or Tuesday for review.


Updated patch attached.

I expect it will take a while to review, test and get the ACK, but
in any case: IMO it should not be merged until after ipa-4-4 branch
gets created.


1) Please fix these:

$ git show -U0 | pep8 --diff
./ipaserver/install/cainstance.py:508:13: E128 continuation line
under-indented for visual indent
./ipaserver/install/dsinstance.py:259:5: E303 too many blank lines (2)
./ipaserver/install/installutils.py:999:1: E302 expected 2 blank lines,
found 1
./ipaserver/install/server/common.py:161:80: E501 line too long (101 >
79 characters)
./ipaserver/install/server/replicainstall.py:93:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:604:80: E501 line too long
(81 > 79 characters)


2) Please put 3rd party library (such as six) imports between standard
library imports and ipa imports.


3) ipa-ca-install should also have the --subject-base and --ca-subject
options.


4) Please use the original method of getting the configured subject base
from ipacertificatesubjectbase of the config object rather than
DSInstance.find_subject_base(), which is horrendous and should in fact
be obliterated (not necessarily in this 

Re: [Freeipa-devel] [PATCH 0060] Add --force-join option to ipa-replica-install

[Jan Cholasta]

On 23.9.2016 09:01, Standa Laznicka wrote:

On 09/23/2016 08:50 AM, Jan Cholasta wrote:

On 25.8.2016 15:31, Martin Basti wrote:



On 10.08.2016 07:53, Stanislav Laznicka wrote:

On 08/10/2016 07:31 AM, Jan Cholasta wrote:

On 9.8.2016 18:52, Petr Vobornik wrote:

On 08/09/2016 04:18 PM, Martin Basti wrote:



On 09.08.2016 16:07, Stanislav Laznicka wrote:

https://fedorahosted.org/freeipa/ticket/6183




Didn't we agreed that --force-join should be always used (without
extra
replica-install option)


+1


Did we?

IMO the default behavior should be the same as in domain level 0 when
trying to install replica on an already enrolled host.

That was my impression as well.


OK then, I don't like to add mostly useless option, but client install
is broken by design so whatever.


Bump, what is the status of this?

FTR this is what happens on domain level 0 if the host is already
enrolled:

# ipa-replica-install replica-info-test.example.com.gpg
WARNING: conflicting time synchronization service 'chronyd' will
be disabled in favor of ntpd

Directory Manager (existing master) password:

The host test.example.com already exists on the master server.
You should remove it before proceeding:
% ipa host-del test.example.com
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See
/var/log/ipareplica-install.log for more information



There's been no status change.

I think the problem here is more about client-install advertising the
--force-join option which does not exist for ipa-replica-install. I do
not think we can detect that exactly this error occurred during
client-install being run from replica-install (can we?) but we can add
this option and pass it to client-install if required.


We could detect it before running ipa-client-install, but adding the 
option to ipa-replica-install is easier, so IMO that's what we should do.


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0060] Add --force-join option to ipa-replica-install

[Standa Laznicka]

On 09/23/2016 08:50 AM, Jan Cholasta wrote:

On 25.8.2016 15:31, Martin Basti wrote:



On 10.08.2016 07:53, Stanislav Laznicka wrote:

On 08/10/2016 07:31 AM, Jan Cholasta wrote:

On 9.8.2016 18:52, Petr Vobornik wrote:

On 08/09/2016 04:18 PM, Martin Basti wrote:



On 09.08.2016 16:07, Stanislav Laznicka wrote:

https://fedorahosted.org/freeipa/ticket/6183




Didn't we agreed that --force-join should be always used (without
extra
replica-install option)


+1


Did we?

IMO the default behavior should be the same as in domain level 0 when
trying to install replica on an already enrolled host.

That was my impression as well.


OK then, I don't like to add mostly useless option, but client install
is broken by design so whatever.


Bump, what is the status of this?

FTR this is what happens on domain level 0 if the host is already 
enrolled:


# ipa-replica-install replica-info-test.example.com.gpg
WARNING: conflicting time synchronization service 'chronyd' will
be disabled in favor of ntpd

Directory Manager (existing master) password:

The host test.example.com already exists on the master server.
You should remove it before proceeding:
% ipa host-del test.example.com
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe 
ipa-replica-install command failed. See 
/var/log/ipareplica-install.log for more information




There's been no status change.

I think the problem here is more about client-install advertising the 
--force-join option which does not exist for ipa-replica-install. I do 
not think we can detect that exactly this error occurred during 
client-install being run from replica-install (can we?) but we can add 
this option and pass it to client-install if required.


--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0091 Allow full customisability of CA subject name

[Jan Cholasta]

On 25.8.2016 12:08, Jan Cholasta wrote:

On 22.8.2016 07:00, Fraser Tweedale wrote:

On Fri, Aug 19, 2016 at 08:09:33PM +1000, Fraser Tweedale wrote:

On Mon, Aug 15, 2016 at 10:54:25PM +1000, Fraser Tweedale wrote:

On Mon, Aug 15, 2016 at 02:08:54PM +0200, Jan Cholasta wrote:

On 19.7.2016 12:05, Jan Cholasta wrote:

On 19.7.2016 11:54, Fraser Tweedale wrote:

On Tue, Jul 19, 2016 at 09:36:17AM +0200, Jan Cholasta wrote:

Hi,

On 15.7.2016 07:05, Fraser Tweedale wrote:

On Fri, Jul 15, 2016 at 03:04:48PM +1000, Fraser Tweedale wrote:

The attached patch is a work in progress for
https://fedorahosted.org/freeipa/ticket/2614 (BZ 828866).

I am sharing now to make the approach clear and solicit feedback.

It has been tested for server install, replica install (with and
without CA) and CA-replica install (all hosts running
master+patch).

Migration from earlier versions and server/replica/CA install
on a
CA-less deployment are not yet tested; these will be tested over
coming days and patch will be tweaked as necessary.

Commit message has a fair bit to say so I won't repeat here
but let
me know your questions and comments.

Thanks,
Fraser


It does help to attach the patch, of course ^_^


IMO explicit is better than implicit, so instead of introducing
additional
magic around --subject, I would rather add a new separate option
for
specifying the CA subject name (I think --ca-subject, for
consistency
with
--ca-signing-algorithm).


The current situation - the --subject argument which specifies the
not the subject but the subject base, is confusing enough (to say
nothing of the limitations that give rise to the RFE).

Retaining --subject for specifying the subject base and adding
--ca-subject for specifying the *actual* subject DN gets us over the
line in terms of the RFE, but does not make the installer less
confusing.  This is why I made --subject accept the full subject DN,
with provisions to retain existing behaviour.

IMO if we want to have separate arguments for subject DN and subject
base (I am not against it), let's bite the bullet and name arguments
accordingly.  --subject should be used to specify full Subject DN,
--subject-base (or similar) for specifying subject base.


IMHO --ca-subject is better than --subject, because it is more
explicit
whose subject name that is (the CA's). I agree that --subject
should be
deprecated and replaced with --subject-base.



(I intentionally defer discussion of specific behaviour if one, none
or both are specified; let's resolve the question or renaming /
changing meaning of arguments first).



By specifying the option you would override the default
"CN=Certificate
Authority,$SUBJECT_BASE" subject name. If --external-ca was not
used,
additional validation would be done to make sure the subject
name meets
Dogtag's expectations. Actually, it might make sense to always
do the
additional validation, to be able to print a warning that if a
Dogtag-incompatible subject name is used, it won't be possible to
change the
CA cert chaining from externally signed to self-signed later.

Honza


Bump, any update on this?


I have an updated patch that fixes some issues Sebastian encountered
in testing, but I've not yet tackled the main change requested by
Honza (in brief: adding --ca-subject for specifying that, adding
--subject-base for specifying that, and deprecating --subject;
Sebastian, see discussion above and feel free to give your
thoughts).  I expect I'll get back onto this work within the next
few days.


Update: I've got an updated version of patch almost ready for
review, but I'm still ironing out some wrinkles in replica
installation.

Expect to be able to send it Monday or Tuesday for review.


Updated patch attached.

I expect it will take a while to review, test and get the ACK, but
in any case: IMO it should not be merged until after ipa-4-4 branch
gets created.


1) Please fix these:

$ git show -U0 | pep8 --diff
./ipaserver/install/cainstance.py:508:13: E128 continuation line
under-indented for visual indent
./ipaserver/install/dsinstance.py:259:5: E303 too many blank lines (2)
./ipaserver/install/installutils.py:999:1: E302 expected 2 blank lines,
found 1
./ipaserver/install/server/common.py:161:80: E501 line too long (101 >
79 characters)
./ipaserver/install/server/replicainstall.py:93:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:604:80: E501 line too long
(81 > 79 characters)


2) Please put 3rd party library (such as six) imports between standard
library imports and ipa imports.


3) ipa-ca-install should also have the --subject-base and --ca-subject
options.


4) Please use the original method of getting the configured subject base
from ipacertificatesubjectbase of the config object rather than
DSInstance.find_subject_base(), which is horrendous and should in fact
be obliterated (not necessarily in this patch).


5) You can use "unicode(x509.get_subject(cert))" to get subject name of
a cert instead of 

[Freeipa-devel] [freeipa PR#109][opened] sudorule: add SELinux transition examples to plugin doc

[frasertweedale]

   URL: https://github.com/freeipa/freeipa/pull/109
Author: frasertweedale
 Title: #109: sudorule: add SELinux transition examples to plugin doc
Action: opened

PR body:
"""
It is not obvious how to add SELinux type and role transitions to a
Sudo rule.  Update the 'sudorule' plugin documentation with examples
of how to do this.

Fixes: https://fedorahosted.org/freeipa/ticket/3461
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/109/head:pr109
git checkout pr109
From 76a3bd068db039834c9d497c69948bf27a8e25da Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Fri, 23 Sep 2016 16:43:19 +1000
Subject: [PATCH] sudorule: add SELinux transition examples to plugin doc

It is not obvious how to add SELinux type and role transitions to a
Sudo rule.  Update the 'sudorule' plugin documentation with examples
of how to do this.

Fixes: https://fedorahosted.org/freeipa/ticket/3461
---
 ipaserver/plugins/sudorule.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/ipaserver/plugins/sudorule.py b/ipaserver/plugins/sudorule.py
index 15d03c6..9077107 100644
--- a/ipaserver/plugins/sudorule.py
+++ b/ipaserver/plugins/sudorule.py
@@ -88,6 +88,10 @@
 """) + _("""
  Set a default Sudo option:
ipa sudorule-add-option defaults --sudooption '!authenticate'
+""") + _("""
+ Set SELinux type and role transitions on a rule:
+   ipa sudorule-add-option sysadmin_sudo --sudooption type=unconfined_t
+   ipa sudorule-add-option sysadmin_sudo --sudooption role=unconfined_r
 """)
 
 register = Registry()
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0060] Add --force-join option to ipa-replica-install

[Jan Cholasta]

On 25.8.2016 15:31, Martin Basti wrote:



On 10.08.2016 07:53, Stanislav Laznicka wrote:

On 08/10/2016 07:31 AM, Jan Cholasta wrote:

On 9.8.2016 18:52, Petr Vobornik wrote:

On 08/09/2016 04:18 PM, Martin Basti wrote:



On 09.08.2016 16:07, Stanislav Laznicka wrote:

https://fedorahosted.org/freeipa/ticket/6183




Didn't we agreed that --force-join should be always used (without
extra
replica-install option)


+1


Did we?

IMO the default behavior should be the same as in domain level 0 when
trying to install replica on an already enrolled host.

That was my impression as well.


OK then, I don't like to add mostly useless option, but client install
is broken by design so whatever.


Bump, what is the status of this?

FTR this is what happens on domain level 0 if the host is already enrolled:

# ipa-replica-install replica-info-test.example.com.gpg
WARNING: conflicting time synchronization service 'chronyd' will
be disabled in favor of ntpd

Directory Manager (existing master) password:

The host test.example.com already exists on the master server.
You should remove it before proceeding:
% ipa host-del test.example.com
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe 
ipa-replica-install command failed. See /var/log/ipareplica-install.log 
for more information



--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#108][edited] Bump pki min version and add commentary about sub-CA revocation on delete

[frasertweedale]

   URL: https://github.com/freeipa/freeipa/pull/108
Author: frasertweedale
 Title: #108: Bump pki min version and add commentary about sub-CA revocation 
on delete
Action: edited

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/108/head:pr108
git checkout pr108
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#108][edited] Bump pki min version and add commentary about sub-CA revocation on delete

[frasertweedale]

   URL: https://github.com/freeipa/freeipa/pull/108
Author: frasertweedale
 Title: #108: Bump pki min version and add commentary about sub-CA revocation 
on delete
Action: edited

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/108/head:pr108
git checkout pr108
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#108][opened] https://fedorahosted.org/freeipa/ticket/6256

[frasertweedale]

   URL: https://github.com/freeipa/freeipa/pull/108
Author: frasertweedale
 Title: #108: https://fedorahosted.org/freeipa/ticket/6256
Action: opened

PR body:
"""
None
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/108/head:pr108
git checkout pr108
From b3a5c7face04c6a9a3b2c78f0794fde98b855387 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Fri, 23 Sep 2016 16:01:19 +1000
Subject: [PATCH 1/2] spec: require Dogtag >= 10.3.5-6

Require Dogtag 10.3.5-6, which is the first release that implements
revocation of lightweight CA signing certificates upon deletion.

Part of: https://fedorahosted.org/freeipa/ticket/6256
---
 freeipa.spec.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 3b0e4b2..cab0233 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -97,7 +97,7 @@ BuildRequires:  libunistring-devel
 BuildRequires:  python-lesscpy
 BuildRequires:  python-yubico >= 1.2.3
 BuildRequires:  openssl-devel
-BuildRequires:  pki-base >= 10.3.3-3
+BuildRequires:  pki-base >= 10.3.5-6
 BuildRequires:  python-pytest-multihost >= 0.5
 BuildRequires:  python-pytest-sourceorder
 BuildRequires:  python-kdcproxy >= 0.3
@@ -161,8 +161,8 @@ Requires(post): systemd-units
 Requires: selinux-policy >= %{selinux_policy_version}
 Requires(post): selinux-policy-base >= %{selinux_policy_version}
 Requires: slapi-nis >= %{slapi_nis_version}
-Requires: pki-ca >= 10.3.3-3
-Requires: pki-kra >= 10.3.3-3
+Requires: pki-ca >= 10.3.5-6
+Requires: pki-kra >= 10.3.5-6
 Requires(preun): python systemd-units
 Requires(postun): python systemd-units
 Requires: zip

From 610cb77a7f42d6c0eb20725f6319a46b786b106d Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Fri, 23 Sep 2016 16:05:55 +1000
Subject: [PATCH 2/2] Add commentary about CA deletion to plugin doc

Add commentary to 'ca' plugin documentation to explain what happens
when a CA gets deleted - namely, that its signing cert gets revoked
and its private key deleted.

Fixes: https://fedorahosted.org/freeipa/ticket/6256
---
 ipaserver/plugins/ca.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index 4d83fe8..3cdc9f2 100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -25,6 +25,9 @@
 prevents it from issuing certificates but does not affect the validity of its
 certificate.
 
+CAs (all except the 'IPA' CA) can be deleted.  Deleting a CA causes its signing
+certificate to be revoked and its private key deleted.
+
 
 EXAMPLES:
 
@@ -41,6 +44,10 @@
 
 ipa ca-enable puppet
 
+  Delete a CA.
+
+ipa ca-del puppet
+
 """)
 
 
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#106][+ack] Pylint: enable additional checks

[tomaskrizek]

  URL: https://github.com/freeipa/freeipa/pull/106
Title: #106: Pylint: enable additional checks

Label: +ack
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0107 Fix cert revocation when removing all certs via host/service-mod

[Jan Cholasta]

On 23.9.2016 05:30, Fraser Tweedale wrote:

Bump for review.


Works for me, ACK.

Pushed to master: 97d4ffc2dc5db00fd7ed10b0b290cc97a506d0ef

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code