github-actions[bot] commented on code in PR #63527:
URL: https://github.com/apache/doris/pull/63527#discussion_r3296644218
##########
cloud/src/recycler/recycler_service.cpp:
##########
@@ -507,204 +508,43 @@ void
RecyclerServiceImpl::http(::google::protobuf::RpcController* controller,
const ::doris::cloud::MetaServiceHttpRequest*
request,
::doris::cloud::MetaServiceHttpResponse*
response,
::google::protobuf::Closure* done) {
- auto cntl = static_cast<brpc::Controller*>(controller);
- LOG(INFO) << "rpc from " << cntl->remote_side() << " request: " <<
request->DebugString();
+ auto* cntl = static_cast<brpc::Controller*>(controller);
+ LOG(INFO) << "rpc from " << cntl->remote_side()
+ << " request: " << cntl->http_request().uri().path();
brpc::ClosureGuard closure_guard(done);
- MetaServiceCode code = MetaServiceCode::OK;
- int status_code = 200;
- std::string msg = "OK";
- std::string req;
- std::string response_body;
- std::string request_body;
- DORIS_CLOUD_DEFER {
- status_code = std::get<0>(convert_ms_code_to_http_code(code));
- LOG(INFO) << (code == MetaServiceCode::OK ? "succ to " : "failed to ")
<< "http"
- << " " << cntl->remote_side() << " request=\n"
- << req << "\n ret=" << code << " msg=" << msg;
- cntl->http_response().set_status_code(status_code);
- cntl->response_attachment().append(response_body);
+ const auto& unresolved_path = cntl->http_request().unresolved_path();
+ auto api_path = split_http_api_path(unresolved_path);
+ const auto& handlers = get_http_handlers();
+ auto it = handlers.find(api_path.route);
+ const auto* handler =
+ it == handlers.end() ? nullptr : resolve_http_handler(it->second,
api_path.version);
+ if (handler == nullptr ||
+ (it->second.role != HttpRole::RECYCLER && it->second.role !=
HttpRole::BOTH)) {
Review Comment:
This rejects unknown/non-recycler paths before validating `token`, while
valid recycler paths proceed to the auth check and return 403. That changes the
previous behavior where every request was authenticated before dispatch and
also lets unauthenticated callers distinguish existing recycler endpoints from
non-existing ones. Please move token validation before route lookup/role
rejection so unauthorized requests are handled uniformly.
##########
.github/workflows/license-eyes.yml:
##########
@@ -48,6 +48,62 @@ jobs:
with:
ref: ${{ github.event.pull_request.head.sha }}
+ - name: Get changed files
+ if: github.event_name == 'pull_request_target'
+ id: changed-files
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ run: |
+ if ! all=$(gh api repos/${{ github.repository }}/pulls/${{
github.event.pull_request.number }}/files \
+ --paginate --jq '.[].filename' 2>/dev/null); then
+ echo "config_file=.licenserc.yaml" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if [ "$(echo "$all" | wc -l)" -ge 3000 ]; then
+ echo "config_file=.licenserc.yaml" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if echo "$all" | grep -qxF '.licenserc.yaml'; then
+ echo "config_file=.licenserc.yaml" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if ! acmr=$(gh api repos/${{ github.repository }}/pulls/${{
github.event.pull_request.number }}/files \
+ --paginate --jq '.[] | select(.status != "removed") | .filename'
2>/dev/null); then
+ echo "config_file=.licenserc.yaml" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
+ if [ -z "$acmr" ]; then
+ exit 0
+ fi
+ {
+ echo "added_modified<<EOF"
+ echo "$acmr"
+ echo "EOF"
+ } >> "$GITHUB_OUTPUT"
+ echo "config_file=.licenserc-incremental.yaml" >> "$GITHUB_OUTPUT"
+
+ - name: Generate incremental licenserc
+ if: >-
+ github.event_name == 'pull_request_target' &&
+ steps.changed-files.outputs.added_modified != ''
+ env:
+ CHANGED_FILES: ${{ steps.changed-files.outputs.added_modified }}
+ run: |
+ python3 - <<'EOF'
+ import yaml, os
+
+ with open('.licenserc.yaml') as f:
+ config = yaml.safe_load(f)
+
+ changed = os.environ.get('CHANGED_FILES', '').strip().split('\n')
+ changed = [p.strip() for p in changed if p.strip()]
+
+ config['header']['paths'] = changed
+
+ with open('.licenserc-incremental.yaml', 'w') as f:
+ yaml.dump(config, f, default_flow_style=False,
allow_unicode=True)
+ EOF
+
- name: Check License
uses: apache/[email protected]
Review Comment:
The preceding steps compute `steps.changed-files.outputs.config_file` and
generate `.licenserc-incremental.yaml`, but this action step never uses that
selected config path. As a result the run still uses the default
`.licenserc.yaml`, so the intended incremental license check is effectively
disabled for normal PRs. Please pass the generated config file to
`apache/skywalking-eyes` (or otherwise wire
`steps.changed-files.outputs.config_file` into the invocation).
##########
cloud/src/meta-service/meta_service_http.cpp:
##########
@@ -980,15 +191,18 @@ void
MetaServiceImpl::http(::google::protobuf::RpcController* controller,
return;
}
- // Process http request
- auto& unresolved_path = cntl->http_request().unresolved_path();
- HttpHandler handler = process_unknown;
- auto it = http_handlers.find(unresolved_path);
- if (it != http_handlers.end()) {
- handler = it->second;
Review Comment:
The old dispatch intentionally defaulted unknown MetaService HTTP paths to
`process_unknown`, whose comment says it is for compatibility with
cloud-manager versions newer than this MS. This new 404 path makes an
authenticated request to a newly introduced cloud-manager endpoint fail on
older MS nodes during rolling or mixed-version deployments. Please preserve the
authenticated unknown-route OK fallback (or otherwise gate this behind an
explicit compatibility decision) instead of returning 404 here.
--
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]
