[RFC0030 - 3] Rename 'file-based-service-bindings' to 'service-binding-k8s' and introduce 'file-based-vcap-services' (#4235)

* Add column 'service_binding_k8s_enabled' to apps table

* Rename 'file-based-service-bindings' feature to 'service-binding-k8s'

* Introduce 'file-based-vcap-services' app feature

* Improve migration and add migration tests

* Fix migration tests for mysql

* Do not try to create/drop check constraint on mysql < 8

* Use constants for app feature names

Author: svkrieger

app/actions/app_feature_update.rb

@@ -2,12 +2,14 @@ module VCAP::CloudController
   class AppFeatureUpdate
     def self.update(feature_name, app, message)
       case feature_name
-      when 'ssh'
+      when AppFeaturesController::SSH_FEATURE
         app.update(enable_ssh: message.enabled)
-      when 'revisions'
+      when AppFeaturesController::REVISIONS_FEATURE
         app.update(revisions_enabled: message.enabled)
-      when 'file-based-service-bindings'
-        app.update(file_based_service_bindings_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)
       end
     end
   end

app/controllers/v3/app_features_controller.rb

@@ -2,7 +2,8 @@
 require 'controllers/v3/mixins/app_sub_resource'
 require 'presenters/v3/app_ssh_feature_presenter'
 require 'presenters/v3/app_revisions_feature_presenter'
-require 'presenters/v3/app_file_based_service_bindings_feature_presenter'
+require 'presenters/v3/app_service_binding_k8s_feature_presenter'
+require 'presenters/v3/app_file_based_vcap_services_feature_presenter'
 require 'presenters/v3/app_ssh_status_presenter'
 require 'actions/app_feature_update'
 
@@ -11,9 +12,10 @@ class AppFeaturesController < ApplicationController
 
   SSH_FEATURE = 'ssh'.freeze
   REVISIONS_FEATURE = 'revisions'.freeze
-  FILE_BASED_SERVICE_BINDINGS_FEATURE = 'file-based-service-bindings'.freeze
+  SERVICE_BINDING_K8S_FEATURE = 'service-binding-k8s'.freeze
+  FILE_BASED_VCAP_SERVICES_FEATURE = 'file-based-vcap-services'.freeze
 
-  TRUSTED_APP_FEATURES = [SSH_FEATURE, FILE_BASED_SERVICE_BINDINGS_FEATURE].freeze
+  TRUSTED_APP_FEATURES = [SSH_FEATURE, SERVICE_BINDING_K8S_FEATURE, FILE_BASED_VCAP_SERVICES_FEATURE].freeze
   UNTRUSTED_APP_FEATURES = [REVISIONS_FEATURE].freeze
   APP_FEATURES = (TRUSTED_APP_FEATURES + UNTRUSTED_APP_FEATURES).freeze
 
@@ -53,6 +55,10 @@ def update
     message = VCAP::CloudController::AppFeatureUpdateMessage.new(hashed_params['body'])
     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.")
+    end
+
     AppFeatureUpdate.update(hashed_params[:name], app, message)
     render status: :ok, json: feature_presenter_for(hashed_params[:name], app)
   end
@@ -83,7 +89,8 @@ def feature_presenter_for(feature_name, app)
     presenters = {
       SSH_FEATURE => Presenters::V3::AppSshFeaturePresenter,
       REVISIONS_FEATURE => Presenters::V3::AppRevisionsFeaturePresenter,
-      FILE_BASED_SERVICE_BINDINGS_FEATURE => Presenters::V3::AppFileBasedServiceBindingsFeaturePresenter
+      SERVICE_BINDING_K8S_FEATURE => Presenters::V3::AppServiceBindingK8sFeaturePresenter,
+      FILE_BASED_VCAP_SERVICES_FEATURE => Presenters::V3::AppFileBasedVcapServicesFeaturePresenter
     }
     presenters[feature_name].new(app)
   end
@@ -92,7 +99,16 @@ def presented_app_features(app)
     [
       Presenters::V3::AppSshFeaturePresenter.new(app),
       Presenters::V3::AppRevisionsFeaturePresenter.new(app),
-      Presenters::V3::AppFileBasedServiceBindingsFeaturePresenter.new(app)
+      Presenters::V3::AppServiceBindingK8sFeaturePresenter.new(app),
+      Presenters::V3::AppFileBasedVcapServicesFeaturePresenter.new(app)
     ]
   end
+
+  def both_service_binding_features_enabled?(app, feature_name)
+    if feature_name == FILE_BASED_VCAP_SERVICES_FEATURE
+      app.service_binding_k8s_enabled
+    elsif feature_name == SERVICE_BINDING_K8S_FEATURE
+      app.file_based_vcap_services_enabled
+    end
+  end
 end

app/presenters/v3/app_file_based_service_bindings_feature_presenter.rb

@@ -0,0 +1,19 @@
+require 'presenters/v3/base_presenter'
+
+module VCAP::CloudController::Presenters::V3
+  class AppFileBasedVcapServicesFeaturePresenter < BasePresenter
+    def to_hash
+      {
+        name: AppFeaturesController::FILE_BASED_VCAP_SERVICES_FEATURE,
+        description: 'Enable file-based VCAP service bindings for the app',
+        enabled: app.file_based_vcap_services_enabled
+      }
+    end
+
+    private
+
+    def app
+      @resource
+    end
+  end
+end

app/presenters/v3/app_file_based_vcap_services_feature_presenter.rb

@@ -0,0 +1,19 @@
+require 'presenters/v3/base_presenter'
+
+module VCAP::CloudController::Presenters::V3
+  class AppServiceBindingK8sFeaturePresenter < BasePresenter
+    def to_hash
+      {
+        name: AppFeaturesController::SERVICE_BINDING_K8S_FEATURE,
+        description: 'Enable k8s service bindings for the app',
+        enabled: app.service_binding_k8s_enabled
+      }
+    end
+
+    private
+
+    def app
+      @resource
+    end
+  end
+end

app/presenters/v3/app_service_binding_k8s_feature_presenter.rb

@@ -0,0 +1,47 @@
+Sequel.migration do
+  up do
+    if database_type == :postgres
+      alter_table :apps do
+        add_column :service_binding_k8s_enabled, :boolean, default: false, null: false, if_not_exists: true
+        add_column :file_based_vcap_services_enabled, :boolean, default: false, null: false, if_not_exists: true
+
+        unless check_constraint_exists?(@db)
+          add_constraint(name: :only_one_sb_feature_enabled, if_not_exists: true) do
+            Sequel.lit('NOT (service_binding_k8s_enabled AND file_based_vcap_services_enabled)')
+          end
+        end
+      end
+
+    elsif database_type == :mysql
+      add_column :apps, :service_binding_k8s_enabled, :boolean, default: false, null: false unless schema(:apps).map(&:first).include?(:service_binding_k8s_enabled)
+      add_column :apps, :file_based_vcap_services_enabled, :boolean, default: false, null: false unless schema(:apps).map(&:first).include?(:file_based_vcap_services_enabled)
+
+      if check_constraint_supported?(self) && !check_constraint_exists?(self)
+        run('ALTER TABLE apps ADD CONSTRAINT only_one_sb_feature_enabled CHECK (NOT (service_binding_k8s_enabled AND file_based_vcap_services_enabled))')
+      end
+    end
+  end
+
+  down do
+    alter_table :apps do
+      drop_constraint :only_one_sb_feature_enabled if check_constraint_supported?(@db) && check_constraint_exists?(@db)
+      drop_column :service_binding_k8s_enabled if @db.schema(:apps).map(&:first).include?(:service_binding_k8s_enabled)
+      drop_column :file_based_vcap_services_enabled if @db.schema(:apps).map(&:first).include?(:file_based_vcap_services_enabled)
+    end
+  end
+end
+
+def check_constraint_exists?(database)
+  if database.database_type == :postgres
+    database.check_constraints(:apps).include?(:only_one_sb_feature_enabled)
+  elsif database.database_type == :mysql
+    database[:information_schema__table_constraints].where(TABLE_SCHEMA: database.opts[:database], TABLE_NAME: 'apps', CONSTRAINT_TYPE: 'CHECK',
+                                                           CONSTRAINT_NAME: 'only_one_sb_feature_enabled').any?
+  end
+end
+
+# check constraints are not available in Mysql versions < 8
+# this is also enforced on application level, so it should be fine not to create it on that version
+def check_constraint_supported?(database)
+  database.database_type == :postgres || (database.database_type == :mysql && database.server_version >= 80_000)
+end

db/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns.rb

@@ -20,13 +20,18 @@
       "enabled": false
     },
     {
-      "name": "file-based-service-bindings",
-      "description": "Enable file-based service bindings for the app",
+      "name": "service-binding-k8s",
+      "description": "Enable k8s service bindings for the app",
+      "enabled": false
+    },
+    {
+      "name": "file-based-vcap-services",
+      "description": "Enable file-based VCAP service bindings for the app",
       "enabled": false
     }
   ],
   "pagination": {
-    "total_results": 3,
+    "total_results": 4,
     "total_pages": 1,
     "first": { "href": "/v3/apps/05d39de4-2c9e-4c76-8fd6-10417da07e42/features" },
     "last": { "href": "/v3/apps/05d39de4-2c9e-4c76-8fd6-10417da07e42/features" },

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

@@ -6,4 +6,5 @@ Name | Description
 ---- | -----------
 **ssh** | Enable SSHing into the app
 **revisions** | Enable [versioning](#revisions) of an application
-**file-based-service-bindings** | Enable file-based service bindings for the app (experimental)
+**service-binding-k8s** | Enable k8s service bindings for the app (experimental)
+**file-based-vcap-services** | Enable file-based VCAP service bindings for the app (experimental)