-
Notifications
You must be signed in to change notification settings - Fork 65
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 DNS Service Zone MR #305
Conversation
82aee17
to
a9e502a
Compare
fde0e73
to
e44687d
Compare
The reasoning behind my suggestion of capitalizing DNS record type names comes from the DNS RFC: |
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've left some nitpicks for you to consider. As long as we behave correctly, the decision is yours. Thank you very much @sergenyalcin.
289ba35
to
c84be0c
Compare
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
c84be0c
to
f441145
Compare
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.
Thank you @sergenyalcin for working on this PR, very much appreciated!
Description of your changes
Fixes #266
Implementation of DNS Zone MR. The implementation of the resource is done on the Network API Group. Because when you want to create a DNS Zone via az cli, you need to run
az network dns zone create
command. This may indicate that it would be correct for the DNS Zone resource to be in this API Group.Every field in the Resource's Spec was not used in the update check. Since some fields are immutable, the fields that are likely to be updated have been selected and checked (pkg/clients/network/network.go:162)
I have:
make reviewable test
to ensure this PR is ready for review.How has this code been tested