Re: [Freeipa-devel] [patch 0032] ipatests: add missing certprofile fixture

2016-02-11 Thread Martin Babinsky

On 02/09/2016 04:06 PM, Milan Kubík wrote:

On 02/09/2016 02:37 PM, Milan Kubík wrote:

Fixes the CA ACL tests broken by removed import. This patch doesn't
rely on undocumented behavior of pytest.

The patch invalidates patch 133 by Martin Babinsky.




Patch updated with trac link

--
Milan Kubik




ACK I guess, although copypasta makes me very sad.

Why do you think the conftest.py approach is not practical for us?

--
Martin^3 Babinsky

--
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 0030] Modernize mod_nss's cipher suites

2016-02-11 Thread Martin Basti



On 03.02.2016 15:35, Christian Heimes wrote:

On 2016-01-29 15:05, Martin Basti wrote:


On 29.01.2016 14:42, Christian Heimes wrote:

On 2016-01-28 09:47, Martin Basti wrote:

On 22.01.2016 12:32, Martin Kosek wrote:

On 01/21/2016 04:21 PM, Christian Heimes wrote:

The list of supported TLS cipher suites in /etc/httpd/conf.d/nss.conf
has been modernized. Insecure or less secure algorithms such as RC4,
DES and 3DES are removed. Perfect forward secrecy suites with
ephemeral
ECDH key exchange have been added. IE 8 on Windows XP is no longer
supported.

The list of enabled cipher suites has been generated with the script
contrib/nssciphersuite/nssciphersuite.py.

The supported suites are currently:

TLS_RSA_WITH_AES_128_CBC_SHA256
TLS_RSA_WITH_AES_256_CBC_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
TLS_RSA_WITH_AES_128_GCM_SHA256
TLS_RSA_WITH_AES_128_CBC_SHA
TLS_RSA_WITH_AES_256_GCM_SHA384
TLS_RSA_WITH_AES_256_CBC_SHA

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

Thanks for the patch! I updated the ticket to make sure this change is
release notes.


Hello,

I'm not sure if I'm the right person to do review on this, but I will
try :-)

1)
Your patch adds whitespace error

Applying: Modernize mod_nss's cipher suites
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:52: new blank
line at EOF.
+
warning: 1 line adds whitespace errors.


2)
+import urllib.request  # pylint: disable=E0611

Please specify pylint disabled check by name

3)
+def update_mod_nss_cipher_suite(http):

in this upgrade, is there any possibility that ciphers might be upgraded
again in future? (IMO yes).

I think, it can be better to store revision of change instead of boolean

LAST_REVISION =  1

if revision >= LAST_REVISION:
  return

sysupgrade.set_upgrade_state('nss.conf', 'cipher_suite_revision',
LAST_REVISION)

Thanks for the review. I have addressed the problems. Instead of a
revision number I'm using a date string. The sysupgrade module only
stores str and bool. With a date-based revision it's easy to see when
the cipher suite was checked last time.

Christian


Thanks

1) Pylint :-)
+with urllib.request.urlopen(SOURCE) as r:  # pylint: disable=E1101

Thanks! It was easier to change the import to get rid of the second
pylint stanza.


2)
+if revision == httpinstance.NSS_CIPHER_REVISION:

may happen a case where just comparation with '==' can cause a issues
(docker world)? Should not be there rather '>='?

Makes sense, I've changed the comparison operator to >=. This may still
override user settings, though.


3)
+root_logger.info("Cipher suite already updated")

Sorry that I did not noticed earlier, this should be just debug level,
IMO this message is not so important, it will cause only mess on output
(we already have plenty of unneeded info messages in upgrade, they will
be fixed once)

Fine with me :)

Christian

ACK

Pushed to:
master: 5ac3a3cee534a16db86c541b9beff4939f03410e
ipa-4-3: c3496a4a4893c75789bdf0c617e46923361fb43b

--
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 0407] make lint: migration to config file and pylint plugin due pylint 1.5.2

2016-02-11 Thread Martin Basti



On 09.02.2016 15:26, David Kupka wrote:

On 09/02/16 11:28, Martin Basti wrote:



On 03.02.2016 10:19, David Kupka wrote:

On 01/02/16 14:18, Martin Basti wrote:



On 26.01.2016 14:16, Martin Basti wrote:



On 20.01.2016 14:38, Jan Cholasta wrote:

Hi,

On 19.1.2016 13:43, Martin Basti wrote:

New pylint version will broke our custom make-lint script again,
attached patch migrates make-lint to:
* config file
* pylint plugin
which are supported by pylint and should not have regular
compatibility
issues

to test new approach run ./make-lint2

Advantages:
* compatibility with pylint
* works on both pylint-1.4.3-3.fc23.noarch and
pylint-1.5.2-1.fc24.noarch
* pylint plugin works in different way than the previous custom
checker.
Missing ("dynamic") attributes are added to abstract syntax tree
instead
of ignoring them and all their sub-members. This makes check 
better,
pylint can detect more typos in tests configurations, api, env, 
etc..


Disadvantages:
* any new attribute in api, test config, etc.. must be added to
definition of missing members (pylint plugin) - this should not
happen
too often


1) Please "mv pylint_plugins/fix_ipa_members.py pylint_plugins.py"
and "rm -rf pylint_plugins/", no need for this redundant directory
structure.

2) Rename pylintrc to freeipa.pylintrc so you have to always specify
it explicitly with --rcfile.

3) Use the load-plugins directive in freeipa.pylintrc to load the
plugins rather than --load-plugins.

4) Instead of running pylint twice, run it only once with both 
normal

and Python 3 checks enabled:

[MESSAGE CONTROL]
enable=all,python3
disable=...,no-absolute-import




Q:
* make-lint: should it be just bash script or rather python script?


IMO neither, it should be a make target (make lint).


* add dynamic detection of python files to be checked


You can use "find . -type f -executable ! -path \*/.\* ! -name
\*.py\* -exec grep -lsm1 '^#!.*\bpython' \{\} \;".


* should I keep the current options from original make-lint?


No, but allow pylint options to be overridable (make lint
PYLINTFLAGS="--disable=python3")

* several false positive errors I haven't been able to fix in 
plugin

yet, in worst case they can be locally disabled:


Disable them locally.

Honza


Updated patch attached.

Please note that make-lint script has been removed, to execute lint
check use 'make lint'




Updated patch attached:
* fixes recently added error
* fixes PEP8
* cleanup of pylint config file

Patch is only for master branch, for 4.3 and 4.2 I will send different
patches when this will be acked




Could you please add an extensive comment to the 4-lines-long find
command? I can after a while (and with the help of man page) decode
what it does so it would definitely help to have it described.
Otherwise it looks good to me.


Updated patch attached, only for master, patches for 4.2, 4.3 will
follow if this one will be ACKed


The comment is probably sufficient, thanks.
Also works for me on current master, ACK.


Pushed to master: 2ce8921fe69ed58871f8e33e3899ad80dcc28c0e

Patches for 4.2 and 4.3 will follow

--
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] [PATCH] 0005 webui: topology graph: canvas resizes itself according to the window size

2016-02-11 Thread Pavel Vomacka

Hello,

The canvas of the graph had static size. This patch fixes this issue and 
from now the graph canvas is resized according to the window size.


Pavel Vomacka
>From 648037bf3952d6cb55c1951d55fad6b05f11f4b7 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Tue, 9 Feb 2016 16:17:08 +0100
Subject: [PATCH] Resize topology graph canvas according to the window size.

Calculate proper size of the svg element which contains the topology graph.
The svg element size is recalculated when the window is resized.

https://fedorahosted.org/freeipa/ticket/5647
---
 install/ui/src/freeipa/topology_graph.js | 32 
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/install/ui/src/freeipa/topology_graph.js b/install/ui/src/freeipa/topology_graph.js
index 1c29c14a42470623fd34e446715eb20a2e98bca8..4d34b7f521b169164a580bb3606792b3ce9fa153 100644
--- a/install/ui/src/freeipa/topology_graph.js
+++ b/install/ui/src/freeipa/topology_graph.js
@@ -27,8 +27,6 @@ var topology_graph = {
  * @class
  */
 topology_graph.TopoGraph = declare([Evented], {
-width: 960,
-height: 500,
 _colors: d3.scale.category10(),
 _svg : null,
 _path: null,
@@ -62,6 +60,15 @@ topology_graph.TopoGraph = declare([Evented], {
  * @param  {HTMLElement} container container where to put the graph svg element
  */
 initialize: function(container) {
+var that = this;
+
+$(window).on("resize", function () {
+clearTimeout(this.delay);
+this.delay = setTimeout( function () {
+that._recalculate_svg_size();
+}, 100);
+});
+
 this._create_svg(container);
 this.update(this.nodes, this.links, this.suffixes);
 return;
@@ -131,8 +138,25 @@ topology_graph.TopoGraph = declare([Evented], {
 _create_svg: function(container) {
 this._svg = d3.select(container[0]).
 append('svg').
-attr('width', this.width).
-attr('height', this.height);
+classed('col-sm-12', true);
+
+this._recalculate_svg_size();
+},
+
+_recalculate_svg_size: function() {
+var right_padding = parseInt( $('svg').css('paddingRight') );
+
+// The right padding of svg is substracted from the height because we want to have
+// the same space at the bottom as on the both sides.
+this.height = $(window).height() - $('svg').offset().top - right_padding;
+this.width = $(window).width() - $('svg').offset().left.toFixed(1) - right_padding;
+
+// Negative numbers cause exceptions in counting position of the graph.
+if (this.height >= 0 && this.width >= 0) {
+this._svg
+.attr("width", this.width)
+.attr("height", this.height);
+}
 },
 
 _init_layout: function() {
-- 
2.5.0

-- 
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 0032] ipatests: add missing certprofile fixture

2016-02-11 Thread Milan Kubík

On 02/11/2016 10:00 AM, Martin Babinsky wrote:

On 02/09/2016 04:06 PM, Milan Kubík wrote:

On 02/09/2016 02:37 PM, Milan Kubík wrote:

Fixes the CA ACL tests broken by removed import. This patch doesn't
rely on undocumented behavior of pytest.

The patch invalidates patch 133 by Martin Babinsky.




Patch updated with trac link

--
Milan Kubik




ACK I guess, although copypasta makes me very sad.

Why do you think the conftest.py approach is not practical for us?

I think introducing new file to hold shared fixtures is not worth 
managing it and it also hides (by the virtue of pytest's working) from 
where the fixture comes. I think it is better in our case to have all 
fixtures the test uses in its module, even though it looks like 
copy-pasting. If I had renamed the entry for the test only, would it 
still been considered copy-paste?


Anyway, patch for ipa-4-3 attached.


--
Milan Kubik

From 7615c5ca6bed0392d1483840c828119eff4306f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Tue, 2 Feb 2016 11:34:26 +0100
Subject: [PATCH] ipatests: Add missing certificate profile fixture

https://fedorahosted.org/freeipa/ticket/5630
---
 ipatests/test_xmlrpc/test_caacl_plugin.py | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_plugin.py b/ipatests/test_xmlrpc/test_caacl_plugin.py
index d5ded19516ec917bcb2e90034b4fac8a6324b2e9..f20b02b295024313008be7b75bdcae2ade70b4ff 100644
--- a/ipatests/test_xmlrpc/test_caacl_plugin.py
+++ b/ipatests/test_xmlrpc/test_caacl_plugin.py
@@ -11,13 +11,21 @@ import pytest
 from ipalib import errors
 from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
 
-# reuse the fixture
-from ipatests.test_xmlrpc.test_certprofile_plugin import default_profile
+from ipatests.test_xmlrpc.tracker.certprofile_plugin import CertprofileTracker
 from ipatests.test_xmlrpc.tracker.caacl_plugin import CAACLTracker
 from ipatests.test_xmlrpc.tracker.stageuser_plugin import StageUserTracker
 
 
 @pytest.fixture(scope='class')
+def default_profile(request):
+name = 'caIPAserviceCert'
+desc = u'Standard profile for network services'
+tracker = CertprofileTracker(name, store=True, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='class')
 def default_acl(request):
 name = u'hosts_services_caIPAserviceCert'
 tracker = CAACLTracker(name, service_category=u'all', host_category=u'all')
-- 
2.7.1

-- 
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] INFO: changes to pylint

2016-02-11 Thread Martin Basti
Please note that current implementation of pylint check has been changed 
due to upcoming changes in pylint 1.5.x


./make-lint  command has been removed

To do lint check, please use * make lint *

Details https://fedorahosted.org/freeipa/ticket/5615

--
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] [TESTS][PATCH 0009] WebUI tests fix

2016-02-11 Thread Lenka Doudova

Hi all,

most of webUI tests fail with

AssertionError: Can't click on checkbox label: table.table
Message: Element is not clickable at point (37, 340.338964844). Other 
element would receive the click:



The problem seems to be that the tests attempt to click on a checkbox 
label that is no longer there. Since the checkbox is clickable directly, 
I changed the code accordingly. Most of the tests should now proceed 
successfully.


Lenka
From c628f695cc1441b0bde7ec41bee811fbad5fd92e Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 10 Feb 2016 16:16:22 +0100
Subject: [PATCH] WebUI tests: fix failing tests

---
 ipatests/test_webui/ui_driver.py | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py
index fc22f8612d49e300437eb91cb58add1a0376eb2c..1c6a852b823165a0bcdf33bb16b1939bf650958d 100644
--- a/ipatests/test_webui/ui_driver.py
+++ b/ipatests/test_webui/ui_driver.py
@@ -933,12 +933,9 @@ class UI_driver(object):
 s = self.get_table_selector(table_name)
 input_s = s + " tbody td input[value='%s']" % pkey
 checkbox = self.find(input_s, By.CSS_SELECTOR, parent, strict=True)
-checkbox_id = checkbox.get_attribute('id')
-label_s = s + " tbody td label[for='%s']" % checkbox_id
-print(label_s)
-label = self.find(label_s, By.CSS_SELECTOR, parent, strict=True)
 try:
-ActionChains(self.driver).move_to_element(label).click().perform()
+ActionChains(self.driver).move_to_element(checkbox)\
+.click().perform()
 except WebDriverException as e:
 assert False, 'Can\'t click on checkbox label: %s \n%s' % (s, e)
 self.wait()
-- 
2.5.0

-- 
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 0032] ipatests: add missing certprofile fixture

2016-02-11 Thread Martin Babinsky

On 02/11/2016 02:08 PM, Milan Kubík wrote:

On 02/11/2016 10:00 AM, Martin Babinsky wrote:

On 02/09/2016 04:06 PM, Milan Kubík wrote:

On 02/09/2016 02:37 PM, Milan Kubík wrote:

Fixes the CA ACL tests broken by removed import. This patch doesn't
rely on undocumented behavior of pytest.

The patch invalidates patch 133 by Martin Babinsky.




Patch updated with trac link

--
Milan Kubik




ACK I guess, although copypasta makes me very sad.

Why do you think the conftest.py approach is not practical for us?


I think introducing new file to hold shared fixtures is not worth
managing it and it also hides (by the virtue of pytest's working) from
where the fixture comes. I think it is better in our case to have all
fixtures the test uses in its module, even though it looks like
copy-pasting. If I had renamed the entry for the test only, would it
still been considered copy-paste?

Anyway, patch for ipa-4-3 attached.



ACK to both.

--
Martin^3 Babinsky

--
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] 0007 Refactor test_sudocmd_plugin

2016-02-11 Thread Aleš Mareček
NACK.

"create_sudocmd" and "delete_sudocmd" should be placed in Tracker. So this 
patch should create the tracker as well.

- Original Message -
> From: "Filip Skola" 
> To: freeipa-devel@redhat.com
> Sent: Monday, January 25, 2016 3:57:25 PM
> Subject: [Freeipa-devel] [PATCH] 0007 Refactor test_sudocmd_plugin
> 
> Hello,
> 
> attaching refactored sudocmd_plugin.
> 
> Filip
> --
> 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

-- 
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 0032] ipatests: add missing certprofile fixture

2016-02-11 Thread Martin Basti



On 11.02.2016 14:12, Martin Babinsky wrote:

On 02/11/2016 02:08 PM, Milan Kubík wrote:

On 02/11/2016 10:00 AM, Martin Babinsky wrote:

On 02/09/2016 04:06 PM, Milan Kubík wrote:

On 02/09/2016 02:37 PM, Milan Kubík wrote:

Fixes the CA ACL tests broken by removed import. This patch doesn't
rely on undocumented behavior of pytest.

The patch invalidates patch 133 by Martin Babinsky.




Patch updated with trac link

--
Milan Kubik




ACK I guess, although copypasta makes me very sad.

Why do you think the conftest.py approach is not practical for us?


I think introducing new file to hold shared fixtures is not worth
managing it and it also hides (by the virtue of pytest's working) from
where the fixture comes. I think it is better in our case to have all
fixtures the test uses in its module, even though it looks like
copy-pasting. If I had renamed the entry for the test only, would it
still been considered copy-paste?

Anyway, patch for ipa-4-3 attached.



ACK to both.


Pushed to master: 87ee451c7d9b311192893b2a2c82d8d757281fa6
Pushed to ipa-4-3: 8aec20124d415daeea3764294224fea47f8e7b0a


--
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] 0008 Refactor test_sudocmdgroup_plugin, create SudoCmdGroupTracker

2016-02-11 Thread Aleš Mareček
NACK.

"create_sudocmd" and "delete_sudocmd" should be imported from Tracker, not from 
the previous test (sudocmd_plugin).

  - alich -

- Original Message -
> From: "Filip Skola" 
> To: freeipa-devel@redhat.com
> Sent: Thursday, January 28, 2016 12:49:17 PM
> Subject: [Freeipa-devel] [PATCH] 0008 Refactor test_sudocmdgroup_plugin, 
> create SudoCmdGroupTracker
> 
> Hi,
> 
> sending the next sudo patch. This one depends on the previous one
> (sudocmd_plugin).
> 
> Filip
> 
> --
> 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

-- 
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 0030] Modernize mod_nss's cipher suites

2016-02-11 Thread Martin Kosek
On 02/11/2016 10:45 AM, Martin Basti wrote:
> 
> 
> On 03.02.2016 15:35, Christian Heimes wrote:
>> On 2016-01-29 15:05, Martin Basti wrote:
>>>
>>> On 29.01.2016 14:42, Christian Heimes wrote:
 On 2016-01-28 09:47, Martin Basti wrote:
> On 22.01.2016 12:32, Martin Kosek wrote:
>> On 01/21/2016 04:21 PM, Christian Heimes wrote:
>>> The list of supported TLS cipher suites in /etc/httpd/conf.d/nss.conf
>>> has been modernized. Insecure or less secure algorithms such as RC4,
>>> DES and 3DES are removed. Perfect forward secrecy suites with
>>> ephemeral
>>> ECDH key exchange have been added. IE 8 on Windows XP is no longer
>>> supported.
>>>
>>> The list of enabled cipher suites has been generated with the script
>>> contrib/nssciphersuite/nssciphersuite.py.
>>>
>>> The supported suites are currently:
>>>
>>> TLS_RSA_WITH_AES_128_CBC_SHA256
>>> TLS_RSA_WITH_AES_256_CBC_SHA256
>>> TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
>>> TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
>>> TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
>>> TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
>>> TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
>>> TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
>>> TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
>>> TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
>>> TLS_RSA_WITH_AES_128_GCM_SHA256
>>> TLS_RSA_WITH_AES_128_CBC_SHA
>>> TLS_RSA_WITH_AES_256_GCM_SHA384
>>> TLS_RSA_WITH_AES_256_CBC_SHA
>>>
>>> https://fedorahosted.org/freeipa/ticket/5589
>> Thanks for the patch! I updated the ticket to make sure this change is
>> release notes.
>>
> Hello,
>
> I'm not sure if I'm the right person to do review on this, but I will
> try :-)
>
> 1)
> Your patch adds whitespace error
>
> Applying: Modernize mod_nss's cipher suites
> /home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:52: new blank
> line at EOF.
> +
> warning: 1 line adds whitespace errors.
>
>
> 2)
> +import urllib.request  # pylint: disable=E0611
>
> Please specify pylint disabled check by name
>
> 3)
> +def update_mod_nss_cipher_suite(http):
>
> in this upgrade, is there any possibility that ciphers might be upgraded
> again in future? (IMO yes).
>
> I think, it can be better to store revision of change instead of boolean
>
> LAST_REVISION =  1
>
> if revision >= LAST_REVISION:
>   return
>
> sysupgrade.set_upgrade_state('nss.conf', 'cipher_suite_revision',
> LAST_REVISION)
 Thanks for the review. I have addressed the problems. Instead of a
 revision number I'm using a date string. The sysupgrade module only
 stores str and bool. With a date-based revision it's easy to see when
 the cipher suite was checked last time.

 Christian

