fix(discovery/kubernetes): adjust id length#12536
Conversation
SkyeYoung
changed the title
id length and regexp of service.host schema
dosubot
Bot
added
the
size:L
There was a problem hiding this comment.
Pull Request Overview
This PR addresses schema limitations in the Kubernetes discovery module to support a wider range of use cases, specifically for EKS environments where API server addresses may contain uppercase letters and cluster IDs may exceed 8 characters.
- Expand
discovery.kubernetes.service.hostpattern to accept uppercase letters in addition to lowercase - Increase
discovery.kubernetes.idmaximum length from 8 to 64 characters - Add comprehensive test coverage for both schema validation scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apisix/discovery/kubernetes/schema.lua | Updates schema patterns for host (uppercase support) and id (64 char limit) |
| t/core/kubernetes_schema.t | Adds new test file with validation tests for id and host field constraints |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Related Documentation No published documentation to review for changes on this repository. How did I do? Any feedback? |
| 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])?]] .. |
There was a problem hiding this comment.
according to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names, we don't need to support uppercase letters
There was a problem hiding this 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) is not a Kubernetes object name.
There was a problem hiding this comment.
@SkyeYoung, please modify the corresponding description information.
There was a problem hiding this comment.
@Baoyuantop What descriptions need to be adjusted? 🤔
SkyeYoung
changed the title
dosubot
Bot
added
size:M
Description
This PR is designed to solve problems found in #12479:
discovery.kubernetes.idneeds to support more than 8 characters.These designs were originally artificially limited, and there was a lack of discussion at the time.
Considering that
ids have common cases likeaws-jp-commonandaws-sgp-common, the following adjustments were made in this PR:discovery.kubernetes.idto support a maximum of 64 characters.Which issue(s) this PR fixes:
Fixes #12479
Checklist