-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Update the developer guide to add a section to run remote backed storage from dev environment #16223
base: main
Are you sure you want to change the base?
Update the developer guide to add a section to run remote backed storage from dev environment #16223
Conversation
Signed-off-by: Srikanth Padakanti <[email protected]>
Signed-off-by: Srikanth Padakanti <[email protected]>
DEVELOPER_GUIDE.md
Outdated
|
||
|
||
// Add AWS credentials to the keystore | ||
keystore 's3.client.default.access_key', '<access_key>' |
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 don't think storing credentials in this file should be a recommended practice.
How about reading from env variables instead as an alternative unless there are better ways:
keystore 's3.client.default.access_key', System.getenv('AWS_ACCESS_KEY_ID')
keystore 's3.client.default.secret_key', System.getenv('AWS_SECRET_ACCESS_KEY')
keystore 's3.client.default.session_token', System.getenv('AWS_SESSION_TOKEN')
export AWS_ACCESS_KEY_ID=<access_key>
export AWS_SECRET_ACCESS_KEY=<secret_key>
export AWS_SESSION_TOKEN=<session_token>
In that way onlly authorized users are able to run this script.
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 understand that storing credentials directly isn't the recommended approach, and I agree. You can definitely export these values and access them through system variables. My goal here is to demonstrate how to run OpenSearch locally while connecting to remote storage.
The reason I’m using them directly in the file is that my tokens [access_key, secret, token] are temporarily generated and have a validity of only 45 minutes.
Made the recommended changes.
Signed-off-by: Srikanth Padakanti <[email protected]>
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.
Made the recommended changes.
DEVELOPER_GUIDE.md
Outdated
|
||
|
||
// Add AWS credentials to the keystore | ||
keystore 's3.client.default.access_key', '<access_key>' |
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 understand that storing credentials directly isn't the recommended approach, and I agree. You can definitely export these values and access them through system variables. My goal here is to demonstrate how to run OpenSearch locally while connecting to remote storage.
The reason I’m using them directly in the file is that my tokens [access_key, secret, token] are temporarily generated and have a validity of only 45 minutes.
Made the recommended changes.
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.
Made the recommended changes.
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.
Thanks! I made a bunch of nitpicky asks, choose the ones that you think are good ones.
Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]> Signed-off-by: Srikanth Padakanti <[email protected]>
Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]> Signed-off-by: Srikanth Padakanti <[email protected]>
Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]> Signed-off-by: Srikanth Padakanti <[email protected]>
Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]> Signed-off-by: Srikanth Padakanti <[email protected]>
Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]> Signed-off-by: Srikanth Padakanti <[email protected]>
Signed-off-by: Srikanth Padakanti <[email protected]>
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.
Considered the recommended suggestions and made sure to incorporate everyone of them.
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.
Made the recommended changes
Signed-off-by: Srikanth Padakanti <[email protected]>
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.
More nits, sorry to be annoying. Thanks for hanging in here with me!
The TOC is the one really needed.
@@ -556,6 +556,40 @@ Then, you need to apply patterns for git-secrets, you can install the AWS standa | |||
git secrets --register-aws | |||
``` | |||
|
|||
#### Remote Backed 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.
``` | ||
testClusters { | ||
runTask { | ||
// Add following lines to enable remote store |
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.
To be consistent with the text below.
// Add following lines to enable remote store | |
// Enable remote store |
setting 'node.attr.remote_store.repository.my-repository.settings.bucket', '<bucket_name>' | ||
setting 'node.attr.remote_store.repository.my-repository.settings.base_path', '<base_path>' | ||
setting 'node.attr.remote_store.repository.my-repository.settings.region', '<region>' # e.g. us-west-2 | ||
|
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.
Nit: remove one of the empty lines.
keystore 's3.client.default.secret_key', System.getenv('AWS_SECRET_ACCESS_KEY') | ||
keystore 's3.client.default.session_token', System.getenv('AWS_SESSION_TOKEN') | ||
``` | ||
Then run by giving the required plugin as parameter |
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.
Then run by giving the required plugin as parameter | |
Then run by specifying the required plugin as parameter. |
``` | ||
Then run by giving the required plugin as parameter | ||
``` | ||
Exporting values to use as system variable. |
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.
Remove this line, it's text and can't be copy-pasted.
export AWS_SECRET_ACCESS_KEY=<secret_key> | ||
export AWS_SESSION_TOKEN=<session_token> | ||
|
||
Example: plugin can be 'repository-s3' |
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.
In this section it has to be repository-s3
, just remove this line.
This PR is stalled because it has been open for 30 days with no activity. |
Description
Add a section to run remote backed storage from dev environment
Related Issues
Closes #16096
#10651
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.