diff --git a/Gemfile b/Gemfile index 6211ae580e1..ad25e3d0b7d 100644 --- a/Gemfile +++ b/Gemfile @@ -30,7 +30,8 @@ gem 'public_suffix' gem 'railties' gem 'rake' gem 'rfc822' -gem 'rubyzip' +gem 'rubyzip', git: 'https://github.com/idoru/rubyzip.git', branch: 'handle-gpbit-3' + gem 'sequel' gem 'sinatra', '~> 1.4' gem 'sinatra-contrib' diff --git a/Gemfile.lock b/Gemfile.lock index 5b5a596467a..b50e973e945 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -16,6 +16,13 @@ GIT specs: vcap-concurrency (0.1.0) +GIT + remote: https://github.com/idoru/rubyzip.git + revision: c787d94852b7d3e90212e8f7b08ad6ab6279c74d + branch: handle-gpbit-3 + specs: + rubyzip (1.2.1) + GIT remote: https://github.com/zipmark/rspec_api_documentation.git revision: de69a7a87adace29f39cbe937ae7bb8da9a4c9c1 @@ -360,7 +367,6 @@ GEM ruby-progressbar (1.9.0) ruby_parser (3.8.3) sexp_processor (~> 4.1) - rubyzip (1.2.1) safe_yaml (1.0.4) scientist (1.1.0) sequel (4.49.0) @@ -488,7 +494,7 @@ DEPENDENCIES rspec_api_documentation! rubocop ruby-debug-ide (>= 0.6.1.beta4) - rubyzip + rubyzip! scientist sequel sinatra (~> 1.4) diff --git a/app/controllers/runtime/buildpacks_controller.rb b/app/controllers/runtime/buildpacks_controller.rb index 88f56f4d457..deee30897eb 100644 --- a/app/controllers/runtime/buildpacks_controller.rb +++ b/app/controllers/runtime/buildpacks_controller.rb @@ -6,17 +6,18 @@ def self.dependencies define_attributes do attribute :name, String + attribute :stack, String, default: nil attribute :position, Integer, default: 0 attribute :enabled, Message::Boolean, default: true attribute :locked, Message::Boolean, default: false end - query_parameters :name + query_parameters :name, :stack def self.translate_validation_exception(e, attributes) - buildpack_errors = e.errors.on(:name) + buildpack_errors = e.errors.on([:name, :stack]) if buildpack_errors && buildpack_errors.include?(:unique) - CloudController::Errors::ApiError.new_from_details('BuildpackNameTaken', attributes['name']) + CloudController::Errors::ApiError.new_from_details('BuildpackNameStackTaken', attributes['name'], attributes['stack']) else CloudController::Errors::ApiError.new_from_details('BuildpackInvalid', e.errors.full_messages) end diff --git a/app/fetchers/buildpack_lifecycle_fetcher.rb b/app/fetchers/buildpack_lifecycle_fetcher.rb index a8ef6224a4c..ab0a83ee4b3 100644 --- a/app/fetchers/buildpack_lifecycle_fetcher.rb +++ b/app/fetchers/buildpack_lifecycle_fetcher.rb @@ -4,14 +4,19 @@ class << self def fetch(buildpack_names, stack_name) { stack: Stack.find(name: stack_name), - buildpack_infos: ordered_buildpacks(buildpack_names), + buildpack_infos: ordered_buildpacks(buildpack_names, stack_name), } end private - def ordered_buildpacks(buildpack_names) - buildpacks = Buildpack.where(name: buildpack_names).all + ## NOTE: if a requested system buildpack is not on the requested stack, + ## the BuildpackInfo object will have a name and not a record (even + ## though the buildpack exists). At this point the error returned + ## to the user will probably be VERY confusing + + def ordered_buildpacks(buildpack_names, stack_name) + buildpacks = Buildpack.where(name: buildpack_names, stack: stack_name).all buildpack_names.map do |buildpack_name| buildpack_record = buildpacks.find { |b| b.name == buildpack_name } diff --git a/app/jobs/runtime/buildpack_installer.rb b/app/jobs/runtime/buildpack_installer.rb index 9432ad95fbc..15628aebe47 100644 --- a/app/jobs/runtime/buildpack_installer.rb +++ b/app/jobs/runtime/buildpack_installer.rb @@ -14,7 +14,13 @@ def perform logger = Steno.logger('cc.background') logger.info "Installing buildpack #{name}" - buildpack = Buildpack.find(name: name) + buildpacks = find_existing_buildpacks + if buildpacks.count > 1 + logger.error "Update failed: Unable to determine buildpack to update as there are multiple buildpacks named #{name} for different stacks." + return + end + + buildpack = buildpacks.first if buildpack.nil? buildpacks_lock = Locking[name: 'buildpacks'] buildpacks_lock.db.transaction do @@ -55,6 +61,19 @@ def buildpack_uploader buildpack_blobstore = CloudController::DependencyLocator.instance.buildpack_blobstore UploadBuildpack.new(buildpack_blobstore) end + + private + + def find_existing_buildpacks + stack = VCAP::CloudController::Buildpacks::StackNameExtractor.extract_from_file(file) + if stack.present? + buildpacks_by_stack = Buildpack.where(name: name, stack: stack) + return buildpacks_by_stack if buildpacks_by_stack.any? + return Buildpack.where(name: name, stack: nil) + end + + Buildpack.where(name: name) + end end end end diff --git a/app/models/runtime/buildpack.rb b/app/models/runtime/buildpack.rb index 6ab63186a41..70cd3733e0f 100644 --- a/app/models/runtime/buildpack.rb +++ b/app/models/runtime/buildpack.rb @@ -2,11 +2,13 @@ module VCAP::CloudController class Buildpack < Sequel::Model plugin :list - export_attributes :name, :position, :enabled, :locked, :filename - import_attributes :name, :position, :enabled, :locked, :filename, :key + export_attributes :name, :stack, :position, :enabled, :locked, :filename + import_attributes :name, :stack, :position, :enabled, :locked, :filename, :key - def self.list_admin_buildpacks - exclude(key: nil).exclude(key: '').order(:position).all + def self.list_admin_buildpacks(stack_name=nil) + scoped = exclude(key: nil).exclude(key: '') + scoped = scoped.where(stack: stack_name) if stack_name + scoped.order(:position).all end def self.at_last_position @@ -18,8 +20,11 @@ def self.user_visibility_filter(user) end def validate - validates_unique :name + validates_unique [:name, :stack] validates_format(/\A(\w|\-)+\z/, :name, message: 'name can only contain alphanumeric characters') + + validate_stack_existence + validate_stack_change end def locked? @@ -43,5 +48,16 @@ def to_json def custom? false end + + private + def validate_stack_change + return if initial_value(:stack).nil? + errors.add(:stack, :buildpack_cant_change_stacks) if column_changes.key?(:stack) + end + + def validate_stack_existence + return unless stack + errors.add(:stack, :buildpack_stack_does_not_exist) if Stack.where(name: stack).empty? + end end end diff --git a/db/migrations/20180102183100_add_stack_to_buildpack_table.rb b/db/migrations/20180102183100_add_stack_to_buildpack_table.rb new file mode 100644 index 00000000000..4c87df0f5dd --- /dev/null +++ b/db/migrations/20180102183100_add_stack_to_buildpack_table.rb @@ -0,0 +1,42 @@ +require 'yaml' + +def stacks_yml + stacks_yml_path = ENV.fetch("STACKS_YML", nil) + YAML.load(File.read(stacks_yml_path)) if stacks_yml_path && File.exist?(stacks_yml_path) +end + +def default_stack + stacks_yml["default"] if stacks_yml +end + +def latest_windows_stack + return unless stacks_yml + stack_names = stacks_yml["stacks"].map { |stack| stack["name"] } + if stack_names.include?("windows2016") + return "windows2016" + end + if stack_names.include?("windows2012R2") + return "windows2012R2" + end + return nil +end + +Sequel.migration do + up do + alter_table(:buildpacks) do + add_column :stack, String, size: 255, null: true + drop_index :name, unique: true + add_index [:name, :stack], unique: true, name: :unique_name_and_stack + end + self[:buildpacks].where(Sequel.negate(name: "hwc_buildpack")).update(stack: default_stack) if default_stack + self[:buildpacks].where(name: "hwc_buildpack").update(stack: latest_windows_stack) if latest_windows_stack + end + + down do + alter_table(:buildpacks) do + drop_index [:name, :stack], unique: true, name: :unique_name_and_stack + drop_column :stack + add_index :name, unique: true, name: :buildpacks_name_index + end + end +end diff --git a/hello.zip b/hello.zip new file mode 100644 index 00000000000..a57e96bfd76 Binary files /dev/null and b/hello.zip differ diff --git a/lib/cloud_controller.rb b/lib/cloud_controller.rb index c0c88bf1a93..94f7f11181b 100644 --- a/lib/cloud_controller.rb +++ b/lib/cloud_controller.rb @@ -29,6 +29,7 @@ module VCAP::CloudController; end require 'cloud_controller/errors/invalid_app_relation' require 'cloud_controller/errors/invalid_route_relation' require 'cloud_controller/errors/no_running_instances' +require 'cloud_controller/errors/buildpack_error' require 'delayed_job_plugins/deserialization_retry' require 'delayed_job_plugins/after_enqueue_hook' require 'sequel_plugins/sequel_plugins' @@ -60,6 +61,7 @@ module VCAP::CloudController; end require 'cloud_controller/blobstore/client' require 'cloud_controller/blobstore/url_generator' require 'cloud_controller/blobstore/blob_key_generator' +require 'cloud_controller/buildpacks/stack_name_extractor' require 'cloud_controller/dependency_locator' require 'cloud_controller/file_path_checker' require 'cloud_controller/rule_validator' diff --git a/lib/cloud_controller/buildpacks/stack_name_extractor.rb b/lib/cloud_controller/buildpacks/stack_name_extractor.rb new file mode 100644 index 00000000000..c11f8568e6c --- /dev/null +++ b/lib/cloud_controller/buildpacks/stack_name_extractor.rb @@ -0,0 +1,24 @@ +module VCAP::CloudController + module Buildpacks + class StackNameExtractor + ONE_MEGABYTE = 1024 * 1024 + + def self.extract_from_file(bits_file_path) + bits_file_path = bits_file_path.path if bits_file_path.respond_to?(:path) + Zip::File.open(bits_file_path) do |zip_file| + zip_file.each do |entry| + if entry.name == 'manifest.yml' + raise CloudController::Errors::BuildpackError.new('buildpack manifest is too large') if entry.size > ONE_MEGABYTE + return YAML.safe_load(entry.get_input_stream.read).dig('stack') + end + end + end + nil + rescue Psych::Exception + raise CloudController::Errors::BuildpackError.new('buildpack manifest is not valid') + rescue Zip::Error + raise CloudController::Errors::BuildpackError.new('buildpack zipfile is not valid') + end + end + end +end diff --git a/lib/cloud_controller/diego/buildpack/buildpack_entry_generator.rb b/lib/cloud_controller/diego/buildpack/buildpack_entry_generator.rb index 792ddcbb80c..f8aa9517b28 100644 --- a/lib/cloud_controller/diego/buildpack/buildpack_entry_generator.rb +++ b/lib/cloud_controller/diego/buildpack/buildpack_entry_generator.rb @@ -6,8 +6,8 @@ def initialize(blobstore_url_generator) @blobstore_url_generator = blobstore_url_generator end - def buildpack_entries(buildpack_infos) - return default_admin_buildpacks if buildpack_infos.empty? + def buildpack_entries(buildpack_infos, stack_name) + return default_admin_buildpacks(stack_name) if buildpack_infos.empty? buildpack_infos.map do |buildpack_info| if buildpack_info.buildpack_exists_in_db? && buildpack_info.buildpack_enabled? @@ -26,8 +26,8 @@ def custom_buildpack_entry(buildpack) { name: 'custom', key: buildpack.url, url: buildpack.url } end - def default_admin_buildpacks - VCAP::CloudController::Buildpack.list_admin_buildpacks. + def default_admin_buildpacks(stack_name) + VCAP::CloudController::Buildpack.list_admin_buildpacks(stack_name). select(&:enabled). collect { |buildpack| admin_buildpack_entry(buildpack) } end diff --git a/lib/cloud_controller/diego/buildpack/lifecycle_protocol.rb b/lib/cloud_controller/diego/buildpack/lifecycle_protocol.rb index 5e921c9ffa5..8b76c29cb5a 100644 --- a/lib/cloud_controller/diego/buildpack/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/buildpack/lifecycle_protocol.rb @@ -18,20 +18,15 @@ def initialize(blobstore_url_generator=::CloudController::DependencyLocator.inst end def lifecycle_data(staging_details) + stack = staging_details.lifecycle.staging_stack lifecycle_data = Diego::Buildpack::LifecycleData.new lifecycle_data.app_bits_download_uri = @blobstore_url_generator.package_download_url(staging_details.package) lifecycle_data.app_bits_checksum = staging_details.package.checksum_info lifecycle_data.buildpack_cache_checksum = staging_details.package.app.buildpack_cache_sha256_checksum - lifecycle_data.build_artifacts_cache_download_uri = @blobstore_url_generator.buildpack_cache_download_url( - staging_details.package.app_guid, - staging_details.lifecycle.staging_stack - ) - lifecycle_data.build_artifacts_cache_upload_uri = @blobstore_url_generator.buildpack_cache_upload_url( - staging_details.package.app_guid, - staging_details.lifecycle.staging_stack - ) + lifecycle_data.build_artifacts_cache_download_uri = @blobstore_url_generator.buildpack_cache_download_url(staging_details.package.app_guid, stack) + lifecycle_data.build_artifacts_cache_upload_uri = @blobstore_url_generator.buildpack_cache_upload_url(staging_details.package.app_guid, stack) lifecycle_data.droplet_upload_uri = @blobstore_url_generator.droplet_upload_url(staging_details.staging_guid) - lifecycle_data.buildpacks = @buildpack_entry_generator.buildpack_entries(staging_details.lifecycle.buildpack_infos) + lifecycle_data.buildpacks = @buildpack_entry_generator.buildpack_entries(staging_details.lifecycle.buildpack_infos, stack) lifecycle_data.stack = staging_details.lifecycle.staging_stack lifecycle_data.message diff --git a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb index 33e543a38eb..78b56395ee7 100644 --- a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb @@ -36,7 +36,7 @@ def staging_environment_variables end def staging_stack - requested_stack || app_stack || VCAP::CloudController::Stack.default.name + requested_stack || app_stack || buildpack_stack || VCAP::CloudController::Stack.default.name end private @@ -57,6 +57,13 @@ def app_stack @package.app.buildpack_lifecycle_data.try(:stack) end + def buildpack_stack + stacks = Buildpack.where(name: buildpacks_to_use).select(:stack).map(&:stack).uniq + if stacks.length == 1 + stacks.first + end + end + attr_reader :validator end end diff --git a/lib/cloud_controller/errors/buildpack_error.rb b/lib/cloud_controller/errors/buildpack_error.rb new file mode 100644 index 00000000000..16c16df430a --- /dev/null +++ b/lib/cloud_controller/errors/buildpack_error.rb @@ -0,0 +1,5 @@ +module CloudController + module Errors + class BuildpackError < StandardError; end + end +end diff --git a/lib/cloud_controller/upload_buildpack.rb b/lib/cloud_controller/upload_buildpack.rb index 3b5865cefb2..8f3893337e8 100644 --- a/lib/cloud_controller/upload_buildpack.rb +++ b/lib/cloud_controller/upload_buildpack.rb @@ -1,8 +1,10 @@ require 'vcap/digester' module VCAP::CloudController + class UploadBuildpack attr_reader :buildpack_blobstore + ONE_MEGABYTE = 1024*1024 def initialize(blobstore) @buildpack_blobstore = blobstore @@ -24,6 +26,8 @@ def upload_buildpack(buildpack, bits_file_path, new_filename) old_buildpack_key = nil + new_stack = determine_new_stack(buildpack, bits_file_path) + begin Buildpack.db.transaction do buildpack.lock! @@ -32,9 +36,20 @@ def upload_buildpack(buildpack, bits_file_path, new_filename) key: new_key, filename: new_filename, sha256_checksum: sha256, + stack: new_stack ) end - rescue Sequel::Error + rescue Sequel::ValidationFailed + if buildpack.errors.on([:name, :stack]).try(:include?, :unique) + raise CloudController::Errors::ApiError.new_from_details('BuildpackNameStackTaken', buildpack.name, new_stack) + end + if buildpack.errors.on(:stack).try(:include?, :buildpack_cant_change_stacks) + raise CloudController::Errors::ApiError.new_from_details('BuildpackStacksDontMatch', new_stack, buildpack.initial_value(:stack)) + end + if buildpack.errors.on(:stack).try(:include?, :buildpack_stack_does_not_exist) + raise CloudController::Errors::ApiError.new_from_details('BuildpackStackDoesNotExist', new_stack) + end + rescue Sequel::Error => e BuildpackBitsDelete.delete_when_safe(new_key, 0) return false end @@ -49,6 +64,15 @@ def upload_buildpack(buildpack, bits_file_path, new_filename) private + def determine_new_stack(buildpack, bits_file_path) + extracted_stack = Buildpacks::StackNameExtractor.extract_from_file(bits_file_path) + new_stack = [extracted_stack, buildpack.stack, Stack.default.name].find { |s| s.present? } + + new_stack + rescue CloudController::Errors::BuildpackError => e + raise CloudController::Errors::ApiError.new_from_details('BuildpackZipError', e.message) + end + def new_bits?(buildpack, key) buildpack.key != key end diff --git a/spec/api/api_version_spec.rb b/spec/api/api_version_spec.rb index c095a6a01be..3bd4bba400a 100644 --- a/spec/api/api_version_spec.rb +++ b/spec/api/api_version_spec.rb @@ -2,7 +2,7 @@ require 'vcap/digester' RSpec.describe 'Stable API warning system', api_version_check: true do - API_FOLDER_CHECKSUM = '3655fc8ee4b47c9bbf4773e5743b65b52fa04305'.freeze + API_FOLDER_CHECKSUM = '88a6ea92882a662438d397872ea2a38662caa549'.freeze it 'tells the developer if the API specs change' do api_folder = File.expand_path('..', __FILE__) diff --git a/spec/api/documentation/buildpacks_api_spec.rb b/spec/api/documentation/buildpacks_api_spec.rb index 5e88b80e225..3ee54d27e9b 100644 --- a/spec/api/documentation/buildpacks_api_spec.rb +++ b/spec/api/documentation/buildpacks_api_spec.rb @@ -19,6 +19,7 @@ required: opts[:required], example_values: ['Golang_buildpack'] + field :stack, 'The name of the stack to associate this buildpack with', example_values: ['cflinuxfs2'] field :position, 'The order in which the buildpacks are checked during buildpack auto-detection.' field :enabled, 'Whether or not the buildpack will be used for staging', default: true field :locked, 'Whether or not the buildpack is locked to prevent updates', default: false @@ -33,7 +34,7 @@ include_context 'updatable_fields', required: true example 'Creates an admin Buildpack' do - client.post '/v2/buildpacks', fields_json, headers + client.post '/v2/buildpacks', fields_json(stack: 'cflinuxfs2'), headers expect(status).to eq 201 standard_entity_response parsed_response, :buildpack, expected_values: { name: 'Golang_buildpack' } end diff --git a/spec/fixtures/config/stacks_windows2012R2.yml b/spec/fixtures/config/stacks_windows2012R2.yml new file mode 100644 index 00000000000..8ae503bdc66 --- /dev/null +++ b/spec/fixtures/config/stacks_windows2012R2.yml @@ -0,0 +1,11 @@ +default: "default-stack-name" + +stacks: + - name: "cflinuxfs2" + description: "cflinuxfs2" + - name: "default-stack-name" + description: "default-stack-description" + - name: "cider" + description: "cider-description" + - name: "windows2012R2" + description: "windows2012R2-description" diff --git a/spec/fixtures/config/stacks_windows2016.yml b/spec/fixtures/config/stacks_windows2016.yml new file mode 100644 index 00000000000..4d40fcd55e1 --- /dev/null +++ b/spec/fixtures/config/stacks_windows2016.yml @@ -0,0 +1,11 @@ +default: "default-stack-name" + +stacks: + - name: "cflinuxfs2" + description: "cflinuxfs2" + - name: "default-stack-name" + description: "default-stack-description" + - name: "cider" + description: "cider-description" + - name: "windows2016" + description: "windows2016-description" diff --git a/spec/fixtures/config/stacks_windows_all.yml b/spec/fixtures/config/stacks_windows_all.yml new file mode 100644 index 00000000000..4c9cd5bdd37 --- /dev/null +++ b/spec/fixtures/config/stacks_windows_all.yml @@ -0,0 +1,13 @@ +default: "default-stack-name" + +stacks: + - name: "cflinuxfs2" + description: "cflinuxfs2" + - name: "default-stack-name" + description: "default-stack-description" + - name: "cider" + description: "cider-description" + - name: "windows2012R2" + description: "windows2012R2-description" + - name: "windows2016" + description: "windows2016-description" diff --git a/spec/migrations/20180102183100_add_stack_to_buildpack_table_spec.rb b/spec/migrations/20180102183100_add_stack_to_buildpack_table_spec.rb new file mode 100644 index 00000000000..c6dba3a5873 --- /dev/null +++ b/spec/migrations/20180102183100_add_stack_to_buildpack_table_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +RSpec.describe 'Adding multiple stack support to buildpack model', isolation: :truncation do + let(:start_event_created_at) { Time.new(2017, 1, 1) } + let(:migrations_dir) { File.expand_path(File.join(File.dirname(__FILE__), '../../db/migrations')) } + let(:pre_down_migration_stack) { VCAP::CloudController::Stack.make } + + before do + VCAP::CloudController::Buildpack.make(name: 'a-great-buildpack-really', stack: pre_down_migration_stack.name) + VCAP::CloudController::Buildpack.make(name: 'hwc_buildpack', stack: pre_down_migration_stack.name) + Sequel::Migrator.run(VCAP::CloudController::Buildpack.db, migrations_dir, target: 20171220183100) + end + + after do + Sequel::Migrator.run(VCAP::CloudController::Buildpack.db, migrations_dir) + end + + context 'STACKS_YML set to filepath specifying default stack' do + let!(:original_env) { ENV["STACKS_YML"] } + + before do + ENV["STACKS_YML"] = stacks_yml_path + Sequel::Migrator.run(VCAP::CloudController::Buildpack.db, migrations_dir, target: 20180102183100) + end + + after { ENV["STACKS_YML"] = original_env } + + context 'STACKS_YML file does not define any windows stack' do + let(:stacks_yml_path) { File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/config/stacks.yml')) } + + it 'assigns the default stack to existing buildpacks' do + expect(VCAP::CloudController::Buildpack.where(name: 'a-great-buildpack-really', stack: 'default-stack-name').count).to eq(1) + end + + it 'assigns nil to the hwc_buildpack' do + expect(VCAP::CloudController::Buildpack.where(name: 'hwc_buildpack', stack: nil).count).to eq(1) + end + end + + context 'STACKS_YML file defines windows2012R2 stack' do + let(:stacks_yml_path) { File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/config/stacks_windows2012R2.yml')) } + + it 'assigns the default stack to existing buildpacks' do + expect(VCAP::CloudController::Buildpack.where(name: 'a-great-buildpack-really', stack: 'default-stack-name').count).to eq(1) + end + + it 'assigns windows2012R2 to the hwc_buildpack' do + expect(VCAP::CloudController::Buildpack.where(name: 'hwc_buildpack', stack: 'windows2012R2').count).to eq(1) + end + end + + context 'STACKS_YML file defines windows2016 stack' do + let(:stacks_yml_path) { File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/config/stacks_windows2016.yml')) } + + it 'assigns the default stack to existing buildpacks' do + expect(VCAP::CloudController::Buildpack.where(name: 'a-great-buildpack-really', stack: 'default-stack-name').count).to eq(1) + end + + it 'assigns windows2016 to the hwc_buildpack' do + expect(VCAP::CloudController::Buildpack.where(name: 'hwc_buildpack', stack: 'windows2016').count).to eq(1) + end + end + + context 'STACKS_YML file defines windows 2012R2 and windows2016 stack' do + let(:stacks_yml_path) { File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/config/stacks_windows_all.yml')) } + + it 'assigns the default stack to existing buildpacks' do + expect(VCAP::CloudController::Buildpack.where(name: 'a-great-buildpack-really', stack: 'default-stack-name').count).to eq(1) + end + + it 'assigns windows2016 to the hwc_buildpack' do + expect(VCAP::CloudController::Buildpack.where(name: 'hwc_buildpack', stack: 'windows2016').count).to eq(1) + end + end + end + + context 'STACKS_YML not set' do + it 'assigns nil stack to existing buildpacks' do + Sequel::Migrator.run(VCAP::CloudController::Buildpack.db, migrations_dir, target: 20180102183100) + expect(VCAP::CloudController::Buildpack.where(name: 'a-great-buildpack-really', stack: nil).count).to eq(1) + expect(VCAP::CloudController::Buildpack.where(name: 'hwc_buildpack', stack: nil).count).to eq(1) + end + end +end diff --git a/spec/request/app_manifests_spec.rb b/spec/request/app_manifests_spec.rb index af90a62ef93..b2770a4db2c 100644 --- a/spec/request/app_manifests_spec.rb +++ b/spec/request/app_manifests_spec.rb @@ -22,7 +22,7 @@ 'memory' => '2048MB', 'disk_quota' => '1.5GB', 'buildpack' => buildpack.name, - 'stack' => 'cflinuxfs2', + 'stack' => buildpack.stack, 'command' => 'new-command', 'env' => { 'k1' => 'mangos', @@ -54,7 +54,7 @@ app_model.reload lifecycle_data = app_model.lifecycle_data expect(lifecycle_data.buildpacks).to include(buildpack.name) - expect(lifecycle_data.stack).to eq('cflinuxfs2') + expect(lifecycle_data.stack).to eq(buildpack.stack) expect(app_model.environment_variables).to match({ 'k1' => 'mangos', 'k2' => 'pears', diff --git a/spec/request/apps_spec.rb b/spec/request/apps_spec.rb index 8dbda47750d..9ec7d3750eb 100644 --- a/spec/request/apps_spec.rb +++ b/spec/request/apps_spec.rb @@ -4,6 +4,7 @@ let(:user) { VCAP::CloudController::User.make } let(:user_header) { headers_for(user, email: user_email, user_name: user_name) } let(:space) { VCAP::CloudController::Space.make } + let(:stack) { VCAP::CloudController::Stack.make } let(:user_email) { Sham.email } let(:user_name) { 'some-username' } @@ -14,14 +15,14 @@ describe 'POST /v3/apps' do it 'creates an app' do - buildpack = VCAP::CloudController::Buildpack.make + buildpack = VCAP::CloudController::Buildpack.make(stack: stack.name) create_request = { name: 'my_app', environment_variables: { open: 'source' }, lifecycle: { type: 'buildpack', data: { - stack: nil, + stack: buildpack.stack, buildpacks: [buildpack.name] } }, @@ -50,7 +51,7 @@ 'type' => 'buildpack', 'data' => { 'buildpacks' => [buildpack.name], - 'stack' => VCAP::CloudController::Stack.default.name, + 'stack' => stack.name, } }, 'relationships' => { diff --git a/spec/support/fakes/blueprints.rb b/spec/support/fakes/blueprints.rb index 106816a3449..ed8fdb8c0f4 100644 --- a/spec/support/fakes/blueprints.rb +++ b/spec/support/fakes/blueprints.rb @@ -26,6 +26,7 @@ status { |_| %w(active suspended cancelled).sample(1).first } error_message { |index| "error-message-#{index}" } sequence_id { |index| index } + stack { |index| "cflinuxfs-#{index}" } end module VCAP::CloudController @@ -397,6 +398,7 @@ module VCAP::CloudController Buildpack.blueprint do name { Sham.name } + stack { Stack.make.name } key { Sham.guid } position { 0 } enabled { true } diff --git a/spec/support/test_zip.rb b/spec/support/test_zip.rb index 199249c44fe..cbca72070eb 100644 --- a/spec/support/test_zip.rb +++ b/spec/support/test_zip.rb @@ -1,14 +1,15 @@ +require 'zip' + module TestZip - def self.create(zip_name, file_count, file_size=1024) - files = [] - file_count.times do |i| - tf = Tempfile.new("ziptest_#{i}") - files << tf - tf.write('A' * file_size) - tf.close - end + def self.create(zip_name, file_count, file_size=1024, &blk) + Zip::File.open(zip_name, Zip::File::CREATE) do |zipfile| + file_count.times do |i| + zipfile.get_output_stream("ziptest_#{i}") do |f| + f.write('A' * file_size) + end + end - child = POSIX::Spawn::Child.new('zip', zip_name, *files.map(&:path)) - child.status.exitstatus == 0 || raise("Failed zipping:\n#{child.err}\n#{child.out}") + blk.call(zipfile) if blk + end end end diff --git a/spec/unit/actions/app_update_spec.rb b/spec/unit/actions/app_update_spec.rb index 44406c9374f..f9e0eb803e0 100644 --- a/spec/unit/actions/app_update_spec.rb +++ b/spec/unit/actions/app_update_spec.rb @@ -11,7 +11,7 @@ module VCAP::CloudController let(:user_audit_info) { UserAuditInfo.new(user_email: user_email, user_guid: user_guid) } let(:buildpack) { 'http://original.com' } let(:app_name) { 'original name' } - let!(:ruby_buildpack) { Buildpack.make(name: 'ruby') } + let!(:ruby_buildpack) { Buildpack.make(name: 'ruby', stack: stack.name) } let(:stack) { Stack.make(name: 'SUSE') } before do @@ -89,7 +89,7 @@ module VCAP::CloudController it 'raises an AppUpdate::InvalidApp error' do expect { app_update.update(app_model, message, lifecycle) - }.to raise_error(AppUpdate::InvalidApp, 'Stack must be an existing stack') + }.to raise_error(AppUpdate::InvalidApp, 'Buildpack "ruby" must be an existing admin buildpack or a valid git URI, Stack must be an existing stack') end end diff --git a/spec/unit/controllers/runtime/buildpack_bits_controller_spec.rb b/spec/unit/controllers/runtime/buildpack_bits_controller_spec.rb index 159b1b836c8..3f2a6a13e8e 100644 --- a/spec/unit/controllers/runtime/buildpack_bits_controller_spec.rb +++ b/spec/unit/controllers/runtime/buildpack_bits_controller_spec.rb @@ -8,6 +8,26 @@ module VCAP::CloudController let(:sha_valid_zip) { Digester.new(algorithm: Digest::SHA256).digest_file(valid_zip) } let(:sha_valid_zip2) { Digester.new(algorithm: Digest::SHA256).digest_file(valid_zip2) } let(:sha_valid_tar_gz) { Digester.new(algorithm: Digest::SHA256).digest_file(valid_tar_gz) } + let(:valid_zip_manifest) do + zip_name = File.join(tmpdir, filename) + TestZip.create(zip_name, 1, 1024) do |zipfile| + zipfile.get_output_stream('manifest.yml') do |f| + f.write("---\nstack: stack-from-manifest\n") + end + end + zip_file = File.new(zip_name) + Rack::Test::UploadedFile.new(zip_file) + end + let(:valid_zip_unknown_stack) do + zip_name = File.join(tmpdir, filename) + TestZip.create(zip_name, 1, 1024) do |zipfile| + zipfile.get_output_stream('manifest.yml') do |f| + f.write("---\nstack: unknown-stack\n") + end + end + zip_file = File.new(zip_name) + Rack::Test::UploadedFile.new(zip_file) + end let(:valid_zip) do zip_name = File.join(tmpdir, filename) TestZip.create(zip_name, 1, 1024) @@ -29,10 +49,7 @@ module VCAP::CloudController let(:expected_sha_valid_zip) { "#{@buildpack.guid}_#{sha_valid_zip}" } before do - @file = double(:file, { - public_url: 'https://some-bucket.example.com/ab/cd/abcdefg', - key: '123-456', - }) + @file = double(:file, { public_url: 'https://some-bucket.example.com/ab/cd/abcdefg', key: '123-456' }) buildpack_blobstore = CloudController::DependencyLocator.instance.buildpack_blobstore allow(buildpack_blobstore).to receive(:files).and_return(double(:files, head: @file, create: {})) @@ -42,7 +59,7 @@ module VCAP::CloudController after { FileUtils.rm_rf(tmpdir) } context 'Buildpack binaries' do - let(:test_buildpack) { VCAP::CloudController::Buildpack.create_from_hash({ name: 'upload_binary_buildpack', position: 0 }) } + let(:test_buildpack) { VCAP::CloudController::Buildpack.create_from_hash({ name: 'upload_binary_buildpack', stack: nil, position: 0 }) } before { CloudController::DependencyLocator.instance.register(:upload_handler, UploadHandler.new(TestConfig.config_instance)) } @@ -51,6 +68,9 @@ module VCAP::CloudController TestConfig.override(directories: { tmpdir: File.dirname(valid_zip.path) }) @cache = Delayed::Worker.delay_jobs Delayed::Worker.delay_jobs = false + + Stack.create(name: 'stack') + Stack.create(name: 'stack-from-manifest') end after { Delayed::Worker.delay_jobs = @cache } @@ -70,25 +90,67 @@ module VCAP::CloudController end it 'takes a buildpack file and adds it to the custom buildpacks blobstore with the correct key' do + test_buildpack.update(stack: 'stack') + allow(CloudController::DependencyLocator.instance.upload_handler).to receive(:uploaded_file).and_return(valid_zip.path) buildpack_blobstore = CloudController::DependencyLocator.instance.buildpack_blobstore expected_key = "#{test_buildpack.guid}_#{sha_valid_zip}" put "/v2/buildpacks/#{test_buildpack.guid}/bits", upload_body + expect(last_response.status).to eq(201) + buildpack = Buildpack.find(name: 'upload_binary_buildpack') expect(buildpack.key).to eq(expected_key) expect(buildpack.filename).to eq(filename) + expect(buildpack.stack).to eq('stack') expect(buildpack_blobstore.exists?(expected_key)).to be true end it 'gets the uploaded file from the upload handler' do upload_handler = CloudController::DependencyLocator.instance.upload_handler - expect(upload_handler).to receive(:uploaded_file). - with(hash_including('buildpack_name' => filename), 'buildpack'). - and_return(valid_zip) + expect(upload_handler).to receive(:uploaded_file).with(hash_including('buildpack_name' => filename), 'buildpack').and_return(valid_zip) put "/v2/buildpacks/#{test_buildpack.guid}/bits", upload_body end + it 'sets the buildpack stack if it is unset and in buildpack manifest' do + put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip_manifest, buildpack_name: valid_zip_manifest.path } + expect(last_response.status).to eql 201 + + buildpack = Buildpack.find(name: 'upload_binary_buildpack') + expect(buildpack.stack).to eq('stack-from-manifest') + end + + it 'returns ERROR (422) if provided stack does not exist' do + put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip_unknown_stack, buildpack_name: valid_zip_unknown_stack.path } + expect(last_response.status).to eql 422 + + buildpack = Buildpack.find(name: 'upload_binary_buildpack') + expect(buildpack.stack).to be_nil + end + + it 'sets the buildpack stack to default if it is unset and NOT in buildpack manifest' do + put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip, buildpack_name: valid_zip.path } + expect(last_response.status).to eql 201 + + buildpack = Buildpack.find(name: 'upload_binary_buildpack') + expect(buildpack.stack).to eq('default-stack-name') + end + + it 'requires an existing stack to be the same as one in the manifest if it exists' do + Stack.make(name: 'not-from-manifest') + test_buildpack.update(stack: 'not-from-manifest') + + put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip_manifest, buildpack_name: valid_zip_manifest.path } + expect(last_response.status).to eql 422 + + json = MultiJson.load(last_response.body) + expect(json['code']).to eq(390011) + expect(json['description']).to eql 'Uploaded buildpack stack (stack-from-manifest) does not match not-from-manifest' + + buildpack = Buildpack.find(name: 'upload_binary_buildpack') + expect(buildpack.stack).to eq('not-from-manifest') + end + it 'requires a filename as part of the upload' do put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: 'abc' } expect(last_response.status).to eql 400 @@ -118,6 +180,8 @@ module VCAP::CloudController end it 'removes the old buildpack binary when a new one is uploaded' do + test_buildpack.update(stack: 'stack') + put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip2 } expected_sha = "#{test_buildpack.guid}_#{sha_valid_zip2}" @@ -126,19 +190,29 @@ module VCAP::CloudController put "/v2/buildpacks/#{test_buildpack.guid}/bits", upload_body response = MultiJson.load(last_response.body) - entity = response['entity'] - expect(entity['name']).to eq('upload_binary_buildpack') - expect(entity['filename']).to eq(filename) + expect(response['entity']['name']).to eq('upload_binary_buildpack') + expect(response['entity']['filename']).to eq("#{filename}") expect(buildpack_blobstore.exists?(expected_sha)).to be false end it 'reports a no content if the same buildpack is uploaded again' do - put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip } - put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip } + 2.times { put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip } } expect(last_response.status).to eq(204) end + it 'does not allow uploading a buildpack which will update the stack that already has a buildpack with the same name' do + first_buildpack = VCAP::CloudController::Buildpack.create_from_hash({ name: 'nice_buildpack', stack: nil, position: 0 }) + put "/v2/buildpacks/#{first_buildpack.guid}/bits", { buildpack: valid_zip_manifest } + + expect(Buildpack.find(name: 'nice_buildpack').stack).to eq('stack-from-manifest') + + new_buildpack = VCAP::CloudController::Buildpack.create_from_hash({ name: first_buildpack.name, stack: nil, position: 0 }) + put "/v2/buildpacks/#{new_buildpack.guid}/bits", { buildpack: valid_zip_manifest } + + expect(last_response.status).to eq(422) + end + it 'allowed when same bits but different filename are uploaded again' do put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip } new_name = File.join(File.dirname(valid_zip.path), 'newfilename.zip') @@ -155,13 +229,13 @@ module VCAP::CloudController end it 'does not allow upload if the buildpack is locked' do - locked_buildpack = VCAP::CloudController::Buildpack.create_from_hash({ name: 'locked_buildpack', locked: true, position: 0 }) + locked_buildpack = VCAP::CloudController::Buildpack.create_from_hash({ name: 'locked_buildpack', stack: 'stack', locked: true, position: 0 }) put "/v2/buildpacks/#{locked_buildpack.guid}/bits", { buildpack: valid_zip2 } expect(last_response.status).to eq(409) end it 'does allow upload if the buildpack has been unlocked' do - locked_buildpack = VCAP::CloudController::Buildpack.create_from_hash({ name: 'locked_buildpack', locked: true, position: 0 }) + locked_buildpack = VCAP::CloudController::Buildpack.create_from_hash({ name: 'locked_buildpack', stack: 'stack', locked: true, position: 0 }) put "/v2/buildpacks/#{locked_buildpack.guid}", '{"locked": false}' put "/v2/buildpacks/#{locked_buildpack.guid}/bits", { buildpack: valid_zip2 } @@ -177,7 +251,7 @@ module VCAP::CloudController end context 'when the same bits are uploaded twice' do - let(:test_buildpack2) { VCAP::CloudController::Buildpack.create_from_hash({ name: 'buildpack2', position: 0 }) } + let(:test_buildpack2) { VCAP::CloudController::Buildpack.create_from_hash({ name: 'buildpack2', stack: 'stack', position: 0 }) } before do put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip2 } put "/v2/buildpacks/#{test_buildpack2.guid}/bits", { buildpack: valid_zip2 } @@ -196,20 +270,14 @@ module VCAP::CloudController let(:staging_password) { 'password' } let(:staging_config) do { - staging: { - timeout_in_seconds: 240, - auth: { - user: staging_user, - password: staging_password, - }, - }, + staging: { timeout_in_seconds: 240, auth: { user: staging_user, password: staging_password } }, directories: { tmpdir: File.dirname(valid_zip.path) }, } end before do TestConfig.override(staging_config) - VCAP::CloudController::Buildpack.create_from_hash({ name: 'get_binary_buildpack', key: 'xyz', position: 0 }) + VCAP::CloudController::Buildpack.create_from_hash({ name: 'get_binary_buildpack', stack: nil, key: 'xyz', position: 0 }) end it 'returns NOT AUTHENTICATED (401) users without correct basic auth' do diff --git a/spec/unit/controllers/runtime/buildpacks_controller_spec.rb b/spec/unit/controllers/runtime/buildpacks_controller_spec.rb index 705d1cf34f3..d11d589c421 100644 --- a/spec/unit/controllers/runtime/buildpacks_controller_spec.rb +++ b/spec/unit/controllers/runtime/buildpacks_controller_spec.rb @@ -7,7 +7,8 @@ def ordered_buildpacks end let(:user) { make_user } - let(:req_body) { MultiJson.dump({ name: 'dynamic_test_buildpack', position: 1 }) } + let(:stack) { Stack.make } + let(:req_body) { MultiJson.dump({ name: 'dynamic_test_buildpack', stack: stack.name, position: 1 }) } before { set_current_user_as_admin } @@ -19,6 +20,7 @@ def ordered_buildpacks it do expect(VCAP::CloudController::BuildpacksController).to have_creatable_attributes({ name: { type: 'string', required: true }, + stack: { type: 'string'}, position: { type: 'integer', default: 0 }, enabled: { type: 'bool', default: true }, locked: { type: 'bool', default: false } @@ -28,6 +30,7 @@ def ordered_buildpacks it do expect(VCAP::CloudController::BuildpacksController).to have_updatable_attributes({ name: { type: 'string' }, + stack: { type: 'string' }, position: { type: 'integer' }, enabled: { type: 'bool' }, locked: { type: 'bool' } @@ -59,12 +62,30 @@ def ordered_buildpacks expect(buildpack.position).to eq(1) end + it 'defaults stack to nil' do + expect do + post '/v2/buildpacks', MultiJson.dump({ name: 'a_buildpack', position: 1 }) + expect(last_response.status).to eq(201) + end.to change { Buildpack.count }.from(0).to(1) + buildpack = Buildpack.first + expect(buildpack.stack).to be_nil + end + + it 'uses specified stack' do + expect do + post '/v2/buildpacks', MultiJson.dump({ name: 'a_buildpack', stack: stack.name, position: 1 }) + expect(last_response.status).to eq(201) + end.to change { Buildpack.count }.from(0).to(1) + buildpack = Buildpack.first + expect(buildpack.stack).to eq(stack.name) + end + it 'respects position param' do - Buildpack.create(name: 'pre-existing-buildpack', position: 1) - Buildpack.create(name: 'pre-existing-buildpack-2', position: 2) + Buildpack.create(name: 'pre-existing-buildpack', stack: stack.name, position: 1) + Buildpack.create(name: 'pre-existing-buildpack-2', stack: stack.name, position: 2) expect { - post '/v2/buildpacks', MultiJson.dump({ name: 'new-buildpack', position: 2 }) + post '/v2/buildpacks', MultiJson.dump({ name: 'new-buildpack', stack: stack.name, position: 2 }) }.to change { ordered_buildpacks }.from( [['pre-existing-buildpack', 1], ['pre-existing-buildpack-2', 2]] ).to( @@ -73,15 +94,15 @@ def ordered_buildpacks end it 'returns duplicate name message correctly' do - Buildpack.make(name: 'dynamic_test_buildpack') + Buildpack.make(name: 'dynamic_test_buildpack', stack: stack.name) post '/v2/buildpacks', req_body - expect(last_response.status).to eq(400) - expect(decoded_response['code']).to eq(290001) + expect(last_response.status).to eq(422) + expect(decoded_response['code']).to eq(290000) expect(Buildpack.count).to eq(1) end it 'returns buildpack invalid message correctly' do - post '/v2/buildpacks', MultiJson.dump({ name: 'invalid_name!' }) + post '/v2/buildpacks', MultiJson.dump({ name: 'invalid_name!', stack: stack.name }) expect(last_response.status).to eq(400) expect(decoded_response['code']).to eq(290003) expect(Buildpack.count).to eq(0) @@ -97,8 +118,8 @@ def ordered_buildpacks end describe '#update' do - let!(:buildpack1) { VCAP::CloudController::Buildpack.create({ name: 'first_buildpack', key: 'xyz', filename: 'a', position: 1 }) } - let!(:buildpack2) { VCAP::CloudController::Buildpack.create({ name: 'second_buildpack', key: 'xyz', filename: 'b', position: 2 }) } + let!(:buildpack1) { VCAP::CloudController::Buildpack.create({ name: 'first_buildpack', stack: stack.name, key: 'xyz', filename: 'a', position: 1 }) } + let!(:buildpack2) { VCAP::CloudController::Buildpack.create({ name: 'second_buildpack', stack: stack.name, key: 'xyz', filename: 'b', position: 2 }) } it 'can update the buildpack name' do set_current_user_as_admin @@ -127,7 +148,7 @@ def ordered_buildpacks end describe '#delete' do - let!(:buildpack1) { VCAP::CloudController::Buildpack.create({ name: 'first_buildpack', key: 'xyz', position: 1 }) } + let!(:buildpack1) { VCAP::CloudController::Buildpack.create({ name: 'first_buildpack', stack: stack.name, key: 'xyz', position: 1 }) } it 'returns NOT AUTHORIZED (403) for non admins' do set_current_user(user) diff --git a/spec/unit/controllers/v3/builds_controller_spec.rb b/spec/unit/controllers/v3/builds_controller_spec.rb index 7a688076ec9..98ffe20885c 100644 --- a/spec/unit/controllers/v3/builds_controller_spec.rb +++ b/spec/unit/controllers/v3/builds_controller_spec.rb @@ -154,6 +154,7 @@ describe '#create' do let(:org) { VCAP::CloudController::Organization.make } let(:space) { VCAP::CloudController::Space.make(organization: org) } + let(:stack) { VCAP::CloudController::Stack.default.name } let(:app_model) { VCAP::CloudController::AppModel.make(space: space) } let(:package) do VCAP::CloudController::PackageModel.make( @@ -175,7 +176,7 @@ before do allow(CloudController::DependencyLocator.instance).to receive(:stagers).and_return(stagers) allow(stagers).to receive(:stager_for_app).and_return(stager) - app_model.lifecycle_data.update(buildpacks: nil, stack: VCAP::CloudController::Stack.default.name) + app_model.lifecycle_data.update(buildpacks: nil, stack: stack) set_current_user_as_admin end @@ -248,14 +249,14 @@ end describe 'buildpack lifecycle' do - let(:buildpack) { VCAP::CloudController::Buildpack.make } + let(:buildpack) { VCAP::CloudController::Buildpack.make(stack: stack) } let(:buildpack_request) { 'http://dan-and-zach-awesome-pack.com' } let(:buildpack_lifecycle) do { type: 'buildpack', data: { buildpacks: [buildpack_request], - stack: 'cflinuxfs2' + stack: stack }, } end @@ -316,7 +317,7 @@ type: 'buildpack', data: { buildpacks: [], - stack: 'cflinuxfs2' + stack: stack }, } end @@ -335,7 +336,7 @@ type: 'buildpack', data: { buildpacks: nil, - stack: 'cflinuxfs2' + stack: stack }, } end diff --git a/spec/unit/jobs/runtime/buildpack_delete_spec.rb b/spec/unit/jobs/runtime/buildpack_delete_spec.rb index 3d89f71baca..75c319541e4 100644 --- a/spec/unit/jobs/runtime/buildpack_delete_spec.rb +++ b/spec/unit/jobs/runtime/buildpack_delete_spec.rb @@ -7,7 +7,7 @@ module Jobs::Runtime let(:buildpack_guid) { buildpack.guid } let(:timeout) { 90000 } - let(:buildpack) { VCAP::CloudController::Buildpack.create({ name: 'first_buildpack', key: 'xyz', position: 1 }) } + let(:buildpack) { VCAP::CloudController::Buildpack.make } before do allow(BuildpackBitsDelete).to receive(:delete_when_safe) diff --git a/spec/unit/jobs/runtime/buildpack_installer_spec.rb b/spec/unit/jobs/runtime/buildpack_installer_spec.rb index 6fbef48f115..2bcb37660d3 100644 --- a/spec/unit/jobs/runtime/buildpack_installer_spec.rb +++ b/spec/unit/jobs/runtime/buildpack_installer_spec.rb @@ -20,30 +20,132 @@ module Jobs::Runtime context 'when the buildpack is enabled and unlocked' do let(:options) { { locked: true } } - it 'creates a new buildpack' do - expect { - job.perform - }.to change { Buildpack.count }.from(0).to(1) - - buildpack = Buildpack.find(name: buildpack_name) - expect(buildpack).to_not be_nil - expect(buildpack.name).to eq(buildpack_name) - expect(buildpack.key).to start_with(buildpack.guid) - expect(buildpack.filename).to end_with(File.basename(zipfile)) - expect(buildpack).to be_locked + context 'buildpack zip does not specify stack' do + it 'creates a new buildpack with default stack' do + expect { + job.perform + }.to change { Buildpack.count }.from(0).to(1) + + buildpack = Buildpack.first + expect(buildpack).to_not be_nil + expect(buildpack.name).to eq(buildpack_name) + expect(buildpack.stack).to eq(Stack.default.name) + expect(buildpack.key).to start_with(buildpack.guid) + expect(buildpack.filename).to end_with(File.basename(zipfile)) + expect(buildpack).to be_locked + end + + it 'updates an existing buildpack' do + buildpack1 = Buildpack.make(name: buildpack_name, key: 'new_key') + + update_job = BuildpackInstaller.new(buildpack_name, zipfile2, { enabled: false }) + update_job.perform + + buildpack2 = Buildpack.find(name: buildpack_name) + expect(buildpack2).to_not be_nil + expect(buildpack2.enabled).to be false + expect(buildpack2.filename).to end_with(File.basename(zipfile2)) + expect(buildpack2.key).to_not eql(buildpack1.key) + end + + it 'does nothing if multiple buildpacks with same name' do + Stack.make(name: 'stack-1') + Stack.make(name: 'stack-2') + Buildpack.make(name: buildpack_name, stack: 'stack-1', filename: nil) + Buildpack.make(name: buildpack_name, stack: 'stack-2', filename: nil) + + update_job = BuildpackInstaller.new(buildpack_name, zipfile2, { enabled: false }) + expect { + update_job.perform + }.to_not change { Buildpack.count } + + buildpack1 = Buildpack.find(name: buildpack_name, stack: 'stack-1') + expect(buildpack1).to_not be_nil + expect(buildpack1.filename).to be_nil + + buildpack2 = Buildpack.find(name: buildpack_name, stack: 'stack-2') + expect(buildpack2).to_not be_nil + expect(buildpack2.filename).to be_nil + end end - it 'updates an existing buildpack' do - buildpack1 = Buildpack.make(name: buildpack_name, key: 'new_key') + context 'buildpack zip specifies stack' do + before { Stack.make(name: 'manifest-stack') } + let(:zipfile) do + path = Tempfile.new('bp-zip-with-stack').path + TestZip.create(path, 1, 1024) do |zipfile| + zipfile.get_output_stream('manifest.yml') do |f| + f.write("---\nstack: manifest-stack\n") + end + end + path + end + after { FileUtils.rm(zipfile) } - update_job = BuildpackInstaller.new(buildpack_name, zipfile2, { enabled: false }) - update_job.perform + it 'creates a new buildpack with that stack' do + expect { + job.perform + }.to change { Buildpack.count }.from(0).to(1) + + buildpack = Buildpack.first + expect(buildpack).to_not be_nil + expect(buildpack.name).to eq(buildpack_name) + expect(buildpack.stack).to eq('manifest-stack') + expect(buildpack.key).to start_with(buildpack.guid) + expect(buildpack.filename).to end_with(File.basename(zipfile)) + expect(buildpack).to be_locked + end - buildpack2 = Buildpack.find(name: buildpack_name) - expect(buildpack2).to_not be_nil - expect(buildpack2.enabled).to be false - expect(buildpack2.filename).to end_with(File.basename(zipfile2)) - expect(buildpack2.key).to_not eql(buildpack1.key) + it 'updates an existing buildpack' do + buildpack1 = Buildpack.make(name: buildpack_name, stack: 'manifest-stack', key: 'new_key') + + update_job = BuildpackInstaller.new(buildpack_name, zipfile, { enabled: false }) + update_job.perform + + buildpack2 = Buildpack.find(name: buildpack_name) + expect(buildpack2).to_not be_nil + expect(buildpack2.enabled).to be false + expect(buildpack2.filename).to end_with(File.basename(zipfile)) + expect(buildpack2.key).to_not eql(buildpack1.key) + end + + it 'updates an existing buildpack with nil stack' do + buildpack1 = Buildpack.make(name: buildpack_name, stack: nil, key: 'new_key') + + update_job = BuildpackInstaller.new(buildpack_name, zipfile, { enabled: false }) + update_job.perform + + buildpack2 = Buildpack.find(name: buildpack_name) + expect(buildpack2).to_not be_nil + expect(buildpack2.enabled).to be false + expect(buildpack2.filename).to end_with(File.basename(zipfile)) + expect(buildpack2.key).to_not eql(buildpack1.key) + expect(buildpack2.stack).to eql('manifest-stack') + end + + it 'creates a new buildpack if existing buildpacks have different stacks' do + Stack.make(name: 'stack-1') + Stack.make(name: 'stack-2') + Buildpack.make(name: buildpack_name, stack: 'stack-1', filename: nil) + Buildpack.make(name: buildpack_name, stack: 'stack-2', filename: nil) + + update_job = BuildpackInstaller.new(buildpack_name, zipfile, { enabled: false }) + expect { + update_job.perform + }.to change { Buildpack.count }.by(1) + + buildpack1 = Buildpack.find(name: buildpack_name, stack: 'stack-1') + expect(buildpack1).to_not be_nil + expect(buildpack1.filename).to be_nil + + buildpack2 = Buildpack.find(name: buildpack_name, stack: 'stack-2') + expect(buildpack2).to_not be_nil + expect(buildpack2.filename).to be_nil + + buildpack3 = Buildpack.find(name: buildpack_name, stack: 'manifest-stack') + expect(buildpack3).to_not be_nil + expect(buildpack3.filename).to_not be_nil + end end end diff --git a/spec/unit/lib/cloud_controller/buildpacks/stack_name_extractor_spec.rb b/spec/unit/lib/cloud_controller/buildpacks/stack_name_extractor_spec.rb new file mode 100644 index 00000000000..bacecd22a97 --- /dev/null +++ b/spec/unit/lib/cloud_controller/buildpacks/stack_name_extractor_spec.rb @@ -0,0 +1,82 @@ +require 'spec_helper' + +module VCAP::CloudController::Buildpacks + RSpec.describe StackNameExtractor do + let(:tmpdir) { Dir.mktmpdir } + let(:zip_path) { File.join(tmpdir, 'file.zip') } + + def zip_with_manifest_content(manifest_content) + TestZip.create(zip_path, 1, 1024) do |zipfile| + if manifest_content + zipfile.get_output_stream('manifest.yml') do |f| + f.write(manifest_content) + end + end + end + end + + describe '.extract_from_file' do + context 'buildpack zip is not a valid zip file' do + before do + File.write(zip_path, 'INVALID_FOR_A_ZIP_FILE_THIS_IS') + end + + it 'raises an error' do + expect { StackNameExtractor.extract_from_file(zip_path) }.to raise_error(CloudController::Errors::BuildpackError, 'buildpack zipfile is not valid') + end + end + + context 'buildpack zip contains a manifest specifying the stack' do + before do + zip_with_manifest_content("---\nstack: ITSaSTACK\n") + end + + it 'returns the stack in the manifest' do + expect(StackNameExtractor.extract_from_file(zip_path)).to eq('ITSaSTACK') + end + end + + context 'buildpack zip contains manifest that does not specify stack' do + before do + zip_with_manifest_content("---\nsomethingelse: NOTstackTHOUGH\n") + end + + it 'returns nil' do + expect(StackNameExtractor.extract_from_file(zip_path)).to be_nil + end + end + + context 'buildpack zip contains manifest which is not valid' do + before do + zip_with_manifest_content("\0xde\0xad\0xbe\0xef") + end + + it 'raises an error' do + expect { StackNameExtractor.extract_from_file(zip_path) }.to raise_error(CloudController::Errors::BuildpackError, 'buildpack manifest is not valid') + end + end + + context 'buildpack zip does not contain manifest' do + before do + TestZip.create(zip_path, 3, 1024) + end + + it 'returns nil' do + expect(StackNameExtractor.extract_from_file(zip_path)).to be_nil + end + end + + context 'buildpack zip contains manifest but it is too large (>1mb)' do + before do + alphachars = [*'A'..'Z'] + megabyte_string = (0...(1024 * 1024)).map { alphachars.sample }.join + zip_with_manifest_content("---\nstack: cflinuxfs2\nabsurdly_long_value: " + megabyte_string) + end + + it 'raises an error' do + expect { StackNameExtractor.extract_from_file(zip_path) }.to raise_error(CloudController::Errors::BuildpackError, 'buildpack manifest is too large') + end + end + end + end +end diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/buildpack_entry_generator_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/buildpack_entry_generator_spec.rb index 97d78d45582..58d6ba164b8 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/buildpack_entry_generator_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/buildpack_entry_generator_spec.rb @@ -12,12 +12,17 @@ module Buildpack let(:build_artifacts_cache_download_uri) { 'http://buildpack-artifacts-cache.example.com' } let(:blobstore_url_generator) { double('fake url generator') } + let(:stack) { Stack.make } + let(:stack2) { Stack.make } let!(:java_buildpack) do - VCAP::CloudController::Buildpack.create(name: 'java', key: 'java-buildpack-key', position: 1, sha256_checksum: 'checksum') + VCAP::CloudController::Buildpack.create(name: 'java', stack: stack.name, key: 'java-buildpack-key', position: 1, sha256_checksum: 'checksum') end let!(:ruby_buildpack) do - VCAP::CloudController::Buildpack.create(name: 'ruby', key: 'ruby-buildpack-key', position: 2, sha256_checksum: 'checksum') + VCAP::CloudController::Buildpack.create(name: 'ruby', stack: stack.name, key: 'ruby-buildpack-key', position: 2, sha256_checksum: 'checksum') + end + let!(:ruby_buildpack_other_stack) do + VCAP::CloudController::Buildpack.create(name: 'ruby', stack: stack2.name, key: 'ruby-buildpack-stack2-key', position: 3, sha256_checksum: 'checksum') end before do @@ -40,8 +45,8 @@ module Buildpack let(:admin_buildpack_info) { BuildpackInfo.new('java', VCAP::CloudController::Buildpack.find(name: 'java')) } let(:buildpack_infos) { [custom_buildpack_info, admin_buildpack_info] } - it 'returns both buildpacks' do - expect(buildpack_entry_generator.buildpack_entries(buildpack_infos)).to eq([ + it 'returns both buildpacks for the stack' do + expect(buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name)).to eq([ { name: 'custom', key: 'http://example.com/my_buildpack_url.zip', url: 'http://example.com/my_buildpack_url.zip', skip_detect: true }, { name: 'java', key: 'java-buildpack-key', url: admin_buildpack_download_url, skip_detect: true, sha256: 'checksum' }, ]) @@ -53,7 +58,7 @@ module Buildpack let(:buildpack) { 'http://example.com/my_buildpack_url.zip' } it "should use the buildpack_uri and name it 'custom', and use the url as the key" do - expect(buildpack_entry_generator.buildpack_entries(buildpack_infos)).to eq([ + expect(buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name)).to eq([ { name: 'custom', key: 'http://example.com/my_buildpack_url.zip', url: 'http://example.com/my_buildpack_url.zip', skip_detect: true } ]) end @@ -63,7 +68,7 @@ module Buildpack let(:buildpack) { 'http://example.com/my_buildpack_url' } it "should use the buildpack_uri and name it 'custom', and use the url as the key" do - expect(buildpack_entry_generator.buildpack_entries(buildpack_infos)).to eq([ + expect(buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name)).to eq([ { name: 'custom', key: 'http://example.com/my_buildpack_url', url: 'http://example.com/my_buildpack_url', skip_detect: true } ]) end @@ -74,7 +79,7 @@ module Buildpack let(:buildpack) { 'java' } it 'should use that buildpack' do - expect(buildpack_entry_generator.buildpack_entries(buildpack_infos)).to eq([ + expect(buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name)).to eq([ { name: 'java', key: 'java-buildpack-key', url: admin_buildpack_download_url, skip_detect: true, sha256: 'checksum' } ]) end @@ -85,7 +90,7 @@ module Buildpack end it 'fails fast with a clear error' do - expect { buildpack_entry_generator.buildpack_entries(buildpack_infos) }.to raise_error /Unsupported buildpack type/ + expect { buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name) }.to raise_error /Unsupported buildpack type/ end end end @@ -93,10 +98,18 @@ module Buildpack context 'when the user has not requested a buildpack' do let(:buildpack_infos) { [] } - it 'should use the list of admin buildpacks' do - expect(buildpack_entry_generator.buildpack_entries(buildpack_infos)).to eq([ + it 'should use the list of admin buildpacks for the specified stack' do + expect(buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name)).to eq([ + { name: 'java', key: 'java-buildpack-key', url: admin_buildpack_download_url, sha256: 'checksum', skip_detect: false }, + { name: 'ruby', key: 'ruby-buildpack-key', url: admin_buildpack_download_url, sha256: 'checksum', skip_detect: false }, + ]) + end + + it 'should use the list of all admin buildpacks if no stack is specified' do + expect(buildpack_entry_generator.buildpack_entries(buildpack_infos, nil)).to eq([ { name: 'java', key: 'java-buildpack-key', url: admin_buildpack_download_url, sha256: 'checksum', skip_detect: false }, { name: 'ruby', key: 'ruby-buildpack-key', url: admin_buildpack_download_url, sha256: 'checksum', skip_detect: false }, + { name: 'ruby', key: 'ruby-buildpack-stack2-key', url: admin_buildpack_download_url, sha256: 'checksum', skip_detect: false }, ]) end end @@ -105,7 +118,7 @@ module Buildpack let(:buildpack) { '???' } it 'fails fast with a clear error' do - expect { buildpack_entry_generator.buildpack_entries(buildpack_infos) }.to raise_error /Unsupported buildpack type/ + expect { buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name) }.to raise_error /Unsupported buildpack type/ end end end diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb index 23d919b92cb..4a0dac79431 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb @@ -41,7 +41,8 @@ module Buildpack end before do - VCAP::CloudController::Buildpack.create(name: 'ruby', key: 'ruby-buildpack-key', position: 2) + Stack.create(name: "potato-stack") + VCAP::CloudController::Buildpack.create(name: 'ruby', stack: 'potato-stack', key: 'ruby-buildpack-key', position: 2) allow(blobstore_url_generator).to receive(:admin_buildpack_download_url).and_return('bp-download-url') end diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb index 8a630acb1f4..0286951244a 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb @@ -146,8 +146,33 @@ module VCAP::CloudController end context 'and the app does not have a stack' do - it 'uses the default value for stack' do - expect(buildpack_lifecycle.staging_stack).to eq(Stack.default.name) + let(:request_data) do + { + buildpacks: ['cool-buildpack'] + } + end + before do + Stack.create(name: 'cool-stack') + end + + context 'and there is only one buildpack with the specified name' do + before do + Buildpack.make(name: 'cool-buildpack', stack: 'cool-stack') + end + it 'uses the stack of the buildpack' do + expect(buildpack_lifecycle.staging_stack).to eq('cool-stack') + end + end + + context 'and there are multiple buildpacks with different stacks' do + before do + Buildpack.make(name: 'cool-buildpack', stack: 'cool-stack') + Buildpack.make(name: 'cool-buildpack', stack: Stack.default.name) + end + + it 'uses the default value for stack' do + expect(buildpack_lifecycle.staging_stack).to eq(Stack.default.name) + end end end end diff --git a/spec/unit/lib/cloud_controller/upload_buildpack_spec.rb b/spec/unit/lib/cloud_controller/upload_buildpack_spec.rb index f8334202d4b..cd11eba6cd7 100644 --- a/spec/unit/lib/cloud_controller/upload_buildpack_spec.rb +++ b/spec/unit/lib/cloud_controller/upload_buildpack_spec.rb @@ -3,7 +3,7 @@ module VCAP::CloudController RSpec.describe UploadBuildpack do let(:buildpack_blobstore) { double(:buildpack_blobstore).as_null_object } - let!(:buildpack) { VCAP::CloudController::Buildpack.create_from_hash({ name: 'upload_binary_buildpack', position: 0 }) } + let!(:buildpack) { VCAP::CloudController::Buildpack.create_from_hash({ name: 'upload_binary_buildpack', stack: 'cflinuxfs2', position: 0 }) } let(:upload_buildpack) { UploadBuildpack.new(buildpack_blobstore) } @@ -13,9 +13,16 @@ module VCAP::CloudController let(:sha_valid_zip) { Digester.new(algorithm: Digest::SHA256).digest_file(valid_zip) } let(:sha_valid_zip2) { Digester.new(algorithm: Digest::SHA256).digest_file(valid_zip2) } + let(:valid_zip_manifest_stack) { nil } let(:valid_zip) do zip_name = File.join(tmpdir, filename) - TestZip.create(zip_name, 1, 1024) + TestZip.create(zip_name, 1, 1024) do |zipfile| + if valid_zip_manifest_stack + zipfile.get_output_stream('manifest.yml') do |f| + f.write("---\nstack: #{valid_zip_manifest_stack}\n") + end + end + end zip_file = File.new(zip_name) Rack::Test::UploadedFile.new(zip_file) end @@ -37,6 +44,102 @@ module VCAP::CloudController end context 'and the upload to the blobstore succeeds' do + context 'stack from manifest' do + context 'manifest file is too large (>1mb)' do + let(:valid_zip_manifest_stack) { 'cflinuxfs2' } + let(:zip_with_massive_manifest) do + zip_name = File.join(tmpdir, filename) + TestZip.create(zip_name, 1, 1024) do |zipfile| + if valid_zip_manifest_stack + zipfile.get_output_stream('manifest.yml') do |f| + alphachars = [*'A'..'Z'] + megabyte_string = (0...(1024*1024)).map { alphachars.sample }.join + f.write("---\nstack: cflinuxfs2\nabsurdly_long_value: " + megabyte_string) + end + end + end + zip_file = File.new(zip_name) + Rack::Test::UploadedFile.new(zip_file) + end + + it 'returns an error and does not update stack' do + expect { + upload_buildpack.upload_buildpack(buildpack, zip_with_massive_manifest, filename) + }.to raise_error(CloudController::Errors::ApiError, /Buildpack zip error/) + bp = Buildpack.find(name: buildpack.name) + expect(bp).to_not be_nil + expect(bp.stack).to eq('cflinuxfs2') + end + end + + context 'same as buildpack' do + let(:valid_zip_manifest_stack) { 'cflinuxfs2' } + + it 'copies new bits to the blobstore' do + expect(buildpack_blobstore).to receive(:cp_to_blobstore).with(valid_zip, expected_sha_valid_zip) + + upload_buildpack.upload_buildpack(buildpack, valid_zip, filename) + end + end + + context 'different from buildpack' do + let(:valid_zip_manifest_stack) { 'cflinuxfs3' } + before do + VCAP::CloudController::Stack.create(name: 'cflinuxfs3') + end + + it 'raises an error and does not update stack' do + expect { + upload_buildpack.upload_buildpack(buildpack, valid_zip, filename) + }.to raise_error(CloudController::Errors::ApiError, /Uploaded buildpack stack \(cflinuxfs3\) does not match cflinuxfs2/) + bp = Buildpack.find(name: buildpack.name) + expect(bp).to_not be_nil + expect(bp.stack).to eq('cflinuxfs2') + end + end + + context 'stack previously unknown' do + let!(:buildpack) { VCAP::CloudController::Buildpack.create_from_hash({ name: 'upload_binary_buildpack', stack: nil, position: 0 }) } + context 'known' do + let(:valid_zip_manifest_stack) { 'cflinuxfs3' } + before do + VCAP::CloudController::Stack.create(name: 'cflinuxfs3') + end + + it 'copies new bits to the blobstore and updates the stack' do + expect(buildpack_blobstore).to receive(:cp_to_blobstore).with(valid_zip, expected_sha_valid_zip) + + expect do + upload_buildpack.upload_buildpack(buildpack, valid_zip, filename) + end.to change { buildpack.stack }.from(nil).to('cflinuxfs3') + end + + context 'buildpack with same name and stack exists' do + let(:valid_zip_manifest_stack) { 'cflinuxfs3' } + + it 'raises an error' do + VCAP::CloudController::Buildpack.create(name: buildpack.name, stack: 'cflinuxfs3') + expect { + upload_buildpack.upload_buildpack(buildpack, valid_zip, filename) + }.to raise_error(CloudController::Errors::ApiError, /The buildpack name #{buildpack.name} is already in use for the stack #{valid_zip_manifest_stack}/) + end + end + end + + context 'non-existent buildpack' do + let(:valid_zip_manifest_stack) { 'new-and-unknown' } + it 'raises an error' do + expect { + upload_buildpack.upload_buildpack(buildpack, valid_zip, filename) + }.to raise_error(CloudController::Errors::ApiError, /Uploaded buildpack stack \(#{valid_zip_manifest_stack}\) does not exist/) + bp = Buildpack.find(name: buildpack.name) + expect(bp).to_not be_nil + expect(bp.filename).to be_nil + end + end + end + end + it 'updates the buildpack filename' do expect { upload_buildpack.upload_buildpack(buildpack, valid_zip, filename) @@ -116,7 +219,7 @@ module VCAP::CloudController end context 'when the same bits are uploaded twice' do - let(:buildpack2) { VCAP::CloudController::Buildpack.create_from_hash({ name: 'buildpack2', position: 0 }) } + let(:buildpack2) { VCAP::CloudController::Buildpack.create_from_hash({ name: 'buildpack2', stack: 'cflinuxfs2', position: 0 }) } it 'should have different keys' do upload_buildpack.upload_buildpack(buildpack, valid_zip2, filename) diff --git a/spec/unit/models/runtime/buildpack_spec.rb b/spec/unit/models/runtime/buildpack_spec.rb index f0f5bce0948..00560effdfd 100644 --- a/spec/unit/models/runtime/buildpack_spec.rb +++ b/spec/unit/models/runtime/buildpack_spec.rb @@ -9,7 +9,32 @@ def ordered_buildpacks it { is_expected.to have_timestamp_columns } describe 'Validations' do - it { is_expected.to validate_uniqueness :name } + it { is_expected.to validate_uniqueness [:name, :stack] } + + describe 'stack' do + it 'can be changed if not set' do + buildpack = Buildpack.create(name: 'test', stack: nil) + buildpack.stack = Stack.make.name + + expect(buildpack).to be_valid + end + + it 'cannot be changed once it is set' do + buildpack = Buildpack.create(name: 'test', stack: Stack.make.name) + buildpack.stack = Stack.make.name + + expect(buildpack).not_to be_valid + expect(buildpack.errors.on(:stack)).to include(:buildpack_cant_change_stacks) + end + + it 'cannot be changed to a stack that doesn\'t exist' do + buildpack = Buildpack.create(name: 'test', stack: nil) + buildpack.stack = 'this-stack-isnt-real' + + expect(buildpack).not_to be_valid + expect(buildpack.errors.on(:stack)).to include(:buildpack_stack_does_not_exist) + end + end describe 'name' do it 'does not allow non-word non-dash characters' do @@ -30,8 +55,8 @@ def ordered_buildpacks end describe 'Serialization' do - it { is_expected.to export_attributes :name, :position, :enabled, :locked, :filename } - it { is_expected.to import_attributes :name, :position, :enabled, :locked, :filename, :key } + it { is_expected.to export_attributes :name, :stack, :position, :enabled, :locked, :filename } + it { is_expected.to import_attributes :name, :stack, :position, :enabled, :locked, :filename, :key } it 'does not string mung(e)?' do expect(Buildpack.new(name: "my_custom_buildpack\r\n").to_json).to eq '"my_custom_buildpack\r\n"' @@ -92,7 +117,7 @@ def ordered_buildpacks end context 'and there are buildpacks with null keys' do - let!(:null_buildpack) { Buildpack.create(name: 'nil_key_custom_buildpack', position: 0) } + let!(:null_buildpack) { Buildpack.create(name: 'nil_key_custom_buildpack', stack: Stack.make.name, position: 0) } it 'only returns buildpacks with non-null keys' do expect(Buildpack.all).to include(null_buildpack) @@ -102,7 +127,7 @@ def ordered_buildpacks end context 'and there are buildpacks with empty keys' do - let!(:empty_buildpack) { Buildpack.create(name: 'nil_key_custom_buildpack', key: '', position: 0) } + let!(:empty_buildpack) { Buildpack.create(name: 'nil_key_custom_buildpack', stack: Stack.make.name, key: '', position: 0) } it 'only returns buildpacks with non-null keys' do expect(Buildpack.all).to include(empty_buildpack) @@ -137,7 +162,7 @@ def ordered_buildpacks describe '#update' do let!(:buildpacks) do - Array.new(4) { |i| Buildpack.create(name: "name_#{100 - i}", position: i + 1) } + Array.new(4) { |i| Buildpack.create(name: "name_#{100 - i}", stack: Stack.make.name, position: i + 1) } end it 'does not modify the frozen hash provided by Sequel' do @@ -148,8 +173,8 @@ def ordered_buildpacks end describe '#destroy' do - let!(:buildpack1) { VCAP::CloudController::Buildpack.create({ name: 'first_buildpack', key: 'xyz', position: 1 }) } - let!(:buildpack2) { VCAP::CloudController::Buildpack.create({ name: 'second_buildpack', key: 'xyz', position: 2 }) } + let!(:buildpack1) { VCAP::CloudController::Buildpack.create({ name: 'first_buildpack', stack: Stack.make.name, key: 'xyz', position: 1 }) } + let!(:buildpack2) { VCAP::CloudController::Buildpack.create({ name: 'second_buildpack', stack: Stack.make.name, key: 'xyz', position: 2 }) } it 'removes the specified buildpack' do expect { diff --git a/vendor/errors/v2.yml b/vendor/errors/v2.yml index 7ff6e7b2200..d86d514829c 100644 --- a/vendor/errors/v2.yml +++ b/vendor/errors/v2.yml @@ -858,6 +858,11 @@ http_code: 502 message: "Service broker returned dashboard client configuration without a dashboard URL" +290000: + name: BuildpackNameStackTaken + http_code: 422 + message: "The buildpack name %s is already in use for the stack %s" + 290001: name: BuildpackNameTaken http_code: 400 @@ -1148,3 +1153,18 @@ name: SharedServiceInstanceNotDeletableInTargetSpace http_code: 403 message: 'You cannot delete service instances that have been shared with you' + +390011: + name: BuildpackStacksDontMatch + http_code: 422 + message: 'Uploaded buildpack stack (%s) does not match %s' + +390012: + name: BuildpackStackDoesNotExist + http_code: 422 + message: 'Uploaded buildpack stack (%s) does not exist' + +390013: + name: BuildpackZipError + http_code: 422 + message: 'Buildpack zip error: %s'