Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC0031] Support for System Cloud Native Buildpacks #3898

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/actions/build_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ def create_and_stage(package:, lifecycle:, metadata: nil, start_after_staging: f

def requested_buildpacks_disabled!(lifecycle)
return if lifecycle.type == Lifecycles::DOCKER
return if lifecycle.type == Lifecycles::CNB

admin_buildpack_records = lifecycle.buildpack_infos.map(&:buildpack_record).compact
disabled_buildpacks = admin_buildpack_records.reject(&:enabled)
Expand Down
5 changes: 4 additions & 1 deletion app/actions/buildpack_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ def create(message)
buildpack = Buildpack.create(
name: message.name,
stack: message.stack,
lifecycle: (message.lifecycle.nil? ? Config.config.get(:default_app_lifecycle) : message.lifecycle),
enabled: (message.enabled.nil? ? DEFAULT_ENABLED : message.enabled),
locked: (message.locked.nil? ? DEFAULT_LOCKED : message.locked)
)
Expand All @@ -28,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)
Expand Down
5 changes: 4 additions & 1 deletion app/actions/buildpack_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ def validation_error!(error, message)
error!(%(Stack '#{message.stack}' does not exist)) if error.errors.on(:stack)&.include?(:buildpack_stack_does_not_exist)
error!(%(Buildpack stack cannot be changed)) if error.errors.on(:stack)&.include?(:buildpack_cant_change_stacks)
error!(%(Buildpack with name '#{error.model.name}' and an unassigned stack already exists)) if error.errors.on(:stack)&.include?(:unique)
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!(error.message)
end
Expand Down
7 changes: 4 additions & 3 deletions app/controllers/runtime/buildpacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@ def self.dependencies
define_attributes do
attribute :name, String
attribute :stack, String, default: nil
attribute :lifecycle, String, default: nil, exclude_in: :update
attribute :position, Integer, default: 0
attribute :enabled, Message::Boolean, default: true
attribute :locked, Message::Boolean, default: false
end

query_parameters :name, :stack
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
Expand Down
10 changes: 6 additions & 4 deletions app/fetchers/buildpack_lifecycle_fetcher.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
require 'cloud_controller/diego/lifecycles/lifecycles'

module VCAP::CloudController
class BuildpackLifecycleFetcher
class << self
def fetch(buildpack_names, stack_name)
def fetch(buildpack_names, stack_name, lifecycle=Config.config.get(:default_app_lifecycle))
{
stack: Stack.find(name: stack_name),
buildpack_infos: ordered_buildpacks(buildpack_names, stack_name)
buildpack_infos: ordered_buildpacks(buildpack_names, stack_name, lifecycle)
}
end

private

def ordered_buildpacks(buildpack_names, stack_name)
buildpacks_with_stacks, buildpacks_without_stacks = Buildpack.list_admin_buildpacks(stack_name).partition(&:stack)
def ordered_buildpacks(buildpack_names, stack_name, lifecycle)
buildpacks_with_stacks, buildpacks_without_stacks = Buildpack.list_admin_buildpacks(stack_name, lifecycle).partition(&:stack)

buildpack_names.map do |buildpack_name|
buildpack_record = buildpacks_with_stacks.find { |b| b.name == buildpack_name } || buildpacks_without_stacks.find { |b| b.name == buildpack_name }
Expand Down
2 changes: 2 additions & 0 deletions app/fetchers/buildpack_list_fetcher.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'cloud_controller/paging/sequel_paginator'
require 'cloud_controller/paging/paginated_result'
require 'cloud_controller/diego/lifecycles/lifecycles'
require 'fetchers/null_filter_query_generator'
require 'fetchers/base_list_fetcher'

Expand All @@ -18,6 +19,7 @@ def filter(message, dataset)
dataset = dataset.where(name: message.names) if message.requested?(:names)

dataset = NullFilterQueryGenerator.add_filter(dataset, :stack, message.stacks) if message.requested?(:stacks)
dataset = dataset.where(lifecycle: message.requested?(:lifecycle) ? message.lifecycle : Config.config.get(:default_app_lifecycle))

