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

feat: support setting a plan #123

Closed
wants to merge 1 commit into from
Closed

Conversation

oikarinen
Copy link
Contributor

@oikarinen oikarinen commented Jan 31, 2025

This pull request introduces support for managing Cloudflare plan types within the octodns_cloudflare provider. The change is implemented using a custom record type.

Aligned with change of _zones structure from #122

Key changes include:

  • Added a new optional plan_type parameter to the CloudflareProvider class to specify the Cloudflare plan type (e.g., free, enterprise).
  • Created a new custom record type CloudflareZoneRecord to support Cloudflare zone plan updates.
  • Adjusted the _apply_Create, _apply_Update, and _apply_Delete methods to manage plan updates based on the new CF_ZONE record type.
  • Updated README.md to document the new plan_type option.

@oikarinen oikarinen marked this pull request as ready for review January 31, 2025 16:22
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.

So played around with the option to add "meta" changes during the provider planning process in octodns/octodns#1236.

That should allow CloudflareProvider to implement _plan_meta and return a dictionary with information about the CF plan for the zone that can be used during the apply phase to make changes if necessary.

I believe all the code needed to get things to function is already in here, it'll just need to be shifted around a bit and the custom record type can go away.

LMK what you think and if you see any problems or shortcomings with the prototype implementation.

Once it's all vetted we can merge the octodns PR and then cut a release. This provider can then either bump to require that versuion of octoDNS or add support so that it makes the changes if octoDNS is new enough and warns or errors if someone is trying to manage the plan when it's not.

@oikarinen
Copy link
Contributor Author

LMK what you think and if you see any problems or shortcomings with the prototype implementation.

Pretty straight forward and all good with this approach! Opened new PR and closing this

Once it's all vetted we can merge the octodns PR and then cut a release. This provider can then either bump to require that versuion of octoDNS or add support so that it makes the changes if octoDNS is new enough and warns or errors if someone is trying to manage the plan when it's not.

Code would support missing meta property but tests would require the change: #127

@oikarinen oikarinen closed this Feb 3, 2025
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