WIP: Fix passing lifecycle

Co-authored-by: Nicolas Bender <[email protected]>

Author: 2 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/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 = VCAP::CloudController::Lifecycles::BUILDPACK)
         {
           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/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) }
@@ -291,7 +290,7 @@ def cnb_lifecycle_data
         requested_buildpacks = @buildpacks
       elsif requested?(:buildpack)
         requested_buildpacks = []
-        requested_buildpacks.push(@buildpack)
+        requested_buildpacks.push(@buildpack) unless should_autodetect?(@buildpack)
       end
 
       {
@@ -485,13 +484,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))
 

app/messages/buildpack_upload_message.rb

@@ -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
 
@@ -43,10 +45,16 @@ def tmpdir
       VCAP::CloudController::Config.config.get(:directories, :tmpdir)
     end
 
-    def is_zip
+    def is_archive
       return unless bits_name
+      return unless bits_path
+
+      case IO.read(bits_path, 4)
+      when /^#{VCAP::CloudController::GZIP_MIME}/, /^#{VCAP::CloudController::ZIP_MIME}/
+      else
+        errors.add(:base, "#{bits_name} is not a zip or gzip archive")
+      end
 
-      errors.add(:base, "#{bits_name} is not a zip") unless File.extname(bits_name) == '.zip'
     end
 
     def missing_file_path

app/models/runtime/buildpack.rb

@@ -1,3 +1,5 @@
+require 'cloud_controller/diego/lifecycles/lifecycles'
+
 module VCAP::CloudController
   class Buildpack < Sequel::Model
     plugin :list, scope: :lifecycle
@@ -20,7 +22,7 @@ def self.user_visibility_filter(_user)
       full_dataset_filter
     end
 
-    def self.list_admin_buildpacks(stack_name=nil, lifecycle='buildpack')
+    def self.list_admin_buildpacks(stack_name=nil, lifecycle=VCAP::CloudController::Lifecycles::BUILDPACK)
       scoped = exclude(key: nil).exclude(key: '')
       scoped = scoped.filter(:lifecycle => lifecycle)
       if stack_name.present?

app/models/runtime/cnb_lifecycle_data_model.rb

@@ -44,7 +44,7 @@ def buildpacks
     def buildpack_models
       if buildpack_lifecycle_buildpacks.present?
         buildpack_lifecycle_buildpacks.map do |buildpack|
-          CustomBuildpack.new(buildpack.name)
+          Buildpack.find(name: buildpack.name) || CustomBuildpack.new(buildpack.name)
         end
       else
         []
@@ -64,11 +64,7 @@ def first_custom_buildpack_url
     end
 
     def using_custom_buildpack?
-      true
-    end
-
-    def attributes_from_buildpack(buildpack_name)
-      { buildpack_url: buildpack_name, admin_buildpack_name: nil }
+      buildpack_lifecycle_buildpacks.any?(&:custom?)
     end
 
     def to_hash
@@ -96,5 +92,44 @@ def credentials
     def credentials=(creds)
       self.registry_credentials_json = Oj.dump(creds)
     end
+
+    private
+
+    def attributes_from_buildpack_name(buildpack_name)
+      if UriUtils.is_buildpack_uri?(buildpack_name)
+        { buildpack_url: buildpack_name, admin_buildpack_name: nil }
+      else
+        { buildpack_url: nil, admin_buildpack_name: buildpack_name }
+      end
+    end
+
+    def attributes_from_buildpack_key(key)
+      admin_buildpack = Buildpack.find(key:)
+      if admin_buildpack
+        { buildpack_url: nil, admin_buildpack_name: admin_buildpack.name }
+      elsif UriUtils.is_buildpack_uri?(key)
+        { buildpack_url: key, admin_buildpack_name: nil }
+      else
+        {} # Will fail a validity check downstream
+      end
+    end
+
+    def attributes_from_buildpack_hash(buildpack)
+      {
+        buildpack_name: buildpack[:name],
+        version: buildpack[:version]
+      }.merge(buildpack[:key] ? attributes_from_buildpack_key(buildpack[:key]) : attributes_from_buildpack_name(buildpack[:name]))
+    end
+
+    def attributes_from_buildpack(buildpack)
+      if buildpack.is_a?(String)
+        attributes_from_buildpack_name buildpack
+      elsif buildpack.is_a?(Hash)
+        attributes_from_buildpack_hash buildpack
+      else
+        # Don't set anything -- this will fail later on a validity check
+        {}
+      end
+    end
   end
 end

