flyway / flyway

Flyway by Redgate • Database Migrations Made Easy.
https://flywaydb.org
Apache License 2.0
8.18k stars 1.5k forks source link

S3 locations: Trailing slash is stripped, makes unexpected migration selections #3536

Closed ikkala closed 2 years ago

ikkala commented 2 years ago
Which version and edition of Flyway are you using?

9.4.0 Community

If this is not the latest version, can you reproduce the issue with the latest one as well? (Many bugs are fixed in newer releases and upgrading will often resolve the issue)

The 9.4.0 is currently the latest version.

Which client are you using? (Command-line, Java API, Maven plugin, Gradle plugin)

Command-line

Which database are you using? (Type & version)

Not relevant in case of S3 bucket prefix resolving.

Which operating system are you using?

Ubuntu 20.04 inside Docker container, though shouldn't be relevant in case of S3 bucket prefix resolving.

What did you do? (Please include the content causing the issue, any relevant configuration settings, the SQL statement(s) that failed (if any), and the command you ran)

We have flyway migration files in S3 buckets, and they are versioned for example into the following "folders":

s3:testbucket/testpath/1.2.0/
s3:testbucket/testpath/1.2.1/

Our deployment system select correct "folder" by the version number that is the last fraction in the path. The problem occurs in the following case when version 1.2.1 is tried to deploy and we actually have also test version 1.2.1-test1 in the bucket:

s3:testbucket/testpath/1.2.1/
s3:testbucket/testpath/1.2.1-test/
What did you expect to see?

Only "s3:testbucket/testpath/1.2.1/" path is expected to be scanned and migrated.

What did you see instead?

Both, the "s3:testbucket/testpath/1.2.1/" and "s3:testbucket/testpath/1.2.1-test/", are scanned and tried to be migrated.

The issue in the source code

I have tracked the problem to core Location class line 100. It strips trailing slash away from any type of the locations (file systems, classpaths, s3, GCS):

        if (rawPath.endsWith(File.separator)) {
            rawPath = rawPath.substring(0, rawPath.length() - 1);
        }

This stripping has been done already before the existence of the S3 and GCS location support, so most likely it's accidentally there for them.

Possible fix

Just don't do the stripping for cloud buckets? Though, this could theoretically break some existing deployments of the Flyway. In that case, some kind of "preserve trailing flash" switch could be introduced, though it's more dirty.

ikkala commented 2 years ago

As workaround, one can use double-slash in the end of the S3 location, for example "s3:testbucket/testpath/1.2.1//" in the example above. The current implementation removes just one of the slashes and therefore AWS S3 is scanned with the prefix string "s3:testbucket/testpath/1.2.1/".

Barry-RG commented 2 years ago

Thank you for your issue. As this could be a breaking change and there is a work around we will close this issue for now and return upon further interest.