From 77f498298566fbe119dd5363e639f69f9dc066c7 Mon Sep 17 00:00:00 2001 From: johha Date: Wed, 28 Oct 2026 15:32:48 +0100 Subject: [PATCH 1/4] Implement RFC-0046 custom stack support Add custom container-image stacks for buildpack and CNB lifecycle apps behind the diego_custom_stacks feature flag. - Accept and validate docker:// stack image URIs without creating Stack records or persisting the deprecated stack_id field. - Use custom root filesystems for staging, runtime LRPs, and tasks, with lifecycle-bundle fallback for droplets whose original stack was removed. - Support custom buildpacks and stackless admin buildpacks, including buildpack auto-detection. - Store registry credentials encrypted, redact them from app events, reject credentials embedded in stack URIs, and pass only complete matching username/password pairs to Diego. - Reject unsupported token credentials for custom stack registries while retaining token support for CNB buildpack images. - Derive CF_STACK and CNB_STACK_ID from the final image repository component, excluding tags and digests. - Skip stack-record validation and buildpack-cache handling where they do not apply to remote custom stack images. - Document custom stack syntax, credentials, buildpack restrictions, stack identifiers, and image-layer disk quota behavior. - Add coverage for validation, lifecycle staging, desired LRPs, tasks, credentials, feature-flag enforcement, manifests, and event redaction. Co-authored-by: FloThinksPi <5863788+FloThinksPi@users.noreply.github.com> --- app/actions/build_create.rb | 1 + app/controllers/v3/apps_controller.rb | 12 ++ app/fetchers/buildpack_lifecycle_fetcher.rb | 11 ++ app/messages/app_manifest_message.rb | 7 + app/models/runtime/feature_flag.rb | 1 + .../includes/concepts/_lifecycles.md.erb | 12 +- .../resources/feature_flags/_flags.md.erb | 1 + .../resources/manifests/_object.md.erb | 5 +- .../diego/app_recipe_builder.rb | 17 +- .../diego/buildpack/desired_lrp_builder.rb | 24 ++- .../diego/buildpack/task_action_builder.rb | 16 +- .../diego/cnb/desired_lrp_builder.rb | 22 ++- .../diego/cnb/staging_action_builder.rb | 4 +- .../diego/image_credential_resolver.rb | 65 +++++++ .../diego/lifecycle_bundle_stack_resolver.rb | 13 ++ .../diego/lifecycle_protocol.rb | 8 +- .../diego/lifecycles/app_base_lifecycle.rb | 5 + .../diego/lifecycles/buildpack_lifecycle.rb | 6 +- .../buildpack_lifecycle_data_validator.rb | 37 +++- .../diego/lifecycles/lifecycle_base.rb | 10 +- .../diego/staging_action_builder.rb | 34 ++-- .../diego/task_recipe_builder.rb | 28 ++- lib/utils/uri_utils.rb | 34 ++++ .../documentation/feature_flags_api_spec.rb | 2 +- .../controllers/v3/apps_controller_spec.rb | 90 ++++++++- .../buildpack_lifecycle_fetcher_spec.rb | 18 ++ .../diego/app_recipe_builder_spec.rb | 15 ++ .../desired_lrp_builder_custom_stacks_spec.rb | 108 +++++++++++ .../buildpack/lifecycle_protocol_spec.rb | 16 +- .../buildpack/staging_action_builder_spec.rb | 9 + .../buildpack/task_action_builder_spec.rb | 28 ++- .../diego/cnb/lifecycle_protocol_spec.rb | 22 ++- .../diego/cnb/staging_action_builder_spec.rb | 17 ++ .../app_buildpack_lifecycle_spec.rb | 18 ++ .../lifecycles/app_cnb_lifecycle_spec.rb | 16 ++ ...cycle_data_validator_custom_stacks_spec.rb | 150 ++++++++++++++ .../lifecycles/buildpack_lifecycle_spec.rb | 23 ++- .../task_recipe_builder_custom_stacks_spec.rb | 183 ++++++++++++++++++ spec/unit/lib/utils/uri_utils_spec.rb | 35 ++++ .../messages/app_manifest_message_spec.rb | 28 ++- spec/unit/models/runtime/feature_flag_spec.rb | 2 +- .../repositories/app_event_repository_spec.rb | 21 ++ 42 files changed, 1109 insertions(+), 65 deletions(-) create mode 100644 lib/cloud_controller/diego/image_credential_resolver.rb create mode 100644 lib/cloud_controller/diego/lifecycle_bundle_stack_resolver.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/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 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..0b6f385a858 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_stack_enabled!(custom_stack_from_message(message)) unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid? app_creator = AppCreate.new(user_audit_info) @@ -128,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) @@ -188,6 +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_stack_enabled!(app.lifecycle_data.stack) if app.lifecycle_data.respond_to?(:stack) AppStart.start(app:, user_audit_info:) TelemetryLogger.v3_emit( @@ -231,6 +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_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( @@ -397,6 +401,14 @@ def translate_error(e) private + 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 f5d399a64e2..0edb210280e 100644 --- a/app/fetchers/buildpack_lifecycle_fetcher.rb +++ b/app/fetchers/buildpack_lifecycle_fetcher.rb @@ -4,6 +4,8 @@ module VCAP::CloudController class BuildpackLifecycleFetcher class << self def fetch(buildpack_names, stack_name, lifecycle=Config.config.get(:default_app_lifecycle)) + 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) @@ -12,6 +14,15 @@ def fetch(buildpack_names, stack_name, lifecycle=Config.config.get(:default_app_ 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/app_manifest_message.rb b/app/messages/app_manifest_message.rb index 0f44fc387a8..57998e02864 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 CloudController::Errors::ApiError => e + errors.add(:base, e.message) + end + def validate_docker_buildpacks_combination! return unless requested?(:docker) && (requested?(:buildpack) || requested?(:buildpacks)) 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..952033a5a99 100644 --- a/docs/v3/source/includes/concepts/_lifecycles.md.erb +++ b/docs/v3/source/includes/concepts/_lifecycles.md.erb @@ -40,8 +40,10 @@ 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.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`.
When the `diego_custom_stacks` feature flag is enabled, this can also be a container image URI with the `docker://` scheme. A custom stack requires custom buildpacks or admin buildpacks that are not associated with a stack. Its image layers count toward the app instance's disk quota. +**data.credentials** | _object_ | Credentials used to download custom buildpacks from private Git repositories or to pull custom stacks from private registries, keyed by host. Each entry contains a username/password pair. If any buildpack URL embeds credentials, **data.credentials** must be empty; provide credentials in one place only. + +For custom stacks, Cloud Foundry uses the final repository component of the image URI, excluding its tag or digest, as `CF_STACK`. For example, `docker://registry.example.com/team/cflinuxfs4:1.268.0` sets `CF_STACK` to `cflinuxfs4`. The custom stack and buildpack must be compatible. For records whose historical lifecycle data is no longer available, **data.buildpacks** may be an empty list and **data.stack** may be `null`. @@ -84,8 +86,10 @@ 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.credentials** | _object_ | Credentials used to download the configured buildpacks or to pull custom stacks, keyed by registry host. Each entry contains a username/password pair or a token. Custom stack credentials must be a username/password pair. 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`.
When the `diego_custom_stacks` feature flag is enabled, this can also be a container image URI with the `docker://` scheme. A custom stack requires custom buildpacks or admin buildpacks that are not associated with a stack. Its image layers count toward the app instance's disk quota. + +For custom stacks, Cloud Foundry uses the final repository component of the image URI, excluding its tag or digest, as `CNB_STACK_ID`. For example, `docker://registry.example.com/team/cflinuxfs4:1.268.0` sets `CNB_STACK_ID` to `cflinuxfs4`. The custom stack and buildpack must be compatible. 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..61a1036da4b 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 as the stack for apps that use the buildpack or CNB lifecycle. When disabled, apps can use only platform-provided stacks."], ["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/docs/v3/source/includes/resources/manifests/_object.md.erb b/docs/v3/source/includes/resources/manifests/_object.md.erb index 8727c331be8..2512e5e5322 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 or to pull custom stacks, keyed by host. Each entry contains a username/password pair; CNB buildpack credentials can instead contain a token. Custom stack credentials must be a username/password pair. 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,12 +117,11 @@ 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 container image URI with the `docker://` scheme. A custom stack requires custom buildpacks or admin buildpacks that are not associated with a stack. Custom stack URIs must not include credentials; use **credentials** instead. Custom stack image layers count toward each process instance's **disk_quota**. **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** - #### Process-level configuration This configuration is for the individual process. Each process is created if it does not already exist. diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index 25a4c10866f..08d5a7dbb93 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/image_credential_resolver' require 'cloud_controller/diego/process_guid' require 'cloud_controller/diego/ssh_key' require 'cloud_controller/diego/service_binding_files_builder' @@ -54,6 +55,7 @@ def app_lrp_arguments desired_lrp_builder = LifecycleProtocol.protocol_for_type(process.app.lifecycle_type).desired_lrp_builder(config, process) ports = desired_lrp_builder.ports.dup routes = generate_routes(routing_info) + image_credentials = lrp_image_credentials if allow_ssh? ports << DEFAULT_SSH_PORT @@ -101,14 +103,25 @@ 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: image_credentials.username, + image_password: image_credentials.password, 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 + @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) 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..7e81e558d0a 100644 --- a/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb @@ -1,3 +1,7 @@ +require 'cloud_controller/diego/docker/docker_uri_converter' +require 'cloud_controller/diego/lifecycle_bundle_stack_resolver' +require 'utils/uri_utils' + module VCAP::CloudController module Diego module Buildpack @@ -25,7 +29,6 @@ def initialize(config, opts) def cached_dependencies return nil if @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"buildpack/#{@stack}" 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-#{lifecycle_bundle_stack}-lifecycle" ) ] end def root_fs + 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 @@ -64,16 +69,15 @@ def setup def image_layers return [] unless @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"buildpack/#{@stack}" 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)[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-#{@stack}-lifecycle", + name: "buildpack-#{lifecycle_bundle_stack}-lifecycle", url: LifecycleBundleUriGenerator.uri(lifecycle_bundle), destination_path: '/tmp/lifecycle', layer_type: ::Diego::Bbs::Models::ImageLayer::Type::SHARED, @@ -117,6 +121,16 @@ def port_environment_variables def privileged? @config.get(:diego, :use_privileged_containers_for_running) end + + private + + def lifecycle_bundle_key + :"buildpack/#{lifecycle_bundle_stack}" + end + + def lifecycle_bundle_stack + LifecycleBundleStackResolver.resolve(@stack) + 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..d1ca7a82ef6 100644 --- a/lib/cloud_controller/diego/buildpack/task_action_builder.rb +++ b/lib/cloud_controller/diego/buildpack/task_action_builder.rb @@ -1,5 +1,7 @@ require 'diego/action_builder' require 'cloud_controller/diego/task_environment_variable_collector' +require 'cloud_controller/diego/docker/docker_uri_converter' +require 'cloud_controller/diego/lifecycle_bundle_stack_resolver' require 'credhub/config_helpers' module VCAP::CloudController @@ -58,12 +60,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)[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-#{lifecycle_stack}-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, @@ -91,6 +93,8 @@ def task_environment_variables end def 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 @@ -103,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}-#{lifecycle_stack}-lifecycle" + cache_key: "#{@prefix}-#{lifecycle_bundle_stack}-lifecycle" )] end def lifecycle_bundle_key - :"#{@prefix}/#{lifecycle_stack}" + :"#{@prefix}/#{lifecycle_bundle_stack}" end private @@ -118,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 1c81ce430ce..08643e458c4 100644 --- a/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb @@ -1,3 +1,7 @@ +require 'cloud_controller/diego/docker/docker_uri_converter' +require 'cloud_controller/diego/lifecycle_bundle_stack_resolver' +require 'utils/uri_utils' + module VCAP::CloudController module Diego module CNB @@ -25,7 +29,6 @@ def initialize(config, opts) def cached_dependencies return nil if @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"cnb/#{@stack}" 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-#{lifecycle_bundle_stack}-lifecycle" ) ] end def root_fs + 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 @@ -64,16 +69,15 @@ def setup def image_layers return [] unless @config.get(:diego, :enable_declarative_asset_downloads) - lifecycle_bundle_key = :"cnb/#{@stack}" 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)[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-#{@stack}-lifecycle", + name: "cnb-#{lifecycle_bundle_stack}-lifecycle", url: LifecycleBundleUriGenerator.uri(lifecycle_bundle), destination_path: '/tmp/lifecycle', layer_type: ::Diego::Bbs::Models::ImageLayer::Type::SHARED, @@ -118,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/staging_action_builder.rb b/lib/cloud_controller/diego/cnb/staging_action_builder.rb index 34d750d04ea..2797d16771a 100644 --- a/lib/cloud_controller/diego/cnb/staging_action_builder.rb +++ b/lib/cloud_controller/diego/cnb/staging_action_builder.rb @@ -2,6 +2,7 @@ require 'diego/action_builder' require 'cloud_controller/diego/staging_action_builder' require 'digest/xxhash' +require 'utils/uri_utils' module VCAP::CloudController module Diego @@ -15,9 +16,10 @@ 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) ] + cnb_stack_id = UriUtils.custom_stack_buildpack_identifier(lifecycle_stack) || 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 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..58643a0a0de --- /dev/null +++ b/lib/cloud_controller/diego/image_credential_resolver.rb @@ -0,0 +1,65 @@ +require 'utils/uri_utils' + +module VCAP::CloudController + module Diego + module ImageCredentialResolver + Credentials = Struct.new(:username, :password, keyword_init: true) + private_constant :Credentials + + module_function + + def resolve(primary_username:, primary_password:, lifecycle_data:) + return credentials(primary_username, primary_password) if primary_username.present? && primary_password.present? + return credentials(primary_username, primary_password) unless lifecycle_data.respond_to?(:stack) && custom_stack?(lifecycle_data.stack) + + resolve_from_custom_stack(lifecycle_data) + end + + def resolve_for_staging(primary_username:, primary_password:, lifecycle:) + return credentials(primary_username, primary_password) if primary_username.present? && primary_password.present? + return credentials(nil, nil) unless lifecycle.respond_to?(:credentials_for_staging) && lifecycle.respond_to?(:staging_stack) + return credentials(primary_username, primary_password) unless custom_stack?(lifecycle.staging_stack) + + stack_host = UriUtils.custom_stack_registry_host(lifecycle.staging_stack) + return credentials(nil, nil) unless stack_host + + extract_credentials(lifecycle.credentials_for_staging, stack_host) + end + + def resolve_from_custom_stack(lifecycle_data) + return credentials(nil, nil) unless lifecycle_data.respond_to?(:credentials) && lifecycle_data.respond_to?(:stack) + + stack_host = UriUtils.custom_stack_registry_host(lifecycle_data.stack) + return credentials(nil, nil) unless stack_host + + extract_credentials(lifecycle_data.credentials, stack_host) + 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_by_host, host) + return credentials(nil, nil) unless credentials_by_host.is_a?(Hash) + + credentials_by_host = credentials_by_host.deep_stringify_keys + creds = credentials_by_host[host] + return credentials(nil, nil) unless creds.is_a?(Hash) + + username = creds['username'] + password = creds['password'] + return credentials(nil, nil) unless username.present? && password.present? + + credentials(username, password) + end + private_class_method :extract_credentials + + def credentials(username, password) + Credentials.new(username:, password:) + end + private_class_method :credentials + end + end +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 de76fbe3db8..7c97c143345 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,11 @@ def lifecycle_data(staging_details) ) lifecycle_data.stack = stack + 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 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..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) @@ -13,6 +14,8 @@ def initialize(message, app=nil) @validator = validator_class.new( 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) ) @@ -68,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.rb b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb index 5744c020a57..c46644114ec 100644 --- a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb @@ -19,12 +19,16 @@ def create_lifecycle_data_model(build) def staging_environment_variables { - 'CF_STACK' => staging_stack + 'CF_STACK' => buildpack_stack } end private + def buildpack_stack + UriUtils.custom_stack_buildpack_identifier(staging_stack) || staging_stack + end + def app_stack @package.app.buildpack_lifecycle_data.try(:stack) 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 7196895458c..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,10 +4,13 @@ module VCAP::CloudController class BuildpackLifecycleDataValidator include ActiveModel::Model - attr_accessor :buildpack_infos, :stack, :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_has_auto_detect_buildpack + validate :no_custom_stack_userinfo validate :no_dual_credential_sources def buildpacks_are_uri_or_nil @@ -25,9 +28,32 @@ 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) + 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_uri_is_valid + return unless custom_stack_uri?(stack_name) + return if UriUtils.is_custom_stack_uri?(stack_name) + + 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 return unless url_has_userinfo? && credentials.present? @@ -41,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 1e78212858a..85178b49c11 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] @@ -18,6 +20,8 @@ def initialize(package, staging_message) @validator = validator_class.new( 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) ) @@ -26,11 +30,15 @@ def initialize(package, staging_message) delegate :valid?, :errors, to: :validator def staging_stack - requested_stack || app_stack || VCAP::CloudController::Stack.default.name + @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 3cca3aca34b..e5ffb84a94c 100644 --- a/lib/cloud_controller/diego/staging_action_builder.rb +++ b/lib/cloud_controller/diego/staging_action_builder.rb @@ -1,6 +1,8 @@ require 'credhub/config_helpers' require 'diego/action_builder' require 'digest/xxhash' +require 'cloud_controller/diego/docker/docker_uri_converter' +require 'cloud_controller/diego/lifecycle_bundle_stack_resolver' module VCAP::CloudController module Diego @@ -130,10 +132,14 @@ def image_layers end def 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) + 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 + + "preloaded:#{stack_obj.build_rootfs_image}" + end end def lifecycle_stack @@ -172,21 +178,23 @@ 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( - user: 'vcap', - artifact: 'build artifacts cache', - from: @cache_source, - to: upload_buildpack_artifacts_cache_uri.to_s ) ] + + 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 def skip_detect? @@ -194,7 +202,7 @@ def skip_detect? end def lifecycle_bundle_key - :"#{@prefix}/#{lifecycle_data[:stack]}" + :"#{@prefix}/#{LifecycleBundleStackResolver.resolve(lifecycle_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 f1a4962095d..8b66efb078a 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' @@ -22,6 +23,7 @@ def build_app_task(config, task) task_completion_callback = VCAP::CloudController::Diego::TaskCompletionCallbackGenerator.new(config).generate(task) app_volume_mounts = VCAP::CloudController::Diego::Protocol::AppVolumeMounts.new(task.app).as_json task_action_builder = LifecycleProtocol.protocol_for_type(task.droplet.lifecycle_type).task_action_builder(config, task) + image_credentials = task_image_credentials(task) ::Diego::Bbs::Models::TaskDefinition.new({ completion_callback_url: task_completion_callback, @@ -52,8 +54,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: image_credentials.username, + image_password: image_credentials.password, volume_mounted_files: ServiceBindingFilesBuilder.build(task.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -63,6 +65,7 @@ def build_app_task(config, task) def build_staging_task(config, staging_details) lifecycle_type = staging_details.lifecycle.type action_builder = LifecycleProtocol.protocol_for_type(lifecycle_type).staging_action_builder(config, staging_details) + image_credentials = staging_image_credentials(staging_details) ::Diego::Bbs::Models::TaskDefinition.new({ completion_callback_url: staging_completion_callback(config, staging_details), @@ -93,8 +96,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: image_credentials.username, + image_password: image_credentials.password, volume_mounted_files: ServiceBindingFilesBuilder.build(staging_details.package.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -103,6 +106,23 @@ def build_staging_task(config, staging_details) private + def task_image_credentials(task) + droplet = task.droplet + 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) { '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..f9cb55bccac 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 @@ -44,6 +45,37 @@ def url_has_userinfo?(url) false end + def is_custom_stack_uri?(candidate) + !custom_stack_registry_host(candidate).nil? + end + + def custom_stack_registry_host(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(body) + host.presence || DOCKER_INDEX_SERVER + rescue InvalidDockerURI + nil + end + + def custom_stack_buildpack_identifier(stack_uri) + return nil unless stack_uri.is_a?(String) && stack_uri.start_with?('docker://') + + _, path, = parse_docker_uri(stack_uri.delete_prefix('docker://')) + path.delete_suffix('@sha256').split('/').last + 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 @@ -75,6 +107,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/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..e6e0cec2310 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 @@ -571,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 @@ -609,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' } @@ -1264,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 @@ -1367,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 new file mode 100644 index 00000000000..aeeb07e6f2c --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb @@ -0,0 +1,108 @@ +require 'spec_helper' +require 'cloud_controller/diego/buildpack/desired_lrp_builder' + +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 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 + 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/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_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/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb index 49f1e9267db..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, - 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, - 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 @@ -87,6 +93,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,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', - 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_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_spec.rb index e2bc1bc11c4..570b2af0533 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 final image repository component as CNB_STACK_ID' do + stack_id_env = ::Diego::Bbs::Models::EnvironmentVariable.new(name: 'CNB_STACK_ID', value: 'custom-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/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 new file mode 100644 index 00000000000..e6fcd0baab8 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_custom_stacks_spec.rb @@ -0,0 +1,150 @@ +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] } + let(:lifecycle_type) { Lifecycles::BUILDPACK } + + subject(:validator) do + BuildpackLifecycleDataValidator.new( + buildpack_infos: buildpack_infos, + stack: stack, + stack_name: stack&.name, + lifecycle_type: lifecycle_type + ) + 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, + lifecycle_type: lifecycle_type + ) + 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 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[: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 + + 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/lifecycles/buildpack_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb index a69a89cfd2c..7c761c74fd8 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 @@ -119,9 +119,24 @@ module VCAP::CloudController it 'provides staging environment variables' do staging_message.buildpack_data.stack = 'cool-stack' - expect(buildpack_lifecycle.staging_environment_variables).to eq({ - 'CF_STACK' => 'cool-stack' - }) + expect(buildpack_lifecycle.staging_environment_variables).to eq({ 'CF_STACK' => 'cool-stack' }) + end + + context 'with a custom stack' do + let(:request_data) do + { + buildpacks: ['https://github.com/cloudfoundry/nodejs-buildpack'], + stack: 'docker://registry.example.com/team/cflinuxfs4:1.268.0' + } + end + + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: true) + end + + it 'uses the final image repository component as CF_STACK' do + expect(buildpack_lifecycle.staging_environment_variables).to eq({ 'CF_STACK' => 'cflinuxfs4' }) + end end end @@ -218,6 +233,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 new file mode 100644 index 00000000000..4e844e4edf6 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb @@ -0,0 +1,183 @@ +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_credentials (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 + credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(credentials).to have_attributes(username: 'my-user', password: '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' + } + } + ) + + credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(credentials).to have_attributes(username: 'my-user', password: 'my-pass') + end + end + + describe '#staging_image_credentials (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 + credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(credentials).to have_attributes(username: 'docker-user', password: 'docker-pass') + 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 + credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(credentials).to have_attributes(username: nil, password: nil) + end + end + + describe '#staging_image_credentials (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 + credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(credentials).to have_attributes(username: 'stack-user', password: 'stack-pass') + 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 + credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) + expect(credentials).to have_attributes(username: 'only-user', password: nil) + end + end + + describe '#task_image_credentials (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 + credentials = task_recipe_builder.send(:task_image_credentials, task) + expect(credentials).to have_attributes(username: 'task-user', password: 'task-pass') + end + end + + describe '#task_image_credentials (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 + credentials = task_recipe_builder.send(:task_image_credentials, task) + expect(credentials).to have_attributes(username: 'droplet-user', password: 'droplet-pass') + end + end + 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..23d94009ef0 100644 --- a/spec/unit/lib/utils/uri_utils_spec.rb +++ b/spec/unit/lib/utils/uri_utils_spec.rb @@ -104,6 +104,41 @@ 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 '.custom_stack_buildpack_identifier' do + it 'returns the final repository component without its tag' do + expect(UriUtils.custom_stack_buildpack_identifier('docker://registry.example.com/team/cflinuxfs4:1.268.0')).to eq('cflinuxfs4') + end + + it 'returns the final repository component without its digest' do + digest = 'e118d023acaee5cf13471ead39f68416ad6172ff0899f3257ce1481cd2b28a6a' + + expect(UriUtils.custom_stack_buildpack_identifier("docker://registry.example.com/team/alpine-stack@sha256:#{digest}")).to eq('alpine-stack') + end + + it 'supports Docker Hub shorthand references' do + expect(UriUtils.custom_stack_buildpack_identifier('docker://ubuntu:22.04')).to eq('ubuntu') + end + + it 'returns nil for invalid or non-Docker references' do + expect(UriUtils.custom_stack_buildpack_identifier('docker://registry.example.com/')).to be_nil + expect(UriUtils.custom_stack_buildpack_identifier('cflinuxfs4')).to be_nil + 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 e134e6ae7b4..796d7ff0881 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: { username: 'password' } } } } + 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) @@ -2190,7 +2193,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 @@ -2309,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/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 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 bc6361d8cf38d6dfab4f9e3349c382baa306d3e7 Mon Sep 17 00:00:00 2001 From: johha Date: Thu, 29 Oct 2026 13:14:55 +0100 Subject: [PATCH 2/4] Improve custom stack validation feedback --- .../includes/concepts/_lifecycles.md.erb | 4 ++-- .../buildpack_lifecycle_data_validator.rb | 15 +++++++++---- ...cycle_data_validator_custom_stacks_spec.rb | 21 ++++++++++++++----- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/docs/v3/source/includes/concepts/_lifecycles.md.erb b/docs/v3/source/includes/concepts/_lifecycles.md.erb index 952033a5a99..547b31bd843 100644 --- a/docs/v3/source/includes/concepts/_lifecycles.md.erb +++ b/docs/v3/source/includes/concepts/_lifecycles.md.erb @@ -43,7 +43,7 @@ Name | Type | Description **data.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 container image URI with the `docker://` scheme. A custom stack requires custom buildpacks or admin buildpacks that are not associated with a stack. Its image layers count toward the app instance's disk quota. **data.credentials** | _object_ | Credentials used to download custom buildpacks from private Git repositories or to pull custom stacks from private registries, keyed by host. Each entry contains a username/password pair. If any buildpack URL embeds credentials, **data.credentials** must be empty; provide credentials in one place only. -For custom stacks, Cloud Foundry uses the final repository component of the image URI, excluding its tag or digest, as `CF_STACK`. For example, `docker://registry.example.com/team/cflinuxfs4:1.268.0` sets `CF_STACK` to `cflinuxfs4`. The custom stack and buildpack must be compatible. +During staging with a custom stack, Cloud Foundry uses the final repository component of the image URI, excluding its tag or digest, as `CF_STACK`. For example, `docker://registry.example.com/team/cflinuxfs4:1.268.0` sets `CF_STACK` to `cflinuxfs4`. The custom stack and buildpack must be compatible. For records whose historical lifecycle data is no longer available, **data.buildpacks** may be an empty list and **data.stack** may be `null`. @@ -89,7 +89,7 @@ Name | Type | Description **data.credentials** | _object_ | Credentials used to download the configured buildpacks or to pull custom stacks, keyed by registry host. Each entry contains a username/password pair or a token. Custom stack credentials must be a username/password pair. 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`.
When the `diego_custom_stacks` feature flag is enabled, this can also be a container image URI with the `docker://` scheme. A custom stack requires custom buildpacks or admin buildpacks that are not associated with a stack. Its image layers count toward the app instance's disk quota. -For custom stacks, Cloud Foundry uses the final repository component of the image URI, excluding its tag or digest, as `CNB_STACK_ID`. For example, `docker://registry.example.com/team/cflinuxfs4:1.268.0` sets `CNB_STACK_ID` to `cflinuxfs4`. The custom stack and buildpack must be compatible. +During staging with a custom stack, Cloud Foundry uses the final repository component of the image URI, excluding its tag or digest, as `CNB_STACK_ID`. For example, `docker://registry.example.com/team/cflinuxfs4:1.268.0` sets `CNB_STACK_ID` to `cflinuxfs4`. The custom stack and buildpack must be compatible. 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/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb index 71aa45ce8c1..f19e7156ce0 100644 --- a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb +++ b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb @@ -30,15 +30,22 @@ 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? + return unless stack.nil? + + message = if FeatureFlag.enabled?(:diego_custom_stacks) + 'must be an existing stack or a valid container image URI starting with "docker://"' + else + 'must be an existing stack' + end + errors.add(:stack, message) end def custom_stack_uri_is_valid + return if stack return unless custom_stack_uri?(stack_name) return if UriUtils.is_custom_stack_uri?(stack_name) - errors.add(:stack, 'must be a valid container image URI') + errors.add(:stack, 'must be a valid container image URI starting with "docker://"') end def no_custom_stack_userinfo @@ -51,7 +58,7 @@ 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') + errors.add(:base, 'You must either specify a custom buildpack URL or your platform operator must provide an enabled buildpack that is not pinned to a stack') end def no_dual_credential_sources 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 e6fcd0baab8..b87980bad80 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 @@ -57,8 +57,8 @@ module VCAP::CloudController 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' + expect(validator.errors.full_messages).to include( + 'You must either specify a custom buildpack URL or your platform operator must provide an enabled buildpack that is not pinned to a stack' ) end end @@ -85,8 +85,8 @@ module VCAP::CloudController 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' + expect(validator.errors.full_messages).to include( + 'You must either specify a custom buildpack URL or your platform operator must provide an enabled buildpack that is not pinned to a stack' ) end end @@ -105,7 +105,18 @@ module VCAP::CloudController 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') + expect(validator.errors.full_messages).to contain_exactly('Stack must be a valid container image URI starting with "docker://"') + end + end + + context 'with a stack that is neither an existing stack nor a valid custom stack URI' do + let(:custom_stack_uri) { 'ocker://registry.example.com/org/stack:tag' } + + it 'reports both supported stack formats' do + expect(validator).not_to be_valid + expect(validator.errors.full_messages).to contain_exactly( + 'Stack must be an existing stack or a valid container image URI starting with "docker://"' + ) end end end From 202232d29b688d4b16a7a54371683810270867d7 Mon Sep 17 00:00:00 2001 From: johha Date: Thu, 29 Oct 2026 13:58:06 +0100 Subject: [PATCH 3/4] Separate Docker and custom stack image credentials --- .../diego/app_recipe_builder.rb | 18 +- .../diego/custom_stack_credential_resolver.rb | 23 +++ .../diego/image_credential_resolver.rb | 65 ------- .../diego/task_recipe_builder.rb | 39 ++-- .../diego/app_recipe_builder_spec.rb | 25 +++ .../custom_stack_credential_resolver_spec.rb | 59 ++++++ .../task_recipe_builder_custom_stacks_spec.rb | 183 ------------------ .../diego/task_recipe_builder_spec.rb | 91 +++++++++ 8 files changed, 230 insertions(+), 273 deletions(-) create mode 100644 lib/cloud_controller/diego/custom_stack_credential_resolver.rb delete mode 100644 lib/cloud_controller/diego/image_credential_resolver.rb create mode 100644 spec/unit/lib/cloud_controller/diego/custom_stack_credential_resolver_spec.rb delete mode 100644 spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index 08d5a7dbb93..f66da841513 100644 --- a/lib/cloud_controller/diego/app_recipe_builder.rb +++ b/lib/cloud_controller/diego/app_recipe_builder.rb @@ -4,7 +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/image_credential_resolver' +require 'cloud_controller/diego/custom_stack_credential_resolver' require 'cloud_controller/diego/process_guid' require 'cloud_controller/diego/ssh_key' require 'cloud_controller/diego/service_binding_files_builder' @@ -103,8 +103,8 @@ 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: image_credentials.username, - image_password: image_credentials.password, + image_username: image_credentials[:username], + image_password: image_credentials[:password], volume_mounted_files: ServiceBindingFilesBuilder.build(process) }.compact rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -112,13 +112,13 @@ def app_lrp_arguments end def lrp_image_credentials - @lrp_image_credentials ||= begin + # The app lifecycle determines the LRP rootfs, even if its droplet has a different lifecycle. + if process.app.docker? droplet = process.desired_droplet - ImageCredentialResolver.resolve( - primary_username: droplet.docker_receipt_username, - primary_password: droplet.docker_receipt_password, - lifecycle_data: process.app.lifecycle_data - ) + { username: droplet.docker_receipt_username, password: droplet.docker_receipt_password } + else + lifecycle_data = process.app.lifecycle_data + CustomStackCredentialResolver.resolve(stack_uri: lifecycle_data.stack, credentials_by_host: lifecycle_data.credentials) end end diff --git a/lib/cloud_controller/diego/custom_stack_credential_resolver.rb b/lib/cloud_controller/diego/custom_stack_credential_resolver.rb new file mode 100644 index 00000000000..b5e0db90b4a --- /dev/null +++ b/lib/cloud_controller/diego/custom_stack_credential_resolver.rb @@ -0,0 +1,23 @@ +require 'utils/uri_utils' + +module VCAP::CloudController + module Diego + module CustomStackCredentialResolver + module_function + + def resolve(stack_uri:, credentials_by_host:) + registry_host = UriUtils.custom_stack_registry_host(stack_uri) + return { username: nil, password: nil } unless registry_host && credentials_by_host.is_a?(Hash) + + registry_entry = credentials_by_host.deep_stringify_keys[registry_host] + return { username: nil, password: nil } unless registry_entry.is_a?(Hash) + + username = registry_entry['username'] + password = registry_entry['password'] + return { username: nil, password: nil } unless username.present? && password.present? + + { username:, password: } + end + end + end +end diff --git a/lib/cloud_controller/diego/image_credential_resolver.rb b/lib/cloud_controller/diego/image_credential_resolver.rb deleted file mode 100644 index 58643a0a0de..00000000000 --- a/lib/cloud_controller/diego/image_credential_resolver.rb +++ /dev/null @@ -1,65 +0,0 @@ -require 'utils/uri_utils' - -module VCAP::CloudController - module Diego - module ImageCredentialResolver - Credentials = Struct.new(:username, :password, keyword_init: true) - private_constant :Credentials - - module_function - - def resolve(primary_username:, primary_password:, lifecycle_data:) - return credentials(primary_username, primary_password) if primary_username.present? && primary_password.present? - return credentials(primary_username, primary_password) unless lifecycle_data.respond_to?(:stack) && custom_stack?(lifecycle_data.stack) - - resolve_from_custom_stack(lifecycle_data) - end - - def resolve_for_staging(primary_username:, primary_password:, lifecycle:) - return credentials(primary_username, primary_password) if primary_username.present? && primary_password.present? - return credentials(nil, nil) unless lifecycle.respond_to?(:credentials_for_staging) && lifecycle.respond_to?(:staging_stack) - return credentials(primary_username, primary_password) unless custom_stack?(lifecycle.staging_stack) - - stack_host = UriUtils.custom_stack_registry_host(lifecycle.staging_stack) - return credentials(nil, nil) unless stack_host - - extract_credentials(lifecycle.credentials_for_staging, stack_host) - end - - def resolve_from_custom_stack(lifecycle_data) - return credentials(nil, nil) unless lifecycle_data.respond_to?(:credentials) && lifecycle_data.respond_to?(:stack) - - stack_host = UriUtils.custom_stack_registry_host(lifecycle_data.stack) - return credentials(nil, nil) unless stack_host - - extract_credentials(lifecycle_data.credentials, stack_host) - 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_by_host, host) - return credentials(nil, nil) unless credentials_by_host.is_a?(Hash) - - credentials_by_host = credentials_by_host.deep_stringify_keys - creds = credentials_by_host[host] - return credentials(nil, nil) unless creds.is_a?(Hash) - - username = creds['username'] - password = creds['password'] - return credentials(nil, nil) unless username.present? && password.present? - - credentials(username, password) - end - private_class_method :extract_credentials - - def credentials(username, password) - Credentials.new(username:, password:) - end - private_class_method :credentials - end - end -end diff --git a/lib/cloud_controller/diego/task_recipe_builder.rb b/lib/cloud_controller/diego/task_recipe_builder.rb index 8b66efb078a..5fb3f9e5267 100644 --- a/lib/cloud_controller/diego/task_recipe_builder.rb +++ b/lib/cloud_controller/diego/task_recipe_builder.rb @@ -3,7 +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/custom_stack_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' @@ -54,8 +54,8 @@ def build_app_task(config, task) "app:#{task.app_guid}" ] ), - image_username: image_credentials.username, - image_password: image_credentials.password, + image_username: image_credentials[:username], + image_password: image_credentials[:password], volume_mounted_files: ServiceBindingFilesBuilder.build(task.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -96,8 +96,8 @@ def build_staging_task(config, staging_details) "app:#{staging_details.package.app_guid}" ] ), - image_username: image_credentials.username, - image_password: image_credentials.password, + image_username: image_credentials[:username], + image_password: image_credentials[:password], volume_mounted_files: ServiceBindingFilesBuilder.build(staging_details.package.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -107,20 +107,27 @@ def build_staging_task(config, staging_details) private def task_image_credentials(task) - droplet = task.droplet - ImageCredentialResolver.resolve( - primary_username: droplet.docker_receipt_username, - primary_password: droplet.docker_receipt_password, - lifecycle_data: task.app.lifecycle_data - ) + # The droplet lifecycle determines the task rootfs, even if the app lifecycle differs. + if task.droplet.docker? + { username: task.droplet.docker_receipt_username, password: task.droplet.docker_receipt_password } + else + lifecycle_data = task.app.lifecycle_data + credentials_by_host = lifecycle_data.credentials if lifecycle_data.respond_to?(:credentials) + CustomStackCredentialResolver.resolve(stack_uri: lifecycle_data.stack, credentials_by_host: credentials_by_host) + end 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 - ) + if staging_details.lifecycle.type == Lifecycles::DOCKER + username = staging_details.package.docker_username + password = staging_details.package.docker_password + return { username:, password: } if username.present? && password.present? + + { username: nil, password: nil } + else + lifecycle = staging_details.lifecycle + CustomStackCredentialResolver.resolve(stack_uri: lifecycle.staging_stack, credentials_by_host: lifecycle.credentials_for_staging) + end end def metric_tags(source) 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 66cc1b79180..15b650d2fa1 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 @@ -377,6 +377,18 @@ module Diego expect(lrp.image_username).to eq('stack-user') expect(lrp.image_password).to eq('stack-password') end + + context 'when a Docker droplet is assigned to the buildpack app' do + let(:droplet) do + create(:droplet_model, :docker, app: app_model, docker_receipt_username: 'docker-user', docker_receipt_password: 'docker-pass') + end + + it 'uses credentials for the app stack rather than the Docker droplet image' do + lrp = builder.build_app_lrp + expect(lrp.image_username).to eq('stack-user') + expect(lrp.image_password).to eq('stack-password') + end + end end context 'when the space is not entitled to any isolation segments' do @@ -1047,6 +1059,19 @@ module Diego expect(lrp.image_layers).to eq(expected_image_layers) end + context 'when the CNB app uses a private custom stack' do + before do + app_model.cnb_lifecycle_data.update(stack: 'docker://registry.example.com/team/stack:latest', + credentials: { 'registry.example.com' => { 'username' => 'stack-user', 'password' => 'stack-pass' } }) + end + + it 'passes the stack registry credentials to Diego' do + lrp = builder.build_app_lrp + expect(lrp.image_username).to eq('stack-user') + expect(lrp.image_password).to eq('stack-pass') + end + end + describe 'ssh' do before do process.app.update(enable_ssh: true) diff --git a/spec/unit/lib/cloud_controller/diego/custom_stack_credential_resolver_spec.rb b/spec/unit/lib/cloud_controller/diego/custom_stack_credential_resolver_spec.rb new file mode 100644 index 00000000000..f3ace5e5834 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/custom_stack_credential_resolver_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' +require 'cloud_controller/diego/custom_stack_credential_resolver' + +module VCAP::CloudController + module Diego + RSpec.describe CustomStackCredentialResolver do + let(:stack_uri) { 'docker://registry.example.com/team/stack:latest' } + let(:credentials_by_host) do + { 'registry.example.com' => { 'username' => 'stack-user', 'password' => 'stack-pass' } } + end + + subject(:image_credentials) { described_class.resolve(stack_uri:, credentials_by_host:) } + + it 'selects credentials for the stack registry' do + expect(image_credentials).to eq(username: 'stack-user', password: 'stack-pass') + end + + context 'with symbol keys' do + let(:credentials_by_host) { { 'registry.example.com': { username: 'stack-user', password: 'stack-pass' } } } + + it 'accepts the credentials' do + expect(image_credentials).to eq(username: 'stack-user', password: 'stack-pass') + end + end + + context 'with credentials for another registry' do + let(:credentials_by_host) { { 'other.example.com' => { 'username' => 'other', 'password' => 'secret' } } } + + it 'does not forward them' do + expect(image_credentials).to eq(username: nil, password: nil) + end + end + + context 'with an incomplete pair' do + let(:credentials_by_host) { { 'registry.example.com' => { 'username' => 'stack-user' } } } + + it 'does not forward the partial pair' do + expect(image_credentials).to eq(username: nil, password: nil) + end + end + + context 'without registry credentials' do + let(:credentials_by_host) { nil } + + it 'uses no image credentials' do + expect(image_credentials).to eq(username: nil, password: nil) + end + end + + context 'with a system stack' do + let(:stack_uri) { 'cflinuxfs4' } + + it 'does not use registry credentials' do + expect(image_credentials).to eq(username: nil, password: nil) + 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 deleted file mode 100644 index 4e844e4edf6..00000000000 --- a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_custom_stacks_spec.rb +++ /dev/null @@ -1,183 +0,0 @@ -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_credentials (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 - credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) - expect(credentials).to have_attributes(username: 'my-user', password: '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' - } - } - ) - - credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) - expect(credentials).to have_attributes(username: 'my-user', password: 'my-pass') - end - end - - describe '#staging_image_credentials (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 - credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) - expect(credentials).to have_attributes(username: 'docker-user', password: 'docker-pass') - 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 - credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) - expect(credentials).to have_attributes(username: nil, password: nil) - end - end - - describe '#staging_image_credentials (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 - credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) - expect(credentials).to have_attributes(username: 'stack-user', password: 'stack-pass') - 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 - credentials = task_recipe_builder.send(:staging_image_credentials, staging_details) - expect(credentials).to have_attributes(username: 'only-user', password: nil) - end - end - - describe '#task_image_credentials (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 - credentials = task_recipe_builder.send(:task_image_credentials, task) - expect(credentials).to have_attributes(username: 'task-user', password: 'task-pass') - end - end - - describe '#task_image_credentials (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 - credentials = task_recipe_builder.send(:task_image_credentials, task) - expect(credentials).to have_attributes(username: 'droplet-user', password: 'droplet-pass') - end - end - end - end -end diff --git a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb index 9daaf547724..1047b185402 100644 --- a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb @@ -238,6 +238,36 @@ module Diego expect(result.environment_variables).to eq(lifecycle_environment_variables) end + context 'when staging with a custom stack' do + let(:request_data) do + { + stack: 'docker://registry.example.com/team/stack:latest', + credentials: { 'registry.example.com' => { 'username' => 'stack-user', 'password' => 'stack-pass' } } + } + end + + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: true) + end + + it 'sets the registry credentials on the Diego task' do + result = task_recipe_builder.build_staging_task(config, staging_details) + expect(result.image_username).to eq('stack-user') + expect(result.image_password).to eq('stack-pass') + end + + context 'with a CNB lifecycle' do + let(:app) { create(:app_model, :cnb, name: 'MyApp', guid: 'banana-guid', space: space) } + let(:lifecycle_type) { Lifecycles::CNB } + + it 'sets the registry credentials on the Diego task' do + result = task_recipe_builder.build_staging_task(config, staging_details) + expect(result.image_username).to eq('stack-user') + expect(result.image_password).to eq('stack-pass') + end + end + end + context 'when there is no isolation segment' do let(:isolation_segment) { nil } @@ -398,6 +428,16 @@ module Diego expect(result.image_username).to eq('dockeruser') expect(result.image_password).to eq('dockerpass') end + + context 'when Docker package credentials are incomplete' do + let(:package) { create(:package_model, :docker, app: app, docker_username: 'dockeruser', docker_password: nil) } + + it 'does not pass the partial pair to Diego' do + result = task_recipe_builder.build_staging_task(config, staging_details) + expect(result.image_username).to be_empty + expect(result.image_password).to be_empty + end + end end end @@ -568,6 +608,29 @@ module Diego expect(result.volume_mounted_files).to be_empty end + context 'when the app uses a custom stack' do + before do + app.lifecycle_data.update(stack: 'docker://registry.example.com/team/stack:latest', + credentials: { 'registry.example.com' => { 'username' => 'stack-user', 'password' => 'stack-pass' } }) + end + + it 'sets the registry credentials on the Diego task' do + result = task_recipe_builder.build_app_task(config, task) + expect(result.image_username).to eq('stack-user') + expect(result.image_password).to eq('stack-pass') + end + end + + context 'when the app has Docker lifecycle data but the task uses a buildpack droplet' do + let(:app) { create(:app_model, :docker, name: 'MyApp', guid: 'banana-guid', space: space) } + + it 'does not try to read custom stack credentials from the Docker lifecycle' do + result = task_recipe_builder.build_app_task(config, task) + expect(result.image_username).to be_empty + expect(result.image_password).to be_empty + end + end + context 'when a volume mount is provided' do let(:service_instance) { create(:managed_service_instance, space: app.space) } let(:multiple_volume_mounts) do @@ -733,6 +796,34 @@ module Diego expect(result.volume_mounted_files).to be_empty end + context 'when Docker droplet credentials are incomplete' do + let(:droplet) do + create(:droplet_model, :docker, app: app, docker_receipt_username: 'dockerusername', docker_receipt_password: nil) + end + + it 'preserves the receipt username for the runtime task' do + result = task_recipe_builder.build_app_task(config, task) + expect(result.image_username).to eq('dockerusername') + expect(result.image_password).to be_empty + end + end + + context 'when the buildpack app also has custom stack credentials' do + let(:droplet) { create(:droplet_model, :docker, app: app) } + + before do + app.lifecycle_data.update(stack: 'docker://registry.example.com/team/stack:latest', + credentials: { 'registry.example.com' => { 'username' => 'stack-user', 'password' => 'stack-pass' } }) + end + + it 'does not use the stack credentials for the Docker task image' do + result = task_recipe_builder.build_app_task(config, task) + expect(result.root_fs).to eq('docker://potato-stack') + expect(result.image_username).to be_empty + expect(result.image_password).to be_empty + end + end + context 'when a volume mount is provided' do let(:service_instance) { create(:managed_service_instance, space: app.space) } let(:multiple_volume_mounts) do From 618d4831e051d04df2407361c666fa98609f7237 Mon Sep 17 00:00:00 2001 From: johha Date: Fri, 30 Oct 2026 11:33:00 +0100 Subject: [PATCH 4/4] Refine custom stack validation, credentials, and test coverage Preserve configured buildpacks during stack-only updates and reject non-string custom stack credentials before constructing Diego messages. Reject empty image repository path components, align staging lifecycle asset cache keys with the resolved platform stack, and expand image credential hashes directly into Diego recipes. Clarify staging-only build credentials versus app-level runtime credentials. Consolidate lifecycle tests, reuse existing fixtures, and remove duplicated coverage while retaining classic, CNB, and mixed-lifecycle regressions. --- .../includes/concepts/_lifecycles.md.erb | 4 +- .../includes/resources/builds/_create.md.erb | 2 +- .../diego/app_recipe_builder.rb | 6 +- .../diego/custom_stack_credential_resolver.rb | 8 +- .../diego/lifecycles/app_base_lifecycle.rb | 6 +- .../buildpack_lifecycle_data_validator.rb | 16 +++ .../diego/staging_action_builder.rb | 28 ++--- .../diego/task_recipe_builder.rb | 14 +-- lib/utils/uri_utils.rb | 6 +- spec/unit/actions/build_create_spec.rb | 32 ++++++ .../controllers/v3/apps_controller_spec.rb | 20 ++-- .../buildpack_lifecycle_fetcher_spec.rb | 2 +- .../desired_lrp_builder_custom_stacks_spec.rb | 108 ------------------ .../buildpack/desired_lrp_builder_spec.rb | 39 +++++++ .../buildpack/staging_action_builder_spec.rb | 9 -- .../diego/cnb/lifecycle_protocol_spec.rb | 22 +--- .../custom_stack_credential_resolver_spec.rb | 12 +- .../app_buildpack_lifecycle_spec.rb | 17 +-- .../lifecycles/app_cnb_lifecycle_spec.rb | 17 +-- .../diego/lifecycles/app_lifecycle_shared.rb | 53 +++++++++ ...cycle_data_validator_custom_stacks_spec.rb | 88 +++++++------- .../diego/staging_action_builder_spec.rb | 47 ++++++++ spec/unit/lib/utils/uri_utils_spec.rb | 7 ++ .../repositories/app_event_repository_spec.rb | 21 ---- 24 files changed, 299 insertions(+), 285 deletions(-) delete mode 100644 spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb diff --git a/docs/v3/source/includes/concepts/_lifecycles.md.erb b/docs/v3/source/includes/concepts/_lifecycles.md.erb index 547b31bd843..726007e626f 100644 --- a/docs/v3/source/includes/concepts/_lifecycles.md.erb +++ b/docs/v3/source/includes/concepts/_lifecycles.md.erb @@ -41,7 +41,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`.
When the `diego_custom_stacks` feature flag is enabled, this can also be a container image URI with the `docker://` scheme. A custom stack requires custom buildpacks or admin buildpacks that are not associated with a stack. Its image layers count toward the app instance's disk quota. -**data.credentials** | _object_ | Credentials used to download custom buildpacks from private Git repositories or to pull custom stacks from private registries, keyed by host. Each entry contains a username/password pair. If any buildpack URL embeds credentials, **data.credentials** must be empty; provide credentials in one place only. +**data.credentials** | _object_ | Credentials used to download custom buildpacks from private Git repositories or to pull custom stacks from private registries, keyed by host. Each entry contains a username/password pair. If any buildpack URL embeds credentials, **data.credentials** must be empty; provide credentials in one place only.
For private custom stacks, configure credentials on the app lifecycle for running instances and tasks. During staging with a custom stack, Cloud Foundry uses the final repository component of the image URI, excluding its tag or digest, as `CF_STACK`. For example, `docker://registry.example.com/team/cflinuxfs4:1.268.0` sets `CF_STACK` to `cflinuxfs4`. The custom stack and buildpack must be compatible. @@ -86,7 +86,7 @@ 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 or to pull custom stacks, keyed by registry host. Each entry contains a username/password pair or a token. Custom stack credentials must be a username/password pair. CNB buildpack URLs must not embed credentials; use **data.credentials** instead. +**data.credentials** | _object_ | Credentials used to download the configured buildpacks or to pull custom stacks, keyed by registry host. Each entry contains a username/password pair or a token. Custom stack credentials must be a username/password pair. CNB buildpack URLs must not embed credentials; use **data.credentials** instead.
For private custom stacks, configure credentials on the app lifecycle for running instances and tasks. **data.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 container image URI with the `docker://` scheme. A custom stack requires custom buildpacks or admin buildpacks that are not associated with a stack. Its image layers count toward the app instance's disk quota. During staging with a custom stack, Cloud Foundry uses the final repository component of the image URI, excluding its tag or digest, as `CNB_STACK_ID`. For example, `docker://registry.example.com/team/cflinuxfs4:1.268.0` sets `CNB_STACK_ID` to `cflinuxfs4`. The custom stack and buildpack must be compatible. diff --git a/docs/v3/source/includes/resources/builds/_create.md.erb b/docs/v3/source/includes/resources/builds/_create.md.erb index 1ff9e360122..249c2714555 100644 --- a/docs/v3/source/includes/resources/builds/_create.md.erb +++ b/docs/v3/source/includes/resources/builds/_create.md.erb @@ -40,7 +40,7 @@ Name | Type | Description Name | Type | Description | Default ---- | ---- | ----------- | ------- -**lifecycle** | [_lifecycle object_](#the-lifecycle-object) | Lifecycle information for a build | lifecycle on the app +**lifecycle** | [_lifecycle object_](#the-lifecycle-object) | Lifecycle information for a build.
Credentials supplied here apply only to staging and do not update the app's stored credentials. | lifecycle on the app **staging_memory_in_mb** | _integer_ | Memory in MB allocated for staging of the build **staging_disk_in_mb** | _integer_ | Disk space in MB allocated for staging of the build **staging_log_rate_limit_bytes_per_second** | _integer_ | Log rate limit in bytes per second allocated for staging of the build diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index f66da841513..cedf097dd06 100644 --- a/lib/cloud_controller/diego/app_recipe_builder.rb +++ b/lib/cloud_controller/diego/app_recipe_builder.rb @@ -55,7 +55,6 @@ def app_lrp_arguments desired_lrp_builder = LifecycleProtocol.protocol_for_type(process.app.lifecycle_type).desired_lrp_builder(config, process) ports = desired_lrp_builder.ports.dup routes = generate_routes(routing_info) - image_credentials = lrp_image_credentials if allow_ssh? ports << DEFAULT_SSH_PORT @@ -103,8 +102,7 @@ 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: image_credentials[:username], - image_password: image_credentials[:password], + **lrp_image_credentials, volume_mounted_files: ServiceBindingFilesBuilder.build(process) }.compact rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -115,7 +113,7 @@ def lrp_image_credentials # The app lifecycle determines the LRP rootfs, even if its droplet has a different lifecycle. if process.app.docker? droplet = process.desired_droplet - { username: droplet.docker_receipt_username, password: droplet.docker_receipt_password } + { image_username: droplet.docker_receipt_username, image_password: droplet.docker_receipt_password } else lifecycle_data = process.app.lifecycle_data CustomStackCredentialResolver.resolve(stack_uri: lifecycle_data.stack, credentials_by_host: lifecycle_data.credentials) diff --git a/lib/cloud_controller/diego/custom_stack_credential_resolver.rb b/lib/cloud_controller/diego/custom_stack_credential_resolver.rb index b5e0db90b4a..da3fa890ab6 100644 --- a/lib/cloud_controller/diego/custom_stack_credential_resolver.rb +++ b/lib/cloud_controller/diego/custom_stack_credential_resolver.rb @@ -7,16 +7,16 @@ module CustomStackCredentialResolver def resolve(stack_uri:, credentials_by_host:) registry_host = UriUtils.custom_stack_registry_host(stack_uri) - return { username: nil, password: nil } unless registry_host && credentials_by_host.is_a?(Hash) + return { image_username: nil, image_password: nil } unless registry_host && credentials_by_host.is_a?(Hash) registry_entry = credentials_by_host.deep_stringify_keys[registry_host] - return { username: nil, password: nil } unless registry_entry.is_a?(Hash) + return { image_username: nil, image_password: nil } unless registry_entry.is_a?(Hash) username = registry_entry['username'] password = registry_entry['password'] - return { username: nil, password: nil } unless username.present? && password.present? + return { image_username: nil, image_password: nil } unless username.present? && password.present? - { username:, password: } + { image_username: username, image_password: password } end end end diff --git a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb index 02654d89777..36a5fd89738 100644 --- a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb @@ -53,10 +53,12 @@ def lifecycle_data_for_validation(app) end def validating_stored_buildpacks? + return false if message.buildpack_data.requested?(:buildpacks) + return true if message.buildpack_data.requested?(:stack) && message.buildpack_data.stack.present? + # Stored URLs are needed to detect embedded userinfo when credentials are added separately. message.buildpack_data.requested?(:credentials) && - message.buildpack_data.credentials.present? && - !message.buildpack_data.requested?(:buildpacks) + message.buildpack_data.credentials.present? end def effective_credentials(app) 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 f19e7156ce0..ea5ffa1e1d3 100644 --- a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb +++ b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb @@ -11,6 +11,7 @@ class BuildpackLifecycleDataValidator validate :stack_exists_in_db validate :custom_stack_has_auto_detect_buildpack validate :no_custom_stack_userinfo + validate :custom_stack_credentials_are_strings validate :no_dual_credential_sources def buildpacks_are_uri_or_nil @@ -61,6 +62,21 @@ def custom_stack_has_auto_detect_buildpack errors.add(:base, 'You must either specify a custom buildpack URL or your platform operator must provide an enabled buildpack that is not pinned to a stack') end + def custom_stack_credentials_are_strings + registry_host = UriUtils.custom_stack_registry_host(stack_name) + return unless registry_host && credentials.is_a?(Hash) + + registry_credentials = credentials.deep_stringify_keys[registry_host] + return unless registry_credentials.is_a?(Hash) + + %w[username password].each do |field| + value = registry_credentials[field] + next if value.nil? || value.is_a?(String) + + errors.add(:credentials, "#{field} for custom stack registry \"#{registry_host}\" must be a string") + end + end + def no_dual_credential_sources return unless url_has_userinfo? && credentials.present? diff --git a/lib/cloud_controller/diego/staging_action_builder.rb b/lib/cloud_controller/diego/staging_action_builder.rb index e5ffb84a94c..fc071640b80 100644 --- a/lib/cloud_controller/diego/staging_action_builder.rb +++ b/lib/cloud_controller/diego/staging_action_builder.rb @@ -45,7 +45,7 @@ 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}-#{lifecycle_bundle_stack}-lifecycle" ) ] @@ -94,7 +94,7 @@ def image_layers layers = [ ::Diego::Bbs::Models::ImageLayer.new( - name: "#{@prefix}-#{lifecycle_stack}-lifecycle", + name: "#{@prefix}-#{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, @@ -178,23 +178,21 @@ 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( + user: 'vcap', + artifact: 'build artifacts cache', + from: @cache_source, + to: upload_buildpack_artifacts_cache_uri.to_s ) ] - - 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 def skip_detect? @@ -202,7 +200,11 @@ def skip_detect? end def lifecycle_bundle_key - :"#{@prefix}/#{LifecycleBundleStackResolver.resolve(lifecycle_stack)}" + :"#{@prefix}/#{lifecycle_bundle_stack}" + end + + def lifecycle_bundle_stack + LifecycleBundleStackResolver.resolve(lifecycle_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 5fb3f9e5267..4e55e677a09 100644 --- a/lib/cloud_controller/diego/task_recipe_builder.rb +++ b/lib/cloud_controller/diego/task_recipe_builder.rb @@ -23,7 +23,6 @@ def build_app_task(config, task) task_completion_callback = VCAP::CloudController::Diego::TaskCompletionCallbackGenerator.new(config).generate(task) app_volume_mounts = VCAP::CloudController::Diego::Protocol::AppVolumeMounts.new(task.app).as_json task_action_builder = LifecycleProtocol.protocol_for_type(task.droplet.lifecycle_type).task_action_builder(config, task) - image_credentials = task_image_credentials(task) ::Diego::Bbs::Models::TaskDefinition.new({ completion_callback_url: task_completion_callback, @@ -54,8 +53,7 @@ def build_app_task(config, task) "app:#{task.app_guid}" ] ), - image_username: image_credentials[:username], - image_password: image_credentials[:password], + **task_image_credentials(task), volume_mounted_files: ServiceBindingFilesBuilder.build(task.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -65,7 +63,6 @@ def build_app_task(config, task) def build_staging_task(config, staging_details) lifecycle_type = staging_details.lifecycle.type action_builder = LifecycleProtocol.protocol_for_type(lifecycle_type).staging_action_builder(config, staging_details) - image_credentials = staging_image_credentials(staging_details) ::Diego::Bbs::Models::TaskDefinition.new({ completion_callback_url: staging_completion_callback(config, staging_details), @@ -96,8 +93,7 @@ def build_staging_task(config, staging_details) "app:#{staging_details.package.app_guid}" ] ), - image_username: image_credentials[:username], - image_password: image_credentials[:password], + **staging_image_credentials(staging_details), volume_mounted_files: ServiceBindingFilesBuilder.build(staging_details.package.app) }.compact) rescue ServiceBindingFilesBuilder::IncompatibleBindings => e @@ -109,7 +105,7 @@ def build_staging_task(config, staging_details) def task_image_credentials(task) # The droplet lifecycle determines the task rootfs, even if the app lifecycle differs. if task.droplet.docker? - { username: task.droplet.docker_receipt_username, password: task.droplet.docker_receipt_password } + { image_username: task.droplet.docker_receipt_username, image_password: task.droplet.docker_receipt_password } else lifecycle_data = task.app.lifecycle_data credentials_by_host = lifecycle_data.credentials if lifecycle_data.respond_to?(:credentials) @@ -121,9 +117,9 @@ def staging_image_credentials(staging_details) if staging_details.lifecycle.type == Lifecycles::DOCKER username = staging_details.package.docker_username password = staging_details.package.docker_password - return { username:, password: } if username.present? && password.present? + return { image_username: username, image_password: password } if username.present? && password.present? - { username: nil, password: nil } + { image_username: nil, image_password: nil } else lifecycle = staging_details.lifecycle CustomStackCredentialResolver.resolve(stack_uri: lifecycle.staging_stack, credentials_by_host: lifecycle.credentials_for_staging) diff --git a/lib/utils/uri_utils.rb b/lib/utils/uri_utils.rb index f9cb55bccac..e6e7454ee03 100644 --- a/lib/utils/uri_utils.rb +++ b/lib/utils/uri_utils.rb @@ -112,7 +112,7 @@ def parse_docker_uri(docker_uri) 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) - raise InvalidDockerURI.new "Invalid image name [#{path}]" unless DOCKER_PATH_REGEX =~ path + raise InvalidDockerURI.new "Invalid image name [#{path}]" unless valid_docker_path?(path) raise InvalidDockerURI.new "Invalid image tag [#{tag_digest}]" if tag_digest && DOCKER_TAG_DIGEST_REGEX !~ tag_digest # if only sha256 presented, we add hash value as fragment to the uri, @@ -128,6 +128,10 @@ def parse_docker_uri(docker_uri) private + def valid_docker_path?(path) + DOCKER_PATH_REGEX.match?(path) && path.split('/', -1).none?(&:empty?) + end + def official_docker_registry(host) host == DOCKER_INDEX_SERVER end diff --git a/spec/unit/actions/build_create_spec.rb b/spec/unit/actions/build_create_spec.rb index 697670b2f66..f4dd831e568 100644 --- a/spec/unit/actions/build_create_spec.rb +++ b/spec/unit/actions/build_create_spec.rb @@ -266,6 +266,38 @@ module VCAP::CloudController end end + %w[buildpack cnb].each do |lifecycle_type| + context "with #{lifecycle_type} build-only custom stack credentials" do + let(:app) { create(:app_model, lifecycle_type.to_sym, space:) } + let(:lifecycle) { LifecycleProvider.provide(package, staging_message) } + let(:runtime_stack) { 'docker://registry.example.com/team/cflinuxfs4:runtime' } + let(:build_credentials) { { 'registry.example.com' => { 'username' => 'build-user', 'password' => 'build-pass' } } } + let(:request_lifecycle) { { type: lifecycle_type, data: lifecycle_data } } + let(:lifecycle_data) do + { + stack: 'docker://registry.example.com/team/cflinuxfs4:staging', + buildpacks: [buildpack_git_url], + credentials: build_credentials + } + end + + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: true) + app.lifecycle_data.update(stack: runtime_stack, credentials: nil) + end + + it 'uses the build credentials for staging without changing app credentials or stack' do + action.create_and_stage(package:, lifecycle:) + + expect(stager).to have_received(:stage) do |staging_details| + expect(staging_details.lifecycle.staging_stack).to eq(lifecycle_data[:stack]) + expect(staging_details.lifecycle.credentials_for_staging.deep_stringify_keys).to eq(build_credentials) + end + expect(app.reload.lifecycle_data).to have_attributes(stack: runtime_stack, credentials: nil) + end + end + end + describe 'creating a stage request' do it 'initiates a staging request' do build = action.create_and_stage(package:, lifecycle:) diff --git a/spec/unit/controllers/v3/apps_controller_spec.rb b/spec/unit/controllers/v3/apps_controller_spec.rb index e6e0cec2310..c9c4fe4de35 100644 --- a/spec/unit/controllers/v3/apps_controller_spec.rb +++ b/spec/unit/controllers/v3/apps_controller_spec.rb @@ -589,15 +589,14 @@ end before do - create(:feature_flag, name: 'diego_custom_stacks', enabled: false, error_message: nil) + create(:feature_flag, name: 'diego_custom_stacks', enabled: false) 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') + expect(response.body).to include('FeatureDisabled', 'diego_custom_stacks') end end end @@ -652,15 +651,14 @@ end before do - create(:feature_flag, name: 'diego_custom_stacks', enabled: false, error_message: nil) + create(:feature_flag, name: 'diego_custom_stacks', enabled: false) 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') + expect(response.body).to include('FeatureDisabled', 'diego_custom_stacks') end end @@ -1323,15 +1321,14 @@ 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) + create(:feature_flag, name: 'diego_custom_stacks', enabled: false) 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') + expect(response.body).to include('FeatureDisabled', 'diego_custom_stacks') end end end @@ -1443,15 +1440,14 @@ 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) + create(:feature_flag, name: 'diego_custom_stacks', enabled: false) 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') + expect(response.body).to include('FeatureDisabled', 'diego_custom_stacks') end end diff --git a/spec/unit/fetchers/buildpack_lifecycle_fetcher_spec.rb b/spec/unit/fetchers/buildpack_lifecycle_fetcher_spec.rb index 470af857d7c..b8177e6bc27 100644 --- a/spec/unit/fetchers/buildpack_lifecycle_fetcher_spec.rb +++ b/spec/unit/fetchers/buildpack_lifecycle_fetcher_spec.rb @@ -53,7 +53,7 @@ module VCAP::CloudController 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') } + 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) 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 deleted file mode 100644 index aeeb07e6f2c..00000000000 --- a/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_custom_stacks_spec.rb +++ /dev/null @@ -1,108 +0,0 @@ -require 'spec_helper' -require 'cloud_controller/diego/buildpack/desired_lrp_builder' - -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 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 - 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/desired_lrp_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_spec.rb index 54b1f60c342..ee4bdeb9dc6 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_spec.rb @@ -276,6 +276,45 @@ module Buildpack end end + context 'with a custom stack' do + let(:stack) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } + let(:lifecycle_bundles) { { "buildpack/#{Stack.default.name}": 'platform-lifecycle-bundle' } } + let(:droplet_destinations) { { Stack.default.name.to_sym => '/value/from/config/based/on/stack' } } + + before do + allow(LifecycleBundleUriGenerator).to receive(:uri).with('platform-lifecycle-bundle').and_return('foo://bar.baz') + end + + it 'uses the custom image as the rootfs' do + expect(builder.root_fs).to eq('docker://docker.io/cloudfoundry/cflinuxfs4#1.268.0') + end + + it 'uses the default stack lifecycle bundle' do + expect(builder.cached_dependencies.first).to have_attributes( + from: 'foo://bar.baz', + cache_key: "buildpack-#{Stack.default.name}-lifecycle" + ) + end + + context 'with declarative asset downloads enabled' do + let(:enable_declarative_asset_downloads) { true } + let(:opts) { super().merge(checksum_algorithm: 'sha256') } + + it 'uses the default stack for the lifecycle bundle and droplet destination' do + layers = builder.image_layers + expect(layers.first).to have_attributes( + name: "buildpack-#{Stack.default.name}-lifecycle", + url: 'foo://bar.baz', + destination_path: '/tmp/lifecycle' + ) + expect(layers.find { |layer| layer.name == 'droplet' }).to have_attributes( + url: 'http://droplet-uri.com:1234?token=&%40home---%3E', + destination_path: '/value/from/config/based/on/stack' + ) + end + end + end + describe '#global_environment_variables' do context 'when there are additional container-level env vars provided' do let(:additional_env_vars) do 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 c0d20ddc279..a30b06cdb0a 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,15 +152,6 @@ 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/cnb/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb index e039ab4454a..49f1e9267db 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,11 +29,8 @@ 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, + credentials: '{"registry":{"username":"password"}}') end end let(:buildpack_infos) { [BuildpackInfo.new('http://some-buildpack.url', nil)] } @@ -63,11 +60,8 @@ 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, + credentials: '{"registry":{"username":"password"}}') end end @@ -93,7 +87,6 @@ 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' } }) @@ -155,11 +148,8 @@ 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', + credentials: '{"registry":{"username":"password"}}') details.package = package details.staging_guid = droplet.guid end diff --git a/spec/unit/lib/cloud_controller/diego/custom_stack_credential_resolver_spec.rb b/spec/unit/lib/cloud_controller/diego/custom_stack_credential_resolver_spec.rb index f3ace5e5834..7c9b2853db9 100644 --- a/spec/unit/lib/cloud_controller/diego/custom_stack_credential_resolver_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/custom_stack_credential_resolver_spec.rb @@ -12,14 +12,14 @@ module Diego subject(:image_credentials) { described_class.resolve(stack_uri:, credentials_by_host:) } it 'selects credentials for the stack registry' do - expect(image_credentials).to eq(username: 'stack-user', password: 'stack-pass') + expect(image_credentials).to eq(image_username: 'stack-user', image_password: 'stack-pass') end context 'with symbol keys' do let(:credentials_by_host) { { 'registry.example.com': { username: 'stack-user', password: 'stack-pass' } } } it 'accepts the credentials' do - expect(image_credentials).to eq(username: 'stack-user', password: 'stack-pass') + expect(image_credentials).to eq(image_username: 'stack-user', image_password: 'stack-pass') end end @@ -27,7 +27,7 @@ module Diego let(:credentials_by_host) { { 'other.example.com' => { 'username' => 'other', 'password' => 'secret' } } } it 'does not forward them' do - expect(image_credentials).to eq(username: nil, password: nil) + expect(image_credentials).to eq(image_username: nil, image_password: nil) end end @@ -35,7 +35,7 @@ module Diego let(:credentials_by_host) { { 'registry.example.com' => { 'username' => 'stack-user' } } } it 'does not forward the partial pair' do - expect(image_credentials).to eq(username: nil, password: nil) + expect(image_credentials).to eq(image_username: nil, image_password: nil) end end @@ -43,7 +43,7 @@ module Diego let(:credentials_by_host) { nil } it 'uses no image credentials' do - expect(image_credentials).to eq(username: nil, password: nil) + expect(image_credentials).to eq(image_username: nil, image_password: nil) end end @@ -51,7 +51,7 @@ module Diego let(:stack_uri) { 'cflinuxfs4' } it 'does not use registry credentials' do - expect(image_credentials).to eq(username: nil, password: nil) + expect(image_credentials).to eq(image_username: nil, image_password: nil) 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 4e3d2e35c01..6a9513935c1 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 @@ -10,6 +10,7 @@ module VCAP::CloudController let(:lifecycle_request_data) { {} } it_behaves_like 'a app lifecycle' + it_behaves_like 'custom-stack lifecycle updates', 'buildpack', 'https://example.com/buildpack.zip' describe '#create_lifecycle_data_model' do let!(:app) { create(:app_model) } @@ -281,22 +282,6 @@ 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 6650bbc9a4b..19d8ddccce8 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 @@ -10,6 +10,7 @@ module VCAP::CloudController let(:lifecycle_request_data) { { buildpacks: ['http://acme.com'] } } it_behaves_like 'a app lifecycle' + it_behaves_like 'custom-stack lifecycle updates', 'cnb', 'docker://registry.example.com/team/buildpack:latest' describe '#create_lifecycle_data_model' do let!(:app) { create(:app_model) } @@ -124,22 +125,6 @@ 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/app_lifecycle_shared.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/app_lifecycle_shared.rb index 55afb340b54..a66cb14e09b 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/app_lifecycle_shared.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/app_lifecycle_shared.rb @@ -22,3 +22,56 @@ expect(subject.type).to be_a(String) end end + +RSpec.shared_examples_for 'custom-stack lifecycle updates' do |lifecycle_type, buildpack_url| + subject(:lifecycle) { described_class.new(message, app) } + + let(:app) { create(:app_model, lifecycle_type.to_sym) } + let(:stored_stack) { 'docker://registry.example.com/team/stack:v1' } + let(:requested_stack) { 'docker://registry.example.com/team/stack:v2' } + let(:stored_buildpacks) { [buildpack_url] } + let(:pinned_buildpack) { create(:buildpack, name: 'default-stack-buildpack', stack: VCAP::CloudController::Stack.default.name, lifecycle: lifecycle_type) } + let(:message) { VCAP::CloudController::AppUpdateMessage.new(lifecycle: { type: lifecycle_type, data: { stack: requested_stack } }) } + + before do + create(:feature_flag, name: 'diego_custom_stacks', enabled: true) + app.lifecycle_data.update(stack: stored_stack, buildpacks: stored_buildpacks) + end + + it 'validates and retains the configured custom buildpacks' do + expect(lifecycle).to be_valid + lifecycle.update_lifecycle_data_model(app) + + expect(app.lifecycle_data.reload).to have_attributes(stack: requested_stack, buildpacks: stored_buildpacks) + end + + context 'with a retained stack-pinned admin buildpack' do + let(:stored_stack) { VCAP::CloudController::Stack.default.name } + let(:stored_buildpacks) { [pinned_buildpack.name] } + + it 'rejects the incompatible buildpack instead of treating the request as auto-detection' do + 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 + + context 'when only buildpacks are updated' do + let(:message) { VCAP::CloudController::AppUpdateMessage.new(lifecycle: { type: lifecycle_type, data: { buildpacks: [pinned_buildpack.name] } }) } + + it 'validates against the retained custom stack, not the default stack' do + 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 + + context 'when resetting the stack to null' do + let(:requested_stack) { nil } + + it 'keeps the existing stack-reset behavior' do + expect(lifecycle).to be_valid + lifecycle.update_lifecycle_data_model(app) + + expect(app.lifecycle_data.reload).to have_attributes(stack: nil, buildpacks: stored_buildpacks) + 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 index b87980bad80..736a30ee5bd 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 @@ -3,23 +3,13 @@ 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] } let(:lifecycle_type) { Lifecycles::BUILDPACK } - - subject(:validator) do - BuildpackLifecycleDataValidator.new( - buildpack_infos: buildpack_infos, - stack: stack, - stack_name: stack&.name, - lifecycle_type: lifecycle_type - ) - end + let(:credentials) { nil } describe 'custom stacks' do - let(:stack) { nil } let(:custom_stack_uri) { 'docker://docker.io/cloudfoundry/cflinuxfs4:1.268.0' } subject(:validator) do @@ -27,7 +17,8 @@ module VCAP::CloudController buildpack_infos: buildpack_infos, stack: nil, stack_name: custom_stack_uri, - lifecycle_type: lifecycle_type + lifecycle_type: lifecycle_type, + credentials: credentials ) end @@ -36,12 +27,8 @@ module VCAP::CloudController 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 + it 'accepts a custom buildpack URL' do + expect(validator).to be_valid end context 'with an unpinned admin buildpack' do @@ -100,8 +87,46 @@ module VCAP::CloudController end end + context 'with registry credentials' do + let(:custom_stack_uri) { 'docker://registry.example.com/team/stack:latest' } + let(:registry_credentials) { { 'username' => 'registry-user', 'password' => 'registry-pass' } } + let(:credentials) { { 'registry.example.com' => registry_credentials } } + + it 'accepts string credentials' do + expect(validator).to be_valid + end + + %w[username password].each do |field| + context "with a non-string #{field}" do + let(:registry_credentials) { super().merge(field => 123) } + + it 'rejects the value without exposing it in the error' do + expect(validator).not_to be_valid + expect(validator.errors[:credentials]).to contain_exactly("#{field} for custom stack registry \"registry.example.com\" must be a string") + end + end + end + + context 'with null credential fields' do + let(:registry_credentials) { { 'username' => nil, 'password' => nil } } + + it 'preserves support for absent credentials' do + expect(validator).to be_valid + end + end + + context 'with CNB token credentials for a buildpack registry' do + let(:lifecycle_type) { Lifecycles::CNB } + let(:credentials) { super().merge('buildpacks.example.com' => { 'token' => 'buildpack-token' }) } + + it 'accepts tokens separately from the stack credentials' do + expect(validator).to be_valid + end + end + end + context 'with an invalid custom stack URI' do - let(:custom_stack_uri) { 'docker://registry.example.com/' } + let(:custom_stack_uri) { 'docker://registry.example.com/team/' } it 'reports an invalid container image URI' do expect(validator).not_to be_valid @@ -132,30 +157,5 @@ module VCAP::CloudController 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/staging_action_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/staging_action_builder_spec.rb index 737105ab317..cc7e86439a3 100644 --- a/spec/unit/lib/cloud_controller/diego/staging_action_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/staging_action_builder_spec.rb @@ -8,6 +8,53 @@ module Diego let(:config) { Config.new({ staging: { legacy_md5_buildpack_paths_enabled: } }) } + %w[buildpack cnb].each do |prefix| + context "with a custom stack and #{prefix} lifecycle assets" do + subject(:builder) { described_class.new(config, nil, lifecycle_data, prefix, nil, nil, nil) } + + let(:enable_declarative_asset_downloads) { false } + let(:config) do + Config.new({ + diego: { + lifecycle_bundles: { "#{prefix}/#{Stack.default.name}": 'platform-bundle' }, + enable_declarative_asset_downloads: enable_declarative_asset_downloads + } + }) + end + let(:lifecycle_data) do + { + stack: 'docker://registry.example.com/team/stack:v1', + buildpacks: [], + app_bits_checksum: { type: 'sha1' } + } + end + + before do + allow(LifecycleBundleUriGenerator).to receive(:uri).with('platform-bundle').and_return('http://lifecycle.example.com/bundle.tgz') + end + + it 'shares the platform lifecycle cache entry across custom image tags' do + dependency = builder.cached_dependencies.first + expect(dependency).to have_attributes(from: 'http://lifecycle.example.com/bundle.tgz', cache_key: "#{prefix}-#{Stack.default.name}-lifecycle") + + lifecycle_data[:stack] = 'docker://registry.example.com/team/stack:v2' + expect(builder.cached_dependencies.first).to eq(dependency) + end + + context 'with declarative asset downloads enabled' do + let(:enable_declarative_asset_downloads) { true } + + it 'names the lifecycle layer after the platform stack' do + expect(builder.image_layers.first).to have_attributes( + name: "#{prefix}-#{Stack.default.name}-lifecycle", + url: 'http://lifecycle.example.com/bundle.tgz', + destination_path: '/tmp/lifecycle' + ) + end + end + end + end + describe '#buildpack_path' do context 'when legacy_md5_buildpack_paths_enabled is false' do let(:legacy_md5_buildpack_paths_enabled) { false } diff --git a/spec/unit/lib/utils/uri_utils_spec.rb b/spec/unit/lib/utils/uri_utils_spec.rb index 23d94009ef0..145c4a63579 100644 --- a/spec/unit/lib/utils/uri_utils_spec.rb +++ b/spec/unit/lib/utils/uri_utils_spec.rb @@ -115,6 +115,7 @@ 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) + expect(UriUtils.is_custom_stack_uri?('docker://registry.example.com/team/')).to be(false) end end @@ -208,6 +209,12 @@ expect { UriUtils.parse_docker_uri('publish/buildpack:?@sha256:e118d023acaee5cf13471ead39f68416ad6172ff0899f3257ce1481cd2b28a6a') }.to raise_error(UriUtils::InvalidDockerURI) expect { UriUtils.parse_docker_uri('publish/buildpack@sha256:e118d0') }.to raise_error(UriUtils::InvalidDockerURI) end + + it 'rejects empty repository path components' do + %w[registry.example.com/team/ registry.example.com//stack registry.example.com/team//stack].each do |image| + expect { UriUtils.parse_docker_uri(image) }.to raise_error(UriUtils::InvalidDockerURI, /Invalid image name/) + end + end end describe '.url_has_userinfo?' do diff --git a/spec/unit/repositories/app_event_repository_spec.rb b/spec/unit/repositories/app_event_repository_spec.rb index e167381cca7..e73ac02b304 100644 --- a/spec/unit/repositories/app_event_repository_spec.rb +++ b/spec/unit/repositories/app_event_repository_spec.rb @@ -662,27 +662,6 @@ 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