Skip to content

Commit

Permalink
feat(core/amazon/titus): do not allow create/clone in managed clusters (
Browse files Browse the repository at this point in the history
#7754)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
anotherchrisberry and mergify[bot] committed Jan 10, 2020
1 parent 19ff2aa commit 4302a0f
Show file tree
Hide file tree
Showing 8 changed files with 189 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -404,4 +404,79 @@ describe('Service: awsServerGroupConfiguration', function() {
expect(result.dirty.amiName).toBe(true);
});
});

describe('managedResources', () => {
beforeEach(() => {
this.command = {
viewState: {},
backingData: {
filtered: {},
credentialsKeyedByAccount: {
prod: {
regions: [
{ name: 'us-east-1', availabilityZones: [] },
{ name: 'us-west-1', availabilityZones: [] },
],
},
test: {
regions: [
{ name: 'us-east-1', availabilityZones: [] },
{ name: 'us-west-1', availabilityZones: [] },
],
},
},
managedResources: [
{
kind: 'cluster',
locations: {
account: 'prod',
regions: [{ name: 'us-east-1' }],
},
moniker: {
stack: 'foo',
detail: 'bar',
},
},
],
securityGroups: {},
preferredZones: {},
},
credentials: 'test',
region: 'us-west-1',
stack: '',
freeFormDetails: '',
};
service.attachEventHandlers(this.command);
});

it('does not add managedResource on initial load if not matched', () => {
expect(this.command.viewState.resourceSummary).toBeFalsy();
});

it('adds managedResource when all fields match a non-paused resource', () => {
const { command } = this;
const managedResource = command.backingData.managedResources[0];
expect(command.resourceSummary).toBeFalsy();

command.credentials = managedResource.locations.account;
command.credentialsChanged(command);
expect(command.resourceSummary).toBeFalsy();

command.region = managedResource.locations.regions[0].name;
command.regionChanged(command);
expect(command.resourceSummary).toBeFalsy();

command.stack = managedResource.moniker.stack;
command.clusterChanged(command);
expect(command.resourceSummary).toBeFalsy();

command.freeFormDetails = managedResource.moniker.detail;
command.clusterChanged(command);
expect(command.resourceSummary).toBe(managedResource);

managedResource.isPaused = true;
command.clusterChanged(command);
expect(command.resourceSummary).toBeFalsy();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
SECURITY_GROUP_READER,
SecurityGroupReader,
SERVER_GROUP_COMMAND_REGISTRY_PROVIDER,
setMatchingResourceSummary,
ServerGroupCommandRegistry,
SubnetReader,
IServerGroupCommandViewState,
Expand Down Expand Up @@ -99,7 +100,6 @@ export interface IAmazonServerGroupCommand extends IServerGroupCommand {
getBlockDeviceMappingsSource: (command: IServerGroupCommand) => IBlockDeviceMappingSource;
selectBlockDeviceMappingsSource: (command: IServerGroupCommand, selection: string) => void;
usePreferredZonesChanged: (command: IServerGroupCommand) => IAmazonServerGroupCommandResult;
clusterChanged: (command: IServerGroupCommand) => void;
regionIsDeprecated: (command: IServerGroupCommand) => boolean;
}

Expand Down Expand Up @@ -219,6 +219,7 @@ export class AwsServerGroupConfigurationService {
backingData.filtered = {} as IAmazonServerGroupCommandBackingDataFiltered;
backingData.scalingProcesses = AutoScalingProcessService.listProcesses();
backingData.appLoadBalancers = application.getDataSource('loadBalancers').data;
backingData.managedResources = application.getDataSource('managedResources')?.data?.resources;
cmd.backingData = backingData as IAmazonServerGroupCommandBackingData;
this.configureVpcId(cmd);
backingData.filtered.securityGroups = this.getRegionalSecurityGroups(cmd);
Expand Down Expand Up @@ -577,12 +578,13 @@ export class AwsServerGroupConfigurationService {
} else {
filteredData.regionalAvailabilityZones = null;
}

setMatchingResourceSummary(command);
return result;
};

cmd.clusterChanged = (command: IAmazonServerGroupCommand): void => {
command.moniker = NameUtils.getMoniker(command.application, command.stack, command.freeFormDetails);
setMatchingResourceSummary(command);
};

cmd.credentialsChanged = (command: IAmazonServerGroupCommand): IServerGroupCommandResult => {
Expand All @@ -602,6 +604,7 @@ export class AwsServerGroupConfigurationService {
} else {
command.region = null;
}
setMatchingResourceSummary(command);
return result;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { Field, FormikProps } from 'formik';
import { Field, FormikErrors, FormikProps } from 'formik';

import {
AccountSelectInput,
Expand All @@ -12,6 +12,7 @@ import {
IServerGroup,
IWizardPageComponent,
Markdown,
DeployingIntoManagedClusterWarning,
TaskReason,
} from '@spinnaker/core';

Expand Down Expand Up @@ -111,8 +112,8 @@ export class ServerGroupBasicSettings
setFieldValue('subnetType', values.subnetType);
};

public validate(values: IAmazonServerGroupCommand): { [key: string]: string } {
const errors: { [key: string]: string } = {};
public validate(values: IAmazonServerGroupCommand): FormikErrors<IAmazonServerGroupCommand> {
const errors: FormikErrors<IAmazonServerGroupCommand> = {};

if (!isStackPattern(values.stack)) {
errors.stack = 'Only dot(.) and underscore(_) special characters are allowed in the Stack field.';
Expand All @@ -127,6 +128,12 @@ export class ServerGroupBasicSettings
errors.amiName = 'Image required.';
}

// this error is added exclusively to disable the "create/clone" button - it is not visible aside from the warning
// rendered by the DeployingIntoManagedClusterWarning component
if (values.resourceSummary) {
errors.resourceSummary = { id: 'Cluster is managed' };
}

return errors;
}

Expand Down Expand Up @@ -206,6 +213,7 @@ export class ServerGroupBasicSettings
</div>
</div>
)}
<DeployingIntoManagedClusterWarning app={app} formik={formik} />
<div className="form-group">
<div className="col-md-3 sm-label-right">Account</div>
<div className="col-md-7">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import React from 'react';
import { IServerGroupCommand } from 'core/serverGroup';
import { FormikProps } from 'formik';
import { Application } from 'core/application';
import { toggleResourcePause } from './toggleResourceManagement';

export interface IDeployingIntoManagedClusterWarningProps {
app: Application;
formik: FormikProps<IServerGroupCommand>;
}

export const DeployingIntoManagedClusterWarning = ({ app, formik }: IDeployingIntoManagedClusterWarningProps) => {
const [userPaused, setUserPaused] = React.useState(false);

const command = formik.values;
const pauseResource = React.useCallback(() => {
const { resourceSummary, backingData } = formik.values;
toggleResourcePause(resourceSummary, app).then(() => {
backingData.managedResources = app.getDataSource('managedResources')?.data?.resources;
setUserPaused(true);
formik.setFieldValue('resourceSummary', null);
});
}, [app, formik]);

if (!command.resourceSummary && !userPaused) {
return null;
}

if (userPaused) {
return (
<div className="alert alert-info">
<div className="horizontal top">
<div>
<i className="fa fa-check-circle" />
</div>
<div className="sp-margin-m-left">Resource management has been paused.</div>
</div>
</div>
);
}

return (
<div className="alert alert-danger">
<p>
<b>🌈 Spinnaker is continuously managing this resource.</b>
</p>
<p>Any changes you make to this cluster will be stomped in favor of the declarative configuration.</p>
<p>If you need to manually deploy a new version of this server group, you should pause management.</p>
<div className="sp-margin-m-top">
<button className="passive" onClick={pauseResource}>
<i className="fa fa-pause" /> Pause management
</button>
</div>
</div>
);
};
1 change: 1 addition & 0 deletions app/scripts/modules/core/src/managed/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './DeployingIntoManagedClusterWarning';
export * from './ManagedReader';
export * from './ManagedWriter';
export * from './ManagedResourceDetailsIndicator';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { module } from 'angular';

import { Application } from 'core/application/application.model';
import { IMoniker } from 'core/naming/IMoniker';
import { ILoadBalancer, ISecurityGroup, ISubnet, IPipeline, IStage } from 'core/domain';
import { ILoadBalancer, ISecurityGroup, ISubnet, IPipeline, IStage, IManagedResourceSummary } from 'core/domain';
import { ICapacity } from 'core/serverGroup/serverGroupWriter.service';
import { IDeploymentStrategy } from 'core/deploymentStrategy';
import { ISecurityGroupsByAccountSourceData } from 'core/securityGroup/securityGroupReader.service';
Expand Down Expand Up @@ -75,6 +75,7 @@ export interface IServerGroupCommandBackingData {
enabledMetrics: string[];
healthCheckTypes: string[];
instanceTypes: string[];
managedResources: IManagedResourceSummary[];
loadBalancers: ILoadBalancer[];
terminationPolicies: string[];
subnets: ISubnet[];
Expand Down Expand Up @@ -111,6 +112,7 @@ export interface IServerGroupCommand {
preferSourceCapacity?: boolean;
reason?: string;
region: string;
resourceSummary?: IManagedResourceSummary;
securityGroups: string[];
selectedProvider: string;
source?: {
Expand Down Expand Up @@ -138,8 +140,21 @@ export interface IServerGroupCommand {
credentialsChanged: (command: IServerGroupCommand) => IServerGroupCommandResult;
imageChanged: (command: IServerGroupCommand) => IServerGroupCommandResult;
instanceTypeChanged: (command: IServerGroupCommand) => void;
clusterChanged?: (command: IServerGroupCommand) => void;
}

export const setMatchingResourceSummary = (command: IServerGroupCommand) => {
command.resourceSummary = (command.backingData.managedResources ?? []).find(
resource =>
!resource.isPaused &&
resource.kind === 'cluster' &&
resource.locations.regions.some(r => r.name === command.region) &&
(resource.moniker.stack ?? '') === command.stack &&
(resource.moniker.detail ?? '') === command.freeFormDetails &&
resource.locations.account === command.credentials,
);
};

export class ServerGroupCommandBuilderService {
private getDelegate(provider: string, skin?: string): any {
return this.providerServiceDelegate.getDelegate(provider, 'serverGroup.commandBuilder', skin);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
SecurityGroupReader,
IVpc,
ISecurityGroup,
NameUtils,
setMatchingResourceSummary,
} from '@spinnaker/core';
import {
IAmazonApplicationLoadBalancer,
Expand Down Expand Up @@ -135,14 +137,21 @@ export class TitusServerGroupConfigurationService {
command.viewState.dirty = { ...(command.viewState.dirty || {}), ...result.dirty };
this.configureLoadBalancerOptions(command);
this.configureSecurityGroupOptions(command);
setMatchingResourceSummary(command);
return result;
};

cmd.regionChanged = (command: ITitusServerGroupCommand) => {
this.configureLoadBalancerOptions(command);
this.configureSecurityGroupOptions(command);
setMatchingResourceSummary(command);
return {};
};

cmd.clusterChanged = (command: ITitusServerGroupCommand): void => {
command.moniker = NameUtils.getMoniker(command.application, command.stack, command.freeFormDetails);
setMatchingResourceSummary(command);
};
}

public configureCommand(cmd: ITitusServerGroupCommand) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Field, FormikErrors, FormikProps } from 'formik';

import {
DeploymentStrategySelector,
DeployingIntoManagedClusterWarning,
HelpField,
NameUtils,
RegionSelectField,
Expand Down Expand Up @@ -114,6 +115,12 @@ export class ServerGroupBasicSettings
}
}

// this error is added exclusively to disable the "create/clone" button - it is not visible aside from the warning
// rendered by the DeployingIntoManagedClusterWarning component
if (values.resourceSummary) {
errors.resourceSummary = { id: 'Cluster is managed' };
}

return errors;
}

Expand All @@ -137,11 +144,15 @@ export class ServerGroupBasicSettings
};

private stackChanged = (stack: string) => {
this.props.formik.setFieldValue('stack', stack);
const { formik } = this.props;
formik.setFieldValue('stack', stack);
formik.values.clusterChanged(formik.values);
};

private freeFormDetailsChanged = (freeFormDetails: string) => {
this.props.formik.setFieldValue('freeFormDetails', freeFormDetails);
const { formik } = this.props;
formik.setFieldValue('freeFormDetails', freeFormDetails);
formik.values.clusterChanged(formik.values);
};

public componentWillReceiveProps(nextProps: IServerGroupBasicSettingsProps) {
Expand All @@ -164,14 +175,16 @@ export class ServerGroupBasicSettings
};

public render() {
const { errors, setFieldValue, values } = this.props.formik;
const { app, formik } = this.props;
const { errors, setFieldValue, values } = formik;
const { createsNewCluster, latestServerGroup, namePreview, showPreviewAsWarning } = this.state;

const accounts = values.backingData.accounts;
const readOnlyFields = values.viewState.readOnlyFields || {};

return (
<div className="container-fluid form-horizontal">
<DeployingIntoManagedClusterWarning app={app} formik={formik} />
<div className="form-group">
<div className="col-md-3 sm-label-right">Account</div>
<div className="col-md-7">
Expand Down

0 comments on commit 4302a0f

Please sign in to comment.