>>> Thanks
>>>
>>> 1) Pylint :-)
>>> +with urllib.request.urlopen(SOURCE) as r:  # pylint: disable=E1101
>> Thanks! It was easier to change the import to get rid of the second
>> pylint stanza.
>>
>>> 2)
>>> +if revision == httpinstance.NSS_CIPHER_REVISION:
>>>
>>> may happen a case where just comparation with '==' can cause a issues
>>> (docker world)? Should not be there rather '>='?
>> Makes sense, I've changed the comparison operator to >=. This may still
>> override user settings, though.
>>
>>> 3)
>>> +root_logger.info("Cipher suite already updated")
>>>
>>> Sorry that I did not noticed earlier, this should be just debug level,
>>> IMO this message is not so important, it will cause only mess on output
>>> (we already have plenty of unneeded info messages in upgrade, they will
>>> be fixed once)
>> Fine with me :)
>>
>> Christian
> ACK
> 
> Pushed to:
> master: 5ac3a3cee534a16db86c541b9beff4939f03410e
> ipa-4-3: c3496a4a4893c75789bdf0c617e46923361fb43b
> 

Very cool! Thanks guys! Looking forward to deploying FreeIPA 4.3.1 on the
FreeIPA public demo :-)

-- 
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 0411] upgrade: log to ipaupgrade.log if ipa is not installed

