Merge pull request #2390 from cloudfoundry/update-cloud-config-with-n…

…ame-param

Add name parameter to cloud_configs_controller

src/bosh-director/lib/bosh/director/api/cloud_config_manager.rb

@@ -2,17 +2,17 @@ module Bosh
   module Director
     module Api
       class CloudConfigManager
-        def update(cloud_config_yaml)
+        def update(cloud_config_yaml, name='default')
           cloud_config = Bosh::Director::Models::Config.new(
             type: 'cloud',
-            name: 'default',
             content: cloud_config_yaml,
+            name: name,
           )
           cloud_config.save
         end
 
-        def list(limit)
-          Bosh::Director::Models::Config.where(deleted: false, type: 'cloud', name: 'default').order(Sequel.desc(:id)).limit(limit).to_a
+        def list(limit, name='default')
+          Bosh::Director::Models::Config.where(deleted: false, type: 'cloud', name: name).order(Sequel.desc(:id)).limit(limit).to_a
         end
 
         def self.interpolated_manifest(cloud_configs, deployment_name)

src/bosh-director/lib/bosh/director/api/controllers/cloud_configs_controller.rb

@@ -4,30 +4,27 @@ module Bosh::Director
   module Api::Controllers
     class CloudConfigsController < BaseController
       post '/', :consumes => :yaml do
+        config_name = name_from_params(params)
         manifest_text = request.body.read
         begin
           validate_manifest_yml(manifest_text)
 
-          latest_cloud_config = Bosh::Director::Api::CloudConfigManager.new.list(1)
+          latest_cloud_config = Bosh::Director::Api::CloudConfigManager.new.list(1, config_name)
 
           if latest_cloud_config.empty? || latest_cloud_config.first[:content] != manifest_text
-            Bosh::Director::Api::CloudConfigManager.new.update(manifest_text)
-            create_event
+            Bosh::Director::Api::CloudConfigManager.new.update(manifest_text, config_name)
+            create_event(config_name)
           end
 
         rescue => e
-          create_event e
+          create_event(config_name, e)
           raise e
         end
         status(201)
       end
 
       post '/diff', :consumes => :yaml do
-        old_config_hash = cloud_config_or_empty(
-          Bosh::Director::Models::Config.
-            latest_set('cloud').
-            find{|config| config[:name] == 'default'}
-        )
+        old_config_hash = cloud_config_or_empty(cloud_config_by_name(name_from_params(params)))
         new_config_hash = validate_manifest_yml(request.body.read) || {}
 
         result = {}
@@ -57,7 +54,10 @@ class CloudConfigsController < BaseController
           return
         end
 
-        cloud_configs = Bosh::Director::Api::CloudConfigManager.new.list(limit)
+        config_name = name_from_params(params)
+
+        cloud_configs = Bosh::Director::Api::CloudConfigManager.new.list(limit, config_name)
+
         json_encode(
           cloud_configs.map do |cloud_config|
             {
@@ -70,17 +70,28 @@ class CloudConfigsController < BaseController
 
       private
 
+      def name_from_params(params)
+        name = params['name']
+        name.nil? || name.empty? ? 'default' : name
+      end
+
+      def cloud_config_by_name(config_name)
+        Bosh::Director::Models::Config.latest_set('cloud').find do |cloud_config|
+          cloud_config.name == config_name
+        end
+      end
+
       def cloud_config_or_empty(cloud_config_model)
         return {} if cloud_config_model.nil? || cloud_config_model.raw_manifest.nil?
         cloud_config_model.raw_manifest
       end
 
-      def create_event(error = nil)
+      def create_event(config_name, error = nil)
         @event_manager.create_event({
             user:        current_user,
             action:      'update',
             object_type: 'cloud-config',
-            object_name: 'default',
+            object_name: config_name,
             error:       error
         })
       end

src/bosh-director/spec/unit/api/cloud_config_manager_spec.rb

@@ -7,14 +7,26 @@
   let(:event_manager) {Bosh::Director::Api::EventManager.new(true)}
 
   describe '#update' do
-    it 'saves the cloud config' do
+    it 'saves default cloud config' do
       expect {
         manager.update(valid_cloud_manifest)
       }.to change(Bosh::Director::Models::Config, :count).from(0).to(1)
 
       cloud_config = Bosh::Director::Models::Config.first
       expect(cloud_config.created_at).to_not be_nil
       expect(cloud_config.content).to eq(valid_cloud_manifest)
+      expect(cloud_config.name).to eq('default')
+    end
+
+    it 'saves named cloud config' do
+      expect {
+        manager.update(valid_cloud_manifest, 'foo-cloud-config')
+      }.to change(Bosh::Director::Models::Config, :count).from(0).to(1)
+
+      cloud_config = Bosh::Director::Models::Config.first
+      expect(cloud_config.created_at).to_not be_nil
+      expect(cloud_config.content).to eq(valid_cloud_manifest)
+      expect(cloud_config.name).to eq('foo-cloud-config')
     end
 
     context 'when cloud config uses placeholders' do
@@ -72,6 +84,21 @@
         expect(cloud_configs[1]).to eq(@oldest_cloud_config)
       end
     end
+
+    context 'when name is specified' do
+      let(:name){ 'some-foo-name'}
+
+      it 'returns the specified number of cloud configs (most recent first)' do
+        named_config1 = Bosh::Director::Models::Config.new(type: 'cloud', content: 'named_config', name: 'some-foo-name').save
+        Bosh::Director::Models::Config.new(type: 'cloud', content: 'default_config', name: 'default').save
+        Bosh::Director::Models::Config.new(type: 'cloud', content: 'default_config', name: 'some-other-foo-name').save
+        named_config2 = Bosh::Director::Models::Config.new(type: 'cloud', content: 'named_config2', name: 'some-foo-name').save
+
+        runtime_configs = manager.list(2, name)
+
+        expect(runtime_configs).to eq([named_config2, named_config1])
+      end
+    end
   end
 
   describe '.interpolated_manifest' do

src/bosh-director/spec/unit/api/controllers/cloud_configs_controller_spec.rb

@@ -116,34 +116,125 @@ module Bosh::Director
         expect(event.user).to eq('admin')
         expect(event.error).to eq('Manifest should not be empty')
       end
+
+      context 'when name is the empty string' do
+        before { authorize 'admin', 'admin' }
+
+        let(:path) { '/?name=' }
+        let(:content) { YAML.dump(Bosh::Spec::Deployments.simple_cloud_config) }
+
+        it "creates a new cloud config with name 'default'" do
+          post path, content, 'CONTENT_TYPE' => 'text/yaml'
+
+          expect(last_response.status).to eq(201)
+          expect(Bosh::Director::Models::Config.first.name).to eq('default')
+        end
+      end
+
+      context 'when a name is passed in via a query param' do
+        before { authorize 'admin', 'admin' }
+
+        let(:path) { '/?name=smurf' }
+        let(:content) { YAML.dump(Bosh::Spec::Deployments.simple_cloud_config) }
+
+        it 'creates a new named cloud config' do
+          post path, content, 'CONTENT_TYPE' => 'text/yaml'
+
+          expect(last_response.status).to eq(201)
+          expect(Bosh::Director::Models::Config.first.name).to eq('smurf')
+        end
+
+        it 'ignores named cloud config when config already exists' do
+          Models::Config.make(:cloud, content: content, name: 'smurf')
+
+          expect do
+            post path, content, 'CONTENT_TYPE' => 'text/yaml'
+          end.to_not change(Models::Config, :count)
+
+          expect(last_response.status).to eq(201)
+        end
+
+        it 'creates a new event and add name to event context' do
+          expect do
+            post path, content, 'CONTENT_TYPE' => 'text/yaml'
+          end.to change(Bosh::Director::Models::Event, :count).from(0).to(1)
+
+          event = Bosh::Director::Models::Event.first
+          expect(event.object_type).to eq('cloud-config')
+          expect(event.object_name).to eq('smurf')
+          expect(event.action).to eq('update')
+          expect(event.user).to eq('admin')
+        end
+      end
     end
 
     describe 'GET', '/' do
-      it 'returns the number of cloud configs specified by ?limit' do
+      it "returns the number of cloud configs specified by ?limit and only considers 'default' names" do
         authorize('admin', 'admin')
 
         Models::Config.make(
           :cloud,
           content: 'config_from_time_immortal',
+          created_at: Time.now - 4,
+          name: 'not-default'
+        )
+        Models::Config.make(
+          :cloud,
+          content: 'config_from_second_last_year',
           created_at: Time.now - 3,
+          name: 'default'
         )
         Models::Config.make(
           :cloud,
           content: 'config_from_last_year',
           created_at: Time.now - 2,
+          name: 'not-default'
         )
         newer_cloud_config_content = "---\nsuper_shiny: new_config"
         Models::Config.make(
           :cloud,
           content: newer_cloud_config_content,
           created_at: Time.now - 1,
+          name: 'default'
         )
 
         get '/?limit=2'
 
         expect(last_response.status).to eq(200)
         expect(JSON.parse(last_response.body).count).to eq(2)
-        expect(JSON.parse(last_response.body).first['properties']).to eq(newer_cloud_config_content)
+        expect(JSON.parse(last_response.body)[0]['properties']).to eq(newer_cloud_config_content)
+        expect(JSON.parse(last_response.body)[1]['properties']).to eq('config_from_second_last_year')
+      end
+
+      it "returns the cloud configs specified by name" do
+        authorize('admin', 'admin')
+
+        Models::Config.make(
+          :cloud,
+          content: 'config_from_second_last_year',
+          created_at: Time.now - 3,
+          name: 'not-default'
+        )
+        Models::Config.make(
+          :cloud,
+          content: 'config_from_last_year',
+          created_at: Time.now - 2,
+          name: 'not-default'
+        )
+        newer_cloud_config_content = "---\nsuper_shiny: new_config"
+        Models::Config.make(
+          :cloud,
+          content: newer_cloud_config_content,
+          created_at: Time.now - 1,
+          name: 'default'
+        )
+
+        get '/?limit=2&name=not-default'
+
+        expect(last_response.status).to eq(200)
+        expect(JSON.parse(last_response.body).count).to eq(2)
+        expect(JSON.parse(last_response.body)[0]['properties']).to eq('config_from_last_year')
+        expect(JSON.parse(last_response.body)[1]['properties']).to eq('config_from_second_last_year')
       end
 
       it 'returns STATUS 400 if limit was not specified or malformed' do
@@ -277,7 +368,7 @@ module Bosh::Director
 
         end
 
-        context 'when there are two previous cloud config ' do
+        context 'when there are two previous cloud config' do
           before do
             Models::Config.make(:cloud, name: 'foo',content: YAML.dump(cloud_config_hash_with_one_az))
             Models::Config.make(:cloud, content: YAML.dump(cloud_config_hash_with_two_azs))
@@ -292,6 +383,21 @@ module Bosh::Director
           end
         end
 
+        context 'when a config name is provided' do
+          before do
+            Models::Config.make(:cloud, name: 'foo',content: YAML.dump(cloud_config_hash_with_one_az))
+            Models::Config.make(:cloud, content: YAML.dump(cloud_config_hash_with_two_azs))
+          end
+          it 'always diffs against the named cloud config' do
+            post(
+                '/diff?name=foo',
+                YAML.dump(cloud_config_hash_with_one_az),
+                {'CONTENT_TYPE' => 'text/yaml'}
+            )
+            expect(last_response.body).to eq('{"diff":[]}')
+          end
+        end
+
         context 'when there is no previous cloud config' do
           it 'returns the diff' do
             post(