Status

Current state: Accepted

Discussion thread: https://lists.apache.org/thread/somdgdoz1o3z5rwqozsty97fdo2bgkqk

Vote thread: https://lists.apache.org/thread/4gqwz6g4msw0j85784tf500vk0yr3qtz

JIRA: KAFKA-18926 - Getting issue details... STATUS

Please keep the discussion on the mailing list rather than commenting on the wiki (wiki discussions get unwieldy fast).

Motivation

Before KRaft, custom KafkaPrincipalBuilder implementations only needed to build principals. However, KRaft requires brokers to forward requests involving these principals to controllers. Without corresponding KafkaPrincipalSerde implementations, brokers cannot serialize/deserialize them, causing failures.

Currently, the API doesn't enforce the implementation of KafkaPrincipalSerde alongside KafkaPrincipalBuilder. Users would have already encountered serialization/deserialization issues during controller communication. This KIP aims to rectify this by making it a compile-time error, allowing developers to identify and fix the issue when implementing their custom KafkaPrincipalBuilder classes.

Public Interfaces

org.apache.kafka.common.security.auth.KafkaPrincipalBuilder

public interface KafkaPrincipalBuilder {
    KafkaPrincipal build(AuthenticationContext context);
}

Proposed Changes

The proposed change is to modify the KafkaPrincipalBuilder interface to extend the KafkaPrincipalSerde interface. The updated interface definition will be:

public interface KafkaPrincipalBuilder extends KafkaPrincipalSerde {
    KafkaPrincipal build(AuthenticationContext context);
}

This change will introduce a compile-time error for any existing KafkaPrincipalBuilder implementation that does not also implement KafkaPrincipalSerde.

Compatibility, Deprecation, and Migration Plan

This is a change to a public API, and therefore has the potential to break existing code. However, the risk is considered acceptable because custom KafkaPrincipalBuilder must implement KafkaPrincipalSerde to work with KRaft. (This is already noted in the official doc)

Thus this change is proposed for a minor or feature release (specifically, 4.1 as suggested), and it will not affect any user.

One thing need to be mentioned is that DefaultKafkaPrincipalBuilder already implements KafkaPrincipalBuilder and KafkaPrincipalSerde. After this KIP, it will only implement KafkaPrincipalBuilder.

Now

public class DefaultKafkaPrincipalBuilder implements KafkaPrincipalBuilder, KafkaPrincipalSerde {
 // Implementation...
}

To-be

public class DefaultKafkaPrincipalBuilder implements KafkaPrincipalBuilder {
 // Implementation...
}

Test Plan

Given that existing KafkaPrincipalBuilder implementations used with KRaft already implement KafkaPrincipalSerde, this change should not result in new compile-time errors, and existing test cases are expected to pass.

Rejected Alternatives

An alternative solution is to introduce a new interface that extends both KafkaPrincipalBuilder and KafkaPrincipalSerde. This approach avoids directly modifying the existing KafkaPrincipalBuilder interface and offers a more conservative evolution of the API.

public interface KafkaPrincipalBuilderWithSerde extends KafkaPrincipalBuilder, KafkaPrincipalSerde {
    // Inherits build(AuthenticationContext context), serialize(KafkaPrincipal),
    // and deserialize(byte[]) methods.
}

However, given that any existing KafkaPrincipalBuilder implementation within a KRaft environment must have already implemented KafkaPrincipalSerde, introducing a new interface is unnecessary.

  • No labels