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.

Sign up for GitHub

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

Jump to bottom

s3: Put bucket tagging to return an error when bucket is not found #13232

Merged
merged 1 commit into from
Sep 17, 2021
Merged

s3: Put bucket tagging to return an error when bucket is not found #13232

merged 1 commit into from
Sep 17, 2021

Conversation

vadmeste
Copy link
Member

Description

loadBucketMetadata() should return an error when the bucket metadata
file doesn't exist. This should not cause a problem since the bucket metadata
file is always created already when a new bucket is created.

Motivation and Context

PutBucketTagging handler should return bucket not found error when the bucket
doesn't exist.

How to test this PR?

aws s3api put-bucket-tagging --debug --endpoint http://localhost:9000 --profile minio --bucket testbucket --tagging '{"TagSet": [{ "Key": "organization", "Value": "marketing" }]}'

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Documentation updated
  • Unit tests added/updated

s3: Put bucket tagging to return an error when bucket is not found
5fc30de 
instead of creating new metadata in .minio.sys directory
@minio-trusted
Copy link
Contributor

Mint Automation

Test Result
mint-large-bucket.sh ✔️
mint-fs.sh ✔️
mint-gateway-s3.sh ✔️
mint-erasure.sh ✔️
mint-dist-erasure.sh ✔️
mint-zoned.sh ✔️
mint-gateway-nas.sh ✔️
mint-compress-encrypt-dist-erasure.sh ✔️
Deleting image on docker hub
Deleting image locally

klauspost
klauspost approved these changes Sep 17, 2021
View reviewed changes
Copy link
Contributor

@klauspost klauspost left a comment

Choose a reason for hiding this comment

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

LGTM.

@harshavardhana
Copy link
Member

@vadmeste a mint test might make sense here.

@harshavardhana harshavardhana merged commit 91567ba into minio:master Sep 17, 2021
@jmsmkn jmsmkn mentioned this pull request Sep 19, 2021
Closed
@caugner

This comment has been minimized.

Sign in to view
@klauspost
Copy link
Contributor

@caugner Modifying the backend manually has never, and never will be supported.

If you could describe your problem instead of just complaining we might be able to actually help you.

If this is a problem in NAS mode, then it is a problem that should be fixed AFAICT.

@jmsmkn
Copy link

jmsmkn commented Sep 20, 2021

Indeed this works fine in NAS mode, a very simple change to my project, and happy to know I'm doing things correctly now!

Once you're at minio's scale every change will break something for someone so no problems with the versioning from me, I only tagged this PR for my own tracking. Thank you for developing and maintaining this excellent software, it's enabled us to do a lot more a lot faster ❤

@harshavardhana harshavardhana mentioned this pull request Sep 20, 2021
Closed
harshavardhana added a commit that referenced this pull request Sep 23, 2021
@harshavardhana
Revert "s3: Put bucket tagging to return an error when bucket is not …
f9b104f 
…found (#13232)"

This reverts commit 91567ba.

Revert because the error was incorrectly converted, there are
callers that rely on errConfigNotFound and it also took away
the migration code.

Instead the correct fix is PutBucketTaggingHandler() which
is already added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harshavardhana harshavardhana harshavardhana approved these changes

@klauspost klauspost klauspost approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@vadmeste @minio-trusted @harshavardhana @caugner @klauspost @jmsmkn