fix(global-rule): drop --id from create, derive id from plugin name#45
Merged
Merged
Conversation
A global rule's id is set by the backend to the (single) plugin key, so any user-supplied id was silently ignored. - Remove the --id flag and its required-flag check. - Reject --file payloads that include an "id" field, pointing users at "a7 global-rule update" for edits. - Require one of --file or --plugins-json. - Remove the dead PUT branch in the --file path; create always POSTs. - Update the user guide to reflect id-from-plugin behavior. Tests cover create-without-id, file-with-id rejection, and the missing-payload / missing-gateway-group guards. Fixes #37
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR removes the ChangesGlobal Rule ID Derivation from Plugin Name
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/user-guide/global-rule.md`:
- Around line 53-55: The docs table incorrectly states that the `--output` flag
defaults to `yaml` for the `create` command; update the `--output` row in
global-rule.md so the default value shows `json` to match the actual `create`
command behavior (change the table cell for `--output` from `yaml` to `json` and
ensure the `create` command docs referencing `--output` are consistent).
In `@pkg/cmd/global-rule/create/create_test.go`:
- Line 1: The file create_test.go (package create) is missing the required
Apache 2.0 license header; prepend the standard Apache 2.0 header comment block
to the top of the file (above the "package create" declaration) so all Go
sources follow the repository's licensing guideline for **/*.go files.
In `@pkg/cmd/global-rule/create/create.go`:
- Around line 61-63: The current validation only checks that one of opts.File or
opts.PluginsJSON is provided but allows both; update the validation in create.go
(the block using opts.File and opts.PluginsJSON) to return an error when both
are set (e.g., "only one of --file or --plugins-json may be provided") and keep
the existing check for neither being set; locate the check referencing opts.File
and opts.PluginsJSON and replace it with a two-step validation: first error if
both are non-empty, then error if both are empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d758835e-7990-4ea9-9f2f-0dea7d16966d
📒 Files selected for processing (3)
docs/user-guide/global-rule.mdpkg/cmd/global-rule/create/create.gopkg/cmd/global-rule/create/create_test.go
The create handler now rejects payloads containing an "id" field; update the e2e fixtures and skill examples that were still setting one. The resulting id is still asserted to match the plugin name in the verification steps.
- get: default is `table`, not `yaml`. - create / update: default is `json`, not `yaml`. - create: document that `--file` takes precedence when both `--file` and `--plugins-json` are passed.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the misleading --id flag from a7 global-rule create and aligns CLI/documentation/tests with API7 EE’s model where a global rule’s id is derived from the (single) plugin name.
Changes:
- Dropped
--id/Options.IDfromglobal-rule create; create now alwaysPOSTs and rejects file payloads that contain anid. - Added
--plugins-jsonsupport forglobal-rule createand enforced that one of--fileor--plugins-jsonmust be provided. - Updated docs and skill/e2e examples to remove
idfrom create payloads; added unit tests for the new behaviors.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/global_rule_test.go | Updates e2e fixtures/examples to create global rules without an explicit id. |
| skills/a7-recipe-multi-tenant/SKILL.md | Removes id from a global-rule create example JSON payload. |
| skills/a7-persona-operator/SKILL.md | Removes id from global-rule create example payloads. |
| pkg/cmd/global-rule/create/create.go | Removes --id, adds --plugins-json, enforces payload requirements, rejects id in file payloads, and always POSTs. |
| pkg/cmd/global-rule/create/create_test.go | Adds unit tests covering create-with-plugins-json, file payload id rejection, payload requirement, and missing gateway group. |
| docs/user-guide/global-rule.md | Documents id-from-plugin behavior, new --plugins-json flag, and updates examples/flag defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
a7 global-rule create --id <X> --plugins-json '{"cors":...}'ignored--idand returned a resource atid = "cors". API7 EE models eachglobal rule 1:1 with a plugin and uses the plugin key as the rule's
id, so
--idwas a flag the backend would always silently override.This PR drops the misleading flag and tightens the file path:
--idflag,Options.ID, the required-flag check, and theID:field from the request body.--filepayloads that contain anidfield with an expliciterror pointing users at
a7 global-rule updatefor edits.--fileor--plugins-json.--filepath; create always POSTs(
updatealready handles edits via PUT).docs/user-guide/global-rule.mdto reflect id-from-pluginbehavior and document the new
--plugins-jsonflag.Fixes #37.
Test plan
TestGlobalRule_CreateWithoutID— POST with--plugins-json '{"cors":{}}'returns id"cors".TestGlobalRule_CreateFromFileRejectsID— file containingidfails before any HTTP call.TestGlobalRule_CreateRequiresPayload— neither flag supplied returns a clear error.TestGlobalRule_CreateMissingGatewayGroup— no gateway group returns the existing guard.go vet ./...clean.go test ./...clean (full suite green).make checkfailed on pre-existing tooling issues unrelated to thischange (golangci-lint's bundled type checker is out of date for Go
1.25 and reports false positives on
yamlin files this PR does nottouch).
gofmtis clean on the changed files.Summary by CodeRabbit
New Features
Changes
Documentation
Tests