Skip to content

Commit

Permalink
refactor(meta): reject directly setting license key when it's managed…
Browse files Browse the repository at this point in the history
… by watching a file (#18823)

Signed-off-by: Bugen Zhao <[email protected]>
  • Loading branch information
BugenZhao authored Oct 11, 2024
1 parent 7fa1dff commit b01906e
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/meta/node/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
18 changes: 17 additions & 1 deletion src/meta/service/src/system_params_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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,
}
}
}
Expand All @@ -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?,
Expand Down
14 changes: 14 additions & 0 deletions src/meta/src/manager/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down

0 comments on commit b01906e

Please sign in to comment.