[DISCUSS] PIP-296: Support storing broker internal client certificates in metadata store

[mattison chao]

Hello, folks.

I hope this email finds you well. I would like to start a discussion about 
PIP-296 Support storing broker internal client certificates in metadata 
store[1].

Please don't hesitate to leave any concerns or questions.

Best,
Mattison

[1] https://github.com/apache/pulsar/pull/21044/files

Re: [DISCUSS] PIP-296: Support storing broker internal client certificates in metadata store

[Rajan Dhabalia]

Hi,

Pulsar stores different types of metadata into a metadata store which
contains tenant, namespaces and topic metadata. Metadata-store should store
metadata definition and should avoid combining other non metadata related
information especially certificates or keys like encryption/decryption
keys. Apache Pulsar supports envelope encryption for which it requires to
store and retrieve encryption keys but if Pulsar starts managing such keys
then Pulsar will start playing responsibilities of any other KMS systems
and that MUST BE out of the scope of the Pulsar because KMS itself is a big
beast and it comes with lot of security requirements and responsibilities.
And that's why we made it a pluggable component and one can implement its
own implementation. Similar way, managing certificates MUST be out of
Pulsar scope else it will become can of worms which comes with lot security
bugs and concerns , and definitely it's not something we want to store in
Metadata store, So, we MUST not introduce a support of cert management in
Pulsar and MUST NOT store certs into metadata-store, We can certainly see
if we can provide interface and API to make it pluggable and let users
manage their own implementation without introducing such critical
complexities in Pulsar.

Thanks,
Rajan

On Fri, Sep 8, 2023 at 1:21 AM mattison chao  wrote:

>
> Hello, folks.
>
> I hope this email finds you well. I would like to start a discussion about
> PIP-296 Support storing broker internal client certificates in metadata
> store[1].
>
> Please don't hesitate to leave any concerns or questions.
>
> Best,
> Mattison
>
> [1] https://github.com/apache/pulsar/pull/21044/files
>
>

Re: [DISCUSS] PIP-296: Support storing broker internal client certificates in metadata store

[Yubiao Feng]

Hi Mattion

+1

I think this pretty makes Pulsar easy to use.

Thanks
Yubiao


On Fri, Sep 8, 2023 at 4:21 PM mattison chao  wrote:

>
> Hello, folks.
>
> I hope this email finds you well. I would like to start a discussion about
> PIP-296 Support storing broker internal client certificates in metadata
> store[1].
>
> Please don't hesitate to leave any concerns or questions.
>
> Best,
> Mattison
>
> [1] https://github.com/apache/pulsar/pull/21044/files
>
>

Re: [DISCUSS] PIP-296: Support storing broker internal client certificates in metadata store

[mattison chao]

Hi, Rajan

I understand your concerns about the cert management. I just want to let 
cluster data support store base64 encoded brokerClientTrustCerts to avoid 
multiple cluster replication problems in the private cert assignment. We 
already have the ClusterDataImpl that stores the metadata. We can easily expand 
this entity to support base64 encoded private cert.

Additionally, apart from the option of retaining the certificate of the 
destination cluster, are there any other recommendations you might have in mind?

Best,
Mattison
On 8 Sep 2023 at 16:47 +0800, Rajan Dhabalia , wrote:
> Hi,
>
> Pulsar stores different types of metadata into a metadata store which
> contains tenant, namespaces and topic metadata. Metadata-store should store
> metadata definition and should avoid combining other non metadata related
> information especially certificates or keys like encryption/decryption
> keys. Apache Pulsar supports envelope encryption for which it requires to
> store and retrieve encryption keys but if Pulsar starts managing such keys
> then Pulsar will start playing responsibilities of any other KMS systems
> and that MUST BE out of the scope of the Pulsar because KMS itself is a big
> beast and it comes with lot of security requirements and responsibilities.
> And that's why we made it a pluggable component and one can implement its
> own implementation. Similar way, managing certificates MUST be out of
> Pulsar scope else it will become can of worms which comes with lot security
> bugs and concerns , and definitely it's not something we want to store in
> Metadata store, So, we MUST not introduce a support of cert management in
> Pulsar and MUST NOT store certs into metadata-store, We can certainly see
> if we can provide interface and API to make it pluggable and let users
> manage their own implementation without introducing such critical
> complexities in Pulsar.
>
> Thanks,
> Rajan
>
> On Fri, Sep 8, 2023 at 1:21 AM mattison chao  wrote:
>
> >
> > Hello, folks.
> >
> > I hope this email finds you well. I would like to start a discussion about
> > PIP-296 Support storing broker internal client certificates in metadata
> > store[1].
> >
> > Please don't hesitate to leave any concerns or questions.
> >
> > Best,
> > Mattison
> >
> > [1] https://github.com/apache/pulsar/pull/21044/files
> >
> >

Re: [VOTE] Pulsar DotPulsar Release 3.0.0 Candidate 1

[tison]

+1 (binding)

I checked

- Signature and checksums match
- Build the client from the source
- Run examples

Best,
tison.


Yunze Xu  于2023年9月8日周五 01:15写道：

> +1 (binding)
>
> - Verified signature and checksums
> - Build from source with dotnet 7.0.400 on Windows 11
> - Run the example by adding the dotpulsar 3.0.0 dependency
>
> But I think there are some points to improve with the document. I'm
> new to the dotnet CLI though I have some experiences of C# a few years
> ago. I created the project via Visual Studio. However, when I tried to
> add the dependency via `dotnet add` command, I found it failed with a
> project template created by Visual Studio. Eventually I have to create
> and run a project via dotnet CLI:
>
> ```
> cd project-dir
> dotnet new console
> dotnet add package DotPulsar --version 3.0.0
> # Edit the Program.cs
> dotnet build
> dotnet run
> ```
>
> It would be helpful to provide more guide, even links to existings
> tutorial from MSDN.
>
> Thanks,
> Yunze
>
> On Tue, Sep 5, 2023 at 3:51 PM Zike Yang  wrote:
> >
> > +1 (non-binding)
> >
> > - Verified signature and checksums
> > - Install the nupkg file
> > - Build the client from the source
> > - Run examples
> >
> > But I find that the dotpulsar 3.0.0 has already been published to the
> > nuget.org: https://www.nuget.org/packages/DotPulsar/3.0.0
> >
> > BR,
> > Zike Yang
> >
> > On Sun, Sep 3, 2023 at 11:48 AM tison  wrote:
> > >
> > > Hi everyone,
> > >
> > > This is the first release candidate for Apache DotPulsar, version
> 3.0.0.
> > >
> > > It fixes the following issues:
> > > https://github.com/apache/pulsar-dotpulsar/compare/2.11.1...3.0.0
> > >
> > > Please download the source files and review this release candidate:
> > > - Download the source package, verify shasum and asc
> > > - Follow the README.md to build and run the DotPulsar.
> > >
> > > The vote will be open for at least 72 hours. It is adopted by majority
> > > approval, with at least 3 PMC affirmative votes.
> > >
> > > Source files:
> > >
> https://dist.apache.org/repos/dist/dev/pulsar/pulsar-dotpulsar-3.0.0-candidate-1/
> > >
> > > Pulsar's KEYS file containing PGP keys we use to sign the release:
> > > https://downloads.apache.org/pulsar/KEYS
> > >
> > > The tag to be voted upon:
> > > v3.0.0
> > > https://github.com/apache/pulsar-dotpulsar/releases/tag/3.0.0
> > >
> > > Please review and vote on the release candidate #1 for the version
> 3.0.0,
> > > as follows:
> > >
> > > [ ] +1, Approve the release
> > > [ ] -1, Do not approve the release (please provide specific comments)
> > >
> > > Best,
> > > tison.
>

RE: [VOTE] Pulsar DotPulsar Release 3.0.0 Candidate 1

[Zili Chen]

Hi David,

Glad to see you participating in the vote process.

Generally, on a vote for release candidate, we +1 with reasons. You can take 
Zike's reply as an example.

