Skip to content

Commit

Permalink
fix(provider/oracle): fix update/edit LoadBalancer (#6043)
Browse files Browse the repository at this point in the history
  • Loading branch information
shihchang authored and Justin Reynolds committed Nov 15, 2018
1 parent c1bfbcd commit d1acd1b
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 24 deletions.
6 changes: 5 additions & 1 deletion app/scripts/modules/oracle/src/domain/IOracleLoadBalancer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface IOracleLoadBalancer extends ILoadBalancer {
shape: string; // required
isPrivate: boolean; // required
subnets: IOracleSubnet[]; // required 1 for private LB, 2 for public LB
certificates: IOracleListenerCertificate[];
certificates?: { [name: string]: IOracleListenerCertificate };
listeners?: { [name: string]: IOracleListener }; // not required to create LB, but useless without it
hostnames?: IOracleHostname[];
backendSets?: { [name: string]: IOracleBackEndSet }; // not required to create LB, but useless without it
Expand Down Expand Up @@ -63,6 +63,8 @@ export interface IOracleBackEndSet {
policy: LoadBalancingPolicy;
healthChecker: IOracleBackendSetHealthCheck;
// TODO desagar sessionPersistenceConfiguration?: IOracleLoadBalancerSessionPersistenceConfiguration;
backends: any[];
isNew: boolean;
}

export interface IOracleListenerCertificate {
Expand All @@ -71,6 +73,7 @@ export interface IOracleListenerCertificate {
caCertificate: string;
privateKey: string;
passphrase: string;
isNew: boolean;
}

export interface IOracleBackendSetHealthCheck {
Expand All @@ -92,6 +95,7 @@ export interface IOracleLoadBalancerUpsertCommand extends ILoadBalancerUpsertCom
shape: string; // required
isPrivate: boolean; // required
subnetIds: string[]; // required 1 for private LB, 2 for public LB
certificates?: { [name: string]: IOracleListenerCertificate };
listeners?: { [name: string]: IOracleListener }; // not required to create LB, but useless without it
hostnames?: IOracleHostname[];
backendSets?: { [name: string]: IOracleBackEndSet }; // not required to create LB, but useless without it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
</thead>
<tbody>
<tr ng-repeat="backendSet in ctrl.backendSets">
<td>
<td ng-if="backendSet.isNew">
<input class="form-control input-sm" ng-model="backendSet.name" required
ng-focus="prevBackendSetNames[$index] = backendSet.name"
ng-blur="ctrl.backendSetNameChanged($index)"/>
</td>
<td ng-if="!backendSet.isNew">{{backendSet.name}}
</td>
<td>
<select class="form-control input-sm" ng-model="backendSet.policy" ng-options="policy for policy in ctrl.loadBalancingPolicies"></select>
</td>
Expand All @@ -31,8 +33,10 @@
<td>
<input class="form-control input-sm" ng-model="backendSet.healthChecker.urlPath" required/>
</td>
<td><a href class="sm-label" ng-click="ctrl.removeBackendSet($index)"><span
class="glyphicon glyphicon-trash"></span></a></td>
<td>
<a href class="sm-label" ng-if="ctrl.isBackendSetRemovable($index)"
ng-click="ctrl.removeBackendSet($index)"><span class="glyphicon glyphicon-trash"></span></a>
</td>
</tr>
</tbody>
<tfoot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,33 @@
</thead>
<tbody>
<tr ng-repeat="cert in ctrl.certificates">
<td>
<td ng-if="!cert.isNew">{{cert.certificateName}}</td>
<td ng-if="cert.isNew">
<input class="form-control input-sm" ng-model="cert.certificateName" required
ng-focus="prevCertNames[$index] = cert.name"
ng-blur="ctrl.certNameChanged($index)"/>
</td>
<td>
<textarea ng-model="cert.publicCertificate" required
<textarea ng-if="cert.isNew" ng-model="cert.publicCertificate" required
class="form-control input-sm"></textarea>
</td>
<td>
<textarea ng-model="cert.caCertificate"
<textarea ng-if="cert.isNew" ng-model="cert.caCertificate"
class="form-control input-sm"></textarea>
</td>
<td>
<textarea ng-model="cert.privateKey" required
<textarea ng-if="cert.isNew" ng-model="cert.privateKey" required
class="form-control input-sm"></textarea>
</td>
<td>
<textarea ng-model="cert.passphrase"
<textarea ng-if="cert.isNew" ng-model="cert.passphrase"
class="form-control input-sm"></textarea>
</td>
<td><a href class="sm-label" ng-click="ctrl.removeCert($index)"><span
class="glyphicon glyphicon-trash"></span></a></td>
<td>
<a href class="sm-label" ng-if="ctrl.isCertRemovable($index)"
ng-click="ctrl.removeCert($index)"><span class="glyphicon glyphicon-trash"></span>
</a>
</td>
</tr>
</tbody>
<tfoot>
Expand All @@ -48,7 +52,6 @@
</tr>
</tfoot>
</table>

</div>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('Controller: oracleCreateLoadBalancerCtrl', function() {
expect(controller.backendSets[0].policy).toEqual(LoadBalancingPolicy.ROUND_ROBIN);
expect(controller.backendSets[0].healthChecker.protocol).toEqual('HTTP');
expect(controller.backendSets[0].healthChecker.port).toEqual(80);
expect(controller.backendSets[0].healthChecker.urlPath).toEqual('/healthZ');
expect(controller.backendSets[0].healthChecker.urlPath).toEqual('/');
});

it('correctly creates default certificate', function() {
Expand All @@ -103,13 +103,55 @@ describe('Controller: oracleCreateLoadBalancerCtrl', function() {
expect(controller.certificates[0].certificateName).toEqual('certificate1');
});

it('adds & removes certificate', function() {
controller.addCert();
expect(controller.certificates).toBeDefined();
expect(controller.certificates.length).toEqual(2);
expect(controller.certificates[1].certificateName).toEqual('certificate2');
controller.removeCert(0);
expect(controller.certificates.length).toEqual(1);
expect(controller.certificates[0].certificateName).toEqual('certificate2');
});

it('cannot remove certificate if used by listener', function() {
const newCertName = 'myCert';
controller.addCert();
expect(controller.certificates.length).toEqual(2);
controller.certificates[1].certificateName = newCertName;
controller.certNameChanged(1);
controller.addListener();
controller.listeners[1].isSsl = true;
controller.listeners[1].sslConfiguration = {
certificateName: newCertName,
verifyDepth: 0,
verifyPeerCertificates: false,
};
expect(controller.isCertRemovable(1)).toEqual(false);
controller.removeListener(1);
expect(controller.isCertRemovable(1)).toEqual(true);
controller.removeCert(1);
});

it('changed backend set name updates listener', function() {
expect(controller.listeners[0].defaultBackendSetName).toEqual('backendSet1');
controller.backendSets[0].name = 'UpdatedBackendSetName';
controller.backendSetNameChanged(0);
expect(controller.listeners[0].defaultBackendSetName).toEqual('UpdatedBackendSetName');
});

it('cannot remove backendset if used by listener', function() {
const newBackendSetName = 'myBackendSet';
controller.addBackendSet();
controller.addListener();
controller.backendSets[1].name = newBackendSetName;
controller.backendSetNameChanged(1);
controller.listeners[1].defaultBackendSetName = newBackendSetName;
expect(controller.isBackendSetRemovable(1)).toEqual(false);
controller.removeListener(1);
expect(controller.isBackendSetRemovable(1)).toEqual(true);
controller.removeBackendSet(1);
});

it('remove backend set updates listener', function() {
controller.removeBackendSet(0);
expect(controller.listeners[0].defaultBackendSetName).not.toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ export class OracleLoadBalancerController implements IController {
this.backendSets.push(this.$scope.loadBalancerCmd.backendSets[b]);
});
}
if (this.$scope.loadBalancerCmd.certificates) {
Object.keys(this.$scope.loadBalancerCmd.certificates).forEach(b => {
this.certificates.push(this.$scope.loadBalancerCmd.certificates[b]);
});
}
}

public initializeController() {
Expand Down Expand Up @@ -334,6 +339,20 @@ export class OracleLoadBalancerController implements IController {
});
}

public isBackendSetRemovable(idx: number): boolean {
const backendSet = this.backendSets[idx];
if (backendSet && backendSet.backends && backendSet.backends.length > 0) {
return false;
}
let hasListener = false;
this.listeners.forEach(lis => {
if (lis.defaultBackendSetName === backendSet.name) {
hasListener = true;
}
});
return !hasListener;
}

public addBackendSet() {
const nameSuffix: number = this.backendSets.length + 1;
const name: string = 'backendSet' + nameSuffix;
Expand All @@ -350,13 +369,24 @@ export class OracleLoadBalancerController implements IController {
}
}

public isCertRemovable(idx: number): boolean {
const cert = this.certificates[idx];
let hasListener = false;
this.listeners.forEach(lis => {
if (lis.isSsl && lis.sslConfiguration && lis.sslConfiguration.certificateName === cert.certificateName) {
hasListener = true;
}
});
return !hasListener;
}

public removeCert(idx: number) {
const cert = this.certificates[idx];
this.certificates.splice(idx, 1);
this.$scope.prevCertNames.splice(idx, 1);
// Also clear the certificateName field of any listeners who are using this certificate
this.listeners.forEach(lis => {
if (lis.sslConfiguration.certificateName === cert.certificateName) {
if (lis.sslConfiguration && lis.sslConfiguration.certificateName === cert.certificateName) {
lis.sslConfiguration.certificateName = undefined;
}
});
Expand All @@ -372,9 +402,11 @@ export class OracleLoadBalancerController implements IController {
public certNameChanged(idx: number) {
const prevName = this.$scope.prevCertNames && this.$scope.prevCertNames[idx];
if (prevName && prevName !== this.certificates[idx].certificateName) {
this.listeners.filter(lis => lis.sslConfiguration.certificateName === prevName).forEach(lis => {
lis.sslConfiguration.certificateName = this.certificates[idx].certificateName;
});
this.listeners
.filter(lis => lis.sslConfiguration && lis.sslConfiguration.certificateName === prevName)
.forEach(lis => {
lis.sslConfiguration.certificateName = this.certificates[idx].certificateName;
});
}
}

Expand Down Expand Up @@ -404,7 +436,7 @@ export class OracleLoadBalancerController implements IController {
});
}

if (this.backendSets.length > 0) {
if (this.backendSets) {
this.$scope.loadBalancerCmd.backendSets = this.backendSets.reduce(
(backendSetsMap: { [name: string]: IOracleBackEndSet }, backendSet: IOracleBackEndSet) => {
backendSetsMap[backendSet.name] = backendSet;
Expand All @@ -414,7 +446,7 @@ export class OracleLoadBalancerController implements IController {
);
}

if (this.listeners.length > 0) {
if (this.listeners) {
this.$scope.loadBalancerCmd.listeners = this.listeners.reduce(
(listenersMap: { [name: string]: IOracleListener }, listener: IOracleListener) => {
listener.name = listener.protocol + '_' + listener.port;
Expand All @@ -425,10 +457,14 @@ export class OracleLoadBalancerController implements IController {
);
}

if (this.certificates.length > 0) {
if (this.certificates) {
this.$scope.loadBalancerCmd.certificates = this.certificates.reduce(
(certMap: { [name: string]: IOracleListenerCertificate }, cert: IOracleListenerCertificate) => {
certMap[cert.certificateName] = cert;
if (!cert.isNew) {
// existing certificate sends only the name
certMap[cert.certificateName].publicCertificate = null;
}
return certMap;
},
{},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
<form name="form" novalidate validate-on-submit>
<v2-modal-wizard heading="Edit {{loadBalancer.name}}: {{loadBalancer.region}}: {{loadBalancer.credentials}}" task-monitor="taskMonitor">
<form name="form" class="form-horizontal" novalidate validate-on-submit>
<v2-modal-wizard heading="Edit Load Balancer {{ctrl.loadBalancer.name}}" task-monitor="taskMonitor" dismiss="$dismiss()">
<v2-wizard-page key="Certificates" label="SSL Certificates" done="true">
<ng-include src="ctrl.pages.certificates"></ng-include>
</v2-wizard-page>
<v2-wizard-page key="Backend Sets" label="Backend Sets" done="true">
<ng-include src="ctrl.pages.backendSets"></ng-include>
</v2-wizard-page>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ export class OracleLoadBalancerDetailController implements IController {
public editLoadBalancer() {
this.$uibModal.open({
templateUrl: require('oracle/loadBalancer/configure/editLoadBalancer.html'),
// controller: 'oracleCreateLoadBalancerCtrl as ctrl',
controller: OracleLoadBalancerController,
controllerAs: 'ctrl',
size: 'lg',
resolve: {
application: () => {
return this.app;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export class OracleLoadBalancerTransformer {
shape: loadBalancer.shape,
isPrivate: loadBalancer.isPrivate,
subnetIds: loadBalancer.subnets.map(subnet => subnet.id),
certificates: loadBalancer.certificates,
listeners: loadBalancer.listeners,
hostnames: loadBalancer.hostnames,
backendSets: loadBalancer.backendSets,
Expand Down Expand Up @@ -104,7 +105,9 @@ export class OracleLoadBalancerTransformer {
return {
name: name,
policy: LoadBalancingPolicy.ROUND_ROBIN,
healthChecker: { protocol: 'HTTP', port: 80, urlPath: '/healthZ' },
healthChecker: { protocol: 'HTTP', port: 80, urlPath: '/' },
backends: [],
isNew: true,
};
}

Expand All @@ -123,6 +126,7 @@ export class OracleLoadBalancerTransformer {
caCertificate: undefined,
privateKey: undefined,
passphrase: undefined,
isNew: true,
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,11 @@ module.exports = angular
backingData.filtered.shapes = shapesMap;
backingData.filtered.allShapes = _.uniqBy(_.flatten(_.values(shapesMap)), 'name');
command.backingData = backingData;
if (command.account) {
loadLoadBalancers(command);
backingData.loadBalancerOnChange();
backingData.backendSetOnChange();
}
});
}

Expand Down

0 comments on commit d1acd1b

Please sign in to comment.