feat: Implement tls checker for webhook

[ethernoy]

Signed-off-by: Ethern Su ehaprime@gmail.com

What this PR does / why we need it:

This PR is a purposed solution of #1684, which adds a TLS checker over healthz probe in webhook server to detect the case where file in certsDir is not correctly served in webhook server (it can actually be reproduced easily)

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #1684

Special notes for your reviewer:

I am submitting this draft PR because I want to have a working example of the solution I suggested, so that we can proceed to discuss whether this solution is preferred and make changes if approriate. If this solution is preferred, I will ready this PR with more documentation in websites and code comments.

Also I am new to this project, please let me know if there is any implementation/formatting issue in my code :)

[maxsmythe]

Thanks for the PR! It's marked as a draft, is that still the case?

[ethernoy]

Thanks for the PR! It's marked as a draft, is that still the case?

@maxsmythe Hi! I mark this as draft because I hope a reviewer can help to check if this feature is good to go. If it is, I will add the relevant documentation and remove the draft label.

[ethernoy]

I have implemented the tlsChecker that is attached in liveness probe when enabled. It compares the certificate served by webhook server and the certificate saved in certDir, and returns error when mismatched is found during a liveness probe check.

Attached is the log of a failed check. Kubelet is about to kill this pod, after this, the secret should resume normal:

[ethernoy]

@maxsmythe Hi, do you mind take a look? :)

[maxsmythe]

Sorry I dropped the ball on reviewing this. LGTM. Thank you for flag-gating it!

[codecov-commenter]

Codecov Report

Merging #1696 (37e8d88) into master (48eab16) will decrease coverage by 0.39%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1696      +/-   ##
==========================================
- Coverage   52.14%   51.74%   -0.40%     
==========================================
  Files          98       99       +1     
  Lines        8781     8816      +35     
==========================================
- Hits         4579     4562      -17     
- Misses       3837     3887      +50     
- Partials      365      367       +2     

Flag	Coverage Δ
unittests	51.74% <0.00%> (-0.40%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/webhook/health_check.go	0.00% <0.00%> (ø)
...onstrainttemplate/constrainttemplate_controller.go	53.36% <0.00%> (-5.77%)	⬇️
pkg/readiness/ready_tracker.go	70.33% <0.00%> (+0.50%)	⬆️
pkg/watch/replay.go	81.25% <0.00%> (+2.27%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ecf4538...37e8d88. Read the comment docs.

[ethernoy]

The case fails as this healthcheck probe needs use an insecure http client to get the certificates served in webhook no matter what CA it is using. I believe this is not an issue.

[maxsmythe]

Can you set a skip directive for the linter for that line.

I agree the insecure TLS is not an issue here.

[ethernoy]

@maxsmythe

skip directive added and tested locally on my machine.

[ritazh]

Thanks for the PR @ethernoy! Approved pending #1696 (comment) and make manifests

[sozercan]

Thank you! LGTM

[sozercan]

@ethernoy looks like there is a diff here when running make manifests. https://github.com/open-policy-agent/gatekeeper/tree/master/cmd/build/helmify/static needs to be updated with the correct values. after running make manifests there shouldn't be any diff

[ethernoy]

@ethernoy looks like there is a diff here when running make manifests. https://github.com/open-policy-agent/gatekeeper/tree/master/cmd/build/helmify/static needs to be updated with the correct values. after running make manifests there shouldn't be any diff

@sozercan updated helmify static manifests.

[arvisha16]

Hello team,

When can we merge the fix into a release?
We need fix, please plan and let me know, thanks so much!

[ethernoy]

Hello team,

When can we merge the fix into a release? We need fix, please plan and let me know, thanks so much!

Hi, this fix should already be in release 3.8.0

[arvisha16]

Hello @ethernoy ethernoy

Thanks so much for the quick reply, we will use 3.8.0

Cheers,
Arvind