[PoC] Implement Custom Stacks RFC

FloThinksPi · FloThinksPi · commit 96ea6487ec29 · 2025-10-06T16:07:58.000+02:00
diff --git a/app/controllers/v3/space_manifests_controller.rb b/app/controllers/v3/space_manifests_controller.rb
@@ -127,6 +127,9 @@ def incompatible_with_buildpacks(lifecycle_type, manifest)
   end
 
   def incompatible_with_docker(lifecycle_type, manifest)
+    # Allow docker + buildpack when lifecycle is explicitly set to buildpack (custom stack usage)
+    return false if lifecycle_type == 'buildpack' && manifest.requested?(:lifecycle) && manifest.lifecycle == 'buildpack'
+
     lifecycle_type == 'buildpack' && manifest.docker
   end
 end
diff --git a/app/fetchers/buildpack_lifecycle_fetcher.rb b/app/fetchers/buildpack_lifecycle_fetcher.rb
@@ -4,8 +4,15 @@ module VCAP::CloudController
   class BuildpackLifecycleFetcher
     class << self
       def fetch(buildpack_names, stack_name, lifecycle=Config.config.get(:default_app_lifecycle))
+        # Handle custom stacks (docker:// URLs) - they don't exist in the database
+        if stack_name.is_a?(String) && stack_name.include?('docker://')
+          stack = stack_name
+        else
+          stack = Stack.find(name: stack_name)
+        end
+
         {
-          stack: Stack.find(name: stack_name),
+          stack: stack,
           buildpack_infos: ordered_buildpacks(buildpack_names, stack_name, lifecycle)
         }
       end
diff --git a/app/messages/app_manifest_message.rb b/app/messages/app_manifest_message.rb
@@ -138,9 +138,17 @@ def app_lifecycle_hash
                          Lifecycles::DOCKER
                        end
 
+      # Use docker image as custom stack when lifecycle is explicitly buildpack
+      stack_value = if requested?(:lifecycle) && @lifecycle == 'buildpack' && requested?(:docker) && docker
+                      docker_image = docker[:image] || docker['image']
+                      docker_image ? "docker://#{docker_image}" : @stack
+                    else
+                      @stack
+                    end
+
       data = {
         buildpacks: requested_buildpacks,
-        stack: @stack,
+        stack: stack_value,
         credentials: @cnb_credentials
       }.compact
 
@@ -475,6 +483,9 @@ def validate_cnb_enabled!
     def validate_docker_buildpacks_combination!
       return unless requested?(:docker) && (requested?(:buildpack) || requested?(:buildpacks))
 
+      # Allow docker + buildpacks when lifecycle is explicitly set to buildpack (custom stack usage)
+      return if requested?(:lifecycle) && @lifecycle == 'buildpack'
+
       errors.add(:base, 'Cannot specify both buildpack(s) and docker keys')
     end
 
diff --git a/app/messages/buildpack_lifecycle_data_message.rb b/app/messages/buildpack_lifecycle_data_message.rb
@@ -1,3 +1,5 @@
+require 'uri'
+
 module VCAP::CloudController
   class BuildpackLifecycleDataMessage < BaseMessage
     register_allowed_keys %i[buildpacks stack credentials]
@@ -19,6 +21,7 @@ class BuildpackLifecycleDataMessage < BaseMessage
 
     validate :buildpacks_content
     validate :credentials_content
+    validate :custom_stack_requires_custom_buildpack
 
     def buildpacks_content
       return unless buildpacks.is_a?(Array)
@@ -40,7 +43,34 @@ def buildpacks_content
     def credentials_content
       return unless credentials.is_a?(Hash)
 
-      errors.add(:credentials, 'credentials value must be a hash') if credentials.any? { |_, v| !v.is_a?(Hash) }
+      credentials.each do |registry, creds|
+        unless creds.is_a?(Hash)
+          errors.add(:credentials, "for registry '#{registry}' must be a hash")
+          next
+        end
+
+        has_username = creds.key?('username') || creds.key?(:username)
+        has_password = creds.key?('password') || creds.key?(:password)
+        errors.add(:base, "credentials for #{registry} must include 'username' and 'password'") unless has_username && has_password
+      end
+    end
+
+    def custom_stack_requires_custom_buildpack
+      return unless stack.is_a?(String) && stack.include?('docker://')
+      return unless FeatureFlag.enabled?(:diego_custom_stacks)
+      return unless buildpacks.is_a?(Array)
+
+      buildpacks.each do |buildpack_name|
+        # If buildpack is a URL, it's custom
+        next if buildpack_name&.match?(URI::DEFAULT_PARSER.make_regexp)
+
+        # Check if it's a system buildpack
+        system_buildpack = Buildpack.find(name: buildpack_name)
+        if system_buildpack
+          errors.add(:base, 'Buildpack must be a custom buildpack when using a custom stack')
+          break
+        end
+      end
     end
   end
 end
