From 3ae06937322cef3e97fbe30c8b98ca7d716b93da Mon Sep 17 00:00:00 2001 From: Max Stoliar Date: Tue, 2 Aug 2016 06:14:46 +0300 Subject: [PATCH 1/5] added yaml support to expend_keys, #54 --- lib/consul/index.js | 32 ++++++++++++++++++++++++++++++-- package.json | 4 +++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/lib/consul/index.js b/lib/consul/index.js index 9698410..3d84495 100644 --- a/lib/consul/index.js +++ b/lib/consul/index.js @@ -1,6 +1,8 @@ var _ = require('underscore'); var fs = require('fs'); var path = require('path'); +var yaml = require('js-yaml'); +var eachAsync = require('each-async'); var utils = require('../utils.js'); var logger = require('../logging.js'); @@ -104,7 +106,7 @@ var file_modified = function(branch, file, cb) { fs.readFile(file_path, {encoding: 'utf8'}, function (err, body) { /* istanbul ignore if */ if (err) return cb('Failed to read key ' + file_path + ' due to ' + err); - var body = body ? body.trim() : ''; + body ? body.trim() : ''; try { var obj = JSON.parse(body); populate_kvs_from_object(branch, create_key_name(branch, file), obj, cb); @@ -148,11 +150,32 @@ var file_modified = function(branch, file, cb) { }); }; + var handle_yaml_kv_file = function(file_path, cb) { + fs.readFile(file_path, {encoding: 'utf8'}, function (err, body) { + /* istanbul ignore if */ + if (err) return cb('Failed to read key ' + file_path + ' due to ' + err); + body ? body.trim() : ''; + var docs = []; + try { + yaml.loadAll(body, function(doc) { + docs.push(doc); + }); + + eachAsync(docs, function(doc,index,cb) { + populate_kvs_from_object(branch, create_key_name(branch, file), doc, cb); + }, cb); + } catch (e) { + logger.warn("Failed to parse .yaml file " + file + ". Using body string as a KV.",e); + write_content_to_consul(create_key_name(branch, file), body, cb); + } + }); + }; + var handle_as_flat_file = function(fqf, branch, file, cb) { fs.readFile(fqf, {encoding: 'utf8'}, function (err, body) { /* istanbul ignore if */ if (err) return cb('Failed to read key ' + fqf + ' due to ' + err); - var body = body ? body.trim() : ''; + body ? body.trim() : ''; write_content_to_consul(create_key_name(branch, file), body, cb); }); }; @@ -170,6 +193,11 @@ var file_modified = function(branch, file, cb) { if (err) return cb('Failed to delete key ' + key_name + ' due to ' + err); handle_properties_kv_file(fqf, branch.common_properties, cb); }); + } else if (file.endsWith('.yaml') || file.endsWith('.yml')) { + file_deleted(branch, file, function (err) { + if (err) return cb('Failed to delete key ' + key_name + ' due to ' + err); + handle_yaml_kv_file(fqf, branch.common_properties, cb); + }); } else { handle_as_flat_file(fqf, branch, file, cb); } diff --git a/package.json b/package.json index 1303fb4..42d9443 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,9 @@ "mkdirp": "0.5.0", "rimraf": "2.2.8", "underscore": "^1.8.0", - "properties": "1.2.1" + "properties": "1.2.1", + "js-yaml": "^3.6.1", + "each-async": "^1.1.1" }, "devDependencies": { "grunt": "^0.4.5", From e9ffb760344fab06cfbb210700d2697ea96c0cab Mon Sep 17 00:00:00 2001 From: Chris Stevens Date: Sun, 21 Aug 2016 20:16:32 -0500 Subject: [PATCH 2/5] Switch to js-yaml safeLoad from loadAll After reviewing the js-yaml documentation and examples, the primary benefit of "loadAll" over "load" or even "safeLoad" is multi-document support, which seems overly complicated for this use case. By switching, eachAsync was no longer needed and testing was more straightforward. Fixed a serious bug when reacting to yaml file_deleted that had already been observed while using the yaml expansion against real data. --- lib/consul/index.js | 22 +++++++--------------- package.json | 3 +-- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/lib/consul/index.js b/lib/consul/index.js index 3d84495..d25f304 100644 --- a/lib/consul/index.js +++ b/lib/consul/index.js @@ -2,7 +2,6 @@ var _ = require('underscore'); var fs = require('fs'); var path = require('path'); var yaml = require('js-yaml'); -var eachAsync = require('each-async'); var utils = require('../utils.js'); var logger = require('../logging.js'); @@ -118,7 +117,6 @@ var file_modified = function(branch, file, cb) { }; var handle_properties_kv_file = function(file_path, common_properties_relative_path, cb) { - function extract_and_populate_properties(file_body, common_body, cb) { utils.load_properties(file_body, common_body, function (error, obj) { if (error) { @@ -155,17 +153,11 @@ var file_modified = function(branch, file, cb) { /* istanbul ignore if */ if (err) return cb('Failed to read key ' + file_path + ' due to ' + err); body ? body.trim() : ''; - var docs = []; try { - yaml.loadAll(body, function(doc) { - docs.push(doc); - }); - - eachAsync(docs, function(doc,index,cb) { - populate_kvs_from_object(branch, create_key_name(branch, file), doc, cb); - }, cb); + var obj = yaml.safeLoad(body); + populate_kvs_from_object(branch, create_key_name(branch, file), obj, cb); } catch (e) { - logger.warn("Failed to parse .yaml file " + file + ". Using body string as a KV.",e); + logger.warn("Failed to parse .yaml file " + file + ". Using body string as a KV."); write_content_to_consul(create_key_name(branch, file), body, cb); } }); @@ -188,15 +180,15 @@ var file_modified = function(branch, file, cb) { if (err) return cb('Failed to delete key ' + key_name + ' due to ' + err); handle_json_kv_file(fqf, cb); }); - } else if (file.endsWith('.properties')) { + } else if (file.endsWith('.yaml') || file.endsWith('.yml')) { file_deleted(branch, file, function (err) { if (err) return cb('Failed to delete key ' + key_name + ' due to ' + err); - handle_properties_kv_file(fqf, branch.common_properties, cb); + handle_yaml_kv_file(fqf, cb); }); - } else if (file.endsWith('.yaml') || file.endsWith('.yml')) { + } else if (file.endsWith('.properties')) { file_deleted(branch, file, function (err) { if (err) return cb('Failed to delete key ' + key_name + ' due to ' + err); - handle_yaml_kv_file(fqf, branch.common_properties, cb); + handle_properties_kv_file(fqf, branch.common_properties, cb); }); } else { handle_as_flat_file(fqf, branch, file, cb); diff --git a/package.json b/package.json index 42d9443..439b7c5 100644 --- a/package.json +++ b/package.json @@ -23,8 +23,7 @@ "rimraf": "2.2.8", "underscore": "^1.8.0", "properties": "1.2.1", - "js-yaml": "^3.6.1", - "each-async": "^1.1.1" + "js-yaml": "^3.6.1" }, "devDependencies": { "grunt": "^0.4.5", From 6f418215a0f1520ae9c6fc42bd67e8aa1c47ee0a Mon Sep 17 00:00:00 2001 From: Chris Stevens Date: Sun, 21 Aug 2016 20:16:59 -0500 Subject: [PATCH 3/5] Added expand_keys tests for yaml files --- test/git2consul_expand_keys_test.js | 115 +++++++++++++++++++++++++++- 1 file changed, 114 insertions(+), 1 deletion(-) diff --git a/test/git2consul_expand_keys_test.js b/test/git2consul_expand_keys_test.js index 8738054..4176511 100644 --- a/test/git2consul_expand_keys_test.js +++ b/test/git2consul_expand_keys_test.js @@ -147,6 +147,119 @@ describe('Expand keys', function() { }); }); + /*YAML*/ + + it ('should handle additions of new YAML files', 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.yaml/first_level/second_level', 'is_all_we_need', function(err, value) { + if (err) return done(err); + done(); + }); + }); + }); + }); + + it ('should handle changing YAML files', function(done) { + var sample_key = 'changeme.yaml'; + var sample_value = "---\n\nfirst_level:\n 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/changeme.yaml/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.yaml/first_level', 'is super different', function(err, value) { + if (err) return done(err); + + done(); + }); + }); + }); + }); + }); + }); + }); + + it ('should handle busted YAML files', function(done) { + var sample_key = 'busted.yaml'; + // from: js-yaml / test / samples-load-errors / forbidden-value.yml + var sample_value = "---\n\ntest: key: value\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/busted.yaml', sample_value, 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\nnot_busted: yaml\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/busted.yaml/not_busted', 'yaml', function(err, value) { + if (err) return done(err); + + done(); + }); + }); + }); + }); + }); + }); + }); + + it ('should handle YAML files with special characters', function(done) { + var sample_key = 'special.yaml'; + var sample_value = "---\n\nfuzzy:\n second level: ain\'t no one got time for that\n second/level:\n ok?: yes\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/special.yaml/fuzzy/second%20level', "ain\'t no one got time for that", function(err, value) { + // At this point, the repo should have populated consul with our sample_key + consul_utils.validateValue('test_repo/master/special.yaml/fuzzy/second%2Flevel/ok%3F', 'yes', function(err, value) { + if (err) return done(err); + done(); + }); + }); + }); + }); + }); + /*properties*/ it ('should handle additions of new properties files', function(done) { @@ -473,4 +586,4 @@ describe('Expand keys with invalid common properties path ', function() { }); }); -}); \ No newline at end of file +}); From 88fa6762a14fc210208b62c9e9ae9dda8b4bb5d1 Mon Sep 17 00:00:00 2001 From: Chris Stevens Date: Sun, 21 Aug 2016 20:24:40 -0500 Subject: [PATCH 4/5] Update README to add YAML expansion examples --- README.md | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 210e740..90f00a8 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ git2consul expects to be run on the same node as a Consul agent. git2consul exp }] },{ "name" : "github_data", - "mode" : "expand_keys", + "expand_keys" : true, "url" : "git@github.com:ryanbreen/git2consul_data.git", "branches" : [ "master" ], "hooks": [{ @@ -203,6 +203,30 @@ A few notes on how this behaves: * Any non-JSON files, including files with the extension ".json" that contain invalid JSON, are stored in your KV as if expand_keys mode was not enabled. +###### YAML + +Similarly to JSON, git2consul can treat YAML documents in your repo as fully formed subtrees. + +```yaml +--- +# file: example.yaml or example.yml +first_level: + second_level: + third_level: + my_key: my_value +``` + +git2consul in expand_keys mode will generate the following KV: + +``` +/expando_keys/example.yaml/first_level/second_level/third_level/my_key +or +/expando_keys/example.yml/first_level/second_level/third_level/my_key +``` + +The value in that KV pair will be `my_value`. + + ###### .properties Similarly to JSON, git2consul can also treat [Java .properties](http://docs.oracle.com/javase/7/docs/api/java/util/Properties.html#load%28java.io.Reader%29) as a simple k/v format. From aac0ca35fc3b96fb0462cc959fa2c00c07d283d9 Mon Sep 17 00:00:00 2001 From: Chris Stevens Date: Mon, 22 Aug 2016 06:44:27 -0500 Subject: [PATCH 5/5] Ensure the trimmed body is used --- lib/consul/index.js | 6 +++--- test/git2consul_expand_keys_test.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/consul/index.js b/lib/consul/index.js index d25f304..dd70a63 100644 --- a/lib/consul/index.js +++ b/lib/consul/index.js @@ -105,7 +105,7 @@ var file_modified = function(branch, file, cb) { fs.readFile(file_path, {encoding: 'utf8'}, function (err, body) { /* istanbul ignore if */ if (err) return cb('Failed to read key ' + file_path + ' due to ' + err); - body ? body.trim() : ''; + body = body ? body.trim() : ''; try { var obj = JSON.parse(body); populate_kvs_from_object(branch, create_key_name(branch, file), obj, cb); @@ -152,7 +152,7 @@ var file_modified = function(branch, file, cb) { fs.readFile(file_path, {encoding: 'utf8'}, function (err, body) { /* istanbul ignore if */ if (err) return cb('Failed to read key ' + file_path + ' due to ' + err); - body ? body.trim() : ''; + body = body ? body.trim() : ''; try { var obj = yaml.safeLoad(body); populate_kvs_from_object(branch, create_key_name(branch, file), obj, cb); @@ -167,7 +167,7 @@ var file_modified = function(branch, file, cb) { fs.readFile(fqf, {encoding: 'utf8'}, function (err, body) { /* istanbul ignore if */ if (err) return cb('Failed to read key ' + fqf + ' due to ' + err); - body ? body.trim() : ''; + body = body ? body.trim() : ''; write_content_to_consul(create_key_name(branch, file), body, cb); }); }; diff --git a/test/git2consul_expand_keys_test.js b/test/git2consul_expand_keys_test.js index 4176511..2f4dc53 100644 --- a/test/git2consul_expand_keys_test.js +++ b/test/git2consul_expand_keys_test.js @@ -214,7 +214,7 @@ describe('Expand keys', function() { 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/busted.yaml', sample_value, function(err, value) { + consul_utils.validateValue('test_repo/master/busted.yaml', "---\n\ntest: key: value", 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.