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

Update to Kafka 2.13-2.7.0 #18

Merged
merged 1 commit into from
Mar 29, 2021
Merged

Conversation

mdanielolsson
Copy link
Contributor

Updating Scala to 2.13 and Kafka 2.7.0. This comes with big changes since Kafka has deprecated their old libraries and the new API is preferred.
The input to OPA will change after this change. More details are described in the proposed changelog update.
I'm suggesting this would be a major release.

if (superUsers.contains(session.principal.toString)) {
logger.trace(s"User ${session.principal} is super user")
return true
override def authorize(requestContext: AuthorizableRequestContext, actions: ju.List[Action]): ju.List[AuthorizationResult] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rename ju to something more descriptive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, not sure if we even need to shorten it. I'll remove and use java.util instead.

case class Request(input: Input)
case class CachableRequest(principal: KafkaPrincipal, actions: ju.List[Action], connectionId: InetAddress)
case class AzRequestContext(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it's only used in tests, I'd suggest moving it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it to a new file with the tests since it's used in both tests, benchmark and spec.

@codecov
Copy link

codecov bot commented Mar 18, 2021

Codecov Report

Merging #18 (4a0e144) into master (44b0749) will increase coverage by 6.38%.
The diff coverage is 51.51%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #18      +/-   ##
============================================
+ Coverage     46.34%   52.72%   +6.38%     
- Complexity       14       20       +6     
============================================
  Files             1        1              
  Lines            41       55      +14     
  Branches          9       14       +5     
============================================
+ Hits             19       29      +10     
- Misses            7       10       +3     
- Partials         15       16       +1     
Impacted Files Coverage Δ Complexity Δ
...om/bisnode/kafka/authorization/OpaAuthorizer.scala 52.72% <51.51%> (+6.38%) 20.00 <9.00> (+6.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44b0749...4a0e144. Read the comment docs.

@mdanielolsson mdanielolsson self-assigned this Mar 22, 2021
@mdanielolsson mdanielolsson marked this pull request as draft March 23, 2021 10:22
@mdanielolsson mdanielolsson force-pushed the update_kafka_2.13-2.7.0 branch from d03954e to 4a0e144 Compare March 26, 2021 16:01
@mdanielolsson mdanielolsson marked this pull request as ready for review March 26, 2021 16:05
@mdanielolsson mdanielolsson merged commit b46db48 into master Mar 29, 2021
@anderseknert
Copy link
Member

Nice! 👍

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.

4 participants