On 2023/09/06 17:29:10 David Jensen wrote:
>  Thanks for helping to bring this release forward, Tison :)
> 
> +1 (non-binding)
> 
> Greetings David Jensen
> 
> On 2023/09/03 03:47:18 tison wrote:
> > Hi everyone,
> >
> > This is the first release candidate for Apache DotPulsar, version 3.0.0.
> >
> > It fixes the following issues:
> > https://github.com/apache/pulsar-dotpulsar/compare/2.11.1...3.0.0
> >
> > Please download the source files and review this release candidate:
> > - Download the source package, verify shasum and asc
> > - Follow the README.md to build and run the DotPulsar.
> >
> > The vote will be open for at least 72 hours. It is adopted by majority
> > approval, with at least 3 PMC affirmative votes.
> >
> > Source files:
> >
> https://dist.apache.org/repos/dist/dev/pulsar/pulsar-dotpulsar-3.0.0-candidate-1/
> >
> > Pulsar's KEYS file containing PGP keys we use to sign the release:
> > https://downloads.apache.org/pulsar/KEYS
> >
> > The tag to be voted upon:
> > v3.0.0
> > https://github.com/apache/pulsar-dotpulsar/releases/tag/3.0.0
> >
> > Please review and vote on the release candidate #1 for the version 3.0.0,
> > as follows:
> >
> > [ ] +1, Approve the release
> > [ ] -1, Do not approve the release (please provide specific comments)
> >
> > Best,
> > tison.
> >
> 

Re: [DISCUSS] PIP-296: Support storing broker internal client certificates in metadata store

[Rajan Dhabalia]

As I said, Pulsar is not made for cert management system and introducing
such extensions will bring a lot of other complexities such as cert
rotation, access rights, expiry of certs, securing certs, supporting
different types of keystore and cert format etc,..  and that will be out of
the scope of Pulsar as a messaging system. If we need such things then make
it pluggable same as how we do with encryption key and users can have their
custom implementation where users can store certs in any format and at any
location (metadata, file-system) but that implementation must be outside of
Pulsar repo to avoid scope of this complexity.

Thanks,
Rajan

On Fri, Sep 8, 2023 at 2:11 AM mattison chao  wrote:

> Hi, Rajan
>
> I understand your concerns about the cert management. I just want to let
> cluster data support store base64 encoded brokerClientTrustCerts to avoid
> multiple cluster replication problems in the private cert assignment. We
> already have the ClusterDataImpl that stores the metadata. We can easily
> expand this entity to support base64 encoded private cert.
>
> Additionally, apart from the option of retaining the certificate of the
> destination cluster, are there any other recommendations you might have in
> mind?
>
> Best,
> Mattison
> On 8 Sep 2023 at 16:47 +0800, Rajan Dhabalia ,
> wrote:
> > Hi,
> >
> > Pulsar stores different types of metadata into a metadata store which
> > contains tenant, namespaces and topic metadata. Metadata-store should
> store
> > metadata definition and should avoid combining other non metadata related
> > information especially certificates or keys like encryption/decryption
> > keys. Apache Pulsar supports envelope encryption for which it requires to
> > store and retrieve encryption keys but if Pulsar starts managing such
> keys
> > then Pulsar will start playing responsibilities of any other KMS systems
> > and that MUST BE out of the scope of the Pulsar because KMS itself is a
> big
> > beast and it comes with lot of security requirements and
> responsibilities.
> > And that's why we made it a pluggable component and one can implement its
> > own implementation. Similar way, managing certificates MUST be out of
> > Pulsar scope else it will become can of worms which comes with lot
> security
> > bugs and concerns , and definitely it's not something we want to store in
> > Metadata store, So, we MUST not introduce a support of cert management in
> > Pulsar and MUST NOT store certs into metadata-store, We can certainly see
> > if we can provide interface and API to make it pluggable and let users
> > manage their own implementation without introducing such critical
> > complexities in Pulsar.
> >
> > Thanks,
> > Rajan
> >
> > On Fri, Sep 8, 2023 at 1:21 AM mattison chao 
> wrote:
> >
> > >
> > > Hello, folks.
> > >
> > > I hope this email finds you well. I would like to start a discussion
> about
> > > PIP-296 Support storing broker internal client certificates in metadata
> > > store[1].
> > >
> > > Please don't hesitate to leave any concerns or questions.
> > >
> > > Best,
> > > Mattison
> > >
> > > [1] https://github.com/apache/pulsar/pull/21044/files
> > >
> > >
>

Re: [DISCUSS] PIP-296: Support storing broker internal client certificates in metadata store

[Michael Marshall]

Thanks for your PIP. I respect that this is a challenge for users. I have
spent hours debugging basic cert mistakes, so I agree it is worth
discussing. That being said, I have some concerns about the design.

The PIP dropped the section that is supposed to describe the security
implications of the feature. I’d like to see that section covered in detail
for this PIP.

