[RFC0031] Support for System Cloud Native Buildpacks (#3898)

Co-authored-by: Pavel Busko <[email protected]>
Co-authored-by: Nicolas Bender <[email protected]>
Co-authored-by: Ralf Pannemans <[email protected]>
Co-authored-by: Johannes Dillmann <[email protected]>

Author: 4 people

app/actions/build_create.rb

@@ -95,7 +95,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)

app/actions/buildpack_create.rb

@@ -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)
         )
@@ -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)

app/actions/buildpack_update.rb

@@ -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

app/controllers/runtime/buildpacks_controller.rb

@@ -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

app/fetchers/buildpack_lifecycle_fetcher.rb

@@ -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 }

app/fetchers/buildpack_list_fetcher.rb

@@ -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'
 
@@ -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(

app/jobs/runtime/buildpack_installer_factory.rb

@@ -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|

app/jobs/runtime/create_buildpack_installer.rb

@@ -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))

app/messages/app_manifest_message.rb

@@ -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) }
@@ -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
 
@@ -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: {
@@ -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
@@ -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))
 
@@ -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

app/messages/buildpack_create_message.rb

@@ -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,
@@ -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