Skip to content

fix(openapi): support user-token deletion of encoded keys - #5676

Merged
nobodyiam merged 2 commits into
apolloconfig:masterfrom
nobodyiam:codex/fix-user-token-encoded-delete
Sep 20, 2026
Merged

nobodyiam merged 2 commits into
apolloconfig:masterfrom
nobodyiam:codex/fix-user-token-encoded-delete

Conversation

@nobodyiam

@nobodyiam nobodyiam commented Sep 20, 2026 •

Copy link
Copy Markdown
Member

What's the purpose of this PR

User-token deletion of keys containing / or \ fails before reaching the controller because the generated encoded-items DELETE interface requires an operator. Upgrade the Portal contract reference to the released apollo-openapi v0.3.12, which makes that parameter optional so the existing controller can use the token owner. Consumer-token operator validation remains in place.

Which issue(s) this PR fixes

Contract fix: apolloconfig/apollo-openapi#39

Brief changelog

  • Pin Portal generation to the released v0.3.12 contract.
  • Cover plain, slash, and backslash keys in Spring parameter-binding tests for user-token identity, spoofed operators, consumer-token operators, and permission denial.
  • Add three server-owned E2E cases that create real items, delete without an operator, verify 404 on read-back, and check the remaining keys and values. The existing @regression workflow includes them.

Validation

  • ./mvnw -B -ntp clean test: 1281 passed, 1 skipped, no failures or errors. Used the released contract URL from the POM without a local spec override.
  • Scoped spotless:apply and full spotless:check passed.
  • OpenAPI compatibility checker tests passed; v0.3.11 to v0.3.12 comparison passed for 153 operations and 49 schemas.
  • Fresh Assembly build passed using the released contract URL, without a local spec override.
  • All three server-owned deletion E2E cases passed on the new Assembly.
  • Replayed the original failure through the real apollo-cli: user-token deletion without an operator passed for plain, slash, and backslash keys; subsequent reads returned not_found, and other keys and values were preserved. Test apps and tokens were cleaned up.

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Run mvn spotless:apply to format your code.
  • Update the CHANGES log.

Copilot AI lite review requested due to automatic review settings September 20, 2026 10:24
@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f6a7399f-ef71-4a70-a8b3-dec9b5a46e71

📥 Commits

Reviewing files that changed from the base of the PR and between 74d9850 and bb2bc45.

📒 Files selected for processing (5)
  • CHANGES.md
  • apollo-portal/pom.xml
  • apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/ItemControllerParamBindLowLevelTest.java
  • e2e/README.md
  • e2e/portal-e2e/tests/portal-item-delete.spec.js

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The PR updates the OpenAPI specification reference and adds low-level and Playwright regression coverage for user-token deletion of plain, slash-containing, and backslash-containing keys.

Changes

Item deletion regression

Layer / File(s) Summary
OpenAPI specification update
apollo-portal/pom.xml
The Portal build now uses the v0.3.12 OpenAPI specification.
Low-level deletion authorization coverage
apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/ItemControllerParamBindLowLevelTest.java
Parameterized tests cover operator resolution, permission rejection, and normal or encoded item routes.
End-to-end deletion regression
e2e/portal-e2e/tests/portal-item-delete.spec.js, e2e/README.md, CHANGES.md
The Playwright test creates scoped tokens, deletes each key shape, verifies remaining data, cleans up resources, and documents the regression and execution command.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: klboke

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (3 skipped: 3 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: support for user-token deletion of encoded keys in the OpenAPI contract.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The E2E assertion conflicts with current behavior, and required release-note and documentation updates remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Low severity

Open (2)
What changed in this PR

Updates Portal to OpenAPI v0.3.12, enabling user-token deletion of encoded keys without an operator.

Changes:

  • Adds parameter-binding and identity tests.
  • Adds end-to-end deletion coverage.
  • Documents and pins the updated contract.
File Summary Review notes
e2e/​README.md Documents regression coverage. Add the required CHANGES.md entry. Nit, 1 vote.
e2e/​portal-e2e/​tests/​portal-item-delete.spec.js Adds deletion scenarios for encoded keys. Read-after-delete expects 404, but the current implementation returns 200 with an empty body. Critical, 1 vote.
apollo-portal/​src/​test/​java/​com/​ctrip/​framework/​apollo/​openapi/​v1/​controller/​ItemControllerParamBindLowLevelTest.java Expands operator and permission tests. No final comments.
apollo-portal/​pom.xml Pins the OpenAPI contract to v0.3.12. Add the CHANGES.md entry and update API documentation and contract references. Nits, 2 and 1 votes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

expect(deleteItem.status(), `delete ${target.kind} key without operator`).toBe(200);

const deletedItem = await request.get(`${namespaceUrl}/${target.resource}`, { headers });
expect(deletedItem.status(), 'deleted key must no longer be readable').toBe(404);
Comment thread apollo-portal/pom.xml
<name>Apollo Portal</name>
<properties>
<apollo.openapi.spec.url>https://raw.githubusercontent.com/apolloconfig/apollo-openapi/v0.3.11/apollo-openapi.yaml</apollo.openapi.spec.url>
<apollo.openapi.spec.url>https://raw.githubusercontent.com/apolloconfig/apollo-openapi/v0.3.12/apollo-openapi.yaml</apollo.openapi.spec.url>
@nobodyiam
nobodyiam merged commit cb72c72 into apolloconfig:master Sep 20, 2026
18 checks passed
@nobodyiam
nobodyiam deleted the codex/fix-user-token-encoded-delete branch September 20, 2026 10:42
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 20, 2026
@nobodyiam nobodyiam added this to the 3.0.0 milestone Sep 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants