From 55de1bcb380d06dcc9d1119539ee6e35d5a6c4b8 Mon Sep 17 00:00:00 2001 From: Blaine Gardner Date: Wed, 18 Jun 2025 14:33:38 -0600 Subject: [PATCH] osd: implement automated cephx key rotation for osds Implement automated CephX key rotation for OSDs. OSDs have marshalled CephX statuses applied as an annotation on the OSD pod spec, which differs from other Rook Ceph daemons for a two reasons: A large cluster could have hundreds or thousands of OSDs, so it is more critical that Rook avoid rotating OSD keys more than once in the event a CephCluster reconcile restarts partway through OSD creation/updation. OSDs do not have their CephX keys stored in K8s Secrets, whose resource version is used for enforcing the desired restart behavior other daemon pods like RGWs. Signed-off-by: Blaine Gardner (cherry picked from commit 4189a196c1b7012db66c82d09c2dd43c5adf925a) --- Documentation/CRDs/specification.md | 13 ++ .../charts/rook-ceph/templates/resources.yaml | 22 ++ deploy/examples/crds.yaml | 22 ++ pkg/apis/ceph.rook.io/v1/types.go | 2 + .../ceph.rook.io/v1/zz_generated.deepcopy.go | 5 + pkg/operator/ceph/cluster/cluster.go | 1 + pkg/operator/ceph/cluster/osd/create.go | 7 + pkg/operator/ceph/cluster/osd/osd.go | 100 ++++++--- pkg/operator/ceph/cluster/osd/osd_test.go | 211 +++++++++++++++++- pkg/operator/ceph/cluster/osd/spec.go | 13 +- pkg/operator/ceph/cluster/osd/spec_test.go | 2 +- pkg/operator/ceph/cluster/osd/update.go | 40 ++++ pkg/operator/ceph/cluster/osd/update_test.go | 153 +++++++++++++ 13 files changed, 560 insertions(+), 31 deletions(-) diff --git a/Documentation/CRDs/specification.md b/Documentation/CRDs/specification.md index 0ee815f1b..c35f24e2e 100644 --- a/Documentation/CRDs/specification.md +++ b/Documentation/CRDs/specification.md @@ -5058,6 +5058,19 @@ CephxStatus +osd
+ + +CephxStatus + + + + +

OSD shows the CephX key status of of OSDs

