Dennis-Mircea commented on code in PR #1118:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/1118#discussion_r3302035466
##########
docs/content/docs/operations/helm.md:
##########
@@ -128,6 +128,7 @@ The configurable parameters of the Helm chart and which
default values as detail
| watchNamespaces | List of kubernetes
namespaces to watch for FlinkDeployment changes, empty means all namespaces.
|
|
| webhook.create | Whether to enable
validating and mutating webhooks for flink-kubernetes-operator.
| true
|
| webhook.keystore | The ConfigMap of webhook
key store.
| useDefaultPassword:
true
|
+| webhook.keystore.pkcs12Profile | PKCS12 encryption profile
for the webhook certificate. Options: `Modern2023`, `LegacyDES`, `LegacyRC2`.
Use `Modern2023` for FIPS-compliant environments. |
|
Review Comment:
Please format the table accordingly and sync the chinese docs as well.
##########
helm/flink-kubernetes-operator/templates/cert-manager/certificate.yaml:
##########
@@ -29,6 +29,9 @@ spec:
keystores:
pkcs12:
create: true
+ {{- if .Values.webhook.keystore.pkcs12Profile }}
+ profile: {{ .Values.webhook.keystore.pkcs12Profile }}
Review Comment:
`profile: {{ .Values.webhook.keystore.pkcs12Profile }}` is unquoted. The
three valid values are safe identifiers, but `| quote` is idiomatic for
string-typed values pulled from user-supplied values and costs nothing. It also
guards against future values that start with a digit, contain a colon, or are
otherwise YAML-sensitive.
````suggestion
profile: {{ .Values.webhook.keystore.pkcs12Profile | quote }}
````
Also, please add unit test support for this change. The
`helm/flink-kubernetes-operator/tests/cert-manager/certificate_test.yaml` is
the established place for this. Two assertions would lock the behavior:
- With `pkcs12Profile` unset, `spec.keystores.pkcs12.profile` is absent
(default unchanged).
- With `pkcs12Profile: Modern2023`, `spec.keystores.pkcs12.profile ==
Modern2023`.
This is worth doing because the whole point of the PR is that the default
render stays identical, which is exactly what a test should prove.
--
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]
