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

Enable auth to Cloudflare DNS via API Tokens #1564

Merged
merged 3 commits into from Mar 14, 2021
Merged

Conversation

c-w
Copy link
Member

@c-w c-w commented Mar 7, 2021

Enable auth to Cloudflare DNS via API Tokens

Description

This pull request adds support to the Cloudflare DNS driver to authenticate via API Tokens (see docs) in addition to the previous Global API Key authentication mechanism.

Status

  • done, ready for review

Checklist

@c-w c-w force-pushed the dns-cloudflare-token-auth branch from 39508bf to 126b273 Compare March 7, 2021 18:12
@c-w
Copy link
Member Author

c-w commented Mar 7, 2021

@Kami "Generate Code Coverage" failure in this pull request seems unrelated to the changes:

Screenshot of CI failure

@micafer
Copy link
Contributor

micafer commented Mar 10, 2021

@Kami "Generate Code Coverage" failure in this pull request seems unrelated to the changes:

Screenshot of CI failure

This error has been fixed in trunk, merge the changes and this tests will pass.

@@ -146,27 +147,37 @@ def parse_error(self):
raise exception_class(**kwargs)


class CloudFlareDNSConnection(ConnectionUserAndKey):
class BaseDNSConnection:
Copy link
Member

Choose a reason for hiding this comment

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

Minor style thing - we tend to explicitly inherit from object everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 1d209ed.

Copy link
Member

@Kami Kami left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I use CloudFlare driver myself in various places, so this will also come handy for me :)

@codecov-io
Copy link

Codecov Report

Merging #1564 (1d209ed) into trunk (6bac710) will decrease coverage by 0.00%.
The diff coverage is 85.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            trunk    #1564      +/-   ##
==========================================
- Coverage   83.00%   83.00%   -0.01%     
==========================================
  Files         394      394              
  Lines       84943    84959      +16     
  Branches     9036     9037       +1     
==========================================
+ Hits        70508    70521      +13     
- Misses      11369    11372       +3     
  Partials     3066     3066              
Impacted Files Coverage Δ
libcloud/dns/drivers/cloudflare.py 91.18% <78.57%> (-0.98%) ⬇️
libcloud/test/dns/test_cloudflare.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bac710...1d209ed. Read the comment docs.

@c-w c-w merged commit 1dfd3cd into trunk Mar 14, 2021
@c-w c-w deleted the dns-cloudflare-token-auth branch March 14, 2021 00:25
d-mo pushed a commit to mistio/libcloud that referenced this pull request Nov 15, 2021
d-mo pushed a commit to mistio/libcloud that referenced this pull request Nov 19, 2021
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.

None yet

4 participants