Add feature flags to v1 app manifests

app/actions/app_apply_manifest.rb

@@ -28,6 +28,8 @@ def apply(app_guid, message)
       app = AppModel.first(guid: app_guid)
       app_instance_not_found!(app_guid) unless app
 
+      AppFeatureUpdate.bulk_update(app, message.manifest_features_update_message)
+
       message.manifest_process_update_messages.each do |manifest_process_update_msg|
         if manifest_process_update_msg.requested_keys == [:type] && manifest_process_update_msg.type == 'web'
           # ignore trivial messages, most likely from manifest

app/actions/app_feature_update.rb

@@ -1,15 +1,41 @@
 module VCAP::CloudController
   class AppFeatureUpdate
-    def self.update(feature_name, app, message)
-      case feature_name
-      when AppFeaturesController::SSH_FEATURE
-        app.update(enable_ssh: message.enabled)
-      when AppFeaturesController::REVISIONS_FEATURE
-        app.update(revisions_enabled: message.enabled)
-      when AppFeaturesController::SERVICE_BINDING_K8S_FEATURE
-        app.update(service_binding_k8s_enabled: message.enabled)
-      when AppFeaturesController::FILE_BASED_VCAP_SERVICES_FEATURE
-        app.update(file_based_vcap_services_enabled: message.enabled)
+    class InvalidCombination < StandardError; end
+
+    class << self
+      def update(feature_name, app, app_feature_update_message)
+        app.update({ feature_column_name(feature_name) => app_feature_update_message.enabled })
+      end
+
+      def bulk_update(app, manifest_features_update_message)
+        flags = {}
+
+        manifest_features_update_message.features&.each do |feature_name, enabled|
+          flags[feature_column_name(feature_name)] = enabled
+        end
+
+        return if flags.empty?
+
+        check_invalid_combination!(app, flags)
+        app.update(flags)
+      end
+
+      private
+
+      def feature_column_name(feature_name)
+        column = AppFeatures::DATABASE_COLUMNS_MAPPING[feature_name.to_s]
+        raise "Unknown feature name: #{feature_name}" if column.nil?
+
+        column
+      end
+
+      def check_invalid_combination!(app, flags)
+        file_based_vcap_services_enabled = flags[feature_column_name(AppFeatures::FILE_BASED_VCAP_SERVICES_FEATURE)].present? || app.file_based_vcap_services_enabled
+        service_binding_k8s_enabled = flags[feature_column_name(AppFeatures::SERVICE_BINDING_K8S_FEATURE)].present? || app.service_binding_k8s_enabled
+        return unless file_based_vcap_services_enabled && service_binding_k8s_enabled
+
+        msg = "'#{AppFeatures::FILE_BASED_VCAP_SERVICES_FEATURE}' and '#{AppFeatures::SERVICE_BINDING_K8S_FEATURE}' features cannot be enabled at the same time."
+        raise InvalidCombination.new(msg)
       end
     end
   end

app/actions/space_diff_manifest.rb

@@ -51,6 +51,8 @@ def generate_diff(app_manifests, space)
 
             remove_default_missing_fields(existing_value, key, value) if nested_attribute_exists
 
+            remove_unspecified_app_features(existing_value, key, value)
+
             # To preserve backwards compability, we've decided to skip diffs that satisfy this conditon
             next if !nested_attribute_exists && %w[disk_quota disk-quota memory].include?(key)
 
@@ -211,6 +213,11 @@ def remove_default_missing_fields(existing_value, current_key, value)
           end
         end
       end
+
+      def remove_unspecified_app_features(existing_value, key, value)
+        # Only show the diff for features that are specified in the manifest
+        existing_value.slice!(*value.keys) if key == 'features'
+      end
     end
   end
 end

app/controllers/v3/app_features_controller.rb

@@ -6,17 +6,14 @@
 require 'presenters/v3/app_file_based_vcap_services_feature_presenter'
 require 'presenters/v3/app_ssh_status_presenter'
 require 'actions/app_feature_update'
+require 'models/helpers/app_features'
 
 class AppFeaturesController < ApplicationController
   include AppSubResource
 
-  SSH_FEATURE = 'ssh'.freeze
-  REVISIONS_FEATURE = 'revisions'.freeze
-  SERVICE_BINDING_K8S_FEATURE = 'service-binding-k8s'.freeze
-  FILE_BASED_VCAP_SERVICES_FEATURE = 'file-based-vcap-services'.freeze
-
-  TRUSTED_APP_FEATURES = [SSH_FEATURE, SERVICE_BINDING_K8S_FEATURE, FILE_BASED_VCAP_SERVICES_FEATURE].freeze
-  UNTRUSTED_APP_FEATURES = [REVISIONS_FEATURE].freeze
+  TRUSTED_APP_FEATURES = [VCAP::CloudController::AppFeatures::SSH_FEATURE, VCAP::CloudController::AppFeatures::SERVICE_BINDING_K8S_FEATURE,
+                          VCAP::CloudController::AppFeatures::FILE_BASED_VCAP_SERVICES_FEATURE].freeze
+  UNTRUSTED_APP_FEATURES = [VCAP::CloudController::AppFeatures::REVISIONS_FEATURE].freeze
   APP_FEATURES = (TRUSTED_APP_FEATURES + UNTRUSTED_APP_FEATURES).freeze
 
   def index
