[Fix/Operator] JKS password issue

CHANGELOG.md

@@ -11,6 +11,7 @@
 ### Fixed Bugs
 
 - [PR #82](https://github.com/Orange-OpenSource/nifikop/pull/82) - **[Operator/NifiParameterContext]** Enable empty value
+- [PR #83](https://github.com/Orange-OpenSource/nifikop/pull/83) - **[Operator/NiFiUser]** Rework the certificate secret creation, to prevent issues with JKS password creation.
 
 ## v0.5.2
 

api/v1alpha1/common_types.go

@@ -253,8 +253,10 @@ const (
 	TLSCert string = "tls.crt"
 	// TLSCert is where a private key is stored in a user secret when requested
 	TLSKey string = "tls.key"
-	// TLSJKSKey is where a JKS is stored in a user secret when requested
-	TLSJKSKey string = "tls.jks"
+	// TLSJKSKeyStore is where a JKS keystore is stored in a user secret when requested
+	TLSJKSKeyStore string = "keystore.jks"
+	// TLSJKSTrustStore is where a JKS truststore is stored in a user secret when requested
+	TLSJKSTrustStore string = "truststore.jks"
 	// CoreCACertKey is where ca ceritificates are stored in user certificates
 	CoreCACertKey string = "ca.crt"
 	// CACertKey is the key where the CA certificate is stored in the operator secrets

controllers/nifiuser_controller.go

@@ -93,16 +93,17 @@ func (r *NifiUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		}
 		return RequeueWithError(r.Log, "failed to lookup referenced cluster", err)
 	}
-	// Avoid panic if the user wants to create a nifi user but the cluster is in plaintext mode
-	// TODO: refactor this and use webhook to validate if the cluster is eligible to create a nifi user
-	if cluster.Spec.ListenersConfig.SSLSecrets == nil {
-		return RequeueWithError(r.Log, "could not create Nifi user since cluster does not use ssl", errors.New("failed to create Nifi user"))
-	}
-
-	pkiManager := pki.GetPKIManager(r.Client, cluster)
 
 	if instance.Spec.GetCreateCert() {
 
+		// Avoid panic if the user wants to create a nifi user but the cluster is in plaintext mode
+		// TODO: refactor this and use webhook to validate if the cluster is eligible to create a nifi user
+		if cluster.Spec.ListenersConfig.SSLSecrets == nil {
+			return RequeueWithError(r.Log, "could not create Nifi user since cluster does not use ssl", errors.New("failed to create Nifi user"))
+		}
+
+		pkiManager := pki.GetPKIManager(r.Client, cluster)
+
 		// Reconcile no matter what to get a user certificate instance for ACL management
 		// TODO (tinyzimmer): This can go wrong if the user made a mistake in their secret path
 		// using the vault backend, then tried to delete and fix it. Should probably
@@ -137,14 +138,19 @@ func (r *NifiUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 				return RequeueWithError(r.Log, "failed to reconcile user secret", err)
 			}
 		}
+		// check if marked for deletion
+		if k8sutil.IsMarkedForDeletion(instance.ObjectMeta) {
+			r.Log.Info("Nifi user is marked for deletion, revoking certificates")
+			if err = pkiManager.FinalizeUserCertificate(ctx, instance); err != nil {
+				return RequeueWithError(r.Log, "failed to finalize user certificate", err)
+			}
+			return r.checkFinalizers(ctx, instance, cluster)
+		}
+
 	}
 
 	// check if marked for deletion
 	if k8sutil.IsMarkedForDeletion(instance.ObjectMeta) {
-		r.Log.Info("Nifi user is marked for deletion, revoking certificates")
-		if err = pkiManager.FinalizeUserCertificate(ctx, instance); err != nil {
-			return RequeueWithError(r.Log, "failed to finalize user certificate", err)
-		}
 		return r.checkFinalizers(ctx, instance, cluster)
 	}
 

pkg/pki/certmanagerpki/certmanager_user.go

@@ -48,6 +48,12 @@ func (c *certManager) ReconcileUserCertificate(ctx context.Context, user *v1alph
 
 	if err != nil && apierrors.IsNotFound(err) {
 		// the certificate does not exist, let's make one
+		// check if jks is required and create password for it
+		if user.Spec.IncludeJKS {
+			if err := c.injectJKSPassword(ctx, user); err != nil {
+				return nil, err
+			}
+		}
 		cert := c.clusterCertificateForUser(user, scheme)
 		if err = c.client.Create(ctx, cert); err != nil {
 			return nil, errorfactory.New(errorfactory.APIFailure{}, err, "could not create user certificate")
@@ -64,42 +70,37 @@ func (c *certManager) ReconcileUserCertificate(ctx context.Context, user *v1alph
 		return nil, err
 	}
 
-	if user.Spec.IncludeJKS {
-		if secret, err = c.injectJKS(ctx, user, secret); err != nil {
-			return nil, err
-		}
-	}
-
 	// Ensure controller reference on user secret
 	if err = c.ensureControllerReference(ctx, user, secret, scheme); err != nil {
 		return nil, err
 	}
 
-	// Ensure that the secret is populated with the required values.
-	for k, v := range secret.Data {
-		if len(v) == 0 && k != v1alpha1.CoreCACertKey {
-			return nil, errorfactory.New(errorfactory.APIFailure{}, errors.New("not all secret value populated"), "secret is not ready")
-		}
-	}
-
 	return &pkicommon.UserCertificate{
 		CA:          secret.Data[v1alpha1.CoreCACertKey],
 		Certificate: secret.Data[corev1.TLSCertKey],
 		Key:         secret.Data[corev1.TLSPrivateKeyKey],
 	}, nil
 }
 
-// injectJKS ensures that a secret contains JKS format when requested
-func (c *certManager) injectJKS(ctx context.Context, user *v1alpha1.NifiUser, secret *corev1.Secret) (*corev1.Secret, error) {
+// injectJKSPassword ensures that a secret contains JKS password when requested
+func (c *certManager) injectJKSPassword(ctx context.Context, user *v1alpha1.NifiUser) error {
 	var err error
-	if secret, err = certutil.EnsureSecretJKS(secret); err != nil {
-		return nil, errorfactory.New(errorfactory.InternalError{}, err, "could not inject secret with jks")
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      user.Spec.SecretName,
+			Namespace: user.Namespace,
+		},
+		Data: map[string][]byte{},
 	}
-	if err = c.client.Update(ctx, secret); err != nil {
-		return nil, errorfactory.New(errorfactory.APIFailure{}, err, "could not update secret with jks")
+	secret, err = certutil.EnsureSecretPassJKS(secret)
+	if err != nil {
+		return errorfactory.New(errorfactory.InternalError{}, err, "could not inject secret with jks password")
 	}
-	// Fetch the updated secret
-	return c.getUserSecret(ctx, user)
+	if err = c.client.Create(ctx, secret); err != nil {
+		return errorfactory.New(errorfactory.APIFailure{}, err, "could not create secret with jks password")
+	}
+
+	return nil
 }
 
 // ensureControllerReference ensures that a NifiUser owns a given Secret
@@ -128,33 +129,63 @@ func (c *certManager) getUserSecret(ctx context.Context, user *v1alpha1.NifiUser
 	err = c.client.Get(ctx, types.NamespacedName{Name: user.Spec.SecretName, Namespace: user.Namespace}, secret)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			err = errorfactory.New(errorfactory.ResourceNotReady{}, err, "user secret not ready")
-		} else {
-			err = errorfactory.New(errorfactory.APIFailure{}, err, "failed to get user secret")
+			return secret, errorfactory.New(errorfactory.ResourceNotReady{}, err, "user secret not ready")
 		}
+		return secret, errorfactory.New(errorfactory.APIFailure{}, err, "failed to get user secret")
 	}
-	return
+	if user.Spec.IncludeJKS {
+		if len(secret.Data) != 6 {
+			return secret, errorfactory.New(errorfactory.ResourceNotReady{}, err, "user secret not populated yet")
+		}
+	} else {
+		if len(secret.Data) != 3 {
+			return secret, errorfactory.New(errorfactory.ResourceNotReady{}, err, "user secret not populated yet")
+		}
+	}
+
+	for _, v := range secret.Data {
+		if len(v) == 0 {
+			return secret, errorfactory.New(errorfactory.ResourceNotReady{},
+				errors.New("not all secret value populated"), "secret is not ready")
+		}
+	}
+
+	return secret, nil
 }
 
 // clusterCertificateForUser generates a Certificate object for a NifiUser
 func (c *certManager) clusterCertificateForUser(user *v1alpha1.NifiUser, scheme *runtime.Scheme) *certv1.Certificate {
 	caName, caKind := c.getCA()
 	cert := &certv1.Certificate{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      user.Name,
-			Namespace: user.Namespace,
+			Name:      user.GetName(),
+			Namespace: user.GetNamespace(),
 		},
 		Spec: certv1.CertificateSpec{
 			SecretName:  user.Spec.SecretName,
 			KeyEncoding: certv1.PKCS8,
-			CommonName:  user.Name,
+			CommonName:  user.GetName(),
+			URISANs:     []string{fmt.Sprintf(pkicommon.SpiffeIdTemplate, c.cluster.Name, user.GetNamespace(), user.GetName())},
 			Usages:      []certv1.KeyUsage{certv1.UsageClientAuth, certv1.UsageServerAuth},
 			IssuerRef: certmeta.ObjectReference{
 				Name: caName,
 				Kind: caKind,
 			},
 		},
 	}
+	if user.Spec.IncludeJKS {
+		cert.Spec.Keystores = &certv1.CertificateKeystores{
+			JKS: &certv1.JKSKeystore{
+				Create: true,
+				PasswordSecretRef: certmeta.SecretKeySelector{
+					LocalObjectReference: certmeta.LocalObjectReference{
+						Name: user.Spec.SecretName,
+					},
+					Key: v1alpha1.PasswordKey,
+				},
+			},
+		}
+	}
 	if user.Spec.DNSNames != nil && len(user.Spec.DNSNames) > 0 {
 		cert.Spec.DNSNames = user.Spec.DNSNames
 	}

pkg/pki/certmanagerpki/certmanager_user_test.go

@@ -40,9 +40,12 @@ func newMockUserSecret() *corev1.Secret {
 	secret.Namespace = "test-namespace"
 	cert, key, _, _ := certutil.GenerateTestCert()
 	secret.Data = map[string][]byte{
-		corev1.TLSCertKey:       cert,
-		corev1.TLSPrivateKeyKey: key,
-		v1alpha1.CoreCACertKey:  cert,
+		corev1.TLSCertKey:         cert,
+		corev1.TLSPrivateKeyKey:   key,
+		v1alpha1.TLSJKSKeyStore:   []byte("testkeystore"),
+		v1alpha1.PasswordKey:      []byte("testpassword"),
+		v1alpha1.TLSJKSTrustStore: []byte("testtruststore"),
+		v1alpha1.CoreCACertKey:    cert,
 	}
 	return secret
 }
@@ -64,7 +67,12 @@ func TestReconcileUserCertificate(t *testing.T) {
 	} else if reflect.TypeOf(err) != reflect.TypeOf(errorfactory.ResourceNotReady{}) {
 		t.Error("Expected resource not ready error, got:", reflect.TypeOf(err))
 	}
-	manager.client.Create(context.TODO(), newMockUserSecret())
+	if err := manager.client.Delete(context.TODO(), newMockUserSecret()); err != nil {
+		t.Error("could not delete test secret")
+	}
+	if err := manager.client.Create(context.TODO(), newMockUserSecret()); err != nil {
+		t.Error("could not update test secret")
+	}
 	if _, err := manager.ReconcileUserCertificate(ctx, newMockUser(), scheme.Scheme); err != nil {
 		t.Error("Expected no error, got:", err)
 	}

pkg/resources/nifi/configmap.go

@@ -148,7 +148,8 @@ func (r *Reconciler) getNifiPropertiesConfigString(nConfig *v1alpha1.NodeConfig,
 		"SuperUsers":                         strings.Join(generateSuperUsers(superUsers), ";"),
 		"ServerKeystorePath":                 serverKeystorePath,
 		"ClientKeystorePath":                 clientKeystorePath,
-		"KeystoreFile":                       v1alpha1.TLSJKSKey,
+		"KeystoreFile":                       v1alpha1.TLSJKSKeyStore,
+		"TrustStoreFile":                     v1alpha1.TLSJKSTrustStore,
 		"ServerKeystorePassword":             serverPass,
 		"ClientKeystorePassword":             clientPass,
 		//

pkg/resources/templates/config/nifi_properties.go

@@ -169,7 +169,7 @@ nifi.security.keystore={{ .ServerKeystorePath }}/{{ .KeystoreFile }}
 nifi.security.keystoreType=JKS
 nifi.security.keystorePasswd={{ .ServerKeystorePassword }}
 nifi.security.keyPasswd={{ .ServerKeystorePassword }}
-nifi.security.truststore={{ .ServerKeystorePath }}/{{ .KeystoreFile }}
+nifi.security.truststore={{ .ServerKeystorePath }}/{{ .TrustStoreFile }}
 nifi.security.truststoreType=JKS
 nifi.security.truststorePasswd={{ .ServerKeystorePassword }}
 {{ end }}

pkg/util/cert/certutil.go

@@ -86,27 +86,16 @@ func GeneratePass(length int) (passw []byte) {
 	return
 }
 
-// EnsureSecretJKS ensures a JKS is present in a certificate secret
-func EnsureSecretJKS(secret *corev1.Secret) (injected *corev1.Secret, err error) {
+// EnsureSecretPassJKS ensures a JKS password is present in a certificate secret
+func EnsureSecretPassJKS(secret *corev1.Secret) (injected *corev1.Secret, err error) {
 
-	// If the JKS is already present - return
-	if _, ok := secret.Data[v1alpha1.TLSJKSKey]; ok {
+	// If the JKS Pass is already present - return
+	if _, ok := secret.Data[v1alpha1.PasswordKey]; ok {
 		return secret, nil
 	}
 
 	injected = secret.DeepCopy()
-
-	jks, passw, err := GenerateJKS(
-		secret.Data[corev1.TLSCertKey],
-		secret.Data[corev1.TLSPrivateKeyKey],
-		secret.Data[v1alpha1.CoreCACertKey],
-	)
-	if err != nil {
-		return
-	}
-
-	injected.Data[v1alpha1.TLSJKSKey] = jks
-	injected.Data[v1alpha1.PasswordKey] = passw
+	injected.Data[v1alpha1.PasswordKey] = GeneratePass(16)
 	return
 }
 

pkg/util/cert/certutil_test.go

@@ -123,7 +123,7 @@ func TestGenerateJKS(t *testing.T) {
 	}
 }
 
-func TestEnsureJKS(t *testing.T) {
+func TesEnsureSecretPassJKS(t *testing.T) {
 	cert, key, _, err := GenerateTestCert()
 	if err != nil {
 		t.Error("Failed to generate test certificate")
@@ -136,17 +136,14 @@ func TestEnsureJKS(t *testing.T) {
 		v1alpha1.CoreCACertKey:  cert,
 	}
 
-	if injectedSecret, err := EnsureSecretJKS(secret); err != nil {
+	if injectedSecret, err := EnsureSecretPassJKS(secret); err != nil {
 		t.Error("Expected injected corev1 secret, got error:", err)
 	} else {
-		if _, ok := injectedSecret.Data[v1alpha1.TLSJKSKey]; !ok {
-			t.Error("Expected JKS to be present in injected secret")
-		}
 		if _, ok := injectedSecret.Data[v1alpha1.PasswordKey]; !ok {
 			t.Error("Expected generated password in injected secret")
 		}
 
-		noModify, _ := EnsureSecretJKS(injectedSecret)
+		noModify, _ := EnsureSecretPassJKS(injectedSecret)
 		if !reflect.DeepEqual(noModify.Data, injectedSecret.Data) {
 			t.Error("Expected already injected secret to be returned identical")
 		}

pkg/util/pki/common.go

@@ -42,6 +42,8 @@ const (
 	// NodeControllerFQDNTemplate is combined with the above and cluster namespace
 	// to create a 'fake' full-name for the controller user
 	NodeControllerFQDNTemplate = "%s.%s.mgt.%s"
+	//
+	SpiffeIdTemplate = "spiffe://%s/ns/%s/nifiuser/%s"
 	// CAIntermediateTemplate is the template used for intermediate CA resources
 	CAIntermediateTemplate = "%s-intermediate.%s"
 	// CAFQDNTemplate is the template used for the FQDN of a CA