Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix documentation for ClientSSLCertSecret and ServerSSLCertSecret #840

Merged
merged 3 commits into from
Aug 2, 2022

Conversation

bartam1
Copy link
Contributor

@bartam1 bartam1 commented Jul 28, 2022

Q A
Bug fix? yes
New feature? no
API breaks? no
Deprecations? no
Related tickets fixes #839
License Apache 2.0

What's in this PR?

It fixes the documentation for the ClientSSLCertSecret and ServerSSLCertSecret.

Why?

The secrets which are referenced by the ClientSSLCertSecret and ServerSSLCertSecret need to contain other fields than the keystore.jks, truststore.jks and password when they are used for inner broker or controller communication

@bartam1 bartam1 requested a review from a team as a code owner July 28, 2022 09:41
panyuenlau
panyuenlau previously approved these changes Jul 28, 2022
Copy link
Member

@panyuenlau panyuenlau left a comment

Choose a reason for hiding this comment

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

Not sure if these are the better way of rephrasing them

// The secret must contains the keystore, truststore jks files and the password for them in base64 encoded format
// under the keystore.jks, truststore.jks, password data fields.
// The secret must contain the keystore, truststore jks files and the password for them in base64 encoded format and
// the tls certificate, tls private key, CA certificate in PEM and base64 encoded format
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// the tls certificate, tls private key, CA certificate in PEM and base64 encoded format
// the tls certificate, tls private key, CA certificate in PEM format with base64 encoding

@@ -510,7 +511,7 @@ type CommonListenerSpec struct {
// +kubebuilder:validation:Enum=ssl;plaintext;sasl_ssl;sasl_plaintext
Type SecurityProtocol `json:"type"`
// ServerSSLCertSecret is a reference to the Kubernetes secret that contains the server certificate for the listener to be used for SSL communication.
// The secret must contain the keystore, truststore jks files and the password for them in base64 encoded format
// The secret must contain the keystore, truststore jks files and the password for them in base64 encoded format and the tls certificate in PEM and base64 encoded format
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// The secret must contain the keystore, truststore jks files and the password for them in base64 encoded format and the tls certificate in PEM and base64 encoded format
// The secret must contain the keystore, truststore jks files and the password for them in base64 encoded format and the tls certificate in PEM format with base64 encoding

pregnor
pregnor previously approved these changes Jul 28, 2022
Copy link
Member

@pregnor pregnor left a comment

Choose a reason for hiding this comment

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

LGTM aside from Darren's comments

Copy link
Member

@stoader stoader left a comment

Choose a reason for hiding this comment

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

Run make manifests to ensure that all artifacts are updated

@bartam1 bartam1 dismissed stale reviews from panyuenlau and pregnor via 36415f0 July 28, 2022 10:28
Copy link
Member

@pregnor pregnor left a comment

Choose a reason for hiding this comment

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

LGTM

@panyuenlau
Copy link
Member

@bartam1 Can we merge this?

@bartam1 bartam1 merged commit fcd3d02 into master Aug 2, 2022
@bartam1 bartam1 deleted the fixdocssslcert branch August 2, 2022 08:59
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.

serverSSLCertSecret: no certificates found
4 participants