[SRVKE-1687] Unify KafkaChannel and KafkaBroker secret format #95471

ochromy · 2025-07-01T09:12:34Z

Version(s):
serverless-docs-1.35+

Issue:

https://issues.redhat.com/browse/SRVKE-1687

Link to docs preview:

QE review:

QE has approved this change.

ocpdocs-previewbot · 2025-07-01T09:18:32Z

🤖 Mon Jul 21 14:21:54 - Prow CI generated the docs preview:

https://95471--ocpdocs-pr.netlify.app/openshift-serverless/latest/eventing/brokers/kafka-broker.html
https://95471--ocpdocs-pr.netlify.app/openshift-serverless/latest/eventing/channels/serverless-kafka-admin-security-channels.html

ochromy · 2025-07-11T08:30:39Z

QE review request here.

maschmid · 2025-07-15T16:58:27Z

modules/serverless-kafka-broker-sasl-default-config.adoc

@@ -32,14 +32,14 @@ $ oc create secret -n knative-eventing generic <secret_name> \
  --from-literal=user="my-sasl-user"
 ----
 ** Use the key names `ca.crt`, `password`, and `sasl.mechanism`. Do not change them.


(all of the key names must be such, so protocol, and user should be also listed here ) ( I suppose the original reason for this note is the ca.crt , as this is not obvious that it needs to be literally that, but since we list other, we should probably list all of them?)

maschmid · 2025-07-15T17:12:44Z

modules/serverless-kafka-broker-sasl-default-config.adoc

@@ -32,14 +32,14 @@ $ oc create secret -n knative-eventing generic <secret_name> \
  --from-literal=user="my-sasl-user"
 ----
 ** Use the key names `ca.crt`, `password`, and `sasl.mechanism`. Do not change them.
-** If you want to use SASL with public CA certificates, you must use the `tls.enabled=true` flag, rather than the `ca.crt` argument, when creating the secret. For example:
+** If you want to use SASL with public CA certificates, you must use the protocol="SASL_SSL" flag, rather than the ca.crt argument, when creating the secret. For example:


I don't think this makes sense as is... It did make sense with the original format, as not listing the "ca.crt" would not work if you also didn't explicitly enable TLS with tls.enabled=true .. However, with the new format, the protocol is required to be specified either way, and if it is set to protocol=SASL_SSL it means SSL is enabled. (you can also optionally use protocol=SASL_PLAIN to use SASL over plaintext, without using TLS )

(the only difference between "with public CA certificates" now is that the "ca.crt" argument is not specified.

We could perhaps simplify the whole section by having just a single example with "ca.crt", and having a note that would say "ca.crt" is optional, if the Kafka cluster is using a public CA certificate (verified by one of the default certificate authorities present in the default system truststores in the knative kafka images, if we wanted to be pedantic)

(same comment applies to the sasl-channels, as they now use the same format)

I see, thanks for the explanation and suggestion. I'll update this.

@maschmid Updated and pushed.
Please let me know if this matches what you suggested.

looks good to me!

maschmid · 2025-07-17T17:46:48Z

QE approved

ochromy · 2025-07-18T07:36:09Z

/label peer-review-needed

rh-tokeefe · 2025-07-18T12:34:15Z

/label peer-review-in-progress

rh-tokeefe

LGTM

rh-tokeefe · 2025-07-18T12:38:54Z

modules/serverless-kafka-broker-sasl-default-config.adoc

-  --from-literal=saslType="SCRAM-SHA-512" \
-  --from-literal=user="my-sasl-user"
----
+[NOTE]


Kafka is a product name, so you might want to consider creating an attribute for Kafka.

rh-tokeefe · 2025-07-18T12:41:39Z

/label peer-review-done

rh-tokeefe · 2025-07-18T12:42:13Z

/remove-label peer-review-needed
/remove-label peer-review-in-progress

ochromy · 2025-07-19T06:00:02Z

/label merge-review-needed

briandooley · 2025-07-21T12:14:02Z

/remove-label merge-review-needed
/label merge-review-in-progress

briandooley

Made a few small suggestions.

briandooley · 2025-07-21T12:43:48Z

modules/serverless-kafka-broker-sasl-default-config.adoc

----
+[NOTE]
+====
+The `ca.crt` key is optional if the Kafka cluster uses a certificate signed by a public CA that is trusted by the system truststore.


Suggested change

The `ca.crt` key is optional if the Kafka cluster uses a certificate signed by a public CA that is trusted by the system truststore.

The `ca.crt` key is optional if the Kafka cluster uses a certificate signed by a public CA whose certificate is already in the system truststore.

briandooley · 2025-07-21T12:44:24Z

modules/serverless-kafka-sasl-channels.adoc

----
+[NOTE]
+====
+The `ca.crt` key is optional if the Kafka cluster uses a certificate signed by a public CA that is trusted by the system truststore.


Suggested change

The `ca.crt` key is optional if the Kafka cluster uses a certificate signed by a public CA that is trusted by the system truststore.

The `ca.crt` key is optional if the Kafka cluster uses a certificate signed by a public CA whose certificate is already in the system truststore.

qe feedback no1 mr feedback no1

openshift-ci · 2025-07-21T14:22:36Z

@ochromy: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

ochromy · 2025-07-21T15:51:39Z

Good suggestions, @briandooley!
I've pushed the changes.

briandooley · 2025-07-21T16:01:14Z

/cherry-pick serverless-docs-1.35

openshift-cherrypick-robot · 2025-07-21T16:02:03Z

@briandooley: new pull request created: #96411

In response to this:

/cherry-pick serverless-docs-1.35

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

briandooley · 2025-07-21T16:02:12Z

/cherry-pick serverless-docs-1.36

openshift-cherrypick-robot · 2025-07-21T16:03:01Z

@briandooley: new pull request created: #96413

In response to this:

/cherry-pick serverless-docs-1.36

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

briandooley · 2025-07-23T11:41:14Z

/cherry-pick serverless-docs-1.37

openshift-cherrypick-robot · 2025-07-23T11:42:03Z

@briandooley: new pull request created: #96524

In response to this:

/cherry-pick serverless-docs-1.37

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 1, 2025

maschmid reviewed Jul 15, 2025

View reviewed changes

ochromy force-pushed the SRVKE-1687 branch from 6328469 to 173d786 Compare July 16, 2025 14:32

openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 16, 2025

openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Jul 18, 2025

openshift-ci bot added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Jul 18, 2025

rh-tokeefe reviewed Jul 18, 2025

View reviewed changes

openshift-ci bot added the peer-review-done Signifies that the peer review team has reviewed this PR label Jul 18, 2025

openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Jul 19, 2025

ShaunaDiaz removed peer-review-in-progress Signifies that the peer review team is reviewing this PR peer-review-needed Signifies that the peer review team needs to review this PR labels Jul 21, 2025

openshift-ci bot added merge-review-in-progress Signifies that the merge review team is reviewing this PR and removed merge-review-needed Signifies that the merge review team needs to review this PR labels Jul 21, 2025

briandooley requested changes Jul 21, 2025

View reviewed changes

[SRVKE-1687] Unify KafkaChannel and KafkaBroker secret format

6bf9067

qe feedback no1 mr feedback no1

ochromy force-pushed the SRVKE-1687 branch from 173d786 to 6bf9067 Compare July 21, 2025 14:01

briandooley merged commit f745eda into openshift:serverless-docs-main Jul 21, 2025
2 checks passed

openshift-cherrypick-robot mentioned this pull request Jul 21, 2025

[serverless-docs-1.35] [SRVKE-1687] Unify KafkaChannel and KafkaBroker secret format #96411

Merged

openshift-cherrypick-robot mentioned this pull request Jul 21, 2025

[serverless-docs-1.36] [SRVKE-1687] Unify KafkaChannel and KafkaBroker secret format #96413

Merged

openshift-cherrypick-robot mentioned this pull request Jul 23, 2025

[serverless-docs-1.37] [SRVKE-1687] Unify KafkaChannel and KafkaBroker secret format #96524

Merged

	The `ca.crt` key is optional if the Kafka cluster uses a certificate signed by a public CA that is trusted by the system truststore.
	The `ca.crt` key is optional if the Kafka cluster uses a certificate signed by a public CA whose certificate is already in the system truststore.

[SRVKE-1687] Unify KafkaChannel and KafkaBroker secret format #95471

[SRVKE-1687] Unify KafkaChannel and KafkaBroker secret format #95471

Uh oh!

Conversation

ochromy commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ocpdocs-previewbot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ochromy commented Jul 11, 2025

Uh oh!

maschmid Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

maschmid Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ochromy Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

ochromy Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

maschmid Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

maschmid commented Jul 17, 2025

Uh oh!

ochromy commented Jul 18, 2025

Uh oh!

rh-tokeefe commented Jul 18, 2025

Uh oh!

rh-tokeefe left a comment

Choose a reason for hiding this comment

Uh oh!

rh-tokeefe Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

rh-tokeefe commented Jul 18, 2025

Uh oh!

rh-tokeefe commented Jul 18, 2025

Uh oh!

ochromy commented Jul 19, 2025

Uh oh!

briandooley commented Jul 21, 2025

Uh oh!

briandooley left a comment

Choose a reason for hiding this comment

Uh oh!

briandooley Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

briandooley Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jul 21, 2025

Uh oh!

ochromy commented Jul 21, 2025

Uh oh!

Uh oh!

briandooley commented Jul 21, 2025

Uh oh!

openshift-cherrypick-robot commented Jul 21, 2025

Uh oh!

briandooley commented Jul 21, 2025

Uh oh!

openshift-cherrypick-robot commented Jul 21, 2025

Uh oh!

briandooley commented Jul 23, 2025

Uh oh!

openshift-cherrypick-robot commented Jul 23, 2025

Uh oh!

Uh oh!

ochromy commented Jul 1, 2025 •

edited

Loading

ocpdocs-previewbot commented Jul 1, 2025 •

edited

Loading

maschmid Jul 15, 2025 •

edited

Loading