deardeng opened a new pull request, #69732:
URL: https://github.com/apache/doris/pull/69732
Meta service finds the instance of a request by its cloud_unique_id. When
the node index misses, `get_instance_id` parses the degraded format
`1:<instance_id>:<any>` and, with `enable_check_instance_id`, only checks that
the instance exists and is not deleted. So a caller knowing an instance_id can
build a cloud_unique_id with any third segment and is treated as a node of that
instance.
Besides, `get_cluster` falls back to the request supplied `instance_id` when
the cloud_unique_id can not be resolved, and rpcs carrying both fields use
`instance_id` without checking that it belongs to the cloud_unique_id, so a
registered cloud_unique_id of one instance plus the instance_id of another
reaches the other instance.
Fix:
1. A degraded format cloud_unique_id is only accepted when it is a
registered node of the parsed instance.
2. `MetaServiceProxy::call_impl` rejects an rpc whose `instance_id` is not
the instance its `cloud_unique_id` belongs to with INVALID_ARGUMENT. The http
api, authorized by token, calls the impl directly and is not affected. Rpcs
carrying only `instance_id` are still accepted, since FE sends `alter_cluster`
and `create_instance` that way.
In unit test, before the fix a forged third segment, an unresolvable
cloud_unique_id with an instance_id, and a cloud_unique_id of instance A with
the instance_id of instance B all get OK with the clusters of the target
instance; after the fix all get INVALID_ARGUMENT, and a registered
cloud_unique_id still gets OK.
### Release note
Meta service rejects a cloud_unique_id that is not a registered node of its
instance, and an rpc whose instance_id does not belong to its cloud_unique_id.
### Check List (For Author)
- Test: Unit Test
- meta_service_test, resource_test and RecycleVersionedKeysTest.
Fixtures using an unregistered cloud_unique_id now register the node.
- Behavior changed: Yes. Such requests get INVALID_ARGUMENT. When the
degraded format fallback fails to read the instance from kv, the request is now
rejected instead of accepted.
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
