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

chore(warehouse): validate object storage credentials #2440

Merged
merged 57 commits into from Oct 11, 2022

Conversation

deepakrai9185720
Copy link
Contributor

@deepakrai9185720 deepakrai9185720 commented Sep 14, 2022

Description

We store events that are flowing through our systems so that in case of any issues, we can replay them back and get at correct state. We store these events in bucket owned by rudderstack.

Going forward, we will be asking customers if they want to store the events on there own systems and if yes, allow them to add the credentials of the storage. The task is to expose a validation endpoint in warehouse to capture the creds and validate them on account of ability to upload and download.

The outcome of this validation will be presented to the user.

Notion Ticket

https://www.notion.so/rudderstacks/Object-Storage-Validation-Design-0a3300c4754d4271bed53d9ad15d683d

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Base: 44.25% // Head: 45.14% // Increases project coverage by +0.89% 🎉

Coverage data is based on head (20a6402) compared to base (73cc48a).
Patch coverage: 29.28% of modified lines in pull request are covered.

❗ Current head 20a6402 differs from pull request most recent head b524789. Consider uploading reports for the commit b524789 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2440      +/-   ##
==========================================
+ Coverage   44.25%   45.14%   +0.89%     
==========================================
  Files         181      178       -3     
  Lines       38413    37777     -636     
==========================================
+ Hits        17000    17056      +56     
+ Misses      20339    19643     -696     
- Partials     1074     1078       +4     
Impacted Files Coverage Δ
services/filemanager/filemanager.go 53.09% <0.00%> (-1.45%) ⬇️
warehouse/errors.go 0.00% <0.00%> (ø)
warehouse/api.go 70.41% <26.50%> (-6.44%) ⬇️
warehouse/warehousegrpc.go 73.43% <38.00%> (-22.72%) ⬇️
warehouse/schema.go 47.82% <0.00%> (-1.16%) ⬇️
jobsdb/jobsdb.go 67.75% <0.00%> (-0.10%) ⬇️
warehouse/utils/utils.go 58.83% <0.00%> (ø)
warehouse/bigquery/bigquery.go 0.00% <0.00%> (ø)
warehouse/postgres/postgres.go 0.00% <0.00%> (ø)
warehouse/redshift/redshift.go 0.00% <0.00%> (ø)
... and 16 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@abhimanyubabbar abhimanyubabbar changed the title Chore.validation object storage review chore(warehouse): validate object storage credentials Sep 16, 2022
warehouse/api.go Outdated Show resolved Hide resolved
warehouse/warehousegrpc.go Show resolved Hide resolved
warehouse/api.go Outdated Show resolved Hide resolved
warehouse/api.go Outdated Show resolved Hide resolved
warehouse/api.go Show resolved Hide resolved
warehouse/api.go Outdated Show resolved Hide resolved
warehouse/api.go Outdated Show resolved Hide resolved
}

//
func ifNotExistThenSet(keyToReplace string, replaceWith interface{}, configMap map[string]interface{}) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's have unit Test for this alter.

warehouse/errors.go Show resolved Hide resolved

// overrideWithEnv overrides the config keys in the filemanager settings
// with fallback values pulled from env. Only supported for S3 for now.
func overrideWithEnv(settings *filemanager.SettingsT) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's have a unit test for this afterwards.

warehouse/warehousegrpc.go Show resolved Hide resolved
Config map[string]interface{} `json:"config"`
}

func validateObjectStorageRequestBody(request *proto.ValidateObjectStorageRequest) (*ObjectStorageValidationRequest, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's have a test case for this afterwards.

@achettyiitr achettyiitr self-requested a review September 19, 2022 13:48
@deepakrai9185720 deepakrai9185720 merged commit 3d2d87e into master Oct 11, 2022
@github-actions github-actions bot deleted the chore.validationObjectStorage-Review branch December 12, 2022 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants