Re: [Freeipa-devel] [PATCH] 0049 Remove workaround for CA running check

2016-01-26 Thread Jan Cholasta

On 21.1.2016 14:10, Martin Basti wrote:



On 20.01.2016 10:52, Fraser Tweedale wrote:

On Wed, Jan 20, 2016 at 09:30:29AM +0100, Martin Kosek wrote:

On 01/20/2016 08:45 AM, Fraser Tweedale wrote:

The attached patch removes a workaround introduced as part of
https://fedorahosted.org/freeipa/ticket/4676.

Alternatively, if we want to keep the "workaround" I will submit a
different patch that removes unused code and FIXME comments :)

Cheers,
Fraser

You may also want to check FreeIPA spec file, if there is now no
extra curl
dependency. I would leave it up to Martin Basti, to confirm that the
original
issue cannot appear again. It was a nightmare to troubleshoot, as I
heard :)


Good pickup on the curl dependency; indeed it is no longer needed.
Updated patch attached.

Thank you, patch works for me. However, I'm not sure where the original
error was located, it looked like something in _httplib_request doesn't
work properly with SSL. Your patch uses _httplib_request without TLS so
it should work.

I would like to push this patch only to master, as the issue before
wasn't regularly reproducible, and I will keep eye on it.

Also I will remove the ticket #4676 from description, because ticket has
been closed in 4.1 Milestone.

ACK with keeping eyes on it

Pushed to master: fd7ea2c9395651d5bce41cc603557fea107f65a7


Please don't introduce additional patches to tickets closed in released 
milestones. You should open a new ticket for the additional change so 
that it can be properly triaged and you don't have to guess where it 
should be pushed.


Honza

--
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] 0049 Remove workaround for CA running check

2016-01-21 Thread Martin Basti



On 20.01.2016 10:52, Fraser Tweedale wrote:

On Wed, Jan 20, 2016 at 09:30:29AM +0100, Martin Kosek wrote:

On 01/20/2016 08:45 AM, Fraser Tweedale wrote:

The attached patch removes a workaround introduced as part of
https://fedorahosted.org/freeipa/ticket/4676.

Alternatively, if we want to keep the "workaround" I will submit a
different patch that removes unused code and FIXME comments :)

Cheers,
Fraser

You may also want to check FreeIPA spec file, if there is now no extra curl
dependency. I would leave it up to Martin Basti, to confirm that the original
issue cannot appear again. It was a nightmare to troubleshoot, as I heard :)


Good pickup on the curl dependency; indeed it is no longer needed.
Updated patch attached.
Thank you, patch works for me. However, I'm not sure where the original 
error was located, it looked like something in _httplib_request doesn't 
work properly with SSL. Your patch uses _httplib_request without TLS so 
it should work.


I would like to push this patch only to master, as the issue before 
wasn't regularly reproducible, and I will keep eye on it.


Also I will remove the ticket #4676 from description, because ticket has 
been closed in 4.1 Milestone.


ACK with keeping eyes on it

Pushed to master: fd7ea2c9395651d5bce41cc603557fea107f65a7

--
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] 0049 Remove workaround for CA running check

2016-01-20 Thread Fraser Tweedale
On Wed, Jan 20, 2016 at 07:52:32PM +1000, Fraser Tweedale wrote:
> Good pickup on the curl dependency; indeed it is no longer needed.
> Updated patch attached.
>
Whups, that was same patch, different name. *Here* is the new patch.
From ba5750b7a805841abd8d4795d9c4bcec2a3518a0 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 20 Jan 2016 18:35:15 +1100
Subject: [PATCH] Remove workaround for CA running check

A workaround was introduced for ticket #4676 that used wget to
perform an (unauthenticated) https request to check the CA status.
Later, wget was changed to curl (the request remained
unauthenticated).

Remove the workaround and use an http request (no TLS) to check the
CA status.  Also remove the now-unused unauthenticated_http_request
method, and update specfile to remove ipalib dependency on curl.

https://fedorahosted.org/freeipa/ticket/4676
---
 freeipa.spec.in|  2 --
 ipaplatform/redhat/services.py | 25 +
 ipapython/dogtag.py| 25 +++--
 3 files changed, 4 insertions(+), 48 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