diff --git a/app/messages/validators.rb b/app/messages/validators.rb
@@ -198,7 +198,7 @@ def validate(record)
       lifecycle_data_message = lifecycle_data_message_class.new(record.lifecycle_data)
       return if lifecycle_data_message.valid?
 
-      lifecycle_data_message.errors.full_messages.each do |message|
+      lifecycle_data_message.errors.each do |attribute, message|
         record.errors.add(:lifecycle, message:)
       end
     end
diff --git a/app/models/runtime/cnb_lifecycle_data_model.rb b/app/models/runtime/cnb_lifecycle_data_model.rb
@@ -68,13 +68,10 @@ def using_custom_buildpack?
     end
 
     def to_hash
-      hash = {
+      {
         buildpacks: buildpacks.map { |buildpack| CloudController::UrlSecretObfuscator.obfuscate(buildpack) },
         stack: stack
       }
-      hash[:credentials] = Presenters::Censorship::REDACTED_CREDENTIAL unless credentials.nil?
-
-      hash
     end
 
     def validate
diff --git a/app/models/runtime/feature_flag.rb b/app/models/runtime/feature_flag.rb
@@ -14,6 +14,7 @@ class UndefinedFeatureFlagError < StandardError
       service_instance_creation: true,
       diego_docker: false,
       diego_cnb: false,
+      diego_custom_stacks: false,
       set_roles_by_username: true,
       unset_roles_by_username: true,
       task_creation: true,
diff --git a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb
@@ -8,6 +8,14 @@ class BuildpackLifecycleDataValidator
 
     validate :buildpacks_are_uri_or_nil
     validate :stack_exists_in_db
+    validate :custom_stack_requires_custom_buildpack
+
+    def custom_stack_requires_custom_buildpack
+      return unless stack.is_a?(String) && stack.include?('docker://')
+      return if buildpack_infos.all?(&:custom?)
+
+      errors.add(:buildpack, 'must be a custom buildpack when using a custom stack')
+    end
 
     def buildpacks_are_uri_or_nil
       buildpack_infos.each do |buildpack_info|
@@ -16,15 +24,34 @@ def buildpacks_are_uri_or_nil
         next if buildpack_info.buildpack_url
 
         if stack
-          errors.add(:buildpack, %("#{buildpack_info.buildpack}" for stack "#{stack.name}" must be an existing admin buildpack or a valid git URI))
+          stack_name = stack.is_a?(String) ? stack : stack.name
+          errors.add(:buildpack, %("#{buildpack_info.buildpack}" for stack "#{stack_name}" must be an existing admin buildpack or a valid git URI))
         else
           errors.add(:buildpack, %("#{buildpack_info.buildpack}" must be an existing admin buildpack or a valid git URI))
         end
       end
     end
 
     def stack_exists_in_db
-      errors.add(:stack, 'must be an existing stack') if stack.nil?
+      # Explicitly check for nil first
+      if stack.nil?
+        errors.add(:stack, 'must be an existing stack')
+        return
+      end
+
+      # Handle custom stacks (docker:// URLs passed as strings)
+      if stack.is_a?(String) && stack.include?('docker://') && FeatureFlag.enabled?(:diego_custom_stacks)
+        return
+      end
+
+      # Handle existing stack objects or string names
+      if stack.is_a?(String)
+        # For string stack names, verify they exist in the database
+        unless VCAP::CloudController::Stack.where(name: stack).any?
+          errors.add(:stack, 'must be an existing stack')
+        end
+      end
+      # If stack is an object (not nil, not string), assume it's valid
     end
   end
 end
diff --git a/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb b/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb
@@ -17,7 +17,13 @@ def initialize(package, staging_message)
     delegate :valid?, :errors, to: :validator
 
     def staging_stack
-      requested_stack || app_stack || VCAP::CloudController::Stack.default.name
+      stack = requested_stack || app_stack || VCAP::CloudController::Stack.default.name
+      FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if stack.is_a?(String) && stack.include?('docker://')
+      stack
+    end
+
+    def credentials
+      staging_message.lifecycle_data[:credentials]
     end
 
     private
diff --git a/lib/cloud_controller/diego/task_recipe_builder.rb b/lib/cloud_controller/diego/task_recipe_builder.rb
@@ -91,12 +91,36 @@ def build_staging_task(config, staging_details)
               "app:#{staging_details.package.app_guid}"
             ]
           ),
