-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(azure): dns metadata (tags) support #5984
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
base: master
Are you sure you want to change the base?
Conversation
|
Welcome @vaspoz! |
|
Hi @vaspoz. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
d8b61e6 to
f5a5147
Compare
|
/ok-to-test |
Pull Request Test Coverage Report for Build 20573485375Details
💛 - Coveralls |
2199546 to
3b0223c
Compare
3b0223c to
83edb29
Compare
|
Hi. Could you share similar results for this PR #5085 (comment). Need to make sure it works before we merge. What we looking as well, where or not the behaviour works when tags added, deleted, modified. |
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.
Hi,
You could add test cases in TestAzureApplyChanges() like this one:
endpoint.NewEndpointWithTTL("metadata.example.com", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").
WithProviderSpecific("azure-metadata-foo", "bar").
WithProviderSpecific("azure-metadata-baz", "qux"),This should cover your code without adding new tests.
|
@ivankatliarchuk , here's the tests outputs:
Test B: Update metadata
Test C: Delete metadata
Test D: Special characters
Logs: |
| parentRefs: | ||
| - name: my-gateway | ||
| hostnames: | ||
| - "api.example.com" |
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.
FYI: This is under discussion in gateway-api if this is what we should support or not.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
41a2e88 to
212f7db
Compare
|
/retest |
1 similar comment
|
/retest |
Move provider-specific Azure tags annotation parsing from source/annotations to provider/azure, following the same pattern as Cloudflare tags handling.
bfab470 to
0d0f4c5
Compare
|
/lgtm |
|
|
||
| // parseAzureTagsAnnotation parses the azure-tags annotation value (key1=value1,key2=value2) | ||
| // and adds individual metadata properties to the endpoint. | ||
| func parseAzureTagsAnnotation(ep *endpoint.Endpoint, tagString string) { |
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.
There is the exact same func for (at least) cloudflare:
external-dns/provider/cloudflare/cloudflare.go
Lines 732 to 743 in 02abd24
| func parseTagsAnnotation(tagString string) []string { | |
| tags := strings.Split(tagString, ",") | |
| cleanedTags := make([]string, 0, len(tags)) | |
| for _, tag := range tags { | |
| trimmed := strings.TrimSpace(tag) | |
| if trimmed != "" { | |
| cleanedTags = append(cleanedTags, trimmed) | |
| } | |
| } | |
| sort.Strings(cleanedTags) | |
| return cleanedTags | |
| } |
🤔 Wdyt about moving it to source/annotations and use it in both ?
|
New changes are detected. LGTM label has been removed. |
331ed60 to
0d0f4c5
Compare
What does it do ?
This PR adds support for Azure DNS metadata (tags) in external-dns. Users can now annotate Kubernetes resources with
external-dns.alpha.kubernetes.io/azure-metadata-*annotations to set Azure resource tags on the corresponding DNS records.Motivation
Azure DNS records support metadata (tags) for resource organization, cost tracking, and compliance. This feature enables Kubernetes users to manage DNS record tags declaratively through annotations, making it easier to:
More