Skip to content
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

feat: Handle S3 region #574

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file.
- Run a `containerdebug` process in the background of each Hive container to collect debugging information ([#554]).
- Aggregate emitted Kubernetes events on the CustomResources ([#560]).
- Support configuring JVM arguments ([#572]).
- Support for S3 region ([#574]).

### Changed

Expand All @@ -27,6 +28,7 @@ All notable changes to this project will be documented in this file.
[#560]: https://github.com/stackabletech/hive-operator/pull/560
[#561]: https://github.com/stackabletech/hive-operator/pull/561
[#572]: https://github.com/stackabletech/hive-operator/pull/572
[#574]: https://github.com/stackabletech/hive-operator/pull/574

## [24.11.1] - 2025-01-10

Expand Down
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 9 additions & 9 deletions Cargo.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ repository = "https://github.com/stackabletech/hive-operator"

[workspace.dependencies]
stackable-versioned = { git = "https://github.com/stackabletech/operator-rs.git", features = ["k8s"], tag = "stackable-versioned-0.5.0" }
stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.85.0" }
stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.86.2" }
product-config = { git = "https://github.com/stackabletech/product-config.git", tag = "0.7.0" }

anyhow = "1.0"
Expand Down
6 changes: 3 additions & 3 deletions crate-hashes.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions deploy/helm/hive-operator/crds/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,32 @@ spec:
minimum: 0.0
nullable: true
type: integer
region:
default:
name: us-east-1
description: |-
AWS service API region used by the AWS SDK when using AWS S3 buckets.

This defaults to `us-east-1` and can be ignored if not using AWS S3 buckets.

NOTE: This is not the bucket region, and is used by the AWS SDK to construct endpoints for various AWS service APIs. It is only useful when using AWS S3 buckets.

When using AWS S3 buckets, you can configure optimal AWS service API connections in the following ways: - From **inside** AWS: Use an auto-discovery source (eg: AWS IMDS). - From **outside** AWS, or when IMDS is disabled, explicity set the region name nearest to where the client application is running from.
oneOf:
- required:
- source
- required:
- name
properties:
name:
description: 'An explicit region, eg: eu-central-1'
type: string
source:
description: Defer region detection to an auto-discovery mechanism.
enum:
- AwsImds
type: string
type: object
tls:
description: Use a TLS connection. If not specified no TLS will be used.
nullable: true
Expand Down
5 changes: 5 additions & 0 deletions rust/operator-binary/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,11 @@ fn build_metastore_rolegroup_config_map(
Some(s3.endpoint().context(ConfigureS3Snafu)?.to_string()),
);

data.insert(
MetaStoreConfig::S3_REGION_NAME.to_string(),
s3.region.name().map(String::from),
);

Comment on lines +628 to +632
Copy link
Member Author

@NickLarsenNZ NickLarsenNZ Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ok as is, since it is a BTreeMap<String, Option<String>> (ie, V = Option<_>). Or do we want to make it like this:

if let Some(region_name) = s3.region.name() {
  data.insert(
    MetaStoreConfig::S3_REGION_NAME.to_string(),
    Some(region_name.to_string()),
  );
}

if let Some((access_key_file, secret_key_file)) = s3.credentials_mount_paths() {
// Will be replaced by config-utils
data.insert(
Expand Down
1 change: 1 addition & 0 deletions rust/operator-binary/src/crd/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ impl MetaStoreConfig {
pub const METASTORE_WAREHOUSE_DIR: &'static str = "hive.metastore.warehouse.dir";
// S3
pub const S3_ENDPOINT: &'static str = "fs.s3a.endpoint";
pub const S3_REGION_NAME: &'static str = "fs.s3a.endpoint.region";
pub const S3_ACCESS_KEY: &'static str = "fs.s3a.access.key";
pub const S3_SECRET_KEY: &'static str = "fs.s3a.secret.key";
pub const S3_SSL_ENABLED: &'static str = "fs.s3a.connection.ssl.enabled";
Expand Down
Loading