Skip to content

[Do not merge]Review Support TLS listeners - #1223

Closed
678098 wants to merge 1 commit into
bloomberg:mainfrom
678098:260326_tls_review
Closed

[Do not merge]Review Support TLS listeners#1223
678098 wants to merge 1 commit into
bloomberg:mainfrom
678098:260326_tls_review

Conversation

@678098

@678098 678098 commented Mar 26, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Added
=====

- TLS configuration in broker config
- Helper script for generating test certs and CAs
- TLS options for NtcChannel
- Loading certificates and authority data specified from bmqbrkrcfg.json
- SessionOptions to bmq package for configuring client sessions
- --tls-authority and --tls-version options to bmqtool to configure
  session options
- Client sessions will now require broker TLS sessions when TLS protocol
  versions are specified
- Create CertificateStore component for bmqio
- Integration tests for TLS

Changed
=======

- Update ntf-core and bde dependencies

Signed-off-by: Taylor Foxhall <tfoxhall@bloomberg.net>

Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
@678098
678098 requested a review from a team as a code owner March 26, 2026 21:20
Comment thread .gitignore

# 'sim_cpp11_features.pl' backups
*.bak
*.bak

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Might be moved to another PR to simplify

class TcpInterfaceConfigValidatorTest : public ::testing::Test {};

BMQTST_TEST_F(TcpInterfaceConfigValidatorTest, breathingTest)
TEST_F(TcpInterfaceConfigValidatorTest, breathingTest)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Is it worth supporting both GTEST/non-GTEST UTs?
I can see two problems with GTEST usage:

  • This is an extra dependency that we can avoid. Currently we have to build it for sanitizers that increases its build time. Also most of the tests are written without GTEST, so converting GTEST to native tests format is a low-effort work. Not sure about mocks though.
  • Each time GTEST UT fails, we have a UT number to debug. However, this number is not mentioned anywhere in UT sources, the numbers are assigned automatically in the same order as we declare these test functions. This is not convenient.

libfl-dev \
libbenchmark-dev \
libgmock-dev \
libgtest-dev \

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Have a comment about gtest below, would rather not add an extra dependency if not really necessary.
This should also reduce the number of changes in the PR.

Comment thread bin/build-darwin.sh
# :: Optionally install prerequisites :::::::::::::::::::::::::::::::::::::::::

REQ_PKGS=(cmake flex bison google-benchmark googletest ninja pkg-config zlib)
REQ_PKGS=(cmake flex bison google-benchmark googletest lz4 ninja openssl pkg-config zlib zstd)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We can update ntf-core build in a separate PR so it does not require lz4 and zstd for now

Comment thread bin/codegen.sh
;;
*)
echo "Unrecognized/missing codegen target" >&2
usage

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The changes in this file might be moved to a separate PR to simplify this PR and be merged faster

// '#include'd in the original header.
//
// Generated on Wed Jun 18 14:44:06 2025
// Generated on Wed Mar 25 20:05:02 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This file might be removed from diff

# define COMPILING_BMQU_OPERATIONCHAIN_H
# include <bmqu_operationchain_cpp03.h>
#undef COMPILING_BMQU_OPERATIONCHAIN_H
# undef COMPILING_BMQU_OPERATIONCHAIN_H

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The file might be removed from diff

// Automatically generated file. **DO NOT EDIT**

// Generated on Wed Jun 18 14:44:06 2025
// Generated on Wed Mar 25 20:05:02 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The file might be removed from diff

// '#include'd in the original header.
//
// Generated on Wed Jun 18 14:44:06 2025
// Generated on Wed Mar 25 20:05:02 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The file might be removed from diff

fsm_cluster,
)
# Load the fixtures defined in the BlazingMQ support library
pytest_plugins = "blazingmq.dev.it.fixtures"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Does it remove pylint warnings?
This might be moved to another PR

@chrisbeard
chrisbeard marked this pull request as draft March 30, 2026 15:33
@678098 678098 closed this Jul 7, 2026
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