Merge pull request #130 from trevorbox/fixes#126

Fixes #126
redhat-cop · Mar 1, 2023 · 18c909d · 18c909d
2 parents c56bae0 + 4b06f9d
commit 18c909d
Show file tree

Hide file tree

Showing 29 changed files with 139 additions and 6 deletions.
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/redhatcop.redhat.io_authenginemounts.yaml b/config/crd/bases/redhatcop.redhat.io_authenginemounts.yaml
@@ -170,6 +170,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_databasesecretengineconfigs.yaml b/config/crd/bases/redhatcop.redhat.io_databasesecretengineconfigs.yaml
@@ -120,6 +120,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_databasesecretengineroles.yaml b/config/crd/bases/redhatcop.redhat.io_databasesecretengineroles.yaml
@@ -110,6 +110,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_databasesecretenginestaticroles.yaml b/config/crd/bases/redhatcop.redhat.io_databasesecretenginestaticroles.yaml
@@ -110,6 +110,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_githubsecretengineconfigs.yaml b/config/crd/bases/redhatcop.redhat.io_githubsecretengineconfigs.yaml
@@ -114,6 +114,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_githubsecretengineroles.yaml b/config/crd/bases/redhatcop.redhat.io_githubsecretengineroles.yaml
@@ -109,6 +109,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_jwtoidcauthengineconfigs.yaml b/config/crd/bases/redhatcop.redhat.io_jwtoidcauthengineconfigs.yaml
@@ -261,6 +261,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_jwtoidcauthengineroles.yaml b/config/crd/bases/redhatcop.redhat.io_jwtoidcauthengineroles.yaml
@@ -166,6 +166,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_kubernetesauthengineconfigs.yaml b/config/crd/bases/redhatcop.redhat.io_kubernetesauthengineconfigs.yaml
@@ -118,6 +118,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_kubernetesauthengineroles.yaml b/config/crd/bases/redhatcop.redhat.io_kubernetesauthengineroles.yaml
@@ -132,6 +132,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_kubernetessecretengineconfigs.yaml b/config/crd/bases/redhatcop.redhat.io_kubernetessecretengineconfigs.yaml
@@ -110,6 +110,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_kubernetessecretengineroles.yaml b/config/crd/bases/redhatcop.redhat.io_kubernetessecretengineroles.yaml
@@ -118,6 +118,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_ldapauthengineconfigs.yaml b/config/crd/bases/redhatcop.redhat.io_ldapauthengineconfigs.yaml
@@ -255,6 +255,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_ldapauthenginegroups.yaml b/config/crd/bases/redhatcop.redhat.io_ldapauthenginegroups.yaml
@@ -109,6 +109,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_passwordpolicies.yaml b/config/crd/bases/redhatcop.redhat.io_passwordpolicies.yaml
@@ -108,6 +108,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_pkisecretengineconfigs.yaml b/config/crd/bases/redhatcop.redhat.io_pkisecretengineconfigs.yaml
@@ -149,6 +149,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_pkisecretengineroles.yaml b/config/crd/bases/redhatcop.redhat.io_pkisecretengineroles.yaml
@@ -188,6 +188,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_policies.yaml b/config/crd/bases/redhatcop.redhat.io_policies.yaml
@@ -108,6 +108,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_quaysecretengineconfigs.yaml b/config/crd/bases/redhatcop.redhat.io_quaysecretengineconfigs.yaml
@@ -113,6 +113,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_quaysecretengineroles.yaml b/config/crd/bases/redhatcop.redhat.io_quaysecretengineroles.yaml
@@ -112,6 +112,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_quaysecretenginestaticroles.yaml b/config/crd/bases/redhatcop.redhat.io_quaysecretenginestaticroles.yaml
@@ -110,6 +110,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_rabbitmqsecretengineconfigs.yaml b/config/crd/bases/redhatcop.redhat.io_rabbitmqsecretengineconfigs.yaml
@@ -110,6 +110,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_rabbitmqsecretengineroles.yaml b/config/crd/bases/redhatcop.redhat.io_rabbitmqsecretengineroles.yaml
@@ -110,6 +110,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_randomsecrets.yaml b/config/crd/bases/redhatcop.redhat.io_randomsecrets.yaml
@@ -108,6 +108,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_secretenginemounts.yaml b/config/crd/bases/redhatcop.redhat.io_secretenginemounts.yaml
@@ -163,6 +163,7 @@ spec:
                               TODO: Add other useful fields. apiVersion, kind, uid?'
                             type: string
                         type: object
+                        x-kubernetes-map-type: atomic
                       tlsServerName:
                         description: TLSServerName Name to use as the SNI host when
                           connecting via TLS.

diff --git a/config/crd/bases/redhatcop.redhat.io_vaultsecrets.yaml b/config/crd/bases/redhatcop.redhat.io_vaultsecrets.yaml
@@ -116,6 +116,63 @@ spec:
                           type: object
                           x-kubernetes-map-type: atomic
                       type: object
+                    connection:
+                      description: Connection represents the information needed to
+                        connect to Vault. This operator uses the standard Vault environment
+                        variables to connect to Vault. If you need to override those
+                        settings and for example connect to a different Vault instance,
+                        you can do with this section of the CR.
+                      properties:
+                        address:
+                          description: 'Address Address of the Vault server expressed
+                            as a URL and port, for example: https://127.0.0.1:8200/'
+                          type: string
+                        maxRetries:
+                          description: MaxRetries Maximum number of retries when certain
+                            error codes are encountered. The default is 2, for three
+                            total attempts. Set this to 0 or less to disable retrying.
+                            Error codes that are retried are 412 (client consistency
+                            requirement not satisfied) and all 5xx except for 501
+                            (not implemented).
+                          type: integer
+                        tLSConfig:
+                          properties:
+                            cacert:
+                              description: Cacert Path to a PEM-encoded CA certificate
+                                file on the local disk. This file is used to verify
+                                the Vault server's SSL certificate. This environment
+                                variable takes precedence over a cert passed via the
+                                secret.
+                              type: string
+                            skipVerify:
+                              description: SkipVerify Do not verify Vault's presented
+                                certificate before communicating with it. Setting
+                                this variable is not recommended and voids Vault's
+                                security model.
+                              type: boolean
+                            tlsSecret:
+                              description: 'TLSSecret namespace-local secret containing
+                                the tls material for the connection. the expected
+                                keys for the secret are: ca bundle -> "ca.crt", certificate
+                                -> "tls.crt", key -> "tls.key"'
+                              properties:
+                                name:
+                                  description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                                    TODO: Add other useful fields. apiVersion, kind,
+                                    uid?'
+                                  type: string
+                              type: object
+                              x-kubernetes-map-type: atomic
+                            tlsServerName:
+                              description: TLSServerName Name to use as the SNI host
+                                when connecting via TLS.
+                              type: string
+                          type: object
+                        timeOut:
+                          description: Timeout Timeout variable. The default value
+                            is 60s.
+                          type: string
+                      type: object
                     name:
                       description: Name is an arbitrary, but unique, name for this
                         KV Vault secret and referenced when templating.

diff --git a/controllers/randomsecret_controller.go b/controllers/randomsecret_controller.go
@@ -76,11 +76,6 @@ func (r *RandomSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return reconcile.Result{}, err
 	}
 
-	// how to read this if: if the secret has been initialized once and there is no refresh period or time to refresh has not arrived yet, return.
-	if instance.Status.LastVaultSecretUpdate != nil && (instance.Spec.RefreshPeriod == nil || (instance.Spec.RefreshPeriod != nil && !instance.Status.LastVaultSecretUpdate.Add(instance.Spec.RefreshPeriod.Duration).Before(time.Now()))) {
-		return reconcile.Result{}, nil
-	}
-
 	ctx1, err := prepareContext(ctx, r.ReconcilerBase, instance)
 	if err != nil {
 		r.Log.Error(err, "unable to prepare context", "instance", instance)
@@ -105,6 +100,11 @@ func (r *RandomSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return reconcile.Result{}, nil
 	}
 
+	// how to read this if: if the secret has been initialized once and there is no refresh period or time to refresh has not arrived yet, return.
+	if instance.Status.LastVaultSecretUpdate != nil && (instance.Spec.RefreshPeriod == nil || (instance.Spec.RefreshPeriod != nil && !instance.Status.LastVaultSecretUpdate.Add(instance.Spec.RefreshPeriod.Duration).Before(time.Now()))) {
+		return reconcile.Result{}, nil
+	}
+
 	err = r.manageReconcileLogic(ctx1, instance)
 	if err != nil {
 		r.Log.Error(err, "unable to complete reconcile logic", "instance", instance)
@@ -169,7 +169,7 @@ func (r *RandomSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
 			if !ok {
 				return false
 			}
-			return newSecret.Spec.RefreshPeriod != oldSecret.Spec.RefreshPeriod || !reflect.DeepEqual(newSecret.Spec.SecretFormat, oldSecret.Spec.SecretFormat)
+			return util.IsBeingDeleted(newSecret) || newSecret.Spec.RefreshPeriod != oldSecret.Spec.RefreshPeriod || !reflect.DeepEqual(newSecret.Spec.SecretFormat, oldSecret.Spec.SecretFormat)
 		},
 		CreateFunc: func(e event.CreateEvent) bool {
 			return true

diff --git a/controllers/vaultsecret_controller.go b/controllers/vaultsecret_controller.go
@@ -93,6 +93,24 @@ func (r *VaultSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	ctx = context.WithValue(ctx, "kubeClient", r.GetClient())
 	ctx = context.WithValue(ctx, "restConfig", r.GetRestConfig())
 
+	if util.IsBeingDeleted(instance) {
+		if !util.HasFinalizer(instance, vaultutils.GetFinalizer(instance)) {
+			return reconcile.Result{}, nil
+		}
+		err := r.manageCleanUpLogic(ctx, instance)
+		if err != nil {
+			r.Log.Error(err, "unable to delete instance", "instance", instance)
+			return vaultresourcecontroller.ManageOutcome(ctx, r.ReconcilerBase, instance, err)
+		}
+		util.RemoveFinalizer(instance, vaultutils.GetFinalizer(instance))
+		err = r.GetClient().Update(ctx, instance)
+		if err != nil {
+			r.Log.Error(err, "unable to update instance", "instance", instance)
+			return vaultresourcecontroller.ManageOutcome(ctx, r.ReconcilerBase, instance, err)
+		}
+		return reconcile.Result{}, nil
+	}
+
 	shouldSync, err := r.shouldSync(ctx, instance)
 	if err != nil {
 		// There was a problem determining if the event should cause a sync.
@@ -130,6 +148,26 @@ func (r *VaultSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 }
 
+func (r *VaultSecretReconciler) manageCleanUpLogic(context context.Context, instance *redhatcopv1alpha1.VaultSecret) error {
+
+	k8sSecret := &corev1.Secret{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       secretKind,
+			APIVersion: secretAPIVersion,
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      instance.Spec.TemplatizedK8sSecret.Name,
+			Namespace: instance.Namespace,
+		},
+	}
+	err := r.DeleteResourceIfExists(context, k8sSecret)
+	if err != nil {
+		r.Log.Error(err, "unable to delete k8s secret", "instance", instance, "k8s secret", k8sSecret)
+		return err
+	}
+	return nil
+}
+
 func (r *VaultSecretReconciler) formatK8sSecret(instance *redhatcopv1alpha1.VaultSecret, data interface{}) (*corev1.Secret, error) {
 
 	bytesData := make(map[string][]byte)
@@ -303,6 +341,8 @@ func (r *VaultSecretReconciler) manageSyncLogic(ctx context.Context, instance *r
 			return errors.New("secret not found at path: " + vaultSecretDefinition.GetPath())
 		}
 
+		r.Log.V(1).Info("", "", vaultSecret.LeaseDuration)
+
 		definitionsStatus[idx] = redhatcopv1alpha1.VaultSecretDefinitionStatus{
 			Name:          vaultSecretDefinition.Name,
 			LeaseID:       vaultSecret.LeaseID,
@@ -356,10 +396,16 @@ func (r *VaultSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
 				return false
 			}
 
+			if util.IsBeingDeleted(newVaultSecret) {
+				r.Log.V(1).Info("Update Event - Marked for deletion", "kind", vaultSecretKind, "namespacedName", toNamespacedName(e.ObjectNew))
+				return true
+			}
+
 			if !reflect.DeepEqual(oldVaultSecret.Spec, newVaultSecret.Spec) {
 				r.Log.V(1).Info("Update Event - Spec changed", "kind", vaultSecretKind, "namespacedName", toNamespacedName(e.ObjectNew))
 				return true
 			}
+
 			return false
 		},
 		CreateFunc: func(e event.CreateEvent) bool {