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/onboarding/main.go b/onboarding/main.go index 170feb4e46..856b65aa96 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, err } return clientset, nil @@ -122,3 +162,22 @@ 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/ocs.openshift.io/v1/namespaces/%s/storageclusters/%s", + operatorNamespace, + storageClusterName, + ) + storageClusterMetadataJSON, 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(storageClusterMetadataJSON, &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