Repository: metal3-io/baremetal-operator
Pull request #1282 (2023-06-07 12:29:08)

:sparkles: Add TLS configuration flags for BMO
Added tls configuration flags for bmo.

This enables flags `--tls-min-version` and `--tls-max-version` flags, which can be used to limit TLS versions server accepts. Choices are `TLS12` and `TLS13`, so you can force TLS 1.2 only, TLS 1.3 only, or (default) TLS 1.2 and TLS 1.3 is negotiated between server and client.

This also disables TLS 1.0 and TLS 1.1 as they're insecure and obsolete and not supported by Go TLS library.

metal3-io-bot: Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with `/test all`
metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please assign andfasano for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["andfasano"]} -->
metal3-io-bot: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
as20203: /hold
as20203: /hold
lentzi90: /test-ubuntu-integration-main
/test-centos-e2e-integration-main
tuminoid: Building manager and running `./bin/baremetal-operator --help` says:

```
  -tls-max-version string
    	The maximum TLS version in use by the webhook server.
    	Possible values are VersionTLS10, VersionTLS11, VersionTLS12, VersionTLS13. (default "VersionTLS13")
  -tls-min-version string
    	The minimum TLS version in use by the webhook server.
    	Possible values are VersionTLS10, VersionTLS11, VersionTLS12, VersionTLS13. (default "VersionTLS12")
```

We are not supporting TLS 1.0 or TLS 1.1.
metal3-io-bot: New changes are detected. LGTM label has been removed.
tuminoid: A bit nit-picky, but the options are still `VersionTLS12` format. Why not just `TLS12`?
as20203: > A bit nit-picky, but the options are still `VersionTLS12` format. Why not just `TLS12`?

The reason to pass it like this is because this is how the function requires it. Below you can see we pass the version to the the funciton.

```
tlsMinVersion, err := cliflag.TLSVersion(options.TLSMinVersion)
	if err != nil {
		return nil, err
	}
```

Now inside this function there is a mapping of strings as below where you can see the keys 

```
var versions = map[string]uint16{
	"VersionTLS10": tls.VersionTLS10,
	"VersionTLS11": tls.VersionTLS11,
	"VersionTLS12": tls.VersionTLS12,
	"VersionTLS13": tls.VersionTLS13,
}
```
If we give only TLS12 it will error out in below function

```
func TLSVersion(versionName string) (uint16, error) {
	if len(versionName) == 0 {
		return DefaultTLSVersion(), nil
	}
	if version, ok := versions[versionName]; ok {
		return version, nil
	}
	return 0, fmt.Errorf("unknown tls version %q", versionName)
}
```

So in conclusion that's why we pass it like this.

tuminoid: > > A bit nit-picky, but the options are still `VersionTLS12` format. Why not just `TLS12`?
> 
> The reason to pass it like this is because this is how the function requires it. Below you can see we pass the version to the the funciton.
> ```
> 
> So in conclusion that's why we pass it like this.

IMO that is not acceptable reason to use such strings. For example, in the etcd PR which was used as an example how to add this feature, there is a mapping function https://github.com/etcd-io/etcd/pull/15156/files#diff-b4c1a72913d9878ddd102a03fc27e7b78404fc89b80d705832e56b541819c55a that creates user friendly version strings.
lentzi90: That makes sense, let's add a mapping function and simplify the strings.
/lgtm cancel
as20203: > > > A bit nit-picky, but the options are still `VersionTLS12` format. Why not just `TLS12`?
> > 
> > 
> > The reason to pass it like this is because this is how the function requires it. Below you can see we pass the version to the the funciton.
> > ```
> > 
> > So in conclusion that's why we pass it like this.
> > ```
> 
> IMO that is not acceptable reason to use such strings. For example, in the etcd PR which was used as an example how to add this feature, there is a mapping function https://github.com/etcd-io/etcd/pull/15156/files#diff-b4c1a72913d9878ddd102a03fc27e7b78404fc89b80d705832e56b541819c55a that creates user friendly version strings.

@lentzi90 @tuminoid just wanted to confirm as per the pr the string are TLS1.2 and TLS1.3 should I keep them like this or simply TLS12 and TLS13?
lentzi90: Either is fine for me as long as it is shown in the help text :slightly_smiling_face: 
metal3-io-bot: New changes are detected. LGTM label has been removed.
tuminoid: As discussed offline with @as20203, I also want to see a test run via test dev-env PR with these options applied to see TLS 1.3 only mode actually works as expected.
metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/1282#pullrequestreview-1488118802" title="LGTM">tuminoid</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign andfasano for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["andfasano"]} -->
tuminoid: /test-ubuntu-integration-main  
/test-centos-e2e-integration-main  
lentzi90: /test-ubuntu-integration-main
metal3-io-bot: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
metal3-io-bot: New changes are detected. LGTM label has been removed.
metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/1282#pullrequestreview-1488118802" title="LGTM">tuminoid</a>*
**Once this PR has been reviewed and has the lgtm label**, please ask for approval from dtantsur. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["dtantsur"]} -->
metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/1282#pullrequestreview-1488118802" title="LGTM">tuminoid</a>*
**Once this PR has been reviewed and has the lgtm label**, please ask for approval from dtantsur. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["dtantsur"]} -->
metal3-io-bot: @as20203: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
golint | 31e7d492b20e9978d5e6c2160f507b627d0a89b8 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1282/golint/1671798961234513920) | true | `/test golint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1282). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:as20203).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
tuminoid: Please rebase, linter should be fine now.
as20203: /test-centos-e2e-integration-main
/test-ubuntu-integration-main
as20203: @dtantsur @zaneb please review.
metal3-io-bot: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

Patch:
diff --git a/go.mod b/go.mod
index 49e70e25..e13939e5 100644
--- a/go.mod
+++ b/go.mod
@@ -8,6 +8,7 @@ require (
 	github.com/gophercloud/gophercloud v1.5.0
 	github.com/metal3-io/baremetal-operator/apis v0.2.0
 	github.com/metal3-io/baremetal-operator/pkg/hardwareutils v0.2.0
+	github.com/onsi/gomega v1.27.7
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.16.0
 	github.com/stretchr/testify v1.8.4
@@ -16,6 +17,8 @@ require (
 	k8s.io/api v0.27.2
 	k8s.io/apimachinery v0.27.2
 	k8s.io/client-go v0.27.2
+	k8s.io/component-base v0.27.2
+	k8s.io/klog/v2 v2.90.1
 	k8s.io/utils v0.0.0-20230209194617-a36077c30491
 	sigs.k8s.io/controller-runtime v0.15.0
 	sigs.k8s.io/yaml v1.3.0
@@ -41,6 +44,7 @@ require (
 	github.com/google/gofuzz v1.1.0 // indirect
 	github.com/google/uuid v1.3.0 // indirect
 	github.com/imdario/mergo v0.3.12 // indirect
+	github.com/inconshreveable/mousetrap v1.0.1 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
@@ -52,6 +56,7 @@ require (
 	github.com/prometheus/client_model v0.4.0 // indirect
 	github.com/prometheus/common v0.42.0 // indirect
 	github.com/prometheus/procfs v0.10.1 // indirect
+	github.com/spf13/cobra v1.6.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	go.uber.org/atomic v1.7.0 // indirect
 	go.uber.org/multierr v1.6.0 // indirect
@@ -69,8 +74,6 @@ require (
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/apiextensions-apiserver v0.27.2 // indirect
-	k8s.io/component-base v0.27.2 // indirect
-	k8s.io/klog/v2 v2.90.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
diff --git a/go.sum b/go.sum
index d6491117..fbd40fca 100644
--- a/go.sum
+++ b/go.sum
@@ -8,6 +8,7 @@ github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA
 github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
 github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
+github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
@@ -75,6 +76,8 @@ github.com/gophercloud/gophercloud v1.5.0 h1:cDN6XFCLKiiqvYpjQLq9AiM7RDRbIC9450W
 github.com/gophercloud/gophercloud v1.5.0/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM=
 github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
 github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
+github.com/inconshreveable/mousetrap v1.0.1 h1:U3uMjPSQEBMNp1lFxmllqCPM6P5u/Xq7Pgzkat/bFNc=
+github.com/inconshreveable/mousetrap v1.0.1/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
 github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
 github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
 github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
@@ -103,6 +106,7 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/onsi/ginkgo/v2 v2.9.5 h1:+6Hr4uxzP4XIUyAkg61dWBw8lb/gc4/X5luuxN/EC+Q=
 github.com/onsi/gomega v1.27.7 h1:fVih9JD6ogIiHUN6ePK7HJidyEDpWGVB5mzM7cWNXoU=
+github.com/onsi/gomega v1.27.7/go.mod h1:1p8OOlwo2iUUDsHnOrjE5UKYJ+e3W8eQ3qSlRahPmr4=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
@@ -118,6 +122,9 @@ github.com/prometheus/common v0.42.0/go.mod h1:xBwqVerjNdUDjgODMpudtOMwlOwf2SaTr
 github.com/prometheus/procfs v0.10.1 h1:kYK1Va/YMlutzCGazswoHKo//tZVlFpKYh+PymziUAg=
 github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM=
 github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ=
+github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
+github.com/spf13/cobra v1.6.0 h1:42a0n6jwCot1pUmomAp4T7DeMD+20LFv4Q54pxLf2LI=
+github.com/spf13/cobra v1.6.0/go.mod h1:IOw/AERYS7UzyrGinqmz6HLUo219MORXGxhbaJUqzrY=
 github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
diff --git a/main.go b/main.go
index cc27bb86..6a1cb387 100644
--- a/main.go
+++ b/main.go
@@ -21,11 +21,13 @@ import (
 	"fmt"
 	"os"
 	"runtime"
+	"strings"
 
 	"go.uber.org/zap/zapcore"
 	k8sruntime "k8s.io/apimachinery/pkg/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
+	cliflag "k8s.io/component-base/cli/flag"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
@@ -44,10 +46,26 @@ import (
 	// +kubebuilder:scaffold:imports
 )
 
+type TLSVersion string
+
+// Constants for TLS versions.
+const (
+	TLSVersion12 TLSVersion = "TLS12"
+	TLSVersion13 TLSVersion = "TLS13"
+)
+
+type TLSOptions struct {
+	TLSMaxVersion   string
+	TLSMinVersion   string
+	TLSCipherSuites string
+}
+
 var (
-	scheme     = k8sruntime.NewScheme()
-	setupLog   = ctrl.Log.WithName("setup")
-	healthAddr string
+	scheme               = k8sruntime.NewScheme()
+	setupLog             = ctrl.Log.WithName("setup")
+	healthAddr           string
+	tlsOptions           = TLSOptions{}
+	tlsSupportedVersions = []string{"TLS12", "TLS13"}
 )
 
 const leaderElectionID = "baremetal-operator"
@@ -132,6 +150,22 @@ func main() {
 		"Maximum queries per second from the controller client to the Kubernetes API server. Default 20")
 	flag.IntVar(&restConfigBurst, "kube-api-burst", 30,
 		"Maximum number of queries that should be allowed in one burst from the controller client to the Kubernetes API server. Default 30")
+	flag.StringVar(&tlsOptions.TLSMinVersion, "tls-min-version", "TLS12",
+		"The minimum TLS version in use by the webhook server.\n"+
+			fmt.Sprintf("Possible values are %s.", strings.Join(tlsSupportedVersions, ", ")),
+	)
+	flag.StringVar(&tlsOptions.TLSMaxVersion, "tls-max-version", "TLS13",
+		"The maximum TLS version in use by the webhook server.\n"+
+			fmt.Sprintf("Possible values are %s.", strings.Join(tlsSupportedVersions, ", ")),
+	)
+
+	tlsCipherPreferredValues := cliflag.PreferredTLSCipherNames()
+	tlsCipherInsecureValues := cliflag.InsecureTLSCipherNames()
+	flag.StringVar(&tlsOptions.TLSCipherSuites, "tls-cipher-suites", "",
+		"Comma-separated list of cipher suites for the webhook server. "+
+			"If omitted, the default Go cipher suites will be used. \n"+
+			"Preferred values: "+strings.Join(tlsCipherPreferredValues, ", ")+". \n"+
+			"Insecure values: "+strings.Join(tlsCipherInsecureValues, ", ")+".")
 	flag.Parse()
 
 	logOpts := zap.Options{}
@@ -149,7 +183,11 @@ func main() {
 	if leaderElectionNamespace == "" {
 		leaderElectionNamespace = watchNamespace
 	}
-
+	tlsOptionOverrides, err := GetTLSOptionOverrideFuncs(tlsOptions)
+	if err != nil {
+		setupLog.Error(err, "unable to add TLS settings to the webhook server")
+		os.Exit(1)
+	}
 	restConfig := ctrl.GetConfigOrDie()
 	restConfig.QPS = float32(restConfigQPS)
 	restConfig.Burst = restConfigBurst
@@ -158,14 +196,8 @@ func main() {
 		Scheme:             scheme,
 		MetricsBindAddress: metricsBindAddr,
 		WebhookServer: webhook.NewServer(webhook.Options{
-			Port: webhookPort,
-			TLSOpts: []func(*tls.Config){
-				func(c *tls.Config) {
-					if c.MinVersion < tls.VersionTLS12 {
-						c.MinVersion = tls.VersionTLS12
-					}
-				},
-			},
+			Port:    webhookPort,
+			TLSOpts: tlsOptionOverrides,
 		}),
 		LeaderElection:          enableLeaderElection,
 		LeaderElectionID:        leaderElectionID,
@@ -254,3 +286,76 @@ func main() {
 		os.Exit(1)
 	}
 }
+
+// GetTLSOptionOverrideFuncs returns a list of TLS configuration overrides to be used
+// by the webhook server.
+func GetTLSOptionOverrideFuncs(options TLSOptions) ([]func(*tls.Config), error) {
+	var tlsOptions []func(config *tls.Config)
+
+	tlsMinVersion, err := GetTLSVersion(options.TLSMinVersion)
+	if err != nil {
+		return nil, err
+	}
+
+	tlsMaxVersion, err := GetTLSVersion(options.TLSMaxVersion)
+	if err != nil {
+		return nil, err
+	}
+
+	if tlsMaxVersion != 0 && tlsMinVersion > tlsMaxVersion {
+		return nil, fmt.Errorf("TLS version flag min version (%s) is greater than max version (%s)",
+			options.TLSMinVersion, options.TLSMaxVersion)
+	}
+
+	tlsOptions = append(tlsOptions, func(cfg *tls.Config) {
+		cfg.MinVersion = tlsMinVersion
+	})
+
+	tlsOptions = append(tlsOptions, func(cfg *tls.Config) {
+		cfg.MaxVersion = tlsMaxVersion
+	})
+	// Cipher suites should not be set if empty.
+	if options.TLSMinVersion == string(TLSVersion13) &&
+		options.TLSMaxVersion == string(TLSVersion13) &&
+		options.TLSCipherSuites != "" {
+		setupLog.Info("warning: Cipher suites should not be set for TLS version 1.3. Ignoring ciphers")
+		options.TLSCipherSuites = ""
+	}
+
+	if options.TLSCipherSuites != "" {
+		tlsCipherSuites := strings.Split(options.TLSCipherSuites, ",")
+		suites, err := cliflag.TLSCipherSuites(tlsCipherSuites)
+		if err != nil {
+			return nil, err
+		}
+
+		insecureCipherValues := cliflag.InsecureTLSCipherNames()
+		for _, cipher := range tlsCipherSuites {
+			for _, insecureCipherName := range insecureCipherValues {
+				if insecureCipherName == cipher {
+					setupLog.Info(fmt.Sprintf("warning: use of insecure cipher '%s' detected.", cipher))
+				}
+			}
+		}
+		tlsOptions = append(tlsOptions, func(cfg *tls.Config) {
+			cfg.CipherSuites = suites
+		})
+	}
+
+	return tlsOptions, nil
+}
+
+// GetTLSVersion returns the corresponding tls.Version or error.
+func GetTLSVersion(version string) (uint16, error) {
+	var v uint16
+
+	switch version {
+	case string(TLSVersion12):
+		v = tls.VersionTLS12
+	case string(TLSVersion13):
+		v = tls.VersionTLS13
+	default:
+		return 0, fmt.Errorf("unexpected TLS version %q (must be one of: TLS12, TLS13)", version)
+	}
+	return v, nil
+}


Test patch:
diff --git a/main_test.go b/main_test.go
new file mode 100644
index 00000000..f3a11311
--- /dev/null
+++ b/main_test.go
@@ -0,0 +1,111 @@
+/*
+Copyright 2023 The Metal3 Authors.
+
+Licensed 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.
+*/
+
+package main
+
+import (
+	"bytes"
+	"testing"
+
+	. "github.com/onsi/gomega"
+	"k8s.io/klog/v2"
+	ctrl "sigs.k8s.io/controller-runtime"
+)
+
+func TestTLSInsecureCiperSuite(t *testing.T) {
+	t.Run("test insecure cipher suite passed as TLS flag", func(t *testing.T) {
+		g := NewWithT(t)
+		tlsMockOptions := TLSOptions{
+			TLSMaxVersion:   "TLS13",
+			TLSMinVersion:   "TLS12",
+			TLSCipherSuites: "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
+		}
+		ctrl.Log.WithName("setup")
+		ctrl.SetLogger(klog.Background())
+
+		bufWriter := bytes.NewBuffer(nil)
+		klog.SetOutput(bufWriter)
+		klog.LogToStderr(false) // this is important, because klog by default logs to stderr only
+		_, err := GetTLSOptionOverrideFuncs(tlsMockOptions)
+		g.Expect(err).Should(BeNil())
+		g.Expect(bufWriter.String()).Should(ContainSubstring("use of insecure cipher 'TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256' detected."))
+	})
+}
+
+func TestTLSMinAndMaxVersion(t *testing.T) {
+	t.Run("should fail if TLS min version is greater than max version.", func(t *testing.T) {
+		g := NewWithT(t)
+		tlsMockOptions := TLSOptions{
+			TLSMaxVersion: "TLS12",
+			TLSMinVersion: "TLS13",
+		}
+		_, err := GetTLSOptionOverrideFuncs(tlsMockOptions)
+		g.Expect(err.Error()).To(Equal("TLS version flag min version (TLS13) is greater than max version (TLS12)"))
+	})
+}
+
+func Test13CipherSuite(t *testing.T) {
+	t.Run("should reset ciphersuite flag if TLS min and max version are set to 1.3", func(t *testing.T) {
+		g := NewWithT(t)
+
+		// Here TLS_RSA_WITH_AES_128_CBC_SHA is a tls12 cipher suite.
+		tlsMockOptions := TLSOptions{
+			TLSMaxVersion:   "TLS13",
+			TLSMinVersion:   "TLS13",
+			TLSCipherSuites: "TLS_RSA_WITH_AES_128_CBC_SHA,TLS_AES_256_GCM_SHA384",
+		}
+
+		ctrl.Log.WithName("setup")
+		ctrl.SetLogger(klog.Background())
+
+		bufWriter := bytes.NewBuffer(nil)
+		klog.SetOutput(bufWriter)
+		klog.LogToStderr(false) // this is important, because klog by default logs to stderr only
+		_, err := GetTLSOptionOverrideFuncs(tlsMockOptions)
+		g.Expect(bufWriter.String()).Should(ContainSubstring("warning: Cipher suites should not be set for TLS version 1.3. Ignoring ciphers"))
+		g.Expect(err).Should(BeNil())
+	})
+}
+
+func TestGetTLSVersion(t *testing.T) {
+	t.Run("should error out when incorrect tls version passed", func(t *testing.T) {
+		g := NewWithT(t)
+		tlsVersion := "TLS11"
+		_, err := GetTLSVersion(tlsVersion)
+		g.Expect(err.Error()).Should(Equal("unexpected TLS version \"TLS11\" (must be one of: TLS12, TLS13)"))
+	})
+	t.Run("should pass and output correct tls version", func(t *testing.T) {
+		const VersionTLS12 uint16 = 771
+		g := NewWithT(t)
+		tlsVersion := "TLS12"
+		version, err := GetTLSVersion(tlsVersion)
+		g.Expect(version).To(Equal(VersionTLS12))
+		g.Expect(err).Should(BeNil())
+	})
+}
+
+func TestTLSOptions(t *testing.T) {
+	t.Run("should pass with all the correct options below with no error.", func(t *testing.T) {
+		g := NewWithT(t)
+		tlsMockOptions := TLSOptions{
+			TLSMinVersion:   "TLS12",
+			TLSMaxVersion:   "TLS13",
+			TLSCipherSuites: "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+		}
+		_, err := GetTLSOptionOverrideFuncs(tlsMockOptions)
+		g.Expect(err).Should(BeNil())
+	})
+}