2016-02-11 Thread Martin Babinsky

On 01/29/2016 10:48 AM, Martin Basti wrote:

Missing record in ipaupgrade.log that upgrade failed because IPA is not
installed, causes harder time to debugging upgrade from log.

Patch attached.



ACK.

--
Martin^3 Babinsky

--
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 0411] upgrade: log to ipaupgrade.log if ipa is not installed

2016-02-11 Thread Martin Basti



On 11.02.2016 18:34, Martin Babinsky wrote:

On 01/29/2016 10:48 AM, Martin Basti wrote:

Missing record in ipaupgrade.log that upgrade failed because IPA is not
installed, causes harder time to debugging upgrade from log.

Patch attached.



ACK.


Pushed to master: 0ea7433d09e24904a06d6ed02c9a89b1ea4bbc43

--
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 544] replica promotion: fix AVC denials in remote connection check

2016-02-11 Thread Martin Basti



On 04.02.2016 11:18, Martin Babinsky wrote:

On 02/04/2016 09:40 AM, Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza




Connection check for replica/CA install now works in enforcing mode, ACK.


Pushed to:
master: b3411dc985c26146c91c2733ebdc0c098ec22266
ipa-4-3: a0c06038f44ec450eb0dca0e789a71b97def3abc

--
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 0407] make lint: migration to config file and pylint plugin due pylint 1.5.2

2016-02-11 Thread Martin Basti



On 11.02.2016 11:08, Martin Basti wrote:



On 09.02.2016 15:26, David Kupka wrote:

On 09/02/16 11:28, Martin Basti wrote:



On 03.02.2016 10:19, David Kupka wrote:

On 01/02/16 14:18, Martin Basti wrote:



On 26.01.2016 14:16, Martin Basti wrote:



On 20.01.2016 14:38, Jan Cholasta wrote:

Hi,

On 19.1.2016 13:43, Martin Basti wrote:

New pylint version will broke our custom make-lint script again,
attached patch migrates make-lint to:
* config file
* pylint plugin
which are supported by pylint and should not have regular
compatibility
issues

to test new approach run ./make-lint2

Advantages:
* compatibility with pylint
* works on both pylint-1.4.3-3.fc23.noarch and
pylint-1.5.2-1.fc24.noarch
* pylint plugin works in different way than the previous custom
checker.
Missing ("dynamic") attributes are added to abstract syntax tree
instead
of ignoring them and all their sub-members. This makes check 
better,
pylint can detect more typos in tests configurations, api, env, 
etc..


Disadvantages:
* any new attribute in api, test config, etc.. must be added to
definition of missing members (pylint plugin) - this should not
happen
too often


1) Please "mv pylint_plugins/fix_ipa_members.py pylint_plugins.py"
and "rm -rf pylint_plugins/", no need for this redundant directory
structure.

