From 4a574789a4589b95fc53bb358f7d77aa1c2bdf2e Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Tue, 29 Sep 2020 11:26:05 -0400 Subject: [PATCH] Create a flyway index file and verify correctness (#819) * Create a flyway index file and verify correctness Create an index file (flyway.txt) containing the names of all of the flyway files and verify that it is ordered and in sync with the actual contents of the flyway directory. Also provide a target (generateFlywayIndex) to automatically generate it. The purpose of flyway.txt is to cause a merge conflict in the event that two different developers add a flyway file with the same sequence number, an event which has occurred multiple times. --- build.gradle | 2 +- config/presubmits.py | 90 ++++++++++++++++++++++++++++ db/README.md | 5 ++ db/build.gradle | 22 +++++++ db/src/main/resources/sql/flyway.txt | 58 ++++++++++++++++++ 5 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 db/src/main/resources/sql/flyway.txt diff --git a/build.gradle b/build.gradle index 6063a7ed8..c506fb99f 100644 --- a/build.gradle +++ b/build.gradle @@ -191,7 +191,7 @@ allprojects { } task runPresubmits(type: Exec) { - executable '/usr/bin/python' + executable '/usr/bin/python3' args('config/presubmits.py') } diff --git a/config/presubmits.py b/config/presubmits.py index 3d827aaba..44fb511f4 100644 --- a/config/presubmits.py +++ b/config/presubmits.py @@ -18,6 +18,7 @@ Error Prone) so we must write them manually. """ import os +from typing import List, Tuple import sys import re @@ -178,6 +179,90 @@ PRESUBMITS = { "JavaScript files should not include console logging." } +# Note that this regex only works for one kind of Flyway file. If we want to +# start using "R" and "U" files we'll need to update this script. +FLYWAY_FILE_RX = re.compile(r'V(\d+)__.*') + + +def get_seqnum(filename: str, location: str) -> int: + """Extracts the sequence number from a filename.""" + m = FLYWAY_FILE_RX.match(filename) + if m is None: + raise ValueError('Illegal Flyway filename: %s in %s' % (filename, location)) + return int(m.group(1)) + + +def files_by_seqnum(files: List[str], location: str) -> List[Tuple[int, str]]: + """Returns the list of seqnum, filename sorted by sequence number.""" + return [(get_seqnum(filename, location), filename) for filename in files] + + +def has_valid_order(indexed_files: List[Tuple[int, str]], location: str) -> bool: + """Verify that sequence numbers are in order without gaps or duplicates. + + Args: + files: List of seqnum, filename for a list of Flyway files. + location: Where the list of files came from (for error reporting). + + Returns: + True if the file list is valid. + """ + last_index = 0 + valid = True + for seqnum, filename in indexed_files: + if seqnum == last_index: + print('duplicate Flyway file sequence number found in %s: %s' % + (location, filename)) + valid = False + elif seqnum < last_index: + print('File %s in %s is out of order.' % (filename, location)) + valid = False + elif seqnum != last_index + 1: + print('Missing Flyway sequence number %d in %s. Next file is %s' % + (last_index + 1, location, filename)) + valid = False + last_index = seqnum + return valid + + +def verify_flyway_index(): + """Verifies that the Flyway index file is in sync with the directory.""" + success = True + + # Sort the files in the Flyway directory by their sequence number. + files = sorted( + files_by_seqnum(os.listdir('db/src/main/resources/sql/flyway'), + 'Flyway directory')) + + # Make sure that there are no gaps and no duplicate sequence numbers in the + # files themselves. + if not has_valid_order(files, 'Flyway directory'): + success = False + + # Remove the sequence numbers and compare against the index file contents. + files = [filename[1] for filename in sorted(files)] + with open('db/src/main/resources/sql/flyway.txt') as index: + indexed_files = index.read().splitlines() + if files != indexed_files: + unindexed = set(files) - set(indexed_files) + if unindexed: + print('The following Flyway files are not in flyway.txt: %s' % unindexed) + + nonexistent = set(indexed_files) - set(files) + if nonexistent: + print('The following files are in flyway.txt but not in the Flyway ' + 'directory: %s' % nonexistent) + + # Do an ordering check on the index file (ignore the result, we're failing + # anyway). + has_valid_order(files_by_seqnum(indexed_files, 'flyway.txt'), 'flyway.txt') + success = False + + if not success: + print('Please fix any conflicts and run "./nom_build :db:generateFlywayIndex"') + + return not success + def get_files(): for root, dirnames, filenames in os.walk("."): @@ -197,5 +282,10 @@ if __name__ == "__main__": failed = True print("%s had errors: \n %s" % (file, "\n ".join(error_messages))) + # And now for something completely different: check to see if the Flyway + # index is up-to-date. It's quicker to do it here than in the unit tests: + # when we put it here it fails fast before all of the tests are run. + failed |= verify_flyway_index() + if failed: sys.exit(1) diff --git a/db/README.md b/db/README.md index a082a5c44..f8abe7007 100644 --- a/db/README.md +++ b/db/README.md @@ -61,6 +61,11 @@ Below are the steps to submit a schema change: You'll want to have a look at the diffs in the golden schema to verify that all changes are intentional. +5. Run ./nom_build :db:generateFlywayIndex to regenerate the Flyway index. + This is a file listing all of the current Flyway files. Its purpose is to + produce a merge conflict when more than one person adds a Flyway file with + the same sequence number. + Relevant files (under db/src/main/resources/sql/schema/): * nomulus.golden.sql is the schema dump (pg_dump for postgres) of the final diff --git a/db/build.gradle b/db/build.gradle index 888e0eaa7..a8efe0c82 100644 --- a/db/build.gradle +++ b/db/build.gradle @@ -170,6 +170,28 @@ dependencies { testCompile project(path: ':common', configuration: 'testing') } +task generateFlywayIndex { + def flywayBase = "$projectDir/src/main/resources/sql/flyway" + def filenamePattern = /V(\d+)__.*\.sql/ + + def getSeqNum = { file -> + def match = file.getName() =~ filenamePattern + if (match.size() != 1) { + throw new IllegalArgumentException("Bad Flyway filename: $file") + } + return match[0][1] as int + } + + doLast { + def files = new File(flywayBase).listFiles() + def indexFile = new File("${flywayBase}.txt") + indexFile.write '' + for (def file : files.sort{a, b -> getSeqNum(a) <=> getSeqNum(b)}) { + indexFile << "${file.name}\n" + } + } +} + flywayInfo.dependsOn('buildNeeded') flywayValidate.dependsOn('buildNeeded') diff --git a/db/src/main/resources/sql/flyway.txt b/db/src/main/resources/sql/flyway.txt new file mode 100644 index 000000000..fbb5317bd --- /dev/null +++ b/db/src/main/resources/sql/flyway.txt @@ -0,0 +1,58 @@ +V1__create_claims_list_and_entry.sql +V2__create_premium_list_and_entry.sql +V3__create_registry_lock.sql +V4__registry_lock_add_index_on_verification_code.sql +V5__update_premium_list.sql +V6__premium_list_bloom_filter.sql +V7__update_claims_list.sql +V8__registry_lock_registrar_index.sql +V9__premium_list_currency_type.sql +V10__create_reserved_list_and_entry.sql +V11__premium_entry_reorder_column.sql +V12__create_cursor.sql +V13__refactor_registry_lock.sql +V14__load_extension_for_hstore.sql +V15__add_epp_resources.sql +V16__create_registrar.sql +V17__create_registrar_poc.sql +V18__create_lock.sql +V19__add_registry_relock_reference.sql +V20__add_relock_duration.sql +V21__add_registry_lock_email_to_poc.sql +V22__update_ns_hosts.sql +V23__create_contact.sql +V24__domain_base_contacts.sql +V25__rename_vkey_fields.sql +V26__create_billing_event.sql +V27__create_pollmessage.sql +V28__superordinate_domain_vkey.sql +V29__add_columns_for_transfer_data.sql +V30__inet_address_converter.sql +V31__client_id_to_registrar_id.sql +V32__drop_unused_transafer_data_columns_in_contact.sql +V33__create_host_history.sql +V34__rename_fully_qualified_names.sql +V35__rename_allow_list.sql +V36__create_safebrowsing_threats.sql +V37__update_spec11threatmatch.sql +V38__create_contact_history.sql +V39__add_updatetime_column.sql +V40__spec11threatmatch_remove_registrar_foreign_key.sql +V41__add_columns_to_domain.sql +V42__add_txn_table.sql +V43__update_relock_duration_type.sql +V44__create_domain_history.sql +V45__add_grace_period_table.sql +V46__Contact_contactId_index_to_non_unique.sql +V47__remove_spec11_domain_foreign_key.sql +V48__domain_add_autorenew_end_time_column.sql +V49__create_allocation_token.sql +V50__use_composite_key_for_registrar_poc.sql +V51__use_composite_primary_key_for_domain_history_table.sql +V52__update_billing_constraint.sql +V53__add_temp_history_id_sequence.sql +V54__add_tld_table.sql +V55__domain_history_fields.sql +V56__rename_host_table.sql +V57__history_null_content.sql +V58__drop_default_value_and_sequences_for_billing_event.sql