This feature affects what remote peers a client broker will trust and then
subsequently send user data and admin level auth data. As such, I view the
trust store as a secret and think it should be handled accordingly.

My understanding is that none of our metadata stores are intended for
secrets, and if that is true, then I do not support storing secrets or
recommending that any of our users store secrets in the metadata store.

Thanks,
Michael

On Fri, Sep 8, 2023 at 12:58 PM Rajan Dhabalia 
wrote:

> As I said, Pulsar is not made for cert management system and introducing
> such extensions will bring a lot of other complexities such as cert
> rotation, access rights, expiry of certs, securing certs, supporting
> different types of keystore and cert format etc,..  and that will be out of
> the scope of Pulsar as a messaging system. If we need such things then make
> it pluggable same as how we do with encryption key and users can have their
> custom implementation where users can store certs in any format and at any
> location (metadata, file-system) but that implementation must be outside of
> Pulsar repo to avoid scope of this complexity.
>
> Thanks,
> Rajan
>
> On Fri, Sep 8, 2023 at 2:11 AM mattison chao 
> wrote:
>
> > Hi, Rajan
> >
> > I understand your concerns about the cert management. I just want to let
> > cluster data support store base64 encoded brokerClientTrustCerts to avoid
> > multiple cluster replication problems in the private cert assignment. We
> > already have the ClusterDataImpl that stores the metadata. We can easily
> > expand this entity to support base64 encoded private cert.
> >
> > Additionally, apart from the option of retaining the certificate of the
> > destination cluster, are there any other recommendations you might have
> in
> > mind?
> >
> > Best,
> > Mattison
> > On 8 Sep 2023 at 16:47 +0800, Rajan Dhabalia ,
> > wrote:
> > > Hi,
> > >
> > > Pulsar stores different types of metadata into a metadata store which
> > > contains tenant, namespaces and topic metadata. Metadata-store should
> > store
> > > metadata definition and should avoid combining other non metadata
> related
> > > information especially certificates or keys like encryption/decryption
> > > keys. Apache Pulsar supports envelope encryption for which it requires
> to
> > > store and retrieve encryption keys but if Pulsar starts managing such
> > keys
> > > then Pulsar will start playing responsibilities of any other KMS
> systems
> > > and that MUST BE out of the scope of the Pulsar because KMS itself is a
> > big
> > > beast and it comes with lot of security requirements and
> > responsibilities.
> > > And that's why we made it a pluggable component and one can implement
> its
> > > own implementation. Similar way, managing certificates MUST be out of
> > > Pulsar scope else it will become can of worms which comes with lot
> > security
> > > bugs and concerns , and definitely it's not something we want to store
> in
> > > Metadata store, So, we MUST not introduce a support of cert management
> in
> > > Pulsar and MUST NOT store certs into metadata-store, We can certainly
> see
> > > if we can provide interface and API to make it pluggable and let users
> > > manage their own implementation without introducing such critical
> > > complexities in Pulsar.
> > >
> > > Thanks,
> > > Rajan
> > >
> > > On Fri, Sep 8, 2023 at 1:21 AM mattison chao 
> > wrote:
> > >
> > > >
> > > > Hello, folks.
> > > >
> > > > I hope this email finds you well. I would like to start a discussion
> > about
> > > > PIP-296 Support storing broker internal client certificates in
> metadata
> > > > store[1].
> > > >
> > > > Please don't hesitate to leave any concerns or questions.
> > > >
> > > > Best,
> > > > Mattison
> > > >
> > > > [1] https://github.com/apache/pulsar/pull/21044/files
> > > >
> > > >
> >
>

Re: [DISCUSS] PIP-281: Optimize Bundle Unload(Transfer) Protocol for ExtensibleLoadManager

[Michael Marshall]

Thanks for your response, Matteo.

The benefits of my proposed alternative are dependent on the amount of
time between when a broker stops accepting messages and when the
client learns of the new host for the topic. The main areas this will
matter are failure scenarios where that delta or that "mean time to
recovery" is large. My preference is that we build a resilient and
reactive protocol that does not make assumptions about how quickly
certain states will last, like the length of time it will take for
topic transfer. I am partial to the idea that we should "let it fail".
Part of that is sending events to the relevant parties as we discover
them (like telling producers and consumers to pause) as a way to allow
for graceful handling of events.

> Drawbacks:
> 1. Brokers need to be able to handle both cases
> a. Send a pause-producer command and ignore any in-inflight messages
> b. If the producer does not support this feature, the broker would
> have to ignore messages anyway

In my version, the broker would either tell the client to close the
producer or the consumer--just like we do today--or would tell the
producer/consumer to pause. Branching logic in the broker based on the
protocol version doesn't seem complicated to me. We already do this.

>   2. This change would create multiple complications in the clients
> implementations:
> a. A new state to take care of
> b. What to do with the pending messages, eg: manage and addition
> pending messages queue

I disagree with point b: clients already have to handle pending
messages while they are disconnected. I agree we'd need a new state,
but it would be a special kind of "reconnecting" state which we
already have for every client.

>   3. What to do with consumers (eg: broker would still have to ignore the
> acks while the topic is closing)

Consumers would behave the same way they do today when they are
disconnected from a broker. Further, consumers could potentially use
the "pause" state to queue their acks to decrease the number of dupes.
By letting the consumer send acks that are explicitly ignored, we are
removing information that clients have access to today. Of course
we're working with at least once messaging, so it's technically
acceptable to let them send acks that will be ignored, but our goal is
to decrease dupes.

> I don't see the sparing of network traffic during the failover as a huge
> benefit that justifies the added complexity in broker & client libraries.

The difference would likely be small on an individual basis, but
larger clusters could really benefit from a nuanced protocol. Given
that the majority of messaging system costs come from network transfer
across AZ boundaries, I would think we should prioritize efficiency
over simplicity of design when the cost is only an extra state and
protocol message.

Ultimately, I don't see support for my proposed alternative, so I am
not going to push for my idea further unless prompted. Perhaps one
request might be to add a metric to track how long we "ignore protocol
messages" from producers and consumers so we can know the impact and
that data can influence future design decisions.

Thank you for the discussion, Matteo and Heesung.

- Michael

On Tue, Sep 5, 2023 at 8:08 PM Matteo Merli  wrote:
>
> --
> Matteo Merli
> 
>
>
> On Tue, Aug 8, 2023 at 9:56 PM Michael Marshall 
> wrote:
>
> > Thanks for your proposal, Heesung. Sorry for my late response. Just
> > want to make a few points to hopefully improve the implementation.
> > Overall, I think this feature is absolutely the right direction for
> > Pulsar and will be a great improvement. I remember discussing this at
> > some community meetings last year. See the last paragraph of [0].
> >
> > In general, my primary question is about the protocol changes you're
> > proposing, both the commands and the way the commands are sent.
> >
> > > I am worried about the backward compatibility if we add a new proto
> > message
> > > -- how can the old versions of clients handle this new proto message?
> >
> > The protocolVersion [1] object in the proto definition is how we
> > guarantee backwards and forwards compatibility for all combinations of
> > brokers/clients. The client and the broker each share their version
> > during the handshake and then they only use commands known to both
> > peers.
>
>
> > We can introduce a new command here if we want/need. The nuance is
> > that the server logic will branch depending on the client's
> > protocolVersion.
> >
>
> > I don't see versioning the protocol as a negative as long as there is
> > sufficient benefit. In this case, the new command serves an explicit
> > purpose to make the protocol more reactive (the client knows what is
> > happening as it happens without any need to poll) and less wasteful
> > (no unnecessary data transfer).
> >
> > One reason I want to push on this design is because I think it will be
> > harder to change once it is in place. Adding the new broker URL to the
> 

Re: Pulsar Helm Chart: Add HPA scaleUp/scaleDown behavior support

[Michael Marshall]

I approved the PR. The helm chart's status is a bit ambiguous at the
moment. I'm not sure if there is anyone planning on doing a release.

- Michael

On Wed, Sep 6, 2023 at 11:48 AM Frank Kelly
 wrote:
>
> Hello Pulsar Family,
>
>  When folks get a chance I would appreciate some feedback on this proposed
> enhancement for Broker and Proxy autoscaling.
> https://github.com/apache/pulsar-helm-chart/pull/391
>
> Thanks in advance
>
> Frank Kelly
>
> --
>
> *Frank Kelly | Principal Engineer, Core Platform Team*
>
> www.cogitocorp.com | Cogito on LinkedIn
>