[fix][misc] do not require encryption on system topics

[nicoloboschi]

Fixes #18897

Motivation

System topics on user namespace may be subject to have encryption set on the producer side. However they can't be forced to be encrypted since the encryption keys are set on the client side. Also they don't contain sensitive data.

The two main cases that I encountered are:

1. __change_events: if you enable topic level policies
2. transaction side topics

Modifications

• Always allow messages in the system topics to not be encrypted

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

[lhotari]

LGTM

[lhotari]

/pulsarbot rerun-failure-checks

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 47.40%. Comparing base (1be5a69) to head (e336ff6).
Report is 1982 commits behind head on master.

Files with missing lines	Patch %	Lines
...va/org/apache/pulsar/broker/service/ServerCnx.java	0.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #18898       +/-   ##
=============================================
+ Coverage     33.99%   47.40%   +13.40%     
- Complexity     6476    10635     +4159     
=============================================
  Files           623      703       +80     
  Lines         59103    68844     +9741     
  Branches       6147     7382     +1235     
=============================================
+ Hits          20095    32634    +12539     
+ Misses        36347    32582     -3765     
- Partials       2661     3628      +967     

Flag	Coverage Δ
unittests	47.40% <33.33%> (+13.40%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.../pulsar/broker/service/persistent/SystemTopic.java	92.30% <100.00%> (+0.64%)	⬆️
...va/org/apache/pulsar/broker/service/ServerCnx.java	50.22% <0.00%> (+3.64%)	⬆️

... and 245 files with indirect coverage changes

[michaeljmarshall]

@nicoloboschi - I support this change, but I think it needs a little more work.

This code is relevant:

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Producer.java

Lines 785 to 791 in 5b062f3

	public void checkEncryption() {
	if (topic.isEncryptionRequired() && !isEncrypted) {
	log.info("[{}] [{}] Unencrypted producer is not allowed to produce on topic [{}] anymore",
	producerId, producerName, topic.getName());
	disconnect();
	}
	}

It is used here and in the non-partitioned topic:

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java

Line 2642 in 5b062f3

producer.checkPermissionsAsync().thenRun(producer::checkEncryption)));

If I am reading the code correctly, without updating the Producer#checkEncryption method, an update to a namespace policy will result in an unnecessary disconnection for producers to system topics when encryption becomes required for a namespace.

[nicoloboschi]

@nicoloboschi - I support this change, but I think it needs a little more work.

This code is relevant:

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Producer.java

Lines 785 to 791 in 5b062f3

	public void checkEncryption() {
	if (topic.isEncryptionRequired() && !isEncrypted) {
	log.info("[{}] [{}] Unencrypted producer is not allowed to produce on topic [{}] anymore",
	producerId, producerName, topic.getName());
	disconnect();
	}
	}

It is used here and in the non-partitioned topic:

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java

Line 2642 in 5b062f3

producer.checkPermissionsAsync().thenRun(producer::checkEncryption)));

If I am reading the code correctly, without updating the Producer#checkEncryption method, an update to a namespace policy will result in an unnecessary disconnection for producers to system topics when encryption becomes required for a namespace.

I've implemented isEncryptionRequired to always return false so the disconnection is never triggered. or maybe I didn't get your comments 🤔

[michaeljmarshall]

LGTM.

[nicoloboschi]

Since 2.11.0 system topics are enabled by default so I think we must include this fix in 2.11.0.
For example, transactions in namespaces with encryption required, wouldn't work at all.