Skip to content

Commit

Permalink
[blob-service] Make S3 bucket configurable
Browse files Browse the repository at this point in the history
Summary: Part of [[ https://linear.app/comm/issue/ENG-4573/make-blob-service-s3-bucket-name-configurable | ENG-4573 ]]. Added a new `--s3-bucket-name` CLI option and `BLOB_S3_BUCKET_NAME` environment variable that allows to override the default `commapp-blob` S3 bucket. We need this to be able to use a separate bucket in staging environment.

Test Plan: Ran integration tests locally - confirmed that default S3 bucket is used. Ran integration tests with `--s3-bucket-name` set to non-existent bucket - they fail with `NoSuchBucketError` as expected. Repeated with the environment variable set instead.

Reviewers: jon, varun, michal

Reviewed By: jon

Subscribers: ashoat, tomek

Differential Revision: https://phab.comm.dev/D8742
  • Loading branch information
barthap committed Aug 8, 2023
1 parent 83836ec commit f43e384
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 10 deletions.
10 changes: 9 additions & 1 deletion services/blob/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use once_cell::sync::Lazy;
use tracing::info;

use crate::constants::{
DEFAULT_GRPC_PORT, DEFAULT_HTTP_PORT, LOCALSTACK_URL, SANDBOX_ENV_VAR,
DEFAULT_GRPC_PORT, DEFAULT_HTTP_PORT, DEFAULT_S3_BUCKET_NAME, LOCALSTACK_URL,
S3_BUCKET_ENV_VAR, SANDBOX_ENV_VAR,
};

#[derive(Parser)]
Expand All @@ -25,6 +26,9 @@ pub struct AppConfig {
/// AWS Localstack service URL, applicable in sandbox mode
#[arg(long, default_value_t = LOCALSTACK_URL.to_string())]
pub localstack_url: String,
#[arg(env = S3_BUCKET_ENV_VAR)]
#[arg(long, default_value_t = DEFAULT_S3_BUCKET_NAME.to_string())]
pub s3_bucket_name: String,
}

/// Stores configuration parsed from command-line arguments
Expand All @@ -43,6 +47,10 @@ pub(super) fn parse_cmdline_args() -> Result<()> {
"gRPC and HTTP ports cannot be the same: {}",
cfg.grpc_port
);

if cfg.s3_bucket_name != DEFAULT_S3_BUCKET_NAME {
info!("Using custom S3 bucket: {}", &cfg.s3_bucket_name);
}
Ok(())
}

Expand Down
3 changes: 2 additions & 1 deletion services/blob/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,6 @@ pub const LOG_LEVEL_ENV_VAR: &str =

// S3 constants

pub const BLOB_S3_BUCKET_NAME: &str = "commapp-blob";
pub const S3_BUCKET_ENV_VAR: &str = "BLOB_S3_BUCKET_NAME";
pub const DEFAULT_S3_BUCKET_NAME: &str = "commapp-blob";
pub const S3_MULTIPART_UPLOAD_MINIMUM_CHUNK_SIZE: u64 = 5 * 1024 * 1024;
7 changes: 4 additions & 3 deletions services/blob/src/database/old.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ use std::{collections::HashMap, sync::Arc};
use tracing::error;

use crate::{
config::CONFIG,
constants::{
BLOB_REVERSE_INDEX_TABLE_BLOB_HASH_FIELD,
BLOB_REVERSE_INDEX_TABLE_HASH_INDEX_NAME,
BLOB_REVERSE_INDEX_TABLE_HOLDER_FIELD, BLOB_REVERSE_INDEX_TABLE_NAME,
BLOB_S3_BUCKET_NAME, BLOB_TABLE_BLOB_HASH_FIELD, BLOB_TABLE_CREATED_FIELD,
BLOB_TABLE_NAME, BLOB_TABLE_S3_PATH_FIELD,
BLOB_TABLE_BLOB_HASH_FIELD, BLOB_TABLE_CREATED_FIELD, BLOB_TABLE_NAME,
BLOB_TABLE_S3_PATH_FIELD,
},
s3::S3Path,
};
Expand All @@ -32,7 +33,7 @@ impl BlobItem {
BlobItem {
blob_hash: hash_str.clone(),
s3_path: S3Path {
bucket_name: BLOB_S3_BUCKET_NAME.to_string(),
bucket_name: CONFIG.s3_bucket_name.clone(),
object_name: hash_str,
},
created: Utc::now(),
Expand Down
7 changes: 2 additions & 5 deletions services/blob/src/database/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ use comm_services_lib::database::{
use derive_more::Constructor;
use std::collections::HashMap;

use crate::{
constants::{db::*, BLOB_S3_BUCKET_NAME},
s3::S3Path,
};
use crate::{config::CONFIG, constants::db::*, s3::S3Path};

use super::errors::Error as DBError;

Expand Down Expand Up @@ -58,7 +55,7 @@ impl BlobItemInput {
BlobItemInput {
blob_hash: blob_hash.clone(),
s3_path: S3Path {
bucket_name: BLOB_S3_BUCKET_NAME.into(),
bucket_name: CONFIG.s3_bucket_name.clone(),
object_name: blob_hash,
},
}
Expand Down

0 comments on commit f43e384

Please sign in to comment.