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 trailing dot to record values if it's missing #45

Merged
merged 7 commits into from
Jan 11, 2024

Conversation

yugandhar91
Copy link
Contributor

Purpose of this change is to handle an edge case where I am trying to migrate CNAME and MX records from AWS Route53 to GCP Cloud DNS and I encounter an error at the stage where OctoDNS tries to apply the changes. AWS Route53 is stripping away the trailing dots on their platform and GCP doesn't like it.

@yugandhar91
Copy link
Contributor Author

Taking a look at the failed check, looks like CI is pulling google-cloud-dns version 0.35.0. I had the same issue. The requirements.txt file specifies version 0.34.2 and when I uninstalled google-cloud-dns from my venv and reinstalled requirements.txt, everything just fine.

Copy link
Contributor

@ross ross left a comment

Choose a reason for hiding this comment

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

I don't have a working google dns setup atm so trusting you as far as this being needed/correct.

Code looks good. Minor suggestion and an additional test case in the recommendations. Once those are sorted this should be good to go.

octodns_googlecloud/__init__.py Outdated Show resolved Hide resolved
tests/test_octodns_provider_googlecloud.py Show resolved Hide resolved
@yugandhar91
Copy link
Contributor Author

Thanks for reviewing. Committed the two suggested changes.

@ross
Copy link
Contributor

ross commented Jan 11, 2024

CI failures is the persistent google module failures and unrelated to the change.

Thanks!

@ross ross merged commit 82089c5 into octodns:main Jan 11, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants