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

Unit testing #2

Merged
merged 2 commits into from
Jul 31, 2018
Merged

Unit testing #2

merged 2 commits into from
Jul 31, 2018

Conversation

royantman
Copy link
Contributor

Added unitests for:

  • x509 Cert V1 (No SAN extension)
  • x509 Cert V3 (SAN contains SPIFFE ID)
  • x509 Cert V3 (SAN contains normal FQDN, no SPIFFE ID)
  • Non-SSL context handling

@royantman royantman requested a review from adamglt July 30, 2018 20:44
<dependencies>
<dependency>
<groupId>org.apache.kafka</groupId>
<artifactId>kafka_2.11</artifactId>
<version>1.1.1</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

commons not needed here

pom.xml Outdated
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
<version>1.6.4</version>

<scope>test</scope>
Copy link
Contributor

Choose a reason for hiding this comment

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

slf4j-simple should be test-scoped, kafka brings its own impl

public class SpiffePrincipalBuilder implements KafkaPrincipalBuilder {
private static final Logger LOG = LoggerFactory.getLogger(SpiffePrincipalBuilder.class);

private static final String SPIFFE_TYPE = "SPIFFE";

public KafkaPrincipal build(AuthenticationContext context) {
if (context instanceof PlaintextAuthenticationContext) {
if (!(context instanceof SslAuthenticationContext)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

skip the plaintext/sasl difference

try {
// Read cert
ByteArrayInputStream certInputStream =
new ByteArrayInputStream(IOUtils.toByteArray(classLoader.getResourceAsStream(resourcePath)));
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed


try {
X509Certificate cert = getResourceAsCert("subject-only-cert.pem");
Copy link
Contributor

Choose a reason for hiding this comment

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

extract identical code

assertEquals(KafkaPrincipal.USER_TYPE, principal.getPrincipalType());

// Identity should be a string
assertNotNull(principal.getName());
Copy link
Contributor

Choose a reason for hiding this comment

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

assert with goldenfile

@adamglt adamglt merged commit c148e6e into master Jul 31, 2018
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.

2 participants