-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Azure Private DNS Provider #1269
Azure Private DNS Provider #1269
Conversation
Welcome @saidst! |
FYI @timja |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey 👋
I've taken a look through the code, some minor nit's for wording and logs statements, I'll try test this out ~tomorrow
3) Deploy ExternalDNS | ||
|
||
## Prerequisites | ||
- Azure Kubernetes Service available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this actually needed? I did all my azure-dns testing on kind
locally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean AKS in particular?
What do you mean with kind
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant that this works perfectly fine on minikube
or kind
You only need to be running on azure if you're using the system assigned identity for authentication.
@@ -0,0 +1,419 @@ | |||
/* | |||
Copyright 2017 The Kubernetes Authors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this the right copyright?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@njuettner, people used that also in other PR's, which have recently been merged.
Can you answer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, please don't drop it we need it.
Thanks for your review. I am just about to add some unit-tests. After that, I will care for your remarks. |
7817658
to
b0bbe1e
Compare
@timja: have you had the chance to give it a try? |
Not yet will try tomorrow |
@saidst I've made a PR to fix a couple of docs issues (saidst#1), with those fixes I was successful in creating an A record and deleting it with the nginx ingress record in the tutorial. |
/assign @hjacobs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing @saidst 👍 🎉.
One comment, PTAL.
9a040ec
to
ccf9c74
Compare
@njuettner: I've rebased on master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: njuettner, saidst The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR includes support for Azure Private DNS.
Azure Private DNS is the successor of the previous "private-mode" of zones hosted via Azure DNS.
See: https://azure.microsoft.com/en-gb/updates/announcing-preview-refresh-for-azure-dns-private-zones-2/
The provider has been created by cloning the existing Azure Provider.
Both Azure DNS & Azure Private DNS are separate Azure APIs with totally independent type systems. Attempts to keep things DRY and implement business logic (e.g. creating an A Record) only once "failed". An approach in this regard was to come up with a "neutral" interlayer and decouple business logic in the provider from the type system of a particular API.
However, the increased complexity (additional types, converter methods) was assessed to be less favorable than simpler but redundant code.
Differences compared with Azure provider
Related issues:
ToDo: