Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Two Cert Manager functions for cdi-apiserver #3620

Open
cfilleke opened this issue Jan 29, 2025 · 0 comments
Open

Two Cert Manager functions for cdi-apiserver #3620

cfilleke opened this issue Jan 29, 2025 · 0 comments

Comments

@cfilleke
Copy link
Member

cfilleke commented Jan 29, 2025

There are 2 source files named certwatcher.go that perform essentially the same function; one is dated 2019, one dated 2021; one included as a package and apparently not updated, and one included via the vendored official stream that looks like it could also be better maintained in the CDI build as well.

 % find . -name certwatcher.go
./vendor/sigs.k8s.io/controller-runtime/pkg/certwatcher/certwatcher.go
./pkg/util/cert/watcher/certwatcher.go

and

 % diff `!!`
diff `find . -name certwatcher.go`
2c2
< Copyright 2021 The Kubernetes Authors.
---
> Copyright 2019 The Kubernetes Authors.
17c17
< package certwatcher
---
> package watcher
...

The ./pkg/util/cert/watcher/certwatcher.go is the one that is observably used in the cdi-apiserver runtime, as it emits the log messages Updated current TLS certificate and Starting certificate watcher from lines 126 and 82 respectively:

[root@dell-r730-029 ~]# oc logs cdi-apiserver-87777ffdc-2zq67 -n openshift-cnv
I0124 09:49:28.519405       1 apiserver.go:92] Note: increase the -v level in the api deployment for more detailed logging, eg. -v=2 or -v=3
W0124 09:49:28.519833       1 client_config.go:659] Neither --kubeconfig nor --master was specified.  Using the inClusterConfig.  This might not work.
I0124 09:49:28.752647       1 certwatcher.go:126] Updated current TLS certificate
I0124 09:49:28.757698       1 certwatcher.go:82] Starting certificate watcher
2025/01/24 09:49:30 http: TLS handshake error from 10.128.0.2:51846: remote error: tls: bad certificate
...

While this particular cert error was due to the installation of two versions of CDI in the same cluster and unrelated to the code duplication, the code duplication led to some confusion on my part diagnosing the cert issues, and there should probably be only one piece of code in the build tree to perform this function.

Given that this is in the area of cert management, it's likely using the more up-to-date vendored code, and maintaining it, would address current and future security issues.

/wg code-quality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant