affo commented on code in PR #2506: URL: https://github.com/apache/fluss/pull/2506#discussion_r2851652167
########## helm/templates/_sasl.tpl: ########## @@ -0,0 +1,155 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +{{/* +Return true if SASL is configured in any of the listener protocols +*/}} +{{- define "fluss.sasl.enabled" -}} +{{- $enabled := false -}} +{{- range $id, $l := .Values.listeners -}} + {{- if and (not $enabled) (regexFind "SASL" (upper $l.protocol)) -}} + {{- $enabled = true -}} + {{- end -}} +{{- end -}} +{{- if $enabled -}} +{{- true -}} +{{- end -}} +{{- end -}} + +{{/* +Return true if listener protocol is SASL +Usage: include "fluss.listener.sasl.enabled" (dict "root" . "listener" "internal") +*/}} +{{- define "fluss.listener.sasl.enabled" -}} +{{- $listener := index .root.Values.listeners .listener -}} +{{- if and $listener $listener.protocol (regexFind "SASL" (upper $listener.protocol)) -}} Review Comment: ```suggestion {{- if and $listener $listener.protocol (contains "SASL" (upper $listener.protocol)) -}} ``` I think contains would be good as well 🤔 ########## helm/templates/_sasl.tpl: ########## @@ -0,0 +1,155 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +{{/* +Return true if SASL is configured in any of the listener protocols +*/}} +{{- define "fluss.sasl.enabled" -}} +{{- $enabled := false -}} +{{- range $id, $l := .Values.listeners -}} + {{- if and (not $enabled) (regexFind "SASL" (upper $l.protocol)) -}} Review Comment: ```suggestion {{- if and (not $enabled) (contains "SASL" (upper $l.protocol)) -}} ``` ########## website/docs/install-deploy/deploying-with-helm.md: ########## @@ -245,16 +253,69 @@ The chart automatically configures listeners for internal cluster communication - **Internal Port (9123)**: Used for internal communication within the cluster - **Client Port (9124)**: Used for client connections -Custom listener configuration: +Default listeners configuration: ```yaml listeners: internal: + protocol: PLAINTEXT port: 9123 + security: + mechanism: PLAIN + users: [] client: + protocol: PLAINTEXT port: 9124 + security: + mechanism: PLAIN + users: [] ``` +To enable SASL based authentication, set any of the protocols to `SASL`. + +### Enabling Secure Connection + +With the helm deployment, you can specify authentication protocols when connecting to the Fluss cluster. + +The following table shows the supported protocols and security they provide: + +| Method | Authentication | TLS Encryption | +|-------------|:--------------:|:------------------:| +| `PLAINTEXT` | No | No | +| `SASL` | Yes | No | + +By default, the `PLAINTEXT` protocol is used. + +The SASL authentication will be enabled if any of the listener protocols is using `SASL`. + +Set these values for additional configurations: + +```yaml +listeners: Review Comment: Before putting the focus on punctual comments, I would stop one second and think about the design of this. I really like placing the security bit directly into the listener config, e.g.: `listeners.internal.security`. However, I don't think that this schema: ```yaml internal: protocol: SASL security: mechanism: PLAIN users: [] ``` Is actually the best way to proceed, given that different security protocols are going to require different keys to be provided. Just as an example, for `SASL/OAUTHBEARER`, users and password won't fit. Given that every supported protocol will probably have different inputs, why not: ```yaml internal: security: saslPlain: ssl: false users: [] saslScram: {} saslOAuthBearer: {} ``` So that every protocol can specify the required inputs without further validation? @morazow @loserwang1024 what do you think? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