-          image_username: staging_details.package.docker_username,
-          image_password: staging_details.package.docker_password,
+          image_username: image_username(staging_details),
+          image_password: image_password(staging_details),
           volume_mounted_files: ServiceBindingFilesBuilder.build(staging_details.package.app)
         }.compact)
       end
 
+      def image_username(staging_details)
+        return staging_details.package.docker_username if staging_details.package.docker_username.present?
+        return unless staging_details.lifecycle.respond_to?(:credentials) && staging_details.lifecycle.credentials.present?
+
+        cred = get_credentials_for_stack(staging_details)
+        cred ? cred['username'] : nil
+      end
+
+      def image_password(staging_details)
+        return staging_details.package.docker_password if staging_details.package.docker_password.present?
+        return unless staging_details.lifecycle.respond_to?(:credentials) && staging_details.lifecycle.credentials.present?
+
+        cred = get_credentials_for_stack(staging_details)
+        cred ? cred['password'] : nil
+      end
+
+      def get_credentials_for_stack(staging_details)
+        return nil unless staging_details.lifecycle.staging_stack.include?('docker://')
+
+        stack_uri = URI.parse(staging_details.lifecycle.staging_stack)
+        host = stack_uri.host
+        staging_details.lifecycle.credentials[host]
+      end
+
       private
 
       def metric_tags(source)
diff --git a/spec/api/documentation/feature_flags_api_spec.rb b/spec/api/documentation/feature_flags_api_spec.rb
@@ -18,7 +18,7 @@
       client.get '/v2/config/feature_flags', {}, headers
 
       expect(status).to eq(200)
