Skip to content

Treat invalid HTTP S3 endpoint overrides as a startup configuration error#2858

Merged
pcholakov merged 2 commits intomainfrom
chore/support-global-aws-env-defaults
Mar 11, 2025
Merged

Treat invalid HTTP S3 endpoint overrides as a startup configuration error#2858
pcholakov merged 2 commits intomainfrom
chore/support-global-aws-env-defaults

Conversation

@pcholakov
Copy link
Copy Markdown
Contributor

@pcholakov pcholakov commented Mar 7, 2025

It's easy to miss a warning in the startup logs; since this could lead to data loss, I thought it's better to complain loudly and stop:

~/restate/restate chore/support-global-aws-env-defaults* ❯ AWS_ENDPOINT_URL_S3=http://minio:9000 RESTATE_WORKER__SNAPSHOTS__DESTINATION=s3://bucket/snapshots restate-server --default-num-partitions 1

...

2025-03-10T12:18:32.403583Z INFO restate_server
  Starting Restate Server 1.2.2-dev (debug) (7eeb6143c aarch64-apple-darwin 2025-03-10)
    node_name: "Pavels-MacBook-Pro-2.local"
    config_source: [default]
    base_dir: /Users/pavel/restate/restate/restate-data/Pavels-MacBook-Pro-2.local/
on main
2025-03-10T12:18:32.591063Z ERROR restate_server
  Restate application failed
    error: building worker failed: failed creating worker: failed constructing partition snapshot repository: Misconfiguration detected: an HTTP endpoint URL http://minio:9000 override is set for object store destination s3://bucket/snapshots, but plain HTTP is not allowed. Please set allow HTTP to `true` to continue
    restate.error.code: None
    Visit https://docs.restate.dev/references/errors#None for more info about this error.
on main

Additionally, this PR also enables some standard AWS SDK environment variables to be used as fallback defaults - see https://docs.aws.amazon.com/sdkref/latest/guide/feature-ss-endpoints.html.

It might be useful at some point to propagate the dev/production profile into a hidden field in the CommonOptions struct so that we can make better config decisions on the fly deeper into the server construction. WDYT?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 7, 2025

Test Results

  7 files  ± 0    7 suites  ±0   2m 46s ⏱️ - 1m 51s
 52 tests  -  2   51 ✅ ±0  1 💤 ±0  0 ❌ ±0 
213 runs   - 10  210 ✅  - 8  3 💤 ±0  0 ❌ ±0 

Results for commit e2812be. ± Comparison against base commit 3a84972.

This pull request removes 2 tests.
dev.restate.sdktesting.tests.Combinators ‑ awakeableOrTimeoutUsingAwaitAny(Client)
dev.restate.sdktesting.tests.Combinators ‑ firstSuccessfulCompletedAwakeable(Client)

♻️ This comment has been updated with latest results.

@pcholakov pcholakov requested a review from muhamadazmy March 10, 2025 11:48
…aws-allow-http

This is a common Minio configuration snag which results in hard-to-understand errors at runtime.
Since this will just not work, and potentially induces a data loss risk, it's better to complain
loudly upfront.
@pcholakov pcholakov force-pushed the chore/support-global-aws-env-defaults branch from 97118c2 to e2812be Compare March 10, 2025 12:23
@pcholakov pcholakov changed the title Use AWS conventional env vars as fallback for object store endpoints Treat invalid HTTP S3 endpoint overrides as a startup configuration error Mar 10, 2025
@pcholakov pcholakov marked this pull request as ready for review March 10, 2025 12:29
@pcholakov
Copy link
Copy Markdown
Contributor Author

I think the test failures have nothing at all to do with the contents of the PR, have retried the failing checks.

Copy link
Copy Markdown
Contributor

@muhamadazmy muhamadazmy left a comment

Choose a reason for hiding this comment

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

LGTM :) +1 for merging

@pcholakov pcholakov merged commit 352181a into main Mar 11, 2025
27 checks passed
@pcholakov pcholakov deleted the chore/support-global-aws-env-defaults branch March 11, 2025 09:08
pcholakov added a commit that referenced this pull request Mar 12, 2025
Fixes an error introduced by #2858 which only recognizes the global AWS_ALLOW_HTTP setting, and not the snapshots/metadata store client-specific config section keys.
@pcholakov pcholakov mentioned this pull request Mar 12, 2025
pcholakov added a commit that referenced this pull request Mar 12, 2025
Fixes a mistake introduced by #2858 which only recognizes the global AWS_ALLOW_HTTP setting, and not the snapshots/metadata store client-specific config section keys.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants