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

Preserve ETag case for S3 compatibility #7498

Merged
merged 1 commit into from
Apr 8, 2019

Conversation

harshavardhana
Copy link
Member

Description

Preserve ETag case for S3 compatibility

Motivation and Context

Most hadoop distributions hortonworks, cloudera all
depend on aws-sdk-java 1.7.x to 1.10.x - the releases
which have bugs related case sensitive check for
ETag header. Go changes the case of the headers set
to be canonical but only preserves them when set
through a direct map.

This fixes most compatibility issues we have had
in the past supporting older hadoop distributions.

Regression

No

How Has This Been Tested?

Using hortonworks HDP 2.6.5 with hadoop 2.7.3 and aws-sdk-java 1.10.6

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added unit tests to cover my changes.
  • I have added/updated functional tests in mint. (If yes, add mint PR # here: )
  • All new and existing tests passed.

Copy link
Contributor

@poornas poornas left a comment

Choose a reason for hiding this comment

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

For consistency, can you also make this change in checkCopyObjectPreconditions and checkPreconditions

@harshavardhana
Copy link
Member Author

For consistency, can you also make this change in checkCopyObjectPreconditions and checkPreconditions

Done @poornas

donatello
donatello previously approved these changes Apr 8, 2019
poornas
poornas previously approved these changes Apr 8, 2019
Copy link
Contributor

@poornas poornas left a comment

Choose a reason for hiding this comment

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

LGTM

@kannappanr
Copy link
Contributor

@harshavardhana Please fix unit tests. Travis is failing.

@harshavardhana
Copy link
Member Author

@harshavardhana Please fix unit tests. Travis is failing.

Done @kannappanr

Most hadoop distributions hortonworks, cloudera all
depend on aws-sdk-java 1.7.x to 1.10.x - the releases
which have bugs related case sensitive check for
ETag header. Go changes the case of the headers set
to be canonical but only preserves them when set
through a direct map.

This fixes most compatibility issues we have had
in the past supporting older hadoop distributions.
@kannappanr
Copy link
Contributor

ping @poornas @donatello

@codecov
Copy link

codecov bot commented Apr 8, 2019

Codecov Report

Merging #7498 into master will decrease coverage by 0.49%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #7498     +/-   ##
=========================================
- Coverage   48.02%   47.53%   -0.5%     
=========================================
  Files         296      281     -15     
  Lines       46797    46465    -332     
=========================================
- Hits        22476    22086    -390     
- Misses      22236    22306     +70     
+ Partials     2085     2073     -12
Impacted Files Coverage Δ
cmd/object-handlers.go 54.42% <100%> (ø) ⬆️
cmd/bucket-handlers.go 57.16% <100%> (ø) ⬆️
cmd/api-headers.go 73.33% <100%> (ø) ⬆️
cmd/object-handlers-common.go 64.9% <100%> (ø) ⬆️
cmd/disk-usage.go 0% <0%> (-78.95%) ⬇️
cmd/update-notifier.go 55.35% <0%> (-25%) ⬇️
cmd/server-rlimit.go 37.5% <0%> (-16.67%) ⬇️
cmd/namespace-lock.go 58.62% <0%> (-12.32%) ⬇️
cmd/posix-errors.go 40% <0%> (-9.34%) ⬇️
cmd/retry.go 82.14% <0%> (-5.36%) ⬇️
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10a6071...f79072f. Read the comment docs.

@kannappanr kannappanr merged commit a2e344b into minio:master Apr 8, 2019
@harshavardhana harshavardhana deleted the http-s3 branch April 9, 2019 00:08
@minio-ops
Copy link

Mint Automation

Test Result
mint-compression-xl.sh ✔️
mint-xl.sh ✔️
mint-large-bucket.sh ✔️
mint-compression-dist-xl.sh ✔️
mint-compression-fs.sh ✔️
mint-fs.sh ✔️
mint-dist-xl.sh ✔️
mint-worm.sh more...
mint-gateway-nas.sh more...

7498-f79072f/mint-worm.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.57:31090
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            worm
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp 6180b73d3233:/mint/log /tmp/mint-logs'

(1/1) Running worm tests ... FAILED in 3 seconds
{
  "alert": "",
  "args": {
    "bucketName": "bucket-gvkrovcvrewgpfz2h82p4t8a1qvubccbesde8v01widcfz74mj823vr",
    "expiry": 60000000000,
    "objectName": "testObject"
  },
  "duration": 3068,
  "error": "RequestError: send request failed\ncaused by: Put http://72.28.97.57:31090/bucket-gvkrovcvrewgpfz2h82p4t8a1qvubccbesde8v01widcfz74mj823vr: dial tcp 72.28.97.57:31090: connect: no route to host",
  "function": "PutAndDelete",
  "message": "WORM_MODE ON  - CreateBucket Failed",
  "name": "test worm mode",
  "status": "FAIL"
}

Executed 0 out of 1 tests successfully.

7498-f79072f/mint-gateway-nas.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.57:32503
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp af229612f000:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... FAILED in 3 seconds
{
  "alert": "",
  "args": {
    "bucketName": "aws-sdk-go-test-ck6n3vgomiuyff",
    "expiry": 60000000000,
    "objectName": "presignedTest"
  },
  "duration": 2722,
  "error": "RequestError: send request failed\ncaused by: Put http://72.28.97.57:32503/aws-sdk-go-test-ck6n3vgomiuyff: dial tcp 72.28.97.57:32503: connect: no route to host",
  "function": "PresignedPut",
  "message": "AWS SDK Go CreateBucket Failed",
  "name": "aws-sdk-go",
  "status": "FAIL"
}

Executed 0 out of 14 tests successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants