Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CORE-8797] Improve parsing of POST /subject/{subject}/version object #24860

Merged
merged 2 commits into from
Jan 23, 2025
Merged

[CORE-8797] Improve parsing of POST /subject/{subject}/version object #24860

merged 2 commits into from
Jan 23, 2025

Conversation

michael-redpanda
Copy link
Contributor

@michael-redpanda michael-redpanda commented Jan 17, 2025 •
edited
Loading

Fixes: #24782

TODO:

  • write tests

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.3.x
  • v24.2.x
  • v24.1.x

Release Notes

Bug Fixes

  • Fixes integer overflow issues when given a schema via the POST /subject/{subject}/version where version was > INT_MAX or a negative value was provided.

@michael-redpanda michael-redpanda requested a review from a team January 17, 2025 20:28
@michael-redpanda michael-redpanda self-assigned this Jan 17, 2025
@michael-redpanda michael-redpanda requested review from pgellert and BenPope and removed request for a team January 17, 2025 20:28
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Jan 18, 2025 •
edited
Loading

CI test results

test results on build#60928
test_id test_kind job_url test_status passed
rptest.tests.archival_test.ArchivalTest.test_single_partition_leadership_transfer.cloud_storage_type=CloudStorageType.ABS ducktape https://buildkite.com/redpanda/redpanda/builds/60928#01947653-7c1f-43db-903c-c88b51a94b73 FLAKY 1/2
rptest.tests.consumer_group_balancing_test.ConsumerGroupBalancingTest.test_coordinator_nodes_balance ducktape https://buildkite.com/redpanda/redpanda/builds/60928#01947653-7c1c-44e7-9e32-6c9035603d34 FLAKY 1/2
test results on build#61013
test_id test_kind job_url test_status passed
rptest.tests.archival_test.ArchivalTest.test_all_partitions_leadership_transfer.cloud_storage_type=CloudStorageType.S3 ducktape https://buildkite.com/redpanda/redpanda/builds/61013#01948b3c-cc4d-4882-b9de-59eb3c224160 FLAKY 1/2
test results on build#61120
test_id test_kind job_url test_status passed
rptest.tests.datalake.partition_movement_test.PartitionMovementTest.test_cross_core_movements.cloud_storage_type=CloudStorageType.S3 ducktape https://buildkite.com/redpanda/redpanda/builds/61120#019494c4-cafd-4c80-bdfd-c81b004823cb FLAKY 1/2

@michael-redpanda
sr: Updated handling of integer values
c68eeef 
For POST /subject/{subject}/version, updating the JSON object parsing to
properly handle situations where a negative value is provided or a value
greater than INT_MAX is provided.  If a negative value is provided,
convert it to the appropriate "invalid" type so it can be automatically
determined.

Signed-off-by: Michael Boquard <[email protected]>
@michael-redpanda michael-redpanda force-pushed the bug/core-8797/sr-input-parsing branch from e2ad518 to 2402fc5 Compare January 21, 2025 21:55
@michael-redpanda
Copy link
Contributor Author

Force push 2402fc5:

  • Addressed PR comments - fixed issue with version exhaustion check
  • Fixed lint issues
  • added tests

pgellert
pgellert previously approved these changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@pgellert pgellert left a comment

Choose a reason for hiding this comment

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

lgtm

BenPope
BenPope previously approved these changes Jan 23, 2025
View reviewed changes
src/v/pandaproxy/schema_registry/store.h Outdated Show resolved Hide resolved
@michael-redpanda
sr: Added error for version exhaustion
821a040 
Version exhaustion can occur when attempting to insert schemas using
version '0' and the maximum version for the subject is INT_MAX.  This
differs slightly from upstream implementation wherein the version is
reused and previous instance of the version is overwritten.

Signed-off-by: Michael Boquard <[email protected]>
@michael-redpanda michael-redpanda dismissed stale reviews from BenPope and pgellert via 821a040 January 23, 2025 18:49
@michael-redpanda michael-redpanda force-pushed the bug/core-8797/sr-input-parsing branch from 2402fc5 to 821a040 Compare January 23, 2025 18:49
@michael-redpanda
Copy link
Contributor Author

Force push 821a040:

  • Remove unnecessary log line

@michael-redpanda michael-redpanda merged commit 0dcab09 into redpanda-data:dev Jan 23, 2025
17 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v24.3.x

@vbotbuildovich
Copy link
Collaborator

/backport v24.2.x

@vbotbuildovich
Copy link
Collaborator

/backport v24.1.x

This was referenced Jan 23, 2025
Closed
Closed
Merged
@vbotbuildovich vbotbuildovich mentioned this pull request Jan 23, 2025
Merged
@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v24.1.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-24860-v24.1.x-411 remotes/upstream/v24.1.x
git cherry-pick -x c68eeef9eb 821a0402be

Workflow run logs.

@vbotbuildovich vbotbuildovich mentioned this pull request Jan 23, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenPope BenPope BenPope approved these changes

@pgellert pgellert Awaiting requested review from pgellert

Assignees

@michael-redpanda michael-redpanda

Labels
area/redpanda
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Schema Registry validation issues
4 participants
@michael-redpanda @vbotbuildovich @BenPope @pgellert