You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Aravindan Vijayan <av...@hortonworks.com> on 2017/03/28 22:03:49 UTC

Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/
-----------------------------------------------------------

Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.


Bugs: AMBARI-20600
    https://issues.apache.org/jira/browse/AMBARI-20600


Repository: ambari


Description
-------

EXCEPTION TRACE

  File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
    response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
  File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
    grafana_https_enabled, ca_certs)
  File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
    ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
  File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
    .format(host, port, ca_certs, str(ssl_error)))
resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed


PROBLEM
The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.

FIX
Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.


Diffs
-----

  ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
  ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
  ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
  ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 


Diff: https://reviews.apache.org/r/58006/diff/1/


Testing
-------

Manually tested.
Python unit tests passed.


Thanks,

Aravindan Vijayan


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170429
-----------------------------------------------------------


Ship it!




Ship It!

- Robert Levas


On March 29, 2017, 1:06 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 29, 2017, 1:06 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/2/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Aravindan Vijayan <av...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/
-----------------------------------------------------------

(Updated March 29, 2017, 5:06 p.m.)


Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.


Changes
-------

Fixed review issue.


Bugs: AMBARI-20600
    https://issues.apache.org/jira/browse/AMBARI-20600


Repository: ambari


Description
-------

EXCEPTION TRACE

  File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
    response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
  File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
    grafana_https_enabled, ca_certs)
  File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
    ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
  File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
    .format(host, port, ca_certs, str(ssl_error)))
resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed


PROBLEM
The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.

FIX
Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.


Diffs (updated)
-----

  ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
  ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
  ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
  ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 


Diff: https://reviews.apache.org/r/58006/diff/2/

Changes: https://reviews.apache.org/r/58006/diff/1-2/


Testing
-------

Manually tested.
Python unit tests passed.


Thanks,

Aravindan Vijayan


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Sid Wagle <sw...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170345
-----------------------------------------------------------


Ship it!




Ship It!

- Sid Wagle


On March 28, 2017, 10:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 10:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On March 28, 2017, 11:57 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/ambari_commons/network.py
> > Lines 65-66 (original), 64-65 (patched)
> > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66>
> >
> >     Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
> >     
> >     Also, this function will return `ssl.PROTOCOL_SSLv23` 
> >     no matter what, so the caller has no indication that an error may have occurred.
> 
> Aravindan Vijayan wrote:
>     I will make the check certificate step configurable.
> 
> Sid Wagle wrote:
>     I am not sure about making cert check failure/sucess configurable. It just sounds like an overkill. Why not check and warn if there is an issue with the check. The daemon launching with sucess or failure will in any case determine if the cert is valid and configured correctly. There is effectively no stack script that does cert checks like this :-)
> 
> Robert Levas wrote:
>     The daemon (serve-side of this equation?) cannot be trusted to to verify it's own cert, this is why the client should do it.  I think (basically) silently warning the user the server cannot be trusted can be a problem in some situations, depending on the level of paranoia a user may have. And if a user has gone through the trouble of getting a _real_ SSL certificate (as opposed to a self-signed SSL certificate), then they probably want to know if the server's cert is not valid.

IMO, we should go with either printing warning, or failing fast. Making it configurable just adds one more step of complexity which the user might not really use.


- Aravindan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


On March 28, 2017, 10:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 10:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Sumit Mohanty <sm...@hortonworks.com>.

> On March 28, 2017, 11:57 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/ambari_commons/network.py
> > Lines 65-66 (original), 64-65 (patched)
> > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66>
> >
> >     Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
> >     
> >     Also, this function will return `ssl.PROTOCOL_SSLv23` 
> >     no matter what, so the caller has no indication that an error may have occurred.
> 
> Aravindan Vijayan wrote:
>     I will make the check certificate step configurable.
> 
> Sid Wagle wrote:
>     I am not sure about making cert check failure/sucess configurable. It just sounds like an overkill. Why not check and warn if there is an issue with the check. The daemon launching with sucess or failure will in any case determine if the cert is valid and configured correctly. There is effectively no stack script that does cert checks like this :-)
> 
> Robert Levas wrote:
>     The daemon (serve-side of this equation?) cannot be trusted to to verify it's own cert, this is why the client should do it.  I think (basically) silently warning the user the server cannot be trusted can be a problem in some situations, depending on the level of paranoia a user may have. And if a user has gone through the trouble of getting a _real_ SSL certificate (as opposed to a self-signed SSL certificate), then they probably want to know if the server's cert is not valid.
> 
> Aravindan Vijayan wrote:
>     IMO, we should go with either printing warning, or failing fast. Making it configurable just adds one more step of complexity which the user might not really use.
> 
> Robert Levas wrote:
>     How about failing fast and allow the caller to catch the exception and either log and continue or log and fail?  I think this is bascially the functionailty that we currently have - which is being changed by this patch. 
>     
>     It appears that `ambari_commons.network.get_http_connection` conditionally checks the server's SSL certificate if the `ca_certs` value is not `None`. So the caller is already indicating whether a validation check is required or not.

Who are the callers for this function? Are both expected to have different behavior or the same wrt processing the erorr?


- Sumit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


On March 28, 2017, 10:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 10:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Robert Levas <rl...@hortonworks.com>.

> On March 28, 2017, 7:57 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/ambari_commons/network.py
> > Lines 65-66 (original), 64-65 (patched)
> > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66>
> >
> >     Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
> >     
> >     Also, this function will return `ssl.PROTOCOL_SSLv23` 
> >     no matter what, so the caller has no indication that an error may have occurred.
> 
> Aravindan Vijayan wrote:
>     I will make the check certificate step configurable.
> 
> Sid Wagle wrote:
>     I am not sure about making cert check failure/sucess configurable. It just sounds like an overkill. Why not check and warn if there is an issue with the check. The daemon launching with sucess or failure will in any case determine if the cert is valid and configured correctly. There is effectively no stack script that does cert checks like this :-)

The daemon (serve-side of this equation?) cannot be trusted to to verify it's own cert, this is why the client should do it.  I think (basically) silently warning the user the server cannot be trusted can be a problem in some situations, depending on the level of paranoia a user may have. And if a user has gone through the trouble of getting a _real_ SSL certificate (as opposed to a self-signed SSL certificate), then they probably want to know if the server's cert is not valid.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


On March 28, 2017, 6:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 6:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Sid Wagle <sw...@hortonworks.com>.

> On March 28, 2017, 11:57 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/ambari_commons/network.py
> > Lines 65-66 (original), 64-65 (patched)
> > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66>
> >
> >     Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
> >     
> >     Also, this function will return `ssl.PROTOCOL_SSLv23` 
> >     no matter what, so the caller has no indication that an error may have occurred.
> 
> Aravindan Vijayan wrote:
>     I will make the check certificate step configurable.

I am not sure about making cert check failure/sucess configurable. It just sounds like an overkill. Why not check and warn if there is an issue with the check. The daemon launching with sucess or failure will in any case determine if the cert is valid and configured correctly. There is effectively no stack script that does cert checks like this :-)


- Sid


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


