You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "SkyeYoung (via GitHub)" <gi...@apache.org> on 2025/08/20 00:40:28 UTC
[PR] fix(discovery/kubernetes): adjust `id` length and regexp of `service.host` schema [apisix]
SkyeYoung opened a new pull request, #12536:
URL: https://github.com/apache/apisix/pull/12536
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
This PR is designed to solve two problems found in #12479:
1. apiserver address provided by eks contained uppercase letters. (`discovery.kubernetes.service.host`)
2. `discovery.kubernetes.id` needs to support more than 8 characters.
These designs were originally artificially limited, and there was [a lack of discussion](https://github.com/apache/apisix/pull/7895) at the time. Considering that there are many users use eks, and `id`s also have common cases like `aws-jp-common` and `aws-sgp-common`, the following adjustments were made in this PR:
1. Adjust `discovery.kubernetes.service.host` to support capital letters.
2. Adjust `discovery.kubernetes.id` to support a maximum of 64 characters.
#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #12479
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
3. Always add/update tests for any changes unless you have a good reason.
4. Always update the documentation to reflect the changes made in the PR.
5. Make a new commit to resolve conversations instead of `push -f`.
6. To resolve merge conflicts, merge master instead of rebasing.
7. Use "request review" to notify the reviewer after making changes.
8. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(discovery/kubernetes): tweak schema to fit a wider range of use cases [apisix]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12536:
URL: https://github.com/apache/apisix/pull/12536#discussion_r2287351146
##########
apisix/discovery/kubernetes/schema.lua:
##########
@@ -17,7 +17,8 @@
local host_patterns = {
{ pattern = [[^\${[_A-Za-z]([_A-Za-z0-9]*[_A-Za-z])*}$]] },
- { pattern = [[^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$]] },
+ { pattern = [[^[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?]] ..
+ [[(\.[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?)*$]] },
Review Comment:
[nitpick] The regex pattern is split across multiple lines with string concatenation. Consider using a single line or adding comments to explain the pattern structure for better readability.
```suggestion
{ pattern = [[^[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?(\.[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?)*$]] },
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(discovery/kubernetes): tweak schema to fit a wider range of use cases [apisix]
Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12536:
URL: https://github.com/apache/apisix/pull/12536#discussion_r2296522382
##########
apisix/discovery/kubernetes/schema.lua:
##########
@@ -17,7 +17,8 @@
local host_patterns = {
{ pattern = [[^\${[_A-Za-z]([_A-Za-z0-9]*[_A-Za-z])*}$]] },
- { pattern = [[^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$]] },
+ { pattern = [[^[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?]] ..
Review Comment:
@moonming This document is for object names in Kubernetes, and the API server endpoint (e.g., [`https://5E1D0CEXAMPLEA591B746AFC5AB30262.yl4.us-west-2.eks.amazonaws.com`](https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/aws-resource-eks-cluster.html#:~:text=master%20key%20(CMK).-,Endpoint,as%20https%3A//5E1D0CEXAMPLEA591B746AFC5AB30262.yl4.us%2Dwest%2D2.eks.amazonaws.com.,-Id)) is not a Kubernetes object name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(discovery/kubernetes): tweak schema to fit a wider range of use cases [apisix]
Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12536:
URL: https://github.com/apache/apisix/pull/12536#discussion_r2296894173
##########
apisix/discovery/kubernetes/schema.lua:
##########
@@ -17,7 +17,8 @@
local host_patterns = {
{ pattern = [[^\${[_A-Za-z]([_A-Za-z0-9]*[_A-Za-z])*}$]] },
- { pattern = [[^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$]] },
+ { pattern = [[^[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?]] ..
Review Comment:
rollbacked
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(discovery/kubernetes): tweak schema to fit a wider range of use cases [apisix]
Posted by "moonming (via GitHub)" <gi...@apache.org>.
moonming commented on code in PR #12536:
URL: https://github.com/apache/apisix/pull/12536#discussion_r2296032763
##########
apisix/discovery/kubernetes/schema.lua:
##########
@@ -17,7 +17,8 @@
local host_patterns = {
{ pattern = [[^\${[_A-Za-z]([_A-Za-z0-9]*[_A-Za-z])*}$]] },
- { pattern = [[^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$]] },
+ { pattern = [[^[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?]] ..
Review Comment:
according to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names, we don't need to support uppercase letters
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(discovery/kubernetes): adjust id length [apisix]
Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung merged PR #12536:
URL: https://github.com/apache/apisix/pull/12536
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(discovery/kubernetes): tweak schema to fit a wider range of use cases [apisix]
Posted by "dosubot[bot] (via GitHub)" <gi...@apache.org>.
dosubot[bot] commented on PR #12536:
URL: https://github.com/apache/apisix/pull/12536#issuecomment-3204759727
<!-- Answer -->
**Related Documentation**
No published documentation to review for changes on this repository.
[Write your first living document](https://app.dosu.dev/documents?utm_campaign=pr-doc-check-nudge)
<!-- Dosu Comment Footer -->
<sup>How did I do? Any feedback?</sup> [](https://go.dosu.dev/discord-bot)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(discovery/kubernetes): adjust id length [apisix]
Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12536:
URL: https://github.com/apache/apisix/pull/12536#discussion_r2300024351
##########
apisix/discovery/kubernetes/schema.lua:
##########
@@ -17,7 +17,8 @@
local host_patterns = {
{ pattern = [[^\${[_A-Za-z]([_A-Za-z0-9]*[_A-Za-z])*}$]] },
- { pattern = [[^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$]] },
+ { pattern = [[^[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?]] ..
Review Comment:
@Baoyuantop What descriptions need to be adjusted? 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(discovery/kubernetes): adjust id length [apisix]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on code in PR #12536:
URL: https://github.com/apache/apisix/pull/12536#discussion_r2299518791
##########
apisix/discovery/kubernetes/schema.lua:
##########
@@ -17,7 +17,8 @@
local host_patterns = {
{ pattern = [[^\${[_A-Za-z]([_A-Za-z0-9]*[_A-Za-z])*}$]] },
- { pattern = [[^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$]] },
+ { pattern = [[^[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?]] ..
Review Comment:
@SkyeYoung, please modify the corresponding description information.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org