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

Better error logging for s3 #388

Closed
vshalts opened this issue Jan 14, 2016 · 33 comments
Closed

Better error logging for s3 #388

vshalts opened this issue Jan 14, 2016 · 33 comments
Assignees
Labels
type: feature enhancement improving existing features

Comments

@vshalts
Copy link

vshalts commented Jan 14, 2016

Hi,

I found that error reporting is not obvious (maybe not only for s3):

$ restic -r s3://eu-central-1/my_backup check    
enter password for repository: 
unable to open repo: no key could be found

with such error it is not obvious is it broken repo or problem with access rights to s3.
At the end I found problem, but it was not easy.

s3cmd do much better job:

Please wait, attempting to list bucket: s3://backup
ERROR: Test failed: 403 (AccessDenied): Access Denied
ERROR: Are you sure your keys have s3:ListAllMyBuckets permissions?

Can restic report errors more precisely?
Thanks.

@fd0 fd0 added the type: feature enhancement improving existing features label Jan 15, 2016
@fd0
Copy link
Member

fd0 commented Jan 15, 2016

Thanks for raising this, it's indeed a good idea in general and especially in this case. The PR #366 will exchange the library used for s3, I'll have a look once that is merged.

@fd0
Copy link
Member

fd0 commented Jan 18, 2016

I've merged #366 yesterday, could you try again?

Specify the repository like this: s3://s3.amazonaws.com/my_backup

@fd0 fd0 added the state: need feedback waiting for feedback, e.g. from the submitter label Jan 18, 2016
@vshalts
Copy link
Author

vshalts commented Jan 18, 2016

could you try again?

Sure. Thanks.

Specify the repository like this: s3://s3.amazonaws.com/my_backup

I have backup in eu-central-1, not in US default region.
I tried this and it fail:

