Skip to content

Commit

Permalink
fix(cf): Map/Unmap LBs and UI cleanup (spinnaker#6737)
Browse files Browse the repository at this point in the history
* fix(cf): Map/Unmap LBs and UI cleanup

- added 'preventSave' to all required fields in Map / Unmap Load
Balancers pipeline stages
- corrected pluralization
- repaired minor UI glitches in map / unmap
- optionality of route fields in mapping / unmapping screens removed
- removed some Angular wrapping on map / unmap pipeline stages
- repaired minor bugs in AccountRegionClusterSelector in which: a) the
region was not being cleared when a new account is selected; and
b) the cluster list was not being properly initialized for a new stage
when the AccountRegionClusterSelector was specified as single-region
- repaired Rollback Cluster Stage, which was not displaying any of its
CF-specific fields

spinnaker/spinnaker#4146
spinnaker/spinnaker#4147
spinnaker/spinnaker#4169
spinnaker/spinnaker#4171
spinnaker/spinnaker#4172

Co-Authored-By: Stu Pollock <spollock@pivotal.io>

* so isRequired doesn't accidentally create an uncontrolled component.
  • Loading branch information
jasonjchu authored and jkschneider committed Mar 22, 2019
1 parent 1c9e075 commit c37500c
Show file tree
Hide file tree
Showing 21 changed files with 207 additions and 185 deletions.
9 changes: 4 additions & 5 deletions app/scripts/modules/cloudfoundry/src/cf.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,17 @@ import { cfServerGroupDetailsGetter } from './serverGroup/details/cfServerGroupD