lib/cloud_controller/diego/buildpack/staging_action_builder.rb

@@ -8,58 +8,7 @@ module Diego
     module Buildpack
       class StagingActionBuilder < VCAP::CloudController::Diego::StagingActionBuilder
         def initialize(config, staging_details, lifecycle_data)
-          super(config, staging_details, lifecycle_data, 'buildpack', '/tmp/app', '/tmp/output-cache')
-        end
-
-        def additional_image_layers
-          lifecycle_data[:buildpacks].
-            reject { |buildpack| buildpack[:name] == 'custom' }.
-            map do |buildpack|
-            layer = {
-              name: buildpack[:name],
-              url: buildpack[:url],
-              destination_path: buildpack_path(buildpack[:key]),
-              layer_type: ::Diego::Bbs::Models::ImageLayer::Type::SHARED,
-              media_type: ::Diego::Bbs::Models::ImageLayer::MediaType::ZIP
-            }
-            if buildpack[:sha256]
-              layer[:digest_algorithm] = ::Diego::Bbs::Models::ImageLayer::DigestAlgorithm::SHA256
-              layer[:digest_value] = buildpack[:sha256]
-            end
-
-            ::Diego::Bbs::Models::ImageLayer.new(layer.compact)
-          end
-        end
-
-        def cached_dependencies
-          return nil if @config.get(:diego, :enable_declarative_asset_downloads)
-
-          dependencies = [
-            ::Diego::Bbs::Models::CachedDependency.new(
-              from: LifecycleBundleUriGenerator.uri(config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key]),
-              to: '/tmp/lifecycle',
-              cache_key: "buildpack-#{lifecycle_stack}-lifecycle"
-            )
-          ]
-
-          others = lifecycle_data[:buildpacks].map do |buildpack|
-            next if buildpack[:name] == 'custom'
-
-            buildpack_dependency = {
-              name: buildpack[:name],
-              from: buildpack[:url],
-              to: buildpack_path(buildpack[:key]),
-              cache_key: buildpack[:key]
-            }
-            if buildpack[:sha256]
-              buildpack_dependency[:checksum_algorithm] = 'sha256'
-              buildpack_dependency[:checksum_value] = buildpack[:sha256]
-            end
-
-            ::Diego::Bbs::Models::CachedDependency.new(buildpack_dependency.compact)
-          end.compact
-
-          dependencies.concat(others)
+          super(config, staging_details, lifecycle_data, 'buildpack', '/tmp/app', '/tmp/output-cache', ::Diego::Bbs::Models::ImageLayer::MediaType::ZIP)
         end
 
         def task_environment_variables
@@ -96,14 +45,6 @@ def platform_options_env
 
           arr
         end
-
-        def buildpack_path(buildpack_key)
-          if config.get(:staging, :legacy_md5_buildpack_paths_enabled)
-            "/tmp/buildpacks/#{OpenSSL::Digest::MD5.hexdigest(buildpack_key)}"
-          else
-            "/tmp/buildpacks/#{Digest::XXH64.hexdigest(buildpack_key)}"
-          end
-        end
       end
     end
   end

lib/cloud_controller/diego/cnb/staging_action_builder.rb

@@ -8,7 +8,7 @@ module Diego
     module CNB
       class StagingActionBuilder < VCAP::CloudController::Diego::StagingActionBuilder
         def initialize(config, staging_details, lifecycle_data)
-          super(config, staging_details, lifecycle_data, 'cnb', '/home/vcap/workspace', '/tmp/cache-output.tgz')
+          super(config, staging_details, lifecycle_data, 'cnb', '/home/vcap/workspace', '/tmp/cache-output.tgz', ::Diego::Bbs::Models::ImageLayer::MediaType::TGZ)
         end
 
         def cached_dependencies

lib/cloud_controller/diego/docker/staging_action_builder.rb

@@ -67,6 +67,10 @@ def cached_dependencies
           ]
         end
 
+        def additional_image_layers
+          []
+        end
+
         def stack
           "preloaded:#{config.get(:diego, :docker_staging_stack)}"
         end

lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb

@@ -8,7 +8,7 @@ class AppBuildpackLifecycle < AppBaseLifecycle
     def initialize(message)
       @message = message
 
-      db_result       = BuildpackLifecycleFetcher.fetch(buildpacks, stack)
+      db_result       = BuildpackLifecycleFetcher.fetch(buildpacks, stack, type)
       @validator      = BuildpackLifecycleDataValidator.new({
                                                               buildpack_infos: db_result[:buildpack_infos],
                                                               stack: db_result[:stack]