Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

automatically use bundled certificate it set to auto #6707

Merged
merged 3 commits into from Mar 19, 2017

Conversation

printzlau
Copy link
Contributor

@printzlau printzlau commented Mar 19, 2017

Description:

Change the way that bundled certificates is automatically used.
From now on it will be specified by setting certificate to 'auto'.
The previous method of using the port set to 8883, will no longer effect the certificate.

Related issue (if applicable): fixes #6596

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2295

Example entry for configuration.yaml (if applicable):

mqtt:
  broker: <ip>
  port: <port>
  client_id: home-assistant-1
  username: <username>
  password: <password>
  certificate: auto


Checklist:

  • Documentation added/updated in home-assistant.github.io
  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

… to 'auto' and seperate this from which port is specified
@mention-bot
Copy link

@printzlau, thanks for your PR! By analyzing the history of the files in this pull request, we identified @pvizeli, @balloob and @fabaff to be potential reviewers.

@homeassistant
Copy link
Contributor

Hi @printzlau,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

def test_setup_uses_certificate_not_on_mqtts_port(hass):
"""Test setup doesn't use bundled certificates when not mqtts port."""
def test_setup_does_not_use_certificate_on_mqtts_port(hass):
"""Test setup doesn't use bundled certificates when certificate is not set."""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (82 > 79 characters)

@@ -317,8 +317,8 @@ def async_setup(hass, config):
certificate = os.path.join(os.path.dirname(__file__),
'addtrustexternalcaroot.crt')

# When the port indicates mqtts, use bundled certificates from requests
if certificate is None and port == 8883:
# When the certificate is set to auto, use bundled certificates from requests

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (81 > 79 characters)

@@ -119,7 +119,7 @@ def valid_discovery_topic(value):
vol.Optional(CONF_PORT, default=DEFAULT_PORT): cv.port,
vol.Optional(CONF_USERNAME): cv.string,
vol.Optional(CONF_PASSWORD): cv.string,
vol.Optional(CONF_CERTIFICATE): cv.isfile,
vol.Optional(CONF_CERTIFICATE): vol.Any('auto',cv.isfile),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing whitespace after ','

@balloob balloob merged commit 1cb2a6a into home-assistant:dev Mar 19, 2017
@balloob balloob mentioned this pull request Mar 24, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Jun 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.40.0 broke mqtt
6 participants