fix(naming): do not force clusterName to DEFAULT in v3 HTTP API#14778
Open
daguimu wants to merge 1 commit intoalibaba:developfrom
Open
fix(naming): do not force clusterName to DEFAULT in v3 HTTP API#14778daguimu wants to merge 1 commit intoalibaba:developfrom
daguimu wants to merge 1 commit intoalibaba:developfrom
Conversation
|
Thanks for your this PR. 🙏 感谢您提交的PR。 🙏 |
When clusterName is not specified, InstanceListForm.fillDefaultValue() was forcing it to "DEFAULT", causing v3 HTTP API to return NOT_FOUND for services using non-default cluster names. The gRPC path treats empty cluster as "return all instances", but v3 HTTP did an exact match against "DEFAULT". Change fillDefaultValue() to use empty string instead of DEFAULT_CLUSTER_NAME, and add a blank-check in CatalogServiceV2Impl.listInstances() to skip the cluster existence validation when clusterName is blank, aligning behavior with the gRPC path (ServiceUtil.doSelectInstances). Fixes alibaba#14650
397623a to
e2146c5
Compare
Collaborator
|
Duplicate with #14674 If this PR no response and no match the merged status, This PR will be merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When calling the v3 HTTP API to list instances without specifying a
clusterName, the API returnsNOT_FOUNDfor services that use non-default cluster names (e.g.,cluster-hba). The same query via gRPC correctly returns all instances.Root Cause
InstanceListForm.fillDefaultValue()forces an emptyclusterNameto"DEFAULT". ThenCatalogServiceV2Impl.listInstances()does an exact match — if the service has no cluster named"DEFAULT", it throwsNacosException.NOT_FOUND.The gRPC path (
ServiceQueryRequestHandler) treats empty cluster as an empty string, which flows intoServiceUtil.doSelectInstances()where an empty cluster produces an emptyclusterSets, causingcheckCluster()to returntruefor all instances.Fix
InstanceListForm.fillDefaultValue(): UseStringUtils.EMPTYinstead ofUtilsAndCommons.DEFAULT_CLUSTER_NAMEwhenclusterNameis blank, so the v3 HTTP path passes an empty string just like gRPC does.CatalogServiceV2Impl.listInstances(): Skip the cluster-existence check whenclusterNameis blank, lettingServiceUtil.selectInstances()handle the empty-cluster case correctly (return all instances).Tests Added
InstanceListForm: blank clusterName → empty stringInstanceListFormTest.testFillDefaultValueWhenClusterNameIsBlank()InstanceListForm: provided clusterName preservedInstanceListFormTest.testFillDefaultValueWhenClusterNameIsProvided()InstanceListForm: default namespace/groupInstanceListFormTest.testFillDefaultValueForNamespaceAndGroup()InstanceListForm: missing serviceName throwsInstanceListFormTest.testValidateThrowsExceptionWhenServiceNameIsBlank()CatalogServiceV2Impl: blank cluster returns all instancesCatalogServiceV2ImplTest.testListInstancesWithBlankClusterNameReturnsAll()CatalogServiceV2Impl: null cluster returns all instancesCatalogServiceV2ImplTest.testListInstancesWithNullClusterNameReturnsAll()Impact
Only affects the v3 HTTP instance-list endpoint when
clusterNameis omitted. No change to gRPC behavior or any other API.Fixes #14650