Repository navigation
feat(clp-package)!: Refactor S3 log-ingestion configuration to use S3 URL as compression script argument (fixes #842). #852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
40edccb
0217d9d
7bee632
6efc499
d435bf0
10761c4
8c53605
16d8fa7
5b66f4c
7c19855
d43c0de
80dc43a
32116d8
9767601
97c5579
fbcf646
f38a6cd
2e0e23d
3eabef2
73957fc
e8a456b
1bd7aa4
a4206e5
e4f6b21
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import os | ||
| import re | ||
| from pathlib import Path | ||
| from typing import Dict, List, Optional, Tuple, Union | ||
|
|
||
|
|
@@ -7,6 +8,7 @@ | |
| from job_orchestration.scheduler.job_config import S3InputConfig | ||
|
|
||
| from clp_py_utils.clp_config import ( | ||
| AwsAuthentication, | ||
| AwsAuthType, | ||
| CLPConfig, | ||
| COMPRESSION_SCHEDULER_COMPONENT_NAME, | ||
|
|
@@ -51,16 +53,14 @@ def _get_session_credentials(aws_profile: Optional[str] = None) -> Optional[S3Cr | |
| ) | ||
|
|
||
|
|
||
| def get_credential_env_vars(config: S3Config) -> Dict[str, str]: | ||
| def get_credential_env_vars(auth: AwsAuthentication) -> Dict[str, str]: | ||
| """ | ||
| Generates AWS credential environment variables for tasks. | ||
| :param config: S3Config or S3InputConfig from which to retrieve credentials. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to update the docstring as well.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
| :return: A [str, str] Dict which access key pair and session token if applicable. | ||
| :raise: ValueError if auth type is not supported | ||
| """ | ||
| env_vars: Optional[Dict[str, str]] = None | ||
| auth = config.aws_authentication | ||
|
|
||
| aws_credentials: Optional[S3Credentials] = None | ||
|
|
||
| if AwsAuthType.env_vars == auth.type: | ||
|
|
@@ -104,36 +104,46 @@ def generate_container_auth_options( | |
| :return: Tuple of (whether aws config mount is needed, credential env_vars to set). | ||
| :raises: ValueError if environment variables are not set correctly. | ||
| """ | ||
| storages_by_component_type: List[Union[S3Storage, FsStorage]] = [] | ||
| output_storages_by_component_type: List[Union[S3Storage, FsStorage]] = [] | ||
| input_storage_needed = False | ||
|
|
||
| if component_type in ( | ||
| COMPRESSION_SCHEDULER_COMPONENT_NAME, | ||
| COMPRESSION_WORKER_COMPONENT_NAME, | ||
| ): | ||
| storages_by_component_type = [clp_config.logs_input, clp_config.archive_output.storage] | ||
| output_storages_by_component_type = [clp_config.archive_output.storage] | ||
| input_storage_needed = True | ||
| elif component_type in (LOG_VIEWER_WEBUI_COMPONENT_NAME,): | ||
| storages_by_component_type = [clp_config.stream_output.storage] | ||
| output_storages_by_component_type = [clp_config.stream_output.storage] | ||
| elif component_type in ( | ||
| QUERY_SCHEDULER_COMPONENT_NAME, | ||
| QUERY_WORKER_COMPONENT_NAME, | ||
| ): | ||
| storages_by_component_type = [ | ||
| clp_config.logs_input, | ||
| output_storages_by_component_type = [ | ||
| clp_config.archive_output.storage, | ||
| clp_config.stream_output.storage, | ||
| ] | ||
| input_storage_needed = True | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why's this true for the query scheduler & worker?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for catching this. looks like a mistake made in previous PR. |
||
| else: | ||
| raise ValueError(f"Component type {component_type} is not valid.") | ||
| config_mount = False | ||
| add_env_vars = False | ||
|
|
||
| for storage in storages_by_component_type: | ||
| for storage in output_storages_by_component_type: | ||
| if StorageType.S3 == storage.type: | ||
| auth = storage.s3_config.aws_authentication | ||
| if AwsAuthType.profile == auth.type: | ||
| config_mount = True | ||
| elif AwsAuthType.env_vars == auth.type: | ||
| add_env_vars = True | ||
|
|
||
| if input_storage_needed and StorageType.S3 == clp_config.logs_input.type: | ||
| auth = clp_config.logs_input.aws_authentication | ||
| if AwsAuthType.profile == auth.type: | ||
| config_mount = True | ||
| elif AwsAuthType.env_vars == auth.type: | ||
| add_env_vars = True | ||
|
|
||
| credentials_env_vars = [] | ||
|
|
||
| if add_env_vars: | ||
|
|
@@ -186,6 +196,43 @@ def _create_s3_client(s3_config: S3Config, boto3_config: Optional[Config] = None | |
| return s3_client | ||
|
|
||
|
|
||
| def parse_s3_url(s3_url: str) -> Tuple[str, str, str]: | ||
| """ | ||
| Parses the region_code, bucket, and key_prefix from the given S3 URL. | ||
| :param s3_url: A host-style URL or path-style URL. | ||
| :return: A tuple of (region_code, bucket, key_prefix). | ||
| :raise: ValueError if `s3_url` is not a valid host-style URL or path-style URL. | ||
| """ | ||
|
|
||
| host_style_url_regex = re.compile( | ||
| r"https://(?P<bucket_name>[a-z0-9.-]+)\.s3(\.(?P<region_code>[a-z]+-[a-z]+-[0-9]))?" | ||
|
haiqi96 marked this conversation as resolved.
Outdated
|
||
| r"\.(?P<endpoint>[a-z0-9.-]+)/(?P<key_prefix>[^?]+).*" | ||
| ) | ||
|
Eden-D-Zhang marked this conversation as resolved.
|
||
| match = host_style_url_regex.match(s3_url) | ||
|
|
||
| if match is None: | ||
| path_style_url_regex = re.compile( | ||
| r"https://s3(\.(?P<region_code>[a-z]+-[a-z]+-[0-9]))?\.(?P<endpoint>[a-z0-9.-]+)/" | ||
|
haiqi96 marked this conversation as resolved.
Outdated
|
||
| r"(?P<bucket_name>[a-z0-9.-]+)/(?P<key_prefix>[^?]+).*" | ||
| ) | ||
|
Eden-D-Zhang marked this conversation as resolved.
|
||
| match = path_style_url_regex.match(s3_url) | ||
|
|
||
| if match is None: | ||
| raise ValueError(f"Unsupported URL format: {s3_url}") | ||
|
|
||
| region_code = match.group("region_code") | ||
| if region_code is None: | ||
| region_code = "us-east-1" | ||
|
haiqi96 marked this conversation as resolved.
Outdated
|
||
| bucket_name = match.group("bucket_name") | ||
| endpoint = match.group("endpoint") | ||
| key_prefix = match.group("key_prefix") | ||
|
|
||
| if AWS_ENDPOINT != endpoint: | ||
| raise ValueError(f"Unsupported endpoint: {endpoint}") | ||
|
|
||
| return region_code, bucket_name, key_prefix | ||
|
|
||
|
haiqi96 marked this conversation as resolved.
|
||
|
|
||
| def generate_s3_virtual_hosted_style_url( | ||
| region_code: str, bucket_name: str, object_key: str | ||
| ) -> str: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this case possible for InputType.FS as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically yes, added check for FS flow