-
Notifications
You must be signed in to change notification settings - Fork 9
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: path-based locations #288
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f4bfb7e
fix: service info group, artifact and version, and add flexibility in…
mmalenic a38e485
feat(config): implement path-based locations
mmalenic 0a13f96
docs: update docs for path-based and regex config
mmalenic f17a7c3
Merge branch 'refactor/advanced-config' of https://github.com/umccr/h…
mmalenic 26844d6
docs(config): ensure all examples and example files are working
mmalenic 95b3c9b
fix(config): locations prefix should not be appended to the id so tha…
mmalenic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,8 @@ | ||
# An example of running htsget-rs. | ||
# Run with `cargo run --all-features -- --config htsget-config/examples/config-files/basic.toml` | ||
# Run with `cargo run -p htsget-axum --all-features -- --config htsget-config/examples/config-files/basic.toml` | ||
|
||
ticket_server_addr = "127.0.0.1:8080" | ||
data_server_addr = "127.0.0.1:8081" | ||
|
||
# Serve data locally from the `data` directory. | ||
[[resolvers]] | ||
regex = '.*' | ||
substitution_string = '$0' | ||
storage.backend = 'Local' | ||
|
||
# Serve data from S3 if the id is prefixed with `example_bucket`. | ||
[[resolvers]] | ||
regex = '^(example_bucket)/(?P<key>.*)$' | ||
substitution_string = '$key' | ||
storage.backend = 'S3' | ||
# Uses the first capture group in the regex as the bucket. | ||
# Serve data locally from the `data` directory or from an S3 bucket called `bucket` depending on the prefix. | ||
locations = [ "file://data/file_prefix", "s3://bucket/s3_prefix" ] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,19 @@ | ||
# An example of running htsget-rs with Crypt4GH enabled. | ||
# Run with `cargo run -p htsget-axum --features experimental -- --config htsget-config/examples/config-files/c4gh.toml` | ||
# Run with `cargo run -p htsget-axum --features experimental,s3-storage -- --config htsget-config/examples/config-files/c4gh.toml` | ||
|
||
ticket_server_addr = "127.0.0.1:8080" | ||
data_server_addr = "127.0.0.1:8081" | ||
ticket_server.addr = "127.0.0.1:8080" | ||
data_server.addr = "127.0.0.1:8081" | ||
|
||
[[resolvers]] | ||
[[locations]] | ||
regex = ".*" | ||
substitution_string = "$0" | ||
backend.kind = "File" | ||
|
||
[resolvers.storage] | ||
backend = 'Local' | ||
backend.keys.kind = "File" | ||
backend.keys.private = "data/c4gh/keys/bob.sec" # pragma: allowlist secret | ||
backend.keys.public = "data/c4gh/keys/alice.pub" | ||
|
||
[resolvers.storage.keys] | ||
location = "Local" | ||
private_key = "data/c4gh/keys/bob.sec" # pragma: allowlist secret | ||
recipient_public_key = "data/c4gh/keys/alice.pub" | ||
|
||
# Or, use AWS secrets manager to store keys. | ||
#[resolvers.storage.keys] | ||
#location = "SecretsManager" | ||
#private_key = "htsget/test_c4gh_private_key" # pragma: allowlist secret | ||
#recipient_public_key = "htsget/test_c4gh_public_key" | ||
## Or, use AWS secrets manager to store keys. | ||
#backend.keys.kind = "SecretsManager" | ||
#backend.keys.private = "htsget/test_c4gh_private_key" # pragma: allowlist secret | ||
#backend.keys.public = "htsget/test_c4gh_public_key" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,40 +1,37 @@ | ||
# Config generated by running `cargo run -p htsget-axum -- -p` | ||
|
||
formatting_style = "Full" | ||
ticket_server_addr = "127.0.0.1:8080" | ||
ticket_server_cors_allow_credentials = false | ||
ticket_server_cors_allow_origins = ["http://localhost:8080"] | ||
ticket_server_cors_allow_headers = "All" | ||
ticket_server_cors_allow_methods = "All" | ||
ticket_server_cors_max_age = 86400 | ||
ticket_server_cors_expose_headers = [] | ||
data_server_enabled = true | ||
data_server_addr = "127.0.0.1:8081" | ||
data_server_local_path = "./" | ||
data_server_serve_at = "" | ||
data_server_cors_allow_credentials = false | ||
data_server_cors_allow_origins = ["http://localhost:8080"] | ||
data_server_cors_allow_headers = "All" | ||
data_server_cors_allow_methods = "All" | ||
data_server_cors_max_age = 86400 | ||
data_server_cors_expose_headers = [] | ||
|
||
[[resolvers]] | ||
regex = ".*" | ||
substitution_string = "$0" | ||
storage = "Local" | ||
|
||
[resolvers.allow_guard] | ||
allow_reference_names = "All" | ||
allow_fields = "All" | ||
allow_tags = "All" | ||
allow_formats = [ | ||
"BAM", | ||
"CRAM", | ||
"VCF", | ||
"BCF", | ||
] | ||
allow_classes = [ | ||
"body", | ||
"header", | ||
] | ||
|
||
[ticket_server] | ||
addr = "127.0.0.1:8080" | ||
|
||
[ticket_server.cors] | ||
allow_credentials = false | ||
allow_origins = "Mirror" | ||
allow_headers = "Mirror" | ||
allow_methods = "Mirror" | ||
max_age = 2592000 | ||
expose_headers = "All" | ||
|
||
[data_server] | ||
addr = "127.0.0.1:8081" | ||
local_path = "./" | ||
|
||
[data_server.cors] | ||
allow_credentials = false | ||
allow_origins = "Mirror" | ||
allow_headers = "Mirror" | ||
allow_methods = "Mirror" | ||
max_age = 2592000 | ||
expose_headers = "All" | ||
|
||
[service_info] | ||
|
||
[[locations]] | ||
prefix = "" | ||
|
||
[locations.backend] | ||
kind = "File" | ||
scheme = "HTTP" | ||
authority = "127.0.0.1:8081" | ||
local_path = "./" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,18 @@ | ||
# An example for a server which uses s3 storage with data located in "bucket". | ||
# Run with `cargo run -p htsget-axum --features s3-storage -- --config htsget-config/examples/config-files/s3_storage.toml` | ||
|
||
ticket_server_cors_allow_headers = "All" | ||
ticket_server_cors_allow_methods = "All" | ||
ticket_server_cors_allow_credentials = true | ||
ticket_server_cors_max_age = 300 | ||
ticket_server.cors.allow_headers = "All" | ||
ticket_server.cors.allow_methods = "All" | ||
ticket_server.cors.allow_credentials = false | ||
ticket_server.cors.max_age = 300 | ||
|
||
data_server_enabled = false | ||
data_server = "None" | ||
|
||
[[resolvers]] | ||
regex = '^(bucket)/(?P<key>.*)$' | ||
substitution_string = '$key' | ||
storage.backend = 'S3' | ||
locations = "s3://bucket" | ||
|
||
# Or, set the bucket manually | ||
#[resolvers.storage] | ||
#backend = 'S3' | ||
#bucket = 'bucket' | ||
## Or, set using regex locations | ||
#[[locations]] | ||
#regex = "^(bucket)/(?P<key>.*)$" | ||
#substitution_string = "$key" | ||
#backend.kind = "S3" | ||
#backend.bucket = "bucket" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,17 @@ | ||
# An example config file for a TLS data server that uses a local storage backend. | ||
# Run with `cargo run -p htsget-axum -- --config htsget-config/examples/config-files/tls_data_server.toml` | ||
|
||
ticket_server_addr = "0.0.0.0:8080" | ||
data_server_addr = "0.0.0.0:8081" | ||
data_server_cors_allow_origins = "All" | ||
data_server_tls.cert = "cert.pem" | ||
data_server_tls.key = "key.pem" | ||
ticket_server.addr = "0.0.0.0:8080" | ||
data_server.addr = "0.0.0.0:8081" | ||
data_server.cors.allow_origins = "All" | ||
data_server.tls.cert = "cert.pem" | ||
data_server.tls.key = "key.pem" | ||
|
||
[[resolvers]] | ||
regex = ".*" | ||
substitution_string = "$0" | ||
locations = "file://data" | ||
|
||
[resolvers.storage] | ||
backend = 'Local' | ||
use_data_server_config = true | ||
## Or, set using regex locations | ||
#[[locations]] | ||
#regex = ".*" | ||
#substitution_string = "$0" | ||
#backend.kind = "File" | ||
#backend.local_path = "data" |
23 changes: 12 additions & 11 deletions
23
htsget-config/examples/config-files/tls_ticket_server.toml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,17 @@ | ||
# An example config file for a TLS ticket server that uses S3 as a storage backend. | ||
# Run with `cargo run -p htsget-axum --features s3-storage -- --config htsget-config/examples/config-files/tls_ticket_server.toml` | ||
|
||
ticket_server_addr = "0.0.0.0:8080" | ||
ticket_server_cors_allow_origins = "All" | ||
ticket_server_tls.cert = "cert.pem" | ||
ticket_server_tls.key = "key.pem" | ||
data_server_addr = "0.0.0.0:8081" | ||
ticket_server.addr = "0.0.0.0:8080" | ||
ticket_server.cors_allow_origins = "All" | ||
ticket_server.tls.cert = "cert.pem" | ||
ticket_server.tls.key = "key.pem" | ||
data_server.addr = "0.0.0.0:8081" | ||
|
||
[[resolvers]] | ||
regex = ".*" | ||
substitution_string = "$0" | ||
locations = "s3://bucket" | ||
|
||
[resolvers.storage] | ||
backend = 'S3' | ||
bucket = "bucket" | ||
## Or, set using regex locations | ||
#[[locations]] | ||
#regex = ".*" | ||
#substitution_string = "$0" | ||
#backend.kind = "S3" | ||
#backend.bucket = "bucket" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
s3-storage
shouldn't be required, IMHO?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.
Yeah that's fair. It's required because the secrets manager has all the AWS dependencies, which
s3-storage
enables.Originally, only S3 was required for any AWS-specific things, but now that's changed, maybe this feature should be called
aws
rather thans3-storage
.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.
It might be good to rename/simplify
url-storage
too? E.g.s3-storage
->aws
url-storage
->url
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.
I'll make this a separate PR.
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.
Agreed on the renaming, but shouldn't we have a
file://
for the secrets management part? I'm thinking about local testing and/or devving, right?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.
There is
kind = "File"
for local keys:And
kind = "SecretsManager"
for secrets manager keys:Is that what you mean? Or some sort of local SecretsManager option?
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.
No, I'm only concerned about being able to run c4gh infra locally, without requiring
aws
deps... that "Run withcargo run...
" line change seems to imply thats3-storage
is absolutely required? From what you just said it seems it's not, perhaps being explicit about both options (only viaexperimental
andexperimental+aws
feature flags) should be explicitly stated?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.
Yes, you're right, it's not required. It's only required if you remove the comments in that file that use
kind = "SecretsManager"
.I'll make this clearer on the next PR. Maybe it should just be
--all-features
in all the examples to avoid any confusion?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.
🤔 ... that'd exercise our error management logic and corresponding error messages, i.e: running with
file://
paths andaws
-related c4gh secrets management. It might make more sense this way if errors are clear, yeah!