961d8c38e0dd5f954bfca47e8209a5655eaacc86..ae0887390d623b035734dc5c8da703ba33a37e9f
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -466,7 +466,6 @@ Requires: python-pyasn1
 Requires: python-dateutil
 Requires: python-yubico >= 1.2.3
 Requires: python-sss-murmur
-Requires: curl
 Requires: dbus-python
 Requires: python-setuptools
 Requires: python-six
@@ -510,7 +509,6 @@ Requires: python3-pyasn1
 Requires: python3-dateutil
 Requires: python3-yubico >= 1.2.3
 Requires: python3-sss-murmur
-Requires: curl
 Requires: python3-dbus
 Requires: python3-setuptools
 Requires: python3-six
diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 
11292fa4912844db78899d779b84104288e469dc..3c18dbc3c1274ef3852abef5f054b4e37e6b32fa
 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -199,30 +199,7 @@ class RedHatCAService(RedHatService):
 op_timeout = time.time() + timeout
 while time.time() < op_timeout:
 try:
-# FIXME https://fedorahosted.org/freeipa/ticket/4716
-# workaround
-#
-# status = dogtag.ca_status(use_proxy=use_proxy)
-#
-port = 8443
-
-url = "https://%(host_port)s%(path)s" % {
-"host_port": ipautil.format_netloc(api.env.ca_host, port),
-"path": "/ca/admin/ca/getStatus"
-}
-
-args = [
-paths.BIN_CURL,
-'-o', '-',
-'--connect-timeout', '30',
-'-k',
-url
-]
-
-result = ipautil.run(args, capture_output=True)
-
-status = dogtag._parse_ca_status(result.output)
-# end of workaround
+status = dogtag.ca_status()
 except Exception as e:
 status = 'check interrupted due to error: %s' % e
 root_logger.debug('The CA status is: %s' % status)
diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index 
1cb74719c4ce2cc97c54dc7bebfa4b32ceee14a1..6f13880026e9e6043649405245c9cd50a826f652
 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -103,7 +103,7 @@ def _parse_ca_status(body):
 raise error_from_xml(doc, _("Retrieving CA status failed: %s"))
 
 
-def ca_status(ca_host=None, use_proxy=True):
+def ca_status(ca_host=None):
 """Return the status of the CA, and the httpd proxy in front of it
 
 The returned status can be:
@@ -113,13 +113,8 @@ def ca_status(ca_host=None, use_proxy=True):
 """
 if ca_host is None:
 ca_host = api.env.ca_host
-if use_proxy:
-# Use port 443 to test the proxy as well
-ca_port = 443
-else:
-ca_port = 8443
-status, headers, body = unauthenticated_https_request(
-ca_host, ca_port, '/ca/admin/ca/getStatus')
+status, headers, body = http_request(
+ca_host, 8080, '/ca/admin/ca/getStatus')
 if status == 503:
 # Service temporarily unavailable
 return status
@@ -175,20 +170,6 @@ def http_request(host, port, url, **kw):
 'http', host, port, url, httplib.HTTPConnection, body)
 
 
-def unauthenticated_https_request(host, port, url, **kw):
-"""
-:param url: The path (not complete URL!) to post to.
-:param kw: Keyword arguments to encode into POST body.
-:return:   (http_status, http_headers, http_body)
-as (integer, dict, str)
-
-Perform an unauthenticated HTTPS request.
-"""
-body = urlencode(kw)
-return _httplib_request(
-'https', host, port, url, httplib.HTTPSConnection, body)
-
-
 def _httplib_request(
 protocol, host, port, path, connection_factory, request_body,
 method='POST', headers=None):
-- 
2.5

Re: [Freeipa-devel] [PATCH] 0049 Remove workaround for CA running check

2016-01-20 Thread Fraser Tweedale
On Wed, Jan 20, 2016 at 09:30:29AM +0100, Martin Kosek wrote:
> On 01/20/2016 08:45 AM, Fraser Tweedale wrote:
> > The attached patch removes a workaround introduced as part of
> > https://fedorahosted.org/freeipa/ticket/4676.
> > 
> > Alternatively, if we want to keep the "workaround" I will submit a
> > different patch that removes unused code and FIXME comments :)
> > 
> > Cheers,
> > Fraser
> 
> You may also want to check FreeIPA spec file, if there is now no extra curl
> dependency. I would leave it up to Martin Basti, to confirm that the original
> issue cannot appear again. It was a nightmare to troubleshoot, as I heard :)
>
Good pickup on the curl dependency; indeed it is no longer needed.
Updated patch attached.
From df99d69569ddc173c7495eb5cd85133079a24ba9 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 20 Jan 2016 18:35:15 +1100
Subject: [PATCH] Remove workaround for CA running check

A workaround was introduced for ticket #4676 that used wget to
perform an (unauthenticated) https request to check the CA status.
Later, wget was changed to curl (the request remained
unauthenticated).

Remove the workaround and use an http request (no TLS) to check the
CA status.  Also remove the now-unused unauthenticated_http_request
method.

https://fedorahosted.org/freeipa/ticket/4676
---
 ipaplatform/redhat/services.py | 25 +
 ipapython/dogtag.py| 25 +++--
 2 files changed, 4 insertions(+), 46 deletions(-)

diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 
11292fa4912844db78899d779b84104288e469dc..3c18dbc3c1274ef3852abef5f054b4e37e6b32fa
 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -199,30 +199,7 @@ class RedHatCAService(RedHatService):
 op_timeout = time.time() + timeout
 while time.time() < op_timeout:
 try:
-# FIXME https://fedorahosted.org/freeipa/ticket/4716
-# workaround
-#
-# status = dogtag.ca_status(use_proxy=use_proxy)
-#
-port = 8443
-
-url = "https://%(host_port)s%(path)s" % {
-"host_port": ipautil.format_netloc(api.env.ca_host, port),
-"path": "/ca/admin/ca/getStatus"
-}
-
-args = [
-paths.BIN_CURL,
-'-o', '-',
-'--connect-timeout', '30',
-'-k',
-url
-]
-
-result = ipautil.run(args, capture_output=True)
-
-status = dogtag._parse_ca_status(result.output)
-# end of workaround
+status = dogtag.ca_status()
 except Exception as e:
 status = 'check interrupted due to error: %s' % e
 root_logger.debug('The CA status is: %s' % status)
diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index 
1cb74719c4ce2cc97c54dc7bebfa4b32ceee14a1..6f13880026e9e6043649405245c9cd50a826f652
 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -103,7 +103,7 @@ def _parse_ca_status(body):
 raise error_from_xml(doc, _("Retrieving CA status failed: %s"))
 
 
-def ca_status(ca_host=None, use_proxy=True):
+def ca_status(ca_host=None):
 """Return the status of the CA, and the httpd proxy in front of it
 
 The returned status can be:
@@ -113,13 +113,8 @@ def ca_status(ca_host=None, use_proxy=True):
 """
 if ca_host is None:
 ca_host = api.env.ca_host
-if use_proxy:
-# Use port 443 to test the proxy as well
-ca_port = 443
-else:
-ca_port = 8443
-status, headers, body = unauthenticated_https_request(
-ca_host, ca_port, '/ca/admin/ca/getStatus')
+status, headers, body = http_request(
+ca_host, 8080, '/ca/admin/ca/getStatus')
 if status == 503:
 # Service temporarily unavailable
 return status
@@ -175,20 +170,6 @@ def http_request(host, port, url, **kw):
 'http', host, port, url, httplib.HTTPConnection, body)
 
 
-def unauthenticated_https_request(host, port, url, **kw):
-"""
-:param url: The path (not complete URL!) to post to.
-:param kw: Keyword arguments to encode into POST body.
-:return:   (http_status, http_headers, http_body)
-as (integer, dict, str)
-
-Perform an unauthenticated HTTPS request.
-"""
-body = urlencode(kw)
-return _httplib_request(
-'https', host, port, url, httplib.HTTPSConnection, body)
-
-
 def _httplib_request(
 protocol, host, port, path, connection_factory, request_body,
 method='POST', headers=None):
-- 
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] 0049 Remove workaround for CA running check

2016-01-20 Thread Martin Kosek
On 01/20/2016 08:45 AM, Fraser Tweedale wrote:
> The attached patch removes a workaround introduced as part of
> https://fedorahosted.org/freeipa/ticket/4676.
> 
> Alternatively, if we want to keep the "workaround" I will submit a
> different patch that removes unused code and FIXME comments :)
> 
> Cheers,
> Fraser

