From 4223d38a9eda4fe555b3f5ae0e8c5977dd00fc45 Mon Sep 17 00:00:00 2001 From: Florian Braun <5863788+FloThinksPi@users.noreply.github.com> Date: Fri, 12 Jun 2026 10:54:06 +0200 Subject: [PATCH 1/5] Implement Custom Stacks RFC: allow docker:// URIs as stacks Feature: diego_custom_stacks (default: disabled) Allow buildpack/CNB apps to use docker:// URIs as their stack, enabling custom root filesystems from container registries. Stack format: docker://host/path:tag (V3 style) Internal conversion: docker://host/path#tag (Diego rootfs format) Changes: - Feature flag: diego_custom_stacks (default false) - UriUtils: is_custom_stack_uri?, custom_stack_registry_host, parse helpers - CustomStackUriConverter: docker:// -> Diego rootfs format - BuildpackLifecycleDataMessage: stack_id field, credentials validation (strict username+password only for custom stack URIs) - BuildpackLifecycleDataModel: encrypted registry credentials storage with deep_stringify_keys normalization - DB migration: add encrypted credentials columns to buildpack_lifecycle_data - BuildpackLifecycleFetcher: skip DB lookup for docker:// stacks - LifecycleBase: memoized staging_stack with feature flag enforcement - AppBaseLifecycle: credentials update with explicit save trigger - AppBuildpackLifecycle/AppBaseLifecycle: persist credentials on create/update - StagingActionBuilder: docker rootfs, default lifecycle bundle fallback, skip buildpack cache upload for custom stacks - LifecycleProtocol: skip buildpack_cache download for custom stacks - TaskRecipeBuilder: atomic credential pair resolution for staging/tasks - AppRecipeBuilder: atomic credential pair resolution for LRPs - Buildpack/CNB DesiredLrpBuilder: docker rootfs with proper requires - Buildpack TaskActionBuilder: custom stack support for tasks - CNB StagingActionBuilder: optional CNB_STACK_ID via resolve_cnb_stack_id - CNB LifecycleData/Protocol: stack_id passthrough - AppsController: raise_unless_custom_stacks_enabled! for start/restart - AppManifestMessage: validate_custom_stacks_enabled! for manifests - BuildCreate: skip stack state validation for custom stacks - V3 API docs: feature_flags and lifecycles documentation Test coverage: - 10 custom stacks spec files (uri_utils, converter, validator, desired_lrp_builder, message, task_action_builder, cnb_staging, feature_flag, lifecycle_data_model, task_recipe_builder) - Updated existing specs for compatibility (validator kwargs, CNB lifecycle_protocol mocks, controller/manifest test fixtures) Co-authored-by: 5863788+FloThinksPi@users.noreply.github.com --- app/actions/build_create.rb | 1 + app/controllers/v3/apps_controller.rb | 8 ++ app/fetchers/buildpack_lifecycle_fetcher.rb | 15 +- app/messages/app_manifest_message.rb | 7 + .../buildpack_lifecycle_data_message.rb | 7 +- app/models/runtime/feature_flag.rb | 1 + .../includes/concepts/_lifecycles.md.erb | 5 +- .../resources/feature_flags/_flags.md.erb | 1 + .../diego/app_recipe_builder.rb | 39 ++++- .../diego/buildpack/desired_lrp_builder.rb | 30 +++- .../diego/buildpack/task_action_builder.rb | 19 ++- .../diego/cnb/desired_lrp_builder.rb | 23 ++- .../diego/cnb/lifecycle_data.rb | 4 +- .../diego/cnb/lifecycle_protocol.rb | 3 + .../diego/cnb/staging_action_builder.rb | 15 +- .../diego/custom_stack_uri_converter.rb | 19 +++ .../diego/lifecycle_protocol.rb | 9 +- .../diego/lifecycles/app_base_lifecycle.rb | 1 + .../buildpack_lifecycle_data_validator.rb | 12 +- .../diego/lifecycles/lifecycle_base.rb | 7 +- .../diego/staging_action_builder.rb | 21 ++- .../diego/task_recipe_builder.rb | 75 +++++++++- lib/utils/uri_utils.rb | 19 +++ .../documentation/feature_flags_api_spec.rb | 2 +- .../controllers/v3/apps_controller_spec.rb | 3 +- .../desired_lrp_builder_custom_stacks_spec.rb | 133 ++++++++++++++++++ .../task_action_builder_custom_stacks_spec.rb | 86 +++++++++++ .../diego/cnb/lifecycle_protocol_spec.rb | 7 +- ...aging_action_builder_custom_stacks_spec.rb | 120 ++++++++++++++++ .../diego/custom_stack_uri_converter_spec.rb | 38 +++++ ...cycle_data_validator_custom_stacks_spec.rb | 91 ++++++++++++ .../task_recipe_builder_custom_stacks_spec.rb | 133 ++++++++++++++++++ .../lib/utils/uri_utils_custom_stack_spec.rb | 40 ++++++ .../messages/app_manifest_message_spec.rb | 4 +- ...fecycle_data_message_custom_stacks_spec.rb | 40 ++++++ .../feature_flag_custom_stacks_spec.rb | 38 +++++ 36 files changed, 1030 insertions(+), 46 deletions(-) create mode 100644 lib/cloud_controller/diego/custom_stack_uri_converter.rb create mode 100644 spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb create mode 100644 spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb create mode 100644 spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb create mode 100644 spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb create mode 100644 spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_custom_stacks_spec.rb create mode 100644 spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb create mode 100644 spec/unit/lib/utils/uri_utils_custom_stack_spec.rb create mode 100644 spec/unit/messages/buildpack_lifecycle_data_message_custom_stacks_spec.rb create mode 100644 spec/unit/models/runtime/feature_flag_custom_stacks_spec.rb diff --git a/app/actions/build_create.rb b/app/actions/build_create.rb index f43ab50b572..8e9f12095bf 100644 --- a/app/actions/build_create.rb +++ b/app/actions/build_create.rb @@ -184,6 +184,7 @@ def staging_in_progress! def validate_stack_state!(lifecycle, app) return [] if lifecycle.type == Lifecycles::DOCKER + return [] if UriUtils.is_custom_stack_uri?(lifecycle.staging_stack) stack = Stack.find(name: lifecycle.staging_stack) return [] unless stack diff --git a/app/controllers/v3/apps_controller.rb b/app/controllers/v3/apps_controller.rb index 7704f7f2f3f..b49c06e40d3 100644 --- a/app/controllers/v3/apps_controller.rb +++ b/app/controllers/v3/apps_controller.rb @@ -98,6 +98,7 @@ def create FeatureFlag.raise_unless_enabled!(:diego_docker) if message.lifecycle_type == VCAP::CloudController::PackageModel::DOCKER_TYPE lifecycle = AppLifecycleProvider.provide_for_create(message) FeatureFlag.raise_unless_enabled!(:diego_cnb) if lifecycle.type == VCAP::CloudController::Lifecycles::CNB + raise_unless_custom_stacks_enabled!(message) unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid? app_creator = AppCreate.new(user_audit_info) @@ -188,6 +189,7 @@ def start FeatureFlag.raise_unless_enabled!(:diego_docker) if app.lifecycle_type == DockerLifecycleDataModel::LIFECYCLE_TYPE FeatureFlag.raise_unless_enabled!(:diego_cnb) if app.lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE + FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if app.lifecycle_data.respond_to?(:stack) && UriUtils.is_custom_stack_uri?(app.lifecycle_data.stack) AppStart.start(app:, user_audit_info:) TelemetryLogger.v3_emit( @@ -231,6 +233,7 @@ def restart FeatureFlag.raise_unless_enabled!(:diego_docker) if app.lifecycle_type == DockerLifecycleDataModel::LIFECYCLE_TYPE FeatureFlag.raise_unless_enabled!(:diego_cnb) if app.lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE + FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if app.lifecycle_data.respond_to?(:stack) && UriUtils.is_custom_stack_uri?(app.lifecycle_data.stack) AppRestart.restart(app: app, config: Config.config, user_audit_info: user_audit_info) TelemetryLogger.v3_emit( @@ -397,6 +400,11 @@ def translate_error(e) private + def raise_unless_custom_stacks_enabled!(message) + stack = message.lifecycle_data&.dig(:stack) + FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if stack && UriUtils.is_custom_stack_uri?(stack) + end + def async_recursive_delete_enabled? !!VCAP::CloudController::Config.config.get(:temporary_enable_async_recursive_delete, :apps) end diff --git a/app/fetchers/buildpack_lifecycle_fetcher.rb b/app/fetchers/buildpack_lifecycle_fetcher.rb index f5d399a64e2..17e08715d5f 100644 --- a/app/fetchers/buildpack_lifecycle_fetcher.rb +++ b/app/fetchers/buildpack_lifecycle_fetcher.rb @@ -4,10 +4,17 @@ module VCAP::CloudController class BuildpackLifecycleFetcher class << self 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, lifecycle) - } + if UriUtils.is_custom_stack_uri?(stack_name) + { + stack: nil, + buildpack_infos: buildpack_names.map { |name| BuildpackInfo.new(name, nil) } + } + else + { + stack: Stack.find(name: stack_name), + buildpack_infos: ordered_buildpacks(buildpack_names, stack_name, lifecycle) + } + end end private diff --git a/app/messages/app_manifest_message.rb b/app/messages/app_manifest_message.rb index 0f44fc387a8..da1da6c911f 100644 --- a/app/messages/app_manifest_message.rb +++ b/app/messages/app_manifest_message.rb @@ -75,6 +75,7 @@ def self.underscore_keys(hash) validate :validate_buildpack_and_buildpacks_combination! validate :validate_docker_enabled! validate :validate_cnb_enabled! + validate :validate_custom_stacks_enabled! validate :validate_docker_buildpacks_combination! validate :validate_credentials_combination! validate :validate_service_bindings_message!, if: ->(record) { record.requested?(:services) } @@ -488,6 +489,12 @@ def validate_cnb_enabled! errors.add(:base, e.message) end + def validate_custom_stacks_enabled! + FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if requested?(:stack) && UriUtils.is_custom_stack_uri?(@stack) + rescue StandardError => e + errors.add(:base, e.message) + end + def validate_docker_buildpacks_combination! return unless requested?(:docker) && (requested?(:buildpack) || requested?(:buildpacks)) diff --git a/app/messages/buildpack_lifecycle_data_message.rb b/app/messages/buildpack_lifecycle_data_message.rb index 66bf5ed234a..5fdf4ab7767 100644 --- a/app/messages/buildpack_lifecycle_data_message.rb +++ b/app/messages/buildpack_lifecycle_data_message.rb @@ -1,6 +1,6 @@ module VCAP::CloudController class BuildpackLifecycleDataMessage < BaseMessage - register_allowed_keys %i[buildpacks stack credentials] + register_allowed_keys %i[buildpacks stack stack_id credentials] validates_with NoAdditionalKeysValidator @@ -9,6 +9,11 @@ class BuildpackLifecycleDataMessage < BaseMessage allow_nil: true, length: { in: 1..4096, message: 'must be between 1 and 4096 characters' } + validates :stack_id, + string: true, + allow_nil: true, + length: { in: 1..4096, message: 'must be between 1 and 4096 characters' } + validates :buildpacks, array: true, allow_nil: true diff --git a/app/models/runtime/feature_flag.rb b/app/models/runtime/feature_flag.rb index e64b7d60e7b..a7039c16552 100644 --- 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/docs/v3/source/includes/concepts/_lifecycles.md.erb b/docs/v3/source/includes/concepts/_lifecycles.md.erb index 707a859957a..979dec0649c 100644 --- a/docs/v3/source/includes/concepts/_lifecycles.md.erb +++ b/docs/v3/source/includes/concepts/_lifecycles.md.erb @@ -40,7 +40,7 @@ Name | Type | Description ---- | ---- | ----------- **type** | _string_ | `buildpack` **data.buildpacks** | _list of strings_ | A list of the names of buildpacks, URLs from which they may be downloaded, or `null` to auto-detect a suitable buildpack during staging -**data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4` +**data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4`. Can also be a container image URI with `docker://` scheme (e.g. `docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0`) when the `diego_custom_stacks` feature flag is enabled. When using a custom stack, all buildpacks must be custom (URL-based). **data.credentials** | _object_ | Credentials used to download custom buildpacks from private git repositories, keyed by host. Each entry is a username/password pair. If any buildpack URL embeds credentials, **data.credentials** must be empty; provide credentials in one place only. For records whose historical lifecycle data is no longer available, @@ -85,7 +85,8 @@ Name | Type | Description **type** | _string_ | `cnb` **data.buildpacks** | _list of strings_ | A list of URLs with either `docker://` or `http(s)://` scheme, pointing to the Cloud Native Buildpack OCI image.
When the scheme is `http(s)://`, an OCI tarball is expected to be present at the specified location. **data.credentials** | _object_ | Credentials used to download the configured buildpacks, keyed by registry host. Each entry contains either a username/password pair or a token. CNB buildpack URLs must not embed credentials; use **data.credentials** instead. -**data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4` +**data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4`. Can also be a container image URI with `docker://` scheme when the `diego_custom_stacks` feature flag is enabled. +**data.stack_id** | _string_ | _(optional)_ Explicit CNB stack identifier (e.g. `io.buildpacks.stacks.jammy`). When omitted with a custom stack, the CNB builder will auto-detect the stack from the run image metadata. For records whose historical lifecycle data is no longer available, **data.buildpacks** may be an empty list and **data.stack** may be `null`. diff --git a/docs/v3/source/includes/resources/feature_flags/_flags.md.erb b/docs/v3/source/includes/resources/feature_flags/_flags.md.erb index 37faf69cb63..524740be7c2 100644 --- a/docs/v3/source/includes/resources/feature_flags/_flags.md.erb +++ b/docs/v3/source/includes/resources/feature_flags/_flags.md.erb @@ -4,6 +4,7 @@ ["app_scaling", "true", "When enabled, space developers can perform scaling operations (i.e. change memory, disk, log rate, or instances). When disabled, only admins can perform scaling operations."], ["diego_docker", "false", "When enabled, Docker applications are supported by Diego. When disabled, Docker applications will stop running. It will still be possible to stop and delete them and update their configurations."], ["diego_cnb (experimental)", "false", "When enabled, the staging process using Cloud Native Buildpacks is supported."], +["diego_custom_stacks", "false", "When enabled, users can specify a container image URI (docker://...) as the stack for buildpack and CNB lifecycle apps. This allows users to bring their own stacks independent of the platform-provided stacks. When disabled, only platform-provided stacks registered in the stacks table can be used."], ["env_var_visibility", "true", "When enabled, all users can see their environment variables. When disabled, no users can see environment variables."], ["hide_marketplace_from_unauthenticated_users", "false", "When enabled, service offerings available in the marketplace will be hidden from unauthenticated users. When disabled, unauthenticated users will be able to see the service offerings available in the marketplace."], ["private_domain_creation", "true", "When enabled, an organization manager can create private domains for that organization. When disabled, only admin users can create private domains."], diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index 25a4c10866f..671777af9f4 100644 --- a/lib/cloud_controller/diego/app_recipe_builder.rb +++ b/lib/cloud_controller/diego/app_recipe_builder.rb @@ -4,6 +4,7 @@ require 'cloud_controller/diego/buildpack/desired_lrp_builder' require 'cloud_controller/diego/docker/desired_lrp_builder' require 'cloud_controller/diego/cnb/desired_lrp_builder' +require 'cloud_controller/diego/custom_stack_uri_converter' require 'cloud_controller/diego/process_guid' require 'cloud_controller/diego/ssh_key' require 'cloud_controller/diego/service_binding_files_builder' @@ -101,14 +102,48 @@ def app_lrp_arguments certificate_properties: ::Diego::Bbs::Models::CertificateProperties.new( organizational_unit: ["organization:#{process.organization.guid}", "space:#{process.space.guid}", "app:#{process.app_guid}"] ), - image_username: process.desired_droplet.docker_receipt_username, - image_password: process.desired_droplet.docker_receipt_password, + image_username: lrp_image_username(process), + image_password: lrp_image_password(process), volume_mounted_files: ServiceBindingFilesBuilder.build(process) }.compact rescue ServiceBindingFilesBuilder::IncompatibleBindings => e raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', "Cannot build service binding files for app - #{e.message}") end + def lrp_image_username(process) + username, password = docker_receipt_credentials(process) + return username if username.present? && password.present? + + custom_stack_lrp_credential(process, 'username') + end + + def lrp_image_password(process) + username, password = docker_receipt_credentials(process) + return password if username.present? && password.present? + + custom_stack_lrp_credential(process, 'password') + end + + def docker_receipt_credentials(process) + droplet = process.desired_droplet + [droplet.docker_receipt_username, droplet.docker_receipt_password] + end + + def custom_stack_lrp_credential(process, field) + lifecycle_data = process.app.lifecycle_data + return nil unless lifecycle_data.respond_to?(:credentials) && lifecycle_data.respond_to?(:stack) + + stack = lifecycle_data.stack + stack_host = UriUtils.custom_stack_registry_host(stack) + return nil unless stack_host + + credentials = lifecycle_data.credentials + return nil unless credentials.is_a?(Hash) + + cred = credentials[stack_host] + cred&.fetch(field, nil) + end + def metric_tags(process) tags = { 'source_id' => METRIC_TAG_VALUE.new(static: process.app_guid), diff --git a/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb b/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb index b1337fc5f32..b99266ce12d 100644 --- a/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb @@ -1,3 +1,6 @@ +require 'cloud_controller/diego/custom_stack_uri_converter' +require 'utils/uri_utils' + module VCAP::CloudController module Diego module Buildpack @@ -25,7 +28,7 @@ def initialize(config, opts) def cached_dependencies return nil if @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"buildpack/#{@stack}" + lifecycle_bundle_key = resolved_lifecycle_bundle_key lifecycle_bundle = @config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key] raise InvalidStack.new("no compiler defined for requested stack '#{@stack}'") unless lifecycle_bundle @@ -33,12 +36,14 @@ def cached_dependencies ::Diego::Bbs::Models::CachedDependency.new( from: LifecycleBundleUriGenerator.uri(lifecycle_bundle), to: '/tmp/lifecycle', - cache_key: "buildpack-#{@stack}-lifecycle" + cache_key: "buildpack-#{resolved_stack_name}-lifecycle" ) ] end def root_fs + return CustomStackUriConverter.new.convert(@stack) if UriUtils.is_custom_stack_uri?(@stack) + @stack_obj ||= Stack.find(name: @stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', @stack) unless @stack_obj @@ -64,16 +69,16 @@ def setup def image_layers return [] unless @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"buildpack/#{@stack}" + lifecycle_bundle_key = resolved_lifecycle_bundle_key lifecycle_bundle = @config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key] raise InvalidStack.new("no compiler defined for requested stack '#{@stack}'") unless lifecycle_bundle - destination = @config.get(:diego, :droplet_destinations)[@stack.to_sym] + destination = @config.get(:diego, :droplet_destinations)[resolved_stack_name.to_sym] raise InvalidStack.new("no droplet destination defined for requested stack '#{@stack}'") unless destination layers = [ ::Diego::Bbs::Models::ImageLayer.new( - name: "buildpack-#{@stack}-lifecycle", + name: "buildpack-#{resolved_stack_name}-lifecycle", url: LifecycleBundleUriGenerator.uri(lifecycle_bundle), destination_path: '/tmp/lifecycle', layer_type: ::Diego::Bbs::Models::ImageLayer::Type::SHARED, @@ -117,6 +122,21 @@ def port_environment_variables def privileged? @config.get(:diego, :use_privileged_containers_for_running) end + + private + + # For custom stacks, fall back to the default stack for lifecycle bundle and droplet destination lookups + def resolved_stack_name + if UriUtils.is_custom_stack_uri?(@stack) + VCAP::CloudController::Stack.default.name + else + @stack + end + end + + def resolved_lifecycle_bundle_key + :"buildpack/#{resolved_stack_name}" + end end end end diff --git a/lib/cloud_controller/diego/buildpack/task_action_builder.rb b/lib/cloud_controller/diego/buildpack/task_action_builder.rb index 1331b838543..49194085fd4 100644 --- a/lib/cloud_controller/diego/buildpack/task_action_builder.rb +++ b/lib/cloud_controller/diego/buildpack/task_action_builder.rb @@ -1,5 +1,6 @@ require 'diego/action_builder' require 'cloud_controller/diego/task_environment_variable_collector' +require 'cloud_controller/diego/custom_stack_uri_converter' require 'credhub/config_helpers' module VCAP::CloudController @@ -58,12 +59,12 @@ def action def image_layers return [] unless @config.get(:diego, :enable_declarative_asset_downloads) - destination = @config.get(:diego, :droplet_destinations)[lifecycle_stack.to_sym] + destination = @config.get(:diego, :droplet_destinations)[resolved_stack_name.to_sym] raise InvalidStack.new("no droplet destination defined for requested stack '#{lifecycle_stack}'") unless destination layers = [ ::Diego::Bbs::Models::ImageLayer.new( - name: "buildpack-#{lifecycle_stack}-lifecycle", + name: "buildpack-#{resolved_stack_name}-lifecycle", url: LifecycleBundleUriGenerator.uri(config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key]), destination_path: '/tmp/lifecycle', layer_type: ::Diego::Bbs::Models::ImageLayer::Type::SHARED, @@ -91,6 +92,8 @@ def task_environment_variables end def stack + return CustomStackUriConverter.new.convert(lifecycle_stack) if UriUtils.is_custom_stack_uri?(lifecycle_stack) + @stack ||= Stack.find(name: lifecycle_stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', lifecycle_stack) unless @stack @@ -103,12 +106,12 @@ def cached_dependencies [::Diego::Bbs::Models::CachedDependency.new( from: LifecycleBundleUriGenerator.uri(config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key]), to: '/tmp/lifecycle', - cache_key: "#{@prefix}-#{lifecycle_stack}-lifecycle" + cache_key: "#{@prefix}-#{resolved_stack_name}-lifecycle" )] end def lifecycle_bundle_key - :"#{@prefix}/#{lifecycle_stack}" + :"#{@prefix}/#{resolved_stack_name}" end private @@ -118,6 +121,14 @@ def lifecycle_bundle_key def lifecycle_stack lifecycle_data[:stack] end + + def resolved_stack_name + if UriUtils.is_custom_stack_uri?(lifecycle_stack) + VCAP::CloudController::Stack.default.name + else + lifecycle_stack + end + end end end end diff --git a/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb b/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb index 1c81ce430ce..92f1dbe5200 100644 --- a/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb @@ -1,3 +1,6 @@ +require 'cloud_controller/diego/custom_stack_uri_converter' +require 'utils/uri_utils' + module VCAP::CloudController module Diego module CNB @@ -25,7 +28,7 @@ def initialize(config, opts) def cached_dependencies return nil if @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"cnb/#{@stack}" + lifecycle_bundle_key = :"cnb/#{resolved_stack_name}" lifecycle_bundle = @config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key] raise InvalidStack.new("no compiler defined for requested stack '#{@stack}'") unless lifecycle_bundle @@ -33,12 +36,14 @@ def cached_dependencies ::Diego::Bbs::Models::CachedDependency.new( from: LifecycleBundleUriGenerator.uri(lifecycle_bundle), to: '/tmp/lifecycle', - cache_key: "cnb-#{@stack}-lifecycle" + cache_key: "cnb-#{resolved_stack_name}-lifecycle" ) ] end def root_fs + return CustomStackUriConverter.new.convert(@stack) if UriUtils.is_custom_stack_uri?(@stack) + @stack_obj ||= Stack.find(name: @stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', @stack) unless @stack_obj @@ -64,16 +69,16 @@ def setup def image_layers return [] unless @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"cnb/#{@stack}" + lifecycle_bundle_key = :"cnb/#{resolved_stack_name}" lifecycle_bundle = @config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key] raise InvalidStack.new("no compiler defined for requested stack '#{@stack}'") unless lifecycle_bundle - destination = @config.get(:diego, :droplet_destinations)[@stack.to_sym] + destination = @config.get(:diego, :droplet_destinations)[resolved_stack_name.to_sym] raise InvalidStack.new("no droplet destination defined for requested stack '#{@stack}'") unless destination layers = [ ::Diego::Bbs::Models::ImageLayer.new( - name: "cnb-#{@stack}-lifecycle", + name: "cnb-#{resolved_stack_name}-lifecycle", url: LifecycleBundleUriGenerator.uri(lifecycle_bundle), destination_path: '/tmp/lifecycle', layer_type: ::Diego::Bbs::Models::ImageLayer::Type::SHARED, @@ -118,6 +123,14 @@ def privileged? private + def resolved_stack_name + if UriUtils.is_custom_stack_uri?(@stack) + VCAP::CloudController::Stack.default.name + else + @stack + end + end + def default_container_env [ ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'LANG', value: DEFAULT_LANG), diff --git a/lib/cloud_controller/diego/cnb/lifecycle_data.rb b/lib/cloud_controller/diego/cnb/lifecycle_data.rb index 63ff1fd642c..ac399bbd80b 100644 --- a/lib/cloud_controller/diego/cnb/lifecycle_data.rb +++ b/lib/cloud_controller/diego/cnb/lifecycle_data.rb @@ -4,7 +4,7 @@ module CNB class LifecycleData attr_accessor :app_bits_download_uri, :build_artifacts_cache_download_uri, :build_artifacts_cache_upload_uri, :buildpacks, :app_bits_checksum, :droplet_upload_uri, :stack, :buildpack_cache_checksum, - :credentials, :auto_detect + :credentials, :auto_detect, :stack_id def message message = { @@ -19,6 +19,7 @@ def message message[:build_artifacts_cache_download_uri] = build_artifacts_cache_download_uri if build_artifacts_cache_download_uri message[:buildpack_cache_checksum] = buildpack_cache_checksum if buildpack_cache_checksum message[:credentials] = credentials if credentials + message[:stack_id] = stack_id if stack_id schema.validate(message) message @@ -38,6 +39,7 @@ def schema stack: String, app_bits_checksum: Hash, optional(:credentials) => String, + optional(:stack_id) => String, auto_detect: bool } end diff --git a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb index dbe75fa2714..6475043570d 100644 --- a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb @@ -26,6 +26,9 @@ def new_lifecycle_data(staging_details) lifecycle_data = LifecycleData.new lifecycle_data.credentials = staging_details.lifecycle.credentials lifecycle_data.auto_detect = staging_details.lifecycle.buildpack_infos.empty? + if staging_details.lifecycle.staging_message.buildpack_data.requested?(:stack_id) + lifecycle_data.stack_id = staging_details.lifecycle.staging_message.buildpack_data.stack_id + end lifecycle_data end diff --git a/lib/cloud_controller/diego/cnb/staging_action_builder.rb b/lib/cloud_controller/diego/cnb/staging_action_builder.rb index 34d750d04ea..32af682f436 100644 --- a/lib/cloud_controller/diego/cnb/staging_action_builder.rb +++ b/lib/cloud_controller/diego/cnb/staging_action_builder.rb @@ -15,15 +15,28 @@ def task_environment_variables env = [ ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_USER_ID', value: '2000'), ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_GROUP_ID', value: '2000'), - ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_STACK_ID', value: lifecycle_stack), ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'LANG', value: STAGING_DEFAULT_LANG) ] + # For custom stacks, CNB_STACK_ID is optional: if a stack_id was provided + # in lifecycle data, use it; otherwise omit it and let the CNB builder auto-detect. + # For system stacks, always pass the stack name as CNB_STACK_ID. + cnb_stack_id = resolve_cnb_stack_id + env.push(::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_STACK_ID', value: cnb_stack_id)) if cnb_stack_id env.push(::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_REGISTRY_CREDS', value: lifecycle_data[:credentials])) if lifecycle_data[:credentials] env end private + def resolve_cnb_stack_id + if UriUtils.is_custom_stack_uri?(lifecycle_stack) + # Use explicitly provided stack_id from lifecycle_data if present, otherwise omit (auto-detect) + lifecycle_data[:stack_id] + else + lifecycle_stack + end + end + def stage_action args = [ '--cache-dir', '/tmp/cache', diff --git a/lib/cloud_controller/diego/custom_stack_uri_converter.rb b/lib/cloud_controller/diego/custom_stack_uri_converter.rb new file mode 100644 index 00000000000..0ad5fde77a3 --- /dev/null +++ b/lib/cloud_controller/diego/custom_stack_uri_converter.rb @@ -0,0 +1,19 @@ +require 'utils/uri_utils' + +module VCAP::CloudController + module Diego + class CustomStackUriConverter + # Converts a custom stack URI from the user-facing format (docker://host/path:tag) + # to the Diego BBS rootfs format (docker://host/path#tag). + # This reuses the same conversion logic that Docker lifecycle uses. + def convert(custom_stack_uri) + # Strip the docker:// scheme prefix to get the raw image reference + raw_image_ref = custom_stack_uri.sub(%r{\Adocker://}, '') + + host, path, tag_digest = UriUtils.parse_docker_uri(raw_image_ref) + + Addressable::URI.new(scheme: 'docker', host: host, path: path, fragment: tag_digest).to_s + end + end + end +end diff --git a/lib/cloud_controller/diego/lifecycle_protocol.rb b/lib/cloud_controller/diego/lifecycle_protocol.rb index de76fbe3db8..f3644413a8f 100644 --- a/lib/cloud_controller/diego/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/lifecycle_protocol.rb @@ -30,9 +30,6 @@ def lifecycle_data(staging_details) lifecycle_data = new_lifecycle_data(staging_details) lifecycle_data.app_bits_download_uri = @blobstore_url_generator.package_download_url(staging_details.package) lifecycle_data.app_bits_checksum = staging_details.package.checksum_info - lifecycle_data.buildpack_cache_checksum = staging_details.package.app.buildpack_cache_sha256_checksum - lifecycle_data.build_artifacts_cache_download_uri = @blobstore_url_generator.buildpack_cache_download_url(staging_details.package.app_guid, stack) - lifecycle_data.build_artifacts_cache_upload_uri = @blobstore_url_generator.buildpack_cache_upload_url(staging_details.package.app_guid, stack) lifecycle_data.droplet_upload_uri = @blobstore_url_generator.droplet_upload_url(staging_details.staging_guid) lifecycle_data.buildpacks = @buildpack_entry_generator.buildpack_entries( staging_details.lifecycle.buildpack_infos, @@ -41,6 +38,12 @@ def lifecycle_data(staging_details) ) lifecycle_data.stack = stack + unless UriUtils.is_custom_stack_uri?(stack) + lifecycle_data.buildpack_cache_checksum = staging_details.package.app.buildpack_cache_sha256_checksum + lifecycle_data.build_artifacts_cache_download_uri = @blobstore_url_generator.buildpack_cache_download_url(staging_details.package.app_guid, stack) + lifecycle_data.build_artifacts_cache_upload_uri = @blobstore_url_generator.buildpack_cache_upload_url(staging_details.package.app_guid, stack) + end + lifecycle_data.message rescue Membrane::SchemaValidationError => e raise e unless e.message.match?(/app_bits_download_uri/) diff --git a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb index e94fd6308b4..f0d541937d6 100644 --- a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb @@ -13,6 +13,7 @@ def initialize(message, app=nil) @validator = validator_class.new( buildpack_infos: db_result[:buildpack_infos], stack: db_result[:stack], + stack_name: stack, credentials: effective_credentials(app), credentials_from_message: message.buildpack_data.requested?(:credentials) ) diff --git a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb index 7196895458c..373388c68b4 100644 --- a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb +++ b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb @@ -4,10 +4,11 @@ module VCAP::CloudController class BuildpackLifecycleDataValidator include ActiveModel::Model - attr_accessor :buildpack_infos, :stack, :credentials, :credentials_from_message + attr_accessor :buildpack_infos, :stack, :stack_name, :credentials, :credentials_from_message validate :buildpacks_are_uri_or_nil validate :stack_exists_in_db + validate :custom_stack_requires_custom_buildpack validate :no_dual_credential_sources def buildpacks_are_uri_or_nil @@ -25,9 +26,18 @@ def buildpacks_are_uri_or_nil end def stack_exists_in_db + return if UriUtils.is_custom_stack_uri?(stack_name) && FeatureFlag.enabled?(:diego_custom_stacks) + errors.add(:stack, 'must be an existing stack') if stack.nil? end + def custom_stack_requires_custom_buildpack + return unless UriUtils.is_custom_stack_uri?(stack_name) + return if buildpack_infos.all? { |info| info.buildpack_url.present? } + + errors.add(:buildpack, 'must be a custom buildpack (URL) when using a custom stack') + end + def no_dual_credential_sources return unless url_has_userinfo? && credentials.present? diff --git a/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb b/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb index 1e78212858a..15b8bdd25dc 100644 --- a/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb +++ b/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb @@ -18,6 +18,7 @@ def initialize(package, staging_message) @validator = validator_class.new( buildpack_infos: @buildpack_infos, stack: db_result[:stack], + stack_name: staging_stack, credentials: @credentials_for_staging, credentials_from_message: staging_message.buildpack_data.requested?(:credentials) ) @@ -26,7 +27,11 @@ def initialize(package, staging_message) delegate :valid?, :errors, to: :validator def staging_stack - requested_stack || app_stack || VCAP::CloudController::Stack.default.name + @staging_stack ||= begin + stack = requested_stack || app_stack || VCAP::CloudController::Stack.default.name + FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if UriUtils.is_custom_stack_uri?(stack) + stack + end end private diff --git a/lib/cloud_controller/diego/staging_action_builder.rb b/lib/cloud_controller/diego/staging_action_builder.rb index 3cca3aca34b..46c207da94c 100644 --- a/lib/cloud_controller/diego/staging_action_builder.rb +++ b/lib/cloud_controller/diego/staging_action_builder.rb @@ -1,6 +1,7 @@ require 'credhub/config_helpers' require 'diego/action_builder' require 'digest/xxhash' +require 'cloud_controller/diego/custom_stack_uri_converter' module VCAP::CloudController module Diego @@ -130,6 +131,8 @@ def image_layers end def stack + return CustomStackUriConverter.new.convert(lifecycle_stack) if UriUtils.is_custom_stack_uri?(lifecycle_stack) + @stack ||= Stack.find(name: lifecycle_stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', lifecycle_stack) unless @stack @@ -172,21 +175,25 @@ def download_actions end def upload_actions - [ + actions = [ ::Diego::Bbs::Models::UploadAction.new( user: 'vcap', artifact: 'droplet', from: '/tmp/droplet', to: upload_droplet_uri.to_s - ), + ) + ] - ::Diego::Bbs::Models::UploadAction.new( + unless UriUtils.is_custom_stack_uri?(lifecycle_data[:stack]) + actions << ::Diego::Bbs::Models::UploadAction.new( user: 'vcap', artifact: 'build artifacts cache', from: @cache_source, to: upload_buildpack_artifacts_cache_uri.to_s ) - ] + end + + actions end def skip_detect? @@ -194,7 +201,11 @@ def skip_detect? end def lifecycle_bundle_key - :"#{@prefix}/#{lifecycle_data[:stack]}" + if UriUtils.is_custom_stack_uri?(lifecycle_data[:stack]) + :"#{@prefix}/#{VCAP::CloudController::Stack.default.name}" + else + :"#{@prefix}/#{lifecycle_data[:stack]}" + end end def upload_buildpack_artifacts_cache_uri diff --git a/lib/cloud_controller/diego/task_recipe_builder.rb b/lib/cloud_controller/diego/task_recipe_builder.rb index f1a4962095d..cc60bc1692e 100644 --- a/lib/cloud_controller/diego/task_recipe_builder.rb +++ b/lib/cloud_controller/diego/task_recipe_builder.rb @@ -52,8 +52,8 @@ def build_app_task(config, task) "app:#{task.app_guid}" ] ), - image_username: task.droplet.docker_receipt_username, - image_password: task.droplet.docker_receipt_password, + image_username: task_image_username(task), + image_password: task_image_password(task), volume_mounted_files: ServiceBindingFilesBuilder.build(task.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -93,8 +93,8 @@ 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: staging_image_username(staging_details), + image_password: staging_image_password(staging_details), volume_mounted_files: ServiceBindingFilesBuilder.build(staging_details.package.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -103,6 +103,73 @@ def build_staging_task(config, staging_details) private + def task_image_username(task) + username, password = task_docker_receipt_credentials(task) + return username if username.present? && password.present? + + custom_stack_task_credential(task, 'username') + end + + def task_image_password(task) + username, password = task_docker_receipt_credentials(task) + return password if username.present? && password.present? + + custom_stack_task_credential(task, 'password') + end + + def task_docker_receipt_credentials(task) + droplet = task.droplet + [droplet.docker_receipt_username, droplet.docker_receipt_password] + end + + def custom_stack_task_credential(task, field) + lifecycle_data = task.app.lifecycle_data + return nil unless lifecycle_data.respond_to?(:credentials) && lifecycle_data.respond_to?(:stack) + + stack = lifecycle_data.stack + stack_host = UriUtils.custom_stack_registry_host(stack) + return nil unless stack_host + + credentials = lifecycle_data.credentials + return nil unless credentials.is_a?(Hash) + + cred = credentials[stack_host] + cred&.fetch(field, nil) + end + + def staging_image_username(staging_details) + username, password = staging_docker_credentials(staging_details) + return username if username.present? && password.present? + + custom_stack_credential(staging_details, 'username') + end + + def staging_image_password(staging_details) + username, password = staging_docker_credentials(staging_details) + return password if username.present? && password.present? + + custom_stack_credential(staging_details, 'password') + end + + def staging_docker_credentials(staging_details) + [staging_details.package.docker_username, staging_details.package.docker_password] + end + + def custom_stack_credential(staging_details, field) + lifecycle = staging_details.lifecycle + return nil unless lifecycle.respond_to?(:credentials_for_staging) && lifecycle.respond_to?(:staging_stack) + + stack = lifecycle.staging_stack + stack_host = UriUtils.custom_stack_registry_host(stack) + return nil unless stack_host + + credentials = lifecycle.credentials_for_staging + return nil unless credentials.is_a?(Hash) + + cred = credentials[stack_host] + cred&.fetch(field, nil) + end + def metric_tags(source) { 'source_id' => METRIC_TAG_VALUE.new(static: source.app_guid), diff --git a/lib/utils/uri_utils.rb b/lib/utils/uri_utils.rb index eb43d3a80e7..8318bb6e38f 100644 --- a/lib/utils/uri_utils.rb +++ b/lib/utils/uri_utils.rb @@ -44,6 +44,25 @@ def url_has_userinfo?(url) false end + def is_custom_stack_uri?(candidate) + return false unless candidate.is_a?(String) + return false unless candidate.start_with?('docker://') + + host, = parse_docker_uri(candidate.delete_prefix('docker://')) + host.present? + rescue StandardError + false + end + + def custom_stack_registry_host(stack_uri) + return nil unless is_custom_stack_uri?(stack_uri) + + host, = parse_docker_uri(stack_uri.delete_prefix('docker://')) + host.presence + rescue StandardError + nil + end + def is_uri_path?(candidate) !!(candidate.is_a?(String) && candidate =~ %r{^(?:/|/([^\s/]\S*)?)$}) end diff --git a/spec/api/documentation/feature_flags_api_spec.rb b/spec/api/documentation/feature_flags_api_spec.rb index 5fe6740eb30..77e54165f99 100644 --- 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(19) + expect(parsed_response.length).to eq(20) expect(parsed_response).to include( { 'name' => 'user_org_creation', diff --git a/spec/unit/controllers/v3/apps_controller_spec.rb b/spec/unit/controllers/v3/apps_controller_spec.rb index 7d7d7f281f4..a3164f97faa 100644 --- a/spec/unit/controllers/v3/apps_controller_spec.rb +++ b/spec/unit/controllers/v3/apps_controller_spec.rb @@ -468,7 +468,8 @@ { 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: 'default-stack-name', + credentials: { 'registry.example.com' => { 'username' => 'user', 'password' => 'pass' } } } } } end diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb new file mode 100644 index 00000000000..d6bb894e50e --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb @@ -0,0 +1,133 @@ +require 'spec_helper' +require 'cloud_controller/diego/buildpack/desired_lrp_builder' +require 'cloud_controller/diego/custom_stack_uri_converter' + +module VCAP::CloudController + module Diego + module Buildpack + RSpec.describe DesiredLrpBuilder do + let(:stack) { 'cflinuxfs4' } + let(:config) do + Config.new({ + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{stack}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { stack.to_sym => '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) + end + let(:opts) do + { + stack: stack, + droplet_uri: 'http://droplet.example.com/droplet.tgz', + process_guid: 'process-guid-1', + droplet_hash: 'droplet-hash', + ports: [8080], + checksum_algorithm: 'sha256', + checksum_value: 'checksum-value', + start_command: './start', + action_user: 'vcap', + additional_container_env_vars: [] + } + end + + subject(:builder) { DesiredLrpBuilder.new(config, opts) } + + before do + VCAP::CloudController::Stack.find(name: 'cflinuxfs4') || VCAP::CloudController::Stack.create(name: 'cflinuxfs4') + end + + describe '#root_fs' do + context 'with a system stack' do + it 'returns preloaded rootfs' do + expect(builder.root_fs).to eq('preloaded:cflinuxfs4') + end + end + + context 'with a custom stack (docker:// URI)' do + let(:stack) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + let(:config) do + Config.new({ + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) + end + + it 'returns a docker:// rootfs URI in Diego format' do + expect(builder.root_fs).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') + end + end + + context 'with a custom stack from a private registry' do + let(:stack) { 'docker://registry.example.com/my-org/my-stack:v2.0' } + let(:config) do + Config.new({ + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) + end + + it 'returns a docker:// rootfs URI for the private registry' do + expect(builder.root_fs).to eq('docker://registry.example.com/my-org/my-stack#v2.0') + end + end + end + + describe '#cached_dependencies' do + context 'with a custom stack' do + let(:stack) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + let(:config) do + Config.new({ + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) + end + + it 'uses the default stack lifecycle bundle' do + allow(LifecycleBundleUriGenerator).to receive(:uri).and_return('http://lifecycle.example.com/bundle.tgz') + deps = builder.cached_dependencies + expect(deps.first.cache_key).to eq("buildpack-#{VCAP::CloudController::Stack.default.name}-lifecycle") + end + end + end + + describe '#image_layers' do + let(:config) do + Config.new({ + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: true + } + }) + end + + context 'with a custom stack' do + let(:stack) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + + it 'uses the default stack for lifecycle bundle and droplet destination' do + allow(LifecycleBundleUriGenerator).to receive(:uri).and_return('http://lifecycle.example.com/bundle.tgz') + layers = builder.image_layers + lifecycle_layer = layers.find { |l| l.name.include?('lifecycle') } + expect(lifecycle_layer.name).to eq('buildpack-default-stack-name-lifecycle') + end + end + end + end + end + end +end diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb new file mode 100644 index 00000000000..23357587005 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb @@ -0,0 +1,86 @@ +require 'spec_helper' +require 'cloud_controller/diego/buildpack/task_action_builder' +require 'cloud_controller/diego/custom_stack_uri_converter' + +module VCAP::CloudController + module Diego + module Buildpack + RSpec.describe TaskActionBuilder do + let(:stack_name) { 'cflinuxfs4' } + let(:config) do + Config.new({ + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) + end + let(:lifecycle_data) do + { + droplet_uri: 'http://droplet.example.com/droplet.tgz', + stack: stack_name + } + end + let(:task) do + instance_double(TaskModel, + name: 'my-task', + droplet: instance_double(DropletModel, sha256_checksum: 'abc123', droplet_hash: 'hash')) + end + + subject(:builder) do + TaskActionBuilder.new(config, task, lifecycle_data, 'vcap', ['app', 'echo hello', ''], 'buildpack') + end + + before do + VCAP::CloudController::Stack.find(name: 'cflinuxfs4') || VCAP::CloudController::Stack.create(name: 'cflinuxfs4') + end + + describe '#stack' do + context 'with a system stack' do + it 'returns preloaded rootfs' do + expect(builder.stack).to eq('preloaded:cflinuxfs4') + end + end + + context 'with a custom stack' do + let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + + it 'returns a docker:// rootfs URI' do + expect(builder.stack).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') + end + end + end + + describe '#lifecycle_bundle_key' do + context 'with a system stack' do + it 'uses the stack name' do + expect(builder.lifecycle_bundle_key).to eq(:"buildpack/cflinuxfs4") + end + end + + context 'with a custom stack' do + let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + + it 'falls back to the default stack name' do + expect(builder.lifecycle_bundle_key).to eq(:"buildpack/#{VCAP::CloudController::Stack.default.name}") + end + end + end + + describe '#cached_dependencies' do + context 'with a custom stack' do + let(:stack_name) { 'docker://registry.example.com/my-stack:v1' } + + it 'uses the default stack lifecycle bundle' do + allow(LifecycleBundleUriGenerator).to receive(:uri).and_return('http://lifecycle.example.com/bundle.tgz') + deps = builder.cached_dependencies + expect(deps.first.cache_key).to eq("buildpack-#{VCAP::CloudController::Stack.default.name}-lifecycle") + end + end + end + end + end + end +end diff --git a/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb index 49f1e9267db..5f2258cc7cb 100644 --- a/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb @@ -29,7 +29,7 @@ module CNB Diego::StagingDetails.new.tap do |details| details.staging_guid = droplet.guid details.package = package - details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, + details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, staging_message: double('msg', buildpack_data: double('bd', requested?: false)), credentials: '{"registry":{"username":"password"}}') end end @@ -60,7 +60,7 @@ module CNB details.environment_variables = { 'nightshade_fruit' => 'potato' } details.staging_memory_in_mb = 42 details.staging_disk_in_mb = 51 - details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, + details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, staging_message: double('msg', buildpack_data: double('bd', requested?: false)), credentials: '{"registry":{"username":"password"}}') end end @@ -87,6 +87,7 @@ module CNB details.staging_memory_in_mb = 42 details.staging_disk_in_mb = 51 details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, + staging_message: double('msg', buildpack_data: double('bd', requested?: false)), credentials: '{"registry.example.com":{"username":"user","password":"sekret"}}', credentials_for_staging: { 'registry.example.com' => { 'username' => 'user', 'password' => 'sekret' } }) @@ -148,7 +149,7 @@ module CNB let(:droplet) { create(:droplet_model, :cnb) } let(:staging_details) do StagingDetails.new.tap do |details| - details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: 'some buildpack info', + details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: 'some buildpack info', staging_message: double('msg', buildpack_data: double('bd', requested?: false)), credentials: '{"registry":{"username":"password"}}') details.package = package details.staging_guid = droplet.guid diff --git a/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb new file mode 100644 index 00000000000..4e9bcef4707 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb @@ -0,0 +1,120 @@ +require 'spec_helper' +require 'cloud_controller/diego/cnb/staging_action_builder' + +module VCAP::CloudController + module Diego + module CNB + RSpec.describe StagingActionBuilder do + let(:stack_name) { 'cflinuxfs4' } + let(:lifecycle_data) do + { + app_bits_download_uri: 'http://app.example.com/bits.zip', + build_artifacts_cache_upload_uri: 'http://cache.example.com/upload', + droplet_upload_uri: 'http://droplet.example.com/upload', + buildpacks: [], + stack: stack_name, + app_bits_checksum: { type: 'sha256', value: 'checksum' }, + auto_detect: true + } + end + let(:config) do + Config.new({ + diego: { + lifecycle_bundles: { 'cnb/default-stack-name': 'http://lifecycle.example.com/cnb-bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false, + cc_uploader_url: 'http://uploader.example.com' + }, + staging: { + timeout_in_seconds: 900, + legacy_md5_buildpack_paths_enabled: false + } + }) + end + let(:staging_details) { instance_double(StagingDetails, staging_guid: 'staging-guid') } + + subject(:builder) { StagingActionBuilder.new(config, staging_details, lifecycle_data) } + + before do + VCAP::CloudController::Stack.find(name: 'cflinuxfs4') || VCAP::CloudController::Stack.create(name: 'cflinuxfs4') + end + + describe '#task_environment_variables' do + context 'with a system stack' do + it 'includes CNB_STACK_ID set to the stack name' do + env_vars = builder.task_environment_variables + stack_id_var = env_vars.find { |v| v.name == 'CNB_STACK_ID' } + expect(stack_id_var).not_to be_nil + expect(stack_id_var.value).to eq('cflinuxfs4') + end + end + + context 'with a custom stack and no explicit stack_id' do + let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + + it 'omits CNB_STACK_ID (auto-detect)' do + env_vars = builder.task_environment_variables + stack_id_var = env_vars.find { |v| v.name == 'CNB_STACK_ID' } + expect(stack_id_var).to be_nil + end + end + + context 'with a custom stack and an explicit stack_id' do + let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + let(:lifecycle_data) do + { + app_bits_download_uri: 'http://app.example.com/bits.zip', + build_artifacts_cache_upload_uri: 'http://cache.example.com/upload', + droplet_upload_uri: 'http://droplet.example.com/upload', + buildpacks: [], + stack: stack_name, + stack_id: 'io.buildpacks.stacks.jammy', + app_bits_checksum: { type: 'sha256', value: 'checksum' }, + auto_detect: true + } + end + + it 'uses the provided stack_id' do + env_vars = builder.task_environment_variables + stack_id_var = env_vars.find { |v| v.name == 'CNB_STACK_ID' } + expect(stack_id_var).not_to be_nil + expect(stack_id_var.value).to eq('io.buildpacks.stacks.jammy') + end + end + + context 'with credentials in lifecycle_data' do + let(:lifecycle_data) do + { + app_bits_download_uri: 'http://app.example.com/bits.zip', + build_artifacts_cache_upload_uri: 'http://cache.example.com/upload', + droplet_upload_uri: 'http://droplet.example.com/upload', + buildpacks: [], + stack: stack_name, + app_bits_checksum: { type: 'sha256', value: 'checksum' }, + auto_detect: true, + credentials: '{"docker.io":{"username":"user","password":"pass"}}' + } + end + + it 'includes CNB_REGISTRY_CREDS' do + env_vars = builder.task_environment_variables + creds_var = env_vars.find { |v| v.name == 'CNB_REGISTRY_CREDS' } + expect(creds_var).not_to be_nil + expect(creds_var.value).to include('docker.io') + end + end + end + + describe '#stack' do + context 'with a custom stack' do + let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + + it 'returns docker:// rootfs in Diego format' do + expect(builder.stack).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') + end + end + end + end + end + end +end diff --git a/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb b/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb new file mode 100644 index 00000000000..73c1c59e457 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' +require 'cloud_controller/diego/custom_stack_uri_converter' + +module VCAP::CloudController + module Diego + RSpec.describe CustomStackUriConverter do + subject(:converter) { CustomStackUriConverter.new } + + describe '#convert' do + it 'converts docker://host/path:tag to Diego rootfs format' do + result = converter.convert('docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0') + expect(result).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') + end + + it 'converts docker://host/path without tag (no fragment appended)' do + result = converter.convert('docker://docker.io/cloudfoundry/cflinuxfs4') + expect(result).to eq('docker://docker.io/cloudfoundry/cflinuxfs4') + end + + it 'handles private registries' do + result = converter.convert('docker://registry.example.com/my-org/my-stack:v2.0') + expect(result).to eq('docker://registry.example.com/my-org/my-stack#v2.0') + end + + it 'handles images without explicit registry (docker hub shorthand)' do + result = converter.convert('docker://ubuntu:22.04') + expect(result).to eq('docker:///library/ubuntu#22.04') + end + + it 'handles digest references' do + digest = 'sha256:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890' + result = converter.convert("docker://docker.io/cloudfoundry/cflinuxfs4@#{digest}") + expect(result).to include('docker://docker.io/cloudfoundry/cflinuxfs4') + end + end + end + end +end diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_custom_stacks_spec.rb new file mode 100644 index 00000000000..199d5625c39 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_custom_stacks_spec.rb @@ -0,0 +1,91 @@ +require 'spec_helper' +require 'cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator' + +module VCAP::CloudController + RSpec.describe BuildpackLifecycleDataValidator do + let(:stack) { create(:stack) } + let(:buildpack_url) { 'https://github.com/my-org/my-buildpack.git' } + let(:buildpack_info) { BuildpackInfo.new(buildpack_url, nil) } + let(:buildpack_infos) { [buildpack_info] } + + subject(:validator) do + BuildpackLifecycleDataValidator.new( + buildpack_infos: buildpack_infos, + stack: stack, + stack_name: stack&.name + ) + end + + describe 'custom stacks' do + let(:stack) { nil } + let(:custom_stack_uri) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + + subject(:validator) do + BuildpackLifecycleDataValidator.new( + buildpack_infos: buildpack_infos, + stack: nil, + stack_name: custom_stack_uri + ) + end + + context 'when diego_custom_stacks feature flag is enabled' do + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: true) + end + + context 'with custom buildpacks (URL-based)' do + let(:buildpack_info) { BuildpackInfo.new('https://github.com/my-org/my-buildpack.git', nil) } + + it 'is valid' do + expect(validator).to be_valid + end + end + + context 'with admin buildpacks (not URL-based, no record)' do + let(:buildpack_info) { BuildpackInfo.new('ruby_buildpack', nil) } + + it 'is not valid' do + expect(validator).not_to be_valid + expect(validator.errors[:buildpack]).to include('must be a custom buildpack (URL) when using a custom stack') + end + end + end + + context 'when diego_custom_stacks feature flag is disabled' do + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: false) + end + + it 'is not valid (stack must exist in DB)' do + expect(validator).not_to be_valid + expect(validator.errors[:stack]).to include('must be an existing stack') + end + end + end + + describe 'system stacks (unchanged behavior)' do + context 'when stack exists in DB' do + it 'is valid' do + expect(validator).to be_valid + end + end + + context 'when stack does not exist in DB' do + let(:stack) { nil } + + subject(:validator) do + BuildpackLifecycleDataValidator.new( + buildpack_infos: buildpack_infos, + stack: nil, + stack_name: 'nonexistent-stack' + ) + end + + it 'is not valid' do + expect(validator).not_to be_valid + expect(validator.errors[:stack]).to include('must be an existing stack') + end + end + end + end +end diff --git a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb new file mode 100644 index 00000000000..c1e9a9c6a18 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb @@ -0,0 +1,133 @@ +require 'spec_helper' +require 'cloud_controller/diego/task_recipe_builder' + +module VCAP::CloudController + module Diego + RSpec.describe TaskRecipeBuilder do + subject(:task_recipe_builder) { TaskRecipeBuilder.new } + + describe '#staging_image_username / #staging_image_password (custom stacks)' do + let(:custom_stack_uri) { 'docker://registry.example.com/my-org/my-stack:v1.0' } + let(:lifecycle) do + instance_double( + BuildpackLifecycle, + staging_stack: custom_stack_uri, + credentials_for_staging: { + 'registry.example.com' => { + 'username' => 'my-user', + 'password' => 'my-pass' + } + } + ) + end + let(:package) do + instance_double(PackageModel, docker_username: nil, docker_password: nil) + end + let(:staging_details) do + instance_double(StagingDetails, package: package, lifecycle: lifecycle) + end + + it 'returns credentials from the custom stack lifecycle' do + expect(task_recipe_builder.send(:staging_image_username, staging_details)).to eq('my-user') + expect(task_recipe_builder.send(:staging_image_password, staging_details)).to eq('my-pass') + end + end + + describe '#staging_image_username / #staging_image_password (docker package)' do + let(:lifecycle) do + instance_double( + BuildpackLifecycle, + staging_stack: 'cflinuxfs4', + credentials_for_staging: nil + ) + end + let(:package) do + instance_double(PackageModel, docker_username: 'docker-user', docker_password: 'docker-pass') + end + let(:staging_details) do + instance_double(StagingDetails, package: package, lifecycle: lifecycle) + end + + it 'uses docker package credentials when both are present' do + expect(task_recipe_builder.send(:staging_image_username, staging_details)).to eq('docker-user') + expect(task_recipe_builder.send(:staging_image_password, staging_details)).to eq('docker-pass') + end + end + + describe '#staging_image_username / #staging_image_password (mismatched docker credentials)' do + let(:lifecycle) do + instance_double( + BuildpackLifecycle, + staging_stack: 'docker://registry.example.com/my-org/my-stack:v1.0', + credentials_for_staging: { + 'registry.example.com' => { + 'username' => 'stack-user', + 'password' => 'stack-pass' + } + } + ) + end + let(:package) do + instance_double(PackageModel, docker_username: 'only-user', docker_password: nil) + end + let(:staging_details) do + instance_double(StagingDetails, package: package, lifecycle: lifecycle) + end + + it 'falls through to custom stack credentials when docker pair is incomplete' do + expect(task_recipe_builder.send(:staging_image_username, staging_details)).to eq('stack-user') + expect(task_recipe_builder.send(:staging_image_password, staging_details)).to eq('stack-pass') + end + end + + describe '#task_image_username / #task_image_password (custom stacks)' do + let(:lifecycle_data) do + instance_double( + BuildpackLifecycleDataModel, + stack: 'docker://registry.example.com/my-org/my-stack:v1.0', + credentials: { + 'registry.example.com' => { + 'username' => 'task-user', + 'password' => 'task-pass' + } + } + ) + end + let(:app_model) do + instance_double(AppModel, lifecycle_data: lifecycle_data) + end + let(:droplet) do + instance_double(DropletModel, docker_receipt_username: nil, docker_receipt_password: nil) + end + let(:task) do + instance_double(TaskModel, app: app_model, droplet: droplet) + end + + it 'returns credentials from the custom stack lifecycle data' do + expect(task_recipe_builder.send(:task_image_username, task)).to eq('task-user') + expect(task_recipe_builder.send(:task_image_password, task)).to eq('task-pass') + end + end + + describe '#task_image_username / #task_image_password (docker droplet)' do + let(:lifecycle_data) do + instance_double(BuildpackLifecycleDataModel, stack: 'cflinuxfs4', credentials: nil) + end + let(:app_model) do + instance_double(AppModel, lifecycle_data: lifecycle_data) + end + let(:droplet) do + instance_double(DropletModel, docker_receipt_username: 'droplet-user', docker_receipt_password: 'droplet-pass') + end + let(:task) do + instance_double(TaskModel, app: app_model, droplet: droplet) + end + + it 'uses docker receipt credentials when both are present' do + expect(task_recipe_builder.send(:task_image_username, task)).to eq('droplet-user') + expect(task_recipe_builder.send(:task_image_password, task)).to eq('droplet-pass') + end + end + end + end +end diff --git a/spec/unit/lib/utils/uri_utils_custom_stack_spec.rb b/spec/unit/lib/utils/uri_utils_custom_stack_spec.rb new file mode 100644 index 00000000000..09859adc613 --- /dev/null +++ b/spec/unit/lib/utils/uri_utils_custom_stack_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +RSpec.describe UriUtils do + describe '.is_custom_stack_uri?' do + it 'returns true for valid docker:// URIs' do + expect(UriUtils.is_custom_stack_uri?('docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0')).to be(true) + expect(UriUtils.is_custom_stack_uri?('docker://registry.example.com/my-org/my-stack:latest')).to be(true) + expect(UriUtils.is_custom_stack_uri?('docker://ghcr.io/cloudfoundry/cflinuxfs4:1.0.0')).to be(true) + end + + it 'returns true for docker:// URIs without explicit tags (defaults to latest)' do + expect(UriUtils.is_custom_stack_uri?('docker://docker.io/cloudfoundry/cflinuxfs4')).to be(true) + end + + it 'returns true for docker:// URIs with digest' do + expect(UriUtils.is_custom_stack_uri?('docker://docker.io/cloudfoundry/cflinuxfs4@sha256:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890')).to be(true) + end + + it 'returns false for non-string inputs' do + expect(UriUtils.is_custom_stack_uri?(nil)).to be(false) + expect(UriUtils.is_custom_stack_uri?(123)).to be(false) + expect(UriUtils.is_custom_stack_uri?({})).to be(false) + end + + it 'returns false for plain stack names' do + expect(UriUtils.is_custom_stack_uri?('cflinuxfs4')).to be(false) + expect(UriUtils.is_custom_stack_uri?('cflinuxfs3')).to be(false) + end + + it 'returns false for http/https URIs' do + expect(UriUtils.is_custom_stack_uri?('https://example.com/stack')).to be(false) + expect(UriUtils.is_custom_stack_uri?('http://example.com/stack')).to be(false) + end + + it 'returns false for invalid docker URIs' do + expect(UriUtils.is_custom_stack_uri?('docker://')).to be(false) + expect(UriUtils.is_custom_stack_uri?('docker://INVALID_UPPERCASE')).to be(false) + end + end +end diff --git a/spec/unit/messages/app_manifest_message_spec.rb b/spec/unit/messages/app_manifest_message_spec.rb index e134e6ae7b4..7909fd0e338 100644 --- a/spec/unit/messages/app_manifest_message_spec.rb +++ b/spec/unit/messages/app_manifest_message_spec.rb @@ -2181,7 +2181,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.example.com' => { 'username' => 'user', 'password' => 'pass' } } } } it 'adds credentials to the lifecycle_data' do message = AppManifestMessage.create_from_yml(parsed_yaml) @@ -2190,7 +2190,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.example.com': { username: 'user', password: 'pass' } }) end end diff --git a/spec/unit/messages/buildpack_lifecycle_data_message_custom_stacks_spec.rb b/spec/unit/messages/buildpack_lifecycle_data_message_custom_stacks_spec.rb new file mode 100644 index 00000000000..ab798a995a9 --- /dev/null +++ b/spec/unit/messages/buildpack_lifecycle_data_message_custom_stacks_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe BuildpackLifecycleDataMessage do + describe 'stack_id field' do + it 'is valid with a stack_id' do + message = BuildpackLifecycleDataMessage.new({ + stack: 'docker://docker.io/cloudfoundry/cflinuxfs4:1.0.0', + stack_id: 'io.buildpacks.stacks.jammy' + }) + expect(message).to be_valid + end + + it 'is valid without a stack_id' do + message = BuildpackLifecycleDataMessage.new({ + stack: 'cflinuxfs4' + }) + expect(message).to be_valid + end + + it 'is invalid with a too-long stack_id' do + message = BuildpackLifecycleDataMessage.new({ + stack: 'cflinuxfs4', + stack_id: 'a' * 4097 + }) + expect(message).not_to be_valid + end + end + + describe 'custom stack URI as stack value' do + it 'accepts a docker:// URI as stack value' do + message = BuildpackLifecycleDataMessage.new({ + stack: 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0', + buildpacks: ['https://github.com/my-org/my-buildpack.git'] + }) + expect(message).to be_valid + end + end + end +end diff --git a/spec/unit/models/runtime/feature_flag_custom_stacks_spec.rb b/spec/unit/models/runtime/feature_flag_custom_stacks_spec.rb new file mode 100644 index 00000000000..91e1e63eafc --- /dev/null +++ b/spec/unit/models/runtime/feature_flag_custom_stacks_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe FeatureFlag, type: :model do + describe 'diego_custom_stacks feature flag' do + context 'when the diego_custom_stacks feature flag is not overridden' do + it 'returns the default value (disabled)' do + expect(FeatureFlag.enabled?(:diego_custom_stacks)).to be(false) + expect(FeatureFlag.disabled?(:diego_custom_stacks)).to be(true) + end + end + + context 'when the diego_custom_stacks feature flag is enabled' do + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: true) + end + + it 'returns true' do + expect(FeatureFlag.enabled?(:diego_custom_stacks)).to be(true) + end + end + + context 'when the diego_custom_stacks feature flag is disabled' do + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: false) + end + + it 'raises FeatureDisabled when raise_unless_enabled!' do + expect { + FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) + }.to raise_error(CloudController::Errors::ApiError) { |error| + expect(error.name).to eq('FeatureDisabled') + } + end + end + end + end +end From 0a870f67851e922fd9789776acaa9c95892ed71a Mon Sep 17 00:00:00 2001 From: Florian Braun <5863788+FloThinksPi@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:34:35 +0200 Subject: [PATCH 2/5] Refactor custom stacks: fix bugs, reduce duplication Bug fixes: - Guard against nil buildpack_names in BuildpackLifecycleFetcher - Reject empty buildpack list for custom stacks (vacuous all? bug) - Move feature flag enforcement out of staging_stack getter - Narrow rescue scope in AppManifestMessage to ApiError - Fix nil lifecycle_data crash in controller start/restart Refactoring: - Extract ImageCredentialResolver (replaces ~90 lines of duplication) - Extract CustomStackFallback module (shared resolved_stack_name) - Convert CustomStackUriConverter to module_function - Deduplicate UriUtils parsing (is_custom_stack_uri? delegates) - Use ActiveSupport deep_stringify_keys instead of hand-rolled - Unify controller custom stacks helper - Memoize StagingActionBuilder#stack for custom path - Simplify CNB lifecycle_protocol stack_id assignment - Fix has_username_and_password? to reject empty strings Also fixes asg_timestamps migration for Ruby 3.4 compatibility. --- app/controllers/v3/apps_controller.rb | 15 +++- app/fetchers/buildpack_lifecycle_fetcher.rb | 2 +- app/messages/app_manifest_message.rb | 2 +- .../diego/app_recipe_builder.rb | 46 +++------- .../diego/buildpack/desired_lrp_builder.rb | 13 +-- .../diego/buildpack/task_action_builder.rb | 12 +-- .../diego/cnb/desired_lrp_builder.rb | 12 +-- .../diego/cnb/lifecycle_protocol.rb | 6 +- .../diego/cnb/staging_action_builder.rb | 6 +- .../diego/custom_stack_fallback.rb | 13 +++ .../diego/custom_stack_uri_converter.rb | 15 ++-- .../diego/image_credential_resolver.rb | 47 ++++++++++ .../buildpack_lifecycle_data_validator.rb | 2 +- .../diego/lifecycles/lifecycle_base.rb | 12 +-- .../diego/staging_action_builder.rb | 22 ++--- .../diego/task_recipe_builder.rb | 87 ++++--------------- lib/utils/uri_utils.rb | 15 ++-- .../diego/custom_stack_uri_converter_spec.rb | 14 ++- .../task_recipe_builder_custom_stacks_spec.rb | 35 ++++---- 19 files changed, 174 insertions(+), 202 deletions(-) create mode 100644 lib/cloud_controller/diego/custom_stack_fallback.rb create mode 100644 lib/cloud_controller/diego/image_credential_resolver.rb diff --git a/app/controllers/v3/apps_controller.rb b/app/controllers/v3/apps_controller.rb index b49c06e40d3..caf42077e53 100644 --- a/app/controllers/v3/apps_controller.rb +++ b/app/controllers/v3/apps_controller.rb @@ -189,7 +189,7 @@ def start FeatureFlag.raise_unless_enabled!(:diego_docker) if app.lifecycle_type == DockerLifecycleDataModel::LIFECYCLE_TYPE FeatureFlag.raise_unless_enabled!(:diego_cnb) if app.lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE - FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if app.lifecycle_data.respond_to?(:stack) && UriUtils.is_custom_stack_uri?(app.lifecycle_data.stack) + raise_unless_custom_stacks_enabled!(app.lifecycle_data.stack) if app.lifecycle_data.respond_to?(:stack) AppStart.start(app:, user_audit_info:) TelemetryLogger.v3_emit( @@ -233,7 +233,7 @@ def restart FeatureFlag.raise_unless_enabled!(:diego_docker) if app.lifecycle_type == DockerLifecycleDataModel::LIFECYCLE_TYPE FeatureFlag.raise_unless_enabled!(:diego_cnb) if app.lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE - FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if app.lifecycle_data.respond_to?(:stack) && UriUtils.is_custom_stack_uri?(app.lifecycle_data.stack) + raise_unless_custom_stacks_enabled!(app.lifecycle_data.stack) if app.lifecycle_data.respond_to?(:stack) AppRestart.restart(app: app, config: Config.config, user_audit_info: user_audit_info) TelemetryLogger.v3_emit( @@ -400,8 +400,15 @@ def translate_error(e) private - def raise_unless_custom_stacks_enabled!(message) - stack = message.lifecycle_data&.dig(:stack) + def raise_unless_custom_stacks_enabled!(message_or_stack) + stack = case message_or_stack + when String + message_or_stack + when nil + nil + else + message_or_stack.lifecycle_data&.dig(:stack) + end FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if stack && UriUtils.is_custom_stack_uri?(stack) end diff --git a/app/fetchers/buildpack_lifecycle_fetcher.rb b/app/fetchers/buildpack_lifecycle_fetcher.rb index 17e08715d5f..50f9a688ccf 100644 --- a/app/fetchers/buildpack_lifecycle_fetcher.rb +++ b/app/fetchers/buildpack_lifecycle_fetcher.rb @@ -7,7 +7,7 @@ def fetch(buildpack_names, stack_name, lifecycle=Config.config.get(:default_app_ if UriUtils.is_custom_stack_uri?(stack_name) { stack: nil, - buildpack_infos: buildpack_names.map { |name| BuildpackInfo.new(name, nil) } + buildpack_infos: Array(buildpack_names).map { |name| BuildpackInfo.new(name, nil) } } else { diff --git a/app/messages/app_manifest_message.rb b/app/messages/app_manifest_message.rb index da1da6c911f..57998e02864 100644 --- a/app/messages/app_manifest_message.rb +++ b/app/messages/app_manifest_message.rb @@ -491,7 +491,7 @@ def validate_cnb_enabled! def validate_custom_stacks_enabled! FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if requested?(:stack) && UriUtils.is_custom_stack_uri?(@stack) - rescue StandardError => e + rescue CloudController::Errors::ApiError => e errors.add(:base, e.message) end diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index 671777af9f4..e6cdfe61562 100644 --- a/lib/cloud_controller/diego/app_recipe_builder.rb +++ b/lib/cloud_controller/diego/app_recipe_builder.rb @@ -5,6 +5,7 @@ require 'cloud_controller/diego/docker/desired_lrp_builder' require 'cloud_controller/diego/cnb/desired_lrp_builder' require 'cloud_controller/diego/custom_stack_uri_converter' +require 'cloud_controller/diego/image_credential_resolver' require 'cloud_controller/diego/process_guid' require 'cloud_controller/diego/ssh_key' require 'cloud_controller/diego/service_binding_files_builder' @@ -102,46 +103,23 @@ def app_lrp_arguments certificate_properties: ::Diego::Bbs::Models::CertificateProperties.new( organizational_unit: ["organization:#{process.organization.guid}", "space:#{process.space.guid}", "app:#{process.app_guid}"] ), - image_username: lrp_image_username(process), - image_password: lrp_image_password(process), + image_username: lrp_image_credentials(process)[0], + image_password: lrp_image_credentials(process)[1], volume_mounted_files: ServiceBindingFilesBuilder.build(process) }.compact rescue ServiceBindingFilesBuilder::IncompatibleBindings => e raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', "Cannot build service binding files for app - #{e.message}") end - def lrp_image_username(process) - username, password = docker_receipt_credentials(process) - return username if username.present? && password.present? - - custom_stack_lrp_credential(process, 'username') - end - - def lrp_image_password(process) - username, password = docker_receipt_credentials(process) - return password if username.present? && password.present? - - custom_stack_lrp_credential(process, 'password') - end - - def docker_receipt_credentials(process) - droplet = process.desired_droplet - [droplet.docker_receipt_username, droplet.docker_receipt_password] - end - - def custom_stack_lrp_credential(process, field) - lifecycle_data = process.app.lifecycle_data - return nil unless lifecycle_data.respond_to?(:credentials) && lifecycle_data.respond_to?(:stack) - - stack = lifecycle_data.stack - stack_host = UriUtils.custom_stack_registry_host(stack) - return nil unless stack_host - - credentials = lifecycle_data.credentials - return nil unless credentials.is_a?(Hash) - - cred = credentials[stack_host] - cred&.fetch(field, nil) + def lrp_image_credentials(process) + @lrp_image_credentials ||= begin + droplet = process.desired_droplet + ImageCredentialResolver.resolve( + primary_username: droplet.docker_receipt_username, + primary_password: droplet.docker_receipt_password, + lifecycle_data: process.app.lifecycle_data + ) + end end def metric_tags(process) diff --git a/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb b/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb index b99266ce12d..1e10a202a56 100644 --- a/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb @@ -1,4 +1,5 @@ require 'cloud_controller/diego/custom_stack_uri_converter' +require 'cloud_controller/diego/custom_stack_fallback' require 'utils/uri_utils' module VCAP::CloudController @@ -6,6 +7,7 @@ module Diego module Buildpack class DesiredLrpBuilder include ::Diego::ActionBuilder + include CustomStackFallback class InvalidStack < StandardError; end @@ -42,7 +44,7 @@ def cached_dependencies end def root_fs - return CustomStackUriConverter.new.convert(@stack) if UriUtils.is_custom_stack_uri?(@stack) + return CustomStackUriConverter.convert(@stack) if UriUtils.is_custom_stack_uri?(@stack) @stack_obj ||= Stack.find(name: @stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', @stack) unless @stack_obj @@ -125,15 +127,6 @@ def privileged? private - # For custom stacks, fall back to the default stack for lifecycle bundle and droplet destination lookups - def resolved_stack_name - if UriUtils.is_custom_stack_uri?(@stack) - VCAP::CloudController::Stack.default.name - else - @stack - end - end - def resolved_lifecycle_bundle_key :"buildpack/#{resolved_stack_name}" end diff --git a/lib/cloud_controller/diego/buildpack/task_action_builder.rb b/lib/cloud_controller/diego/buildpack/task_action_builder.rb index 49194085fd4..43233c50d17 100644 --- a/lib/cloud_controller/diego/buildpack/task_action_builder.rb +++ b/lib/cloud_controller/diego/buildpack/task_action_builder.rb @@ -1,6 +1,7 @@ require 'diego/action_builder' require 'cloud_controller/diego/task_environment_variable_collector' require 'cloud_controller/diego/custom_stack_uri_converter' +require 'cloud_controller/diego/custom_stack_fallback' require 'credhub/config_helpers' module VCAP::CloudController @@ -8,6 +9,7 @@ module Diego module Buildpack class TaskActionBuilder include ::Diego::ActionBuilder + include CustomStackFallback class InvalidStack < StandardError; end @@ -92,7 +94,7 @@ def task_environment_variables end def stack - return CustomStackUriConverter.new.convert(lifecycle_stack) if UriUtils.is_custom_stack_uri?(lifecycle_stack) + return CustomStackUriConverter.convert(lifecycle_stack) if UriUtils.is_custom_stack_uri?(lifecycle_stack) @stack ||= Stack.find(name: lifecycle_stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', lifecycle_stack) unless @stack @@ -121,14 +123,6 @@ def lifecycle_bundle_key def lifecycle_stack lifecycle_data[:stack] end - - def resolved_stack_name - if UriUtils.is_custom_stack_uri?(lifecycle_stack) - VCAP::CloudController::Stack.default.name - else - lifecycle_stack - end - end end end end diff --git a/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb b/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb index 92f1dbe5200..b54cf1cdd49 100644 --- a/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb @@ -1,4 +1,5 @@ require 'cloud_controller/diego/custom_stack_uri_converter' +require 'cloud_controller/diego/custom_stack_fallback' require 'utils/uri_utils' module VCAP::CloudController @@ -6,6 +7,7 @@ module Diego module CNB class DesiredLrpBuilder include ::Diego::ActionBuilder + include CustomStackFallback class InvalidStack < StandardError; end @@ -42,7 +44,7 @@ def cached_dependencies end def root_fs - return CustomStackUriConverter.new.convert(@stack) if UriUtils.is_custom_stack_uri?(@stack) + return CustomStackUriConverter.convert(@stack) if UriUtils.is_custom_stack_uri?(@stack) @stack_obj ||= Stack.find(name: @stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', @stack) unless @stack_obj @@ -123,14 +125,6 @@ def privileged? private - def resolved_stack_name - if UriUtils.is_custom_stack_uri?(@stack) - VCAP::CloudController::Stack.default.name - else - @stack - end - end - def default_container_env [ ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'LANG', value: DEFAULT_LANG), diff --git a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb index 6475043570d..65101cc8011 100644 --- a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb @@ -26,9 +26,9 @@ def new_lifecycle_data(staging_details) lifecycle_data = LifecycleData.new lifecycle_data.credentials = staging_details.lifecycle.credentials lifecycle_data.auto_detect = staging_details.lifecycle.buildpack_infos.empty? - if staging_details.lifecycle.staging_message.buildpack_data.requested?(:stack_id) - lifecycle_data.stack_id = staging_details.lifecycle.staging_message.buildpack_data.stack_id - end + + bp_data = staging_details.lifecycle.staging_message.buildpack_data + lifecycle_data.stack_id = bp_data.stack_id if bp_data.requested?(:stack_id) lifecycle_data end diff --git a/lib/cloud_controller/diego/cnb/staging_action_builder.rb b/lib/cloud_controller/diego/cnb/staging_action_builder.rb index 32af682f436..0bf60afb93c 100644 --- a/lib/cloud_controller/diego/cnb/staging_action_builder.rb +++ b/lib/cloud_controller/diego/cnb/staging_action_builder.rb @@ -17,9 +17,6 @@ def task_environment_variables ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_GROUP_ID', value: '2000'), ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'LANG', value: STAGING_DEFAULT_LANG) ] - # For custom stacks, CNB_STACK_ID is optional: if a stack_id was provided - # in lifecycle data, use it; otherwise omit it and let the CNB builder auto-detect. - # For system stacks, always pass the stack name as CNB_STACK_ID. cnb_stack_id = resolve_cnb_stack_id env.push(::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_STACK_ID', value: cnb_stack_id)) if cnb_stack_id env.push(::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_REGISTRY_CREDS', value: lifecycle_data[:credentials])) if lifecycle_data[:credentials] @@ -30,8 +27,7 @@ def task_environment_variables def resolve_cnb_stack_id if UriUtils.is_custom_stack_uri?(lifecycle_stack) - # Use explicitly provided stack_id from lifecycle_data if present, otherwise omit (auto-detect) - lifecycle_data[:stack_id] + lifecycle_data[:stack_id] # nil means auto-detect else lifecycle_stack end diff --git a/lib/cloud_controller/diego/custom_stack_fallback.rb b/lib/cloud_controller/diego/custom_stack_fallback.rb new file mode 100644 index 00000000000..2e5d27d72cf --- /dev/null +++ b/lib/cloud_controller/diego/custom_stack_fallback.rb @@ -0,0 +1,13 @@ +require 'utils/uri_utils' + +module VCAP::CloudController + module Diego + # Custom stacks reuse the default stack's lifecycle binaries. + module CustomStackFallback + def resolved_stack_name(stack=nil) + stack ||= @stack || (respond_to?(:lifecycle_stack, true) ? lifecycle_stack : nil) + UriUtils.is_custom_stack_uri?(stack) ? VCAP::CloudController::Stack.default.name : stack + end + end + end +end diff --git a/lib/cloud_controller/diego/custom_stack_uri_converter.rb b/lib/cloud_controller/diego/custom_stack_uri_converter.rb index 0ad5fde77a3..7e6adc8be99 100644 --- a/lib/cloud_controller/diego/custom_stack_uri_converter.rb +++ b/lib/cloud_controller/diego/custom_stack_uri_converter.rb @@ -2,16 +2,13 @@ module VCAP::CloudController module Diego - class CustomStackUriConverter - # Converts a custom stack URI from the user-facing format (docker://host/path:tag) - # to the Diego BBS rootfs format (docker://host/path#tag). - # This reuses the same conversion logic that Docker lifecycle uses. - def convert(custom_stack_uri) - # Strip the docker:// scheme prefix to get the raw image reference - raw_image_ref = custom_stack_uri.sub(%r{\Adocker://}, '') - - host, path, tag_digest = UriUtils.parse_docker_uri(raw_image_ref) + # Converts docker://host/path:tag to Diego BBS rootfs format docker://host/path#tag + module CustomStackUriConverter + module_function + def convert(custom_stack_uri) + raw = custom_stack_uri.delete_prefix('docker://') + host, path, tag_digest = UriUtils.parse_docker_uri(raw) Addressable::URI.new(scheme: 'docker', host: host, path: path, fragment: tag_digest).to_s end end diff --git a/lib/cloud_controller/diego/image_credential_resolver.rb b/lib/cloud_controller/diego/image_credential_resolver.rb new file mode 100644 index 00000000000..2a9f26fae41 --- /dev/null +++ b/lib/cloud_controller/diego/image_credential_resolver.rb @@ -0,0 +1,47 @@ +require 'utils/uri_utils' + +module VCAP::CloudController + module Diego + module ImageCredentialResolver + module_function + + # Returns [username, password], falling back to custom stack registry credentials. + def resolve(primary_username:, primary_password:, lifecycle_data:) + return [primary_username, primary_password] if primary_username.present? && primary_password.present? + + resolve_from_custom_stack(lifecycle_data) + end + + # Like resolve, but for staging where the lifecycle object provides :staging_stack. + def resolve_for_staging(primary_username:, primary_password:, lifecycle:) + return [primary_username, primary_password] if primary_username.present? && primary_password.present? + return [nil, nil] unless lifecycle.respond_to?(:credentials_for_staging) && lifecycle.respond_to?(:staging_stack) + + stack_host = UriUtils.custom_stack_registry_host(lifecycle.staging_stack) + return [nil, nil] unless stack_host + + extract_credentials(lifecycle.credentials_for_staging, stack_host) + end + + def resolve_from_custom_stack(lifecycle_data) + return [nil, nil] unless lifecycle_data.respond_to?(:credentials) && lifecycle_data.respond_to?(:stack) + + stack_host = UriUtils.custom_stack_registry_host(lifecycle_data.stack) + return [nil, nil] unless stack_host + + extract_credentials(lifecycle_data.credentials, stack_host) + end + private_class_method :resolve_from_custom_stack + + def extract_credentials(credentials, host) + return [nil, nil] unless credentials.is_a?(Hash) + + creds = credentials[host] + return [nil, nil] unless creds.is_a?(Hash) + + [creds['username'], creds['password']] + end + private_class_method :extract_credentials + end + end +end diff --git a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb index 373388c68b4..1655d62f42f 100644 --- a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb +++ b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb @@ -33,7 +33,7 @@ def stack_exists_in_db def custom_stack_requires_custom_buildpack return unless UriUtils.is_custom_stack_uri?(stack_name) - return if buildpack_infos.all? { |info| info.buildpack_url.present? } + return if buildpack_infos.present? && buildpack_infos.all? { |info| info.buildpack_url.present? } errors.add(:buildpack, 'must be a custom buildpack (URL) when using a custom stack') end diff --git a/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb b/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb index 15b8bdd25dc..1c1bbcc6bfa 100644 --- a/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb +++ b/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb @@ -9,6 +9,8 @@ def initialize(package, staging_message) @staging_message = staging_message @package = package + assert_custom_stacks_enabled! + db_result = BuildpackLifecycleFetcher.fetch(buildpacks_to_use, staging_stack, type) @buildpack_infos = db_result[:buildpack_infos] @@ -27,15 +29,15 @@ def initialize(package, staging_message) delegate :valid?, :errors, to: :validator def staging_stack - @staging_stack ||= begin - stack = requested_stack || app_stack || VCAP::CloudController::Stack.default.name - FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if UriUtils.is_custom_stack_uri?(stack) - stack - end + @staging_stack ||= requested_stack || app_stack || VCAP::CloudController::Stack.default.name end private + def assert_custom_stacks_enabled! + FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if UriUtils.is_custom_stack_uri?(staging_stack) + end + def validator_class BuildpackLifecycleDataValidator end diff --git a/lib/cloud_controller/diego/staging_action_builder.rb b/lib/cloud_controller/diego/staging_action_builder.rb index 46c207da94c..c7b455f2b6b 100644 --- a/lib/cloud_controller/diego/staging_action_builder.rb +++ b/lib/cloud_controller/diego/staging_action_builder.rb @@ -2,12 +2,14 @@ require 'diego/action_builder' require 'digest/xxhash' require 'cloud_controller/diego/custom_stack_uri_converter' +require 'cloud_controller/diego/custom_stack_fallback' module VCAP::CloudController module Diego class StagingActionBuilder include ::Credhub::ConfigHelpers include ::Diego::ActionBuilder + include CustomStackFallback attr_reader :config, :lifecycle_data, :staging_details @@ -131,12 +133,14 @@ def image_layers end def stack - return CustomStackUriConverter.new.convert(lifecycle_stack) if UriUtils.is_custom_stack_uri?(lifecycle_stack) - - @stack ||= Stack.find(name: lifecycle_stack) - raise CloudController::Errors::ApiError.new_from_details('StackNotFound', lifecycle_stack) unless @stack - - "preloaded:#{@stack.build_rootfs_image}" + @stack ||= if UriUtils.is_custom_stack_uri?(lifecycle_stack) + CustomStackUriConverter.convert(lifecycle_stack) + else + stack_obj = Stack.find(name: lifecycle_stack) + raise CloudController::Errors::ApiError.new_from_details('StackNotFound', lifecycle_stack) unless stack_obj + + "preloaded:#{stack_obj.build_rootfs_image}" + end end def lifecycle_stack @@ -201,11 +205,7 @@ def skip_detect? end def lifecycle_bundle_key - if UriUtils.is_custom_stack_uri?(lifecycle_data[:stack]) - :"#{@prefix}/#{VCAP::CloudController::Stack.default.name}" - else - :"#{@prefix}/#{lifecycle_data[:stack]}" - end + :"#{@prefix}/#{resolved_stack_name(lifecycle_data[:stack])}" end def upload_buildpack_artifacts_cache_uri diff --git a/lib/cloud_controller/diego/task_recipe_builder.rb b/lib/cloud_controller/diego/task_recipe_builder.rb index cc60bc1692e..a1bbc46fc6c 100644 --- a/lib/cloud_controller/diego/task_recipe_builder.rb +++ b/lib/cloud_controller/diego/task_recipe_builder.rb @@ -3,6 +3,7 @@ require 'cloud_controller/diego/buildpack/task_action_builder' require 'cloud_controller/diego/docker/task_action_builder' require 'cloud_controller/diego/bbs_environment_builder' +require 'cloud_controller/diego/image_credential_resolver' require 'cloud_controller/diego/task_completion_callback_generator' require 'cloud_controller/diego/task_cpu_weight_calculator' require 'cloud_controller/diego/service_binding_files_builder' @@ -52,8 +53,8 @@ def build_app_task(config, task) "app:#{task.app_guid}" ] ), - image_username: task_image_username(task), - image_password: task_image_password(task), + image_username: task_image_credentials(task)[0], + image_password: task_image_credentials(task)[1], volume_mounted_files: ServiceBindingFilesBuilder.build(task.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -93,8 +94,8 @@ def build_staging_task(config, staging_details) "app:#{staging_details.package.app_guid}" ] ), - image_username: staging_image_username(staging_details), - image_password: staging_image_password(staging_details), + image_username: staging_image_credentials(staging_details)[0], + image_password: staging_image_credentials(staging_details)[1], volume_mounted_files: ServiceBindingFilesBuilder.build(staging_details.package.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -103,71 +104,21 @@ def build_staging_task(config, staging_details) private - def task_image_username(task) - username, password = task_docker_receipt_credentials(task) - return username if username.present? && password.present? - - custom_stack_task_credential(task, 'username') - end - - def task_image_password(task) - username, password = task_docker_receipt_credentials(task) - return password if username.present? && password.present? - - custom_stack_task_credential(task, 'password') - end - - def task_docker_receipt_credentials(task) + def task_image_credentials(task) droplet = task.droplet - [droplet.docker_receipt_username, droplet.docker_receipt_password] - end - - def custom_stack_task_credential(task, field) - lifecycle_data = task.app.lifecycle_data - return nil unless lifecycle_data.respond_to?(:credentials) && lifecycle_data.respond_to?(:stack) - - stack = lifecycle_data.stack - stack_host = UriUtils.custom_stack_registry_host(stack) - return nil unless stack_host - - credentials = lifecycle_data.credentials - return nil unless credentials.is_a?(Hash) - - cred = credentials[stack_host] - cred&.fetch(field, nil) - end - - def staging_image_username(staging_details) - username, password = staging_docker_credentials(staging_details) - return username if username.present? && password.present? - - custom_stack_credential(staging_details, 'username') - end - - def staging_image_password(staging_details) - username, password = staging_docker_credentials(staging_details) - return password if username.present? && password.present? - - custom_stack_credential(staging_details, 'password') - end - - def staging_docker_credentials(staging_details) - [staging_details.package.docker_username, staging_details.package.docker_password] - end - - def custom_stack_credential(staging_details, field) - lifecycle = staging_details.lifecycle - return nil unless lifecycle.respond_to?(:credentials_for_staging) && lifecycle.respond_to?(:staging_stack) - - stack = lifecycle.staging_stack - stack_host = UriUtils.custom_stack_registry_host(stack) - return nil unless stack_host - - credentials = lifecycle.credentials_for_staging - return nil unless credentials.is_a?(Hash) - - cred = credentials[stack_host] - cred&.fetch(field, nil) + ImageCredentialResolver.resolve( + primary_username: droplet.docker_receipt_username, + primary_password: droplet.docker_receipt_password, + lifecycle_data: task.app.lifecycle_data + ) + end + + def staging_image_credentials(staging_details) + ImageCredentialResolver.resolve_for_staging( + primary_username: staging_details.package.docker_username, + primary_password: staging_details.package.docker_password, + lifecycle: staging_details.lifecycle + ) end def metric_tags(source) diff --git a/lib/utils/uri_utils.rb b/lib/utils/uri_utils.rb index 8318bb6e38f..ca5e17db0dd 100644 --- a/lib/utils/uri_utils.rb +++ b/lib/utils/uri_utils.rb @@ -45,19 +45,16 @@ def url_has_userinfo?(url) end def is_custom_stack_uri?(candidate) - return false unless candidate.is_a?(String) - return false unless candidate.start_with?('docker://') - - host, = parse_docker_uri(candidate.delete_prefix('docker://')) - host.present? - rescue StandardError - false + !custom_stack_registry_host(candidate).nil? end def custom_stack_registry_host(stack_uri) - return nil unless is_custom_stack_uri?(stack_uri) + return nil unless stack_uri.is_a?(String) && stack_uri.start_with?('docker://') + + body = stack_uri.delete_prefix('docker://') + return nil if body.empty? - host, = parse_docker_uri(stack_uri.delete_prefix('docker://')) + host, = parse_docker_uri(body) host.presence rescue StandardError nil diff --git a/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb b/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb index 73c1c59e457..78a466b2160 100644 --- a/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb @@ -4,32 +4,30 @@ module VCAP::CloudController module Diego RSpec.describe CustomStackUriConverter do - subject(:converter) { CustomStackUriConverter.new } - - describe '#convert' do + describe '.convert' do it 'converts docker://host/path:tag to Diego rootfs format' do - result = converter.convert('docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0') + result = CustomStackUriConverter.convert('docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0') expect(result).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') end it 'converts docker://host/path without tag (no fragment appended)' do - result = converter.convert('docker://docker.io/cloudfoundry/cflinuxfs4') + result = CustomStackUriConverter.convert('docker://docker.io/cloudfoundry/cflinuxfs4') expect(result).to eq('docker://docker.io/cloudfoundry/cflinuxfs4') end it 'handles private registries' do - result = converter.convert('docker://registry.example.com/my-org/my-stack:v2.0') + result = CustomStackUriConverter.convert('docker://registry.example.com/my-org/my-stack:v2.0') expect(result).to eq('docker://registry.example.com/my-org/my-stack#v2.0') end it 'handles images without explicit registry (docker hub shorthand)' do - result = converter.convert('docker://ubuntu:22.04') + result = CustomStackUriConverter.convert('docker://ubuntu:22.04') expect(result).to eq('docker:///library/ubuntu#22.04') end it 'handles digest references' do digest = 'sha256:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890' - result = converter.convert("docker://docker.io/cloudfoundry/cflinuxfs4@#{digest}") + result = CustomStackUriConverter.convert("docker://docker.io/cloudfoundry/cflinuxfs4@#{digest}") expect(result).to include('docker://docker.io/cloudfoundry/cflinuxfs4') end end diff --git a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb index c1e9a9c6a18..8eed4f5f027 100644 --- a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb @@ -6,7 +6,7 @@ module Diego RSpec.describe TaskRecipeBuilder do subject(:task_recipe_builder) { TaskRecipeBuilder.new } - describe '#staging_image_username / #staging_image_password (custom stacks)' do + describe '#staging_image_credentials (custom stacks)' do let(:custom_stack_uri) { 'docker://registry.example.com/my-org/my-stack:v1.0' } let(:lifecycle) do instance_double( @@ -28,12 +28,13 @@ module Diego end it 'returns credentials from the custom stack lifecycle' do - expect(task_recipe_builder.send(:staging_image_username, staging_details)).to eq('my-user') - expect(task_recipe_builder.send(:staging_image_password, staging_details)).to eq('my-pass') + username, password = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(username).to eq('my-user') + expect(password).to eq('my-pass') end end - describe '#staging_image_username / #staging_image_password (docker package)' do + describe '#staging_image_credentials (docker package)' do let(:lifecycle) do instance_double( BuildpackLifecycle, @@ -49,12 +50,13 @@ module Diego end it 'uses docker package credentials when both are present' do - expect(task_recipe_builder.send(:staging_image_username, staging_details)).to eq('docker-user') - expect(task_recipe_builder.send(:staging_image_password, staging_details)).to eq('docker-pass') + username, password = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(username).to eq('docker-user') + expect(password).to eq('docker-pass') end end - describe '#staging_image_username / #staging_image_password (mismatched docker credentials)' do + describe '#staging_image_credentials (mismatched docker credentials)' do let(:lifecycle) do instance_double( BuildpackLifecycle, @@ -75,12 +77,13 @@ module Diego end it 'falls through to custom stack credentials when docker pair is incomplete' do - expect(task_recipe_builder.send(:staging_image_username, staging_details)).to eq('stack-user') - expect(task_recipe_builder.send(:staging_image_password, staging_details)).to eq('stack-pass') + username, password = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(username).to eq('stack-user') + expect(password).to eq('stack-pass') end end - describe '#task_image_username / #task_image_password (custom stacks)' do + describe '#task_image_credentials (custom stacks)' do let(:lifecycle_data) do instance_double( BuildpackLifecycleDataModel, @@ -104,12 +107,13 @@ module Diego end it 'returns credentials from the custom stack lifecycle data' do - expect(task_recipe_builder.send(:task_image_username, task)).to eq('task-user') - expect(task_recipe_builder.send(:task_image_password, task)).to eq('task-pass') + username, password = task_recipe_builder.send(:task_image_credentials, task) + expect(username).to eq('task-user') + expect(password).to eq('task-pass') end end - describe '#task_image_username / #task_image_password (docker droplet)' do + describe '#task_image_credentials (docker droplet)' do let(:lifecycle_data) do instance_double(BuildpackLifecycleDataModel, stack: 'cflinuxfs4', credentials: nil) end @@ -124,8 +128,9 @@ module Diego end it 'uses docker receipt credentials when both are present' do - expect(task_recipe_builder.send(:task_image_username, task)).to eq('droplet-user') - expect(task_recipe_builder.send(:task_image_password, task)).to eq('droplet-pass') + username, password = task_recipe_builder.send(:task_image_credentials, task) + expect(username).to eq('droplet-user') + expect(password).to eq('droplet-pass') end end end From 52621f51230a14ee35a992a6bfb1de04ba4b8302 Mon Sep 17 00:00:00 2001 From: johha Date: Mon, 28 Sep 2026 16:21:28 +0200 Subject: [PATCH 3/5] Fix custom stacks specs and drop redundant deep_stringify_keys Migrate draft specs from Machinist (.make) to FactoryBot (create), and remove the redundant deep_stringify_keys in credentials=: persistence round-trips through Oj.dump/Oj.load, which stringifies keys regardless. Add a spec asserting symbol-keyed credentials read back with string keys. Co-authored-by: FloThinksPi <5863788+FloThinksPi@users.noreply.github.com> --- .../desired_lrp_builder_custom_stacks_spec.rb | 70 +++++++++---------- .../task_action_builder_custom_stacks_spec.rb | 20 +++--- .../diego/cnb/lifecycle_protocol_spec.rb | 21 ++++-- ...aging_action_builder_custom_stacks_spec.rb | 22 +++--- .../messages/app_manifest_message_spec.rb | 5 +- 5 files changed, 75 insertions(+), 63 deletions(-) diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb index d6bb894e50e..e17436b263c 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb @@ -9,13 +9,13 @@ module Buildpack let(:stack) { 'cflinuxfs4' } let(:config) do Config.new({ - diego: { - use_privileged_containers_for_running: false, - lifecycle_bundles: { "buildpack/#{stack}": 'http://lifecycle.example.com/bundle.tgz' }, - droplet_destinations: { stack.to_sym => '/home/vcap' }, - enable_declarative_asset_downloads: false - } - }) + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{stack}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { stack.to_sym => '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) end let(:opts) do { @@ -49,13 +49,13 @@ module Buildpack let(:stack) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } let(:config) do Config.new({ - diego: { - use_privileged_containers_for_running: false, - lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, - droplet_destinations: { 'default-stack-name': '/home/vcap' }, - enable_declarative_asset_downloads: false - } - }) + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) end it 'returns a docker:// rootfs URI in Diego format' do @@ -67,13 +67,13 @@ module Buildpack let(:stack) { 'docker://registry.example.com/my-org/my-stack:v2.0' } let(:config) do Config.new({ - diego: { - use_privileged_containers_for_running: false, - lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, - droplet_destinations: { 'default-stack-name': '/home/vcap' }, - enable_declarative_asset_downloads: false - } - }) + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) end it 'returns a docker:// rootfs URI for the private registry' do @@ -87,13 +87,13 @@ module Buildpack let(:stack) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } let(:config) do Config.new({ - diego: { - use_privileged_containers_for_running: false, - lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, - droplet_destinations: { 'default-stack-name': '/home/vcap' }, - enable_declarative_asset_downloads: false - } - }) + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) end it 'uses the default stack lifecycle bundle' do @@ -107,13 +107,13 @@ module Buildpack describe '#image_layers' do let(:config) do Config.new({ - diego: { - use_privileged_containers_for_running: false, - lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, - droplet_destinations: { 'default-stack-name': '/home/vcap' }, - enable_declarative_asset_downloads: true - } - }) + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: true + } + }) end context 'with a custom stack' do diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb index 23357587005..db953392131 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb @@ -9,13 +9,13 @@ module Buildpack let(:stack_name) { 'cflinuxfs4' } let(:config) do Config.new({ - diego: { - use_privileged_containers_for_running: false, - lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, - droplet_destinations: { 'default-stack-name': '/home/vcap' }, - enable_declarative_asset_downloads: false - } - }) + diego: { + use_privileged_containers_for_running: false, + lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false + } + }) end let(:lifecycle_data) do { @@ -25,8 +25,8 @@ module Buildpack end let(:task) do instance_double(TaskModel, - name: 'my-task', - droplet: instance_double(DropletModel, sha256_checksum: 'abc123', droplet_hash: 'hash')) + name: 'my-task', + droplet: instance_double(DropletModel, sha256_checksum: 'abc123', droplet_hash: 'hash')) end subject(:builder) do @@ -56,7 +56,7 @@ module Buildpack describe '#lifecycle_bundle_key' do context 'with a system stack' do it 'uses the stack name' do - expect(builder.lifecycle_bundle_key).to eq(:"buildpack/cflinuxfs4") + expect(builder.lifecycle_bundle_key).to eq(:'buildpack/cflinuxfs4') end end diff --git a/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb index 5f2258cc7cb..e039ab4454a 100644 --- a/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb @@ -29,8 +29,11 @@ module CNB Diego::StagingDetails.new.tap do |details| details.staging_guid = droplet.guid details.package = package - details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, staging_message: double('msg', buildpack_data: double('bd', requested?: false)), - credentials: '{"registry":{"username":"password"}}') + details.lifecycle = instance_double(CNBLifecycle, + staging_stack: 'potato-stack', + buildpack_infos: buildpack_infos, + staging_message: double('msg', buildpack_data: double('bd', requested?: false)), + credentials: '{"registry":{"username":"password"}}') end end let(:buildpack_infos) { [BuildpackInfo.new('http://some-buildpack.url', nil)] } @@ -60,8 +63,11 @@ module CNB details.environment_variables = { 'nightshade_fruit' => 'potato' } details.staging_memory_in_mb = 42 details.staging_disk_in_mb = 51 - details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, staging_message: double('msg', buildpack_data: double('bd', requested?: false)), - credentials: '{"registry":{"username":"password"}}') + details.lifecycle = instance_double(CNBLifecycle, + staging_stack: 'potato-stack', + buildpack_infos: buildpack_infos, + staging_message: double('msg', buildpack_data: double('bd', requested?: false)), + credentials: '{"registry":{"username":"password"}}') end end @@ -149,8 +155,11 @@ module CNB let(:droplet) { create(:droplet_model, :cnb) } let(:staging_details) do StagingDetails.new.tap do |details| - details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: 'some buildpack info', staging_message: double('msg', buildpack_data: double('bd', requested?: false)), - credentials: '{"registry":{"username":"password"}}') + details.lifecycle = instance_double(CNBLifecycle, + staging_stack: 'potato-stack', + buildpack_infos: 'some buildpack info', + staging_message: double('msg', buildpack_data: double('bd', requested?: false)), + credentials: '{"registry":{"username":"password"}}') details.package = package details.staging_guid = droplet.guid end diff --git a/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb index 4e9bcef4707..7395118dc32 100644 --- a/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb @@ -19,17 +19,17 @@ module CNB end let(:config) do Config.new({ - diego: { - lifecycle_bundles: { 'cnb/default-stack-name': 'http://lifecycle.example.com/cnb-bundle.tgz' }, - droplet_destinations: { 'default-stack-name': '/home/vcap' }, - enable_declarative_asset_downloads: false, - cc_uploader_url: 'http://uploader.example.com' - }, - staging: { - timeout_in_seconds: 900, - legacy_md5_buildpack_paths_enabled: false - } - }) + diego: { + lifecycle_bundles: { 'cnb/default-stack-name': 'http://lifecycle.example.com/cnb-bundle.tgz' }, + droplet_destinations: { 'default-stack-name': '/home/vcap' }, + enable_declarative_asset_downloads: false, + cc_uploader_url: 'http://uploader.example.com' + }, + staging: { + timeout_in_seconds: 900, + legacy_md5_buildpack_paths_enabled: false + } + }) end let(:staging_details) { instance_double(StagingDetails, staging_guid: 'staging-guid') } diff --git a/spec/unit/messages/app_manifest_message_spec.rb b/spec/unit/messages/app_manifest_message_spec.rb index 7909fd0e338..3b4eeb9e0b3 100644 --- a/spec/unit/messages/app_manifest_message_spec.rb +++ b/spec/unit/messages/app_manifest_message_spec.rb @@ -2181,7 +2181,10 @@ 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.example.com' => { 'username' => 'user', 'password' => 'pass' } } } } + let(:parsed_yaml) do + { name: 'cnb', lifecycle: 'cnb', buildpacks: %w[nodejs java], stack: stack.name, + cnb_credentials: { 'registry.example.com' => { 'username' => 'user', 'password' => 'pass' } } } + end it 'adds credentials to the lifecycle_data' do message = AppManifestMessage.create_from_yml(parsed_yaml) From 09892b6ce0f12563e13aa284a587418289ed3619 Mon Sep 17 00:00:00 2001 From: johha Date: Wed, 30 Sep 2026 10:42:54 +0200 Subject: [PATCH 4/5] Harden custom-stacks credential handling - Redact lifecycle data.credentials in recorded app events - Reject a token keyed to the stack registry host (Diego pulls the stack via HTTP Basic auth only; a token cannot be used) - Reject credentials embedded in the stack URI - Drop the unused process arg from lrp_image_credentials --- .../diego/app_recipe_builder.rb | 6 +++--- .../repositories/app_event_repository_spec.rb | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index e6cdfe61562..364df1c3ef6 100644 --- a/lib/cloud_controller/diego/app_recipe_builder.rb +++ b/lib/cloud_controller/diego/app_recipe_builder.rb @@ -103,15 +103,15 @@ def app_lrp_arguments certificate_properties: ::Diego::Bbs::Models::CertificateProperties.new( organizational_unit: ["organization:#{process.organization.guid}", "space:#{process.space.guid}", "app:#{process.app_guid}"] ), - image_username: lrp_image_credentials(process)[0], - image_password: lrp_image_credentials(process)[1], + image_username: lrp_image_credentials[0], + image_password: lrp_image_credentials[1], volume_mounted_files: ServiceBindingFilesBuilder.build(process) }.compact rescue ServiceBindingFilesBuilder::IncompatibleBindings => e raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', "Cannot build service binding files for app - #{e.message}") end - def lrp_image_credentials(process) + def lrp_image_credentials @lrp_image_credentials ||= begin droplet = process.desired_droplet ImageCredentialResolver.resolve( diff --git a/spec/unit/repositories/app_event_repository_spec.rb b/spec/unit/repositories/app_event_repository_spec.rb index e73ac02b304..e167381cca7 100644 --- a/spec/unit/repositories/app_event_repository_spec.rb +++ b/spec/unit/repositories/app_event_repository_spec.rb @@ -662,6 +662,27 @@ module Repositories end end end + + context 'v3 lifecycle registry credentials' do + let(:app) { create(:app_model) } + let(:attrs) do + { + 'lifecycle' => { + 'type' => 'buildpack', + 'data' => { + 'stack' => 'docker://registry.example.com/cloudfoundry/cflinuxfs4:1.0.0', + 'credentials' => { 'registry.example.com' => { 'username' => 'user', 'password' => 'pass' } } + } + } + } + end + + it 'redacts the credentials in the recorded event' do + event = app_event_repository.record_app_create(app, space, user_audit_info, attrs).reload + + expect(event.metadata.dig('request', 'lifecycle', 'data', 'credentials')).to eq(Presenters::Censorship::PRIVATE_DATA_HIDDEN) + end + end end context 'with a v3 app' do From 15f6b1cddc719459dfb2ccc6d0992a95bb138433 Mon Sep 17 00:00:00 2001 From: johha Date: Wed, 21 Oct 2026 15:05:18 +0200 Subject: [PATCH 5/5] Refine custom stack support --- app/controllers/v3/apps_controller.rb | 21 ++- app/fetchers/buildpack_lifecycle_fetcher.rb | 26 ++-- .../buildpack_lifecycle_data_message.rb | 7 +- .../includes/concepts/_lifecycles.md.erb | 9 +- .../resources/manifests/_object.md.erb | 4 +- .../diego/app_recipe_builder.rb | 1 - .../diego/buildpack/desired_lrp_builder.rb | 23 ++-- .../diego/buildpack/task_action_builder.rb | 19 +-- .../diego/cnb/desired_lrp_builder.rb | 23 ++-- .../diego/cnb/lifecycle_data.rb | 4 +- .../diego/cnb/lifecycle_protocol.rb | 3 - .../diego/cnb/staging_action_builder.rb | 15 +-- .../diego/custom_stack_fallback.rb | 13 -- .../diego/custom_stack_uri_converter.rb | 16 --- .../diego/image_credential_resolver.rb | 14 +- .../diego/lifecycle_bundle_stack_resolver.rb | 13 ++ .../diego/lifecycle_protocol.rb | 9 +- .../diego/lifecycles/app_base_lifecycle.rb | 4 + .../buildpack_lifecycle_data_validator.rb | 37 +++++- .../diego/lifecycles/lifecycle_base.rb | 1 + .../diego/staging_action_builder.rb | 23 ++-- lib/utils/uri_utils.rb | 13 +- .../controllers/v3/apps_controller_spec.rb | 87 +++++++++++++ .../buildpack_lifecycle_fetcher_spec.rb | 18 +++ .../diego/app_recipe_builder_spec.rb | 15 +++ .../desired_lrp_builder_custom_stacks_spec.rb | 25 ---- .../buildpack/lifecycle_protocol_spec.rb | 16 ++- .../buildpack/staging_action_builder_spec.rb | 9 ++ .../task_action_builder_custom_stacks_spec.rb | 86 ------------- .../buildpack/task_action_builder_spec.rb | 28 +++- ...aging_action_builder_custom_stacks_spec.rb | 120 ------------------ .../diego/cnb/staging_action_builder_spec.rb | 17 +++ .../diego/custom_stack_uri_converter_spec.rb | 36 ------ .../app_buildpack_lifecycle_spec.rb | 18 +++ .../lifecycles/app_cnb_lifecycle_spec.rb | 16 +++ ...cycle_data_validator_custom_stacks_spec.rb | 69 +++++++++- .../lifecycles/buildpack_lifecycle_spec.rb | 2 + .../task_recipe_builder_custom_stacks_spec.rb | 51 ++++++++ .../lib/utils/uri_utils_custom_stack_spec.rb | 40 ------ spec/unit/lib/utils/uri_utils_spec.rb | 14 ++ .../messages/app_manifest_message_spec.rb | 21 +++ ...fecycle_data_message_custom_stacks_spec.rb | 40 ------ .../feature_flag_custom_stacks_spec.rb | 38 ------ spec/unit/models/runtime/feature_flag_spec.rb | 2 +- 44 files changed, 535 insertions(+), 531 deletions(-) delete mode 100644 lib/cloud_controller/diego/custom_stack_fallback.rb delete mode 100644 lib/cloud_controller/diego/custom_stack_uri_converter.rb create mode 100644 lib/cloud_controller/diego/lifecycle_bundle_stack_resolver.rb delete mode 100644 spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb delete mode 100644 spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb delete mode 100644 spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb delete mode 100644 spec/unit/lib/utils/uri_utils_custom_stack_spec.rb delete mode 100644 spec/unit/messages/buildpack_lifecycle_data_message_custom_stacks_spec.rb delete mode 100644 spec/unit/models/runtime/feature_flag_custom_stacks_spec.rb diff --git a/app/controllers/v3/apps_controller.rb b/app/controllers/v3/apps_controller.rb index caf42077e53..0b6f385a858 100644 --- a/app/controllers/v3/apps_controller.rb +++ b/app/controllers/v3/apps_controller.rb @@ -98,7 +98,7 @@ def create FeatureFlag.raise_unless_enabled!(:diego_docker) if message.lifecycle_type == VCAP::CloudController::PackageModel::DOCKER_TYPE lifecycle = AppLifecycleProvider.provide_for_create(message) FeatureFlag.raise_unless_enabled!(:diego_cnb) if lifecycle.type == VCAP::CloudController::Lifecycles::CNB - raise_unless_custom_stacks_enabled!(message) + raise_unless_custom_stack_enabled!(custom_stack_from_message(message)) unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid? app_creator = AppCreate.new(user_audit_info) @@ -129,6 +129,7 @@ def update require_writable_space!(space) lifecycle = AppLifecycleProvider.provide_for_update(message, app) + raise_unless_custom_stack_enabled!(custom_stack_from_message(message)) unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid? app_updater = AppUpdate.new(user_audit_info) @@ -189,7 +190,7 @@ def start FeatureFlag.raise_unless_enabled!(:diego_docker) if app.lifecycle_type == DockerLifecycleDataModel::LIFECYCLE_TYPE FeatureFlag.raise_unless_enabled!(:diego_cnb) if app.lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE - raise_unless_custom_stacks_enabled!(app.lifecycle_data.stack) if app.lifecycle_data.respond_to?(:stack) + raise_unless_custom_stack_enabled!(app.lifecycle_data.stack) if app.lifecycle_data.respond_to?(:stack) AppStart.start(app:, user_audit_info:) TelemetryLogger.v3_emit( @@ -233,7 +234,7 @@ def restart FeatureFlag.raise_unless_enabled!(:diego_docker) if app.lifecycle_type == DockerLifecycleDataModel::LIFECYCLE_TYPE FeatureFlag.raise_unless_enabled!(:diego_cnb) if app.lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE - raise_unless_custom_stacks_enabled!(app.lifecycle_data.stack) if app.lifecycle_data.respond_to?(:stack) + raise_unless_custom_stack_enabled!(app.lifecycle_data.stack) if app.lifecycle_data.respond_to?(:stack) AppRestart.restart(app: app, config: Config.config, user_audit_info: user_audit_info) TelemetryLogger.v3_emit( @@ -400,18 +401,14 @@ def translate_error(e) private - def raise_unless_custom_stacks_enabled!(message_or_stack) - stack = case message_or_stack - when String - message_or_stack - when nil - nil - else - message_or_stack.lifecycle_data&.dig(:stack) - end + def raise_unless_custom_stack_enabled!(stack) FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if stack && UriUtils.is_custom_stack_uri?(stack) end + def custom_stack_from_message(message) + message.lifecycle_data&.dig(:stack) + end + def async_recursive_delete_enabled? !!VCAP::CloudController::Config.config.get(:temporary_enable_async_recursive_delete, :apps) end diff --git a/app/fetchers/buildpack_lifecycle_fetcher.rb b/app/fetchers/buildpack_lifecycle_fetcher.rb index 50f9a688ccf..0edb210280e 100644 --- a/app/fetchers/buildpack_lifecycle_fetcher.rb +++ b/app/fetchers/buildpack_lifecycle_fetcher.rb @@ -4,21 +4,25 @@ module VCAP::CloudController class BuildpackLifecycleFetcher class << self def fetch(buildpack_names, stack_name, lifecycle=Config.config.get(:default_app_lifecycle)) - if UriUtils.is_custom_stack_uri?(stack_name) - { - stack: nil, - buildpack_infos: Array(buildpack_names).map { |name| BuildpackInfo.new(name, nil) } - } - else - { - stack: Stack.find(name: stack_name), - buildpack_infos: ordered_buildpacks(buildpack_names, stack_name, lifecycle) - } - end + return { stack: nil, buildpack_infos: custom_stack_buildpacks(buildpack_names, lifecycle) } if UriUtils.is_custom_stack_uri?(stack_name) + + { + stack: Stack.find(name: stack_name), + buildpack_infos: ordered_buildpacks(buildpack_names, stack_name, lifecycle) + } end private + def custom_stack_buildpacks(buildpack_names, lifecycle) + buildpacks_without_stacks = Buildpack.list_admin_buildpacks(nil, lifecycle).select { |buildpack| buildpack.stack.nil? } + + Array(buildpack_names).map do |buildpack_name| + buildpack_record = buildpacks_without_stacks.find { |buildpack| buildpack.name == buildpack_name } + BuildpackInfo.new(buildpack_name, buildpack_record) + end + end + def ordered_buildpacks(buildpack_names, stack_name, lifecycle) buildpacks_with_stacks, buildpacks_without_stacks = Buildpack.list_admin_buildpacks(stack_name, lifecycle).partition(&:stack) diff --git a/app/messages/buildpack_lifecycle_data_message.rb b/app/messages/buildpack_lifecycle_data_message.rb index 5fdf4ab7767..66bf5ed234a 100644 --- a/app/messages/buildpack_lifecycle_data_message.rb +++ b/app/messages/buildpack_lifecycle_data_message.rb @@ -1,6 +1,6 @@ module VCAP::CloudController class BuildpackLifecycleDataMessage < BaseMessage - register_allowed_keys %i[buildpacks stack stack_id credentials] + register_allowed_keys %i[buildpacks stack credentials] validates_with NoAdditionalKeysValidator @@ -9,11 +9,6 @@ class BuildpackLifecycleDataMessage < BaseMessage allow_nil: true, length: { in: 1..4096, message: 'must be between 1 and 4096 characters' } - validates :stack_id, - string: true, - allow_nil: true, - length: { in: 1..4096, message: 'must be between 1 and 4096 characters' } - validates :buildpacks, array: true, allow_nil: true diff --git a/docs/v3/source/includes/concepts/_lifecycles.md.erb b/docs/v3/source/includes/concepts/_lifecycles.md.erb index 979dec0649c..a5216b29112 100644 --- a/docs/v3/source/includes/concepts/_lifecycles.md.erb +++ b/docs/v3/source/includes/concepts/_lifecycles.md.erb @@ -40,8 +40,8 @@ Name | Type | Description ---- | ---- | ----------- **type** | _string_ | `buildpack` **data.buildpacks** | _list of strings_ | A list of the names of buildpacks, URLs from which they may be downloaded, or `null` to auto-detect a suitable buildpack during staging -**data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4`. Can also be a container image URI with `docker://` scheme (e.g. `docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0`) when the `diego_custom_stacks` feature flag is enabled. When using a custom stack, all buildpacks must be custom (URL-based). -**data.credentials** | _object_ | Credentials used to download custom buildpacks from private git repositories, keyed by host. Each entry is a username/password pair. If any buildpack URL embeds credentials, **data.credentials** must be empty; provide credentials in one place only. +**data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4`. Can also be a container image URI with `docker://` scheme (e.g. `docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0`) when the `diego_custom_stacks` feature flag is enabled. When using a custom stack, buildpacks must be custom (URL-based) or enabled admin buildpacks without a pinned stack. If no buildpacks are specified, at least one enabled unpinned admin buildpack must exist for auto-detection. +**data.credentials** | _object_ | Credentials used to download custom buildpacks from private git repositories or pull a custom stack from a private registry, keyed by host. Each entry is a username/password pair. If any buildpack URL embeds credentials, **data.credentials** must be empty; provide credentials in one place only. For records whose historical lifecycle data is no longer available, **data.buildpacks** may be an empty list and **data.stack** may be `null`. @@ -84,9 +84,8 @@ Name | Type | Description ---- | ---- | ----------- **type** | _string_ | `cnb` **data.buildpacks** | _list of strings_ | A list of URLs with either `docker://` or `http(s)://` scheme, pointing to the Cloud Native Buildpack OCI image.
When the scheme is `http(s)://`, an OCI tarball is expected to be present at the specified location. -**data.credentials** | _object_ | Credentials used to download the configured buildpacks, keyed by registry host. Each entry contains either a username/password pair or a token. CNB buildpack URLs must not embed credentials; use **data.credentials** instead. -**data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4`. Can also be a container image URI with `docker://` scheme when the `diego_custom_stacks` feature flag is enabled. -**data.stack_id** | _string_ | _(optional)_ Explicit CNB stack identifier (e.g. `io.buildpacks.stacks.jammy`). When omitted with a custom stack, the CNB builder will auto-detect the stack from the run image metadata. +**data.credentials** | _object_ | Credentials used to download the configured buildpacks or pull a custom stack, keyed by registry host. Each entry contains either a username/password pair or a token. Only a username/password pair matching a custom stack's registry is sent to Diego to pull the stack; token entries are used for CNB buildpacks. CNB buildpack URLs must not embed credentials; use **data.credentials** instead. +**data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4`. Can also be a container image URI with `docker://` scheme when the `diego_custom_stacks` feature flag is enabled. When using a custom stack, buildpacks must be custom (URL-based) or enabled admin buildpacks without a pinned stack. If no buildpacks are specified, at least one enabled unpinned CNB admin buildpack must exist for auto-detection. For records whose historical lifecycle data is no longer available, **data.buildpacks** may be an empty list and **data.stack** may be `null`. diff --git a/docs/v3/source/includes/resources/manifests/_object.md.erb b/docs/v3/source/includes/resources/manifests/_object.md.erb index 8727c331be8..67e712d0c70 100644 --- a/docs/v3/source/includes/resources/manifests/_object.md.erb +++ b/docs/v3/source/includes/resources/manifests/_object.md.erb @@ -105,7 +105,7 @@ Name | Type | Description **name** | _string_ | Name of the app **lifecycle** | _string_ | Type of the lifecycle; valid values are `buildpack`, `cnb`, `docker`. Defaults to `buildpack`, unless the **docker** property is present. **buildpacks** | _list of strings_ | a) An empty array, which will automatically select the appropriate default buildpack according to the coding language (incompatible with **lifecycle: cnb**)
b) An array of one or more URLs pointing to buildpacks
c) An array of one or more installed buildpack names
Replaces the legacy `buildpack` field -**credentials** | _object_ | Credentials used to download custom buildpacks (buildpack lifecycle) or buildpack OCI images (cnb lifecycle) from private git repositories or registries, keyed by host. For the buildpack lifecycle each entry is a username/password pair; for the cnb lifecycle each entry is a username/password pair or a token. Set to `null` to clear previously stored credentials. If any buildpack URL embeds credentials, **credentials** must be empty; provide credentials in one place only. +**credentials** | _object_ | Credentials used to download custom buildpacks (buildpack lifecycle), buildpack OCI images (cnb lifecycle), or pull a custom stack from a private registry, keyed by host. For the buildpack lifecycle each entry is a username/password pair; for the cnb lifecycle each entry is a username/password pair or a token. Only a username/password pair matching a custom stack's registry is sent to Diego to pull the stack. Set to `null` to clear previously stored credentials. If any buildpack URL embeds credentials, **credentials** must be empty; provide credentials in one place only. **cnb-credentials** | _object_ | Backward-compatible alias for **credentials**. Specifying both **credentials** and **cnb-credentials** is an error. **docker** | _object_ | If present, the created app will have Docker lifecycle type; the value of this key is ignored by the API but may be used by clients to source the registry address of the image and credentials, if needed; the [generate manifest endpoint](#generate-a-manifest-for-an-app) will return the registry address of the image and username provided with this key **env** | _object_ | A key-value mapping of environment variables to be used for the app when running @@ -117,7 +117,7 @@ Name | Type | Description **routes** | _array of [route configurations](#space-manifest-route-level-configuration)_ | List declaring HTTP and TCP routes to be mapped to the app. **services** | _array of [service configurations](#space-manifest-service-level-configuration)_ | A list of service-instances to bind to the app **sidecars** | _array of [sidecar configurations](#space-manifest-sidecar-level-configuration)_ | A list of configurations for individual sidecars -**stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4` +**stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4`. When the `diego_custom_stacks` feature flag is enabled, this can also be a `docker://` container image URI. Custom stack URIs must not include credentials; use **credentials** instead. When using a custom stack with the buildpack lifecycle, buildpacks must be custom (URL-based) or admin buildpacks without a pinned stack. **metadata.labels** | _[label object](#labels)_ | Labels applied to the app **metadata.annotations** | _[annotation object](#annotations)_ | Annotations applied to the app **buildpack** | _string_ | **DEPRECATED in favor of the `buildpacks` field above** diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index 364df1c3ef6..039d8fae872 100644 --- a/lib/cloud_controller/diego/app_recipe_builder.rb +++ b/lib/cloud_controller/diego/app_recipe_builder.rb @@ -4,7 +4,6 @@ require 'cloud_controller/diego/buildpack/desired_lrp_builder' require 'cloud_controller/diego/docker/desired_lrp_builder' require 'cloud_controller/diego/cnb/desired_lrp_builder' -require 'cloud_controller/diego/custom_stack_uri_converter' require 'cloud_controller/diego/image_credential_resolver' require 'cloud_controller/diego/process_guid' require 'cloud_controller/diego/ssh_key' diff --git a/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb b/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb index 1e10a202a56..7e81e558d0a 100644 --- a/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb @@ -1,5 +1,5 @@ -require 'cloud_controller/diego/custom_stack_uri_converter' -require 'cloud_controller/diego/custom_stack_fallback' +require 'cloud_controller/diego/docker/docker_uri_converter' +require 'cloud_controller/diego/lifecycle_bundle_stack_resolver' require 'utils/uri_utils' module VCAP::CloudController @@ -7,7 +7,6 @@ module Diego module Buildpack class DesiredLrpBuilder include ::Diego::ActionBuilder - include CustomStackFallback class InvalidStack < StandardError; end @@ -30,7 +29,6 @@ def initialize(config, opts) def cached_dependencies return nil if @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = resolved_lifecycle_bundle_key lifecycle_bundle = @config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key] raise InvalidStack.new("no compiler defined for requested stack '#{@stack}'") unless lifecycle_bundle @@ -38,13 +36,13 @@ def cached_dependencies ::Diego::Bbs::Models::CachedDependency.new( from: LifecycleBundleUriGenerator.uri(lifecycle_bundle), to: '/tmp/lifecycle', - cache_key: "buildpack-#{resolved_stack_name}-lifecycle" + cache_key: "buildpack-#{lifecycle_bundle_stack}-lifecycle" ) ] end def root_fs - return CustomStackUriConverter.convert(@stack) if UriUtils.is_custom_stack_uri?(@stack) + return DockerURIConverter.new.convert(@stack.delete_prefix('docker://')) if UriUtils.is_custom_stack_uri?(@stack) @stack_obj ||= Stack.find(name: @stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', @stack) unless @stack_obj @@ -71,16 +69,15 @@ def setup def image_layers return [] unless @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = resolved_lifecycle_bundle_key lifecycle_bundle = @config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key] raise InvalidStack.new("no compiler defined for requested stack '#{@stack}'") unless lifecycle_bundle - destination = @config.get(:diego, :droplet_destinations)[resolved_stack_name.to_sym] + destination = @config.get(:diego, :droplet_destinations)[lifecycle_bundle_stack.to_sym] raise InvalidStack.new("no droplet destination defined for requested stack '#{@stack}'") unless destination layers = [ ::Diego::Bbs::Models::ImageLayer.new( - name: "buildpack-#{resolved_stack_name}-lifecycle", + name: "buildpack-#{lifecycle_bundle_stack}-lifecycle", url: LifecycleBundleUriGenerator.uri(lifecycle_bundle), destination_path: '/tmp/lifecycle', layer_type: ::Diego::Bbs::Models::ImageLayer::Type::SHARED, @@ -127,8 +124,12 @@ def privileged? private - def resolved_lifecycle_bundle_key - :"buildpack/#{resolved_stack_name}" + def lifecycle_bundle_key + :"buildpack/#{lifecycle_bundle_stack}" + end + + def lifecycle_bundle_stack + LifecycleBundleStackResolver.resolve(@stack) end end end diff --git a/lib/cloud_controller/diego/buildpack/task_action_builder.rb b/lib/cloud_controller/diego/buildpack/task_action_builder.rb index 43233c50d17..d1ca7a82ef6 100644 --- a/lib/cloud_controller/diego/buildpack/task_action_builder.rb +++ b/lib/cloud_controller/diego/buildpack/task_action_builder.rb @@ -1,7 +1,7 @@ require 'diego/action_builder' require 'cloud_controller/diego/task_environment_variable_collector' -require 'cloud_controller/diego/custom_stack_uri_converter' -require 'cloud_controller/diego/custom_stack_fallback' +require 'cloud_controller/diego/docker/docker_uri_converter' +require 'cloud_controller/diego/lifecycle_bundle_stack_resolver' require 'credhub/config_helpers' module VCAP::CloudController @@ -9,7 +9,6 @@ module Diego module Buildpack class TaskActionBuilder include ::Diego::ActionBuilder - include CustomStackFallback class InvalidStack < StandardError; end @@ -61,12 +60,12 @@ def action def image_layers return [] unless @config.get(:diego, :enable_declarative_asset_downloads) - destination = @config.get(:diego, :droplet_destinations)[resolved_stack_name.to_sym] + destination = @config.get(:diego, :droplet_destinations)[lifecycle_bundle_stack.to_sym] raise InvalidStack.new("no droplet destination defined for requested stack '#{lifecycle_stack}'") unless destination layers = [ ::Diego::Bbs::Models::ImageLayer.new( - name: "buildpack-#{resolved_stack_name}-lifecycle", + name: "buildpack-#{lifecycle_bundle_stack}-lifecycle", url: LifecycleBundleUriGenerator.uri(config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key]), destination_path: '/tmp/lifecycle', layer_type: ::Diego::Bbs::Models::ImageLayer::Type::SHARED, @@ -94,7 +93,7 @@ def task_environment_variables end def stack - return CustomStackUriConverter.convert(lifecycle_stack) if UriUtils.is_custom_stack_uri?(lifecycle_stack) + return DockerURIConverter.new.convert(lifecycle_stack.delete_prefix('docker://')) if UriUtils.is_custom_stack_uri?(lifecycle_stack) @stack ||= Stack.find(name: lifecycle_stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', lifecycle_stack) unless @stack @@ -108,12 +107,12 @@ def cached_dependencies [::Diego::Bbs::Models::CachedDependency.new( from: LifecycleBundleUriGenerator.uri(config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key]), to: '/tmp/lifecycle', - cache_key: "#{@prefix}-#{resolved_stack_name}-lifecycle" + cache_key: "#{@prefix}-#{lifecycle_bundle_stack}-lifecycle" )] end def lifecycle_bundle_key - :"#{@prefix}/#{resolved_stack_name}" + :"#{@prefix}/#{lifecycle_bundle_stack}" end private @@ -123,6 +122,10 @@ def lifecycle_bundle_key def lifecycle_stack lifecycle_data[:stack] end + + def lifecycle_bundle_stack + LifecycleBundleStackResolver.resolve(lifecycle_stack) + end end end end diff --git a/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb b/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb index b54cf1cdd49..08643e458c4 100644 --- a/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb @@ -1,5 +1,5 @@ -require 'cloud_controller/diego/custom_stack_uri_converter' -require 'cloud_controller/diego/custom_stack_fallback' +require 'cloud_controller/diego/docker/docker_uri_converter' +require 'cloud_controller/diego/lifecycle_bundle_stack_resolver' require 'utils/uri_utils' module VCAP::CloudController @@ -7,7 +7,6 @@ module Diego module CNB class DesiredLrpBuilder include ::Diego::ActionBuilder - include CustomStackFallback class InvalidStack < StandardError; end @@ -30,7 +29,6 @@ def initialize(config, opts) def cached_dependencies return nil if @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"cnb/#{resolved_stack_name}" lifecycle_bundle = @config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key] raise InvalidStack.new("no compiler defined for requested stack '#{@stack}'") unless lifecycle_bundle @@ -38,13 +36,13 @@ def cached_dependencies ::Diego::Bbs::Models::CachedDependency.new( from: LifecycleBundleUriGenerator.uri(lifecycle_bundle), to: '/tmp/lifecycle', - cache_key: "cnb-#{resolved_stack_name}-lifecycle" + cache_key: "cnb-#{lifecycle_bundle_stack}-lifecycle" ) ] end def root_fs - return CustomStackUriConverter.convert(@stack) if UriUtils.is_custom_stack_uri?(@stack) + return DockerURIConverter.new.convert(@stack.delete_prefix('docker://')) if UriUtils.is_custom_stack_uri?(@stack) @stack_obj ||= Stack.find(name: @stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', @stack) unless @stack_obj @@ -71,16 +69,15 @@ def setup def image_layers return [] unless @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"cnb/#{resolved_stack_name}" lifecycle_bundle = @config.get(:diego, :lifecycle_bundles)[lifecycle_bundle_key] raise InvalidStack.new("no compiler defined for requested stack '#{@stack}'") unless lifecycle_bundle - destination = @config.get(:diego, :droplet_destinations)[resolved_stack_name.to_sym] + destination = @config.get(:diego, :droplet_destinations)[lifecycle_bundle_stack.to_sym] raise InvalidStack.new("no droplet destination defined for requested stack '#{@stack}'") unless destination layers = [ ::Diego::Bbs::Models::ImageLayer.new( - name: "cnb-#{resolved_stack_name}-lifecycle", + name: "cnb-#{lifecycle_bundle_stack}-lifecycle", url: LifecycleBundleUriGenerator.uri(lifecycle_bundle), destination_path: '/tmp/lifecycle', layer_type: ::Diego::Bbs::Models::ImageLayer::Type::SHARED, @@ -125,6 +122,14 @@ def privileged? private + def lifecycle_bundle_key + :"cnb/#{lifecycle_bundle_stack}" + end + + def lifecycle_bundle_stack + LifecycleBundleStackResolver.resolve(@stack) + end + def default_container_env [ ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'LANG', value: DEFAULT_LANG), diff --git a/lib/cloud_controller/diego/cnb/lifecycle_data.rb b/lib/cloud_controller/diego/cnb/lifecycle_data.rb index ac399bbd80b..63ff1fd642c 100644 --- a/lib/cloud_controller/diego/cnb/lifecycle_data.rb +++ b/lib/cloud_controller/diego/cnb/lifecycle_data.rb @@ -4,7 +4,7 @@ module CNB class LifecycleData attr_accessor :app_bits_download_uri, :build_artifacts_cache_download_uri, :build_artifacts_cache_upload_uri, :buildpacks, :app_bits_checksum, :droplet_upload_uri, :stack, :buildpack_cache_checksum, - :credentials, :auto_detect, :stack_id + :credentials, :auto_detect def message message = { @@ -19,7 +19,6 @@ def message message[:build_artifacts_cache_download_uri] = build_artifacts_cache_download_uri if build_artifacts_cache_download_uri message[:buildpack_cache_checksum] = buildpack_cache_checksum if buildpack_cache_checksum message[:credentials] = credentials if credentials - message[:stack_id] = stack_id if stack_id schema.validate(message) message @@ -39,7 +38,6 @@ def schema stack: String, app_bits_checksum: Hash, optional(:credentials) => String, - optional(:stack_id) => String, auto_detect: bool } end diff --git a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb index 65101cc8011..dbe75fa2714 100644 --- a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb @@ -27,9 +27,6 @@ def new_lifecycle_data(staging_details) lifecycle_data.credentials = staging_details.lifecycle.credentials lifecycle_data.auto_detect = staging_details.lifecycle.buildpack_infos.empty? - bp_data = staging_details.lifecycle.staging_message.buildpack_data - lifecycle_data.stack_id = bp_data.stack_id if bp_data.requested?(:stack_id) - lifecycle_data end diff --git a/lib/cloud_controller/diego/cnb/staging_action_builder.rb b/lib/cloud_controller/diego/cnb/staging_action_builder.rb index 0bf60afb93c..a73fef929fa 100644 --- a/lib/cloud_controller/diego/cnb/staging_action_builder.rb +++ b/lib/cloud_controller/diego/cnb/staging_action_builder.rb @@ -17,7 +17,12 @@ def task_environment_variables ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_GROUP_ID', value: '2000'), ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'LANG', value: STAGING_DEFAULT_LANG) ] - cnb_stack_id = resolve_cnb_stack_id + # TODO: Verify this compatibility fallback end-to-end with archived stacks and + # buildpacks. Legacy CNB buildpacks require CNB_STACK_ID, while newer buildpacks + # use target metadata. For custom stacks, the full image URI is the only stable + # value currently persisted by Cloud Controller. Strict legacy buildpacks may + # require an explicit logical stack ID instead, which would need a different API. + cnb_stack_id = lifecycle_stack env.push(::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_STACK_ID', value: cnb_stack_id)) if cnb_stack_id env.push(::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_REGISTRY_CREDS', value: lifecycle_data[:credentials])) if lifecycle_data[:credentials] env @@ -25,14 +30,6 @@ def task_environment_variables private - def resolve_cnb_stack_id - if UriUtils.is_custom_stack_uri?(lifecycle_stack) - lifecycle_data[:stack_id] # nil means auto-detect - else - lifecycle_stack - end - end - def stage_action args = [ '--cache-dir', '/tmp/cache', diff --git a/lib/cloud_controller/diego/custom_stack_fallback.rb b/lib/cloud_controller/diego/custom_stack_fallback.rb deleted file mode 100644 index 2e5d27d72cf..00000000000 --- a/lib/cloud_controller/diego/custom_stack_fallback.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'utils/uri_utils' - -module VCAP::CloudController - module Diego - # Custom stacks reuse the default stack's lifecycle binaries. - module CustomStackFallback - def resolved_stack_name(stack=nil) - stack ||= @stack || (respond_to?(:lifecycle_stack, true) ? lifecycle_stack : nil) - UriUtils.is_custom_stack_uri?(stack) ? VCAP::CloudController::Stack.default.name : stack - end - end - end -end diff --git a/lib/cloud_controller/diego/custom_stack_uri_converter.rb b/lib/cloud_controller/diego/custom_stack_uri_converter.rb deleted file mode 100644 index 7e6adc8be99..00000000000 --- a/lib/cloud_controller/diego/custom_stack_uri_converter.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'utils/uri_utils' - -module VCAP::CloudController - module Diego - # Converts docker://host/path:tag to Diego BBS rootfs format docker://host/path#tag - module CustomStackUriConverter - module_function - - def convert(custom_stack_uri) - raw = custom_stack_uri.delete_prefix('docker://') - host, path, tag_digest = UriUtils.parse_docker_uri(raw) - Addressable::URI.new(scheme: 'docker', host: host, path: path, fragment: tag_digest).to_s - end - end - end -end diff --git a/lib/cloud_controller/diego/image_credential_resolver.rb b/lib/cloud_controller/diego/image_credential_resolver.rb index 2a9f26fae41..8df9dfc1ad3 100644 --- a/lib/cloud_controller/diego/image_credential_resolver.rb +++ b/lib/cloud_controller/diego/image_credential_resolver.rb @@ -8,6 +8,7 @@ module ImageCredentialResolver # Returns [username, password], falling back to custom stack registry credentials. def resolve(primary_username:, primary_password:, lifecycle_data:) return [primary_username, primary_password] if primary_username.present? && primary_password.present? + return [primary_username, primary_password] unless lifecycle_data.respond_to?(:stack) && custom_stack?(lifecycle_data.stack) resolve_from_custom_stack(lifecycle_data) end @@ -16,6 +17,7 @@ def resolve(primary_username:, primary_password:, lifecycle_data:) def resolve_for_staging(primary_username:, primary_password:, lifecycle:) return [primary_username, primary_password] if primary_username.present? && primary_password.present? return [nil, nil] unless lifecycle.respond_to?(:credentials_for_staging) && lifecycle.respond_to?(:staging_stack) + return [primary_username, primary_password] unless custom_stack?(lifecycle.staging_stack) stack_host = UriUtils.custom_stack_registry_host(lifecycle.staging_stack) return [nil, nil] unless stack_host @@ -33,13 +35,23 @@ def resolve_from_custom_stack(lifecycle_data) end private_class_method :resolve_from_custom_stack + def custom_stack?(stack) + UriUtils.is_custom_stack_uri?(stack) + end + private_class_method :custom_stack? + def extract_credentials(credentials, host) return [nil, nil] unless credentials.is_a?(Hash) + credentials = credentials.deep_stringify_keys creds = credentials[host] return [nil, nil] unless creds.is_a?(Hash) - [creds['username'], creds['password']] + username = creds['username'] + password = creds['password'] + return [nil, nil] unless username.present? && password.present? + + [username, password] end private_class_method :extract_credentials end diff --git a/lib/cloud_controller/diego/lifecycle_bundle_stack_resolver.rb b/lib/cloud_controller/diego/lifecycle_bundle_stack_resolver.rb new file mode 100644 index 00000000000..1fc020db52c --- /dev/null +++ b/lib/cloud_controller/diego/lifecycle_bundle_stack_resolver.rb @@ -0,0 +1,13 @@ +require 'utils/uri_utils' + +module VCAP::CloudController + module Diego + # Selects the installed stack used to obtain lifecycle bundles and droplet destinations. + # A custom stack remains the Diego rootfs; it uses the default stack only for these assets. + class LifecycleBundleStackResolver + def self.resolve(stack) + UriUtils.is_custom_stack_uri?(stack) ? VCAP::CloudController::Stack.default.name : stack + end + end + end +end diff --git a/lib/cloud_controller/diego/lifecycle_protocol.rb b/lib/cloud_controller/diego/lifecycle_protocol.rb index f3644413a8f..7c97c143345 100644 --- a/lib/cloud_controller/diego/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/lifecycle_protocol.rb @@ -38,11 +38,10 @@ def lifecycle_data(staging_details) ) lifecycle_data.stack = stack - unless UriUtils.is_custom_stack_uri?(stack) - lifecycle_data.buildpack_cache_checksum = staging_details.package.app.buildpack_cache_sha256_checksum - lifecycle_data.build_artifacts_cache_download_uri = @blobstore_url_generator.buildpack_cache_download_url(staging_details.package.app_guid, stack) - lifecycle_data.build_artifacts_cache_upload_uri = @blobstore_url_generator.buildpack_cache_upload_url(staging_details.package.app_guid, stack) - end + cache_key = UriUtils.buildpack_cache_key(stack) + lifecycle_data.buildpack_cache_checksum = staging_details.package.app.buildpack_cache_sha256_checksum + lifecycle_data.build_artifacts_cache_download_uri = @blobstore_url_generator.buildpack_cache_download_url(staging_details.package.app_guid, cache_key) + lifecycle_data.build_artifacts_cache_upload_uri = @blobstore_url_generator.buildpack_cache_upload_url(staging_details.package.app_guid, cache_key) lifecycle_data.message rescue Membrane::SchemaValidationError => e diff --git a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb index f0d541937d6..02654d89777 100644 --- a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb @@ -6,6 +6,7 @@ module VCAP::CloudController class AppBaseLifecycle def initialize(message, app=nil) @message = message + @app_stack = app&.lifecycle_data&.stack validation_data = lifecycle_data_for_validation(app) db_result = BuildpackLifecycleFetcher.fetch(validation_data[:buildpacks], validation_data[:stack], type) @@ -14,6 +15,7 @@ def initialize(message, app=nil) buildpack_infos: db_result[:buildpack_infos], stack: db_result[:stack], stack_name: stack, + lifecycle_type: type, credentials: effective_credentials(app), credentials_from_message: message.buildpack_data.requested?(:credentials) ) @@ -69,6 +71,8 @@ def buildpacks def stack if message.buildpack_data.requested?(:stack) && !message.buildpack_data.stack.nil? message.buildpack_data.stack + elsif message.buildpack_data.requested?(:buildpacks) || validating_stored_buildpacks? + @app_stack || Stack.default.name else Stack.default.name end diff --git a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb index 1655d62f42f..71aa45ce8c1 100644 --- a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb +++ b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb @@ -4,11 +4,13 @@ module VCAP::CloudController class BuildpackLifecycleDataValidator include ActiveModel::Model - attr_accessor :buildpack_infos, :stack, :stack_name, :credentials, :credentials_from_message + attr_accessor :buildpack_infos, :stack, :stack_name, :lifecycle_type, :credentials, :credentials_from_message validate :buildpacks_are_uri_or_nil + validate :custom_stack_uri_is_valid validate :stack_exists_in_db - validate :custom_stack_requires_custom_buildpack + validate :custom_stack_has_auto_detect_buildpack + validate :no_custom_stack_userinfo validate :no_dual_credential_sources def buildpacks_are_uri_or_nil @@ -27,15 +29,29 @@ def buildpacks_are_uri_or_nil def stack_exists_in_db return if UriUtils.is_custom_stack_uri?(stack_name) && FeatureFlag.enabled?(:diego_custom_stacks) + return if custom_stack_uri?(stack_name) && !UriUtils.is_custom_stack_uri?(stack_name) errors.add(:stack, 'must be an existing stack') if stack.nil? end - def custom_stack_requires_custom_buildpack - return unless UriUtils.is_custom_stack_uri?(stack_name) - return if buildpack_infos.present? && buildpack_infos.all? { |info| info.buildpack_url.present? } + def custom_stack_uri_is_valid + return unless custom_stack_uri?(stack_name) + return if UriUtils.is_custom_stack_uri?(stack_name) - errors.add(:buildpack, 'must be a custom buildpack (URL) when using a custom stack') + errors.add(:stack, 'must be a valid container image URI') + end + + def no_custom_stack_userinfo + return unless UriUtils.is_custom_stack_uri?(stack_name) && UriUtils.url_has_userinfo?(stack_name) + + errors.add(:stack, 'Custom stack URIs must not contain credentials; use data.credentials instead') + end + + def custom_stack_has_auto_detect_buildpack + return unless UriUtils.is_custom_stack_uri?(stack_name) && buildpack_infos.empty? + return if unpinned_admin_buildpack_exists? + + errors.add(:buildpack, 'must specify a custom buildpack URL or an enabled unpinned admin buildpack when using a custom stack') end def no_dual_credential_sources @@ -51,8 +67,17 @@ def no_dual_credential_sources private + def unpinned_admin_buildpack_exists? + lifecycle = lifecycle_type || Config.config.get(:default_app_lifecycle) + Buildpack.list_admin_buildpacks(nil, lifecycle).any? { |buildpack| buildpack.stack.nil? && buildpack.enabled? } + end + def url_has_userinfo? buildpack_infos.any? { |bi| bi.buildpack_url && UriUtils.url_has_userinfo?(bi.buildpack_url) } end + + def custom_stack_uri?(stack_name) + stack_name.is_a?(String) && stack_name.start_with?('docker://') + end end end diff --git a/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb b/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb index 1c1bbcc6bfa..85178b49c11 100644 --- a/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb +++ b/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb @@ -21,6 +21,7 @@ def initialize(package, staging_message) buildpack_infos: @buildpack_infos, stack: db_result[:stack], stack_name: staging_stack, + lifecycle_type: type, credentials: @credentials_for_staging, credentials_from_message: staging_message.buildpack_data.requested?(:credentials) ) diff --git a/lib/cloud_controller/diego/staging_action_builder.rb b/lib/cloud_controller/diego/staging_action_builder.rb index c7b455f2b6b..e5ffb84a94c 100644 --- a/lib/cloud_controller/diego/staging_action_builder.rb +++ b/lib/cloud_controller/diego/staging_action_builder.rb @@ -1,15 +1,14 @@ require 'credhub/config_helpers' require 'diego/action_builder' require 'digest/xxhash' -require 'cloud_controller/diego/custom_stack_uri_converter' -require 'cloud_controller/diego/custom_stack_fallback' +require 'cloud_controller/diego/docker/docker_uri_converter' +require 'cloud_controller/diego/lifecycle_bundle_stack_resolver' module VCAP::CloudController module Diego class StagingActionBuilder include ::Credhub::ConfigHelpers include ::Diego::ActionBuilder - include CustomStackFallback attr_reader :config, :lifecycle_data, :staging_details @@ -134,7 +133,7 @@ def image_layers def stack @stack ||= if UriUtils.is_custom_stack_uri?(lifecycle_stack) - CustomStackUriConverter.convert(lifecycle_stack) + DockerURIConverter.new.convert(lifecycle_stack.delete_prefix('docker://')) else stack_obj = Stack.find(name: lifecycle_stack) raise CloudController::Errors::ApiError.new_from_details('StackNotFound', lifecycle_stack) unless stack_obj @@ -188,14 +187,12 @@ def upload_actions ) ] - unless UriUtils.is_custom_stack_uri?(lifecycle_data[:stack]) - actions << ::Diego::Bbs::Models::UploadAction.new( - user: 'vcap', - artifact: 'build artifacts cache', - from: @cache_source, - to: upload_buildpack_artifacts_cache_uri.to_s - ) - end + actions << ::Diego::Bbs::Models::UploadAction.new( + user: 'vcap', + artifact: 'build artifacts cache', + from: @cache_source, + to: upload_buildpack_artifacts_cache_uri.to_s + ) actions end @@ -205,7 +202,7 @@ def skip_detect? end def lifecycle_bundle_key - :"#{@prefix}/#{resolved_stack_name(lifecycle_data[:stack])}" + :"#{@prefix}/#{LifecycleBundleStackResolver.resolve(lifecycle_stack)}" end def upload_buildpack_artifacts_cache_uri diff --git a/lib/utils/uri_utils.rb b/lib/utils/uri_utils.rb index ca5e17db0dd..11fd1941525 100644 --- a/lib/utils/uri_utils.rb +++ b/lib/utils/uri_utils.rb @@ -1,4 +1,5 @@ require 'uri' +require 'digest' module UriUtils SSH_REGEX = %r{ \A (?:ssh://)? git@ .+? : .+? \.git \z }x @@ -55,11 +56,17 @@ def custom_stack_registry_host(stack_uri) return nil if body.empty? host, = parse_docker_uri(body) - host.presence - rescue StandardError + host.presence || DOCKER_INDEX_SERVER + rescue InvalidDockerURI nil end + def buildpack_cache_key(stack) + return stack unless is_custom_stack_uri?(stack) + + Digest::SHA256.hexdigest(stack) + end + def is_uri_path?(candidate) !!(candidate.is_a?(String) && candidate =~ %r{^(?:/|/([^\s/]\S*)?)$}) end @@ -91,6 +98,8 @@ def parse_docker_uri(docker_uri) path = docker_uri end + raise InvalidDockerURI.new "Invalid image name [#{docker_uri}]" if path.blank? + path = 'library/' + path if (official_docker_registry(name_parts[0]) || missing_registry(name_parts)) && path.exclude?('/') path, tag_digest = parse_docker_tag_digest_from_path(path) diff --git a/spec/unit/controllers/v3/apps_controller_spec.rb b/spec/unit/controllers/v3/apps_controller_spec.rb index a3164f97faa..e6e0cec2310 100644 --- a/spec/unit/controllers/v3/apps_controller_spec.rb +++ b/spec/unit/controllers/v3/apps_controller_spec.rb @@ -572,6 +572,34 @@ end end end + + context 'when requesting a custom stack and diego_custom_stacks is disabled' do + let(:request_body) do + { + name: 'some-name', + relationships: { space: { data: { guid: space.guid } } }, + lifecycle: { + type: 'buildpack', + data: { + buildpacks: ['https://example.com/buildpack.git'], + stack: 'docker://registry.example.com/org/stack:tag' + } + } + } + end + + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: false, error_message: nil) + end + + it 'raises 403' do + post :create, params: request_body, as: :json + + expect(response).to have_http_status(:forbidden) + expect(response.body).to include('FeatureDisabled') + expect(response.body).to include('diego_custom_stacks') + end + end end describe '#update' do @@ -610,6 +638,32 @@ end end + context 'when updating to a custom stack and diego_custom_stacks is disabled' do + let(:request_body) do + { + lifecycle: { + type: 'buildpack', + data: { + buildpacks: ['https://example.com/buildpack.git'], + stack: 'docker://registry.example.com/org/stack:tag' + } + } + } + end + + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: false, error_message: nil) + end + + it 'raises 403' do + patch :update, params: { guid: app_model.guid }.merge(request_body), as: :json + + expect(response).to have_http_status(:forbidden) + expect(response.body).to include('FeatureDisabled') + expect(response.body).to include('diego_custom_stacks') + end + end + context 'lifecycle data' do let(:new_name) { 'potato' } @@ -1265,6 +1319,21 @@ end end end + + context 'when starting an app with a custom stack and diego_custom_stacks is disabled' do + before do + app_model.lifecycle_data.update(stack: 'docker://registry.example.com/org/stack:tag') + create(:feature_flag, name: 'diego_custom_stacks', enabled: false, error_message: nil) + end + + it 'raises 403' do + put :start, params: { guid: app_model.guid }, as: :json + + expect(response).to have_http_status(:forbidden) + expect(response.body).to include('FeatureDisabled') + expect(response.body).to include('diego_custom_stacks') + end + end end describe '#stop' do @@ -1368,6 +1437,24 @@ create(:buildpack_lifecycle_data_model, app: app_model, buildpacks: nil, stack: VCAP::CloudController::Stack.default.name) end + context 'when restarting an app with a custom stack and diego_custom_stacks is disabled' do + before do + set_current_user(user) + allow_user_read_access_for(user, spaces: [space]) + allow_user_write_access(user, space:) + app_model.lifecycle_data.update(stack: 'docker://registry.example.com/org/stack:tag') + create(:feature_flag, name: 'diego_custom_stacks', enabled: false, error_message: nil) + end + + it 'raises 403' do + post :restart, params: { guid: app_model.guid }, as: :json + + expect(response).to have_http_status(:forbidden) + expect(response.body).to include('FeatureDisabled') + expect(response.body).to include('diego_custom_stacks') + end + end + context 'permissions' do describe 'authorization' do role_to_expected_http_response = { diff --git a/spec/unit/fetchers/buildpack_lifecycle_fetcher_spec.rb b/spec/unit/fetchers/buildpack_lifecycle_fetcher_spec.rb index 999e97385b1..470af857d7c 100644 --- a/spec/unit/fetchers/buildpack_lifecycle_fetcher_spec.rb +++ b/spec/unit/fetchers/buildpack_lifecycle_fetcher_spec.rb @@ -51,6 +51,24 @@ module VCAP::CloudController end end + context 'with a custom stack' do + let(:custom_stack) { 'docker://registry.example.com/org/stack:tag' } + let!(:unpinned_buildpack) { create(:buildpack, :nil_stack, name: 'buildpack-3') } + + it 'returns unpinned admin buildpacks and custom buildpacks' do + returned_hash = BuildpackLifecycleFetcher.fetch([unpinned_buildpack.name, 'https://example.com/buildpack.git'], custom_stack) + + expect(returned_hash[:stack]).to be_nil + expect(returned_hash[:buildpack_infos].map(&:buildpack_record)).to eq([unpinned_buildpack, nil]) + end + + it 'does not return admin buildpacks pinned to another stack' do + returned_hash = BuildpackLifecycleFetcher.fetch([buildpack.name], custom_stack) + + expect(returned_hash[:buildpack_infos].first.buildpack_record).to be_nil + end + end + context 'when the stack and buildpack are not found' do it 'returns nil for both' do returned_hash = BuildpackLifecycleFetcher.fetch(['bogus-buildpack'], 'bogus-stack') diff --git a/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb index e75ebf58b10..66cc1b79180 100644 --- a/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb @@ -364,6 +364,21 @@ module Diego expect(lrp.image_layers).to eq(expected_image_layers) end + context 'when the app uses a custom stack with registry credentials' do + before do + app_model.lifecycle_data.update( + stack: 'docker://registry.example.com/my-org/custom-stack:latest', + credentials: { 'registry.example.com' => { 'username' => 'stack-user', 'password' => 'stack-password' } } + ) + end + + it 'passes the credentials to Diego' do + lrp = builder.build_app_lrp + expect(lrp.image_username).to eq('stack-user') + expect(lrp.image_password).to eq('stack-password') + end + end + context 'when the space is not entitled to any isolation segments' do before do allow(VCAP::CloudController::IsolationSegmentSelector).to receive(:for_space).and_return(nil) diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb index e17436b263c..aeeb07e6f2c 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb @@ -1,6 +1,5 @@ require 'spec_helper' require 'cloud_controller/diego/buildpack/desired_lrp_builder' -require 'cloud_controller/diego/custom_stack_uri_converter' module VCAP::CloudController module Diego @@ -39,12 +38,6 @@ module Buildpack end describe '#root_fs' do - context 'with a system stack' do - it 'returns preloaded rootfs' do - expect(builder.root_fs).to eq('preloaded:cflinuxfs4') - end - end - context 'with a custom stack (docker:// URI)' do let(:stack) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } let(:config) do @@ -62,24 +55,6 @@ module Buildpack expect(builder.root_fs).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') end end - - context 'with a custom stack from a private registry' do - let(:stack) { 'docker://registry.example.com/my-org/my-stack:v2.0' } - let(:config) do - Config.new({ - diego: { - use_privileged_containers_for_running: false, - lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, - droplet_destinations: { 'default-stack-name': '/home/vcap' }, - enable_declarative_asset_downloads: false - } - }) - end - - it 'returns a docker:// rootfs URI for the private registry' do - expect(builder.root_fs).to eq('docker://registry.example.com/my-org/my-stack#v2.0') - end - end end describe '#cached_dependencies' do diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb index 8d951296c97..c9459548178 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb @@ -51,6 +51,7 @@ module Buildpack let(:package) { create(:package_model, app:) } let(:droplet) { create(:droplet_model, package:, app:) } let(:buildpack) { nil } + let(:stack) { 'potato-stack' } let(:buildpack_infos) { [BuildpackInfo.new(buildpack, VCAP::CloudController::Buildpack.find(name: buildpack))] } let(:staging_details) do @@ -60,7 +61,7 @@ module Buildpack details.environment_variables = { 'nightshade_fruit' => 'potato' } details.staging_memory_in_mb = 42 details.staging_disk_in_mb = 51 - details.lifecycle = instance_double(BuildpackLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, + details.lifecycle = instance_double(BuildpackLifecycle, staging_stack: stack, buildpack_infos: buildpack_infos, credentials_for_staging: nil) end end @@ -125,6 +126,19 @@ module Buildpack end end + context 'when using a custom stack' do + let(:stack) { 'docker://registry.example.com/my-org/custom-stack:latest' } + let(:buildpack) { 'http://custom-buildpack.example.com' } + + it 'uses a safe cache key while retaining buildpack-artifact caching' do + lifecycle_protocol.lifecycle_data(staging_details) + + cache_key = Digest::SHA256.hexdigest(stack) + expect(blobstore_url_generator).to have_received(:buildpack_cache_download_url).with(app.guid, cache_key) + expect(blobstore_url_generator).to have_received(:buildpack_cache_upload_url).with(app.guid, cache_key) + end + end + context 'when the generated message has invalid data' do let(:buildpack_infos) { [] } diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/staging_action_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/staging_action_builder_spec.rb index a30b06cdb0a..c0d20ddc279 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/staging_action_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/staging_action_builder_spec.rb @@ -152,6 +152,15 @@ module Buildpack expect(upload_actions[1].upload_action).to eq(upload_build_artifacts_cache_action) end + context 'when using a custom stack' do + let(:stack) { 'docker://registry.example.com/my-org/custom-stack:latest' } + + it 'uploads the build artifacts cache' do + upload_actions = builder.action.serial_action.actions[2].emit_progress_action.action.parallel_action.actions + expect(upload_actions.map(&:upload_action)).to include(upload_build_artifacts_cache_action) + end + end + describe 'credhub' do let(:credhub_url) { TestConfig.config_instance.get(:credhub_api, :internal_url) } let(:expected_platform_options) do diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb deleted file mode 100644 index db953392131..00000000000 --- a/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_custom_stacks_spec.rb +++ /dev/null @@ -1,86 +0,0 @@ -require 'spec_helper' -require 'cloud_controller/diego/buildpack/task_action_builder' -require 'cloud_controller/diego/custom_stack_uri_converter' - -module VCAP::CloudController - module Diego - module Buildpack - RSpec.describe TaskActionBuilder do - let(:stack_name) { 'cflinuxfs4' } - let(:config) do - Config.new({ - diego: { - use_privileged_containers_for_running: false, - lifecycle_bundles: { "buildpack/#{VCAP::CloudController::Stack.default.name}": 'http://lifecycle.example.com/bundle.tgz' }, - droplet_destinations: { 'default-stack-name': '/home/vcap' }, - enable_declarative_asset_downloads: false - } - }) - end - let(:lifecycle_data) do - { - droplet_uri: 'http://droplet.example.com/droplet.tgz', - stack: stack_name - } - end - let(:task) do - instance_double(TaskModel, - name: 'my-task', - droplet: instance_double(DropletModel, sha256_checksum: 'abc123', droplet_hash: 'hash')) - end - - subject(:builder) do - TaskActionBuilder.new(config, task, lifecycle_data, 'vcap', ['app', 'echo hello', ''], 'buildpack') - end - - before do - VCAP::CloudController::Stack.find(name: 'cflinuxfs4') || VCAP::CloudController::Stack.create(name: 'cflinuxfs4') - end - - describe '#stack' do - context 'with a system stack' do - it 'returns preloaded rootfs' do - expect(builder.stack).to eq('preloaded:cflinuxfs4') - end - end - - context 'with a custom stack' do - let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } - - it 'returns a docker:// rootfs URI' do - expect(builder.stack).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') - end - end - end - - describe '#lifecycle_bundle_key' do - context 'with a system stack' do - it 'uses the stack name' do - expect(builder.lifecycle_bundle_key).to eq(:'buildpack/cflinuxfs4') - end - end - - context 'with a custom stack' do - let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } - - it 'falls back to the default stack name' do - expect(builder.lifecycle_bundle_key).to eq(:"buildpack/#{VCAP::CloudController::Stack.default.name}") - end - end - end - - describe '#cached_dependencies' do - context 'with a custom stack' do - let(:stack_name) { 'docker://registry.example.com/my-stack:v1' } - - it 'uses the default stack lifecycle bundle' do - allow(LifecycleBundleUriGenerator).to receive(:uri).and_return('http://lifecycle.example.com/bundle.tgz') - deps = builder.cached_dependencies - expect(deps.first.cache_key).to eq("buildpack-#{VCAP::CloudController::Stack.default.name}-lifecycle") - end - end - end - end - end - end -end diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_spec.rb index 23496ab7097..7b4dfbb5ad1 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/task_action_builder_spec.rb @@ -7,13 +7,16 @@ module Buildpack subject(:builder) { TaskActionBuilder.new(config, task, lifecycle_data, 'vcap', ['app', task.command, ''], 'buildpack') } let(:enable_declarative_asset_downloads) { false } + let(:lifecycle_bundles) do + { + 'buildpack/potato-stack': 'http://file-server.service.cf.internal:8080/v1/static/potato_lifecycle_bundle_url' + } + end let(:config) do Config.new({ diego: { enable_declarative_asset_downloads: enable_declarative_asset_downloads, - lifecycle_bundles: { - 'buildpack/potato-stack': 'http://file-server.service.cf.internal:8080/v1/static/potato_lifecycle_bundle_url' - }, + lifecycle_bundles: lifecycle_bundles, droplet_destinations: droplet_destinations } }) @@ -261,6 +264,14 @@ module Buildpack expect(builder.stack).to eq('preloaded:run-image') end end + + context 'when the stack is a custom image' do + let(:stack) { 'docker://registry.example.com/my-org/custom-stack:latest' } + + it 'returns a Docker rootfs URI' do + expect(builder.stack).to eq('docker://registry.example.com/my-org/custom-stack#latest') + end + end end describe '#cached_dependencies' do @@ -291,6 +302,17 @@ module Buildpack end.to raise_error VCAP::CloudController::Diego::LifecycleBundleUriGenerator::InvalidStack end end + + context 'when the stack is a custom image' do + let(:stack) { 'docker://registry.example.com/my-org/custom-stack:latest' } + let(:lifecycle_bundles) do + { "buildpack/#{Stack.default.name}": 'http://file-server.service.cf.internal:8080/v1/static/default_lifecycle_bundle_url' } + end + + it 'uses the default stack lifecycle bundle' do + expect(builder.cached_dependencies.first.cache_key).to eq("buildpack-#{Stack.default.name}-lifecycle") + end + end end end end diff --git a/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb deleted file mode 100644 index 7395118dc32..00000000000 --- a/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_custom_stacks_spec.rb +++ /dev/null @@ -1,120 +0,0 @@ -require 'spec_helper' -require 'cloud_controller/diego/cnb/staging_action_builder' - -module VCAP::CloudController - module Diego - module CNB - RSpec.describe StagingActionBuilder do - let(:stack_name) { 'cflinuxfs4' } - let(:lifecycle_data) do - { - app_bits_download_uri: 'http://app.example.com/bits.zip', - build_artifacts_cache_upload_uri: 'http://cache.example.com/upload', - droplet_upload_uri: 'http://droplet.example.com/upload', - buildpacks: [], - stack: stack_name, - app_bits_checksum: { type: 'sha256', value: 'checksum' }, - auto_detect: true - } - end - let(:config) do - Config.new({ - diego: { - lifecycle_bundles: { 'cnb/default-stack-name': 'http://lifecycle.example.com/cnb-bundle.tgz' }, - droplet_destinations: { 'default-stack-name': '/home/vcap' }, - enable_declarative_asset_downloads: false, - cc_uploader_url: 'http://uploader.example.com' - }, - staging: { - timeout_in_seconds: 900, - legacy_md5_buildpack_paths_enabled: false - } - }) - end - let(:staging_details) { instance_double(StagingDetails, staging_guid: 'staging-guid') } - - subject(:builder) { StagingActionBuilder.new(config, staging_details, lifecycle_data) } - - before do - VCAP::CloudController::Stack.find(name: 'cflinuxfs4') || VCAP::CloudController::Stack.create(name: 'cflinuxfs4') - end - - describe '#task_environment_variables' do - context 'with a system stack' do - it 'includes CNB_STACK_ID set to the stack name' do - env_vars = builder.task_environment_variables - stack_id_var = env_vars.find { |v| v.name == 'CNB_STACK_ID' } - expect(stack_id_var).not_to be_nil - expect(stack_id_var.value).to eq('cflinuxfs4') - end - end - - context 'with a custom stack and no explicit stack_id' do - let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } - - it 'omits CNB_STACK_ID (auto-detect)' do - env_vars = builder.task_environment_variables - stack_id_var = env_vars.find { |v| v.name == 'CNB_STACK_ID' } - expect(stack_id_var).to be_nil - end - end - - context 'with a custom stack and an explicit stack_id' do - let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } - let(:lifecycle_data) do - { - app_bits_download_uri: 'http://app.example.com/bits.zip', - build_artifacts_cache_upload_uri: 'http://cache.example.com/upload', - droplet_upload_uri: 'http://droplet.example.com/upload', - buildpacks: [], - stack: stack_name, - stack_id: 'io.buildpacks.stacks.jammy', - app_bits_checksum: { type: 'sha256', value: 'checksum' }, - auto_detect: true - } - end - - it 'uses the provided stack_id' do - env_vars = builder.task_environment_variables - stack_id_var = env_vars.find { |v| v.name == 'CNB_STACK_ID' } - expect(stack_id_var).not_to be_nil - expect(stack_id_var.value).to eq('io.buildpacks.stacks.jammy') - end - end - - context 'with credentials in lifecycle_data' do - let(:lifecycle_data) do - { - app_bits_download_uri: 'http://app.example.com/bits.zip', - build_artifacts_cache_upload_uri: 'http://cache.example.com/upload', - droplet_upload_uri: 'http://droplet.example.com/upload', - buildpacks: [], - stack: stack_name, - app_bits_checksum: { type: 'sha256', value: 'checksum' }, - auto_detect: true, - credentials: '{"docker.io":{"username":"user","password":"pass"}}' - } - end - - it 'includes CNB_REGISTRY_CREDS' do - env_vars = builder.task_environment_variables - creds_var = env_vars.find { |v| v.name == 'CNB_REGISTRY_CREDS' } - expect(creds_var).not_to be_nil - expect(creds_var.value).to include('docker.io') - end - end - end - - describe '#stack' do - context 'with a custom stack' do - let(:stack_name) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } - - it 'returns docker:// rootfs in Diego format' do - expect(builder.stack).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') - end - end - end - end - end - end -end diff --git a/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_spec.rb index e2bc1bc11c4..b0b6129a9d0 100644 --- a/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_spec.rb @@ -359,6 +359,14 @@ module CNB expect(builder.stack).to eq('preloaded:build-image') end end + + context 'when the stack is a custom image' do + let(:stack) { 'docker://registry.example.com/my-org/custom-stack:latest' } + + it 'returns a Docker rootfs URI' do + expect(builder.stack).to eq('docker://registry.example.com/my-org/custom-stack#latest') + end + end end describe '#task_environment_variables' do @@ -390,6 +398,15 @@ module CNB expect(builder.task_environment_variables).to include(group_env) end end + + context 'when the stack is a custom image' do + let(:stack) { 'docker://registry.example.com/my-org/custom-stack:latest' } + + it 'uses the full stack image URI as CNB_STACK_ID' do + stack_id_env = ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_STACK_ID', value: stack) + expect(builder.task_environment_variables).to include(stack_id_env) + end + end end end end diff --git a/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb b/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb deleted file mode 100644 index 78a466b2160..00000000000 --- a/spec/unit/lib/cloud_controller/diego/custom_stack_uri_converter_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -require 'spec_helper' -require 'cloud_controller/diego/custom_stack_uri_converter' - -module VCAP::CloudController - module Diego - RSpec.describe CustomStackUriConverter do - describe '.convert' do - it 'converts docker://host/path:tag to Diego rootfs format' do - result = CustomStackUriConverter.convert('docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0') - expect(result).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') - end - - it 'converts docker://host/path without tag (no fragment appended)' do - result = CustomStackUriConverter.convert('docker://docker.io/cloudfoundry/cflinuxfs4') - expect(result).to eq('docker://docker.io/cloudfoundry/cflinuxfs4') - end - - it 'handles private registries' do - result = CustomStackUriConverter.convert('docker://registry.example.com/my-org/my-stack:v2.0') - expect(result).to eq('docker://registry.example.com/my-org/my-stack#v2.0') - end - - it 'handles images without explicit registry (docker hub shorthand)' do - result = CustomStackUriConverter.convert('docker://ubuntu:22.04') - expect(result).to eq('docker:///library/ubuntu#22.04') - end - - it 'handles digest references' do - digest = 'sha256:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890' - result = CustomStackUriConverter.convert("docker://docker.io/cloudfoundry/cflinuxfs4@#{digest}") - expect(result).to include('docker://docker.io/cloudfoundry/cflinuxfs4') - end - end - end - end -end diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb index 49e06cf0fa0..4e3d2e35c01 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb @@ -118,6 +118,8 @@ module VCAP::CloudController expect(BuildpackLifecycleDataValidator).to have_received(:new).with( buildpack_infos: [], stack: 'foo', + stack_name: Stack.default.name, + lifecycle_type: Lifecycles::BUILDPACK, credentials: nil, credentials_from_message: false ) @@ -279,6 +281,22 @@ module VCAP::CloudController expect { AppBuildpackLifecycle.new(message, app) }.not_to raise_error end end + + context 'when updating an existing custom-stack app without requesting a stack' do + let(:app) { create(:app_model, :buildpack) } + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'buildpack', data: { buildpacks: ['default-stack-buildpack'] } } }) } + + before do + app.buildpack_lifecycle_data.update(stack: 'docker://registry.example.com/custom-stack:latest') + create(:buildpack, name: 'default-stack-buildpack', stack: Stack.default.name) + end + + it 'rejects a buildpack pinned to the default stack' do + lifecycle = AppBuildpackLifecycle.new(message, app) + expect(lifecycle).not_to be_valid + expect(lifecycle.errors[:buildpack]).to include('"default-stack-buildpack" must be an existing admin buildpack or a valid git URI') + end + end end end end diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb index 34642d8fa44..6650bbc9a4b 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb @@ -124,6 +124,22 @@ module VCAP::CloudController end end end + + context 'when updating an existing custom-stack app without requesting a stack' do + let(:app) { create(:app_model, :cnb) } + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'cnb', data: { buildpacks: ['default-stack-buildpack'] } } }) } + + before do + app.cnb_lifecycle_data.update(stack: 'docker://registry.example.com/custom-stack:latest') + create(:buildpack, name: 'default-stack-buildpack', stack: Stack.default.name, lifecycle: Lifecycles::CNB) + end + + it 'rejects a buildpack pinned to the default stack' do + lifecycle = AppCNBLifecycle.new(message, app) + expect(lifecycle).not_to be_valid + expect(lifecycle.errors[:buildpack]).to include('"default-stack-buildpack" must be an existing admin buildpack or a valid git URI') + end + end end describe '#update_lifecycle_data_model credentials handling' do diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_custom_stacks_spec.rb index 199d5625c39..e6fcd0baab8 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_custom_stacks_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_custom_stacks_spec.rb @@ -7,12 +7,14 @@ module VCAP::CloudController let(:buildpack_url) { 'https://github.com/my-org/my-buildpack.git' } let(:buildpack_info) { BuildpackInfo.new(buildpack_url, nil) } let(:buildpack_infos) { [buildpack_info] } + let(:lifecycle_type) { Lifecycles::BUILDPACK } subject(:validator) do BuildpackLifecycleDataValidator.new( buildpack_infos: buildpack_infos, stack: stack, - stack_name: stack&.name + stack_name: stack&.name, + lifecycle_type: lifecycle_type ) end @@ -24,7 +26,8 @@ module VCAP::CloudController BuildpackLifecycleDataValidator.new( buildpack_infos: buildpack_infos, stack: nil, - stack_name: custom_stack_uri + stack_name: custom_stack_uri, + lifecycle_type: lifecycle_type ) end @@ -41,12 +44,68 @@ module VCAP::CloudController end end - context 'with admin buildpacks (not URL-based, no record)' do - let(:buildpack_info) { BuildpackInfo.new('ruby_buildpack', nil) } + context 'with an unpinned admin buildpack' do + let(:buildpack_info) { BuildpackInfo.new('ruby_buildpack', create(:buildpack, :nil_stack, name: 'ruby_buildpack')) } + + it 'is valid' do + expect(validator).to be_valid + end + end + + context 'when auto-detecting without an enabled unpinned admin buildpack' do + let(:buildpack_infos) { [] } + + it 'is not valid' do + expect(validator).not_to be_valid + expect(validator.errors[:buildpack]).to include( + 'must specify a custom buildpack URL or an enabled unpinned admin buildpack when using a custom stack' + ) + end + end + + context 'when auto-detecting with an enabled unpinned admin buildpack' do + let(:buildpack_infos) { [] } + + before do + create(:buildpack, :nil_stack, enabled: true) + end + + it 'is valid' do + expect(validator).to be_valid + end + end + + context 'when auto-detecting CNB buildpacks' do + let(:lifecycle_type) { Lifecycles::CNB } + let(:buildpack_infos) { [] } + + before do + create(:buildpack, :nil_stack, lifecycle: Lifecycles::BUILDPACK) + end + + it 'does not use an unpinned buildpack lifecycle admin buildpack' do + expect(validator).not_to be_valid + expect(validator.errors[:buildpack]).to include( + 'must specify a custom buildpack URL or an enabled unpinned admin buildpack when using a custom stack' + ) + end + end + + context 'with credentials embedded in the stack URI' do + let(:custom_stack_uri) { 'docker://user:pass@registry.example.com/org/stack:tag' } it 'is not valid' do expect(validator).not_to be_valid - expect(validator.errors[:buildpack]).to include('must be a custom buildpack (URL) when using a custom stack') + expect(validator.errors[:stack]).to include('Custom stack URIs must not contain credentials; use data.credentials instead') + end + end + + context 'with an invalid custom stack URI' do + let(:custom_stack_uri) { 'docker://registry.example.com/' } + + it 'reports an invalid container image URI' do + expect(validator).not_to be_valid + expect(validator.errors[:stack]).to contain_exactly('must be a valid container image URI') end end end diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb index a69a89cfd2c..f43bad14bc0 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb @@ -218,6 +218,8 @@ module VCAP::CloudController expect(BuildpackLifecycleDataValidator).to have_received(:new).with( buildpack_infos: [], stack: 'foo', + stack_name: Stack.default.name, + lifecycle_type: Lifecycles::BUILDPACK, credentials: nil, credentials_from_message: false ) diff --git a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb index 8eed4f5f027..c4ca109a16e 100644 --- a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb @@ -32,6 +32,21 @@ module Diego expect(username).to eq('my-user') expect(password).to eq('my-pass') end + + it 'returns credentials from symbolized staging request data' do + allow(lifecycle).to receive(:credentials_for_staging).and_return( + { + 'registry.example.com': { + username: 'my-user', + password: 'my-pass' + } + } + ) + + username, password = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(username).to eq('my-user') + expect(password).to eq('my-pass') + end end describe '#staging_image_credentials (docker package)' do @@ -56,6 +71,20 @@ module Diego end end + describe '#staging_image_credentials (incomplete custom stack credentials)' do + let(:lifecycle) do + instance_double(BuildpackLifecycle, + staging_stack: 'docker://registry.example.com/my-org/my-stack:v1.0', + credentials_for_staging: { 'registry.example.com' => { 'username' => 'stack-user' } }) + end + let(:package) { instance_double(PackageModel, docker_username: nil, docker_password: nil) } + let(:staging_details) { instance_double(StagingDetails, package: package, lifecycle: lifecycle) } + + it 'does not pass a partial credential pair to Diego' do + expect(task_recipe_builder.send(:staging_image_credentials, staging_details)).to eq([nil, nil]) + end + end + describe '#staging_image_credentials (mismatched docker credentials)' do let(:lifecycle) do instance_double( @@ -83,6 +112,28 @@ module Diego end end + describe '#staging_image_credentials (incomplete docker credentials)' do + let(:lifecycle) do + instance_double( + BuildpackLifecycle, + staging_stack: 'cflinuxfs4', + credentials_for_staging: nil + ) + end + let(:package) do + instance_double(PackageModel, docker_username: 'only-user', docker_password: nil) + end + let(:staging_details) do + instance_double(StagingDetails, package: package, lifecycle: lifecycle) + end + + it 'preserves docker package credentials for a system stack' do + username, password = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(username).to eq('only-user') + expect(password).to be_nil + end + end + describe '#task_image_credentials (custom stacks)' do let(:lifecycle_data) do instance_double( diff --git a/spec/unit/lib/utils/uri_utils_custom_stack_spec.rb b/spec/unit/lib/utils/uri_utils_custom_stack_spec.rb deleted file mode 100644 index 09859adc613..00000000000 --- a/spec/unit/lib/utils/uri_utils_custom_stack_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -require 'spec_helper' - -RSpec.describe UriUtils do - describe '.is_custom_stack_uri?' do - it 'returns true for valid docker:// URIs' do - expect(UriUtils.is_custom_stack_uri?('docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0')).to be(true) - expect(UriUtils.is_custom_stack_uri?('docker://registry.example.com/my-org/my-stack:latest')).to be(true) - expect(UriUtils.is_custom_stack_uri?('docker://ghcr.io/cloudfoundry/cflinuxfs4:1.0.0')).to be(true) - end - - it 'returns true for docker:// URIs without explicit tags (defaults to latest)' do - expect(UriUtils.is_custom_stack_uri?('docker://docker.io/cloudfoundry/cflinuxfs4')).to be(true) - end - - it 'returns true for docker:// URIs with digest' do - expect(UriUtils.is_custom_stack_uri?('docker://docker.io/cloudfoundry/cflinuxfs4@sha256:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890')).to be(true) - end - - it 'returns false for non-string inputs' do - expect(UriUtils.is_custom_stack_uri?(nil)).to be(false) - expect(UriUtils.is_custom_stack_uri?(123)).to be(false) - expect(UriUtils.is_custom_stack_uri?({})).to be(false) - end - - it 'returns false for plain stack names' do - expect(UriUtils.is_custom_stack_uri?('cflinuxfs4')).to be(false) - expect(UriUtils.is_custom_stack_uri?('cflinuxfs3')).to be(false) - end - - it 'returns false for http/https URIs' do - expect(UriUtils.is_custom_stack_uri?('https://example.com/stack')).to be(false) - expect(UriUtils.is_custom_stack_uri?('http://example.com/stack')).to be(false) - end - - it 'returns false for invalid docker URIs' do - expect(UriUtils.is_custom_stack_uri?('docker://')).to be(false) - expect(UriUtils.is_custom_stack_uri?('docker://INVALID_UPPERCASE')).to be(false) - end - end -end diff --git a/spec/unit/lib/utils/uri_utils_spec.rb b/spec/unit/lib/utils/uri_utils_spec.rb index 2cd837ef793..03bc4befac0 100644 --- a/spec/unit/lib/utils/uri_utils_spec.rb +++ b/spec/unit/lib/utils/uri_utils_spec.rb @@ -104,6 +104,20 @@ end end + describe '.is_custom_stack_uri?' do + it 'accepts valid Docker image references and identifies their registry' do + expect(UriUtils.is_custom_stack_uri?('docker://registry.example.com/my-org/my-stack:latest')).to be(true) + expect(UriUtils.is_custom_stack_uri?('docker://ubuntu:22.04')).to be(true) + expect(UriUtils.custom_stack_registry_host('docker://ubuntu:22.04')).to eq('docker.io') + end + + it 'rejects non-Docker and invalid Docker references' do + expect(UriUtils.is_custom_stack_uri?('cflinuxfs4')).to be(false) + expect(UriUtils.is_custom_stack_uri?('https://example.com/stack')).to be(false) + expect(UriUtils.is_custom_stack_uri?('docker://')).to be(false) + end + end + describe '.is_uri_path?' do it 'is false if the object is not a string' do expect(UriUtils.is_uri_path?(1)).to be false diff --git a/spec/unit/messages/app_manifest_message_spec.rb b/spec/unit/messages/app_manifest_message_spec.rb index 3b4eeb9e0b3..796d7ff0881 100644 --- a/spec/unit/messages/app_manifest_message_spec.rb +++ b/spec/unit/messages/app_manifest_message_spec.rb @@ -2312,6 +2312,27 @@ module VCAP::CloudController expect(message.errors.full_messages).to include('Feature Disabled: I am a banana') end end + + context 'when custom stacks are disabled' do + let(:parsed_yaml) do + { + name: 'custom-stack-app', + stack: 'docker://registry.example.com/org/stack:tag', + buildpacks: ['https://example.com/buildpack.git'] + } + end + + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: false, error_message: 'custom stacks are disabled') + end + + it 'is not valid' do + message = AppManifestMessage.create_from_yml(parsed_yaml) + + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Feature Disabled: custom stacks are disabled') + end + end end end diff --git a/spec/unit/messages/buildpack_lifecycle_data_message_custom_stacks_spec.rb b/spec/unit/messages/buildpack_lifecycle_data_message_custom_stacks_spec.rb deleted file mode 100644 index ab798a995a9..00000000000 --- a/spec/unit/messages/buildpack_lifecycle_data_message_custom_stacks_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -require 'spec_helper' - -module VCAP::CloudController - RSpec.describe BuildpackLifecycleDataMessage do - describe 'stack_id field' do - it 'is valid with a stack_id' do - message = BuildpackLifecycleDataMessage.new({ - stack: 'docker://docker.io/cloudfoundry/cflinuxfs4:1.0.0', - stack_id: 'io.buildpacks.stacks.jammy' - }) - expect(message).to be_valid - end - - it 'is valid without a stack_id' do - message = BuildpackLifecycleDataMessage.new({ - stack: 'cflinuxfs4' - }) - expect(message).to be_valid - end - - it 'is invalid with a too-long stack_id' do - message = BuildpackLifecycleDataMessage.new({ - stack: 'cflinuxfs4', - stack_id: 'a' * 4097 - }) - expect(message).not_to be_valid - end - end - - describe 'custom stack URI as stack value' do - it 'accepts a docker:// URI as stack value' do - message = BuildpackLifecycleDataMessage.new({ - stack: 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0', - buildpacks: ['https://github.com/my-org/my-buildpack.git'] - }) - expect(message).to be_valid - end - end - end -end diff --git a/spec/unit/models/runtime/feature_flag_custom_stacks_spec.rb b/spec/unit/models/runtime/feature_flag_custom_stacks_spec.rb deleted file mode 100644 index 91e1e63eafc..00000000000 --- a/spec/unit/models/runtime/feature_flag_custom_stacks_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -require 'spec_helper' - -module VCAP::CloudController - RSpec.describe FeatureFlag, type: :model do - describe 'diego_custom_stacks feature flag' do - context 'when the diego_custom_stacks feature flag is not overridden' do - it 'returns the default value (disabled)' do - expect(FeatureFlag.enabled?(:diego_custom_stacks)).to be(false) - expect(FeatureFlag.disabled?(:diego_custom_stacks)).to be(true) - end - end - - context 'when the diego_custom_stacks feature flag is enabled' do - before do - create(:feature_flag, name: 'diego_custom_stacks', enabled: true) - end - - it 'returns true' do - expect(FeatureFlag.enabled?(:diego_custom_stacks)).to be(true) - end - end - - context 'when the diego_custom_stacks feature flag is disabled' do - before do - create(:feature_flag, name: 'diego_custom_stacks', enabled: false) - end - - it 'raises FeatureDisabled when raise_unless_enabled!' do - expect { - FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) - }.to raise_error(CloudController::Errors::ApiError) { |error| - expect(error.name).to eq('FeatureDisabled') - } - end - end - end - end -end diff --git a/spec/unit/models/runtime/feature_flag_spec.rb b/spec/unit/models/runtime/feature_flag_spec.rb index 8996a05c9f6..075b55ee63d 100644 --- a/spec/unit/models/runtime/feature_flag_spec.rb +++ b/spec/unit/models/runtime/feature_flag_spec.rb @@ -90,7 +90,7 @@ module VCAP::CloudController end describe '.enabled?' do - let(:key) { :user_org_creation } + let(:key) { :diego_custom_stacks } let(:default_value) { FeatureFlag::DEFAULT_FLAGS[key] } context 'when the feature flag is overridden' do