fix(clp-package): Temporarily remove support for implicit AWS credent…

…ials. (#682) Co-authored-by: Kirk Rodrigues <[email protected]>
y-scope · Jan 21, 2025 · a8551a0 · a8551a0
1 parent ba63a76
commit a8551a0
Show file tree

Hide file tree

Showing 9 changed files with 28 additions and 33 deletions.
diff --git a/components/clp-package-utils/clp_package_utils/scripts/compress.py b/components/clp-package-utils/clp_package_utils/scripts/compress.py
@@ -129,7 +129,7 @@ def _generate_compress_cmd(
             aws_access_key_id, aws_secret_access_key = _parse_aws_credentials_file(
                 pathlib.Path(parsed_args.aws_credentials_file), default_credentials_user
             )
-        if aws_access_key_id and aws_secret_access_key:
+        if bool(aws_access_key_id) and bool(aws_secret_access_key):
             compress_cmd.append("--aws-access-key-id")
             compress_cmd.append(aws_access_key_id)
             compress_cmd.append("--aws-secret-access-key")
@@ -192,11 +192,11 @@ def _validate_s3_input_args(
                 " aws_secret_access_key."
             )
 
-    elif bool(aws_access_key_id) != bool(aws_secret_access_key):
-        args_parser.error(
-            "aws_access_key_id and aws_secret_access_key must be specified together or left"
-            " unspecified."
-        )
+    else:
+        if not bool(aws_access_key_id):
+            args_parser.error("aws_access_key_id not specified or empty")
+        if not bool(aws_secret_access_key):
+            args_parser.error("aws_secret_access_key not specified or empty")
 
 
 def main(argv):

diff --git a/components/clp-package-utils/clp_package_utils/scripts/native/compress.py b/components/clp-package-utils/clp_package_utils/scripts/native/compress.py
@@ -10,7 +10,7 @@
 
 import brotli
 import msgpack
-from clp_py_utils.clp_config import COMPRESSION_JOBS_TABLE_NAME
+from clp_py_utils.clp_config import COMPRESSION_JOBS_TABLE_NAME, S3Credentials
 from clp_py_utils.pretty_size import pretty_size
 from clp_py_utils.s3_utils import parse_s3_url
 from clp_py_utils.sql_adapter import SQL_Adapter
@@ -148,8 +148,10 @@ def _generate_clp_io_config(
             region_code=region_code,
             bucket=bucket_name,
             key_prefix=key_prefix,
-            aws_access_key_id=parsed_args.aws_access_key_id,
-            aws_secret_access_key=parsed_args.aws_secret_access_key,
+            credentials=S3Credentials(
+                access_key_id=parsed_args.aws_access_key_id,
+                secret_access_key=parsed_args.aws_secret_access_key,
+            ),
             timestamp_key=parsed_args.timestamp_key,
         )
     else:

diff --git a/components/clp-package-utils/clp_package_utils/scripts/start_clp.py b/components/clp-package-utils/clp_package_utils/scripts/start_clp.py
@@ -943,15 +943,14 @@ def start_log_viewer_webui(
         )
 
         access_key_id, secret_access_key = s3_config.get_credentials()
-        if access_key_id is not None and secret_access_key is not None:
-            container_cmd_extra_opts.extend(
-                [
-                    "-e",
-                    f"AWS_ACCESS_KEY_ID={access_key_id}",
-                    "-e",
-                    f"AWS_SECRET_ACCESS_KEY={secret_access_key}",
-                ]
+        container_cmd_extra_opts.extend(
+            (
+                "-e",
+                f"AWS_ACCESS_KEY_ID={access_key_id}",
+                "-e",
+                f"AWS_SECRET_ACCESS_KEY={secret_access_key}",
             )
+        )
 
     settings_json = read_and_update_settings_json(settings_json_path, settings_json_updates)
     with open(settings_json_path, "w") as settings_json_file:

diff --git a/components/clp-py-utils/clp_py_utils/clp_config.py b/components/clp-py-utils/clp_py_utils/clp_config.py
@@ -332,7 +332,7 @@ class S3Config(BaseModel):
     bucket: str
     key_prefix: str
 
-    credentials: Optional[S3Credentials]
+    credentials: S3Credentials
 
     @validator("region_code")
     def validate_region_code(cls, field):
@@ -354,9 +354,9 @@ def validate_key_prefix(cls, field):
             raise ValueError('key_prefix must end with "/"')
         return field
 
-    def get_credentials(self) -> Tuple[Optional[str], Optional[str]]:
-        if self.credentials is None:
-            return None, None
+    # TODO: When we support empty credentials, this method should be used to return a tuple that's
+    # either (None, None) if empty, or the credentials otherwise.
+    def get_credentials(self) -> Tuple[str, str]:
         return self.credentials.access_key_id, self.credentials.secret_access_key
 
 

diff --git a/components/clp-py-utils/clp_py_utils/s3_utils.py b/components/clp-py-utils/clp_py_utils/s3_utils.py
@@ -77,8 +77,8 @@ def s3_get_object_metadata(s3_input_config: S3InputConfig) -> List[FileMetadata]
     s3_client = boto3.client(
         "s3",
         region_name=s3_input_config.region_code,
-        aws_access_key_id=s3_input_config.aws_access_key_id,
-        aws_secret_access_key=s3_input_config.aws_secret_access_key,
+        aws_access_key_id=s3_input_config.credentials.access_key_id,
+        aws_secret_access_key=s3_input_config.credentials.secret_access_key,
     )
 
     file_metadata_list: List[FileMetadata] = list()

diff --git a/components/job-orchestration/job_orchestration/executor/compress/compression_task.py b/components/job-orchestration/job_orchestration/executor/compress/compression_task.py
@@ -187,8 +187,8 @@ def make_clp_s_command_and_env(
     if InputType.S3 == clp_config.input.type:
         compression_env_vars = {
             **os.environ,
-            "AWS_ACCESS_KEY_ID": clp_config.input.aws_access_key_id,
-            "AWS_SECRET_ACCESS_KEY": clp_config.input.aws_secret_access_key,
+            "AWS_ACCESS_KEY_ID": clp_config.input.credentials.access_key_id,
+            "AWS_SECRET_ACCESS_KEY": clp_config.input.credentials.secret_access_key,
         }
         compression_cmd.append("--auth")
         compression_cmd.append("s3")

diff --git a/components/job-orchestration/job_orchestration/executor/query/extract_stream_task.py b/components/job-orchestration/job_orchestration/executor/query/extract_stream_task.py
@@ -102,9 +102,6 @@ def _make_clp_s_command_and_env_vars(
         ))
         # fmt: on
         aws_access_key_id, aws_secret_access_key = s3_config.get_credentials()
-        if aws_access_key_id is None or aws_secret_access_key is None:
-            logger.error("Missing credentials for accessing archives on S3")
-            return None, None
         env_vars = {
             **os.environ,
             "AWS_ACCESS_KEY_ID": aws_access_key_id,

diff --git a/components/job-orchestration/job_orchestration/executor/query/fs_search_task.py b/components/job-orchestration/job_orchestration/executor/query/fs_search_task.py
@@ -72,9 +72,6 @@ def _make_core_clp_s_command_and_env_vars(
         ))
         # fmt: on
         aws_access_key_id, aws_secret_access_key = s3_config.get_credentials()
-        if aws_access_key_id is None or aws_secret_access_key is None:
-            logger.error("Missing credentials for accessing archives on S3")
-            return None, None
         env_vars = {
             **os.environ,
             "AWS_ACCESS_KEY_ID": aws_access_key_id,

diff --git a/components/job-orchestration/job_orchestration/scheduler/job_config.py b/components/job-orchestration/job_orchestration/scheduler/job_config.py
@@ -3,6 +3,7 @@
 import typing
 from enum import auto
 
+from clp_py_utils.clp_config import S3Credentials
 from pydantic import BaseModel, validator
 from strenum import LowercaseStrEnum
 
@@ -34,8 +35,7 @@ class S3InputConfig(BaseModel):
     bucket: str
     key_prefix: str
 
-    aws_access_key_id: typing.Optional[str] = None
-    aws_secret_access_key: typing.Optional[str] = None
+    credentials: S3Credentials
 
 
 class OutputConfig(BaseModel):