@@ -56,11 +53,12 @@ def update
     unprocessable!(message.errors.full_messages) unless message.valid?
 
     if message.enabled && both_service_binding_features_enabled?(app, name)
-      unprocessable!("'#{FILE_BASED_VCAP_SERVICES_FEATURE}' and '#{SERVICE_BINDING_K8S_FEATURE}' features cannot be enabled at the same time.")
+      unprocessable!("'#{VCAP::CloudController::AppFeatures::FILE_BASED_VCAP_SERVICES_FEATURE}' and '#{VCAP::CloudController::AppFeatures::SERVICE_BINDING_K8S_FEATURE}' " \
+                     'features cannot be enabled at the same time.')
     end
 
-    AppFeatureUpdate.update(hashed_params[:name], app, message)
-    render status: :ok, json: feature_presenter_for(hashed_params[:name], app)
+    AppFeatureUpdate.update(name, app, message)
+    render status: :ok, json: feature_presenter_for(name, app)
   end
 
   def ssh_enabled
@@ -87,10 +85,10 @@ def present_unpagination_hash(result, path)
 
   def feature_presenter_for(feature_name, app)
     presenters = {
-      SSH_FEATURE => Presenters::V3::AppSshFeaturePresenter,
-      REVISIONS_FEATURE => Presenters::V3::AppRevisionsFeaturePresenter,
-      SERVICE_BINDING_K8S_FEATURE => Presenters::V3::AppServiceBindingK8sFeaturePresenter,
-      FILE_BASED_VCAP_SERVICES_FEATURE => Presenters::V3::AppFileBasedVcapServicesFeaturePresenter
+      VCAP::CloudController::AppFeatures::SSH_FEATURE => Presenters::V3::AppSshFeaturePresenter,
+      VCAP::CloudController::AppFeatures::REVISIONS_FEATURE => Presenters::V3::AppRevisionsFeaturePresenter,
+      VCAP::CloudController::AppFeatures::SERVICE_BINDING_K8S_FEATURE => Presenters::V3::AppServiceBindingK8sFeaturePresenter,
+      VCAP::CloudController::AppFeatures::FILE_BASED_VCAP_SERVICES_FEATURE => Presenters::V3::AppFileBasedVcapServicesFeaturePresenter
     }
     presenters[feature_name].new(app)
   end
@@ -105,9 +103,9 @@ def presented_app_features(app)
   end
 
   def both_service_binding_features_enabled?(app, feature_name)
-    if feature_name == FILE_BASED_VCAP_SERVICES_FEATURE
+    if feature_name == VCAP::CloudController::AppFeatures::FILE_BASED_VCAP_SERVICES_FEATURE
       app.service_binding_k8s_enabled
-    elsif feature_name == SERVICE_BINDING_K8S_FEATURE
+    elsif feature_name == VCAP::CloudController::AppFeatures::SERVICE_BINDING_K8S_FEATURE
       app.file_based_vcap_services_enabled
     end
   end

app/jobs/space_apply_manifest_action_job.rb

@@ -26,7 +26,8 @@ def perform
                AppApplyManifest::ServiceBindingError,
                SidecarCreate::InvalidSidecar,
                SidecarUpdate::InvalidSidecar,
-               ProcessScale::SidecarMemoryLessThanProcessMemory => e
+               ProcessScale::SidecarMemoryLessThanProcessMemory,
+               AppFeatureUpdate::InvalidCombination => e
 
           app_name = AppModel.find(guid: app_guid)&.name
           error_message = app_name ? "For application '#{app_name}': #{e.message}" : e.message

app/messages/app_manifest_message.rb

@@ -4,6 +4,7 @@
 require 'messages/manifest_buildpack_message'
 require 'messages/manifest_service_binding_create_message'
 require 'messages/manifest_routes_update_message'
+require 'messages/manifest_features_update_message'
 require 'messages/validators/metadata_validator'
 require 'cloud_controller/app_manifest/byte_converter'
 require 'models/helpers/health_check_types'
@@ -43,6 +44,7 @@ class AppManifestMessage < BaseMessage
       stack
       timeout
       cnb_credentials
