redhat-cop / cert-utils-operator

Set of functionalities around certificates packaged in a Kubernetes operator
Apache License 2.0
94 stars 35 forks source link

truststore.jks in ConfigMaps updated on every pod restart #142

Closed vinzent closed 1 year ago

vinzent commented 1 year ago

We have ConfigMaps with Certificates that we convert to JKS with the help of cert-utils-operator (cert-utils-operator.redhat-cop.io/generate-java-truststore annotation, currently using v1.3.9). We also have Stakater Reloader that restart our apps as truststores change.

We have discovered that when Cert-Utils-Operator restarts all our ConfigMaps are updated. And then Stakater Reloader kicks in and restarts apps. Even nothing has changed with certs.

This is probably due to the non-idempotent creation of the JKS which involves time.Now():

https://github.com/redhat-cop/cert-utils-operator/blob/master/controllers/configmaptokeystore/configmap_to_keystore_controller.go#L136-L147

Probably this is a required field. :-|

raffaelespazzoli commented 1 year ago

I think your assessment is correct. not sure what to do to prevent this. Asking internally...

vinzent commented 1 year ago

maybe the CreationTimestamp of the ConfigMap could be used?

diff --git a/Dockerfile b/Dockerfile
index 2c1b7ca..4a107d5 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,5 +1,5 @@
 # Build the manager binary
-FROM golang:1.16 as builder
+FROM docker.io/library/golang:1.19 as builder

 WORKDIR /workspace
 # Copy the Go Modules manifests
diff --git a/controllers/configmaptokeystore/configmap_to_keystore_controller.go b/controllers/configmaptokeystore/configmap_to_keystore_controller.go
index 97c2233..a1ecb13 100644
--- a/controllers/configmaptokeystore/configmap_to_keystore_controller.go
+++ b/controllers/configmaptokeystore/configmap_to_keystore_controller.go
@@ -7,7 +7,6 @@ import (
        "errors"
        "reflect"
        "strconv"
-       "time"

        "github.com/go-logr/logr"
        keystore "github.com/pavel-v-chernykh/keystore-go"
@@ -136,7 +135,7 @@ func (r *ConfigMapToKeystoreReconciler) getTrustStoreFromConfigMap(configMap *co
        for p, rest := pem.Decode([]byte(ca)); p != nil; p, rest = pem.Decode(rest) {
                keyStore["alias"+strconv.Itoa(i)] = &keystore.TrustedCertificateEntry{
                        Entry: keystore.Entry{
-                               CreationDate: time.Now(),
+                               CreationDate: configMap.GetCreationTimestamp().Time,
                        },
                        Certificate: keystore.Certificate{
                                Type:    "X.509",

or a checksum of the ca-bundle.crt content could be computed and be persisted as annotation.

raffaelespazzoli commented 1 year ago

it would still be different than the one used to create the truststore. Maybe it's possible to inspect the truststore (if it exists) and use exactly that timestamp

vinzent commented 1 year ago

but if you update the ca-bundle.crt content then the truststore.jks will still have the same timestamp from the first time it was created. I don't think the CreationDate jks property has much meaning.

vinzent commented 1 year ago

@raffaelespazzoli I want to proceed with this., because it "hurts" our cluster every pod restart (dozens of java pods restarted).

found out the secrets keystore controller writes a annotation: https://github.com/redhat-cop/cert-utils-operator/blob/7c20a03107b314a546eb3afb1e6457e94fe3e15e/controllers/secrettokeystore/secret_to_keystore_controller.go#L325-L338

I see these possible ways to fix this situation:

any method but the last will trigger a final "useless" update of the configmap.

raffaelespazzoli commented 1 year ago

let's use the configmap creation time. can you send a PR?

vinzent commented 1 year ago

i've created a patch: https://github.com/redhat-cop/cert-utils-operator/compare/master...vinzent:cert-utils-operator:persistent-jks-creationdate-property?expand=1

Built image (https://quay.io/mueller/cert-utils-operator:latest).

Testing:

Stumbled upon this https://github.com/pavlo-v-chernykh/keystore-go/issues/34 (fun fact: created by @raffaelespazzoli ;-) ). Maybe the WithOrderedAliases and WithCustomRandomNumberGenerator options also needs to be used?

vinzent commented 1 year ago

I figured out that the configmaptokeystore uses an outdated keystore-go (2.0.1) lib. Current is 4.4.1. Also path to lib changed (pavel -> pavlo). And it requires at least go 1.17.

more changes pushed to my branch: https://github.com/redhat-cop/cert-utils-operator/compare/master...vinzent:cert-utils-operator:persistent-jks-creationdate-property?expand=1

So I think to fix this issue, following changes are needed:

raffaelespazzoli commented 1 year ago

ok, please confirm that this change fixes the issue, then send a PR?

vinzent commented 1 year ago

The #144 is now ready to be reviewed.