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.

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

feat: perform SchemaRegistry permissions on C*AS sink subjects #8039

Merged
merged 2 commits into from Aug 25, 2021

Conversation

spena
Copy link
Member

@spena spena commented Aug 23, 2021

Description

Added SchemaRegistry permission checks to the sink subjects used by C*AS commands.

Testing done

Describe the testing strategy. Unit and integration tests are expected for any behavior changes.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@spena spena requested review from jzaralim and a team August 23, 2021 14:11
Comment on lines 199 to 198
keyFormat.supportsFeature(SerdeFeature.SCHEMA_INFERENCE)
? SerdeFeatures.of(SerdeFeature.SCHEMA_INFERENCE)
: SerdeFeatures.of(),
Copy link
Member Author

Choose a reason for hiding this comment

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

keyFormat has a supportedFeatures() method that returns a set of features. I could call SerdeFeatures.from(keyFormat.supportedFeatures()) here, but SerdeFeatures.from throws an exception because of incompatible features returned. Because I just need SCHEMA_INFERENCE, then I just added this condition here.

Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, the only way to hit the incompatible features exception is if UNWRAP_SINGLES and WRAP_SINGLES are both included. Why is that happening?

@spena spena force-pushed the sr_perms_check_sink_subject branch from a3aed56 to 665422d Compare August 23, 2021 16:36
Copy link
Contributor

@jzaralim jzaralim left a comment

Choose a reason for hiding this comment

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

lgtm, just one question

Comment on lines 199 to 198
keyFormat.supportsFeature(SerdeFeature.SCHEMA_INFERENCE)
? SerdeFeatures.of(SerdeFeature.SCHEMA_INFERENCE)
: SerdeFeatures.of(),
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, the only way to hit the incompatible features exception is if UNWRAP_SINGLES and WRAP_SINGLES are both included. Why is that happening?

@spena spena force-pushed the sr_perms_check_sink_subject branch from 665422d to 2e2ba6a Compare August 23, 2021 21:45
@spena spena force-pushed the sr_perms_check_sink_subject branch from 2e2ba6a to 8630967 Compare August 24, 2021 20:01
@spena spena merged commit 6878233 into master Aug 25, 2021
@spena spena deleted the sr_perms_check_sink_subject branch August 25, 2021 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants