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

Update Profile Bucket Validation Code To Account For Custom S3 Endpoint #1631

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

ihcsim
Copy link
Contributor

@ihcsim ihcsim commented Sep 9, 2022

Change Overview

This PR updates the validate.ProfileBucket() method to use custom S3 endpoint, if provided, for its validation steps. Without this change, stow will default back to using the s3.<region>.amazonaws.com domain, even though the custom endpoint is provided.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

Test Plan

Use the latest kanctl CLI to reproduce the issue:

$ kanctl create profile s3compliant --access-key "test" --secret-key "test" --endpoint https://s3.cern.ch/  --bucket my-bucket --region cern --namespace default --verbose

The validation code ignores the s3.cern.ch endpoint:

secret 's3-secret-ggiorh' created
Failed the 'Validate bucket region specified in profile' check.. ❌
validation failed, deleting secret 's3-secret-ggiorh'
secret 's3-secret-ggiorh' deleted
Error: profile validation failed: failed to get bucket my-bucket: GetBucketLocation: RequestError: send request failed
caused by: Get "https://s3.cern.amazonaws.com/my-bucket?location=": dial tcp: lookup s3.cern.amazonaws.com on 127.0.0.53:53: no such host

Run kanctl with this change:

go run ./cmd/kanctl create profile s3compliant --access-key "test" --secret-key "test" --endpoint https://s3.cern.ch/  --bucket my-buclet --region cern --namespace default --verbose

The right endpoint is used:

secret 's3-secret-ijj2lv' created
Failed to obtain reader, failed to marshal fields to JSON, json: unsupported type: func(time.Duration)
Failed the 'Validate bucket region specified in profile' check.. ❌
validation failed, deleting secret 's3-secret-ijj2lv'
secret 's3-secret-ijj2lv' deleted
Error: profile validation failed: failed to get bucket my-bucket: GetBucketLocation: InvalidAccessKeyId: 
        status code: 403, request id: tx000000000000049299681-00631bb733-2af9b497-default, host id: 
{"File":"pkg/kanctl/kanctl.go","Function":"github.com/kanisterio/kanister/pkg/kanctl.Execute","Line":48,"error":"profile validation failed: failed to get bucket my-bucket: GetBucketLocation: InvalidAccessKeyId: \n\tstatus code: 403, request id: tx000000000000049299681-00631bb733-2af9b497-default, host id: ","level":"info","msg":"Kanctl failed to execute","time":"2022-09-09T14:59:15.597305194-07:00"}
exit status 1
  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@ihcsim ihcsim requested a review from pavannd1 September 9, 2022 22:02
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister Sep 9, 2022
Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

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

LGTM

Kanister automation moved this from In Progress to Reviewer approved Sep 12, 2022
@ihcsim
Copy link
Contributor Author

ihcsim commented Sep 13, 2022

/ok-to-test

@ihcsim ihcsim force-pushed the fix-kanctl-profile-s3-endpoint branch from 38227f0 to fa51fd9 Compare September 13, 2022 18:30
@ihcsim
Copy link
Contributor Author

ihcsim commented Sep 13, 2022

/ok-to-test

3 similar comments
@ihcsim
Copy link
Contributor Author

ihcsim commented Sep 14, 2022

/ok-to-test

@akankshakumari393
Copy link
Contributor

/ok-to-test

@ihcsim
Copy link
Contributor Author

ihcsim commented Sep 14, 2022

/ok-to-test

@ihcsim ihcsim added the kueue label Sep 14, 2022
@mergify mergify bot merged commit 7f9742f into master Sep 14, 2022
Kanister automation moved this from Reviewer approved to Done Sep 14, 2022
@mergify mergify bot deleted the fix-kanctl-profile-s3-endpoint branch September 14, 2022 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

[BUG] Wrong URL for S3 endpoint
4 participants