You may also want to check FreeIPA spec file, if there is now no extra curl
dependency. I would leave it up to Martin Basti, to confirm that the original
issue cannot appear again. It was a nightmare to troubleshoot, as I heard :)

-- 
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] 0049 Remove workaround for CA running check

2016-01-19 Thread Fraser Tweedale
The attached patch removes a workaround introduced as part of
https://fedorahosted.org/freeipa/ticket/4676.

Alternatively, if we want to keep the "workaround" I will submit a
different patch that removes unused code and FIXME comments :)

Cheers,
Fraser
From df99d69569ddc173c7495eb5cd85133079a24ba9 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 20 Jan 2016 18:35:15 +1100
Subject: [PATCH] Remove workaround for CA running check

A workaround was introduced for ticket #4676 that used wget to
perform an (unauthenticated) https request to check the CA status.
Later, wget was changed to curl (the request remained
unauthenticated).

Remove the workaround and use an http request (no TLS) to check the
CA status.  Also remove the now-unused unauthenticated_http_request
method.

https://fedorahosted.org/freeipa/ticket/4676
---
 ipaplatform/redhat/services.py | 25 +
 ipapython/dogtag.py| 25 +++--
 2 files changed, 4 insertions(+), 46 deletions(-)

diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 
11292fa4912844db78899d779b84104288e469dc..3c18dbc3c1274ef3852abef5f054b4e37e6b32fa
 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -199,30 +199,7 @@ class RedHatCAService(RedHatService):
 op_timeout = time.time() + timeout
 while time.time() < op_timeout:
 try:
-# FIXME https://fedorahosted.org/freeipa/ticket/4716
-# workaround
-#
-# status = dogtag.ca_status(use_proxy=use_proxy)
-#
-port = 8443
-
-url = "https://%(host_port)s%(path)s" % {
-"host_port": ipautil.format_netloc(api.env.ca_host, port),
-"path": "/ca/admin/ca/getStatus"
-}
-
-args = [
-paths.BIN_CURL,
-'-o', '-',
-'--connect-timeout', '30',
-'-k',
-url
-]
-
-result = ipautil.run(args, capture_output=True)
-
-status = dogtag._parse_ca_status(result.output)
-# end of workaround
+status = dogtag.ca_status()
 except Exception as e:
 status = 'check interrupted due to error: %s' % e
 root_logger.debug('The CA status is: %s' % status)
diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index 
1cb74719c4ce2cc97c54dc7bebfa4b32ceee14a1..6f13880026e9e6043649405245c9cd50a826f652
 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -103,7 +103,7 @@ def _parse_ca_status(body):
 raise error_from_xml(doc, _("Retrieving CA status failed: %s"))
 
 
-def ca_status(ca_host=None, use_proxy=True):
+def ca_status(ca_host=None):
 """Return the status of the CA, and the httpd proxy in front of it
 
 The returned status can be:
@@ -113,13 +113,8 @@ def ca_status(ca_host=None, use_proxy=True):
 """
 if ca_host is None:
 ca_host = api.env.ca_host
-if use_proxy:
-# Use port 443 to test the proxy as well
-ca_port = 443
-else:
-ca_port = 8443
-status, headers, body = unauthenticated_https_request(
-ca_host, ca_port, '/ca/admin/ca/getStatus')
+status, headers, body = http_request(
+ca_host, 8080, '/ca/admin/ca/getStatus')
 if status == 503:
 # Service temporarily unavailable
 return status
@@ -175,20 +170,6 @@ def http_request(host, port, url, **kw):
 'http', host, port, url, httplib.HTTPConnection, body)
 
 
-def unauthenticated_https_request(host, port, url, **kw):
-"""
-:param url: The path (not complete URL!) to post to.
-:param kw: Keyword arguments to encode into POST body.
-:return:   (http_status, http_headers, http_body)
-as (integer, dict, str)
-
-Perform an unauthenticated HTTPS request.
-"""
-body = urlencode(kw)
-return _httplib_request(
-'https', host, port, url, httplib.HTTPSConnection, body)
-
-
 def _httplib_request(
 protocol, host, port, path, connection_factory, request_body,
 method='POST', headers=None):
-- 
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