On March 28, 2017, 10:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 10:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On March 28, 2017, 11:57 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/ambari_commons/network.py
> > Lines 65-66 (original), 64-65 (patched)
> > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66>
> >
> >     Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
> >     
> >     Also, this function will return `ssl.PROTOCOL_SSLv23` 
> >     no matter what, so the caller has no indication that an error may have occurred.
> 
> Aravindan Vijayan wrote:
>     I will make the check certificate step configurable.
> 
> Sid Wagle wrote:
>     I am not sure about making cert check failure/sucess configurable. It just sounds like an overkill. Why not check and warn if there is an issue with the check. The daemon launching with sucess or failure will in any case determine if the cert is valid and configured correctly. There is effectively no stack script that does cert checks like this :-)
> 
> Robert Levas wrote:
>     The daemon (serve-side of this equation?) cannot be trusted to to verify it's own cert, this is why the client should do it.  I think (basically) silently warning the user the server cannot be trusted can be a problem in some situations, depending on the level of paranoia a user may have. And if a user has gone through the trouble of getting a _real_ SSL certificate (as opposed to a self-signed SSL certificate), then they probably want to know if the server's cert is not valid.
> 
> Aravindan Vijayan wrote:
>     IMO, we should go with either printing warning, or failing fast. Making it configurable just adds one more step of complexity which the user might not really use.
> 
> Robert Levas wrote:
>     How about failing fast and allow the caller to catch the exception and either log and continue or log and fail?  I think this is bascially the functionailty that we currently have - which is being changed by this patch. 
>     
>     It appears that `ambari_commons.network.get_http_connection` conditionally checks the server's SSL certificate if the `ca_certs` value is not `None`. So the caller is already indicating whether a validation check is required or not.
> 
> Sumit Mohanty wrote:
>     Who are the callers for this function? Are both expected to have different behavior or the same wrt processing the erorr?

This fucntion is being called in 3 places by AMS whenever Metrics collector or Grafana is HTTPS enabled - Service check, Grafana util and HDFS metrics based alerts.


- Aravindan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


On March 28, 2017, 10:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 10:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Robert Levas <rl...@hortonworks.com>.

> On March 28, 2017, 7:57 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/ambari_commons/network.py
> > Lines 65-66 (original), 64-65 (patched)
> > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66>
> >
> >     Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
> >     
> >     Also, this function will return `ssl.PROTOCOL_SSLv23` 
> >     no matter what, so the caller has no indication that an error may have occurred.
> 
> Aravindan Vijayan wrote:
>     I will make the check certificate step configurable.
> 
> Sid Wagle wrote:
>     I am not sure about making cert check failure/sucess configurable. It just sounds like an overkill. Why not check and warn if there is an issue with the check. The daemon launching with sucess or failure will in any case determine if the cert is valid and configured correctly. There is effectively no stack script that does cert checks like this :-)
> 
> Robert Levas wrote:
>     The daemon (serve-side of this equation?) cannot be trusted to to verify it's own cert, this is why the client should do it.  I think (basically) silently warning the user the server cannot be trusted can be a problem in some situations, depending on the level of paranoia a user may have. And if a user has gone through the trouble of getting a _real_ SSL certificate (as opposed to a self-signed SSL certificate), then they probably want to know if the server's cert is not valid.
> 
> Aravindan Vijayan wrote:
>     IMO, we should go with either printing warning, or failing fast. Making it configurable just adds one more step of complexity which the user might not really use.
> 
> Robert Levas wrote:
>     How about failing fast and allow the caller to catch the exception and either log and continue or log and fail?  I think this is bascially the functionailty that we currently have - which is being changed by this patch. 
>     
>     It appears that `ambari_commons.network.get_http_connection` conditionally checks the server's SSL certificate if the `ca_certs` value is not `None`. So the caller is already indicating whether a validation check is required or not.
> 
> Sumit Mohanty wrote:
>     Who are the callers for this function? Are both expected to have different behavior or the same wrt processing the erorr?
> 
> Aravindan Vijayan wrote:
>     This fucntion is being called in 3 places by AMS whenever Metrics collector or Grafana is HTTPS enabled - Service check, Grafana util and HDFS metrics based alerts.
> 
> Aravindan Vijayan wrote:
>     Rob,
>     
>     Based on your comment - "It appears that ambari_commons.network.get_http_connection conditionally checks the server's SSL certificate if the ca_certs value is not None. So the caller is already indicating whether a validation check is required or not."
>     
>     If ca_certs is passed in, then isnt it better to fail fast if certificate validation fails?

