Swagger Replication Rule invalid JSON #22724
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22724 +/- ##
===========================================
+ Coverage 45.36% 66.01% +20.64%
===========================================
Files 244 1074 +830
Lines 13333 116417 +103084
Branches 2719 2937 +218
===========================================
+ Hits 6049 76855 +70806
- Misses 6983 35313 +28330
- Partials 301 4249 +3948
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Head branch was pushed to by a user without write access
mlimardo1984
force-pushed
the
21032-swagger-replication-rule-invalid-json
branch
2 times, most recently
from
cce9a54 to
516b5fe
Compare
mlimardo1984
force-pushed
the
21032-swagger-replication-rule-invalid-json
branch
5 times, most recently
from
8368dba to
3166ba9
Compare
mlimardo1984
force-pushed
the
21032-swagger-replication-rule-invalid-json
branch
from
3166ba9 to
fed2ca9
Compare
mlimardo1984
force-pushed
the
21032-swagger-replication-rule-invalid-json
branch
3 times, most recently
from
64e3744 to
db99cec
Compare
mlimardo1984
force-pushed
the
21032-swagger-replication-rule-invalid-json
branch
from
db99cec to
c29d508
Compare
reasonerjt
left a comment
reasonerjt
left a comment
There was a problem hiding this comment.
Could you also manually squash the commits and explain it's for backward compatibility of API to remove the "type" feild? (which I believe it's true)?
src/portal/src/app/base/project/p2p-provider/policy/policy.component.ts
Outdated
Show resolved
Hide resolved
Head branch was pushed to by a user without write access
|
@reasonerjt After the code change, all the tests in the description have been re-run successfully. Thanks! |
mlimardo1984
force-pushed
the
21032-swagger-replication-rule-invalid-json
branch
6 times, most recently
from
6501346 to
15081f4
Compare
Vad1mo
left a comment
Vad1mo
left a comment
There was a problem hiding this comment.
I have run some tests, and it looks like the only change really needed is the removal of type: object from swagger.yaml
Can you elaborate on why the conversion and converter is needed?
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical bug in the Harbor API where creating replication rules via Swagger with filters fails with "Invalid JSON" errors. The issue stemmed from a type mismatch between the Swagger schema definition (which expected a string for filter values) and the actual implementation (which uses objects/arrays for label filters).
Changes:
- Modified Swagger API schema to accept any JSON type for replication filter values instead of constraining to
type: object - Added
convertFilterValuefunction in the backend to properly handle filter value type conversion, particularly for label filters which require arrays - Enhanced frontend TypeScript components to gracefully handle different filter value types (arrays, strings, and other types)
- Added comprehensive unit tests for filter value conversion and replication policy operations
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| api/v2.0/swagger.yaml | Removed type: object constraint from ReplicationFilter value field to allow any JSON type |
| src/server/v2.0/handler/replication.go | Added convertFilterValue function to handle label filter array conversion and applied it in Create/Update operations |
| src/server/v2.0/handler/replication_test.go | New test file with 384 lines covering filter conversions, policy conversions, and edge cases |
| src/portal/src/app/base/project/p2p-provider/policy/policy.component.ts | Added convertFilterValueToString helper to handle array-to-string conversion for UI display |
| src/portal/src/app/base/left-side-nav/replication/replication/create-edit-rule/create-edit-rule.component.ts | Enhanced type checking for label filter values to handle arrays, strings, and other types gracefully |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
@Vad1mo As discussed in Slack, the reason behind this was to ensure data is correctly provided but it has been decided to not modify user input, if the data is wrong, it should be clearly reported. Thanks! |
mlimardo1984
force-pushed
the
21032-swagger-replication-rule-invalid-json
branch
2 times, most recently
from
917e6f7 to
cb00c4c
Compare
The 'type' field in ReplicationFilter schema caused JSON serialization issues
because swagger codegen generates it as a required string field, but the actual
value is interface{} which can be string, []string, or other types depending on
the filter type.
Removing the 'type' field from the schema fixes serialization while maintaining
backward compatibility, as the field wasn't documented or used by clients.
Changes:
- Remove 'type' from ReplicationFilter schema in swagger.yaml
- Simplify filter conversion logic in handler
- Add validation to skip invalid filters
- Add comprehensive tests for filter handling
Signed-off-by: Matteo Limardo <mlimardo@mirantis.com>
remove converFilterValue func
Signed-off-by: Matteo Limardo <mlimardo@mirantis.com>
mlimardo1984
force-pushed
the
21032-swagger-replication-rule-invalid-json
branch
from
cb00c4c to
aaf00e5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Vadim Bauer <Bauer.vadim@gmail.com>
Comprehensive Summary of your change
The API call for creating Replication Rules using Swagger with filters fails with error Invalid JSON due to a type mismatch between the swagger schema definition which expects a string, and the actual implementation passing objects/arrays.
This PR is fixing the problem with type mismatch as well as providing a unit test for replication.
How has been tested
JSON content:
Swagger/UI:
Issue being fixed
Fixes #(22718)
Fixes #(21032)
Please indicate you've done the following: