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

Retrieve SOA record using DNS zone instead of building it from record name #1371

Merged

Conversation

matco
Copy link
Contributor

@matco matco commented Feb 11, 2019

[all]

[terraform]

[terraform-beta]

[ansible]

When a DNS record is updated using the SOA entry of the zone must be updated as well.

To retrieve the SOA record in the zone, the module makes a request to Google Cloud API asking for entries of type SOA and with a name built from the name of the record currently updated.

For example, is the user tries to update the record "www.example.com.", the module looks for a record of type SOA and with the name "example.com.".

name = module.params['name'].split('.')[1:]

However, a zone may contain subdomains that don't have their own SOA record. For example, in the same Google DNS managed zone, the user may want to update "sub.www.example.com." without having a SOA record for the subdomain "www.example.com.". The good SOA record to update is still "example.com.".

Currently, in this case, the module fails to retrieve the SOA record, and fails to operate.

With this fix, the SOA is retrieved from the dnsName attribute of the "managed_zone" parameter. This may not be accurate if there is actually a SOA record for the subdomain, but it always work. It may be better to let the user manage SOA records manually.

This fix requires the "managed_zone" parameter to be a dictionary. The documentation mentions that it is possible to use a name or an id for the "managed_zone" parameter but it does not work so this fix won't break this feature.

[inspec]

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@matco
Copy link
Contributor Author

matco commented Feb 11, 2019

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@modular-magician
Copy link
Collaborator

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review. They will authorize it to run through our CI pipeline, which will generate downstream PRs.

Thanks for your contribution! A human will be with you soon.

@bverschueren
Copy link

I faced the same issue, though tried to tackle it differently. As I don't think it's strictly required for every record type to provide SOA data in an update, #51628 does it only in case it does finds SOA data on the (parent) record.

@matco
Copy link
Contributor Author

matco commented Feb 12, 2019

@bverschueren I missed your PR on the Ansible project. Your solution may be better except it's not consistent (update or not the SOA and may update the wrong SOA like mine).

Maybe it's better not to update the SOA record at all and let the user do it. Magic is never good.

@rambleraptor
Copy link
Contributor

rambleraptor commented Feb 12, 2019

For now, I'm going to try and do this solution where it attempts an update. I personally like the modules to try and act consistently. If problems occur, we can do from there.

Thanks so much for the contribution!

@modular-magician
Copy link
Collaborator

I am a robot that works on MagicModules PRs!

I built this PR into one or more PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: modular-magician/ansible#188
depends: modular-magician/inspec-gcp#108

matco and others added 2 commits February 15, 2019 23:51
Tracked submodules are build/terraform-beta build/terraform build/ansible build/inspec.
@modular-magician modular-magician merged commit 16a184c into GoogleCloudPlatform:master Feb 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants