Introduce min-tls and ciphers-suite application options for webhook server

[arsenalzp]

This PR is introducing Minimum TLS version and Ciphers Suite feature which were requested in issue #528.

[cert-manager-prow]

[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 inteon for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[cert-manager-prow]

Hi @arsenalzp. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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-sigs/prow repository.

[erikgb]

@arsenalzp, thanks for working on this. Could you take a look at how this is implemented in cert-manager? We would prefer a similar construct, at least the same defaults, descriptions etc. And the PR also needs to be rebased. 😺

[arsenalzp]

@arsenalzp, thanks for working on this. Could you take a look at how this is implemented in cert-manager? We would prefer a similar construct, at least the same defaults, descriptions etc. And the PR also needs to be rebased. 😺

I did as requested by you.

[arsenalzp]

/ok-to-test

[cert-manager-prow]

@arsenalzp: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/ok-to-test

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-sigs/prow repository.

[erikgb]

/ok-to-test

[erikgb]

Try running make fix-golangci-lint, @arsenalzp.

[arsenalzp]

Try running make fix-golangci-lint, @arsenalzp.

Thank you!
I tried to eliminate linter errors brought by my code, however there are number of errors in util_test.go, certificates_test.go and types_test.go files.

[arsenalzp]

Colleagues, any decision on this PR? Can we merge it?

[SgtCoDFish]

Few comments, what do you think? Thanks for this!

[SgtCoDFish]

suggestion: the defer will never get called if we return here; the order should be different. Does that sound right to you?

Also since this doesn't call t.Error this test won't ever fail.

Suggested change

	if err != nil {
	return
	}
	defer resp.Body.Close()
	defer resp.Body.Close()
	if err == nil {
	t.Error("expected an error from talking to a server with an unsupported cipher suite but got none")
	return
	}

[arsenalzp]

Hello,
Yes, looks good! Thank you!

[SgtCoDFish]

suggestion: If we call Fatalf the Body will never be closed - as far as I know at least!

Suggested change

	resp, err := client.Get(fmt.Sprintf("https://%s", net.JoinHostPort("localhost", "6443"))) //nolint: noctx
	if err != nil {
	t.Fatalf("error to connect to webhook server, %s\n", err)
	}
	resp.Body.Close()
	resp, err := client.Get(fmt.Sprintf("https://%s", net.JoinHostPort("localhost", "6443"))) //nolint: noctx
	defer resp.Body.Close()
	if err != nil {
	t.Fatalf("error to connect to webhook server, %s\n", err)
	}

[SgtCoDFish]

suggestion: Do we need to listen on 0.0.0.0 here? Could we listen on 127.0.0.1 to make it a little safer?