if message.requested?(:label_selector)
dataset = LabelSelectorQueryGenerator.add_selector_queries(
Expand Down
3 changes: 1 addition & 2 deletions app/jobs/runtime/buildpack_installer_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ def plan(buildpack_name, manifest_fields)

planned_jobs = []

found_buildpacks = Buildpack.where(name: buildpack_name).all

found_buildpacks = Buildpack.where(name: buildpack_name, lifecycle: manifest_fields[0][:options][:lifecycle]).all
ensure_no_attempt_to_upgrade_a_stackless_locked_buildpack(buildpack_name, found_buildpacks, manifest_fields)

manifest_fields.each do |buildpack_fields|
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/runtime/create_buildpack_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def perform
buildpacks_lock = Locking[name: 'buildpacks']
buildpacks_lock.db.transaction do
buildpacks_lock.lock!
buildpack = Buildpack.create(name: name, stack: stack_name)
buildpack = Buildpack.create(name: name, stack: stack_name, lifecycle: options[:lifecycle])
end
begin
buildpack_uploader.upload_buildpack(buildpack, file, File.basename(file))
Expand Down
39 changes: 12 additions & 27 deletions app/messages/app_manifest_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ def self.underscore_keys(hash)
validate :validate_buildpack_and_buildpacks_combination!
validate :validate_docker_enabled!
validate :validate_cnb_enabled!
validate :validate_cnb_buildpacks!
validate :validate_docker_buildpacks_combination!
validate :validate_service_bindings_message!, if: ->(record) { record.requested?(:services) }
validate :validate_env_update_message!, if: ->(record) { record.requested?(:env) }
Expand Down Expand Up @@ -128,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

Expand Down Expand Up @@ -285,15 +284,6 @@ def docker_lifecycle_data
end

def cnb_lifecycle_data
return unless requested?(:buildpacks) || requested?(:buildpack) || requested?(:stack)

if requested?(:buildpacks)
requested_buildpacks = @buildpacks
elsif requested?(:buildpack)
requested_buildpacks = []
requested_buildpacks.push(@buildpack)
end

{
type: Lifecycles::CNB,
data: {
Expand All @@ -305,16 +295,8 @@ def cnb_lifecycle_data
end

def buildpacks_lifecycle_data
return unless requested?(:buildpacks) || requested?(:buildpack) || requested?(:stack)

if requested?(:buildpacks)
requested_buildpacks = @buildpacks
elsif requested?(:buildpack)
requested_buildpacks = []
requested_buildpacks.push(@buildpack) unless should_autodetect?(@buildpack)
end

{
type: Lifecycles::BUILDPACK,
data: {
buildpacks: requested_buildpacks,
stack: @stack
Expand Down Expand Up @@ -485,13 +467,6 @@ def validate_cnb_enabled!
errors.add(:base, e.message)
end

def validate_cnb_buildpacks!
return unless @lifecycle == 'cnb'
return if requested?(:lifecycle) && (requested?(:buildpack) || requested?(:buildpacks))

errors.add(:base, 'Buildpack(s) must be specified when using Cloud Native Buildpacks')
end

def validate_docker_buildpacks_combination!
return unless requested?(:docker) && (requested?(:buildpack) || requested?(:buildpacks))

Expand All @@ -501,5 +476,15 @@ def validate_docker_buildpacks_combination!
def add_process_error!(error_message, type)
errors.add(:base, %(Process "#{type}": #{error_message}))
end

def requested_buildpacks
return nil unless requested?(:buildpacks) || requested?(:buildpack)
return @buildpacks if requested?(:buildpacks)

buildpacks = []
buildpacks.push(@buildpack) if requested?(:buildpack) && !should_autodetect?(@buildpack)

buildpacks
end
end
end
8 changes: 7 additions & 1 deletion app/messages/buildpack_create_message.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
require 'messages/metadata_base_message'
require 'messages/validators'
require 'cloud_controller/diego/lifecycles/lifecycles'

module VCAP::CloudController
class BuildpackCreateMessage < MetadataBaseMessage
MAX_BUILDPACK_NAME_LENGTH = 250
MAX_STACK_LENGTH = 250

register_allowed_keys %i[name stack position enabled locked]
register_allowed_keys %i[name stack position enabled locked lifecycle]
validates_with NoAdditionalKeysValidator

validates :name,
Expand All @@ -32,5 +33,10 @@ class BuildpackCreateMessage < MetadataBaseMessage
validates :locked,
allow_nil: true,
boolean: true

validates :lifecycle,
string: true,
allow_nil: true,
inclusion: { in: [VCAP::CloudController::Lifecycles::BUILDPACK, VCAP::CloudController::Lifecycles::CNB], message: 'must be either "buildpack" or "cnb"' }
end
end
15 changes: 11 additions & 4 deletions app/messages/buildpack_upload_message.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
require 'messages/base_message'

module VCAP::CloudController
GZIP_MIME = Regexp.new("\x1F\x8B\x08".force_encoding('binary'))
ZIP_MIME = Regexp.new("PK\x03\x04".force_encoding('binary'))

class BuildpackUploadMessage < BaseMessage
class MissingFilePathError < StandardError; end

register_allowed_keys %i[bits_path bits_name upload_start_time]

validates_with NoAdditionalKeysValidator

validate :nginx_fields
validate :bits_path_in_tmpdir
validate :is_zip
validate :is_archive
validate :is_not_empty
validate :missing_file_path

Expand Down Expand Up @@ -43,10 +45,15 @@ def tmpdir
VCAP::CloudController::Config.config.get(:directories, :tmpdir)
end

def is_zip
def is_archive
return unless bits_name
return unless bits_path

mime_bits = File.read(bits_path, 4)

return if mime_bits =~ /^#{VCAP::CloudController::GZIP_MIME}/ || mime_bits =~ /^#{VCAP::CloudController::ZIP_MIME}/

errors.add(:base, "#{bits_name} is not a zip") unless File.extname(bits_name) == '.zip'
errors.add(:base, "#{bits_name} is not a zip or gzip archive")
end

def missing_file_path
Expand Down
5 changes: 5 additions & 0 deletions app/messages/buildpacks_list_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,17 @@ class BuildpacksListMessage < MetadataListMessage
register_allowed_keys %i[
stacks
names
lifecycle
page
per_page
]

validates :names, array: true, allow_nil: true
validates :stacks, array: true, allow_nil: true
validates :lifecycle,
string: true,
allow_nil: true,
inclusion: { in: [VCAP::CloudController::Lifecycles::BUILDPACK, VCAP::CloudController::Lifecycles::CNB], message: 'must be either "buildpack" or "cnb"' }

validates_with NoAdditionalParamsValidator

Expand Down
35 changes: 25 additions & 10 deletions app/models/runtime/buildpack.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,35 @@
require 'cloud_controller/diego/lifecycles/lifecycles'

module VCAP::CloudController
class Buildpack < Sequel::Model
plugin :list
plugin :list, scope: :lifecycle
plugin :after_initialize

export_attributes :name, :stack, :position, :enabled, :locked, :filename
import_attributes :name, :stack, :position, :enabled, :locked, :filename, :key
export_attributes :name, :stack, :position, :enabled, :locked, :filename, :lifecycle
import_attributes :name, :stack, :position, :enabled, :locked, :filename, :lifecycle, :key

PACKAGE_STATES = [
CREATED_STATE = 'AWAITING_UPLOAD'.freeze,
READY_STATE = 'READY'.freeze
].map(&:freeze).freeze

def after_initialize
self.lifecycle ||= Config.config.get(:default_app_lifecycle)
end

one_to_many :labels, class: 'VCAP::CloudController::BuildpackLabelModel', key: :resource_guid, primary_key: :guid
one_to_many :annotations, class: 'VCAP::CloudController::BuildpackAnnotationModel', key: :resource_guid, primary_key: :guid

add_association_dependencies labels: :destroy
add_association_dependencies annotations: :destroy

def self.list_admin_buildpacks(stack_name=nil)
def self.user_visibility_filter(_user)
full_dataset_filter
end

def self.list_admin_buildpacks(stack_name=nil, lifecycle=Config.config.get(:default_app_lifecycle))
scoped = exclude(key: nil).exclude(key: '')
scoped = scoped.filter(lifecycle:)
if stack_name.present?
scoped = scoped.filter(Sequel.or([
[:stack, stack_name],
Expand All @@ -31,17 +43,14 @@ def self.at_last_position
where(position: max(:position)).first
end

def self.user_visibility_filter(_user)
full_dataset_filter
end

def validate
validates_unique %i[name stack]
validates_unique %i[name stack lifecycle]
validates_format(/\A(\w|-)+\z/, :name, message: 'can only contain alphanumeric characters')

validate_stack_existence
validate_stack_change
validate_multiple_nil_stacks
validate_lifecycle_change
end

def locked?
Expand Down Expand Up @@ -75,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
Expand All @@ -84,6 +93,12 @@ def validate_stack_change
errors.add(:stack, :buildpack_cant_change_stacks) if column_changes.key?(:stack)
end

def validate_lifecycle_change
return if initial_value(:lifecycle).nil?

errors.add(:lifecycle, :buildpack_cant_change_lifecycle) if column_changes.key?(:lifecycle)
end

def validate_stack_existence
return unless stack

Expand Down
Loading