From 84d38bd2525debf0bba03f1703323ad2f871e516 Mon Sep 17 00:00:00 2001 From: moczkows Date: Thu, 16 Aug 2018 14:37:45 +0200 Subject: [PATCH 1/7] adding '/' to consul kv del path to avoid gready pattern matching and remove desired key/subtree --- lib/consul/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/consul/index.js b/lib/consul/index.js index 5985a84..af7e3ee 100644 --- a/lib/consul/index.js +++ b/lib/consul/index.js @@ -289,7 +289,7 @@ var file_deleted = function(branch, file, cb) { logger.trace('Deleting key %s', key_name); // Delete this key. Or, if mode is branch.expand_keys, delete all files underneath this key. - consul.kv.del({'key': key_name, token: token, recurse: branch.expand_keys}, function(err) { + consul.kv.del({'key': key_name + '/', token: token, recurse: branch.expand_keys}, function(err) { /* istanbul ignore if */ if (err) return cb('Failed to delete key ' + key_name + ' due to ' + err); cb(); From 54128f64e5b679689588f7943a2740f8e2f049a4 Mon Sep 17 00:00:00 2001 From: Ryan Breen Date: Mon, 27 Aug 2018 09:22:48 +0000 Subject: [PATCH 2/7] adding failing test for issue --- .../git2consul_expand_no_extensions_update.js | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 test/git2consul_expand_no_extensions_update.js diff --git a/test/git2consul_expand_no_extensions_update.js b/test/git2consul_expand_no_extensions_update.js new file mode 100644 index 0000000..745ce58 --- /dev/null +++ b/test/git2consul_expand_no_extensions_update.js @@ -0,0 +1,106 @@ +var should = require('should'); +var _ = require('underscore'); + +var mkdirp = require('mkdirp'); + +var logger = require('../lib/logging.js'); + +// We want this above any git2consul module to make sure logging gets configured +require('./git2consul_bootstrap_test.js'); + +var repo = require('../lib/git/repo.js'); +var git_utils = require('./utils/git_utils.js'); +var consul_utils = require('./utils/consul_utils.js'); + +var git_commands = require('../lib/git/commands.js'); + +describe('Expand keys', function() { + + // The current copy of the git master branch. This is initialized before each test in the suite. + var branch; + beforeEach(function(done) { + + // Each of these tests needs a working repo instance, so create it here and expose it to the suite + // namespace. These are all tests of expand_keys mode, so set that here. + git_utils.initRepo(_.extend(git_utils.createRepoConfig(), {'expand_keys': true,"ignore_file_extension" : true}), function(err, repo) { + if (err) return done(err); + + // The default repo created by initRepo has a single branch, master. + branch = repo.branches['master']; + + done(); + }); + }); + + /*YAML*/ + + it ('should handle additions of new YAML files when expand_keys and ignore_file_extensions set', function(done) { + var sample_key = 'simple.yaml'; + var sample_value = "---\n\nfirst_level:\n second_level: is_all_we_need\n"; + + // Add the file, call branch.handleRef to sync the commit, then validate that consul contains the correct info. + git_utils.addFileToGitRepo(sample_key, sample_value, "Add a file.", function(err) { + if (err) return done(err); + + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // At this point, the repo should have populated consul with our sample_key + consul_utils.validateValue('test_repo/master/simple/first_level/second_level', 'is_all_we_need', function(err, value) { + if (err) return done(err); + done(); + }); + }); + }); + }); + + + + it ('should handle changing YAML files and not removing another key with suffix when expand_keys and ignore_file_extensions set', function(done) { + var sample_key = 'changeme.yaml'; + var sample_value = "---\n\nfirst_level:\n is_all_we_need\n"; + + var sample_key2 = 'changeme_had-dev.yaml'; + var sample_value2 = "---\n\nfirst_level:\n plikdwa\n"; + + + // Add the file, call branch.handleRef to sync the commit, then validate that consul contains the correct info. + git_utils.addFileToGitRepo(sample_key, sample_value, "Add a file.", function(err) { + if (err) return done(err); + git_utils.addFileToGitRepo(sample_key2, sample_value2, "Add a file.", function(err) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // At this point, the repo should have populated consul with our sample_key + consul_utils.validateValue('test_repo/master/changeme/first_level', 'is_all_we_need', function(err, value) { + if (err) return done(err); + + // Add the file, call branch.handleRef to sync the commit, then validate that consul contains the correct info. + git_utils.addFileToGitRepo(sample_key, "---\n\nfirst_level:\n is super different\n", "Change a file.", function(err) { + if (err) return done(err); + + branch.handleRefChange(0, function(err) { + if (err) return done(err); + + // At this point, the repo should have populated consul with our sample_key + consul_utils.validateValue('test_repo/master/changeme/first_level', 'is super different', function(err, value) { + if (err) return done(err); + + consul_utils.validateValue('test_repo/master/changeme_had-dev/first_level', 'plikdwa', function(err, value) { + if (err) return done(err); + done(); + }); + }); + }); + }); + }); + + }); + }); + }); + + }); + + + + +}); From 33c83e950af86d759c79275bc6667ae8ff4a8f4d Mon Sep 17 00:00:00 2001 From: Ryan Breen Date: Mon, 27 Aug 2018 12:16:59 +0000 Subject: [PATCH 3/7] additional test added --- ...ic_operations_expand_no_extensions_test.js | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 test/git2consul_file_basic_operations_expand_no_extensions_test.js diff --git a/test/git2consul_file_basic_operations_expand_no_extensions_test.js b/test/git2consul_file_basic_operations_expand_no_extensions_test.js new file mode 100644 index 0000000..46db3a7 --- /dev/null +++ b/test/git2consul_file_basic_operations_expand_no_extensions_test.js @@ -0,0 +1,138 @@ +var should = require('should'); +var _ = require('underscore'); + +var mkdirp = require('mkdirp'); + +// We want this above any git2consul module to make sure logging gets configured +require('./git2consul_bootstrap_test.js'); + +var repo = require('../lib/git/repo.js'); +var git_utils = require('./utils/git_utils.js'); +var consul_utils = require('./utils/consul_utils.js'); + +var git_commands = require('../lib/git/commands.js'); + +describe('File operations', function() { + + // The current copy of the git master branch. This is initialized before each test in the suite. + var branch; + beforeEach(function(done) { + + // Each of these tests needs a working repo instance, so create it here and expose it to the suite + // namespace. + git_utils.initRepo(_.extend(git_utils.createRepoConfig(), {'expand_keys': true,"ignore_file_extension" : true}), function(err, repo) { + if (err) return done(err); + + // The default repo created by initRepo has a single branch, master. + branch = repo.branches['master']; + + done(); + }); + }); + + it ('should handle deletions of existing files - expand ignore file extensions', function(done) { + var sample_key = 'sample_key_to_delete'; + var sample_value = 'secret!'; + + // Add the file, call branch.handleRef to sync the commit, then delete the file and sync again. + // Finally, validate that consul contains the correct info. + git_utils.addFileToGitRepo(sample_key, sample_value, "Create file to delete.", function(err) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // Validate that the file was added to consul before we delete it + consul_utils.validateValue('test_repo/master/' + sample_key, sample_value, function(err, value) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + git_utils.deleteFileFromGitRepo(sample_key, "Delete file.", function(err) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // At this point, the branch should have removed our sample_key from consul. + consul_utils.validateValue('test_repo/master/' + sample_key, undefined, function(err, value) { + if (err) return done(err); + done(); + }); + }); + }); + }); + }); + }); + }); + }); + + it ('should handle moving an existing file - expand ignore file extensions', function(done) { + var sample_key = 'sample_movable_key'; + var sample_moved_key = 'sample_moved_key'; + var sample_value = 'movable value'; + + // Add the file, call branch.handleRef to sync the commit, then move the file and sync again. + // Finally, validate that consul contains the correct info. + git_utils.addFileToGitRepo(sample_key, sample_value, "Create file to move.", function(err) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // Validate that the file was added to consul before we move it + consul_utils.validateValue('test_repo/master/' + sample_key, sample_value, function(err, value) { + if (err) return done(err); + git_utils.moveFileInGitRepo(sample_key, sample_moved_key, "Move file.", function(err) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // At this point, the repo should have populated consul with our moved key, deleting the old name + consul_utils.validateValue('test_repo/master/' + sample_key, undefined, function(err) { + if (err) return done(err); + // At this point, the repo should have populated consul with our moved key, adding the new name + consul_utils.validateValue('test_repo/master/' + sample_moved_key, sample_value, function(err) { + if (err) return done(err); + done(); + }); + }); + }); + }); + }); + }); + }); + }); + + it ('should handle moving an existing file into a subfolder - expand ignore file extensions', function(done) { + var sample_key = 'sample_wrong_directory_key'; + var sample_moved_key = 'subfolder/sample_right_directory_key'; + var sample_value = 'movable value'; + + // Add the file, call branch.handleRef to sync the commit, then move the file and sync again. + // Finally, validate that consul contains the correct info. + git_utils.addFileToGitRepo(sample_key, sample_value, "Create file to move to subfolder.", function(err) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // Validate that the file was added to consul before we move it + consul_utils.validateValue('test_repo/master/' + sample_key, sample_value, function(err, value) { + if (err) return done(err); + // Create the subdirectory in the remote repo. + mkdirp(git_utils.TEST_REMOTE_REPO + 'subfolder', function(err) { + if (err) return done(err); + // Move the key to the subdirectory. + git_utils.moveFileInGitRepo(sample_key, sample_moved_key, "Move file to subfolder.", function(err) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // At this point, the repo should have populated consul with our moved key, deleting the old name + consul_utils.validateValue('test_repo/master/' + sample_key, undefined, function(err) { + if (err) return done(err); + // At this point, the repo should have populated consul with our moved key, adding the new name + consul_utils.validateValue('test_repo/master/' + sample_moved_key, sample_value, function(err) { + if (err) return done(err); + done(); + }); + }); + }); + }); + }); + }); + }); + }); + }); + +}); From 00da8338645a14f446d4fa78f26089b5bcdcfb07 Mon Sep 17 00:00:00 2001 From: Ryan Breen Date: Tue, 28 Aug 2018 12:40:52 +0000 Subject: [PATCH 4/7] adding test --- ...l_file_delete_expand_no_extensions_test.js | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 test/git2consul_file_delete_expand_no_extensions_test.js diff --git a/test/git2consul_file_delete_expand_no_extensions_test.js b/test/git2consul_file_delete_expand_no_extensions_test.js new file mode 100644 index 0000000..83b7a4f --- /dev/null +++ b/test/git2consul_file_delete_expand_no_extensions_test.js @@ -0,0 +1,65 @@ +var should = require('should'); +var _ = require('underscore'); + +var mkdirp = require('mkdirp'); + +// We want this above any git2consul module to make sure logging gets configured +require('./git2consul_bootstrap_test.js'); + +var repo = require('../lib/git/repo.js'); +var git_utils = require('./utils/git_utils.js'); +var consul_utils = require('./utils/consul_utils.js'); + +var git_commands = require('../lib/git/commands.js'); + +describe('File operations', function() { + + // The current copy of the git master branch. This is initialized before each test in the suite. + var branch; + beforeEach(function(done) { + + // Each of these tests needs a working repo instance, so create it here and expose it to the suite + // namespace. + git_utils.initRepo(_.extend(git_utils.createRepoConfig(), {'expand_keys': true,"ignore_file_extension" : true}), function(err, repo) { + if (err) return done(err); + + // The default repo created by initRepo has a single branch, master. + branch = repo.branches['master']; + + done(); + }); + }); + + it ('should handle deletions of existing files - expand ignore file extensions', function(done) { + var sample_key = 'simple.yaml'; + var sample_value = "---\n\nfirst_level:\n second_level: is_all_we_need\n"; + + // Add the file, call branch.handleRef to sync the commit, then delete the file and sync again. + // Finally, validate that consul contains the correct info. + git_utils.addFileToGitRepo(sample_key, sample_value, "Create file to delete.", function(err) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // Validate that the file was added to consul before we delete it + consul_utils.validateValue('test_repo/master/simple/first_level/second_level', 'is_all_we_need', function(err, value) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + git_utils.deleteFileFromGitRepo(sample_key, "Delete file.", function(err) { + if (err) return done(err); + branch.handleRefChange(0, function(err) { + if (err) return done(err); + // At this point, the branch should have removed our sample_key from consul. + consul_utils.validateValue('test_repo/master/simple/first_level/second_level', undefined, function(err, value) { + if (err) return done(err); + done(); + }); + }); + }); + }); + }); + }); + }); + }); + +}); From 9b8584a782d7673d31702edcbc175e75608b39aa Mon Sep 17 00:00:00 2001 From: Ryan Breen Date: Tue, 28 Aug 2018 12:43:41 +0000 Subject: [PATCH 5/7] adding test --- ...ic_operations_expand_no_extensions_test.js | 138 ------------------ 1 file changed, 138 deletions(-) delete mode 100644 test/git2consul_file_basic_operations_expand_no_extensions_test.js diff --git a/test/git2consul_file_basic_operations_expand_no_extensions_test.js b/test/git2consul_file_basic_operations_expand_no_extensions_test.js deleted file mode 100644 index 46db3a7..0000000 --- a/test/git2consul_file_basic_operations_expand_no_extensions_test.js +++ /dev/null @@ -1,138 +0,0 @@ -var should = require('should'); -var _ = require('underscore'); - -var mkdirp = require('mkdirp'); - -// We want this above any git2consul module to make sure logging gets configured -require('./git2consul_bootstrap_test.js'); - -var repo = require('../lib/git/repo.js'); -var git_utils = require('./utils/git_utils.js'); -var consul_utils = require('./utils/consul_utils.js'); - -var git_commands = require('../lib/git/commands.js'); - -describe('File operations', function() { - - // The current copy of the git master branch. This is initialized before each test in the suite. - var branch; - beforeEach(function(done) { - - // Each of these tests needs a working repo instance, so create it here and expose it to the suite - // namespace. - git_utils.initRepo(_.extend(git_utils.createRepoConfig(), {'expand_keys': true,"ignore_file_extension" : true}), function(err, repo) { - if (err) return done(err); - - // The default repo created by initRepo has a single branch, master. - branch = repo.branches['master']; - - done(); - }); - }); - - it ('should handle deletions of existing files - expand ignore file extensions', function(done) { - var sample_key = 'sample_key_to_delete'; - var sample_value = 'secret!'; - - // Add the file, call branch.handleRef to sync the commit, then delete the file and sync again. - // Finally, validate that consul contains the correct info. - git_utils.addFileToGitRepo(sample_key, sample_value, "Create file to delete.", function(err) { - if (err) return done(err); - branch.handleRefChange(0, function(err) { - if (err) return done(err); - // Validate that the file was added to consul before we delete it - consul_utils.validateValue('test_repo/master/' + sample_key, sample_value, function(err, value) { - if (err) return done(err); - branch.handleRefChange(0, function(err) { - if (err) return done(err); - git_utils.deleteFileFromGitRepo(sample_key, "Delete file.", function(err) { - if (err) return done(err); - branch.handleRefChange(0, function(err) { - if (err) return done(err); - // At this point, the branch should have removed our sample_key from consul. - consul_utils.validateValue('test_repo/master/' + sample_key, undefined, function(err, value) { - if (err) return done(err); - done(); - }); - }); - }); - }); - }); - }); - }); - }); - - it ('should handle moving an existing file - expand ignore file extensions', function(done) { - var sample_key = 'sample_movable_key'; - var sample_moved_key = 'sample_moved_key'; - var sample_value = 'movable value'; - - // Add the file, call branch.handleRef to sync the commit, then move the file and sync again. - // Finally, validate that consul contains the correct info. - git_utils.addFileToGitRepo(sample_key, sample_value, "Create file to move.", function(err) { - if (err) return done(err); - branch.handleRefChange(0, function(err) { - if (err) return done(err); - // Validate that the file was added to consul before we move it - consul_utils.validateValue('test_repo/master/' + sample_key, sample_value, function(err, value) { - if (err) return done(err); - git_utils.moveFileInGitRepo(sample_key, sample_moved_key, "Move file.", function(err) { - if (err) return done(err); - branch.handleRefChange(0, function(err) { - if (err) return done(err); - // At this point, the repo should have populated consul with our moved key, deleting the old name - consul_utils.validateValue('test_repo/master/' + sample_key, undefined, function(err) { - if (err) return done(err); - // At this point, the repo should have populated consul with our moved key, adding the new name - consul_utils.validateValue('test_repo/master/' + sample_moved_key, sample_value, function(err) { - if (err) return done(err); - done(); - }); - }); - }); - }); - }); - }); - }); - }); - - it ('should handle moving an existing file into a subfolder - expand ignore file extensions', function(done) { - var sample_key = 'sample_wrong_directory_key'; - var sample_moved_key = 'subfolder/sample_right_directory_key'; - var sample_value = 'movable value'; - - // Add the file, call branch.handleRef to sync the commit, then move the file and sync again. - // Finally, validate that consul contains the correct info. - git_utils.addFileToGitRepo(sample_key, sample_value, "Create file to move to subfolder.", function(err) { - if (err) return done(err); - branch.handleRefChange(0, function(err) { - if (err) return done(err); - // Validate that the file was added to consul before we move it - consul_utils.validateValue('test_repo/master/' + sample_key, sample_value, function(err, value) { - if (err) return done(err); - // Create the subdirectory in the remote repo. - mkdirp(git_utils.TEST_REMOTE_REPO + 'subfolder', function(err) { - if (err) return done(err); - // Move the key to the subdirectory. - git_utils.moveFileInGitRepo(sample_key, sample_moved_key, "Move file to subfolder.", function(err) { - if (err) return done(err); - branch.handleRefChange(0, function(err) { - if (err) return done(err); - // At this point, the repo should have populated consul with our moved key, deleting the old name - consul_utils.validateValue('test_repo/master/' + sample_key, undefined, function(err) { - if (err) return done(err); - // At this point, the repo should have populated consul with our moved key, adding the new name - consul_utils.validateValue('test_repo/master/' + sample_moved_key, sample_value, function(err) { - if (err) return done(err); - done(); - }); - }); - }); - }); - }); - }); - }); - }); - }); - -}); From 25948377178caaf221c583cfb30cd002cee4b32a Mon Sep 17 00:00:00 2001 From: Ryan Breen Date: Tue, 28 Aug 2018 12:45:43 +0000 Subject: [PATCH 6/7] renaming test file --- ...s_update.js => git2consul_expand_no_extensions_update_test.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{git2consul_expand_no_extensions_update.js => git2consul_expand_no_extensions_update_test.js} (100%) diff --git a/test/git2consul_expand_no_extensions_update.js b/test/git2consul_expand_no_extensions_update_test.js similarity index 100% rename from test/git2consul_expand_no_extensions_update.js rename to test/git2consul_expand_no_extensions_update_test.js From 51d4f6139023f8eec69350c471eb149e3836e4ff Mon Sep 17 00:00:00 2001 From: Ryan Breen Date: Tue, 28 Aug 2018 12:50:48 +0000 Subject: [PATCH 7/7] fixing issue with removing to many keys while update in expand mode --- lib/consul/index.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/consul/index.js b/lib/consul/index.js index af7e3ee..0384d0c 100644 --- a/lib/consul/index.js +++ b/lib/consul/index.js @@ -288,8 +288,13 @@ var file_deleted = function(branch, file, cb) { logger.trace('Deleting key %s', key_name); + if (branch.expand_keys) { + key_name += "/"; + logger.trace('appending key_name: %s', key_name); + } + // Delete this key. Or, if mode is branch.expand_keys, delete all files underneath this key. - consul.kv.del({'key': key_name + '/', token: token, recurse: branch.expand_keys}, function(err) { + consul.kv.del({'key': key_name, token: token, recurse: branch.expand_keys}, function(err) { /* istanbul ignore if */ if (err) return cb('Failed to delete key ' + key_name + ' due to ' + err); cb();