diff --git a/CHANGELOG.md b/CHANGELOG.md index e513faef..ab4d0bbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Allow overriding ZNode path by setting `status.znodePath` ([#799]). + ### Changed - Bump `built`, `clap`, `rstest`, `stackable-operator` and `strum` @@ -14,6 +18,7 @@ All notable changes to this project will be documented in this file. - Processing of corrupted log events fixed; If errors occur, the error messages are added to the log event ([#821]). +[#799]: https://github.com/stackabletech/zookeeper-operator/pull/799 [#812]: https://github.com/stackabletech/zookeeper-operator/pull/812 [#821]: https://github.com/stackabletech/zookeeper-operator/pull/821 diff --git a/deploy/helm/zookeeper-operator/crds/crds.yaml b/deploy/helm/zookeeper-operator/crds/crds.yaml index c349d32d..9dacc91d 100644 --- a/deploy/helm/zookeeper-operator/crds/crds.yaml +++ b/deploy/helm/zookeeper-operator/crds/crds.yaml @@ -7561,10 +7561,22 @@ spec: type: string type: object type: object + status: + nullable: true + properties: + znodePath: + description: |- + The absolute ZNode allocated to the ZookeeperZnode. This will typically be set by the operator. + + This can be set explicitly by an administrator, such as when restoring from a backup. + nullable: true + type: string + type: object required: - spec title: ZookeeperZnode type: object served: true storage: true - subresources: {} + subresources: + status: {} diff --git a/deploy/helm/zookeeper-operator/templates/roles.yaml b/deploy/helm/zookeeper-operator/templates/roles.yaml index 36d94ae8..a721c98f 100644 --- a/deploy/helm/zookeeper-operator/templates/roles.yaml +++ b/deploy/helm/zookeeper-operator/templates/roles.yaml @@ -120,6 +120,7 @@ rules: - {{ include "operator.name" . }}.stackable.tech resources: - {{ include "operator.name" . }}clusters/status + - {{ include "operator.name" . }}znodes/status verbs: - patch {{ if .Capabilities.APIVersions.Has "security.openshift.io/v1" }} diff --git a/docs/modules/zookeeper/pages/usage_guide/isolating_clients_with_znodes.adoc b/docs/modules/zookeeper/pages/usage_guide/isolating_clients_with_znodes.adoc index bd4da263..680760c1 100644 --- a/docs/modules/zookeeper/pages/usage_guide/isolating_clients_with_znodes.adoc +++ b/docs/modules/zookeeper/pages/usage_guide/isolating_clients_with_znodes.adoc @@ -100,3 +100,20 @@ The Stackable Operators for Kafka and Druid use the discovery ConfigMaps to conn == What's next You can find out more about the discovery ConfigMap xref:discovery.adoc[] and the xref:znodes.adoc[] in the concepts documentation. + +== Restoring from backups + +For security reasons, a unique ZNode path is generated every time the same ZookeeperZnode object is recreated, even if it has the same name. + +If a ZookeeperZnode needs to be associated with an existing ZNode path, the field `status.znodePath` can be set to +the desired path. Note that since this is a subfield of `status`, it must explicitly be updated on the `status` subresource, +and requires RBAC permissions to replace the `zookeeperznodes/status` resource. For example: + +[source,bash] +---- +kubectl get zookeeperznode/test-znode -o json -n $NAMESPACE \ + | jq '.status.znodePath = "/znode-override"' \ + | kubectl replace -f- --subresource=status +---- + +NOTE: The auto-generated ZNode will still be kept, and should be cleaned up by an administrator. diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 321ea669..17f196c2 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -466,16 +466,14 @@ impl ZookeeperCluster { "1.", "2.", "3.0.", "3.1.", "3.2.", "3.3.", "3.4.", "3.5.", "3.6.", "3.7.", ]; - let framework = if zookeeper_versions_with_log4j + if zookeeper_versions_with_log4j .into_iter() .any(|prefix| version.starts_with(prefix)) { LoggingFramework::LOG4J } else { LoggingFramework::LOGBACK - }; - - framework + } } /// The name of the role-level load-balanced Kubernetes `Service` @@ -695,6 +693,7 @@ impl ZookeeperPodRef { plural = "zookeeperznodes", shortname = "zno", shortname = "znode", + status = "ZookeeperZnodeStatus", namespaced, crates( kube_core = "stackable_operator::kube::core", @@ -709,6 +708,15 @@ pub struct ZookeeperZnodeSpec { pub cluster_ref: ClusterRef, } +#[derive(Clone, Default, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct ZookeeperZnodeStatus { + /// The absolute ZNode allocated to the ZookeeperZnode. This will typically be set by the operator. + /// + /// This can be set explicitly by an administrator, such as when restoring from a backup. + pub znode_path: Option, +} + #[cfg(test)] mod tests { use super::*; diff --git a/rust/operator-binary/src/znode_controller.rs b/rust/operator-binary/src/znode_controller.rs index a348e1eb..3d9dd483 100644 --- a/rust/operator-binary/src/znode_controller.rs +++ b/rust/operator-binary/src/znode_controller.rs @@ -1,7 +1,7 @@ //! Reconciles state for ZooKeeper znodes between Kubernetes [`ZookeeperZnode`] objects and the ZooKeeper cluster //! //! See [`ZookeeperZnode`] for more details. -use std::{convert::Infallible, sync::Arc}; +use std::{borrow::Cow, convert::Infallible, sync::Arc}; use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{ @@ -19,9 +19,11 @@ use stackable_operator::{ time::Duration, }; use stackable_zookeeper_crd::{ - security::ZookeeperSecurity, ZookeeperCluster, ZookeeperZnode, DOCKER_IMAGE_BASE_NAME, + security::ZookeeperSecurity, ZookeeperCluster, ZookeeperZnode, ZookeeperZnodeStatus, + DOCKER_IMAGE_BASE_NAME, }; use strum::{EnumDiscriminants, IntoStaticStr}; +use tracing::{debug, info}; use crate::{ discovery::{self, build_discovery_configmaps}, @@ -92,6 +94,11 @@ pub enum Error { cm: ObjectRef, }, + #[snafu(display("failed to update status"))] + ApplyStatus { + source: stackable_operator::client::Error, + }, + #[snafu(display("error managing finalizer"))] Finalizer { source: finalizer::Error, @@ -148,6 +155,7 @@ impl ReconcilerError for Error { Error::EnsureZnodeMissing { zk, .. } => Some(zk.clone().erase()), Error::BuildDiscoveryConfigMap { source: _ } => None, Error::ApplyDiscoveryConfigMap { cm, .. } => Some(cm.clone().erase()), + Error::ApplyStatus { .. } => None, Error::Finalizer { source: _ } => None, Error::DeleteOrphans { source: _ } => None, Error::ObjectHasNoNamespace => None, @@ -174,14 +182,40 @@ pub async fn reconcile_znode( let client = &ctx.client; let zk = find_zk_of_znode(client, &znode).await; - // Use the uid (managed by k8s itself) rather than the object name, to ensure that malicious users can't trick the controller - // into letting them take over a znode owned by someone else - let znode_path = format!("/znode-{}", uid); + let mut default_status_updates: Option = None; + // Store the znode path in the status rather than the object itself, to ensure that only K8s administrators can override it + let znode_path = match znode.status.as_ref().and_then(|s| s.znode_path.as_deref()) { + Some(znode_path) => { + debug!(znode.path = znode_path, "Using configured znode path"); + Cow::Borrowed(znode_path) + } + None => { + // Default to the uid (managed by k8s itself) rather than the object name, to ensure that malicious users can't trick the controller + // into letting them take over a znode owned by someone else + let znode_path = format!("/znode-{}", uid); + info!( + znode.path = znode_path, + "No znode path set, setting to default" + ); + default_status_updates + .get_or_insert_with(Default::default) + .znode_path = Some(znode_path.clone()); + Cow::Owned(znode_path) + } + }; + + if let Some(status) = default_status_updates { + info!("Writing default configuration to status"); + ctx.client + .merge_patch_status(&*znode, &status) + .await + .context(ApplyStatusSnafu)?; + } finalizer( &client.get_api::(&ns), &format!("{OPERATOR_NAME}/znode"), - znode, + znode.clone(), |ev| async { match ev { finalizer::Event::Apply(znode) => { diff --git a/tests/templates/kuttl/znode/01-assert.yaml b/tests/templates/kuttl/znode/01-assert.yaml new file mode 100644 index 00000000..10ad4256 --- /dev/null +++ b/tests/templates/kuttl/znode/01-assert.yaml @@ -0,0 +1,10 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-znode +data: + ZOOKEEPER_CHROOT: /znode-override diff --git a/tests/templates/kuttl/znode/01-set-znode-override.yaml b/tests/templates/kuttl/znode/01-set-znode-override.yaml new file mode 100644 index 00000000..f09089cd --- /dev/null +++ b/tests/templates/kuttl/znode/01-set-znode-override.yaml @@ -0,0 +1,5 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: kubectl get zookeeperznode/test-znode -o json -n $NAMESPACE | jq '.status.znodePath = "/znode-override"' | kubectl replace -f- --subresource=status