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

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

Closed
vinzent opened this issue Nov 24, 2022 · 10 comments · Fixed by #144
Closed

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

vinzent opened this issue Nov 24, 2022 · 10 comments · Fixed by #144

Comments

@vinzent
Copy link
Contributor

vinzent commented Nov 24, 2022

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
Copy link
Contributor

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

@vinzent
Copy link
Contributor Author

vinzent commented Nov 24, 2022

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
Copy link
Contributor

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
Copy link
Contributor Author

vinzent commented Nov 24, 2022

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
Copy link
Contributor Author

vinzent commented Mar 10, 2023

@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:

func (r *SecretToKeyStoreReconciler) getCreationTimestamp(secret *corev1.Secret) (time.Time, error) {
if timeStr, ok := secret.GetAnnotations()[storesCreationTiemstamp]; ok {
creationTime, err := time.Parse(time.RFC3339, timeStr)
if err != nil {
r.Log.Error(err, "unable to parse creation time")
return time.Time{}, err
}
return creationTime, nil
} else {
now := time.Now()
secret.GetAnnotations()[storesCreationTiemstamp] = now.Format(time.RFC3339)
return now, nil
}

I see these possible ways to fix this situation:

  • use a fixed value for creation timestamp (1970-01-01...). As this CreationDate has no real value IMHO the value can be anything.
  • use the same as for the secrets keystore controller. Store timestamp as annotation.
  • use the CreationTime timestamp from the ConfigMap
  • parse the keystore and re-use the effective keystore creationdate

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

@raffaelespazzoli
Copy link
Contributor

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

@vinzent
Copy link
Contributor Author

vinzent commented Mar 15, 2023

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:

  • truststore.jks in ConfigMap still gets updated every restart.
  • comparing "keytool -list -keystore xyz.jks" before and after shows no difference

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

@vinzent
Copy link
Contributor Author

vinzent commented Mar 16, 2023

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:

  • update builder image go >1.17
  • update all code to use keystore-go 4.4.1
  • use configMap CreattionTimestamp for JKS CreationTime and WithOrderedAliases feature of keystore-go

@raffaelespazzoli
Copy link
Contributor

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

@vinzent
Copy link
Contributor Author

vinzent commented Apr 20, 2023

The #144 is now ready to be reviewed.

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

Successfully merging a pull request may close this issue.

2 participants