"If ca_certs is passed in, then isnt it better to fail fast if certificate validation fails?"

Yes, I hope that is what I was communicating. :)


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


On March 28, 2017, 6:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 6:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On March 28, 2017, 11:57 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/ambari_commons/network.py
> > Lines 65-66 (original), 64-65 (patched)
> > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66>
> >
> >     Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
> >     
> >     Also, this function will return `ssl.PROTOCOL_SSLv23` 
> >     no matter what, so the caller has no indication that an error may have occurred.
> 
> Aravindan Vijayan wrote:
>     I will make the check certificate step configurable.
> 
> Sid Wagle wrote:
>     I am not sure about making cert check failure/sucess configurable. It just sounds like an overkill. Why not check and warn if there is an issue with the check. The daemon launching with sucess or failure will in any case determine if the cert is valid and configured correctly. There is effectively no stack script that does cert checks like this :-)
> 
> Robert Levas wrote:
>     The daemon (serve-side of this equation?) cannot be trusted to to verify it's own cert, this is why the client should do it.  I think (basically) silently warning the user the server cannot be trusted can be a problem in some situations, depending on the level of paranoia a user may have. And if a user has gone through the trouble of getting a _real_ SSL certificate (as opposed to a self-signed SSL certificate), then they probably want to know if the server's cert is not valid.
> 
> Aravindan Vijayan wrote:
>     IMO, we should go with either printing warning, or failing fast. Making it configurable just adds one more step of complexity which the user might not really use.
> 
> Robert Levas wrote:
>     How about failing fast and allow the caller to catch the exception and either log and continue or log and fail?  I think this is bascially the functionailty that we currently have - which is being changed by this patch. 
>     
>     It appears that `ambari_commons.network.get_http_connection` conditionally checks the server's SSL certificate if the `ca_certs` value is not `None`. So the caller is already indicating whether a validation check is required or not.
> 
> Sumit Mohanty wrote:
>     Who are the callers for this function? Are both expected to have different behavior or the same wrt processing the erorr?
> 
> Aravindan Vijayan wrote:
>     This fucntion is being called in 3 places by AMS whenever Metrics collector or Grafana is HTTPS enabled - Service check, Grafana util and HDFS metrics based alerts.

Rob,

Based on your comment - "It appears that ambari_commons.network.get_http_connection conditionally checks the server's SSL certificate if the ca_certs value is not None. So the caller is already indicating whether a validation check is required or not."

If ca_certs is passed in, then isnt it better to fail fast if certificate validation fails?


- Aravindan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


On March 28, 2017, 10:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 10:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On March 28, 2017, 11:57 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/ambari_commons/network.py
> > Lines 65-66 (original), 64-65 (patched)
> > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66>
> >
> >     Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
> >     
> >     Also, this function will return `ssl.PROTOCOL_SSLv23` 
> >     no matter what, so the caller has no indication that an error may have occurred.

I will make the check certificate step configurable.


- Aravindan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


On March 28, 2017, 10:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 10:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Robert Levas <rl...@hortonworks.com>.

> On March 28, 2017, 7:57 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/ambari_commons/network.py
> > Lines 65-66 (original), 64-65 (patched)
> > <https://reviews.apache.org/r/58006/diff/1/?file=1677953#file1677953line66>
> >
> >     Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
> >     
> >     Also, this function will return `ssl.PROTOCOL_SSLv23` 
> >     no matter what, so the caller has no indication that an error may have occurred.
> 
> Aravindan Vijayan wrote:
>     I will make the check certificate step configurable.
> 
> Sid Wagle wrote:
>     I am not sure about making cert check failure/sucess configurable. It just sounds like an overkill. Why not check and warn if there is an issue with the check. The daemon launching with sucess or failure will in any case determine if the cert is valid and configured correctly. There is effectively no stack script that does cert checks like this :-)
> 
> Robert Levas wrote:
>     The daemon (serve-side of this equation?) cannot be trusted to to verify it's own cert, this is why the client should do it.  I think (basically) silently warning the user the server cannot be trusted can be a problem in some situations, depending on the level of paranoia a user may have. And if a user has gone through the trouble of getting a _real_ SSL certificate (as opposed to a self-signed SSL certificate), then they probably want to know if the server's cert is not valid.
> 
> Aravindan Vijayan wrote:
>     IMO, we should go with either printing warning, or failing fast. Making it configurable just adds one more step of complexity which the user might not really use.

