diff --git a/app/actions/app_feature_update.rb b/app/actions/app_feature_update.rb index 1fc865f78b0..e2475df8dd1 100644 --- a/app/actions/app_feature_update.rb +++ b/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 diff --git a/app/controllers/v3/app_features_controller.rb b/app/controllers/v3/app_features_controller.rb index 7359e524f76..a83812ea914 100644 --- a/app/controllers/v3/app_features_controller.rb +++ b/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 diff --git a/app/presenters/v3/app_file_based_service_bindings_feature_presenter.rb b/app/presenters/v3/app_file_based_service_bindings_feature_presenter.rb deleted file mode 100644 index fbd5928c7a3..00000000000 --- a/app/presenters/v3/app_file_based_service_bindings_feature_presenter.rb +++ /dev/null @@ -1,19 +0,0 @@ -require 'presenters/v3/base_presenter' - -module VCAP::CloudController::Presenters::V3 - class AppFileBasedServiceBindingsFeaturePresenter < BasePresenter - def to_hash - { - name: 'file-based-service-bindings', - description: 'Enable file-based service bindings for the app', - enabled: app.file_based_service_bindings_enabled - } - end - - private - - def app - @resource - end - end -end diff --git a/app/presenters/v3/app_file_based_vcap_services_feature_presenter.rb b/app/presenters/v3/app_file_based_vcap_services_feature_presenter.rb new file mode 100644 index 00000000000..66f02be8937 --- /dev/null +++ b/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 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 diff --git a/app/presenters/v3/app_service_binding_k8s_feature_presenter.rb b/app/presenters/v3/app_service_binding_k8s_feature_presenter.rb new file mode 100644 index 00000000000..af80df22247 --- /dev/null +++ b/app/presenters/v3/app_service_binding_k8s_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 diff --git a/db/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns.rb b/db/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns.rb new file mode 100644 index 00000000000..9064fcc05eb --- /dev/null +++ b/db/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns.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 diff --git a/docs/v3/source/includes/api_resources/_app_features.erb b/docs/v3/source/includes/api_resources/_app_features.erb index f947f9c0a6c..19704a22ac1 100644 --- a/docs/v3/source/includes/api_resources/_app_features.erb +++ b/docs/v3/source/includes/api_resources/_app_features.erb @@ -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" }, diff --git a/docs/v3/source/includes/resources/app_features/_supported_features.md.erb b/docs/v3/source/includes/resources/app_features/_supported_features.md.erb index 7f8bfede853..32ca219495f 100644 --- a/docs/v3/source/includes/resources/app_features/_supported_features.md.erb +++ b/docs/v3/source/includes/resources/app_features/_supported_features.md.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) diff --git a/spec/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns_spec.rb b/spec/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns_spec.rb new file mode 100644 index 00000000000..7d3f5646362 --- /dev/null +++ b/spec/migrations/20250225132929_add_apps_file_based_service_binding_feature_columns_spec.rb @@ -0,0 +1,241 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add file-based service binding feature columns to apps table', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20250225132929_add_apps_file_based_service_binding_feature_columns.rb' } + end + + describe 'apps table' do + subject(:run_migration) { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) } + + describe 'up' do + describe 'column service_binding_k8s_enabled' do + it 'adds a column `service_binding_k8s_enabled`' do + expect(db[:apps].columns).not_to include(:service_binding_k8s_enabled) + run_migration + expect(db[:apps].columns).to include(:service_binding_k8s_enabled) + end + + it 'sets the default value of existing entries to false' do + db[:apps].insert(guid: 'existing_app_guid') + run_migration + expect(db[:apps].first(guid: 'existing_app_guid')[:service_binding_k8s_enabled]).to be(false) + end + + it 'sets the default value of new entries to false' do + run_migration + db[:apps].insert(guid: 'new_app_guid') + expect(db[:apps].first(guid: 'new_app_guid')[:service_binding_k8s_enabled]).to be(false) + end + + it 'forbids null values' do + run_migration + expect { db[:apps].insert(guid: 'app_guid__nil', service_binding_k8s_enabled: nil) }.to raise_error(Sequel::NotNullConstraintViolation) + end + + context 'when it already exists' do + before do + db.add_column :apps, :service_binding_k8s_enabled, :boolean, default: false, null: false, if_not_exists: true + end + + it 'does not fail' do + expect(db[:apps].columns).to include(:service_binding_k8s_enabled) + expect { run_migration }.not_to raise_error + expect(db[:apps].columns).to include(:service_binding_k8s_enabled) + expect(db[:apps].columns).to include(:file_based_vcap_services_enabled) + expect(check_constraint_exists?(db)).to be(true) if check_constraint_supported?(db) + end + end + end + + describe 'column file_based_vcap_services_enabled' do + it 'adds a column `file_based_vcap_services_enabled`' do + expect(db[:apps].columns).not_to include(:file_based_vcap_services_enabled) + run_migration + expect(db[:apps].columns).to include(:file_based_vcap_services_enabled) + end + + it 'sets the default value of existing entries to false' do + db[:apps].insert(guid: 'existing_app_guid') + run_migration + expect(db[:apps].first(guid: 'existing_app_guid')[:file_based_vcap_services_enabled]).to be(false) + end + + it 'sets the default value of new entries to false' do + run_migration + db[:apps].insert(guid: 'new_app_guid') + expect(db[:apps].first(guid: 'new_app_guid')[:file_based_vcap_services_enabled]).to be(false) + end + + it 'forbids null values' do + run_migration + expect { db[:apps].insert(guid: 'app_guid__nil', file_based_vcap_services_enabled: nil) }.to raise_error(Sequel::NotNullConstraintViolation) + end + + context 'when it already exists' do + before do + db.add_column :apps, :file_based_vcap_services_enabled, :boolean, default: false, null: false, if_not_exists: true + end + + it 'does not fail' do + expect(db[:apps].columns).to include(:file_based_vcap_services_enabled) + expect { run_migration }.not_to raise_error + expect(db[:apps].columns).to include(:service_binding_k8s_enabled) + expect(db[:apps].columns).to include(:file_based_vcap_services_enabled) + expect(check_constraint_exists?(db)).to be(true) if check_constraint_supported?(db) + end + end + end + + describe 'check constraint' do + context 'when supported' do + before do + skip 'check constraint not supported by db' unless check_constraint_supported?(db) + end + + it 'adds the check constraint' do + expect(check_constraint_exists?(db)).to be(false) + run_migration + expect(check_constraint_exists?(db)).to be(true) + end + + it 'forbids setting both features to true' do + run_migration + expect { db[:apps].insert(guid: 'some_app', file_based_vcap_services_enabled: true, service_binding_k8s_enabled: true) }.to(raise_error do |error| + expect(error.inspect).to include('only_one_sb_feature_enabled', 'violate') + end) + end + + context 'when it already exists' do + before do + db.add_column :apps, :service_binding_k8s_enabled, :boolean, default: false, null: false, if_not_exists: true + db.add_column :apps, :file_based_vcap_services_enabled, :boolean, default: false, null: false, if_not_exists: true + db.alter_table :apps do + add_constraint(name: :only_one_sb_feature_enabled) do + Sequel.lit('NOT (service_binding_k8s_enabled AND file_based_vcap_services_enabled)') + end + end + end + + it 'does not fail' do + expect { run_migration }.not_to raise_error + end + end + end + + context 'when not supported' do + before do + skip 'check constraint supported by db' if check_constraint_supported?(db) + end + + it 'does not fail' do + expect { run_migration }.not_to raise_error + expect(db[:apps].columns).to include(:service_binding_k8s_enabled) + expect(db[:apps].columns).to include(:file_based_vcap_services_enabled) + end + end + end + end + + describe 'down' do + subject(:run_rollback) { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) } + + before do + run_migration + end + + describe 'column service_binding_k8s_enabled' do + it 'removes column `service_binding_k8s_enabled`' do + expect(db[:apps].columns).to include(:service_binding_k8s_enabled) + run_rollback + expect(db[:apps].columns).not_to include(:service_binding_k8s_enabled) + end + + context 'when it does not exist' do + before do + db.alter_table :apps do + drop_constraint :only_one_sb_feature_enabled + drop_column :service_binding_k8s_enabled + end + end + + it 'does not fail' do + expect(db[:apps].columns).not_to include(:service_binding_k8s_enabled) + expect { run_rollback }.not_to raise_error + expect(db[:apps].columns).not_to include(:service_binding_k8s_enabled) + expect(db[:apps].columns).not_to include(:file_based_vcap_services_enabled) + expect(check_constraint_exists?(db)).to be(false) + end + end + end + + describe 'column file_based_vcap_services_enabled' do + it 'removes column `file_based_vcap_services_enabled`' do + expect(db[:apps].columns).to include(:file_based_vcap_services_enabled) + run_rollback + expect(db[:apps].columns).not_to include(:file_based_vcap_services_enabled) + end + + context 'when it does not exist' do + before do + db.alter_table :apps do + drop_constraint :only_one_sb_feature_enabled + drop_column :file_based_vcap_services_enabled + end + end + + it 'does not fail' do + expect(db[:apps].columns).not_to include(:file_based_vcap_services_enabled) + expect { run_rollback }.not_to raise_error + expect(db[:apps].columns).not_to include(:service_binding_k8s_enabled) + expect(db[:apps].columns).not_to include(:file_based_vcap_services_enabled) + expect(check_constraint_exists?(db)).to be(false) + end + end + end + + describe 'check constraint' do + context 'when supported' do + before do + skip 'check constraint not supported by db' unless check_constraint_supported?(db) + end + + it 'removes the check constraint' do + expect(check_constraint_exists?(db)).to be(true) + run_rollback + expect(check_constraint_exists?(db)).to be(false) + end + + context 'when it does not exist' do + before do + db.alter_table :apps do + drop_constraint :only_one_sb_feature_enabled + end + end + + it 'does not fail' do + expect(check_constraint_exists?(db)).to be(false) + expect { run_rollback }.not_to raise_error + expect(db[:apps].columns).not_to include(:service_binding_k8s_enabled) + expect(db[:apps].columns).not_to include(:file_based_vcap_services_enabled) + expect(check_constraint_exists?(db)).to be(false) + end + end + end + + context 'when not supported' do + before do + skip 'check constraint supported by db' if check_constraint_supported?(db) + end + + it 'does not fail' do + expect { run_rollback }.not_to raise_error + expect(db[:apps].columns).not_to include(:service_binding_k8s_enabled) + expect(db[:apps].columns).not_to include(:file_based_vcap_services_enabled) + end + end + end + end + end +end diff --git a/spec/request/app_features_spec.rb b/spec/request/app_features_spec.rb index 01abb54e15c..e475dade2ef 100644 --- a/spec/request/app_features_spec.rb +++ b/spec/request/app_features_spec.rb @@ -7,7 +7,17 @@ let(:admin_header) { admin_headers_for(user) } let(:org) { VCAP::CloudController::Organization.make(created_at: 3.days.ago) } let(:space) { VCAP::CloudController::Space.make(organization: org) } - let(:app_model) { VCAP::CloudController::AppModel.make(space: space, enable_ssh: true, file_based_service_bindings_enabled: true) } + let(:service_binding_k8s_enabled) { true } + let(:file_based_vcap_services_enabled) { false } + let(:request_body_enabled) { { body: { enabled: true } } } + let(:app_model) do + VCAP::CloudController::AppModel.make( + space: space, + enable_ssh: true, + service_binding_k8s_enabled: service_binding_k8s_enabled, + file_based_vcap_services_enabled: file_based_vcap_services_enabled + ) + end describe 'GET /v3/apps/:guid/features' do context 'getting a list of available features for the app' do @@ -26,14 +36,19 @@ 'enabled' => true }, { - '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' => true + }, + { + '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/#{app_model.guid}/features" }, 'last' => { 'href' => "/v3/apps/#{app_model.guid}/features" }, @@ -100,18 +115,31 @@ it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS end - context 'file-based-service-bindings app feature' do - let(:api_call) { ->(user_headers) { get "/v3/apps/#{app_model.guid}/features/file-based-service-bindings", nil, user_headers } } + context 'service-binding-k8s app feature' do + let(:api_call) { ->(user_headers) { get "/v3/apps/#{app_model.guid}/features/service-binding-k8s", nil, user_headers } } let(:feature_response_object) do { - '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' => true } end it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS end + + context 'file-based-vcap-services app feature' do + let(:api_call) { ->(user_headers) { get "/v3/apps/#{app_model.guid}/features/file-based-vcap-services", nil, user_headers } } + let(:feature_response_object) do + { + 'name' => 'file-based-vcap-services', + 'description' => 'Enable file-based VCAP service bindings for the app', + 'enabled' => false + } + end + + it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + end end describe 'PATCH /v3/apps/:guid/features/:name' do @@ -191,37 +219,149 @@ end end - context 'file-based-service-bindings app feature' do - let(:api_call) { ->(user_headers) { patch "/v3/apps/#{app_model.guid}/features/file-based-service-bindings", request_body.to_json, user_headers } } - let(:feature_response_object) do - { - 'name' => 'file-based-service-bindings', - 'description' => 'Enable file-based service bindings for the app', - 'enabled' => false - } + context 'service-binding-k8s app feature' do + context 'when feature is enabled' do + let(:service_binding_k8s_enabled) { true } + let(:api_call) { ->(user_headers) { patch "/v3/apps/#{app_model.guid}/features/service-binding-k8s", request_body.to_json, user_headers } } + let(:feature_response_object) do + { + 'name' => 'service-binding-k8s', + 'description' => 'Enable k8s service bindings for the app', + 'enabled' => false + } + end + + let(:expected_codes_and_responses) do + h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze) + %w[no_role org_auditor org_billing_manager].each { |r| h[r] = { code: 404 } } + %w[admin space_developer].each { |r| h[r] = { code: 200, response_object: feature_response_object } } + h + end + + it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + + context 'when organization is suspended' do + let(:expected_codes_and_responses) do + h = super() + h['space_developer'] = { code: 403, errors: CF_ORG_SUSPENDED } + h + end + + before do + org.update(status: VCAP::CloudController::Organization::SUSPENDED) + end + + it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + end end - let(:expected_codes_and_responses) do - h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze) - %w[no_role org_auditor org_billing_manager].each { |r| h[r] = { code: 404 } } - %w[admin space_developer].each { |r| h[r] = { code: 200, response_object: feature_response_object } } - h + context 'when feature is disabled' do + let(:service_binding_k8s_enabled) { false } + let(:api_call) { ->(user_headers) { patch "/v3/apps/#{app_model.guid}/features/service-binding-k8s", request_body_enabled.to_json, user_headers } } + + let(:feature_response_object) do + { + 'name' => 'service-binding-k8s', + 'description' => 'Enable k8s service bindings for the app', + 'enabled' => true + } + end + + let(:expected_codes_and_responses) do + h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze) + %w[no_role org_auditor org_billing_manager].each { |r| h[r] = { code: 404 } } + %w[admin space_developer].each { |r| h[r] = { code: 200, response_object: feature_response_object } } + h + end + + it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + + context 'when file-based-vcap-services is enabled' do + before do + patch "/v3/apps/#{app_model.guid}/features/file-based-vcap-services", request_body_enabled.to_json, admin_header + end + + it 'returns an error which states that both features cannot be enabled at the same time' do + patch "/v3/apps/#{app_model.guid}/features/service-binding-k8s", request_body_enabled.to_json, admin_header + + expect(last_response.status).to eq(422) + expect(parsed_response['errors'][0]['detail']).to eq("'file-based-vcap-services' and 'service-binding-k8s' features cannot be enabled at the same time.") + end + end end + end - it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + context 'file-based-vcap-services app feature' do + context 'when feature is enabled' do + let(:service_binding_k8s_enabled) { false } + let(:file_based_vcap_services_enabled) { true } + let(:api_call) { ->(user_headers) { patch "/v3/apps/#{app_model.guid}/features/file-based-vcap-services", request_body.to_json, user_headers } } + let(:feature_response_object) do + { + 'name' => 'file-based-vcap-services', + 'description' => 'Enable file-based VCAP service bindings for the app', + 'enabled' => false + } + end - context 'when organization is suspended' do let(:expected_codes_and_responses) do - h = super() - h['space_developer'] = { code: 403, errors: CF_ORG_SUSPENDED } + h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze) + %w[no_role org_auditor org_billing_manager].each { |r| h[r] = { code: 404 } } + %w[admin space_developer].each { |r| h[r] = { code: 200, response_object: feature_response_object } } h end - before do - org.update(status: VCAP::CloudController::Organization::SUSPENDED) + it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + + context 'when organization is suspended' do + let(:expected_codes_and_responses) do + h = super() + h['space_developer'] = { code: 403, errors: CF_ORG_SUSPENDED } + h + end + + before do + org.update(status: VCAP::CloudController::Organization::SUSPENDED) + end + + it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + end + end + + context 'when feature is disabled' do + let(:service_binding_k8s_enabled) { false } + let(:file_based_vcap_services_enabled) { false } + let(:api_call) { ->(user_headers) { patch "/v3/apps/#{app_model.guid}/features/file-based-vcap-services", request_body_enabled.to_json, user_headers } } + + let(:feature_response_object) do + { + 'name' => 'file-based-vcap-services', + 'description' => 'Enable file-based VCAP service bindings for the app', + 'enabled' => true + } + end + + let(:expected_codes_and_responses) do + h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze) + %w[no_role org_auditor org_billing_manager].each { |r| h[r] = { code: 404 } } + %w[admin space_developer].each { |r| h[r] = { code: 200, response_object: feature_response_object } } + h end it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + + context 'when service-binding-k8s is enabled' do + before do + patch "/v3/apps/#{app_model.guid}/features/service-binding-k8s", request_body_enabled.to_json, admin_header + end + + it 'returns an error which states that both features cannot be enabled at the same time' do + patch "/v3/apps/#{app_model.guid}/features/file-based-vcap-services", request_body_enabled.to_json, admin_header + + expect(last_response.status).to eq(422) + expect(parsed_response['errors'][0]['detail']).to eq("'file-based-vcap-services' and 'service-binding-k8s' features cannot be enabled at the same time.") + end + end end end end diff --git a/spec/unit/actions/app_feature_update_spec.rb b/spec/unit/actions/app_feature_update_spec.rb index 071c00287a9..fea88f16a7c 100644 --- a/spec/unit/actions/app_feature_update_spec.rb +++ b/spec/unit/actions/app_feature_update_spec.rb @@ -5,7 +5,7 @@ module VCAP::CloudController RSpec.describe AppFeatureUpdate do subject(:app_feature_update) { AppFeatureUpdate } - let(:app) { AppModel.make(enable_ssh: false, revisions_enabled: false, file_based_service_bindings_enabled: false) } + let(:app) { AppModel.make(enable_ssh: false, revisions_enabled: false) } let(:message) { AppFeatureUpdateMessage.new(enabled: true) } describe '.update' do @@ -25,11 +25,19 @@ module VCAP::CloudController end end - context 'when the feature name is file-based-service-bindings' do - it 'updates the file_based_service_bindings_enabled column on the app' do + context 'when the feature name is service-binding-k8s' do + it 'updates the service_binding_k8s_enabled column on the app' do expect do - AppFeatureUpdate.update('file-based-service-bindings', app, message) - end.to change { app.reload.file_based_service_bindings_enabled }.to(true) + AppFeatureUpdate.update('service-binding-k8s', app, message) + end.to change { app.reload.service_binding_k8s_enabled }.to(true) + end + end + + context 'when the feature name is file-based-vcap-services' do + it 'updates the file_based_vcap_services_enabled column on the app' do + expect do + AppFeatureUpdate.update('file-based-vcap-services', app, message) + end.to change { app.reload.file_based_vcap_services_enabled }.to(true) end end end diff --git a/spec/unit/controllers/v3/app_features_controller_spec.rb b/spec/unit/controllers/v3/app_features_controller_spec.rb index f1317edcb39..8cc85b87466 100644 --- a/spec/unit/controllers/v3/app_features_controller_spec.rb +++ b/spec/unit/controllers/v3/app_features_controller_spec.rb @@ -4,14 +4,17 @@ ## NOTICE: Prefer request specs over controller specs as per ADR #0003 ## RSpec.describe AppFeaturesController, type: :controller do - let(:app_model) { VCAP::CloudController::AppModel.make(enable_ssh: true, file_based_service_bindings_enabled: true) } + let(:app_model) { VCAP::CloudController::AppModel.make(enable_ssh: true, service_binding_k8s_enabled: true) } let(:space) { app_model.space } let(:org) { space.organization } let(:user) { VCAP::CloudController::User.make } let(:app_feature_ssh_response) { { 'name' => 'ssh', 'description' => 'Enable SSHing into the app.', 'enabled' => true } } let(:app_feature_revisions_response) { { 'name' => 'revisions', 'description' => 'Enable versioning of an application', 'enabled' => true } } - let(:app_feature_file_based_service_bindings_response) do - { 'name' => 'file-based-service-bindings', 'description' => 'Enable file-based service bindings for the app', 'enabled' => true } + let(:app_feature_service_binding_k8s_response) do + { 'name' => 'service-binding-k8s', 'description' => 'Enable k8s service bindings for the app', 'enabled' => true } + end + let(:app_feature_file_based_vcap_services_response) do + { 'name' => 'file-based-vcap-services', 'description' => 'Enable file-based VCAP service bindings for the app', 'enabled' => false } end before do @@ -23,7 +26,7 @@ describe '#index' do let(:pagination_hash) do { - 'total_results' => 3, + 'total_results' => 4, 'total_pages' => 1, 'first' => { 'href' => "/v3/apps/#{app_model.guid}/features" }, 'last' => { 'href' => "/v3/apps/#{app_model.guid}/features" }, @@ -42,7 +45,7 @@ it 'returns app features' do get :index, params: { app_guid: app_model.guid } expect(parsed_body).to eq( - 'resources' => [app_feature_ssh_response, app_feature_revisions_response, app_feature_file_based_service_bindings_response], + 'resources' => [app_feature_ssh_response, app_feature_revisions_response, app_feature_service_binding_k8s_response, app_feature_file_based_vcap_services_response], 'pagination' => pagination_hash ) end @@ -70,9 +73,14 @@ expect(parsed_body).to eq(app_feature_revisions_response) end - it 'returns the file-based-service-bindings app feature' do - get :show, params: { app_guid: app_model.guid, name: 'file-based-service-bindings' } - expect(parsed_body).to eq(app_feature_file_based_service_bindings_response) + it 'returns the service-binding-k8s app feature' do + get :show, params: { app_guid: app_model.guid, name: 'service-binding-k8s' } + expect(parsed_body).to eq(app_feature_service_binding_k8s_response) + end + + it 'returns the file-based-vcap-services feature' do + get :show, params: { app_guid: app_model.guid, name: 'file-based-vcap-services' } + expect(parsed_body).to eq(app_feature_file_based_vcap_services_response) end it 'throws 404 for a non-existent feature' do diff --git a/spec/unit/presenters/v3/app_feature_presenter_spec.rb b/spec/unit/presenters/v3/app_feature_presenter_spec.rb index c720a02f9de..4abc8e972a5 100644 --- a/spec/unit/presenters/v3/app_feature_presenter_spec.rb +++ b/spec/unit/presenters/v3/app_feature_presenter_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' require 'presenters/v3/app_ssh_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' module VCAP::CloudController::Presenters::V3 RSpec.describe AppSshFeaturePresenter do @@ -16,15 +17,28 @@ module VCAP::CloudController::Presenters::V3 end end - RSpec.describe AppFileBasedServiceBindingsFeaturePresenter do + RSpec.describe AppServiceBindingK8sFeaturePresenter do let(:app) { VCAP::CloudController::AppModel.make } describe '#to_hash' do it 'presents the app feature as json' do - result = AppFileBasedServiceBindingsFeaturePresenter.new(app).to_hash - expect(result[:name]).to eq('file-based-service-bindings') - expect(result[:description]).to eq('Enable file-based service bindings for the app') - expect(result[:enabled]).to eq(app.file_based_service_bindings_enabled) + result = AppServiceBindingK8sFeaturePresenter.new(app).to_hash + expect(result[:name]).to eq('service-binding-k8s') + expect(result[:description]).to eq('Enable k8s service bindings for the app') + expect(result[:enabled]).to eq(app.service_binding_k8s_enabled) + end + end + end + + RSpec.describe AppFileBasedVcapServicesFeaturePresenter do + let(:app) { VCAP::CloudController::AppModel.make } + + describe '#to_hash' do + it 'presents the app feature as json' do + result = AppFileBasedVcapServicesFeaturePresenter.new(app).to_hash + expect(result[:name]).to eq('file-based-vcap-services') + expect(result[:description]).to eq('Enable file-based VCAP service bindings for the app') + expect(result[:enabled]).to eq(app.file_based_vcap_services_enabled) end end end