Update SSE documentation and remove the feature flag (#839)

* Update documentation, remove the feature flag Signed-off-by: Vladislav Volodkin <[email protected]> * Remove the sse_kms feature flag from the CI Signed-off-by: Vladislav Volodkin <[email protected]> * Update doc/CONFIGURATION.md Co-authored-by: Daniel Carl Jones <[email protected]> Signed-off-by: Volodkin Vladislav <[email protected]> * Remove the feature flag Signed-off-by: Vladislav Volodkin <[email protected]> --------- Signed-off-by: Vladislav Volodkin <[email protected]> Signed-off-by: Volodkin Vladislav <[email protected]> Co-authored-by: Vladislav Volodkin <[email protected]> Co-authored-by: Daniel Carl Jones <[email protected]>
awslabs · Apr 10, 2024 · 8af2fb2 · 8af2fb2
1 parent cf5fc24
commit 8af2fb2
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 22 deletions.
diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md
@@ -158,7 +158,9 @@ If necessary, you can use the `--endpoint-url` command-line argument to fully ov
 
 ### Data encryption
 
-Amazon S3 supports a number of [server-side encryption types](https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingEncryption.html). Mountpoint supports reading and writing to buckets that are configured with Amazon S3 managed keys (SSE-S3), with AWS KMS keys (SSE-KMS), or with dual-layer encryption with AWS KMS keys (DSSE-KMS) as the default encryption method. It does not currently support reading objects encrypted with customer-provided keys (SSE-C). Mountpoint does not allow further configuring encryption, and you cannot encrypt new objects written with Mountpoint using a different encryption setting than the bucket's default.
+Amazon S3 supports a number of [server-side encryption types](https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingEncryption.html). Mountpoint supports reading and writing to buckets that are configured with Amazon S3 managed keys (SSE-S3), with AWS KMS keys (SSE-KMS), or with dual-layer encryption with AWS KMS keys (DSSE-KMS) as the default encryption method. It does not currently support reading objects encrypted with customer-provided keys (SSE-C).
+
+New objects can be uploaded using different server-side encryption (SSE) settings than the bucket's default. The CLI argument `--sse <aws:kms|aws:kms:dsse|AES256>` can be used to specify a different SSE encryption type. When either `aws:kms` or `aws:kms:dsse` is used as a type, `--sse-kms-key-id <KEY_ARN>` may be used to optionally specify a KMS key ID. When a KMS key ID is not specified, S3 will use an [AWS managed KMS key](https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html#key-mgmt), which is created automatically. Please note that these command-line arguments only configure server-side encryption for *new* objects created with Mountpoint, all *existing* objects will remain unchanged.
 
 Mountpoint does not support client-side encryption using the Amazon S3 Encryption Client.
 
@@ -184,7 +186,7 @@ If you want to allow file deletion, use the `--allow-delete` flag at mount time.
 
 If you want to forbid all mutating actions on your S3 bucket via Mountpoint, use the `--read-only` command-line flag.
 
-For more details on the behavior of file operations with Mountpoint, see the [file operations section](https://github.com/awslabs/mountpoint-s3/blob/main/doc/SEMANTICS.md#file-operations) of the semantics documentation for more information. 
+For more details on the behavior of file operations with Mountpoint, see the [file operations section](https://github.com/awslabs/mountpoint-s3/blob/main/doc/SEMANTICS.md#file-operations) of the semantics documentation for more information.
 
 ### S3 storage classes
 
@@ -278,7 +280,7 @@ You can instead manually configure the maximum size of the cache with the `--max
 
 We recommend using local storage, such as Amazon EC2 instance storage or an Amazon EBS volume, as the target of the Mountpoint cache.
 When caching to EBS, you can use your instance's root EBS volume, or create and attach a new volume just for caching.
-There are several factors that can affect the performance of EBS volumes. See the [EBS documentation](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-volume-types.html) for more details about EBS volume types and their performance characteristics. 
+There are several factors that can affect the performance of EBS volumes. See the [EBS documentation](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-volume-types.html) for more details about EBS volume types and their performance characteristics.
 If you create a new EBS volume or use EC2 instance storage, you will first need to [create a file system](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/add-instance-store-volumes.html#making-instance-stores-available-on-your-instances) on that storage and mount it at a path such as `/mnt/mp-cache`.
 The user running Mountpoint needs write access to the mounted file system,
 and we recommend setting the permissions on the file system to not allow reads by any other users (e.g., `chmod 0700 /mnt/mp-cache`).

diff --git a/mountpoint-s3/Cargo.toml b/mountpoint-s3/Cargo.toml
@@ -77,7 +77,6 @@ built = { version = "0.7.1", features = ["git2"] }
 [features]
 # Unreleased feature flags
 negative_cache = []
-sse_kms = []
 # Features for choosing tests
 fips_tests = []
 fuse_tests = []

diff --git a/mountpoint-s3/src/cli.rs b/mountpoint-s3/src/cli.rs
@@ -27,7 +27,6 @@ use regex::Regex;
 
 use crate::build_info;
 use crate::data_cache::{CacheLimit, DiskDataCache, DiskDataCacheConfig, ManagedCacheDir};
-#[cfg(feature = "sse_kms")]
 use crate::fs::ServerSideEncryption;
 use crate::fs::{CacheConfig, S3FilesystemConfig, S3Personality};
 use crate::fuse::session::FuseSession;
@@ -272,15 +271,13 @@ pub struct CliArgs {
     )]
     pub user_agent_prefix: Option<String>,
 
-    #[cfg(feature = "sse_kms")]
     #[clap(
         long,
         help = "Server-side encryption algorithm to use when uploading new objects",
         help_heading = BUCKET_OPTIONS_HEADER,
         value_parser = clap::builder::PossibleValuesParser::new(["aws:kms", "aws:kms:dsse", "AES256"]))]
     pub sse: Option<String>,
 
-    #[cfg(feature = "sse_kms")]
     #[clap(
         long,
         help = "AWS Key Management Service (KMS) key ID to use with KMS server-side encryption when uploading new objects",
@@ -610,7 +607,6 @@ where
     tracing::debug!("{:?}", args);
 
     validate_mount_point(&args.mount_point)?;
-    #[cfg(feature = "sse_kms")]
     {
         validate_sse_args(args.sse.as_deref(), args.sse_kms_key_id.as_deref())?;
     }
@@ -637,7 +633,6 @@ where
     filesystem_config.allow_delete = args.allow_delete;
     filesystem_config.allow_overwrite = args.allow_overwrite;
     filesystem_config.s3_personality = s3_personality;
-    #[cfg(feature = "sse_kms")]
     {
         filesystem_config.server_side_encryption = ServerSideEncryption::new(args.sse, args.sse_kms_key_id);
     }
@@ -929,7 +924,6 @@ fn validate_mount_point(path: impl AsRef<Path>) -> anyhow::Result<()> {
 /// Disallow specifying `--sse-kms-key-id` when `--sse=AES256` as this is not allowed by the S3 API.
 /// We are not able to perform this check via clap API (the closest it has is `conflicts_with` method),
 /// thus having a custom validation.
-#[cfg(feature = "sse_kms")]
 fn validate_sse_args(sse_type: Option<&str>, sse_kms_key_id: Option<&str>) -> anyhow::Result<()> {
     if sse_kms_key_id.is_some() && sse_type == Some("AES256") {
         Err(anyhow!("--sse-kms-key-id can not be used with --sse AES256"))

diff --git a/mountpoint-s3/tests/cli.rs b/mountpoint-s3/tests/cli.rs
@@ -249,7 +249,6 @@ fn invalid_ttl() -> Result<(), Box<dyn std::error::Error>> {
     Ok(())
 }
 
-#[cfg(feature = "sse_kms")]
 #[test]
 fn sse_args_non_empty() -> Result<(), Box<dyn std::error::Error>> {
     let dir = assert_fs::TempDir::new()?;
@@ -269,7 +268,6 @@ fn sse_args_non_empty() -> Result<(), Box<dyn std::error::Error>> {
     Ok(())
 }
 
-#[cfg(feature = "sse_kms")]
 #[test]
 fn sse_key_not_allowed_with_aes256() -> Result<(), Box<dyn std::error::Error>> {
     let dir = assert_fs::TempDir::new()?;

diff --git a/mountpoint-s3/tests/fuse_tests/fork_test.rs b/mountpoint-s3/tests/fuse_tests/fork_test.rs
@@ -4,13 +4,13 @@
 use assert_cmd::prelude::*;
 #[cfg(not(feature = "s3express_tests"))]
 use aws_config::BehaviorVersion;
-#[cfg(all(feature = "sse_kms", not(feature = "s3express_tests")))]
+#[cfg(not(feature = "s3express_tests"))]
 use aws_sdk_s3::primitives::ByteStream;
 #[cfg(not(feature = "s3express_tests"))]
 use aws_sdk_sts::config::Region;
 use std::fs;
 use std::io::{BufRead, BufReader};
-#[cfg(all(feature = "sse_kms", not(feature = "s3express_tests")))]
+#[cfg(not(feature = "s3express_tests"))]
 use std::io::{Read, Write};
 use std::path::Path;
 use std::process::{Child, ExitStatus, Stdio};
@@ -20,7 +20,7 @@ use test_case::test_case;
 
 use crate::common::fuse::read_dir_to_entry_names;
 use crate::common::s3::{create_objects, get_test_bucket_and_prefix, get_test_bucket_forbidden, get_test_region};
-#[cfg(all(feature = "sse_kms", not(feature = "s3express_tests")))]
+#[cfg(not(feature = "s3express_tests"))]
 use crate::common::s3::{get_scoped_down_credentials, get_test_kms_key_id, get_test_sdk_client};
 #[cfg(not(feature = "s3express_tests"))]
 use crate::common::s3::{get_subsession_iam_role, tokio_block_on};
@@ -392,7 +392,7 @@ fn mount_scoped_credentials() -> Result<(), Box<dyn std::error::Error>> {
     Ok(())
 }
 
-#[cfg(all(feature = "sse_kms", not(feature = "s3express_tests")))]
+#[cfg(not(feature = "s3express_tests"))]
 fn mount_with_sse(
     bucket: &str,
     mount_point: &Path,
@@ -422,15 +422,14 @@ fn mount_with_sse(
     wait_for_mount("mountpoint-s3", mount_point.to_str().unwrap());
     child
 }
-
-#[cfg(all(feature = "sse_kms", not(feature = "s3express_tests")))]
+#[cfg(not(feature = "s3express_tests"))]
 fn write_to_file(mount_point: &Path, file_name: &str) -> Result<(), std::io::Error> {
     let mut f = fs::File::create(mount_point.join(file_name)).expect("should be able to open file for writing");
     let data = vec![0xaa; 32];
     f.write_all(&data)
 }
 
-#[cfg(all(feature = "sse_kms", not(feature = "s3express_tests")))]
+#[cfg(not(feature = "s3express_tests"))]
 #[test]
 fn write_with_inexistent_key_sse() {
     let (bucket, prefix) = get_test_bucket_and_prefix("write_with_inexistent_key_sse");
@@ -444,7 +443,7 @@ fn write_with_inexistent_key_sse() {
     unmount_and_check_log(child, mount_point.path(), &expected_log_line);
 }
 
-#[cfg(all(feature = "sse_kms", not(feature = "s3express_tests")))]
+#[cfg(not(feature = "s3express_tests"))]
 #[test]
 fn write_with_no_permissions_for_a_key_sse() {
     let policy_with_no_kms_perms = r#"{"Statement": [
@@ -463,7 +462,7 @@ fn write_with_no_permissions_for_a_key_sse() {
     unmount_and_check_log(child, mount_point.path(), &expected_log_line);
 }
 
-#[cfg(all(feature = "sse_kms", not(feature = "s3express_tests")))]
+#[cfg(not(feature = "s3express_tests"))]
 #[test]
 fn read_with_no_permissions_for_a_key_sse() {
     let policy_with_no_kms_perms = r#"{"Statement": [
@@ -624,7 +623,7 @@ fn unmount(mount_point: &Path) {
     panic!("failed to unmount");
 }
 
-#[cfg(all(feature = "sse_kms", not(feature = "s3express_tests")))]
+#[cfg(not(feature = "s3express_tests"))]
 fn unmount_and_check_log(mut process: Child, mount_path: &Path, expected_log_line: &regex::Regex) {
     unmount(mount_path);
     let mut stdout = process