From 80615d7a22ccf80535321f136c8c77cb0df85d4a Mon Sep 17 00:00:00 2001 From: i068838 Date: Mon, 26 Feb 2018 15:09:21 +0530 Subject: [PATCH 1/5] Catalog handling in case og cross consumption * Parameterized the path that gets registered as broker. * Added settings for supported_platform and osbapi_compliant_name * Catalogs are shown based on the supported_platform * Changed implementation for PlatformManagers, instance assignment is removed from it. * Added new test for K8S catalog * Fixed Quota test which was changing the catalog --- config/settings.yml | 15 +++++ lib/constants.js | 10 +++- lib/controllers/ServiceBrokerApiController.js | 11 +--- lib/fabrik/BasePlatformManager.js | 24 +++----- lib/fabrik/CfPlatformManager.js | 55 +++++++++++-------- lib/fabrik/DirectorInstance.js | 17 ++++-- lib/fabrik/DockerInstance.js | 8 ++- lib/fabrik/Fabrik.js | 12 +++- lib/fabrik/K8sPlatformManager.js | 43 +++++++++++++++ lib/fabrik/VirtualHostInstance.js | 8 ++- lib/routes/{cf => broker}/index.js | 4 +- lib/routes/{cf => broker}/v2.js | 4 +- lib/routes/index.js | 2 +- server.js | 2 +- .../service-broker-api.catalog.spec.js | 41 ++++++++++++-- test/fabrik.ServiceBrokerApi.spec.js | 16 +++++- test/quota.QuotaManager.spec.js | 2 +- test/support/apps.js | 2 +- 18 files changed, 210 insertions(+), 66 deletions(-) create mode 100644 lib/fabrik/K8sPlatformManager.js rename lib/routes/{cf => broker}/index.js (66%) rename lib/routes/{cf => broker}/v2.js (95%) diff --git a/config/settings.yml b/config/settings.yml index c03c04ca8..7a833a904 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -432,6 +432,9 @@ defaults: &defaults secret: 'service-fabrik-blueprint-secret' redirect_uri: <%= redirect_uri %> plan_updateable: true + supported_platform: + - k8s + - cf plans: ########################### # CONTAINER SERVICE PLANS # @@ -565,6 +568,10 @@ defaults: &defaults - '1 vCPUs' - '1 GB Memory' - '1 GB Disk' + supported_platform: + - k8s + - cf + osbapi_compliant_name: 'v10-xsmall' - &blueprint_plan4 id: 'bc158c9a-7934-401e-94ab-057082a5073e' name: 'v1.0-small' @@ -594,6 +601,10 @@ defaults: &defaults - '1 vCPUs' - '1 GB Memory' - '2 GB Disk' + supported_platform: + - k8s + - cf + osbapi_compliant_name: 'v10-small' - &blueprint_plan5 id: 'd616b00a-5949-4b1c-bc73-0d3c59f3954a' name: 'v1.0-large' @@ -624,6 +635,10 @@ defaults: &defaults - '1 vCPUs' - '2 GB Memory' - '2 GB Disk' + supported_platform: + - k8s + - cf + osbapi_compliant_name: 'v10-large' - &blueprint_plan6 id: 'b91d9512-b5c9-4c4a-922a-fa54ae67d235' name: 'v1.0-dedicated-large-deprecated' diff --git a/lib/constants.js b/lib/constants.js index bea7f9e2f..544b121cb 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -181,8 +181,16 @@ module.exports = Object.freeze({ K8S: 'kubernetes' }, + PLATFORM_ALIAS: { + CF: 'cf', + K8S: 'k8s' + }, + PLATFORM_MANAGER: { - 'cloudfoundry': 'CfPlatformManager' + 'cloudfoundry': 'CfPlatformManager', + 'cf': 'CfPlatformManager', + 'kubernetes': 'K8sPlatformManager', + 'k8s': 'K8sPlatformManager' }, AGENT: { diff --git a/lib/controllers/ServiceBrokerApiController.js b/lib/controllers/ServiceBrokerApiController.js index 03cfee75e..d489e57ea 100644 --- a/lib/controllers/ServiceBrokerApiController.js +++ b/lib/controllers/ServiceBrokerApiController.js @@ -26,7 +26,6 @@ class ServiceBrokerApiController extends BaseController { /* jshint unused:false */ const minVersion = CONST.SF_BROKER_API_VERSION_MIN; const maybeVersion = CONST.SF_BROKER_API_VERSION_MAYBE; - const minCloudControllerApiVersion = '2.57.0'; const version = _.get(req.headers, 'x-broker-api-version', '1.0'); return Promise .try(() => { @@ -36,20 +35,14 @@ class ServiceBrokerApiController extends BaseController { if (utils.compareVersions(version, maybeVersion) < 0) { throw new PreconditionFailed(`At least Broker API version ${minVersion} is required.`); } - return this.cloudController - .getApiVersion() - .then(apiVersion => { - if (utils.compareVersions(apiVersion, minCloudControllerApiVersion) < 0) { - throw new PreconditionFailed(`At least Cloud Controller API version ${minCloudControllerApiVersion} is required.`); - } - }); + return this.fabrik.getPlatformManager(req.params.platform).performPlatformSpecificChecks(); }) .throw(new ContinueWithNext()); } getCatalog(req, res) { /* jshint unused:false */ - res.status(200).json(catalog); + res.status(200).json(this.fabrik.getPlatformManager(req.params.platform).getPlatformSpecificCatalog(catalog)); } putInstance(req, res) { diff --git a/lib/fabrik/BasePlatformManager.js b/lib/fabrik/BasePlatformManager.js index 4436c1292..fcdf01b90 100644 --- a/lib/fabrik/BasePlatformManager.js +++ b/lib/fabrik/BasePlatformManager.js @@ -3,13 +3,16 @@ const CONST = require('../constants'); class BasePlatformManager { - constructor(guid, context) { - this.guid = guid; - this.context = context; + constructor(platform) { + this.platform = platform; } - get platform() { - return this.context.platform; + performPlatformSpecificChecks() { + + } + + getPlatformSpecificCatalog(catalog) { + return catalog; } postInstanceProvisionOperations(options) { @@ -24,18 +27,9 @@ class BasePlatformManager { /* jshint unused:false */ } - ensureTenantId(tenant_id) { + ensureTenantId(options) { /* jshint unused:false */ } - static getInstance(instance_id, context) { - const PlatformManager = (context && CONST.PLATFORM_MANAGER[context.platform]) ? require(`./${CONST.PLATFORM_MANAGER[context.platform]}`) : undefined;   - if (PlatformManager === undefined) { - return new BasePlatformManager(instance_id, context);   - } else {  - return new PlatformManager(instance_id, context);   - } - } - } module.exports = BasePlatformManager; \ No newline at end of file diff --git a/lib/fabrik/CfPlatformManager.js b/lib/fabrik/CfPlatformManager.js index 31ffae77a..05e5695db 100644 --- a/lib/fabrik/CfPlatformManager.js +++ b/lib/fabrik/CfPlatformManager.js @@ -10,41 +10,52 @@ const cloudController = require('../cf').cloudController; const logger = require('../logger'); const CONST = require('../constants'); const SecurityGroupNotCreated = errors.SecurityGroupNotCreated; +const PreconditionFailed = errors.PreconditionFailed; const SecurityGroupNotFound = errors.SecurityGroupNotFound; const ordinals = ['First', 'Second', 'Third', 'Fourth', 'Fifth', 'Sixth', 'Seventh', 'Eighth', 'Ninth', 'Tenth']; class CfPlatformManager extends BasePlatformManager { - constructor(guid, context) { - super(guid, context); - this.space_guid = context.space_guid; + constructor(platform) { + super(platform); this.cloudController = cloudController; } - get securityGroupName() { - return `${CONST.SERVICE_FABRIK_PREFIX}-${this.guid}`; + performPlatformSpecificChecks() { + const minCloudControllerApiVersion = '2.57.0'; + return this.cloudController + .getApiVersion() + .then(apiVersion => { + if (utils.compareVersions(apiVersion, minCloudControllerApiVersion) < 0) { + throw new PreconditionFailed(`At least Cloud Controller API version ${minCloudControllerApiVersion} is required.`); + } + }); + } + + getSecurityGroupName(guid) { + return `${CONST.SERVICE_FABRIK_PREFIX}-${guid}`; } postInstanceProvisionOperations(options) { - return this.createSecurityGroup(options.ipRuleOptions); + return this.createSecurityGroup(options); } - preInstanceDeleteOperations() { - return this.deleteSecurityGroup(); + preInstanceDeleteOperations(options) { + return this.deleteSecurityGroup(options); } postInstanceUpdateOperations(options) { - return this.ensureSecurityGroupExists(options.ipRuleOptions); + return this.ensureSecurityGroupExists(options); } - createSecurityGroup(ruleOptions) { - const name = this.securityGroupName; - const rules = _.map(ruleOptions, opts => this.buildSecurityGroupRules(opts)); + createSecurityGroup(options) { + const name = this.getSecurityGroupName(options.guid); + const rules = _.map(options.ruleOptions, opts => this.buildSecurityGroupRules(opts)); logger.info(`Creating security group '${name}' with rules ...`, rules); return utils .retry(tries => { logger.info(`+-> ${ordinals[tries]} attempt to create security group '${name}'...`); return this.cloudController - .createSecurityGroup(name, rules, [this.space_guid]) + .createSecurityGroup(name, rules, [options.context.space_guid]) .catch(err => { logger.error(err); throw err; @@ -62,21 +73,21 @@ class CfPlatformManager extends BasePlatformManager { }); } - ensureSecurityGroupExists(ruleOptions) { - const name = this.securityGroupName; + ensureSecurityGroupExists(options) { + const name = this.getSecurityGroupName(options.guid); logger.info(`Ensuring existence of security group '${name}'...`); return this.cloudController .findSecurityGroupByName(name) .tap(() => logger.info('+-> Security group exists')) .catch(SecurityGroupNotFound, () => { logger.warn('+-> Security group does not exist. Trying to create it again.'); - return this.ensureTenantId(this.space_guid) - .then(() => this.createSecurityGroup(ruleOptions)); + return this.ensureTenantId(options.context.space_guid) + .then(() => this.createSecurityGroup(options.ruleOptions)); }); } - deleteSecurityGroup() { - const name = this.securityGroupName; + deleteSecurityGroup(options) { + const name = this.getSecurityGroupName(options.guid); logger.info(`Deleting security group '${name}'...`); return this.cloudController .findSecurityGroupByName(name) @@ -95,10 +106,10 @@ class CfPlatformManager extends BasePlatformManager { }); } - ensureTenantId(space_guid) { + ensureTenantId(options) { return Promise - .try(() => space_guid ? space_guid : this.cloudController - .getServiceInstance(this.guid) + .try(() => options.context.space_guid ? options.context.space_guid : this.cloudController + .getServiceInstance(options.guid) .then(instance => instance.entity.space_guid) ); } diff --git a/lib/fabrik/DirectorInstance.js b/lib/fabrik/DirectorInstance.js index 3004ed5f6..0727f33f7 100644 --- a/lib/fabrik/DirectorInstance.js +++ b/lib/fabrik/DirectorInstance.js @@ -71,7 +71,9 @@ class DirectorInstance extends BaseInstance { if (operation.type === 'delete') { return Promise .all([ - this.platformManager.preInstanceDeleteOperations(), + this.platformManager.preInstanceDeleteOperations({ + guid: this.guid + }), this.deleteRestoreFile() ]); } @@ -80,7 +82,10 @@ class DirectorInstance extends BaseInstance { deleteRestoreFile() { if (_.includes(this.manager.agent.features, 'backup')) { - return Promise.try(() => this.platformManager.ensureTenantId()) + return Promise.try(() => this.platformManager.ensureTenantId({ + context: this.platformContext, + guid: this.guid + })) .then(tenant_id => tenant_id ? this.manager.deleteRestoreFile(tenant_id, this.guid) : Promise.resolve({})) .catch(err => { logger.error(`Failed to delete restore file of instance '${this.guid}'`); @@ -110,13 +115,17 @@ class DirectorInstance extends BaseInstance { minDelay: 1000 }) .then(() => this.platformManager.postInstanceProvisionOperations({ - ipRuleOptions: this.buildIpRules() + ipRuleOptions: this.buildIpRules(), + guid: this.guid, + context: this.platformContext })) .tap(() => operation.state === CONST.OPERATION.SUCCEEDED ? this.scheduleAutoUpdate() : {}); case 'update': return this.platformManager.postInstanceUpdateOperations({ - ipRuleOptions: this.buildIpRules() + ipRuleOptions: this.buildIpRules(), + guid: this.guid, + context: this.platformContext }); } }) diff --git a/lib/fabrik/DockerInstance.js b/lib/fabrik/DockerInstance.js index 0baf88016..65e898de1 100644 --- a/lib/fabrik/DockerInstance.js +++ b/lib/fabrik/DockerInstance.js @@ -81,7 +81,9 @@ class DockerInstance extends BaseInstance { .catchThrow(DockerError.Conflict, new ServiceInstanceAlreadyExists(this.guid)) .then(() => this.ensureContainerIsRunning(true)) .then(() => this.platformManager.postInstanceProvisionOperations({ - ipRuleOptions: this.buildIpRules() + ipRuleOptions: this.buildIpRules(), + guid: this.guid, + context: this.platformContext })); } @@ -106,7 +108,9 @@ class DockerInstance extends BaseInstance { delete(params) { /* jshint unused:false */ return Promise.try(() => this - .platformManager.preInstanceDeleteOperations() + .platformManager.preInstanceDeleteOperations({ + guid: this.guid + }) ) .then(() => this.removeContainer()) .catchThrow(DockerError.NotFound, new ServiceInstanceNotFound(this.guid)) diff --git a/lib/fabrik/Fabrik.js b/lib/fabrik/Fabrik.js index 5b0fd1b1d..a0c872815 100644 --- a/lib/fabrik/Fabrik.js +++ b/lib/fabrik/Fabrik.js @@ -13,6 +13,7 @@ const FabrikStatusPoller = require('./FabrikStatusPoller'); const DBManager = require('./DBManager'); const OobBackupManager = require('./OobBackupManager'); const BasePlatformManager = require('./BasePlatformManager'); +const CONST = require('../constants'); class Fabrik { static createManager(plan) { @@ -41,11 +42,20 @@ class Fabrik { const instance = manager.createInstance(instance_id); return Promise .try(() => context ? context : instance.platformContext) - .then(context => instance.assignPlatformManager(BasePlatformManager.getInstance(instance_id, context))) + .then(context => instance.assignPlatformManager(Fabrik.getPlatformManager(context.platform))) .return(instance); }); } + static getPlatformManager(platform) { + const PlatformManager = (platform && CONST.PLATFORM_MANAGER[platform]) ? require(`./${CONST.PLATFORM_MANAGER[platform]}`) : undefined; + if (PlatformManager === undefined) { + return new BasePlatformManager(platform);   + } else {  + return new PlatformManager(platform);   + } + } + static createOperation(name, opts) { return new ServiceFabrikOperation(name, opts); } diff --git a/lib/fabrik/K8sPlatformManager.js b/lib/fabrik/K8sPlatformManager.js new file mode 100644 index 000000000..11a7409e7 --- /dev/null +++ b/lib/fabrik/K8sPlatformManager.js @@ -0,0 +1,43 @@ +'use strict'; + +const CONST = require('../constants'); +const BasePlatformManager = require('./BasePlatformManager'); +const _ = require('lodash'); + +class K8sPlatformManager extends BasePlatformManager { + constructor(platform) { + super(platform); + this.platform = platform; + } + + getPlatformSpecificCatalog(catalog) { + const modifiedCatalog = _.cloneDeep(catalog); + const platform = this.platform; + _.remove(modifiedCatalog.services, function (service) { + _.remove(service.plans, function (plan) { + plan.name = plan.osbapi_compliant_name; + return !_.includes(_.get(plan, 'supported_platform'), platform); + }) + return !_.includes(_.get(service, 'supported_platform'), platform); + }); + return modifiedCatalog; + } + + postInstanceProvisionOperations(options) { + /* jshint unused:false */ + } + + preInstanceDeleteOperations(options) { + /* jshint unused:false */ + } + + postInstanceUpdateOperations(options) { + /* jshint unused:false */ + } + + ensureTenantId(options) { + /* jshint unused:false */ + } + +} +module.exports = K8sPlatformManager; \ No newline at end of file diff --git a/lib/fabrik/VirtualHostInstance.js b/lib/fabrik/VirtualHostInstance.js index f1e0d9eb0..c1659b0de 100644 --- a/lib/fabrik/VirtualHostInstance.js +++ b/lib/fabrik/VirtualHostInstance.js @@ -5,6 +5,7 @@ const bosh = require('../bosh'); const utils = require('../utils'); const mapper = require('./VirtualHostRelationMapper'); const catalog = require('../models').catalog; +const CONST = require('../constants'); class VirtualHostInstance extends BaseInstance { constructor(guid, manager) { @@ -24,7 +25,12 @@ class VirtualHostInstance extends BaseInstance { .then(deploymentName => this.deploymentName = deploymentName); } - get platformContext() {} + get platformContext() { + const context = { + platform: CONST.PLATFORM.CF + }; + return context; + } create(params) { return this.cloudController.getServiceInstanceByName(params.parameters.dedicated_rabbitmq_instance, params.space_guid) diff --git a/lib/routes/cf/index.js b/lib/routes/broker/index.js similarity index 66% rename from lib/routes/cf/index.js rename to lib/routes/broker/index.js index 61d9a3bc1..1a58d3ac2 100644 --- a/lib/routes/cf/index.js +++ b/lib/routes/broker/index.js @@ -2,5 +2,7 @@ const express = require('express'); require('../../utils/enableAbsMatchingRouteLookup')(express); -const router = module.exports = express.Router(); +const router = module.exports = express.Router({ + mergeParams: true +}); router.use('/v2', require('./v2')); \ No newline at end of file diff --git a/lib/routes/cf/v2.js b/lib/routes/broker/v2.js similarity index 95% rename from lib/routes/cf/v2.js rename to lib/routes/broker/v2.js index 2d1dc27fe..3b6a7ebe9 100644 --- a/lib/routes/cf/v2.js +++ b/lib/routes/broker/v2.js @@ -5,7 +5,9 @@ const config = require('../../config'); const middleware = require('../../middleware'); const controller = require('../../controllers').serviceBrokerApi; -const router = module.exports = express.Router(); +const router = module.exports = express.Router({ + mergeParams: true +}); const instanceRouter = express.Router({ mergeParams: true }); diff --git a/lib/routes/index.js b/lib/routes/index.js index 6a1f60981..8abe70bf6 100644 --- a/lib/routes/index.js +++ b/lib/routes/index.js @@ -1,6 +1,6 @@ 'use strict'; -exports.cf = require('./cf'); +exports.broker = require('./broker'); exports.manage = require('./manage'); exports.admin = require('./admin'); exports.api = require('./api'); diff --git a/server.js b/server.js index 09392b0f1..375da00d4 100644 --- a/server.js +++ b/server.js @@ -19,7 +19,7 @@ const internal = FabrikApp.create('internal', app => { }); app.use('/admin', routes.admin); // cloud foundry service broker api - app.use('/cf', routes.cf); + app.use('/:platform(cf|k8s)', routes.broker); }); // exernal app diff --git a/test/acceptance/service-broker-api.catalog.spec.js b/test/acceptance/service-broker-api.catalog.spec.js index 42c05e448..b4dda3c48 100644 --- a/test/acceptance/service-broker-api.catalog.spec.js +++ b/test/acceptance/service-broker-api.catalog.spec.js @@ -3,28 +3,61 @@ const lib = require('../../lib'); const app = require('../support/apps').internal; const config = lib.config; +const logger = lib.logger; const CONST = require('../../lib/constants'); describe('service-broker-api', function () { - describe('catalog', function () { - const baseUrl = '/cf/v2'; + describe('catalog-cf', function () { + const baseCFUrl = '/cf/v2'; it('returns 200 Ok', function () { return chai.request(app) - .get(`${baseUrl}/catalog`) + .get(`${baseCFUrl}/catalog`) .set('X-Broker-API-Version', CONST.SF_BROKER_API_VERSION_MIN) .auth(config.username, config.password) .then(res => { expect(res).to.have.status(200); expect(res.body.services).to.be.instanceof(Array); expect(res.body.services).to.have.length(2); + console.log(res.body.services[0].plans); + expect(res.body.services[0].plans).to.have.length(7); + expect(res.body.services[1].plans).to.have.length(2); }); }); it('returns 405 Method not allowed', function () { return chai.request(app) - .delete(`${baseUrl}/catalog`) + .delete(`${baseCFUrl}/catalog`) + .set('X-Broker-API-Version', CONST.SF_BROKER_API_VERSION_MIN) + .auth(config.username, config.password) + .catch(err => err.response) + .then(res => { + expect(res).to.have.status(405); + expect(res).to.have.header('allow', ['GET']); + }); + }); + }); + + describe('catalog-k8s', function () { + const baseK8sUrl = '/k8s/v2'; + + it('returns 200 Ok', function () { + return chai.request(app) + .get(`${baseK8sUrl}/catalog`) + .set('X-Broker-API-Version', CONST.SF_BROKER_API_VERSION_MIN) + .auth(config.username, config.password) + .then(res => { + expect(res).to.have.status(200); + expect(res.body.services).to.be.instanceof(Array); + expect(res.body.services).to.have.length(1); + expect(res.body.services[0].plans).to.have.length(3); + }); + }); + + it('returns 405 Method not allowed', function () { + return chai.request(app) + .delete(`${baseK8sUrl}/catalog`) .set('X-Broker-API-Version', CONST.SF_BROKER_API_VERSION_MIN) .auth(config.username, config.password) .catch(err => err.response) diff --git a/test/fabrik.ServiceBrokerApi.spec.js b/test/fabrik.ServiceBrokerApi.spec.js index bbb73b644..246872b2c 100644 --- a/test/fabrik.ServiceBrokerApi.spec.js +++ b/test/fabrik.ServiceBrokerApi.spec.js @@ -43,9 +43,23 @@ describe('fabrik', function () { .catch(err => expect(err).to.be.instanceof(PreconditionFailed)); }); - it('should call the next handler when version is 2.11', function () { + it('For CF : should call the next handler when version is 2.11', function () { cloudController.apiVersion = '2.57.0'; req.headers['x-broker-api-version'] = '2.11'; + req.params = { + platform: 'cf' + }; + return api + .apiVersion(req, res) + .throw(expectToThrow(ContinueWithNext)) + .catch(err => expect(err).to.be.instanceof(ContinueWithNext)); + }); + + it('For K8S : should call the next handler when version is 2.11', function () { + req.headers['x-broker-api-version'] = '2.11'; + req.params = { + platform: 'k8s' + }; return api .apiVersion(req, res) .throw(expectToThrow(ContinueWithNext)) diff --git a/test/quota.QuotaManager.spec.js b/test/quota.QuotaManager.spec.js index f628de10b..9041d2dea 100644 --- a/test/quota.QuotaManager.spec.js +++ b/test/quota.QuotaManager.spec.js @@ -479,7 +479,7 @@ describe('quota', () => { v2smallPlan.name = v2smallPlanName; v2smallPlan.id = v2smallPlanId; - const extendedCatalog = _.clone(catalog); + const extendedCatalog = _.cloneDeep(catalog); const bpService = _.find(extendedCatalog.services, ['name', serviceName]); bpService.plans.push(v2smallPlan); diff --git a/test/support/apps.js b/test/support/apps.js index 3d5276302..971f8a8d4 100644 --- a/test/support/apps.js +++ b/test/support/apps.js @@ -15,7 +15,7 @@ const internal = FabrikApp.create('internal', app => { }); app.use('/admin', routes.admin); // cloud foundry service broker api - app.use('/cf', routes.cf); + app.use('/:platform(cf|k8s)', routes.broker); }); // exernal app From 0cbc78d5dd2ef283e471cf81ca10c29a3fe1de0c Mon Sep 17 00:00:00 2001 From: i068838 Date: Fri, 2 Mar 2018 16:08:21 +0530 Subject: [PATCH 2/5] Fix jshint errors --- lib/fabrik/BasePlatformManager.js | 2 -- lib/fabrik/K8sPlatformManager.js | 3 +-- test/acceptance/service-broker-api.catalog.spec.js | 1 - 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/fabrik/BasePlatformManager.js b/lib/fabrik/BasePlatformManager.js index fcdf01b90..738f0e1c2 100644 --- a/lib/fabrik/BasePlatformManager.js +++ b/lib/fabrik/BasePlatformManager.js @@ -1,7 +1,5 @@ 'use strict'; -const CONST = require('../constants'); - class BasePlatformManager { constructor(platform) { this.platform = platform; diff --git a/lib/fabrik/K8sPlatformManager.js b/lib/fabrik/K8sPlatformManager.js index 11a7409e7..82ed2760a 100644 --- a/lib/fabrik/K8sPlatformManager.js +++ b/lib/fabrik/K8sPlatformManager.js @@ -1,6 +1,5 @@ 'use strict'; -const CONST = require('../constants'); const BasePlatformManager = require('./BasePlatformManager'); const _ = require('lodash'); @@ -17,7 +16,7 @@ class K8sPlatformManager extends BasePlatformManager { _.remove(service.plans, function (plan) { plan.name = plan.osbapi_compliant_name; return !_.includes(_.get(plan, 'supported_platform'), platform); - }) + }); return !_.includes(_.get(service, 'supported_platform'), platform); }); return modifiedCatalog; diff --git a/test/acceptance/service-broker-api.catalog.spec.js b/test/acceptance/service-broker-api.catalog.spec.js index b4dda3c48..fdfc4fe34 100644 --- a/test/acceptance/service-broker-api.catalog.spec.js +++ b/test/acceptance/service-broker-api.catalog.spec.js @@ -3,7 +3,6 @@ const lib = require('../../lib'); const app = require('../support/apps').internal; const config = lib.config; -const logger = lib.logger; const CONST = require('../../lib/constants'); From 51dadf91824a3b8e2d7edb607dc3bb0f15f74279 Mon Sep 17 00:00:00 2001 From: i068838 Date: Fri, 2 Mar 2018 16:46:19 +0530 Subject: [PATCH 3/5] Removing Plan name handling code as the osbapi change is LGTMed https://github.com/openservicebrokerapi/servicebroker/pull/452 --- config/settings.yml | 3 --- lib/fabrik/K8sPlatformManager.js | 1 - test/fabrik.ServiceBrokerApi.spec.js | 4 ++-- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/config/settings.yml b/config/settings.yml index 7a833a904..f61af69e0 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -571,7 +571,6 @@ defaults: &defaults supported_platform: - k8s - cf - osbapi_compliant_name: 'v10-xsmall' - &blueprint_plan4 id: 'bc158c9a-7934-401e-94ab-057082a5073e' name: 'v1.0-small' @@ -604,7 +603,6 @@ defaults: &defaults supported_platform: - k8s - cf - osbapi_compliant_name: 'v10-small' - &blueprint_plan5 id: 'd616b00a-5949-4b1c-bc73-0d3c59f3954a' name: 'v1.0-large' @@ -638,7 +636,6 @@ defaults: &defaults supported_platform: - k8s - cf - osbapi_compliant_name: 'v10-large' - &blueprint_plan6 id: 'b91d9512-b5c9-4c4a-922a-fa54ae67d235' name: 'v1.0-dedicated-large-deprecated' diff --git a/lib/fabrik/K8sPlatformManager.js b/lib/fabrik/K8sPlatformManager.js index 82ed2760a..5ca019601 100644 --- a/lib/fabrik/K8sPlatformManager.js +++ b/lib/fabrik/K8sPlatformManager.js @@ -14,7 +14,6 @@ class K8sPlatformManager extends BasePlatformManager { const platform = this.platform; _.remove(modifiedCatalog.services, function (service) { _.remove(service.plans, function (plan) { - plan.name = plan.osbapi_compliant_name; return !_.includes(_.get(plan, 'supported_platform'), platform); }); return !_.includes(_.get(service, 'supported_platform'), platform); diff --git a/test/fabrik.ServiceBrokerApi.spec.js b/test/fabrik.ServiceBrokerApi.spec.js index 81acbb2cc..9e8c08e3e 100644 --- a/test/fabrik.ServiceBrokerApi.spec.js +++ b/test/fabrik.ServiceBrokerApi.spec.js @@ -51,8 +51,8 @@ describe('fabrik', function () { }; return api .apiVersion(req, res) - .throw(expectToThrow(ContinueWithNext)) - .catch(err => expect(err).to.be.instanceof(ContinueWithNext)); + .throw(expectToThrow(PreconditionFailed)) + .catch(err => expect(err).to.be.instanceof(PreconditionFailed)); }); it('For K8S : should call the next handler when version is 2.11', function () { From f0eb52e8465ecc0a47a443a27e3eb084a2ea5312 Mon Sep 17 00:00:00 2001 From: i068838 Date: Mon, 5 Mar 2018 09:11:45 +0530 Subject: [PATCH 4/5] Removing unnecessary CC api version check CC API version check was added to support 2.8 version. Removing that check now. --- lib/controllers/ServiceBrokerApiController.js | 1 - lib/fabrik/BasePlatformManager.js | 4 ---- lib/fabrik/CfPlatformManager.js | 12 ------------ test/fabrik.ServiceBrokerApi.spec.js | 13 +++---------- 4 files changed, 3 insertions(+), 27 deletions(-) diff --git a/lib/controllers/ServiceBrokerApiController.js b/lib/controllers/ServiceBrokerApiController.js index e7072b6ba..24e4d5847 100644 --- a/lib/controllers/ServiceBrokerApiController.js +++ b/lib/controllers/ServiceBrokerApiController.js @@ -33,7 +33,6 @@ class ServiceBrokerApiController extends BaseController { } else { throw new PreconditionFailed(`At least Broker API version ${minVersion} is required.`); } - return this.fabrik.getPlatformManager(req.params.platform).performPlatformSpecificChecks(); }) .throw(new ContinueWithNext()); } diff --git a/lib/fabrik/BasePlatformManager.js b/lib/fabrik/BasePlatformManager.js index 738f0e1c2..2574233de 100644 --- a/lib/fabrik/BasePlatformManager.js +++ b/lib/fabrik/BasePlatformManager.js @@ -5,10 +5,6 @@ class BasePlatformManager { this.platform = platform; } - performPlatformSpecificChecks() { - - } - getPlatformSpecificCatalog(catalog) { return catalog; } diff --git a/lib/fabrik/CfPlatformManager.js b/lib/fabrik/CfPlatformManager.js index 05e5695db..83f4a7fea 100644 --- a/lib/fabrik/CfPlatformManager.js +++ b/lib/fabrik/CfPlatformManager.js @@ -10,7 +10,6 @@ const cloudController = require('../cf').cloudController; const logger = require('../logger'); const CONST = require('../constants'); const SecurityGroupNotCreated = errors.SecurityGroupNotCreated; -const PreconditionFailed = errors.PreconditionFailed; const SecurityGroupNotFound = errors.SecurityGroupNotFound; const ordinals = ['First', 'Second', 'Third', 'Fourth', 'Fifth', 'Sixth', 'Seventh', 'Eighth', 'Ninth', 'Tenth']; @@ -20,17 +19,6 @@ class CfPlatformManager extends BasePlatformManager { this.cloudController = cloudController; } - performPlatformSpecificChecks() { - const minCloudControllerApiVersion = '2.57.0'; - return this.cloudController - .getApiVersion() - .then(apiVersion => { - if (utils.compareVersions(apiVersion, minCloudControllerApiVersion) < 0) { - throw new PreconditionFailed(`At least Cloud Controller API version ${minCloudControllerApiVersion} is required.`); - } - }); - } - getSecurityGroupName(guid) { return `${CONST.SERVICE_FABRIK_PREFIX}-${guid}`; } diff --git a/test/fabrik.ServiceBrokerApi.spec.js b/test/fabrik.ServiceBrokerApi.spec.js index 9e8c08e3e..bc09a812a 100644 --- a/test/fabrik.ServiceBrokerApi.spec.js +++ b/test/fabrik.ServiceBrokerApi.spec.js @@ -3,7 +3,6 @@ const lib = require('../lib'); const api = lib.controllers.serviceBrokerApi; const errors = lib.errors; -const cloudController = lib.cf.cloudController; const PreconditionFailed = errors.PreconditionFailed; const ContinueWithNext = errors.ContinueWithNext; @@ -22,10 +21,6 @@ describe('fabrik', function () { return new Error(`Expected error '${clazz.name}' has not been thrown`); } - after(function () { - cloudController.apiVersion = undefined; - }); - it('should abort with a PreconditionFailed error when version is 2.7', function () { req.headers['x-broker-api-version'] = '2.7'; return api @@ -34,8 +29,7 @@ describe('fabrik', function () { .catch(err => expect(err).to.be.instanceof(PreconditionFailed)); }); - it('should call the next handler when version is 2.8', function () { - cloudController.apiVersion = '2.55.0'; + it('should abort with a PreconditionFailed error when version is 2.8', function () { req.headers['x-broker-api-version'] = '2.8'; return api .apiVersion(req, res) @@ -43,8 +37,7 @@ describe('fabrik', function () { .catch(err => expect(err).to.be.instanceof(PreconditionFailed)); }); - it('For CF : should call the next handler when version is 2.11', function () { - cloudController.apiVersion = '2.57.0'; + it('For CF : should abort with a PreconditionFailed error when version is 2.11', function () { req.headers['x-broker-api-version'] = '2.11'; req.params = { platform: 'cf' @@ -55,7 +48,7 @@ describe('fabrik', function () { .catch(err => expect(err).to.be.instanceof(PreconditionFailed)); }); - it('For K8S : should call the next handler when version is 2.11', function () { + it('For K8S : should abort with a PreconditionFailed error when version is 2.11', function () { req.headers['x-broker-api-version'] = '2.11'; req.params = { platform: 'k8s' From cad8ee259f9a4c7fbd7f51f04cbb06e72fb8ce08 Mon Sep 17 00:00:00 2001 From: i068838 Date: Wed, 7 Mar 2018 14:32:54 +0530 Subject: [PATCH 5/5] Incorporating review comment. changing the name of method getCatalog in platform specific catalog filtering Removed the duped key values --- lib/constants.js | 10 ++++------ lib/controllers/ServiceBrokerApiController.js | 2 +- lib/fabrik/BasePlatformManager.js | 2 +- lib/fabrik/Fabrik.js | 2 +- lib/fabrik/K8sPlatformManager.js | 2 +- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/constants.js b/lib/constants.js index 2d17a4860..f0a33cebf 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -180,16 +180,14 @@ module.exports = Object.freeze({ K8S: 'kubernetes' }, - PLATFORM_ALIAS: { - CF: 'cf', - K8S: 'k8s' + PLATFORM_ALIAS_MAPPINGS: { + 'cf': 'cloudfoundry', + 'k8s': 'kubernetes' }, PLATFORM_MANAGER: { 'cloudfoundry': 'CfPlatformManager', - 'cf': 'CfPlatformManager', - 'kubernetes': 'K8sPlatformManager', - 'k8s': 'K8sPlatformManager' + 'kubernetes': 'K8sPlatformManager' }, AGENT: { diff --git a/lib/controllers/ServiceBrokerApiController.js b/lib/controllers/ServiceBrokerApiController.js index 24e4d5847..974902032 100644 --- a/lib/controllers/ServiceBrokerApiController.js +++ b/lib/controllers/ServiceBrokerApiController.js @@ -39,7 +39,7 @@ class ServiceBrokerApiController extends BaseController { getCatalog(req, res) { /* jshint unused:false */ - res.status(200).json(this.fabrik.getPlatformManager(req.params.platform).getPlatformSpecificCatalog(catalog)); + res.status(200).json(this.fabrik.getPlatformManager(req.params.platform).getCatalog(catalog)); } putInstance(req, res) { diff --git a/lib/fabrik/BasePlatformManager.js b/lib/fabrik/BasePlatformManager.js index 2574233de..13f3b40f1 100644 --- a/lib/fabrik/BasePlatformManager.js +++ b/lib/fabrik/BasePlatformManager.js @@ -5,7 +5,7 @@ class BasePlatformManager { this.platform = platform; } - getPlatformSpecificCatalog(catalog) { + getCatalog(catalog) { return catalog; } diff --git a/lib/fabrik/Fabrik.js b/lib/fabrik/Fabrik.js index a0c872815..d0a5ca6a0 100644 --- a/lib/fabrik/Fabrik.js +++ b/lib/fabrik/Fabrik.js @@ -48,7 +48,7 @@ class Fabrik { } static getPlatformManager(platform) { - const PlatformManager = (platform && CONST.PLATFORM_MANAGER[platform]) ? require(`./${CONST.PLATFORM_MANAGER[platform]}`) : undefined; + const PlatformManager = (platform && CONST.PLATFORM_MANAGER[platform]) ? require(`./${CONST.PLATFORM_MANAGER[platform]}`) : ((platform && CONST.PLATFORM_MANAGER[CONST.PLATFORM_ALIAS_MAPPINGS[platform]]) ? require(`./${CONST.PLATFORM_MANAGER[CONST.PLATFORM_ALIAS_MAPPINGS[platform]]}`) : undefined); if (PlatformManager === undefined) { return new BasePlatformManager(platform);   } else {  diff --git a/lib/fabrik/K8sPlatformManager.js b/lib/fabrik/K8sPlatformManager.js index 5ca019601..f38af886d 100644 --- a/lib/fabrik/K8sPlatformManager.js +++ b/lib/fabrik/K8sPlatformManager.js @@ -9,7 +9,7 @@ class K8sPlatformManager extends BasePlatformManager { this.platform = platform; } - getPlatformSpecificCatalog(catalog) { + getCatalog(catalog) { const modifiedCatalog = _.cloneDeep(catalog); const platform = this.platform; _.remove(modifiedCatalog.services, function (service) {