How about failing fast and allow the caller to catch the exception and either log and continue or log and fail?  I think this is bascially the functionailty that we currently have - which is being changed by this patch. 

It appears that `ambari_commons.network.get_http_connection` conditionally checks the server's SSL certificate if the `ca_certs` value is not `None`. So the caller is already indicating whether a validation check is required or not.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


On March 28, 2017, 6:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 6:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 58006: AMBARI-20600 : AMS grafana restart fails with ssl error after upgrading from 2.4.2.0

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58006/#review170350
-----------------------------------------------------------


Fix it, then Ship it!




Ship It!


ambari-common/src/main/python/ambari_commons/network.py
Lines 65-66 (original), 64-65 (patched)
<https://reviews.apache.org/r/58006/#comment243166>

    Maybe there should be a flag so that the caller of this can indicate whether they want this to fail or not.  It may be desirable to not allow the connection if the server is not trusted.  
    
    Also, this function will return `ssl.PROTOCOL_SSLv23` 
    no matter what, so the caller has no indication that an error may have occurred.


- Robert Levas


On March 28, 2017, 6:03 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58006/
> -----------------------------------------------------------
> 
> (Updated March 28, 2017, 6:03 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Robert Levas, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20600
>     https://issues.apache.org/jira/browse/AMBARI-20600
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> EXCEPTION TRACE
> 
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 235, in create_grafana_admin_pwd
>     response = perform_grafana_get_call(GRAFANA_USER_URL, serverCall1)
>   File "/var/lib/ambari-agent/cache/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py", line 59, in perform_grafana_get_call
>     grafana_https_enabled, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 49, in get_http_connection
>     ssl_version = check_ssl_certificate_and_return_ssl_version(host, port, ca_certs)
>   File "/usr/lib/python2.6/site-packages/ambari_commons/network.py", line 66, in check_ssl_certificate_and_return_ssl_version
>     .format(host, port, ca_certs, str(ssl_error)))
> resource_management.core.exceptions.Fail: Failed to verify the SSL certificate for https://<host>:3000 with CA certificate in /etc/security/ssl/test.cert. Error : [Errno 1] _ssl.c:492: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
> 
> 
> PROBLEM
> The Grafana util script makes HTTPS calls with the server endpoint to create datasource, dashboards etc. For this call, it validates the server's certificate with the CA certificate using the https://docs.python.org/2/library/ssl.html#ssl.get_server_certificate call. This call checks the certificate validity against a root certificate list.
> The Grafana cert file (/configurations/ams-grafana-ini/cert_file) can be used both by the Grafana server to start up in HTTPS as well as in this validation step if the cert file is not a leaf certificate (for example a self signed certificate). If there is a CA which issued the certificate for Grafana HTTPS, then the ca bundle must be used to validate the server's certificate.
> 
> FIX
> Added a new parameter that takes in the ca_cert, defaulting to the cert file. 
> Grafana start should not fail if we are not able to validate the certificate, but able to make HTTPS calls to the server. We will print out a warning statement instead.
> 
> 
> Diffs
> -----
> 
>   ambari-common/src/main/python/ambari_commons/network.py 6ab92b2 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-grafana-ini.xml b4570b7 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/metrics_grafana_util.py a6a9779 
>   ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/params.py 3276cc1 
> 
> 
> Diff: https://reviews.apache.org/r/58006/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Python unit tests passed.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>