From e98a3dc32d4b3fe30e00a011c8867b5280c4b47d Mon Sep 17 00:00:00 2001 From: Knut Ahlers Date: Mon, 17 Jun 2019 01:09:16 +0200 Subject: [PATCH] Fix broken behaviour when using checksums Signed-off-by: Knut Ahlers --- providers/s3/provider.go | 15 ++++++++------ sync/execute.go | 28 ++++++++++++++++++++++---- sync/sync.go | 43 ++++++++++++++++++++++++++-------------- 3 files changed, 61 insertions(+), 25 deletions(-) diff --git a/providers/s3/provider.go b/providers/s3/provider.go index 2b8502a..4fc307e 100644 --- a/providers/s3/provider.go +++ b/providers/s3/provider.go @@ -36,7 +36,12 @@ func New(uri string) (providers.CloudProvider, error) { return nil, errors.Wrap(err, "Invalid URI specified") } - cfg := aws.NewConfig() + region, err := s3manager.GetBucketRegion(context.Background(), session.New(), u.Host, "us-east-1") + if err != nil { + return nil, errors.Wrap(err, "Unable to find bucket region") + } + + cfg := aws.NewConfig().WithRegion(region) if u.User != nil { user := u.User.Username() pass, _ := u.User.Password() @@ -46,11 +51,6 @@ func New(uri string) (providers.CloudProvider, error) { sess := session.Must(session.NewSession(cfg)) svc := s3.New(sess) - region, err := s3manager.GetBucketRegion(context.Background(), sess, u.Host, "us-east-1") - if err != nil { - return nil, errors.Wrap(err, "Unable to find bucket region") - } - return &Provider{ bucket: u.Host, bucketRegion: region, @@ -165,6 +165,9 @@ func (p *Provider) getFileACL(relativeName string) string { } for _, g := range objACL.Grants { + if g.Grantee == nil || g.Grantee.URI == nil { + continue + } if *g.Grantee.URI == "http://acs.amazonaws.com/groups/global/AllUsers" && *g.Permission == "READ" { return s3.ObjectCannedACLPublicRead } diff --git a/sync/execute.go b/sync/execute.go index 65f5d0f..b0fa917 100644 --- a/sync/execute.go +++ b/sync/execute.go @@ -36,11 +36,21 @@ func (s *Sync) addBothCreated(fileName string) error { return errors.New("Checksums differ") } - if err := s.setDBFileInfo(sideLocal, local.Info()); err != nil { + localInfo, err := s.getFileInfo(local) + if err != nil { + return errors.Wrap(err, "Unable to get file info for local file") + } + + if err := s.setDBFileInfo(sideLocal, localInfo); err != nil { return errors.Wrap(err, "Unable to update DB info for local file") } - if err := s.setDBFileInfo(sideRemote, remote.Info()); err != nil { + remoteInfo, err := s.getFileInfo(remote) + if err != nil { + return errors.Wrap(err, "Unable to get file info for remote file") + } + + if err := s.setDBFileInfo(sideRemote, remoteInfo); err != nil { return errors.Wrap(err, "Unable to update DB info for remote file") } @@ -74,11 +84,21 @@ func (s *Sync) transferFile(from, to providers.CloudProvider, sideFrom, sideTo, return errors.Wrap(err, "Unable to put file") } - if err := s.setDBFileInfo(sideTo, newFile.Info()); err != nil { + newFileInfo, err := s.getFileInfo(newFile) + if err != nil { + return errors.Wrap(err, "Unable to get file info for target file") + } + + if err := s.setDBFileInfo(sideTo, newFileInfo); err != nil { return errors.Wrap(err, "Unable to update DB info for target file") } - if err := s.setDBFileInfo(sideFrom, file.Info()); err != nil { + fileInfo, err := s.getFileInfo(file) + if err != nil { + return errors.Wrap(err, "Unable to get file info for source file") + } + + if err := s.setDBFileInfo(sideFrom, fileInfo); err != nil { return errors.Wrap(err, "Unable to update DB info for source file") } diff --git a/sync/sync.go b/sync/sync.go index 9c509a4..9f5abd6 100644 --- a/sync/sync.go +++ b/sync/sync.go @@ -23,6 +23,9 @@ type Sync struct { log *log.Entry + useChecksum bool + hashMethod hash.Hash + stop chan struct{} } @@ -62,20 +65,32 @@ func (s *Sync) Run() error { func (s *Sync) Stop() { s.stop <- struct{}{} } -func (s *Sync) fillStateFromProvider(syncState *state, provider providers.CloudProvider, side string, useChecksum bool, hashMethod hash.Hash) error { +func (s *Sync) getFileInfo(f providers.File) (providers.FileInfo, error) { + var info = f.Info() + + if !s.useChecksum || info.Checksum != "" { + return info, nil + } + + cs, err := f.Checksum(s.hashMethod) + if err != nil { + return info, errors.Wrap(err, "Unable to fetch checksum") + } + info.Checksum = cs + + return info, nil +} + +func (s *Sync) fillStateFromProvider(syncState *state, provider providers.CloudProvider, side string) error { files, err := provider.ListFiles() if err != nil { return errors.Wrap(err, "Unable to list files") } for _, f := range files { - info := f.Info() - if useChecksum && info.Checksum == "" { - cs, err := f.Checksum(hashMethod) - if err != nil { - return errors.Wrap(err, "Unable to fetch checksum") - } - info.Checksum = cs + info, err := s.getFileInfo(f) + if err != nil { + return errors.Wrap(err, "Unable to get file info") } syncState.Set(side, sourceScan, info) @@ -85,21 +100,19 @@ func (s *Sync) fillStateFromProvider(syncState *state, provider providers.CloudP } func (s *Sync) runSync() error { - var ( - hashMethod = s.remote.GetChecksumMethod() - syncState = newState() - useChecksum = s.remote.Capabilities().Has(providers.CapAutoChecksum) || s.conf.ForceUseChecksum - ) + var syncState = newState() + s.hashMethod = s.remote.GetChecksumMethod() + s.useChecksum = s.remote.Capabilities().Has(providers.CapAutoChecksum) || s.conf.ForceUseChecksum if err := s.updateStateFromDatabase(syncState); err != nil { return errors.Wrap(err, "Unable to load database state") } - if err := s.fillStateFromProvider(syncState, s.local, sideLocal, useChecksum, hashMethod); err != nil { + if err := s.fillStateFromProvider(syncState, s.local, sideLocal); err != nil { return errors.Wrap(err, "Unable to load local files") } - if err := s.fillStateFromProvider(syncState, s.remote, sideRemote, useChecksum, hashMethod); err != nil { + if err := s.fillStateFromProvider(syncState, s.remote, sideRemote); err != nil { return errors.Wrap(err, "Unable to load remote files") }