From b2144ca3442a8086a4d42f27ffff9484379090fe 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 | 3 + .../storagecluster_controller.go | 10 +- .../onboarding-secret-generator-role.yaml | 8 + hack/ticketgen/ticketcheck.go | 3 +- onboarding/main.go | 187 +++++++++++------- rbac/onboarding-secret-generator-role.yaml | 8 + services/provider/server/server.go | 4 +- 7 files changed, 142 insertions(+), 81 deletions(-) diff --git a/controllers/storagecluster/provider_server.go b/controllers/storagecluster/provider_server.go index f405a5d3b0..c341957375 100644 --- a/controllers/storagecluster/provider_server.go +++ b/controllers/storagecluster/provider_server.go @@ -16,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -454,6 +455,8 @@ func getOnboardingJobObject(instance *ocsv1.StorageCluster) *batchv1.Job { Namespace: instance.Namespace, }, Spec: batchv1.JobSpec{ + // Eligible to delete automatically when job finishes + TTLSecondsAfterFinished: ptr.To(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..0e821efc7b 100644 --- a/controllers/storagecluster/storagecluster_controller.go +++ b/controllers/storagecluster/storagecluster_controller.go @@ -164,14 +164,6 @@ func (r *StorageClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { }, } - onboardingSecretPredicates := builder.WithPredicates( - predicate.NewPredicateFuncs( - func(client client.Object) bool { - return client.GetName() == onboardingTicketPublicKeySecretName - }, - ), - ) - builder := ctrl.NewControllerManagedBy(mgr). For(&ocsv1.StorageCluster{}, builder.WithPredicates(scPredicate)). Owns(&cephv1.CephCluster{}). @@ -179,6 +171,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(predicate.GenerationChangedPredicate{})). Watches(&ocsv1.StorageProfile{}, enqueueStorageClusterRequest). Watches( &extv1.CustomResourceDefinition{ @@ -188,7 +181,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/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/hack/ticketgen/ticketcheck.go b/hack/ticketgen/ticketcheck.go index 5da329af66..8eabcb96a5 100644 --- a/hack/ticketgen/ticketcheck.go +++ b/hack/ticketgen/ticketcheck.go @@ -30,11 +30,10 @@ func main() { panic(err) } pemBlock, _ := pem.Decode(keyfile) - key, err := x509.ParsePKIXPublicKey(pemBlock.Bytes) + pubKey, err := x509.ParsePKCS1PublicKey(pemBlock.Bytes) if err != nil { panic(err) } - pubKey := key.(*rsa.PublicKey) fmt.Printf("Reading ticket from: %s\n", *ticketfileStr) ticketData, err := os.ReadFile(*ticketfileStr) diff --git a/onboarding/main.go b/onboarding/main.go index 170feb4e46..e08c7e00b8 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,115 @@ 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) + } + + publicKey := &privateKey.PublicKey + // Export the keys to pem string + privatePem := convertRsaPrivateKeyAsPemStr(privateKey) + publicPem := convertRsaPublicKeyAsPemStr(publicKey) + + // 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) } - // 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) - } + 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 +136,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, err } return clientset, nil @@ -113,12 +149,27 @@ func convertRsaPrivateKeyAsPemStr(privateKey *rsa.PrivateKey) string { return string(privateKeyPem) } -func convertRsaPublicKeyAsPemStr(publicKey *rsa.PublicKey) (string, error) { - publicKeyBytes, err := x509.MarshalPKIXPublicKey(publicKey) +func convertRsaPublicKeyAsPemStr(publicKey *rsa.PublicKey) string { + publicKeyBytes := x509.MarshalPKCS1PublicKey(publicKey) + publicKeyPem := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: publicKeyBytes}) + return string(publicKeyPem) +} + +func getStorageClusterMetadata(ctx context.Context, operatorNamespace string, clientset *kubernetes.Clientset) (*metav1.PartialObjectMetadata, error) { + var storageClusterMetadata metav1.PartialObjectMetadata + storageClusterGVKPath := fmt.Sprintf( + "/apis/ocs.openshift.io/v1/namespaces/%s/storageclusters/%s", + operatorNamespace, + storageClusterName, + ) + storageClusterMetadataJSON, err := clientset.RESTClient().Get().AbsPath(storageClusterGVKPath).Do(ctx).Raw() if err != nil { - return "", err + return nil, fmt.Errorf("failed to get storage cluster metadata: %v", err) + } + + if err = json.Unmarshal(storageClusterMetadataJSON, &storageClusterMetadata); err != nil { + return nil, fmt.Errorf("failed to parse storage cluster metadata response: %v", err) } - publicKeyPem := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: publicKeyBytes}) - return string(publicKeyPem), nil + 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 diff --git a/services/provider/server/server.go b/services/provider/server/server.go index 02e6551100..28b63f0aff 100644 --- a/services/provider/server/server.go +++ b/services/provider/server/server.go @@ -404,12 +404,12 @@ func (s *OCSProviderServer) getOnboardingValidationKey(ctx context.Context) (*rs return nil, fmt.Errorf("invalid PEM block") } - key, err := x509.ParsePKIXPublicKey(block.Bytes) + publicKey, err := x509.ParsePKCS1PublicKey(block.Bytes) if err != nil { return nil, fmt.Errorf("failed to parse public key. %v", err) } - return key.(*rsa.PublicKey), nil + return publicKey, nil } func mustMarshal(data map[string]string) []byte {