Changed interface and implementation to accept an optional target_env and

gamov · Pair · commit d7373c72b680 · 2017-01-12T18:44:02.000+08:00
cf manifest name.
Improved the tests

Signed-off-by: Natalie Tay &lt;ntay@pivotal.io&gt;
diff --git a/README.md b/README.md
@@ -335,13 +335,18 @@ To upload your local values to Heroku you could ran `bundle exec rake config:her
 
 ### Working with Cloud Foundry
 
-Cloud Foundry integration will generate a manifest adding to your CF manifest.yml the defined ENV variables under the `env` section of specified app in the yaml file.
-You must specify the app name and optionally the name of your CF manifest file:
+Cloud Foundry integration will generate a manifest from your CF manifest with the defined ENV variables added 
+under the `env` section. **ENV variables will be added to all applications specified in the manifest.** By default, 
+it uses `manifest.yml` and the current `Rails.env`:
 
-    bundle exec rake config:cf[app_name, cf_manifest.yml]
+    bundle exec rake config:cf
     
-The result of this command will have the manifest file name suffixed with the environment you ran the task in. You can then push your app with the generated manifest.    
+You may optionally pass target environment _and_ the name of your CF manifest file (in that case, both are compulsory):
 
+    bundle exec rake config:cf[target_env, your_manifest.yml]
+    
+The result of this command will create a new manifest file, name suffixed with '-merged'. You can then push your app 
+with the generated manifest.    
 
 ### Fine-tuning
 
diff --git a/lib/config/integrations/cloud_foundry.rb b/lib/config/integrations/cloud_foundry.rb
@@ -4,19 +4,20 @@
 
 module Config
   module Integrations
-    class CloudFoundry < Struct.new(:app_name, :file_path)
+    class CloudFoundry < Struct.new(:target_env, :file_path)
 
       def invoke
-        manifest_path = file_path || 'manifest.yml'
+
+        manifest_path = file_path
         file_name, _ext = manifest_path.split('.yml')
 
         manifest_hash = YAML.load(IO.read(File.join(::Rails.root, manifest_path)))
 
         puts "Generating manifest... (base cf manifest: #{manifest_path})"
 
-        merged_hash = Config::CFManifestMerger.new(app_name, manifest_hash).add_to_env
+        merged_hash = Config::CFManifestMerger.new(target_env, manifest_hash).add_to_env
 
-        target_manifest_path = File.join(::Rails.root, "#{file_name}-#{::Rails.env}.yml")
+        target_manifest_path = File.join(::Rails.root, "#{file_name}-merged.yml")
         IO.write(target_manifest_path, merged_hash.to_yaml)
 
         puts "File #{target_manifest_path} generated."
diff --git a/lib/config/integrations/helpers/cf_manifest_merger.rb b/lib/config/integrations/helpers/cf_manifest_merger.rb
@@ -4,25 +4,25 @@ module Config
   class CFManifestMerger
     include Integrations::Helpers
 
-    def initialize(app_name, manifest_hash)
-      @app_name = app_name
-      @manifest_hash = manifest_hash
-      raise ArgumentError.new("Manifest path & app name must be specified") unless @app_name && @manifest_hash
-    end
-
-    def add_to_env
+    def initialize(target_env, manifest_hash)
+      @manifest_hash = manifest_hash.dup
 
-      settings_hash = Config.const_get(Config.const_name).to_hash.stringify_keys
+      raise ArgumentError.new('Target environment & manifest path must be specified') unless target_env && @manifest_hash
 
-      prefix_keys_with_const_name_hash = to_dotted_hash(settings_hash, namespace: Config.const_name)
+      config_setting_files = Config.setting_files(Rails.root, target_env)
+      @settings_hash = Config.load_files(config_setting_files).to_hash.stringify_keys
+    end
 
-      app_hash = @manifest_hash['applications'].detect { |hash| hash['name'] == @app_name }
+    def add_to_env
 
-      raise ArgumentError, "Application '#{@app_name}' is not specified in your manifest" if app_hash.nil?
+      prefix_keys_with_const_name_hash = to_dotted_hash(@settings_hash, namespace: Config.const_name)
 
-      check_conflicting_keys(app_hash['env'], settings_hash)
+      apps = @manifest_hash['applications']
 
-      app_hash['env'].merge!(prefix_keys_with_const_name_hash)
+      apps.each do |app|
+        check_conflicting_keys(app['env'], @settings_hash)
+        app['env'].merge!(prefix_keys_with_const_name_hash)
+      end
 
       @manifest_hash
     end
diff --git a/lib/config/tasks/cloud_foundry.rake b/lib/config/tasks/cloud_foundry.rake
@@ -3,8 +3,14 @@ require 'config/integrations/cloud_foundry'
 namespace 'config' do
 
   desc 'Create a cf manifest with the env variables defined by config under current environment'
-  task :'cf', [:app_name, :file_path] => :environment do |_, args|
-    Config::Integrations::CloudFoundry.new(args[:app_name], args[:file_path]).invoke
+  task 'cf', [:target_env, :file_path] => [:environment] do |_, args|
+
+    raise ArgumentError, 'Both target_env and file_path arguments must be specified' if args.length == 1
+
+    default_args = {:target_env => Rails.env, :file_path => 'manifest.yml'}
+    merged_args = default_args.merge(args)
+
+    Config::Integrations::CloudFoundry.new(*merged_args.values).invoke
   end
 
 end
diff --git a/spec/fixtures/cf/settings/conflict_settings.yml b/spec/fixtures/cf/settings/conflict_settings.yml
diff --git a/spec/fixtures/cf/settings/multilevel_settings.yml b/spec/fixtures/cf/settings/multilevel_settings.yml
diff --git a/spec/integrations/helpers/cf_manifest_merger_spec.rb b/spec/integrations/helpers/cf_manifest_merger_spec.rb
@@ -3,61 +3,67 @@
 
 describe Config::CFManifestMerger do
 
-  after do
-    Settings.reload_from_files("#{fixture_path}/settings.yml")
-  end
-
-  it 'raises an argument error if you do not specify an app name' do
-    expect {
-      Config::CFManifestMerger.new(nil, load_manifest('cf_manifest.yml'))
-    }.to raise_error(ArgumentError, 'Manifest path & app name must be specified')
-  end
+  let(:mocked_rails_root_path) { "#{fixture_path}/cf/" }
+  let(:manifest_hash) { load_manifest('cf_manifest.yml') }
 
-  it 'raises an argument error if the application name is not found in the manifest' do
+  it 'raises an argument error if you do not specify a target environment' do
     expect {
-      Config::CFManifestMerger.new('undefined', load_manifest('cf_manifest.yml')).add_to_env
-    }.to raise_error(ArgumentError, "Application 'undefined' is not specified in your manifest")
+      Config::CFManifestMerger.new(nil, manifest_hash)
+    }.to raise_error(ArgumentError, 'Target environment & manifest path must be specified')
   end
 
-  it 'returns the cf manifest template if no settings available' do
-    merger = Config::CFManifestMerger.new('app_name', load_manifest('cf_manifest.yml'))
-    Config.load_and_set_settings ''
+  it 'returns the cf manifest unmodified if no settings are available' do
+    merger = Config::CFManifestMerger.new('test', manifest_hash)
 
     resulting_hash = merger.add_to_env
-    expect(resulting_hash).to eq(load_manifest('cf_manifest.yml'))
+    expect(resulting_hash).to eq(manifest_hash)
   end
 
-  it 'merges the given YAML file with the cf manifest YAML file' do
-    merger = Config::CFManifestMerger.new('some-cf-app', load_manifest('cf_manifest.yml'))
-    Config.load_and_set_settings "#{fixture_path}/cf/cf_multilevel.yml"
+  it 'adds the settings for the target_env to the manifest_hash' do
+    allow(Rails).to receive(:root).and_return(mocked_rails_root_path)
+
+    merger = Config::CFManifestMerger.new('multilevel_settings', manifest_hash)
 
     resulting_hash = merger.add_to_env
     expect(resulting_hash).to eq({
-                                     "applications" => [
+                                     'applications' => [
                                          {
-                                             "name" => "some-cf-app",
-                                             "instances" => 1,
-                                             "env" => {
-                                                 "DEFAULT_HOST" => "host",
-                                                 "DEFAULT_PORT" => "port",
-                                                 "FOO" => "BAR",
-                                                 "Settings.world.capitals.europe.germany" => "Berlin",
-                                                 "Settings.world.capitals.europe.poland" => "Warsaw",
-                                                 "Settings.world.array.0.name" => "Alan",
-                                                 "Settings.world.array.1.name" => "Gam",
-                                                 "Settings.world.array_with_index.0.name" => "Bob",
-                                                 "Settings.world.array_with_index.1.name" => "William"
+                                             'name' => 'some-cf-app',
+                                             'instances' => 1,
+                                             'env' => {
+                                                 'DEFAULT_HOST' => 'host',
+                                                 'DEFAULT_PORT' => 'port',
+                                                 'FOO' => 'BAR',
+                                                 'Settings.world.capitals.europe.germany' => 'Berlin',
+                                                 'Settings.world.capitals.europe.poland' => 'Warsaw',
+                                                 'Settings.world.array.0.name' => 'Alan',
+                                                 'Settings.world.array.1.name' => 'Gam',
+                                                 'Settings.world.array_with_index.0.name' => 'Bob',
+                                                 'Settings.world.array_with_index.1.name' => 'William'
                                              }
                                          },
-                                         {"name"=>"app_name", "env"=>{"DEFAULT_HOST"=>"host"}}
+                                         {
+                                             'name' => 'app_name',
+                                             'env' => {
+                                                 'DEFAULT_HOST' => 'host',
+                                                 'Settings.world.capitals.europe.germany' => 'Berlin',
+                                                 'Settings.world.capitals.europe.poland' => 'Warsaw',
+                                                 'Settings.world.array.0.name' => 'Alan',
+                                                 'Settings.world.array.1.name' => 'Gam',
+                                                 'Settings.world.array_with_index.0.name' => 'Bob',
+                                                 'Settings.world.array_with_index.1.name' => 'William'
+                                             }
+                                         }
                                      ]
                                  })
   end
 
   it 'raises an exception if there is conflicting keys' do
-    merger = Config::CFManifestMerger.new('some-cf-app', load_manifest('cf_manifest.yml'))
-    Config.load_and_set_settings "#{fixture_path}/cf/cf_conflict.yml"
+    allow(Rails).to receive(:root).and_return(mocked_rails_root_path)
+
+    merger = Config::CFManifestMerger.new('conflict_settings', manifest_hash)
 
+    # Config.load_and_set_settings "#{fixture_path}/cf/conflict_settings.yml"
     expect {
       merger.add_to_env
     }.to raise_error(ArgumentError, 'Conflicting keys: DEFAULT_HOST, DEFAULT_PORT')
diff --git a/spec/tasks/cloud_foundry_spec.rb b/spec/tasks/cloud_foundry_spec.rb
@@ -4,7 +4,7 @@
   include_context 'rake'
 
   before :all do
-    load File.expand_path("../../../lib/config/tasks/cloud_foundry.rake", __FILE__)
+    load File.expand_path('../../../lib/config/tasks/cloud_foundry.rake', __FILE__)
     Rake::Task.define_task(:environment)
   end
 
@@ -14,52 +14,71 @@
     Settings.reload_from_files("#{fixture_path}/settings.yml")
   end
 
-  it 'creates the merge manifest file for cf' do
-    Config.load_and_set_settings "#{fixture_path}/cf/cf_multilevel.yml"
+  it 'raises an error if the manifest file is missing' do
+    expect {
+      Rake::Task['config:cf'].execute
+    }.to raise_error(SystemCallError)
+  end
+
+  it 'raises an error if the settings file is missing' do
+    expect {
+      Rake::Task['config:cf'].execute({target_env: 'not_existing_env', file_path: 'manifest.yml'})
+    }.to raise_error(SystemCallError)
+  end
 
-    orig_rails_root = Rails.root
+  describe 'without arguments' do
+    it 'creates the merged manifest file with the settings env included for all applications' do
+      allow(Rails).to receive(:root).and_return(Pathname.new Dir.mktmpdir)
 
-    begin
-      Rails.application.config.root = Dir.mktmpdir
+      test_settings_file = "#{fixture_path}/cf/settings/multilevel_settings.yml"
+      test_manifest_file = "#{fixture_path}/cf/cf_manifest.yml"
 
-      FileUtils.cp("#{fixture_path}/cf/cf_manifest.yml", File.join(Rails.root, 'manifest.yml'))
+      FileUtils.mkdir(Rails.root.join('settings'))
+      FileUtils.cp(test_settings_file, Rails.root.join('settings','test.yml'))
 
-      Rake::Task['config:cf'].execute({:app_name => 'app_name'})
+      FileUtils.cp(test_manifest_file, Rails.root.join('manifest.yml'))
 
-      target_file_path = File.join(Rails.root, 'manifest-test.yml')
-      target_file_contents = YAML.load(IO.read(target_file_path))
+      Rake::Task['config:cf'].execute
 
-      expect(target_file_contents["applications"][1]["name"]).to eq "app_name"
-      expect(target_file_contents["applications"][1]["env"]["DEFAULT_HOST"]).to eq "host"
-      expect(target_file_contents["applications"][1]["env"]["Settings.world.array.0.name"]).to eq "Alan"
-    ensure
-      Rails.application.config.root = orig_rails_root
+      merged_manifest_file = File.join(Rails.root, 'manifest-merged.yml')
+      merged_manifest_file_contents = YAML.load(IO.read(merged_manifest_file))
+
+      expect(merged_manifest_file_contents['applications'][0]['name']).to eq 'some-cf-app'
+      expect(merged_manifest_file_contents['applications'][0]['env']['DEFAULT_HOST']).to eq 'host'
+      expect(merged_manifest_file_contents['applications'][0]['env']['Settings.world.array.0.name']).to eq 'Alan'
+
+      expect(merged_manifest_file_contents['applications'][1]['name']).to eq 'app_name'
+      expect(merged_manifest_file_contents['applications'][1]['env']['Settings.world.array.0.name']).to eq 'Alan'
     end
   end
 
-  it 'handles a custom manifest name' do
+  describe 'with arguments' do
+    it 'raises an error if only one argument is provided' do
+      expect {
+        Rake::Task['config:cf'].execute({target_env:'target_env_name'})
+      }.to raise_error(ArgumentError)
+    end
 
-    orig_rails_root = Rails.root
+    it 'takes in account the provided arguments' do
+      allow(Rails).to receive(:root).and_return(Pathname.new Dir.mktmpdir)
 
-    begin
-      Rails.application.config.root = Dir.mktmpdir
+      test_settings_file = "#{fixture_path}/cf/settings/multilevel_settings.yml"
+      development_settings_file = "#{fixture_path}/development.yml"
+      test_manifest_file = "#{fixture_path}/cf/cf_manifest.yml"
 
-      FileUtils.cp("#{fixture_path}/cf/cf_manifest.yml", File.join(Rails.root, 'cf_manifest.yml'))
+      FileUtils.cp(test_manifest_file, Rails.root.join('cf_manifest.yml'))
 
-      Rake::Task['config:cf'].execute({app_name: 'app_name', file_path: 'cf_manifest.yml'})
+      settings_dir = Rails.root.join('settings')
+      settings_dir.mkpath
+      FileUtils.cp(development_settings_file, settings_dir.join('development.yml'))
+      FileUtils.cp(test_settings_file, settings_dir.join('test.yml'))
 
-      target_file_path = File.join(Rails.root, 'cf_manifest-test.yml')
+      Rake::Task['config:cf'].execute({target_env: 'development', file_path: 'cf_manifest.yml'})
 
-      expect(File.size? target_file_path).to be
+      merged_manifest_file = File.join(Rails.root, 'cf_manifest-merged.yml')
+      merged_manifest_file_contents = YAML.load(IO.read(merged_manifest_file))
 
-    ensure
-      Rails.application.config.root = orig_rails_root
+      expect(merged_manifest_file_contents['applications'][0]['env']['Settings.size']).to eq 2
     end
   end
-
-  it 'raises an error if the specified file is missing' do
-    expect {
-      Rake::Task['config:cf'].execute({app_name: 'app_name', file_path: 'null.yml'})
-    }.to raise_error(SystemCallError)
-  end
 end
