[RFC0031] Add support for uploading buildpacks packaged as cnb (tar) files (#4342)

* Remove default buildpacks filter and change default sort to 'lifecycle'
* Fix issue with manifest apply jobs determining incorrect lifecycle
* Validate buildpack filetype based on lifecycle
* Update docs
* Support .cnb, .tgz, and .tar.gz as part of install_buildpacks


Co-authored-by: Tom Kennedy <[email protected]>

Author: sethboyles

app/actions/app_apply_manifest.rb

@@ -59,6 +59,7 @@ def apply(app_guid, message)
 
       app_update_message = message.app_update_message
       lifecycle = AppLifecycleProvider.provide_for_update(app_update_message, app)
+
       AppUpdate.new(@user_audit_info, manifest_triggered: true).update(app, app_update_message, lifecycle)
 
       update_routes(app, message)

app/controllers/v3/buildpacks_controller.rb

@@ -77,7 +77,7 @@ def upload
 
     unauthorized! unless permission_queryer.can_write_globally?
 
-    message = BuildpackUploadMessage.create_from_params(hashed_params[:body])
+    message = BuildpackUploadMessage.create_from_params(hashed_params[:body], buildpack.lifecycle)
     combine_messages(message.errors.full_messages) unless message.valid?
 
     unprocessable!('Buildpack is locked') if buildpack.locked

app/controllers/v3/space_manifests_controller.rb

@@ -16,6 +16,7 @@ def apply_manifest
     can_write_space(space)
 
     messages = parsed_app_manifests.map { |app_manifest| AppManifestMessage.create_from_yml(app_manifest) }
+
     errors = messages.each_with_index.flat_map { |message, i| errors_for_message(message, i) }
     compound_error!(errors) unless errors.empty?
 

app/fetchers/buildpack_list_fetcher.rb

@@ -19,7 +19,8 @@ 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))
+
+        dataset = dataset.where(lifecycle: message.lifecycle) if message.requested?(:lifecycle)
 
         if message.requested?(:label_selector)
           dataset = LabelSelectorQueryGenerator.add_selector_queries(

app/messages/app_manifest_message.rb

@@ -123,16 +123,32 @@ def audit_hash
     end
 
     def app_lifecycle_hash
-      lifecycle_data = if requested?(:lifecycle) && @lifecycle == 'cnb'
-                         cnb_lifecycle_data
+      lifecycle_type = if requested?(:lifecycle) && @lifecycle == 'cnb'
+                         Lifecycles::CNB
+                       elsif requested?(:lifecycle) && @lifecycle == 'buildpack'
+                         Lifecycles::BUILDPACK
                        elsif requested?(:docker)
-                         docker_lifecycle_data
-                       elsif requested?(:buildpacks) || requested?(:buildpack) || requested?(:stack)
-                         buildpacks_lifecycle_data
+                         Lifecycles::DOCKER
                        end
 
+      data = {
+        buildpacks: requested_buildpacks,
+        stack: @stack,
+        credentials: @cnb_credentials
+      }.compact
+
+      if lifecycle_type == Lifecycles::DOCKER
+        lifecycle = {
+          type: Lifecycles::DOCKER
+        }
+      elsif lifecycle_type || data.present?
+        lifecycle = {}
+        lifecycle[:type] = lifecycle_type if lifecycle_type.present?
+        lifecycle[:data] = data if data.present?
+      end
+
       {
-        lifecycle: lifecycle_data,
+        lifecycle: lifecycle,
         metadata: requested?(:metadata) ? metadata : nil
       }.compact
     end
@@ -279,31 +295,6 @@ def service_bindings_attribute_mapping
       mapping
     end
 
-    def docker_lifecycle_data
-      { type: Lifecycles::DOCKER }
-    end
-
-    def cnb_lifecycle_data
-      {
-        type: Lifecycles::CNB,
-        data: {
-          buildpacks: requested_buildpacks,
-          stack: @stack,
-          credentials: @cnb_credentials
-        }.compact
-      }
-    end
-
-    def buildpacks_lifecycle_data
-      {
-        type: Lifecycles::BUILDPACK,
-        data: {
-          buildpacks: requested_buildpacks,
-          stack: @stack
-        }.compact
-      }
-    end
-
     def should_autodetect?(buildpack)
       buildpack == 'default' || buildpack == 'null' || buildpack.nil?
     end

app/messages/buildpack_upload_message.rb

@@ -3,6 +3,7 @@
 module VCAP::CloudController
   GZIP_MIME = Regexp.new("\x1F\x8B\x08".force_encoding('binary'))
   ZIP_MIME = Regexp.new("PK\x03\x04".force_encoding('binary'))
+  CNB_MIME = Regexp.new("\x75\x73\x74\x61\x72\x00\x30\x30".force_encoding('binary'))
 
   class BuildpackUploadMessage < BaseMessage
     class MissingFilePathError < StandardError; end
@@ -16,8 +17,15 @@ class MissingFilePathError < StandardError; end
     validate :is_not_empty
     validate :missing_file_path
 
-    def self.create_from_params(params)
-      BuildpackUploadMessage.new(params.dup.symbolize_keys)
+    attr_reader :lifecycle
+
+    def initialize(params, lifecycle)
+      @lifecycle = lifecycle
+      super(params)
+    end
+
+    def self.create_from_params(params, lifecycle)
+      BuildpackUploadMessage.new(params.dup.symbolize_keys, lifecycle)
     end
 
     def nginx_fields
@@ -51,9 +59,18 @@ def is_archive
 
       mime_bits = File.read(bits_path, 4)
 
-      return if mime_bits =~ /^#{VCAP::CloudController::GZIP_MIME}/ || mime_bits =~ /^#{VCAP::CloudController::ZIP_MIME}/
+      if lifecycle == VCAP::CloudController::Lifecycles::BUILDPACK
+        return if mime_bits =~ /^#{VCAP::CloudController::ZIP_MIME}/
+
+        errors.add(:base, "#{bits_name} is not a zip file. Buildpacks of lifecycle \"#{lifecycle}\" must be valid zip files.")
+      elsif lifecycle == VCAP::CloudController::Lifecycles::CNB
+        return if mime_bits =~ /^#{VCAP::CloudController::GZIP_MIME}/
+
+        mime_bits_at_offset = File.read(bits_path, 8, 257)
+        return if mime_bits_at_offset =~ /^#{VCAP::CloudController::CNB_MIME}/
 
-      errors.add(:base, "#{bits_name} is not a zip or gzip archive")
+        errors.add(:base, "#{bits_name} is not a gzip archive or cnb file. Buildpacks of lifecycle \"#{lifecycle}\" must be valid gzip archives or cnb files.")
+      end
     end
 
     def missing_file_path

app/messages/buildpacks_list_message.rb

@@ -21,7 +21,8 @@ class BuildpacksListMessage < MetadataListMessage
 
     def initialize(params={})
       super
-      pagination_options.default_order_by = :position
+      pagination_options.default_order_by = :lifecycle
+      pagination_options.secondary_default_order_by = :position
     end
 
     def self.from_params(params)
@@ -33,7 +34,7 @@ def to_param_hash
     end
 
     def valid_order_by_values
-      super << :position
+      super + %i[position lifecycle]
     end
   end
 

docs/v3/source/includes/api_resources/_buildpacks.erb

@@ -8,6 +8,7 @@
     "filename": null,
     "stack": "windows64",
     "position": 42,
+    "lifecycle": "buildpack",
     "enabled": true,
     "locked": false,
     "metadata": {
@@ -52,6 +53,7 @@
         "filename": null,
         "stack": "my-stack",
         "position": 1,
+        "lifecycle": "cnb",
         "enabled": true,
         "locked": false,
         "metadata": {

docs/v3/source/includes/resources/apps/_update.md.erb

@@ -39,7 +39,7 @@ Content-Type: application/json
 Name | Type | Description
 ---- | ---- | -----------
 **name** | _string_ | Name of the app
-**lifecycle** | [_lifecycle object_](#the-lifecycle-object) | Lifecycle to be used when updating the app; note: `data` is a required field in lifecycle if lifecycle is updated
+**lifecycle** | [_lifecycle object_](#the-lifecycle-object) | Lifecycle to be used when updating the app; note: `data` is a required field in lifecycle if lifecycle is updated. `type` may NOT be changed from its current value.
 **metadata.labels** | [_label object_](#labels) | Labels applied to the app
 **metadata.annotations**  | [_annotation object_](#annotations) | Annotations applied to the app
 

docs/v3/source/includes/resources/buildpacks/_create.md.erb

@@ -44,6 +44,7 @@ Name         | Type      | Description
 -----------  | --------  | -----------------------------------------------------------------------------  | -------
 **stack**    | _string_  | The name of the stack that the buildpack will use                             | null
 **position** | _integer_ | The order in which the buildpacks are checked during buildpack auto-detection | 1
+**lifecycle**| _string_  | The version of buildpack the buildpack will use. `buildpack` indicates [Classic Buildpacks](https://docs.cloudfoundry.org/buildpacks/classic.html). `cnb` indicates [Cloud Native Buildpacks](https://docs.cloudfoundry.org/buildpacks/cnb/) | buildpack
 **enabled**  | _boolean_ | Whether or not the buildpack will be used for staging                          | true
 **locked**   | _boolean_ | Whether or not the buildpack is locked to prevent updating the bits            | false
 **metadata.labels** | [_label object_](#labels) | Labels applied to the buildpack