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

Add AWS Route53 Dynamic DNS support #17072

Merged
merged 2 commits into from
Oct 13, 2018
Merged

Conversation

keirans
Copy link
Contributor

@keirans keirans commented Oct 2, 2018

Description:

This component adds AWS Route53 dynamic DNS support.

I've tested this and updated the documentation accordingly.

This is my first real Python code, so go easy on me ;)

Example entry for configuration.yaml (if applicable):

route53:
  aws_access_key_id: ABC123
  aws_secret_access_key: DEF456
  zone: ZONEID678
  domain: example.domain.com
  records:
    - vpn
    - hassio
    - home

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.


if response['ResponseMetadata']['HTTPStatusCode'] is not 200:
_LOGGER.warning(response)

Choose a reason for hiding this comment

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

blank line at end of file

changes.append(
{
'Action' : 'UPSERT',
'ResourceRecordSet' : {

Choose a reason for hiding this comment

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

whitespace before ':'


changes.append(
{
'Action' : 'UPSERT',

Choose a reason for hiding this comment

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

whitespace before ':'

return

except:
_LOGGER.warning('The ipify logic in the route53 component failed and needs futher investigation')

Choose a reason for hiding this comment

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

line too long (105 > 79 characters)

homeassistant/components/route53.py Outdated Show resolved Hide resolved
ipaddress = get_ip()

except ConnectionError:
_LOGGER.warning('Unable to reach the ipify service to most likely because of a network error on your end.')

Choose a reason for hiding this comment

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

line too long (115 > 79 characters)

return True


def _update_route53(aws_access_key_id, aws_secret_access_key, zone, domain, records):

Choose a reason for hiding this comment

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

line too long (85 > 79 characters)


def update_records_service(now):
"""Set up service for manual trigger."""
_update_route53(aws_access_key_id, aws_secret_access_key, zone, domain, records)

Choose a reason for hiding this comment

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

line too long (88 > 79 characters)


def update_records_interval(now):
"""Set up recurring update."""
_update_route53(aws_access_key_id, aws_secret_access_key, zone, domain, records)

Choose a reason for hiding this comment

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

line too long (88 > 79 characters)

import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.event import track_time_interval

REQUIREMENTS = ['boto3==1.9.6', 'ipify==1.0.0' ]

Choose a reason for hiding this comment

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

whitespace before ']'

homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.event import track_time_interval

REQUIREMENTS = ['boto3==1.9.6','ipify==1.0.0']

Choose a reason for hiding this comment

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

missing whitespace after ','

@homeassistant
Copy link
Contributor

Hi @keirans,

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!

@awarecan
Copy link
Contributor

awarecan commented Oct 2, 2018

Thanks for your contribution, however I didn't found the reason we need this as part of homeassistant, it is better fit for a hass.io add-on IMO.

@keirans
Copy link
Contributor Author

keirans commented Oct 2, 2018

Hey @awarecan,
Thanks for the review.

This is based on the cloudflare DNS component that is already part of home assistant, alongside duckdns/freedns and a number of other providers.

Given AWS is the biggest cloud provider out there today, it made sense for me to add it.

I have already got this as an add-on, however a number of people i know don't run it as hass, rather under docker directly, so they can't use the add-on.

Let me know what you think,

Cheers,

K

@ludeeus
Copy link
Member

ludeeus commented Oct 2, 2018

To be honest I disagree, I think this goes well in along side the other services we have like DuckDNS, Namecheap, google, freedns, no-ip, Cloudflare.
https://www.home-assistant.io/components/#network

homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
homeassistant/components/route53.py Outdated Show resolved Hide resolved
.coveragerc Outdated Show resolved Hide resolved
@keirans
Copy link
Contributor Author

keirans commented Oct 7, 2018

Hey @fabaff , I've overhauled the code and tested it all works OK.

I've got all the outstanding CI issues resolved and it is ready for your approval and merge into dev.

Let me know if there are any other things i need to do.

Thanks :)

@keirans keirans force-pushed the route53 branch 3 times, most recently from 03ceb4d to a8c73fd Compare October 13, 2018 03:46
@ghost ghost assigned fabaff Oct 13, 2018
@fabaff fabaff merged commit 78e29cd into home-assistant:dev Oct 13, 2018
@ghost ghost removed the in progress label Oct 13, 2018
@balloob balloob mentioned this pull request Oct 26, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
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.

8 participants