2) Rename pylintrc to freeipa.pylintrc so you have to always 
specify

it explicitly with --rcfile.

3) Use the load-plugins directive in freeipa.pylintrc to load the
plugins rather than --load-plugins.

4) Instead of running pylint twice, run it only once with both 
normal

and Python 3 checks enabled:

[MESSAGE CONTROL]
enable=all,python3
disable=...,no-absolute-import




Q:
* make-lint: should it be just bash script or rather python 
script?


IMO neither, it should be a make target (make lint).


* add dynamic detection of python files to be checked


You can use "find . -type f -executable ! -path \*/.\* ! -name
\*.py\* -exec grep -lsm1 '^#!.*\bpython' \{\} \;".


* should I keep the current options from original make-lint?


No, but allow pylint options to be overridable (make lint
PYLINTFLAGS="--disable=python3")

* several false positive errors I haven't been able to fix in 
plugin

yet, in worst case they can be locally disabled:


Disable them locally.

Honza


Updated patch attached.

Please note that make-lint script has been removed, to execute lint
check use 'make lint'




Updated patch attached:
* fixes recently added error
* fixes PEP8
* cleanup of pylint config file

Patch is only for master branch, for 4.3 and 4.2 I will send 
different

patches when this will be acked




Could you please add an extensive comment to the 4-lines-long find
command? I can after a while (and with the help of man page) decode
what it does so it would definitely help to have it described.
Otherwise it looks good to me.


Updated patch attached, only for master, patches for 4.2, 4.3 will
follow if this one will be ACKed


The comment is probably sufficient, thanks.
Also works for me on current master, ACK.


Pushed to master: 2ce8921fe69ed58871f8e33e3899ad80dcc28c0e

Patches for 4.2 and 4.3 will follow


Patches for 4.2, 4.3 attached


From 8b0617083d181b4615312a03732f36e3adcf6d52 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 15 Jan 2016 16:58:38 +0100
Subject: [PATCH] make lint: use config file and plugin for pylint

Our custom implementation of pylint checker is often broken by
incompatible change on pylint side. Using supported solutions (config
file, pylint plugins) should avoid this issue.

The plugin adds missing (dynamic) member to classes in abstract syntax
tree generated for pylint, instead of just ignoring missing members and
all sub-members. This should improve pylint detection of typos and
missing members in api. env and test config.

make-lint python script has been removed, to run pylint execute 'make
lint'

https://fedorahosted.org/freeipa/ticket/5615
---
 Makefile|  14 +-
 ipalib/krb_utils.py |   8 +-
 ipalib/plugins/vault.py |   4 +
 ipatests/test_ipapython/test_ipautil.py |   2 +
 make-lint   | 280 
 pylint_plugins.py   | 211 
 pylintrc|  97 +++
 7 files changed, 330 insertions(+), 286 deletions(-)
 delete mode 100755 make-lint
 create mode 100644 pylint_plugins.py
 create mode 100644 pylintrc

diff --git a/Makefile b/Makefile
index 4a09c96d317389b705cb390a45baa970cca3a480..783bbe0e6aec41f9ea1e62266bff2bf84a131582 100644
--- a/Makefile
+++ b/Makefile
@@ -53,7 +53,9 @@ LIBDIR ?= /usr/lib
 
 DEVELOPER_MODE ?= 0
 ifneq ($(DEVELOPER_MODE),0)
-LINT_OPTIONS=--no-fail
+LINT_IGNORE_FAIL=true
+else
+LINT_IGNORE_FAIL=false
 endif
 
 PYTHON ?= $(shell rpm -E %__python || echo /usr/bin/python2)
@@ -119,8 +121,14 @@ client-dirs:
 	fi
 
 lint: bootstrap-autogen
-