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

fix(api): Improve error handling for invalid domains #18

Merged

Conversation

MichaelEischer
Copy link
Contributor

Updating or deleting DNS records fails if some service is configured with an invalid domain name.

To reproduce the problem, assume that a user has created a domain test1234.runs.on.stackit.cloud, and has set up two loadbalancer services in the cluster. Service A is configured to use domain service.test1234.runs.on.stackit.cloud, and Service B uses domain service.test1234-invalid.runs.on.stackit.cloud (notice the -invalid). Wait until the DNS records of service A are created by external DNS in the test1234 domain. Then remove Service A. Notice that the DNS record is not removed as long as Service B with the invalid domain exists.

This is a result of the error handling in StackitDNSProvider.ApplyChanges which exits on the first error. As a result an invalid domain will block all Update / Delete operations. To address the problem, I've changed the code to collect all changeTasks first and execute all of them at once. This design is also used by some in-tree extension of external-dns and ensures that an invalid domain cannot block other operations.

@MichaelEischer MichaelEischer changed the title bug(dns): Improve error handling for invalid domains fix(dns): Improve error handling for invalid domains Jun 17, 2024
@MichaelEischer MichaelEischer changed the title fix(dns): Improve error handling for invalid domains fix(api): Improve error handling for invalid domains Jun 17, 2024
@PatrickKoss PatrickKoss merged commit c8cce0f into stackitcloud:main Jul 2, 2024
4 checks passed
@MichaelEischer MichaelEischer deleted the improve-error-handling branch July 2, 2024 12:15
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