+      features
     ]
 
     HEALTH_CHECK_TYPE_MAPPING = { HealthCheckTypes::NONE => HealthCheckTypes::PROCESS }.freeze
@@ -82,6 +84,7 @@ def self.underscore_keys(hash)
         record.requested?(:random_route) ||
         record.requested?(:default_route)
     }
+    validate :validate_features!, if: ->(record) { record.requested?(:features) }
 
     def initialize(original_yaml, attrs={})
       super(attrs)
@@ -116,6 +119,10 @@ def manifest_routes_update_message
       @manifest_routes_update_message ||= ManifestRoutesUpdateMessage.new(routes_attribute_mapping)
     end
 
+    def manifest_features_update_message
+      @manifest_features_update_message ||= ManifestFeaturesUpdateMessage.new(features_attribute_mapping)
+    end
+
     def audit_hash
       override_env = original_yaml['env'] ? { 'env' => Presenters::Censorship::PRIVATE_DATA_HIDDEN } : {}
       override_cnb = original_yaml['cnb-credentials'] ? { 'cnb-credentials' => Presenters::Censorship::PRIVATE_DATA_HIDDEN } : {}
@@ -295,6 +302,12 @@ def service_bindings_attribute_mapping
       mapping
     end
 
+    def features_attribute_mapping
+      mapping = {}
+      mapping[:features] = features if requested?(:features)
+      mapping
+    end
+
     def should_autodetect?(buildpack)
       buildpack == 'default' || buildpack == 'null' || buildpack.nil?
     end
@@ -464,6 +477,13 @@ def validate_docker_buildpacks_combination!
       errors.add(:base, 'Cannot specify both buildpack(s) and docker keys')
     end
 
+    def validate_features!
+      manifest_features_update_message.valid?
+      manifest_features_update_message.errors.full_messages.each do |error_message|
+        errors.add(:base, error_message)
+      end
+    end
+
     def add_process_error!(error_message, type)
       errors.add(:base, %(Process "#{type}": #{error_message}))
     end

app/messages/manifest_features_update_message.rb

@@ -0,0 +1,25 @@
+require 'messages/base_message'
+
+module VCAP::CloudController
+  class ManifestFeaturesUpdateMessage < BaseMessage
+    register_allowed_keys [:features]
+
+    validates_with NoAdditionalKeysValidator
+
+    validate :features do
+      errors.add(:features, 'must be a map of valid feature names to booleans (true = enabled, false = disabled)') unless is_valid(features)
+    end
+
+    private
+
+    def is_valid(features)
+      return false unless features.is_a?(Hash) && features.any?
+
+      features.all? { |feature, enabled| valid_feature?(feature) && [true, false].include?(enabled) }
+    end
+
+    def valid_feature?(feature)
+      AppFeatures.all_features.include?(feature.to_s)
+    end
+  end
+end

app/models/helpers/app_features.rb

@@ -0,0 +1,19 @@
+module VCAP::CloudController
+  class AppFeatures
+    SSH_FEATURE = 'ssh'.freeze
+    REVISIONS_FEATURE = 'revisions'.freeze
+    SERVICE_BINDING_K8S_FEATURE = 'service-binding-k8s'.freeze
+    FILE_BASED_VCAP_SERVICES_FEATURE = 'file-based-vcap-services'.freeze
+
+    DATABASE_COLUMNS_MAPPING = {
+      SSH_FEATURE => :enable_ssh,
+      REVISIONS_FEATURE => :revisions_enabled,
+      SERVICE_BINDING_K8S_FEATURE => :service_binding_k8s_enabled,
+      FILE_BASED_VCAP_SERVICES_FEATURE => :file_based_vcap_services_enabled
+    }.freeze
+
+    def self.all_features
+      [SSH_FEATURE, REVISIONS_FEATURE, SERVICE_BINDING_K8S_FEATURE, FILE_BASED_VCAP_SERVICES_FEATURE]
+    end
+  end
+end

app/presenters/v3/app_file_based_vcap_services_feature_presenter.rb

@@ -4,7 +4,7 @@ module VCAP::CloudController::Presenters::V3
   class AppFileBasedVcapServicesFeaturePresenter < BasePresenter
     def to_hash
       {
-        name: AppFeaturesController::FILE_BASED_VCAP_SERVICES_FEATURE,
+        name: VCAP::CloudController::AppFeatures::FILE_BASED_VCAP_SERVICES_FEATURE,
         description: 'Enable file-based VCAP service bindings for the app',
         enabled: app.file_based_vcap_services_enabled
       }

app/presenters/v3/app_manifest_presenter.rb

@@ -7,6 +7,7 @@
 require 'presenters/v3/app_manifest_presenters/process_properties_presenter'
 require 'presenters/v3/app_manifest_presenters/sidecar_properties_presenter'
 require 'presenters/v3/app_manifest_presenters/lifecycle_presenter'
+require 'presenters/v3/app_manifest_presenters/features_presenter'
 
 module VCAP::CloudController
   module Presenters
@@ -17,6 +18,7 @@ class AppManifestPresenter
           AppManifestPresenters::LifecyclePresenter.new,
           AppManifestPresenters::DockerPresenter.new,
           AppManifestPresenters::BuildpackPresenter.new,
+          AppManifestPresenters::FeaturesPresenter.new,
           AppManifestPresenters::ServicesPropertiesPresenter.new,
           AppManifestPresenters::RoutePropertiesPresenter.new,
           AppManifestPresenters::MetadataPresenter.new,

app/presenters/v3/app_manifest_presenters/features_presenter.rb

@@ -0,0 +1,20 @@
+module VCAP::CloudController
+  module Presenters
+    module V3
+      module AppManifestPresenters
+        class FeaturesPresenter
+          def to_hash(app:, **_)
+            features = {}
+            AppFeatures.all_features.each do |feature|
+              features[feature.to_sym] = app.send(AppFeatures::DATABASE_COLUMNS_MAPPING[feature])
+            end
+
+            {
+              features:
+            }
+          end
+        end
+      end
+    end
+  end
+end

app/presenters/v3/app_revisions_feature_presenter.rb

@@ -4,7 +4,7 @@ module VCAP::CloudController::Presenters::V3
   class AppRevisionsFeaturePresenter < BasePresenter
     def to_hash
       {
-        name: 'revisions',
+        name: VCAP::CloudController::AppFeatures::REVISIONS_FEATURE,
         description: 'Enable versioning of an application',
         enabled: app.revisions_enabled
       }

app/presenters/v3/app_service_binding_k8s_feature_presenter.rb

@@ -4,7 +4,7 @@ module VCAP::CloudController::Presenters::V3
   class AppServiceBindingK8sFeaturePresenter < BasePresenter
     def to_hash
       {
-        name: AppFeaturesController::SERVICE_BINDING_K8S_FEATURE,
+        name: VCAP::CloudController::AppFeatures::SERVICE_BINDING_K8S_FEATURE,
         description: 'Enable k8s service bindings for the app',
         enabled: app.service_binding_k8s_enabled
       }

app/presenters/v3/app_ssh_feature_presenter.rb

@@ -4,7 +4,7 @@ module VCAP::CloudController::Presenters::V3
   class AppSshFeaturePresenter < BasePresenter
     def to_hash
       {
-        name: 'ssh',
+        name: VCAP::CloudController::AppFeatures::SSH_FEATURE,
         description: 'Enable SSHing into the app.',
         enabled: app.enable_ssh
       }

docs/v3/source/includes/resources/manifests/_apply.md

@@ -24,7 +24,7 @@ Location: https://api.example.org/v3/jobs/[guid]
 Apply changes specified in a manifest to the named apps and their underlying
 processes. The apps must reside in the space. These changes are additive
 and will not modify any unspecified properties or remove any existing
-environment variables, routes, or services.
+environment variables, app features, routes, or services.
 
 <aside class="notice">
 Apply manifest will only trigger an immediate update for the "instances" property or routing changes. All other properties require an app restart to take effect.

docs/v3/source/includes/resources/manifests/_get.md

@@ -22,6 +22,11 @@ Content-Type: application/x-yaml
 applications:
 - name: my-app
   stack: cflinuxfs4
+  features:
+    ssh: true
+    revisions: true
+    service-binding-k8s: false
+    file-based-vcap-services: false
   services:
   - my-service
   routes:

docs/v3/source/includes/resources/manifests/_object.md.erb

@@ -19,6 +19,11 @@ applications:
   env:
     VAR1: value1
     VAR2: value2
+  features:
+    ssh: true
+    revisions: true
+    service-binding-k8s: false
+    file-based-vcap-services: false
   routes:
   - route: route.example.com
   - route: another-route.example.com
@@ -94,6 +99,7 @@ Name | Type | Description
 **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**) <br>b) An array of one or more URLs pointing to buildpacks <br>c) An array of one or more installed buildpack names <br>Replaces the legacy `buildpack` field
 **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
+**features** | _object_ | A key-value mapping of feature names to booleans (true = enabled, false = disabled)
 **no-route** | _boolean_ | When set to `true`, any routes specified with the `routes` attribute will be ignored and any existing routes will be removed
 **processes** | _array of [process configurations](#space-manifest-process-level-configuration)_ | List of configurations for individual process types
 **random-route** | _boolean_ | Creates a random route for the app if `true`; if `routes` is specified, if the app already has routes, or if `no-route` is specified, this field is ignored regardless of its value