+ + + + rbdMirrorPeer
diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index 4767345bf..001cc4ec2 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -5996,6 +5996,28 @@ spec: format: int32 type: integer type: object + osd: + description: OSD shows the CephX key status of of OSDs + properties: + keyCephVersion: + description: |- + KeyCephVersion reports the Ceph version that created the current generation's keys. This is + same string format as reported by `CephCluster.status.version.version` to allow them to be + compared. E.g., `20.2.0-0`. + For all newly-created resources, this field set to the version of Ceph that created the key. + The special value "Uninitialized" indicates that keys are being created for the first time. + An empty string indicates that the version is unknown, as expected in brownfield deployments. + type: string + keyGeneration: + description: |- + KeyGeneration represents the CephX key generation for the last successful reconcile. + For all newly-created resources, this field is set to `1`. + When keys are rotated due to any rotation policy, the generation is incremented or updated to + the configured policy generation. + Generation `0` indicates that keys existed prior to the implementation of key tracking. + format: int32 + type: integer + type: object rbdMirrorPeer: description: RBDMirrorPeer represents the cephx key rotation status of the `rbd-mirror-peer` user properties: diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index bc38e993a..5d9a74ee7 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -5994,6 +5994,28 @@ spec: format: int32 type: integer type: object + osd: + description: OSD shows the CephX key status of of OSDs + properties: + keyCephVersion: + description: |- + KeyCephVersion reports the Ceph version that created the current generation's keys. This is + same string format as reported by `CephCluster.status.version.version` to allow them to be + compared. E.g., `20.2.0-0`. + For all newly-created resources, this field set to the version of Ceph that created the key. + The special value "Uninitialized" indicates that keys are being created for the first time. + An empty string indicates that the version is unknown, as expected in brownfield deployments. + type: string + keyGeneration: + description: |- + KeyGeneration represents the CephX key generation for the last successful reconcile. + For all newly-created resources, this field is set to `1`. + When keys are rotated due to any rotation policy, the generation is incremented or updated to + the configured policy generation. + Generation `0` indicates that keys existed prior to the implementation of key tracking. + format: int32 + type: integer + type: object rbdMirrorPeer: description: RBDMirrorPeer represents the cephx key rotation status of the `rbd-mirror-peer` user properties: diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index d2e4ebf79..4c9ddac79 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -744,6 +744,8 @@ type LocalCephxStatus struct { type ClusterCephxStatus struct { // Mgr represents the cephx key rotation status of the ceph manager daemon Mgr *CephxStatus `json:"mgr,omitempty"` + // OSD shows the CephX key status of of OSDs + OSD *CephxStatus `json:"osd,omitempty"` // RBDMirrorPeer represents the cephx key rotation status of the `rbd-mirror-peer` user RBDMirrorPeer *CephxStatus `json:"rbdMirrorPeer,omitempty"` // CSI shows the CephX key status for Ceph-CSI components. diff --git a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go b/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go index 7c1ac02d3..19eba2120 100644 --- a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go +++ b/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go @@ -2080,6 +2080,11 @@ func (in *ClusterCephxStatus) DeepCopyInto(out *ClusterCephxStatus) { *out = new(CephxStatus) **out = **in } + if in.OSD != nil { + in, out := &in.OSD, &out.OSD + *out = new(CephxStatus) + **out = **in + } if in.RBDMirrorPeer != nil { in, out := &in.RBDMirrorPeer, &out.RBDMirrorPeer *out = new(CephxStatus) diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index ee9a238ab..e994f3f17 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/pkg/operator/ceph/cluster/cluster.go @@ -798,6 +798,7 @@ func (c *cluster) fetchSecretValue(selector v1.SecretKeySelector) (string, error func initClusterCephxStatus(c *clusterd.Context, cluster *cephv1.CephCluster) error { uninitializedStatus := keyring.UninitializedCephxStatus() cluster.Status.Cephx = &cephv1.ClusterCephxStatus{ + OSD: &uninitializedStatus, RBDMirrorPeer: &uninitializedStatus, Mgr: &uninitializedStatus, CSI: &cephv1.CephxStatusWithKeyCount{ diff --git a/pkg/operator/ceph/cluster/osd/create.go b/pkg/operator/ceph/cluster/osd/create.go index c06780c80..92120947f 100644 --- a/pkg/operator/ceph/cluster/osd/create.go +++ b/pkg/operator/ceph/cluster/osd/create.go @@ -24,6 +24,7 @@ import ( cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" kms "github.com/rook/rook/pkg/daemon/ceph/osd/kms" osdconfig "github.com/rook/rook/pkg/operator/ceph/cluster/osd/config" + "github.com/rook/rook/pkg/operator/ceph/config/keyring" opcontroller "github.com/rook/rook/pkg/operator/ceph/controller" "github.com/rook/rook/pkg/operator/k8sutil" v1 "k8s.io/api/core/v1" @@ -94,6 +95,12 @@ func (c *createConfig) createNewOSDsFromStatus( logger.Debugf("not creating deployment for OSD %d which already exists", osd.ID) continue } + + // osd prepare jobs don't generate cephx status info for OSDs. since this is a new OSD + // deployment, generate a first-deploy cephx status + status.OSDs[i].CephxStatus = keyring.UpdatedCephxStatus(false, c.cluster.spec.Security.CephX.Daemon, + c.cluster.clusterInfo.CephVersion, keyring.UninitializedCephxStatus()) + if status.PvcBackedOSD { logger.Infof("creating OSD %d on PVC %q", osd.ID, nodeOrPVCName) err := createDaemonOnPVCFunc(c.cluster, &status.OSDs[i], nodeOrPVCName, c.provisionConfig) diff --git a/pkg/operator/ceph/cluster/osd/osd.go b/pkg/operator/ceph/cluster/osd/osd.go index f352a9c86..e793af935 100644 --- a/pkg/operator/ceph/cluster/osd/osd.go +++ b/pkg/operator/ceph/cluster/osd/osd.go @@ -20,8 +20,8 @@ package osd import ( "bufio" "context" + "encoding/json" "fmt" - "reflect" "sort" "strconv" "strings" @@ -46,6 +46,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/util/retry" ) var ( @@ -75,6 +76,9 @@ const ( osdStore = "osd-store" deviceType = "device-type" encrypted = "encrypted" + + // CephxStatus is applied to each OSD deployment as value of this annotation key + cephxStatusAnnotationKey = "cephx-status" ) // Cluster keeps track of the OSDs @@ -119,12 +123,13 @@ type OSDInfo struct { CVMode string `json:"lv-mode"` Store string `json:"store"` // Ensure the OSD daemon has affinity with the same topology from the OSD prepare pod - TopologyAffinity string `json:"topologyAffinity"` - Encrypted bool `json:"encrypted"` - ExportService bool `json:"exportService"` - NodeName string `json:"nodeName"` - PVCName string `json:"pvcName"` - DeviceType string `json:"device-type"` + TopologyAffinity string `json:"topologyAffinity"` + Encrypted bool `json:"encrypted"` + ExportService bool `json:"exportService"` + NodeName string `json:"nodeName"` + PVCName string `json:"pvcName"` + DeviceType string `json:"device-type"` + CephxStatus cephv1.CephxStatus `json:"cephxStatus"` } // OrchestrationStatus represents the status of an OSD orchestration @@ -296,7 +301,7 @@ func (c *Cluster) Start() error { return errors.Wrap(err, "failed post reconcile of osd properties") } - err = c.updateCephStorageStatus() + err = c.updateCephOsdStorageStatus() if err != nil { return errors.Wrapf(err, "failed to update ceph storage status") } @@ -724,6 +729,15 @@ func (c *Cluster) getOSDInfo(d *appsv1.Deployment) (OSDInfo, error) { osd.PVCName = d.Labels[OSDOverPVCLabelKey] } + cephxStatus := cephv1.CephxStatus{} + cephxRaw, ok := d.Spec.Template.Annotations[cephxStatusAnnotationKey] + if ok { + if err := json.Unmarshal([]byte(cephxRaw), &cephxStatus); err != nil { + return OSDInfo{}, errors.Wrapf(err, "failed to unmarshal cephx status %q for deployment %q", cephxRaw, d.Name) + } + } + osd.CephxStatus = cephxStatus + return osd, nil } @@ -970,8 +984,7 @@ func (c *Cluster) waitForHealthyPGs() (bool, error) { return true, nil } -func (c *Cluster) updateCephStorageStatus() error { - cephCluster := cephv1.CephCluster{} +func (c *Cluster) updateCephOsdStorageStatus() error { cephClusterStorage := cephv1.CephStorage{} deviceClasses, err := cephclient.GetDeviceClasses(c.context, c.clusterInfo) @@ -983,7 +996,7 @@ func (c *Cluster) updateCephStorageStatus() error { cephClusterStorage.DeviceClasses = append(cephClusterStorage.DeviceClasses, cephv1.DeviceClasses{Name: deviceClass}) } - osdStore, err := c.getOSDStoreStatus() + osdStore, cephx, err := c.getOSDStoreStatus() if err != nil { return errors.Wrapf(err, "failed to get osd store status") } @@ -1002,44 +1015,79 @@ func (c *Cluster) updateCephStorageStatus() error { cephClusterStorage.OSD.MigrationStatus.Pending = len(migrationConfig.osds) } - err = c.context.Client.Get(c.clusterInfo.Context, c.clusterInfo.NamespacedName(), &cephCluster) - if err != nil { - if kerrors.IsNotFound(err) { - logger.Debug("CephCluster resource not found. Ignoring since object must be deleted.") - return nil + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + cephCluster := cephv1.CephCluster{} + err := c.context.Client.Get(c.clusterInfo.Context, c.clusterInfo.NamespacedName(), &cephCluster) + if err != nil { + if kerrors.IsNotFound(err) { + logger.Debug("CephCluster resource not found. Ignoring since object must be deleted.") + return nil + } + return errors.Wrapf(err, "failed to retrieve ceph cluster %q to update ceph Storage", c.clusterInfo.NamespacedName().Name) } - return errors.Wrapf(err, "failed to retrieve ceph cluster %q to update ceph Storage", c.clusterInfo.NamespacedName().Name) - } - if !reflect.DeepEqual(cephCluster.Status.CephStorage, cephClusterStorage) { + cephCluster.Status.CephStorage = &cephClusterStorage - if err := reporting.UpdateStatus(c.context.Client, &cephCluster); err != nil { - return errors.Wrapf(err, "failed to update cluster %q Storage.", c.clusterInfo.NamespacedName().Name) + + if cephx != nil { + if cephCluster.Status.Cephx == nil { + cephCluster.Status.Cephx = &cephv1.ClusterCephxStatus{} + } + cephCluster.Status.Cephx.OSD = cephx } + + if err := reporting.UpdateStatus(c.context.Client, &cephCluster); err != nil { + return errors.Wrapf(err, "failed to update cluster %q status", c.clusterInfo.NamespacedName().Name) + } + + return nil + }) + if err != nil { + return err } return nil } -func (c *Cluster) getOSDStoreStatus() (*cephv1.OSDStatus, error) { +func (c *Cluster) getOSDStoreStatus() (*cephv1.OSDStatus, *cephv1.CephxStatus, error) { label := fmt.Sprintf("%s=%s", k8sutil.AppAttr, AppName) osdDeployments, err := k8sutil.GetDeployments(c.clusterInfo.Context, c.context.Clientset, c.clusterInfo.Namespace, label) if err != nil { if kerrors.IsNotFound(err) { - return nil, nil + return nil, nil, nil } - return nil, errors.Wrap(err, "failed to get osd deployments") + return nil, nil, errors.Wrap(err, "failed to get osd deployments") } storeType := map[string]int{} + var minCephxStatus *cephv1.CephxStatus = nil for i := range osdDeployments.Items { - if osdStore, ok := osdDeployments.Items[i].Labels[osdStore]; ok { + d := osdDeployments.Items[i] + + if osdStore, ok := d.Labels[osdStore]; ok { storeType[osdStore]++ } + + // determine min cephx status on daemons + var cephxStatus cephv1.CephxStatus + cephxRaw, ok := d.Spec.Template.Annotations[cephxStatusAnnotationKey] + if !ok || cephxRaw == "" { + cephxStatus = cephv1.CephxStatus{} // no annotation or empty annotation means empty status + } else { + err := json.Unmarshal([]byte(cephxRaw), &cephxStatus) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to unmarshal cephx status %q for deployment %q", cephxRaw, d.Name) + } + } + if minCephxStatus == nil || cephxStatus.KeyGeneration < minCephxStatus.KeyGeneration { + // assume that min key generation will also give the right min ceph version + // may not be true in extreme corner cases, but it should be true almost all the time + minCephxStatus = &cephxStatus + } } return &cephv1.OSDStatus{ StoreType: storeType, - }, nil + }, minCephxStatus, nil } func getOSDLocationFromArgs(args []string) (string, bool) { diff --git a/pkg/operator/ceph/cluster/osd/osd_test.go b/pkg/operator/ceph/cluster/osd/osd_test.go index 78b34614c..291be518b 100644 --- a/pkg/operator/ceph/cluster/osd/osd_test.go +++ b/pkg/operator/ceph/cluster/osd/osd_test.go @@ -842,7 +842,7 @@ func TestUpdateCephStorageStatus(t *testing.T) { c := New(context, clusterInfo, cephCluster.Spec, "myversion") t.Run("verify ssd device class added to storage status", func(t *testing.T) { - err := c.updateCephStorageStatus() + err := c.updateCephOsdStorageStatus() assert.NoError(t, err) err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster) assert.NoError(t, err) @@ -868,7 +868,7 @@ func TestUpdateCephStorageStatus(t *testing.T) { if _, err := context.Clientset.AppsV1().Deployments(clusterInfo.Namespace).Create(ctx, deployment, metav1.CreateOptions{}); err != nil { logger.Errorf("Error creating fake deployment: %v", err) } - err := c.updateCephStorageStatus() + err := c.updateCephOsdStorageStatus() assert.NoError(t, err) err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster) assert.NoError(t, err) @@ -896,7 +896,7 @@ func TestUpdateCephStorageStatus(t *testing.T) { if _, err := context.Clientset.AppsV1().Deployments(clusterInfo.Namespace).Create(ctx, deployment, metav1.CreateOptions{}); err != nil { logger.Errorf("Error creating fake deployment: %v", err) } - err := c.updateCephStorageStatus() + err := c.updateCephOsdStorageStatus() assert.NoError(t, err) err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster) assert.NoError(t, err) @@ -907,6 +907,211 @@ func TestUpdateCephStorageStatus(t *testing.T) { }) } +func Test_updateCephOsdStorageStatus_cephx(t *testing.T) { + ctx := context.TODO() + clusterInfo := cephclient.AdminTestClusterInfo("fake") + executor := &exectest.MockExecutor{ + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { + logger.Infof("ExecuteCommandWithOutputFile: %s %v", command, args) + if args[1] == "crush" && args[2] == "class" && args[3] == "ls" { + // Mock executor for OSD crush class list command, returning ssd as available device class + return `["ssd"]`, nil + } + return "", nil + }, + } + + cephCluster := &cephv1.CephCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testing", + Namespace: "fake", + }, + Spec: cephv1.ClusterSpec{}, + } + // Objects to track in the fake client. + object := []runtime.Object{ + cephCluster, + } + s := scheme.Scheme + // Create a fake client to mock API calls. + client := clientfake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(object...).Build() + + context := &clusterd.Context{ + Executor: executor, + Client: client, + Clientset: testexec.New(t, 2), + } + + // Initializing an OSD monitoring + c := New(context, clusterInfo, cephCluster.Spec, "myversion") + + // Tests are not independent and rely on prior tests's changes + + t.Run("no osd deployments, no cephcluster status", func(t *testing.T) { + err := c.updateCephOsdStorageStatus() + assert.NoError(t, err) + err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster) + assert.NoError(t, err) + assert.Nil(t, cephCluster.Status.Cephx) + }) + + t.Run("unset cephx status, no cephcluster status", func(t *testing.T) { + deployment := &apps.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osd0", + Namespace: clusterInfo.Namespace, + Labels: map[string]string{ + k8sutil.AppAttr: AppName, + k8sutil.ClusterAttr: clusterInfo.Namespace, + OsdIdLabelKey: "0", + }, + }, + Spec: apps.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + }, + }, + } + _, err := context.Clientset.AppsV1().Deployments(clusterInfo.Namespace).Create(ctx, deployment, metav1.CreateOptions{}) + assert.NoError(t, err) + + err = c.updateCephOsdStorageStatus() + assert.NoError(t, err) + err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster) + assert.NoError(t, err) + assert.NotNil(t, cephCluster.Status.Cephx) + assert.Empty(t, cephCluster.Status.Cephx.OSD) + }) + + t.Run("empty string cephx status, no cephcluster status", func(t *testing.T) { + deployment := &apps.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osd0", + Namespace: clusterInfo.Namespace, + Labels: map[string]string{ + k8sutil.AppAttr: AppName, + k8sutil.ClusterAttr: clusterInfo.Namespace, + OsdIdLabelKey: "0", + }, + }, + Spec: apps.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + cephxStatusAnnotationKey: ``, + }, + }, + }, + }, + } + _, err := context.Clientset.AppsV1().Deployments(clusterInfo.Namespace).Update(ctx, deployment, metav1.UpdateOptions{}) + assert.NoError(t, err) + + err = c.updateCephOsdStorageStatus() + assert.NoError(t, err) + err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster) + assert.NoError(t, err) + assert.NotNil(t, cephCluster.Status.Cephx) + assert.Empty(t, cephCluster.Status.Cephx.OSD) + }) + + t.Run("gen 1 cephx status", func(t *testing.T) { + deployment := &apps.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osd0", + Namespace: clusterInfo.Namespace, + Labels: map[string]string{ + k8sutil.AppAttr: AppName, + k8sutil.ClusterAttr: clusterInfo.Namespace, + OsdIdLabelKey: "0", + }, + }, + Spec: apps.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + cephxStatusAnnotationKey: `{"keyGeneration": 1, "keyCephVersion": "v20"}`, + }, + }, + }, + }, + } + _, err := context.Clientset.AppsV1().Deployments(clusterInfo.Namespace).Update(ctx, deployment, metav1.UpdateOptions{}) + assert.NoError(t, err) + + err = c.updateCephOsdStorageStatus() + assert.NoError(t, err) + err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster) + assert.NoError(t, err) + assert.NotNil(t, cephCluster.Status.Cephx) + assert.Equal(t, cephv1.CephxStatus{KeyGeneration: 1, KeyCephVersion: "v20"}, *cephCluster.Status.Cephx.OSD) + }) + + t.Run("gen 1 and unset cephx status", func(t *testing.T) { + deployment := &apps.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osd1", + Namespace: clusterInfo.Namespace, + Labels: map[string]string{ + k8sutil.AppAttr: AppName, + k8sutil.ClusterAttr: clusterInfo.Namespace, + OsdIdLabelKey: "1", + }, + }, + Spec: apps.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + }, + }, + } + _, err := context.Clientset.AppsV1().Deployments(clusterInfo.Namespace).Create(ctx, deployment, metav1.CreateOptions{}) + assert.NoError(t, err) + + err = c.updateCephOsdStorageStatus() + assert.NoError(t, err) + err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster) + assert.NoError(t, err) + assert.NotNil(t, cephCluster.Status.Cephx) + assert.Equal(t, cephv1.CephxStatus{KeyGeneration: 0, KeyCephVersion: ""}, *cephCluster.Status.Cephx.OSD) + }) + + t.Run("gen 1 and gen 2 cephx status", func(t *testing.T) { + deployment := &apps.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osd1", + Namespace: clusterInfo.Namespace, + Labels: map[string]string{ + k8sutil.AppAttr: AppName, + k8sutil.ClusterAttr: clusterInfo.Namespace, + OsdIdLabelKey: "1", + }, + }, + Spec: apps.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + cephxStatusAnnotationKey: `{"keyGeneration": 2, "keyCephVersion": "v19"}`, + }, + }, + }, + }, + } + _, err := context.Clientset.AppsV1().Deployments(clusterInfo.Namespace).Update(ctx, deployment, metav1.UpdateOptions{}) + assert.NoError(t, err) + + err = c.updateCephOsdStorageStatus() + assert.NoError(t, err) + err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster) + assert.NoError(t, err) + assert.NotNil(t, cephCluster.Status.Cephx) + assert.Equal(t, cephv1.CephxStatus{KeyGeneration: 1, KeyCephVersion: "v20"}, *cephCluster.Status.Cephx.OSD) + }) +} + func TestGetOSDLocationFromArgs(t *testing.T) { args := []string{"--id", "2", "--crush-location=root=default host=minikube"} osdLocaiton, locationFound := getOSDLocationFromArgs(args) diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index e484a9df6..15984bbaf 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -18,6 +18,7 @@ limitations under the License. package osd import ( + "encoding/json" "fmt" "path" "path/filepath" @@ -641,11 +642,21 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd *OSDInfo, provision "", )) + // apply status to each OSD so its key isn't rotated again after a rotation is applied via + // daemon restart. important b/c osd update is often interrupted by reconcile requeues + cephxStatusVal, err := json.Marshal(osd.CephxStatus) + if err != nil { + return nil, errors.Wrapf(err, "failed to marshal OSD info CephX status %#v for OSD deployment %q", osd.CephxStatus, deploymentName) + } + envVars = append(envVars, controller.ApplyNetworkEnv(&c.spec)...) podTemplateSpec := v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Name: AppName, Labels: osdLabels, + Annotations: map[string]string{ + cephxStatusAnnotationKey: string(cephxStatusVal), + }, }, Spec: v1.PodSpec{ RestartPolicy: v1.RestartPolicyAlways, @@ -768,7 +779,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd *OSDInfo, provision cephv1.GetOSDLabels(c.spec.Labels).ApplyToObjectMeta(&deployment.ObjectMeta) cephv1.GetOSDLabels(c.spec.Labels).ApplyToObjectMeta(&deployment.Spec.Template.ObjectMeta) controller.AddCephVersionLabelToDeployment(c.clusterInfo.CephVersion, deployment) - err := c.clusterInfo.OwnerInfo.SetControllerReference(deployment) + err = c.clusterInfo.OwnerInfo.SetControllerReference(deployment) if err != nil { return nil, errors.Wrapf(err, "failed to set owner reference to osd deployment %q", deployment.Name) } diff --git a/pkg/operator/ceph/cluster/osd/spec_test.go b/pkg/operator/ceph/cluster/osd/spec_test.go index 2979220c2..eea2d9684 100644 --- a/pkg/operator/ceph/cluster/osd/spec_test.go +++ b/pkg/operator/ceph/cluster/osd/spec_test.go @@ -166,7 +166,7 @@ func testPodDevices(t *testing.T, dataDir, deviceName string, allDevices bool) { assert.Equal(t, AppName, deployment.Spec.Template.ObjectMeta.Name) assert.Equal(t, AppName, deployment.Spec.Template.ObjectMeta.Labels["app"]) assert.Equal(t, c.clusterInfo.Namespace, deployment.Spec.Template.ObjectMeta.Labels["rook_cluster"]) - assert.Equal(t, 1, len(deployment.Spec.Template.ObjectMeta.Annotations)) + assert.Equal(t, 2, len(deployment.Spec.Template.ObjectMeta.Annotations)) assert.Equal(t, 4, len(deployment.Spec.Template.Spec.InitContainers)) initCont := deployment.Spec.Template.Spec.InitContainers[0] diff --git a/pkg/operator/ceph/cluster/osd/update.go b/pkg/operator/ceph/cluster/osd/update.go index fe3282cf3..2cb3a2e50 100644 --- a/pkg/operator/ceph/cluster/osd/update.go +++ b/pkg/operator/ceph/cluster/osd/update.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" cephclient "github.com/rook/rook/pkg/daemon/ceph/client" + "github.com/rook/rook/pkg/operator/ceph/config/keyring" "github.com/rook/rook/pkg/operator/k8sutil" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" @@ -169,6 +170,13 @@ func (c *updateConfig) updateExistingOSDs(errs *provisionErrors) { continue } + cephxStatus, err := c.cluster.rotateCephxKey(osdInfo) + if err != nil { + // user-desired rotation failed, so report an error, but continue to try to update the OSD deployment + errs.addError("%v", errors.Wrapf(err, "failed to rotate cephx key for OSD %d", osdID)) + } + osdInfo.CephxStatus = cephxStatus // returned status is always correct + var updatedDep *appsv1.Deployment if c.cluster.spec.Network.MultiClusterService.Enabled { @@ -390,3 +398,35 @@ func (c *Cluster) getOSDDeployments() (*appsv1.DeploymentList, error) { } return deps, nil } + +// if needed, rotate cephx key for the OSD +// always returns the cephx status that should be applied to the OSD annotation, even in error case +func (c *Cluster) rotateCephxKey(osdInfo OSDInfo) (cephv1.CephxStatus, error) { + // TODO: for rotation WithCephVersionUpdate fix this to have the right runningCephVersion and desiredCephVersion + runningCephVersion := c.clusterInfo.CephVersion + desiredCephVersion := c.clusterInfo.CephVersion + shouldRotate, err := keyring.ShouldRotateCephxKeys(c.spec.Security.CephX.Daemon, + runningCephVersion, desiredCephVersion, osdInfo.CephxStatus) + if err != nil { + return osdInfo.CephxStatus, errors.Wrapf(err, "failed to determine if cephx key for OSD %d needs rotated", osdInfo.ID) + } + + didRotateCephxStatus := keyring.UpdatedCephxStatus(true, c.spec.Security.CephX.Daemon, + c.clusterInfo.CephVersion, osdInfo.CephxStatus) + didNotRotateCephxStatus := keyring.UpdatedCephxStatus(false, c.spec.Security.CephX.Daemon, + c.clusterInfo.CephVersion, osdInfo.CephxStatus) + + if !shouldRotate { + return didNotRotateCephxStatus, nil + } + + logger.Infof("rotating cephx key of OSD %d for CephCluster in namespace %q", osdInfo.ID, c.clusterInfo.Namespace) + user := fmt.Sprintf("osd.%d", osdInfo.ID) + // Note: OSD key is not stored in k8s secret; rotated key is picked up by OSD init container + _, err = cephclient.AuthRotate(c.context, c.clusterInfo, user) + if err != nil { + return didNotRotateCephxStatus, errors.Wrapf(err, "failed to rotate cephx key for OSD %d", osdInfo.ID) + } + + return didRotateCephxStatus, nil +} diff --git a/pkg/operator/ceph/cluster/osd/update_test.go b/pkg/operator/ceph/cluster/osd/update_test.go index 23f16518b..f0a6a7935 100644 --- a/pkg/operator/ceph/cluster/osd/update_test.go +++ b/pkg/operator/ceph/cluster/osd/update_test.go @@ -28,6 +28,7 @@ import ( "github.com/rook/rook/pkg/clusterd" cephclient "github.com/rook/rook/pkg/daemon/ceph/client" cephclientfake "github.com/rook/rook/pkg/daemon/ceph/client/fake" + "github.com/rook/rook/pkg/operator/ceph/config/keyring" "github.com/rook/rook/pkg/operator/ceph/controller" cephver "github.com/rook/rook/pkg/operator/ceph/version" "github.com/rook/rook/pkg/operator/k8sutil" @@ -779,3 +780,155 @@ func Test_existenceList(t *testing.T) { assert.True(t, l.Exists(1)) assert.Equal(t, 4, l.Len()) } + +func TestCluster_rotateCephxKey(t *testing.T) { + // auth rotate returns an array instead of a single object + rotatedKeyJson := `[{"key":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=="}]` + rotateCalledForEntity := "" + sharedExecutor := &exectest.MockExecutor{ + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { + t.Logf("command: %s %v", command, args) + if command == "ceph" && args[0] == "auth" && args[1] == "rotate" { + rotateCalledForEntity = args[2] + return rotatedKeyJson, nil + } + panic(fmt.Sprintf("unexpected command %q %v", command, args)) + }, + } + + newTest := func(daemonCephxConfig cephv1.CephxConfig) *Cluster { + rotateCalledForEntity = "" // reset to empty each test + + clusterd := clusterd.Context{ + Executor: sharedExecutor, + } + clusterInfo := cephclient.ClusterInfo{ + Context: context.TODO(), + Namespace: "ns", + CephVersion: cephver.CephVersion{Major: 20, Minor: 2}, + } + return &Cluster{ + context: &clusterd, + clusterInfo: &clusterInfo, + spec: cephv1.ClusterSpec{ + Security: cephv1.ClusterSecuritySpec{ + CephX: cephv1.ClusterCephxConfig{ + Daemon: daemonCephxConfig, + }, + }, + }, + } + } + + t.Run("empty status and config", func(t *testing.T) { + c := newTest(cephv1.CephxConfig{}) + osdInfo := OSDInfo{ + ID: 1, + CephxStatus: cephv1.CephxStatus{}, + } + + cephxStatus, err := c.rotateCephxKey(osdInfo) + assert.NoError(t, err) + assert.Equal(t, cephv1.CephxStatus{}, cephxStatus) + assert.Equal(t, rotateCalledForEntity, "") + }) + + t.Run("empty status, disabled config", func(t *testing.T) { // brownfield, no rotation + c := newTest(cephv1.CephxConfig{ + KeyRotationPolicy: "Disabled", + KeyGeneration: 3, + }) + osdInfo := OSDInfo{ + ID: 1, + CephxStatus: cephv1.CephxStatus{}, + } + + cephxStatus, err := c.rotateCephxKey(osdInfo) + assert.NoError(t, err) + assert.Equal(t, cephv1.CephxStatus{}, cephxStatus) + assert.Equal(t, rotateCalledForEntity, "") + }) + + t.Run("empty status, enabled config", func(t *testing.T) { // brownfield with rotation + c := newTest(cephv1.CephxConfig{ + KeyRotationPolicy: "KeyGeneration", + KeyGeneration: 3, + }) + osdInfo := OSDInfo{ + ID: 1, + CephxStatus: cephv1.CephxStatus{}, + } + + cephxStatus, err := c.rotateCephxKey(osdInfo) + assert.NoError(t, err) + assert.Equal(t, cephv1.CephxStatus{KeyCephVersion: "20.2.0-0", KeyGeneration: 3}, cephxStatus) + assert.Equal(t, rotateCalledForEntity, "osd.1") + }) + + t.Run("undefined status, empty config", func(t *testing.T) { // greenfield, no rotation + c := newTest(cephv1.CephxConfig{}) + osdInfo := OSDInfo{ + ID: 1, + CephxStatus: keyring.UninitializedCephxStatus(), // shouldn't happen in reality + } + + cephxStatus, err := c.rotateCephxKey(osdInfo) + assert.NoError(t, err) + // results in unnecessary OSD restart, but ensures that future rotations aren't blocked + // in the unlikely event the uninitialized status is erroneously applied to osd deployment + assert.Equal(t, cephv1.CephxStatus{KeyCephVersion: "20.2.0-0", KeyGeneration: 1}, cephxStatus) + assert.Equal(t, rotateCalledForEntity, "") + }) + + t.Run("set status, empty config", func(t *testing.T) { + c := newTest(cephv1.CephxConfig{}) + osdInfo := OSDInfo{ + ID: 1, + CephxStatus: cephv1.CephxStatus{KeyGeneration: 1, KeyCephVersion: "19.2.6-0"}, + } + + cephxStatus, err := c.rotateCephxKey(osdInfo) + assert.NoError(t, err) + assert.Equal(t, cephv1.CephxStatus{KeyCephVersion: "19.2.6-0", KeyGeneration: 1}, cephxStatus) + assert.Equal(t, rotateCalledForEntity, "") + }) + + t.Run("set status, enabled config", func(t *testing.T) { + c := newTest(cephv1.CephxConfig{ + KeyRotationPolicy: "KeyGeneration", + KeyGeneration: 4, + }) + osdInfo := OSDInfo{ + ID: 2, + CephxStatus: cephv1.CephxStatus{KeyGeneration: 1, KeyCephVersion: "19.2.6-0"}, + } + + cephxStatus, err := c.rotateCephxKey(osdInfo) + assert.NoError(t, err) + assert.Equal(t, cephv1.CephxStatus{KeyCephVersion: "20.2.0-0", KeyGeneration: 4}, cephxStatus) + assert.Equal(t, rotateCalledForEntity, "osd.2") + }) + + t.Run("auth rotate failure", func(t *testing.T) { + // this case shouldn't happen in code, but it could repair a botched greenfield deploy + c := newTest(cephv1.CephxConfig{ + KeyRotationPolicy: "KeyGeneration", + KeyGeneration: 4, + }) + c.context.Executor = &exectest.MockExecutor{ + MockExecuteCommandWithOutput: func(command string, arg ...string) (string, error) { + rotateCalledForEntity = arg[2] + return "", fmt.Errorf("mock error") + }, + } + osdInfo := OSDInfo{ + ID: 2, + CephxStatus: cephv1.CephxStatus{KeyGeneration: 1, KeyCephVersion: "19.2.6-0"}, + } + + cephxStatus, err := c.rotateCephxKey(osdInfo) + assert.Error(t, err) + assert.Equal(t, cephv1.CephxStatus{KeyCephVersion: "19.2.6-0", KeyGeneration: 1}, cephxStatus) + assert.Equal(t, rotateCalledForEntity, "osd.2") + }) +}