-      expect(parsed_response.length).to eq(18)
+      expect(parsed_response.length).to eq(19)
       expect(parsed_response).to include(
         {
           'name' => 'user_org_creation',
diff --git a/spec/unit/actions/app_update_spec.rb b/spec/unit/actions/app_update_spec.rb
@@ -259,6 +259,23 @@ module VCAP::CloudController
           end
         end
 
+        context 'when the lifecycle is valid' do
+          let(:message) do
+            AppUpdateMessage.new({
+                                   lifecycle: {
+                                     type: 'buildpack',
+                                     data: { buildpacks: ['http://new-buildpack.url', 'ruby'], stack: stack.name }
+                                   }
+                                 })
+          end
+
+          it 'does not raise an error' do
+            expect do
+              app_update.update(app_model, message, lifecycle)
+            end.not_to raise_error
+          end
+        end
+
         context 'when changing the lifecycle type' do
           let(:message) do
             AppUpdateMessage.new({
diff --git a/spec/unit/controllers/v3/apps_controller_spec.rb b/spec/unit/controllers/v3/apps_controller_spec.rb
@@ -468,7 +468,7 @@
           {
             name: 'some-name',
             relationships: { space: { data: { guid: space.guid } } },
-            lifecycle: { type: 'cnb', data: { buildpacks: ['http://example.com'], credentials: { registry: { user: 'password' } } } }
+            lifecycle: { type: 'cnb', data: { buildpacks: ['http://example.com'], stack: 'cflinuxfs4', credentials: { 'docker.io' => { 'username' => 'user', 'password' => 'password' } } } }
           }
         end
 
@@ -479,7 +479,7 @@
           lifecycle_data = response_body['lifecycle']['data']
 
           expect(response).to have_http_status :created
-          expect(lifecycle_data).to eq({ 'buildpacks' => ['http://example.com'], 'stack' => 'default-stack-name', 'credentials' => '***' })
+          expect(lifecycle_data).to eq({ 'buildpacks' => ['http://example.com'], 'stack' => 'cflinuxfs4' })
         end
       end
     end
diff --git a/spec/unit/messages/app_manifest_message_spec.rb b/spec/unit/messages/app_manifest_message_spec.rb
@@ -2095,7 +2095,7 @@ module VCAP::CloudController
           end
 
           context 'when cnb_credentials key is specified' do
-            let(:parsed_yaml) { { name: 'cnb', lifecycle: 'cnb', buildpacks: %w[nodejs java], stack: stack.name, cnb_credentials: { registry: { username: 'password' } } } }
+            let(:parsed_yaml) { { name: 'cnb', lifecycle: 'cnb', buildpacks: %w[nodejs java], stack: stack.name, cnb_credentials: { registry: { username: 'user', password: 'password' } } } }
 
             it 'adds credentials to the lifecycle_data' do
               message = AppManifestMessage.create_from_yml(parsed_yaml)
@@ -2104,7 +2104,7 @@ module VCAP::CloudController
               expect(message.app_update_message.lifecycle_type).to eq(Lifecycles::CNB)
               expect(message.app_update_message.buildpack_data.buildpacks).to eq(%w[nodejs java])
               expect(message.app_update_message.buildpack_data.stack).to eq(stack.name)
-              expect(message.app_update_message.buildpack_data.credentials).to eq({ registry: { username: 'password' } })
+              expect(message.app_update_message.buildpack_data.credentials).to eq({ registry: { username: 'user', password: 'password' } })
             end
           end
         end
diff --git a/spec/unit/messages/build_create_message_spec.rb b/spec/unit/messages/build_create_message_spec.rb
@@ -126,6 +126,62 @@ module VCAP::CloudController
               expect(message).not_to be_valid
               expect(message.errors[:lifecycle]).to include('Buildpacks can only contain strings')
             end
+
+            context 'credentials' do
+              it 'is valid with valid credentials' do
+                params[:lifecycle] = {
+                  type: 'buildpack',
+                  data: {
+                    stack: 'cflinuxfs4',
+                    credentials: {
+                      'docker.io' => {
+                        'username' => 'user',
+                        'password' => 'password'
+                      }
+                    }
+                  }
+                }
+                message = BuildCreateMessage.new(params)
+                expect(message).to be_valid
+              end
+
+              it 'is invalid with invalid credentials' do
+                params[:lifecycle] = {
+                  type: 'buildpack',
+                  data: {
+                    stack: 'cflinuxfs4',
+                    credentials: {
+                      'docker.io' => {
+                        'username' => 'user'
+                      }
+                    }
+                  }
+                }
+                message = BuildCreateMessage.new(params)
+                expect(message).not_to be_valid
+                expect(message.errors[:lifecycle]).to include("credentials for docker.io must include 'username' and 'password'")
+              end
+            end
+
+            context 'when a custom stack is used with a system buildpack' do
+              before do
+                VCAP::CloudController::FeatureFlag.make(name: 'diego_custom_stacks', enabled: true)
+                VCAP::CloudController::Buildpack.make(name: 'ruby_buildpack')
+              end
+
+              it 'is not valid' do
+                params[:lifecycle] = {
+                  type: 'buildpack',
+                  data: {
+                    stack: 'docker://cloudfoundry/cflinuxfs4',
+                    buildpacks: ['ruby_buildpack']
+                  }
+                }
+                message = BuildCreateMessage.new(params)
+                expect(message).not_to be_valid
+                expect(message.errors[:lifecycle]).to include('Buildpack must be a custom buildpack when using a custom stack')
+              end
+            end
           end
 
           describe 'docker lifecycle' do
diff --git a/spec/unit/models/runtime/feature_flag_spec.rb b/spec/unit/models/runtime/feature_flag_spec.rb
@@ -168,6 +168,13 @@ module VCAP::CloudController
           end
         end
       end
+
+      context 'when the diego_custom_stacks feature flag is not overridden' do
+        it 'returns the default value' do
+          expect(FeatureFlag.enabled?(:diego_custom_stacks)).to be(false)
+          expect(FeatureFlag.disabled?(:diego_custom_stacks)).to be(true)
+        end
+      end
     end
 
     describe '.raise_unless_enabled!' do