import './logo/cf.logo.less';
import { CloudFoundryNoLoadBalancerModal } from './loadBalancer/configure/cloudFoundryNoLoadBalancerModal';
import 'cloudfoundry/pipeline/config/validation/instanceSize.validator';
import 'cloudfoundry/pipeline/config/validation/cfTargetImpedance.validator';
import 'cloudfoundry/pipeline/config/validation/instanceSize.validator';
import 'cloudfoundry/pipeline/config/validation/requiredRoutes.validator';
import { CLOUD_FOUNDRY_CLONE_SERVER_GROUP_STAGE } from './pipeline/stages/cloneServerGroup/cloudfoundryCloneServerGroupStage.module';
import './pipeline/stages/deployService/cloudfoundryDeployServiceStage.module';
import { CLOUD_FOUNDRY_DESTROY_ASG_STAGE } from './pipeline/stages/destroyAsg/cloudfoundryDestroyAsgStage.module';
import './pipeline/stages/destroyService/cloudfoundryDestroyServiceStage.module';
import { CLOUD_FOUNDRY_DISABLE_ASG_STAGE } from './pipeline/stages/disableAsg/cloudfoundryDisableAsgStage.module';
import { CLOUD_FOUNDRY_ENABLE_ASG_STAGE } from './pipeline/stages/enableAsg/cloudfoundryEnableAsgStage.module';
import { CLOUD_FOUNDRY_MAP_LOAD_BALANCERS_STAGE } from './pipeline/stages/mapLoadBalancers/cloudfoundryMapLoadBalancersStage.module';
import { CLOUD_FOUNDRY_UNMAP_LOAD_BALANCERS_STAGE } from './pipeline/stages/unmapLoadBalancers/cloudfoundryUnmapLoadBalancersStage.module';
import './pipeline/stages/mapLoadBalancers/cloudfoundryMapLoadBalancersStage.module';
import './pipeline/stages/unmapLoadBalancers/cloudfoundryUnmapLoadBalancersStage.module';
import { CLOUD_FOUNDRY_RESIZE_ASG_STAGE } from './pipeline/stages/resizeAsg/cloudfoundryResizeAsgStage.module';
import { CLOUD_FOUNDRY_ROLLBACK_CLUSTER_STAGE } from './pipeline/stages/rollbackCluster/cloudfoundryRollbackClusterStage.module';
import './pipeline/stages/shareService/cloudfoundryShareServiceStage.module';
Expand All @@ -55,8 +56,6 @@ module(CLOUD_FOUNDRY_MODULE, [
CLOUD_FOUNDRY_ENABLE_ASG_STAGE,
CLOUD_FOUNDRY_INSTANCE_DETAILS,
CLOUD_FOUNDRY_LOAD_BALANCER_MODULE,
CLOUD_FOUNDRY_MAP_LOAD_BALANCERS_STAGE,
CLOUD_FOUNDRY_UNMAP_LOAD_BALANCERS_STAGE,
CLOUD_FOUNDRY_REACT_MODULE,
CLOUD_FOUNDRY_RESIZE_ASG_STAGE,
CLOUD_FOUNDRY_ROLLBACK_CLUSTER_STAGE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ const helpContents: { [key: string]: string } = {
'(Optional) <b>Detail</b> is a string of free-form alphanumeric characters and hyphens to describe any other variables.',
'cf.serverGroup.startApplication':
'<b>Start on creation</b> is a boolean value that determines if the server group is started upon creation. Default value is <code>true</code>.',
'cf.serverGroup.requiredRoutes':
'<b>Route</b> is a URI in the form of <code>some.host.some.domain[:9999][/some/path]</code> (port and path are optional). The domain has to be a valid domain in the CloudFoundry Org (Region) that this server group runs in.',
'cf.serverGroup.routes':
'(Optional) <b>Route</b> is a URI in the form of <code>some.host.some.domain[:9999][/some/path]</code> (port and path are optional). The domain has to be a valid domain in the CloudFoundry Org (Region) that this server group runs in.',
'cf.artifact.package': `<p>This option allows you to create a new server group from an existing Droplet. You can use this to relocate an existing Cloud Foundry application from one space to another or to launch a clone of an application with different root filesystem or resource settings.</p>`,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { get } from 'lodash';

import { IPipeline, IStage, IStageOrTriggerValidator, ITrigger, PipelineConfigValidator } from '@spinnaker/core';

export class CfRequiredRoutesFieldValidator implements IStageOrTriggerValidator {
public validate(_pipeline: IPipeline, stage: IStage | ITrigger, validationConfig: any): string {
const routes: string[] = get(stage, validationConfig.fieldName);
const routeErrors = routes
.map((route: string) => {
const regex = /^([-\w]+)\.([-.\w]+)(:\d+)?([-/\w]+)?$/gm;
route = route || '';
if (regex.exec(route) === null) {
return `"${route}" did not match the expected format "host.some.domain[:9999][/some/path]"`;
}
return null;
})
.filter(err => err != null);
return (routeErrors && routeErrors.length && routeErrors[0]) || null;
}
}

PipelineConfigValidator.registerValidator('cfRequiredRoutesField', new CfRequiredRoutesFieldValidator());
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class CloudfoundryDestroyServiceStageConfig extends React.Component<
> {
constructor(props: IStageConfigProps) {
super(props);
props.stage.cloudProvider = 'cloudfoundry';
this.props.updateStageField({ cloudProvider: 'cloudfoundry' });
this.state = {
accounts: [],
regions: [],
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ import { CloudfoundryServiceExecutionDetails } from 'cloudfoundry/presentation';
Registry.pipeline.registerStage({
accountExtractor: (stage: IStage) => stage.context.credentials,
configAccountExtractor: (stage: IStage) => [stage.credentials],
provides: 'destroyService',
key: 'destroyService',
cloudProvider: 'cloudfoundry',
component: CloudfoundryDestroyServiceStageConfig,
templateUrl: require('./cloudfoundryDestroyServiceStage.html'),
controller: 'cfDestroyServiceStageCtrl',
executionDetailsSections: [CloudfoundryServiceExecutionDetails, ExecutionDetailsTasks],
defaultTimeoutMs: 30 * 60 * 1000,
executionDetailsSections: [CloudfoundryServiceExecutionDetails, ExecutionDetailsTasks],
key: 'destroyService',
provides: 'destroyService',
validators: [
{ type: 'requiredField', fieldName: 'region' },
{ type: 'requiredField', fieldName: 'serviceInstanceName', preventSave: true },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';

import {
AccountService,
Application,
IAccount,
IPipeline,
Expand All @@ -14,7 +15,6 @@ import { AccountRegionClusterSelector, Routes } from 'cloudfoundry/presentation'
import { Formik } from 'formik';

interface ICloudfoundryLoadBalancerStageConfigProps extends IStageConfigProps {
accounts: IAccount[];
pipeline: IPipeline;
}

Expand All @@ -23,13 +23,10 @@ interface ICloudFoundryMapLoadBalancersValues {
}

interface ICloudfoundryMapLoadBalancersStageConfigState {
accounts: IAccount[];
application: Application;
cloudProvider: string;
credentials: string;
initialValues: ICloudFoundryMapLoadBalancersValues;
pipeline: IPipeline;
region: string;
target: string;
}

export class CloudfoundryMapLoadBalancersStageConfig extends React.Component<
Expand All @@ -40,37 +37,45 @@ export class CloudfoundryMapLoadBalancersStageConfig extends React.Component<

constructor(props: ICloudfoundryLoadBalancerStageConfigProps) {
super(props);
props.stage.cloudProvider = 'cloudfoundry';
const { loadBalancerNames } = props.stage;
const routes = loadBalancerNames && loadBalancerNames.length ? loadBalancerNames : [''];
this.props.updateStageField({
cloudProvider: 'cloudfoundry',
loadBalancerNames: routes,
});
this.state = {
accounts: [],
application: props.application,
cloudProvider: 'cloudfoundry',
credentials: props.stage.credentials,
initialValues: {
routes: props.stage.loadBalancerNames,
routes,
},
pipeline: props.pipeline,
region: props.stage.region,
target: props.stage.target,
};
}

public componentDidMount = () => {
AccountService.listAccounts('cloudfoundry').then(accounts => {
this.setState({ accounts });
});
};

private targetUpdated = (target: string) => {
this.setState({ target });
this.props.stage.target = target;
this.props.stageFieldUpdated();
this.props.updateStageField({ target });
};

private componentUpdated = (stage: any): void => {
this.props.stage.credentials = stage.credentials;
this.props.stage.region = stage.region;
this.props.stage.cluster = stage.cluster;
this.props.stage.loadBalancerNames = stage.loadBalancerNames;
this.props.stageFieldUpdated();
this.props.updateStageField({
credentials: stage.credentials,
region: stage.region,
cluster: stage.cluster,
loadBalancerNames: stage.loadBalancerNames,
});
};

public render() {
const { accounts, stage } = this.props;
const { application, initialValues, pipeline, target } = this.state;
const { stage } = this.props;
const { accounts, application, initialValues, pipeline } = this.state;
const { target } = stage;
const { TargetSelect } = NgReact;
return (
<div className="form-horizontal">
Expand All @@ -95,6 +100,8 @@ export class CloudfoundryMapLoadBalancersStageConfig extends React.Component<
return (
<Routes
fieldName={'routes'}
isRequired={true}
singleRouteOnly={true}
onChange={(routes: string[]) => {
stage.loadBalancerNames = routes;
this.componentUpdated(stage);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,49 +1,22 @@
import { IController, IScope, module } from 'angular';
import { react2angular } from 'react2angular';

import { CloudfoundryMapLoadBalancersStageConfig } from './CloudfoundryMapLoadBalancersStageConfig';
import { AccountService, ExecutionDetailsTasks, IStage, Registry } from '@spinnaker/core';
import { ExecutionDetailsTasks, IStage, Registry } from '@spinnaker/core';
import { CloudfoundryMapLoadBalancersExecutionDetails } from './CloudfoundryMapLoadBalancersExecutionDetails';

class CloudFoundryMapLoadBalancersStageCtrl implements IController {
public static $inject = ['$scope'];
constructor(public $scope: IScope) {
$scope.accounts = [];
AccountService.listAccounts('cloudfoundry').then(accounts => {
$scope.accounts = accounts;
});
}
}

export const CLOUD_FOUNDRY_MAP_LOAD_BALANCERS_STAGE = 'spinnaker.cloudfoundry.pipeline.stage.mapLoadBalancersStage';
module(CLOUD_FOUNDRY_MAP_LOAD_BALANCERS_STAGE, [])
.config(function() {
Registry.pipeline.registerStage({
accountExtractor: (stage: IStage) => stage.context.credentials,
configAccountExtractor: (stage: IStage) => [stage.credentials],
cloudProvider: 'cloudfoundry',
controller: 'cfMapLoadBalancersStageCtrl',
description: 'Map load balancers',
executionDetailsSections: [CloudfoundryMapLoadBalancersExecutionDetails, ExecutionDetailsTasks],
key: 'mapLoadBalancers',
label: 'Map Load Balancers',
templateUrl: require('./cloudfoundryMapLoadBalancersStage.html'),
validators: [
{ type: 'requiredField', fieldName: 'cluster' },
{ type: 'requiredField', fieldName: 'credentials', fieldLabel: 'account' },
{ type: 'requiredField', fieldName: 'region' },
{ type: 'requiredField', fieldName: 'target' },
],
});
})
.component(
'cfMapLoadBalancersStage',
react2angular(CloudfoundryMapLoadBalancersStageConfig, [
'accounts',
'application',
'pipeline',
'stage',
'stageFieldUpdated',
]),
)
.controller('cfMapLoadBalancersStageCtrl', CloudFoundryMapLoadBalancersStageCtrl);
Registry.pipeline.registerStage({
accountExtractor: (stage: IStage) => stage.context.credentials,
configAccountExtractor: (stage: IStage) => [stage.credentials],
cloudProvider: 'cloudfoundry',
component: CloudfoundryMapLoadBalancersStageConfig,
controller: 'BaseProviderStageCtrl as baseProviderStageCtrl',
description: 'Map a load balancer',
executionDetailsSections: [CloudfoundryMapLoadBalancersExecutionDetails, ExecutionDetailsTasks],
key: 'mapLoadBalancers',
label: 'Map Load Balancer',
validators: [
{ type: 'requiredField', preventSave: true, fieldName: 'cluster' },
{ type: 'requiredField', preventSave: true, fieldName: 'credentials', fieldLabel: 'account' },
{ type: 'requiredField', preventSave: true, fieldName: 'region' },
{ type: 'requiredField', preventSave: true, fieldName: 'target' },
{ type: 'cfRequiredRoutesField', preventSave: true, fieldName: 'loadBalancerNames' },
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export class CloudfoundryRollbackClusterStageConfig extends React.Component<
targetHealthyRollbackPercentage: 100,
});

this.props.stage.regions = this.props.stage.regions || [];

this.state = {
accounts: [],
application: props.application,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';

import {
AccountService,
Application,
IAccount,
IPipeline,
Expand All @@ -14,7 +15,6 @@ import { AccountRegionClusterSelector, Routes } from 'cloudfoundry/presentation'
import { Formik } from 'formik';

interface ICloudfoundryLoadBalancerStageConfigProps extends IStageConfigProps {
accounts: IAccount[];
pipeline: IPipeline;
}

Expand All @@ -23,13 +23,10 @@ interface ICloudFoundryUnmapLoadBalancersValues {
}

interface ICloudfoundryUnmapLoadBalancersStageConfigState {
accounts: IAccount[];
application: Application;
cloudProvider: string;
credentials: string;
initialValues: ICloudFoundryUnmapLoadBalancersValues;
pipeline: IPipeline;
region: string;
target: string;
}

export class CloudfoundryUnmapLoadBalancersStageConfig extends React.Component<
Expand All @@ -40,37 +37,45 @@ export class CloudfoundryUnmapLoadBalancersStageConfig extends React.Component<

constructor(props: ICloudfoundryLoadBalancerStageConfigProps) {
super(props);
props.stage.cloudProvider = 'cloudfoundry';
const { loadBalancerNames } = props.stage;
const routes = loadBalancerNames && loadBalancerNames.length ? loadBalancerNames : [''];
this.props.updateStageField({
cloudProvider: 'cloudfoundry',
loadBalancerNames: routes,
});
this.state = {
accounts: [],
application: props.application,
cloudProvider: 'cloudfoundry',
credentials: props.stage.credentials,
initialValues: {
routes: props.stage.loadBalancerNames,
routes,
},
pipeline: props.pipeline,
region: props.stage.region,
target: props.stage.target,
};
}

public componentDidMount = () => {
AccountService.listAccounts('cloudfoundry').then(accounts => {
this.setState({ accounts });
});
};

private targetUpdated = (target: string) => {
this.setState({ target });
this.props.stage.target = target;
this.props.stageFieldUpdated();
this.props.updateStageField({ target });
};

private componentUpdated = (stage: any): void => {
this.props.stage.credentials = stage.credentials;
this.props.stage.region = stage.region;
this.props.stage.cluster = stage.cluster;
this.props.stage.loadBalancerNames = stage.loadBalancerNames;
this.props.stageFieldUpdated();
this.props.updateStageField({
credentials: stage.credentials,
region: stage.region,
cluster: stage.cluster,
loadBalancerNames: stage.loadBalancerNames,
});
};

public render() {
const { accounts, stage } = this.props;
const { application, initialValues, pipeline, target } = this.state;
const { stage } = this.props;
const { accounts, application, initialValues, pipeline } = this.state;
const { target } = stage;
const { TargetSelect } = NgReact;
return (
<div className="form-horizontal">
Expand All @@ -95,6 +100,8 @@ export class CloudfoundryUnmapLoadBalancersStageConfig extends React.Component<
return (
<Routes
fieldName={'routes'}
isRequired={true}
singleRouteOnly={true}
onChange={(routes: string[]) => {
stage.loadBalancerNames = routes;
this.componentUpdated(stage);
Expand Down

This file was deleted.

Loading

0 comments on commit c37500c

Please sign in to comment.