Locking implementation does not appear to be atomic

[mholt]

Thanks for making this implementation for CertMagic!

It came up in the Gophers Slack today that this implementation may not be atomic; specifically the existence check and the creation are racey between multiple instances:

magicstorage/s3storage.go

Lines 246 to 254 in 0c0f14c

	exists := s.Exists(filename)
	if exists {
	return fmt.Errorf(lockFileExists)
	}
	_, err := s.svc.PutObject(&s3.PutObjectInput{
	Bucket: s.bucket,
	Key: aws.String(filename),
	Body: bytes.NewReader([]byte("lock")),
	})

I'm not sure that S3 provides atomic operations at all; however, maybe some use of DynamoDB could be made to provide proper locking?

I was pointed to Terraform, which uses S3 for storage - I have not vetted its implementation but just wanted to include the link for reference, in case it's helpful: https://github.com/hashicorp/terraform/tree/master/backend/remote-state/s3

[OrkhanAlikhanov]

IfMatch and If-None-Match headers can be used to create/update lockfile atomically. Most servers like MinIO, GCS, Azure, AWS S3, IBM COS etc. support it. Seaweedfs does not seem to support it yet.

[mholt]

@OrkhanAlikhanov Ah, that's interesting, and worth exploring!