serving
serving copied to clipboard
feat(domainmapping): Add wildcard domain claim support
Fixes #14688
Proposed Changes
Update domain claims used for domainmapping to support wildcard. This allows domain claims like *.example.com to be used to match domains like foo.example.com and bar.example.com. Additionally, this allows breaking out nested subdomains into separate claims assigned to different namespaces. For example, *.foo.example.com and *.example.com can both exist. The more specific one will be the one that owns the specified domain (in absence of a specific domainclaim as those always take priority). This allows for additional flexibility.
Release Note
Add support for wildcard DomainClaims
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: arsenetar Once this PR has been reviewed and has the lgtm label, please assign cali0707 for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Hi @arsenetar. Thanks for your PR.
I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions 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/test-infra repository.
Still need to update to add additional tests.
Codecov Report
Attention: 22 lines in your changes are missing coverage. Please review.
Comparison is base (
2125772) 85.77% compared to head (6e252a0) 85.69%.
| Files | Patch % | Lines |
|---|---|---|
| pkg/reconciler/domainmapping/reconciler.go | 42.10% | 17 Missing and 5 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #14881 +/- ##
==========================================
- Coverage 85.77% 85.69% -0.09%
==========================================
Files 198 198
Lines 15153 15187 +34
==========================================
+ Hits 12998 13014 +16
- Misses 1829 1844 +15
- Partials 326 329 +3
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Ah this will need some other way of either indicating wildcard or processing since *.example.com is an invalid object name. I forgot these use the name of the claim directly. Have options:
- Add a item in the spec to indicate wildcard or could even just have an entry for subDomains with a special value
*for wildcard. Subdomain list would allow specifying specific subdomains to match only if desired. - Allow a domain claim of
example.comto match all subdomains (logic in this PR still allows other subdomains to be given to other claims if desired). This is slightly less clear although maybe not entirely unintuitive. This effectively makes every claim a "wildcard".
@braunsonm Since you were also interested in this not sure if one of these options would align better with how you wanted to use this.
I'm fine with either, but with option 2 wouldn't that make it no longer possible to have a cluster domain claim that matches a single domain? If you make them all wildcardable that might be undesirable to others who are already using this feature.
Another option (if allowed) could be any domains prefixed with a . allow wildcard subdomains. So .mydomain.com allows all subdomains of mydomain.com whereas without the prefix is an exact match.
@braunsonm .mydomain.com has the same issue as starting with * as the name needs to start with alphanumeric.
Yeah the way this feature was implemented it uses name conflicts in the cluster scope to ensure there are no duplicates.
If we wanted to support wild cards then we'd have to modify the existing ClusterDomainClaim resource and introduce controllers to try to ensure the same guarantee.
I'm going to close this out - if this is something you're interested in pursuing then we'd probably want to write up a feature track document.