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

Capture all fields in Other class to enable more powerful roles #584

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aheiska
Copy link

@aheiska aheiska commented Jun 14, 2024

  • Added @JsonAnyGetter and @JsonAnySetter and backing hashmap to Other
  • Implemented unit test
  • Implemented integration test
  • Initial documentation
  • Please check if the PR fulfills these requirements
  • The commit messages are descriptive
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • An issue has been created for the pull requests. Some issues might require previous discussion.
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

This adds unmappedFields map with @JsonAnyGetter / @JsonAnySetter to capture all fields. Extra fields are added to rendering data map at Other::asMap() to allow their usage in roles, which allows more powerful roles because resourceName is not limited to fields in Other.

  • What is the current behavior? (You can also link to an open issue here)

resourceName in roles limits to fields in Other class (transactionId, idempotence, group, topic, subject, connector).

  • What is the new behavior (if this is a feature change)?

When rendering a role fields with custom names from yaml becomes available.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

Can't think of anything particularly hazardous. None of the previous tests needed fixing. Spotting typos becomes harder and existing fields in Other become somewhat unnecessary.

  • Other information:

I know project is in hibernation state, but I made this anyway if community finds this useful. Please let me know what you think about this approach. I wrote initial documentation, I can improve it if you think this can be merged. Name mirrorMaker in tests and in documentation is probably a poor choice; permissions given in those are not particularly good example of mirror maker acls, it was just a first public example of an app that uses several topics and has some naming requirements.

- Added @JsonAnyGetter and @JsonAnySetter and backign hashmap
- Implented unit test
- Implemented integration test
- Initial documentation
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.

1 participant