fix buildpack uniqness check and manifest message

Co-authored-by: Pavel Busko <[email protected]>

Author: 2 people

app/actions/buildpack_create.rb

@@ -29,7 +29,9 @@ def create(message)
 
     def validation_error!(error, create_message)
       error!(%(Stack '#{create_message.stack}' does not exist)) if error.errors.on(:stack)&.include?(:buildpack_stack_does_not_exist)
-      error!(%(Buildpack with name '#{error.model.name}' and stack '#{error.model.stack}' already exists)) if error.errors.on(%i[name stack])&.include?(:unique)
+      if error.errors.on(%i[name stack lifecycle])&.include?(:unique)
+        error!(%(Buildpack with name '#{error.model.name}', stack '#{error.model.stack}' and lifecycle '#{error.model.lifecycle}' already exists))
+      end
       error!(%(Buildpack with name '#{error.model.name}' and an unassigned stack already exists)) if error.errors.on(:stack)&.include?(:unique)
 
       error!(error.message)

app/controllers/runtime/buildpacks_controller.rb

@@ -16,9 +16,9 @@ def self.dependencies
     query_parameters :name, :stack, :lifecycle
 
     def self.translate_validation_exception(e, attributes)
-      buildpack_errors = e.errors.on(%i[name stack])
+      buildpack_errors = e.errors.on(%i[name stack lifecycle])
       if buildpack_errors && buildpack_errors.include?(:unique)
-        CloudController::Errors::ApiError.new_from_details('BuildpackNameStackTaken', attributes['name'], attributes['stack'])
+        CloudController::Errors::ApiError.new_from_details('BuildpackNameStackLifecycleTaken', attributes['name'], attributes['stack'], attributes['lifecycle'])
       else
         CloudController::Errors::ApiError.new_from_details('BuildpackInvalid', e.errors.full_messages)
       end

app/messages/app_manifest_message.rb

@@ -127,7 +127,7 @@ def app_lifecycle_hash
                          cnb_lifecycle_data
                        elsif requested?(:docker)
                          docker_lifecycle_data
-                       else
+                       elsif requested?(:buildpacks) || requested?(:buildpack) || requested?(:stack)
                          buildpacks_lifecycle_data
                        end
 

app/models/runtime/buildpack.rb

@@ -84,7 +84,7 @@ def state
     def validate_multiple_nil_stacks
       return unless stack.nil?
 
-      errors.add(:stack, :unique) if Buildpack.exclude(guid:).where(name: name, stack: nil).present?
+      errors.add(:stack, :unique) if Buildpack.exclude(guid:).where(name: name, stack: nil, lifecycle: lifecycle).present?
     end
 
     def validate_stack_change

errors/v2.yml

@@ -964,9 +964,9 @@
   message: "Encountered an error while attempting to sync cloud controller with the service broker's catalog: %s"
 
 290000:
-  name: BuildpackNameStackTaken
+  name: BuildpackNameStackLifecycleTaken
   http_code: 422
-  message: "The buildpack name %s is already in use for the stack %s"
+  message: "The buildpack name %s is already in use for the stack %s and the lifecycle %s"
 
 290001:
   name: BuildpackNameTaken

lib/cloud_controller/upload_buildpack.rb

@@ -56,8 +56,10 @@ def upload_buildpack(buildpack, bits_file_path, new_filename)
     private
 
     def raise_translated_api_error(buildpack)
-      raise CloudController::Errors::ApiError.new_from_details('BuildpackNameStackTaken', buildpack.name, buildpack.stack) if buildpack.errors.on(%i[name stack]).try(:include?,
-                                                                                                                                                                      :unique)
+      if buildpack.errors.on(%i[name stack lifecycle]).try(:include?, :unique)
+        raise CloudController::Errors::ApiError.new_from_details('BuildpackNameStackLifecycleTaken', buildpack.name, buildpack.stack, buildpack.lifecycle)
+      end
+
       raise CloudController::Errors::ApiError.new_from_details('BuildpackStacksDontMatch', buildpack.stack, buildpack.initial_value(:stack)) if buildpack.errors.on(:stack).try(
         :include?, :buildpack_cant_change_stacks
       )

spec/unit/actions/buildpack_create_spec.rb

@@ -178,7 +178,7 @@ module VCAP::CloudController
             message = BuildpackCreateMessage.new(name: name, stack: 'the-stack')
             expect do
               BuildpackCreate.new.create(message)
-            end.to raise_error(BuildpackCreate::Error, "Buildpack with name 'the-name' and stack 'the-stack' already exists")
+            end.to raise_error(BuildpackCreate::Error, "Buildpack with name 'the-name', stack 'the-stack' and lifecycle 'buildpack' already exists")
           end
         end
       end

spec/unit/lib/cloud_controller/upload_buildpack_spec.rb

@@ -131,7 +131,8 @@ module VCAP::CloudController
                   VCAP::CloudController::Buildpack.create(name: buildpack.name, stack: valid_zip_manifest_stack)
                   expect do
                     upload_buildpack.upload_buildpack(buildpack, valid_zip, filename)
-                  end.to raise_error(CloudController::Errors::ApiError, /The buildpack name #{buildpack.name} is already in use for the stack #{valid_zip_manifest_stack}/)
+                  end.to raise_error(CloudController::Errors::ApiError,
+                                     /The buildpack name #{buildpack.name} is already in use for the stack #{valid_zip_manifest_stack} and the lifecycle #{buildpack.lifecycle}/)
                 end
               end
             end

spec/unit/messages/app_manifest_message_spec.rb

@@ -1993,11 +1993,10 @@ module VCAP::CloudController
           context 'when no lifecycle data is requested in the manifest' do
             let(:parsed_yaml) { {} }
 
-            it 'defaults to the buildpack lifecycle' do
+            it 'does not forward missing attributes to the AppUpdateMessage' do
               message = AppManifestMessage.create_from_yml(parsed_yaml)
 
-              expect(message.app_update_message.requested?(:lifecycle)).to be true
-              expect(message.app_update_message.lifecycle_type).to eq('buildpack')
+              expect(message.app_update_message.requested?(:lifecycle)).to be false
             end
           end