restic -r s3://s3.eu-central-1.amazonaws.com/backup.myhost.com check
Amazon S3 endpoint should be 's3.amazonaws.com'`

I think that github.com/minio/minio-go/utils.go is wrong (maybe not only at this point):

// Match if it is exactly Amazon S3 endpoint.
func isAmazonEndpoint(endpointURL *url.URL) bool {
        if endpointURL == nil {
                return false
        }
        if endpointURL.Host == "s3.amazonaws.com" {
                return true
        }
        return false
}

If I use s3.amazonaws.com then second problem appear:

restic -r s3://s3.amazonaws.com/backup.myhost.com check                     
Bucket name contains invalid characters.

But . is valid character for s3 and recommended. Because Amazon recommend:

To take advantage of Amazon S3's CNAME support, you should name your bucket the same as your website's base address (e.g. www.mysite.com).

See more information here:
https://docs.aws.amazon.com/AmazonS3/latest/UG/CreatingaBucket.html

@fd0
Copy link
Member

fd0 commented Jan 19, 2016

As far as I know the minio library automatically uses the default region to create new buckets and will figure out the correct region for existing buckets. I'll contact the minio people.

@fd0
Copy link
Member

fd0 commented Jan 19, 2016

Using . in bucket names is disabled in minio, because otherwise TLS certificate verification fails, see http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html and my discussion with the minio team here: https://gitter.im/minio/minio?at=569dfc9404e908d302c3ddeb

Creating a bucket with restic in a region other than the US default region is not yet implemented, at the moment there is no way to give additional parameters (besides the bucket name and the login credentials) to the s3 backend. I'm working on that, but it will take a few days.

In the meantime, if you need a bucket in another region, please use a different client (or the web console) to create the bucket (without .), then use restic with the s3.amazonaws.com endpoint, and the library will figure out the bucket region by itself.

@fd0 fd0 removed the state: need feedback waiting for feedback, e.g. from the submitter label Jan 19, 2016
@vshalts
Copy link
Author

vshalts commented Jan 19, 2016

Thanks for information. Restriction for . is probably ok by default for Minio, but for restic it look like unnecessary because data already encrypted. I don't see problem to send backup data by HTTP.
But ok, let it be. One more sacrifice to God of security :) I think it will be useful to mention this restriction in restic documentation.

  1. Initially I saw strange error:

    restic -r s3://s3.amazonaws.com/backup-myhost-com check
    Please re-send this request to the specified temporary endpoint. Continue to use the original request endpoint for future requests.
    

    According to this:
    https://forums.aws.amazon.com/message.jspa?messageID=155181
    problem maybe related to DNS. At the same time s3cmd and AWS CLI work fine with this new bucket.

    Is Minio support redirects? Is it possible to add it if it not?

    After 2 hours problem disappear. So it not critical. Maybe not worth fixing but I decide to describe it anyway.

  2. If I remove access rights from bucket then instead of reasonable error I see:

    restic -r s3://s3.amazonaws.com/backup-myhost-com check
    The authorization header is malformed; the region 'us-east-1' is wrong; expecting 'eu-central-1'
    
  3. If access rights are correct, but I mistyped password I see:

    restic -r s3://s3.amazonaws.com/backup-myhost-com check
    enter password for repository: 
    unable to open repo: no key could be found
    

    is it for security reason? Is possible to use simple unencrypted file to check access rights before doing check? For example Description file which will say that this is restic backup.

@harshavardhana
Copy link
Contributor

Thanks for information. Restriction for . is probably ok by default for Minio, but for restic it look like unnecessary because data already encrypted. I don't see problem to send backup data by HTTP.
But ok, let it be. One more sacrifice to God of security :) I think it will be useful to mention this restriction in restic documentation.

We are thinking providing some sort of a solution for this in 'minio/minio-go' .. Will update here on how it goes.

Please re-send this request to the specified temporary endpoint. Continue to use the original request endpoint for future requests.

The problem happens when you initially create a bucket, since bucket is part of DNS it takes time for it to be propagated until 'us-east-1' region.

problem maybe related to DNS. At the same time s3cmd and AWS CLI work fine with this new bucket.

They are performing requests perhaps directly to the end point i.e 'eu-central-1' .

@harshavardhana
Copy link
Contributor

minio/minio-go#309 - fixes all the issues mentioned here, including supporting bucketNames with '.' . Thanks for reporting these problems.

@fd0
Copy link
Member

fd0 commented Jan 19, 2016

@harshavardhana thanks for stepping in! I'll create a PR for restic that updates the minio library as soon as your PR is merged.

@harshavardhana
Copy link
Contributor

minio/minio-go#309 - fixes all the issues mentioned here, including supporting bucketNames with '.' . Thanks for reporting these problems

This has been merged.

harshavardhana pushed a commit to harshavardhana/restic that referenced this issue Jan 20, 2016
@fd0
Copy link
Member

fd0 commented Jan 23, 2016

@harshavardhana shall I update now?

@harshavardhana
Copy link
Contributor

Yes @fd0

@fd0
Copy link
Member

fd0 commented Jan 23, 2016

Thanks, I'll update minio-go tomorrow.

@fd0 fd0 self-assigned this Jan 27, 2016
fd0 added a commit that referenced this issue Jan 27, 2016
This addresses #388
@fd0
Copy link
Member

fd0 commented Jan 27, 2016

Updated minio-go is in #398, once the CI tests pass I'll merge that, then you can please try again :)

@vshalts
Copy link
Author

vshalts commented Jan 27, 2016

Thanks. I am out off access to good internet. Will be able to test again not earlier than in two weeks.

@fd0
Copy link
Member

fd0 commented Jan 27, 2016

Sure, that's fine. I'm working with @harshavardhana to get the CI tests to pass on Go < 1.5 ;)

@vshalts
Copy link
Author

vshalts commented Feb 8, 2016

restic -r s3://s3.amazonaws.com/work-backup-vshalts-com check
The bucket you are attempting to access must be addressed using the specified endpoint. Please send all future requests to this endpoint.

Looks like it don't want to work with my region.

As far as I know the minio library automatically uses the default region to create new buckets and will figure out the correct region for existing buckets

I can't find way how to force it to work. What am I doing wrong?

@harshavardhana
Copy link
Contributor

restic -r s3://s3.amazonaws.com/work-backup-vshalts-com check
The bucket you are attempting to access must be addressed using the specified endpoint. Please send all future requests to this endpoint.

This is more likely a redirect which failed and possibly a wrong a region was indeed selected. Does the restic check perform a HEAD on a bucket to see it exists ? @fd0

I can't find way how to force it to work. What am I doing wrong?

You don't need to force it, minio-go internally does 'getBucketLocation()' - unless this API is disabled by your policies on 'work-backup-vshalts-com' bucket.

@fd0
Copy link
Member

fd0 commented Feb 9, 2016

@vshalts which exact version of restic are you using? What does restic version print?

@harshavardhana When an s3 backend is opened, the following code is executed, this calls client.BucketExists(): https://github.com/restic/restic/blob/master/backend/s3/s3.go#L45-L54

Does this execute a HEAD request?

@harshavardhana
Copy link
Contributor

@harshavardhana When an s3 backend is opened, the following code is executed, this calls client.BucketExists(): https://github.com/restic/restic/blob/master/backend/s3/s3.go#L45-L54

Does this execute a HEAD request?

Yes that is correct, possible issue here could be 'restic' older version.

@vshalts
Copy link
Author

vshalts commented Feb 9, 2016

The version was latest:

restic version
restic 0.1.0 (v0.1.0-348-geccbcb7)
compiled at 2016-02-09 12:54:40 with go1.5.2

But I found issue with my policy (error in bucket name). My mistake :(
I retested it, and now all looks fine and work great including '.' inside name of bucket.
I very thankful for your effort to help me!
Thanks, guys!!

@fd0
Copy link
Member

fd0 commented Feb 9, 2016

Hey, that's great to hear!

Do you think the error message is sufficient in this case? If so, please close this issue. Otherwise we'll see that we can give more high-level error messages in such cases.

@harshavardhana
Copy link
Contributor

Do you think the error message is sufficient in this case? If so, please close this issue. Otherwise we'll see that we can give more high-level error messages in such cases.

Indeed @fd0 is correct, we would like to cleanly handle it - rather than throwing S3 messages. It would be nice if you can show your previous bucket policy so that we can test and fix messaging on our end.

@vshalts
Copy link
Author

vshalts commented Feb 10, 2016

It is simple to reproduce. I use Frankfurt region. I replaced bucket with new one with '.'
So, bucket exist and have new name - work.backup.vshalts.com
While policy use some other name (for example old one):

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": "s3:ListAllMyBuckets",
            "Resource": "arn:aws:s3:::*"
        },
        {
            "Effect": "Allow",
            "Action": "s3:*",
            "Resource": [
                "arn:aws:s3:::work-backup-vshalts-com",
                "arn:aws:s3:::work-backup-vshalts-com/*"
            ]
        }
    ]
}

If I try to execute any operation against correct bucket name:

restic -r s3://s3.amazonaws.com/work.backup.vshalts.com snapshots

I see:
The bucket you are attempting to access must be addressed using the specified endpoint. Please send all future requests to this endpoint.

I prefer to see detailed message in best scenario. Something like that: Unable to access such resource 'name of resource here'. Please ensure you enable such actions 'list of action here'.
But as minimum it was enough for me to distinguish s3 errors from repository verification errors. I already can do it while it may be not that obvious.

@harshavardhana
Copy link
Contributor

I prefer to see detailed message in best scenario. Something like that: Unable to access such resource 'name of resource here'. Please ensure you enable such actions 'list of action here'.

This is not possible since this issue is successful and occurred such that the detected/received value for getBucketLocation() was 'us-east-1' not 'eu-central-1'

But as minimum it was enough for me to distinguish s3 errors from repository verification errors. I already can do it while it may be not that obvious.

This we need to do, but since the policies can change underneath there is no real consistent way one error handling will work for all cases. If the S3 server changes and replies with a different error code then we are basically going to break a lot of applications.

@harshavardhana
Copy link
Contributor

I see:
The bucket you are attempting to access must be addressed using the specified endpoint. Please send all future requests to this endpoint.

What is needed is a trace output of the network request, so that we can see what was replied back from the S3 server for such a bucket. @fd0 any ideas ?

@vshalts
Copy link
Author

vshalts commented Feb 10, 2016

This is not possible since this issue is successful and occurred such that the detected/received value for getBucketLocation() was 'us-east-1' not 'eu-central-1'

Just curious.
http://serverfault.com/questions/452387/cross-region-s3-bucket-policy
If I understand correctly server responds with something like http 301 Error - PermanentRedirect
'Please send all future requests to this endpoint.' imply that s3 somehow gives correct address that should be used instead. Is it possible to use it?

@harshavardhana
Copy link
Contributor

Just curious.
http://serverfault.com/questions/452387/cross-region-s3-bucket-policy
If I understand correctly server responds with something like http 301 Error - PermanentRedirect
'Please send all future requests to this endpoint.' imply that s3 somehow gives correct address that should be used instead. Is it possible to use it?

Actually Golang redirect has some issues which we are working with them upstream, but we would like to see network trace output. That way we can tell if the problem is minio-go libraries not handling some corner case properly.

The bucket you are attempting to access must be addressed using the specified endpoint. Please send all future requests to this endpoint.

What we need to do is look at the error and retry the request again. Request retry is a work in progress right now at minio-go layer.

@vshalts
Copy link
Author

vshalts commented Feb 10, 2016

What we need to do is look at the error and retry the request again. Request retry is a work in progress right now at minio-go layer.

Sound great.

Also, can you prefix unexpected S3 errors somehow?
It will be useful if instead of plain error message that you receive from network layer:

The bucket you are attempting to access must be addressed using the specified endpoint. Please send all future requests to this endpoint.

you will show to user additional hint that this was s3 related error (who was the source of the problem).
For example:

S3 error occurred: The bucket you are attempting to access must be addressed using the specified endpoint. Please send all future requests to this endpoint.

Or maybe not S3 but some general term, like error at transport level.

@harshavardhana
Copy link
Contributor

Also, can you prefix unexpected S3 errors somehow?
It will be useful if instead of plain error message that you receive from network layer:

it is already prefixed, it has an minio.ErrorResponse structure embedded in it - restic can construct a meaningful message from there.

//   import s3 "github.com/minio/minio-go"
//   ...
//   ...
//   reader, stat, err := s3.GetObject(...)
//   if err != nil {
//      resp := s3.ToErrorResponse(err)
//   }
//   ...

@harshavardhana
Copy link
Contributor

These errors should be generally be retried since it's minio-go's issue as it sent a wrong request to the server and server replied accordingly to retry with a valid endpoint.

@vshalts can you please open a bug against https://github.com/minio/minio-go - reference this issue there so that we can track these properly?

@fd0
Copy link
Member

fd0 commented Feb 13, 2016

@harshavardhana this issue is more about the restic UI that does not indicate in what part of the code the error occurred. :)

@fd0
Copy link
Member

fd0 commented Sep 12, 2016

This was implemented when #580 was merged, a stack trace is printed now. Closing.

@fd0 fd0 closed this as completed Sep 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature enhancement improving existing features
Projects
None yet
Development

No branches or pull requests

3 participants