Skip to content

KAFKA-10338; Support PEM format for SSL key and trust stores (KIP-651)#9345

Merged
rajinisivaram merged 3 commits intoapache:trunkfrom
rajinisivaram:KAFKA-10338-ssl-pem
Oct 6, 2020
Merged

KAFKA-10338; Support PEM format for SSL key and trust stores (KIP-651)#9345
rajinisivaram merged 3 commits intoapache:trunkfrom
rajinisivaram:KAFKA-10338-ssl-pem

Conversation

@rajinisivaram
Copy link
Copy Markdown
Contributor

Adds support for SSL key and trust stores to be specified in PEM format either as files or directly as configuration values.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

retest this please

Copy link
Copy Markdown
Contributor

@omkreddy omkreddy left a comment

Choose a reason for hiding this comment

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

@rajinisivaram Thanks for the PR. LGTM.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unused usePemCerts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we remove empty else block?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_CONFIG => SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_DOC

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_CONFIG => SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_DOC

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unused node variable here and other methods.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed these and few other warnings

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

@omkreddy Thanks for the review, have addressed the comments.

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

@omkreddy Thanks for the review, merging to trunk.

@rajinisivaram rajinisivaram merged commit 7be8bd8 into apache:trunk Oct 6, 2020
javierfreire pushed a commit to javierfreire/kafka that referenced this pull request Oct 8, 2020
apache#9345)

Adds support for SSL key and trust stores to be specified in PEM format either as files or directly as configuration values.

Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
ijuma added a commit to confluentinc/kafka that referenced this pull request Oct 8, 2020
* commit '2804257fe221f37e5098bd': (67 commits)
  KAFKA-10562: Properly invoke new StateStoreContext init (apache#9388)
  MINOR: trivial cleanups, javadoc errors, omitted StateStore tests, etc. (apache#8130)
  KAFKA-10564: only process non-empty task directories when internally cleaning obsolete state stores (apache#9373)
  KAFKA-9274: fix incorrect default value for `task.timeout.ms` config (apache#9385)
  KAFKA-10362: When resuming Streams active task with EOS, the checkpoint file is deleted (apache#9247)
  KAFKA-10028: Implement write path for feature versioning system (KIP-584) (apache#9001)
  KAFKA-10402: Upgrade system tests to python3 (apache#9196)
  KAFKA-10186; Abort transaction with pending data with TransactionAbortedException (apache#9280)
  MINOR: Remove `TargetVoters` from `DescribeQuorum` (apache#9376)
  Revert "KAFKA-10469: Resolve logger levels hierarchically (apache#9266)"
  MINOR: Don't publish javadocs for raft module (apache#9336)
  KAFKA-9929: fix: add missing default implementations (apache#9321)
  KAFKA-10188: Prevent SinkTask::preCommit from being called after SinkTask::stop (apache#8910)
  KAFKA-10338; Support PEM format for SSL key and trust stores (KIP-651) (apache#9345)
  KAFKA-10527; Voters should not reinitialize as leader in same epoch (apache#9348)
  MINOR: Refactor unit tests around RocksDBConfigSetter (apache#9358)
  KAFKA-6733: Printing additional ConsumerRecord fields in DefaultMessageFormatter (apache#9099)
  MINOR: Annotate test BlockingConnectorTest as integration test (apache#9379)
  MINOR: Fix failing test due to KAFKA-10556 PR (apache#9372)
  KAFKA-10439: Connect's Values to parse BigInteger as Decimal with zero scale. (apache#9320)
  ...
rgo pushed a commit to rgo/kafka that referenced this pull request Oct 20, 2020
apache#9345)

Adds support for SSL key and trust stores to be specified in PEM format either as files or directly as configuration values.

Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
@teabot
Copy link
Copy Markdown

teabot commented Sep 10, 2021

How is PEM certificate renewal possible on the producer/consumer client? Is this documented anywhere?

Comment on lines +290 to +291
else if (keyPassword == null)
throw new InvalidConfigurationException("SSL PEM key store is specified, but key password is not specified.");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there an intention here to not allow a FileBasedPemStore containing an unencrypted private key when a key could be provided to PemStore with a null keyPassword?

@vendamere
Copy link
Copy Markdown

Was this change meant to work for the schema registry client as well? I've tried connecting to the schema registry using these settings, but it fails with a message indicating it's looking for a file still.

@teabot
Copy link
Copy Markdown

teabot commented Mar 14, 2022 via email

@bachmanity1
Copy link
Copy Markdown
Contributor

How is PEM certificate renewal possible on the producer/consumer client? Is this documented anywhere?

Hi @teabot , could find the answer to your question? I have also faced a similar. More precisely, is it possible to update the certificate without requiring a restart of the broker?

@teabot
Copy link
Copy Markdown

teabot commented Jan 27, 2024 via email

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.

6 participants