From baa693ab7c3696aef9730dc9b7a4ea11d4cd99c4 Mon Sep 17 00:00:00 2001 From: rchikatw Date: Fri, 12 Jan 2024 19:04:01 +0530 Subject: [PATCH] fixed re trigger job issue when public key deleted Signed-off-by: rchikatw --- controllers/storagecluster/provider_server.go | 2 + .../storagecluster_controller.go | 20 +- controllers/util/util.go | 4 + .../onboarding-secret-generator-role.yaml | 8 + onboarding/main.go | 181 ++++++++++++------ rbac/onboarding-secret-generator-role.yaml | 8 + 6 files changed, 152 insertions(+), 71 deletions(-) diff --git a/controllers/storagecluster/provider_server.go b/controllers/storagecluster/provider_server.go index f405a5d3b0..c7580615e2 100644 --- a/controllers/storagecluster/provider_server.go +++ b/controllers/storagecluster/provider_server.go @@ -454,6 +454,8 @@ func getOnboardingJobObject(instance *ocsv1.StorageCluster) *batchv1.Job { Namespace: instance.Namespace, }, Spec: batchv1.JobSpec{ + // Eligible to delete automatically when job finishes + TTLSecondsAfterFinished: util.ToPointer[int32](0), Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyOnFailure, diff --git a/controllers/storagecluster/storagecluster_controller.go b/controllers/storagecluster/storagecluster_controller.go index 9065912622..3051d5229f 100644 --- a/controllers/storagecluster/storagecluster_controller.go +++ b/controllers/storagecluster/storagecluster_controller.go @@ -164,13 +164,17 @@ func (r *StorageClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { }, } - onboardingSecretPredicates := builder.WithPredicates( - predicate.NewPredicateFuncs( - func(client client.Object) bool { - return client.GetName() == onboardingTicketPublicKeySecretName - }, - ), - ) + secretPredicate := predicate.Funcs{ + DeleteFunc: func(e event.DeleteEvent) bool { + return true + }, + CreateFunc: func(e event.CreateEvent) bool { + return e.Object.GetName() != onboardingTicketPublicKeySecretName + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return e.ObjectNew.GetName() != onboardingTicketPublicKeySecretName + }, + } builder := ctrl.NewControllerManagedBy(mgr). For(&ocsv1.StorageCluster{}, builder.WithPredicates(scPredicate)). @@ -179,6 +183,7 @@ func (r *StorageClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&appsv1.Deployment{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Owns(&corev1.Service{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Owns(&corev1.ConfigMap{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). + Owns(&corev1.Secret{}, builder.WithPredicates(secretPredicate)). Watches(&ocsv1.StorageProfile{}, enqueueStorageClusterRequest). Watches( &extv1.CustomResourceDefinition{ @@ -188,7 +193,6 @@ func (r *StorageClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { }, enqueueStorageClusterRequest, ). - Watches(&corev1.Secret{}, enqueueStorageClusterRequest, onboardingSecretPredicates). Watches(&ocsv1alpha1.StorageConsumer{}, enqueueStorageClusterRequest, builder.WithPredicates(ocsClientOperatorVersionPredicate)) if os.Getenv("SKIP_NOOBAA_CRD_WATCH") != "true" { builder.Owns(&nbv1.NooBaa{}) diff --git a/controllers/util/util.go b/controllers/util/util.go index f0d1ad6b93..8e0d9ea140 100644 --- a/controllers/util/util.go +++ b/controllers/util/util.go @@ -22,3 +22,7 @@ func DetectDuplicateInStringSlice(slice []string) bool { } return false } + +func ToPointer[T any](value T) *T { + return &value +} diff --git a/deploy/ocs-operator/manifests/onboarding-secret-generator-role.yaml b/deploy/ocs-operator/manifests/onboarding-secret-generator-role.yaml index cae160c88c..9e83a028e7 100644 --- a/deploy/ocs-operator/manifests/onboarding-secret-generator-role.yaml +++ b/deploy/ocs-operator/manifests/onboarding-secret-generator-role.yaml @@ -11,3 +11,11 @@ rules: - get - list - create + - delete +- apiGroups: + - ocs.openshift.io + resources: + - storageclusters + verbs: + - get + - list diff --git a/onboarding/main.go b/onboarding/main.go index 170feb4e46..e1090e5931 100644 --- a/onboarding/main.go +++ b/onboarding/main.go @@ -4,7 +4,9 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" + "encoding/json" "encoding/pem" + "fmt" "os" "github.com/red-hat-storage/ocs-operator/v4/controllers/util" @@ -18,80 +20,119 @@ import ( ) const ( - onboardingTicketPublicKeySecretName = "onboarding-ticket-key" //Name of existing public key which is used ocs-operator - onboardingPrivateKeySecretName = "onboarding-private-key" - serviceAccountName = "onboarding-secret-generator" + // Name of existing public key which is used ocs-operator + onboardingValidationPublicKeySecretName = "onboarding-ticket-key" + onboardingValidationPrivateKeySecretName = "onboarding-private-key" + storageClusterName = "ocs-storagecluster" ) func main() { clientset, err := newClient() if err != nil { - klog.Error(err, "failed to create controller-runtime client") - return + klog.Errorf("failed to create clientset: %v", err) + os.Exit(1) } + ctx := context.Background() operatorNamespace, err := util.GetOperatorNamespace() if err != nil { - klog.Error(err, "unable to get operator namespace") + klog.Errorf("unable to get operator namespace: %v", err) + os.Exit(1) + } + + // Generate RSA key. + privateKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + klog.Errorf("unable to generate private: %v", err) os.Exit(1) } - // 1. Check public key secret exist or not - _, err = clientset.CoreV1().Secrets(operatorNamespace).Get(context.TODO(), onboardingTicketPublicKeySecretName, metav1.GetOptions{}) - - if err != nil && kerrors.IsNotFound(err) { - // Generate RSA key. - var err error - privateKey, err := rsa.GenerateKey(rand.Reader, 4096) - if err != nil { - klog.Error(err, "unable to generate private") - os.Exit(1) - } - - publicKey := &privateKey.PublicKey - // Export the keys to pem string - privatePem := convertRsaPrivateKeyAsPemStr(privateKey) - publicPem, err := convertRsaPublicKeyAsPemStr(publicKey) - - if err != nil { - klog.Error(err, "failed to convert public key to pem str") - os.Exit(1) - } - - privateSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: onboardingPrivateKeySecretName, - Namespace: operatorNamespace, - Annotations: map[string]string{"kubernetes.io/service-account.name": serviceAccountName}, - }, - Type: "kubernetes.io/service-account-token", - StringData: map[string]string{ - "key": privatePem, - }, - } - - _, err = clientset.CoreV1().Secrets(operatorNamespace).Create(context.Background(), privateSecret, metav1.CreateOptions{}) - - if err != nil { - klog.Error(err, "Failed to create private secret.") - os.Exit(1) - } - publicSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: onboardingTicketPublicKeySecretName, - Namespace: operatorNamespace, - }, - StringData: map[string]string{ - "key": publicPem, - }, - } - - _, err = clientset.CoreV1().Secrets(operatorNamespace).Create(context.Background(), publicSecret, metav1.CreateOptions{}) - if err != nil { - klog.Error(err, "Failed to create public secret.") - os.Exit(1) - } + publicKey := &privateKey.PublicKey + // Export the keys to pem string + privatePem := convertRsaPrivateKeyAsPemStr(privateKey) + publicPem, err := convertRsaPublicKeyAsPemStr(publicKey) + if err != nil { + klog.Errorf("failed to convert public key to pem str: %v", err) + os.Exit(1) + } + // In situations where there is a risk of one secret being updated and potentially + // failing to update another, it is recommended not to rely solely on clientset update mechanisms. + // Instead, a safer approach is to delete both secrets and then recreate them simultaneously + // to ensure consistency and accuracy of all secrets. By this way it will be easier to diagnose the + // issues if one or two secrets do not exist instead of trying to understand if they match + err = clientset.CoreV1(). + Secrets(operatorNamespace). + Delete(ctx, onboardingValidationPrivateKeySecretName, metav1.DeleteOptions{}) + if err != nil && !kerrors.IsNotFound(err) { + klog.Errorf("failed to delete private secret: %v", err) + os.Exit(1) + } + + // Delete public key secret + err = clientset.CoreV1(). + Secrets(operatorNamespace). + Delete(ctx, onboardingValidationPublicKeySecretName, metav1.DeleteOptions{}) + if err != nil && !kerrors.IsNotFound(err) { + klog.Errorf("failed to delete public secret: %v", err) + os.Exit(1) + } + + storageClusterMetadata, err := getStorageClusterMetadata(ctx, operatorNamespace, clientset) + if err != nil { + klog.Errorf("failed to get storage cluster metadata: %v", err) + os.Exit(1) + } + + privateSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: onboardingValidationPrivateKeySecretName, + Namespace: operatorNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + UID: storageClusterMetadata.UID, + APIVersion: storageClusterMetadata.APIVersion, + Kind: storageClusterMetadata.Kind, + Name: storageClusterMetadata.Name, + }, + }}, + StringData: map[string]string{ + "key": privatePem, + }, + } + + _, err = clientset.CoreV1(). + Secrets(operatorNamespace). + Create(ctx, privateSecret, metav1.CreateOptions{}) + + if err != nil { + klog.Errorf("failed to create private secret: %v", err) + os.Exit(1) + } + + publicSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: onboardingValidationPublicKeySecretName, + Namespace: operatorNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + UID: storageClusterMetadata.UID, + APIVersion: storageClusterMetadata.APIVersion, + Kind: storageClusterMetadata.Kind, + Name: storageClusterMetadata.Name, + }, + }}, + StringData: map[string]string{ + "key": publicPem, + }, + } + + _, err = clientset.CoreV1(). + Secrets(operatorNamespace). + Create(ctx, publicSecret, metav1.CreateOptions{}) + if err != nil { + klog.Errorf("failed to create public secret: %v", err) + os.Exit(1) } } @@ -99,9 +140,8 @@ func main() { func newClient() (*kubernetes.Clientset, error) { config := runtime.GetConfigOrDie() clientset, err := kubernetes.NewForConfig(config) - if err != nil { - klog.Error(err, "failed to get clientset") + return nil, fmt.Errorf("failed to create clientset: %v", err) } return clientset, nil @@ -122,3 +162,18 @@ func convertRsaPublicKeyAsPemStr(publicKey *rsa.PublicKey) (string, error) { return string(publicKeyPem), nil } + +func getStorageClusterMetadata(ctx context.Context, operatorNamespace string, clientset *kubernetes.Clientset) (*metav1.PartialObjectMetadata, error) { + var storageClusterMetadata metav1.PartialObjectMetadata + storageClusterGVKPath := fmt.Sprintf("/apis/%s/%s/namespaces/%s/%s/%s", "ocs.openshift.io", "v1", operatorNamespace, "storageclusters", storageClusterName) + instance, err := clientset.RESTClient().Get().AbsPath(storageClusterGVKPath).Do(ctx).Raw() + if err != nil { + return nil, fmt.Errorf("failed to get storage cluster metadata: %v", err) + } + + if err = json.Unmarshal(instance, &storageClusterMetadata); err != nil { + return nil, fmt.Errorf("failed to parse storage cluster metadata response: %v", err) + } + + return &storageClusterMetadata, nil +} diff --git a/rbac/onboarding-secret-generator-role.yaml b/rbac/onboarding-secret-generator-role.yaml index cae160c88c..9e83a028e7 100644 --- a/rbac/onboarding-secret-generator-role.yaml +++ b/rbac/onboarding-secret-generator-role.yaml @@ -11,3 +11,11 @@ rules: - get - list - create + - delete +- apiGroups: + - ocs.openshift.io + resources: + - storageclusters + verbs: + - get + - list