From b01906ec6fe3fe0d962cd02196c2c9c25583c13b Mon Sep 17 00:00:00 2001
From: Bugen Zhao <i@bugenzhao.com>
Date: Fri, 11 Oct 2024 12:48:18 +0800
Subject: [PATCH] refactor(meta): reject directly setting license key when it's
 managed by watching a file (#18823)

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
---
 src/meta/node/src/server.rs                   |  5 ++++-
 src/meta/service/src/system_params_service.rs | 18 +++++++++++++++++-
 src/meta/src/manager/env.rs                   | 14 ++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/meta/node/src/server.rs b/src/meta/node/src/server.rs
index 1d690cb176464..6d190db3b2efc 100644
--- a/src/meta/node/src/server.rs
+++ b/src/meta/node/src/server.rs
@@ -644,7 +644,10 @@ pub async fn start_service_as_election_leader(
     let health_srv = HealthServiceImpl::new();
     let backup_srv = BackupServiceImpl::new(backup_manager);
     let telemetry_srv = TelemetryInfoServiceImpl::new(env.meta_store());
-    let system_params_srv = SystemParamsServiceImpl::new(env.system_params_manager_impl_ref());
+    let system_params_srv = SystemParamsServiceImpl::new(
+        env.system_params_manager_impl_ref(),
+        env.opts.license_key_path.is_some(),
+    );
     let session_params_srv = SessionParamsServiceImpl::new(env.session_params_manager_impl_ref());
     let serving_srv =
         ServingServiceImpl::new(serving_vnode_mapping.clone(), metadata_manager.clone());
diff --git a/src/meta/service/src/system_params_service.rs b/src/meta/service/src/system_params_service.rs
index c2bd555be19de..128ae626a505e 100644
--- a/src/meta/service/src/system_params_service.rs
+++ b/src/meta/service/src/system_params_service.rs
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use async_trait::async_trait;
+use risingwave_common::system_param::LICENSE_KEY_KEY;
 use risingwave_meta::manager::SystemParamsManagerImpl;
 use risingwave_pb::meta::system_params_service_server::SystemParamsService;
 use risingwave_pb::meta::{
@@ -22,12 +23,16 @@ use tonic::{Request, Response, Status};
 
 pub struct SystemParamsServiceImpl {
     system_params_manager: SystemParamsManagerImpl,
+
+    /// Whether the license key is managed by license key file, i.e., `--license-key-path` is set.
+    managed_license_key: bool,
 }
 
 impl SystemParamsServiceImpl {
-    pub fn new(system_params_manager: SystemParamsManagerImpl) -> Self {
+    pub fn new(system_params_manager: SystemParamsManagerImpl, managed_license_key: bool) -> Self {
         Self {
             system_params_manager,
+            managed_license_key,
         }
     }
 }
@@ -53,6 +58,17 @@ impl SystemParamsService for SystemParamsServiceImpl {
         request: Request<SetSystemParamRequest>,
     ) -> Result<Response<SetSystemParamResponse>, Status> {
         let req = request.into_inner();
+
+        // When license key path is specified, license key from system parameters can be easily
+        // overwritten. So we simply reject this case.
+        if self.managed_license_key && req.param == LICENSE_KEY_KEY {
+            return Err(Status::permission_denied(
+                "cannot alter license key manually when \
+                argument `--license-key-path` (or env var `RW_LICENSE_KEY_PATH`) is set, \
+                please update the license key file instead",
+            ));
+        }
+
         let params = match &self.system_params_manager {
             SystemParamsManagerImpl::Kv(mgr) => mgr.set_param(&req.param, req.value).await?,
             SystemParamsManagerImpl::Sql(mgr) => mgr.set_param(&req.param, req.value).await?,
diff --git a/src/meta/src/manager/env.rs b/src/meta/src/manager/env.rs
index 7b30df609ee17..08d2696ab3873 100644
--- a/src/meta/src/manager/env.rs
+++ b/src/meta/src/manager/env.rs
@@ -21,6 +21,7 @@ use risingwave_common::config::{
 };
 use risingwave_common::session_config::SessionConfig;
 use risingwave_common::system_param::reader::SystemParamsReader;
+use risingwave_common::{bail, system_param};
 use risingwave_meta_model_migration::{MigrationStatus, Migrator, MigratorTrait};
 use risingwave_meta_model_v2::prelude::Cluster;
 use risingwave_pb::meta::SystemParams;
@@ -407,6 +408,19 @@ impl MetaSrvEnv {
             opts.event_log_channel_max_size,
         ));
 
+        // When license key path is specified, license key from system parameters can be easily
+        // overwritten. So we simply reject this case.
+        if opts.license_key_path.is_some()
+            && init_system_params.license_key
+                != system_param::default::license_key_opt().map(Into::into)
+        {
+            bail!(
+                "argument `--license-key-path` (or env var `RW_LICENSE_KEY_PATH`) and \
+                 system parameter `license_key` (or env var `RW_LICENSE_KEY`) may not \
+                 be set at the same time"
+            );
+        }
+
         let env = match &meta_store_impl {
             MetaStoreImpl::Kv(meta_store) => {
                 let notification_manager =