Consistent minTLSVersion in IngressController CRD #1679
Conversation
The minTLSVersion sample values proposed in this CRD's documentation are leading to an error because the allowed enum values differ from the syntax of the config samples. Symptoms observed on the operator upon applying an IngressController with the suggested values were: 'error: ingresscontrollers.operator.openshift.io "" is invalid' see also openshift/cluster-ingress-operator#994 Fixed the documentation for minTLSVersion values: $ find . -not -path "./.git/*" -type f -exec grep -Iq . {} \; -print0 | xargs -I {} -0 sed -Ei 's/minTLSVersion: TLSv1.([0-3])/minTLSVersion: VersionTLS1\1/g' {} Replaced plural form for tls versions in the documentation: $ find . -not -path "./.git/*" -type f -exec grep -Iq . {} \; -print0 | xargs -I {} -0 sed -i 's/MinTLSVersions is/minTLSVersion is/g' {} Did not replace "MinTLSVersion is the minimum TLS version supported" which uses an uppercase "M", as all other options are also starting with an uppercase letter. Not sure whether it's correct to also update the generated files - let me know if I should remove changes that are not necessary. Documentation reference for the TLS version naming: https://pkg.go.dev/crypto/tls#pkg-constants
openshift-ci
bot
added
the
size/M
|
Hello @martin-aders! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
needs-ok-to-test
|
Hi @martin-aders. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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/test-infra repository. |
|
/ok-to-test |
openshift-ci
bot
added
ok-to-test
|
This is just a docs change as far as I can tell, so seems correct to me, any objections @candita ? |
I agree, and no objections, @JoelSpeed. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: candita, JoelSpeed, martin-aders The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@martin-aders: 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/test-infra repository. I understand the commands that are listed here. |
|
[ART PR BUILD NOTIFIER] This PR has been included in build ose-cluster-config-api-container-v4.15.0-202311281631.p0.ge1845c5.assembly.stream for distgit ose-cluster-config-api. |
Bring in changes introduced by openshift/api#1679
Bring in changes introduced by openshift/api#1679
The minTLSVersion sample values proposed in this CRD's documentation are leading to an error because the allowed enum values differ from the syntax of the config samples.
Symptoms observed on the operator upon applying an IngressController with the suggested values were: 'error: ingresscontrollers.operator.openshift.io "" is invalid' see also openshift/cluster-ingress-operator#994
Thanks to @candita for reviewing the initial PR and guiding me to this repository.
Fixed the documentation for minTLSVersion values:
Replaced plural form for tls versions in the documentation:
Did not replace "MinTLSVersion is the minimum TLS version supported" which uses an uppercase "M", as all other options are also starting with an uppercase letter.
Not sure whether it's correct to also update the generated files - let me know if I should remove changes that are not necessary.
Documentation reference for the TLS version naming: https://pkg.go.dev/crypto/tls#pkg-constants