Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Disable CIDR feature when AppStream is enabled #632

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable import/prefer-default-export */
const enableBuiltInWorkspaces = process.env.REACT_APP_ENABLE_BUILT_IN_WORKSPACES === 'true';
const enableEgressStore = process.env.REACT_APP_ENABLE_EGRESS_STORE;
const isAppStreamEnabled = process.env.REACT_APP_IS_APP_STREAM_ENABLED === 'true';

export { enableBuiltInWorkspaces, enableEgressStore };
export { enableBuiltInWorkspaces, enableEgressStore, isAppStreamEnabled };
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { displayError } from '@aws-ee/base-ui/dist/helpers/notification';

import ScEnvironmentConnections from './ScEnvironmentConnections';
import ScEnvironmentUpdateCidrs from './ScEnvironmentUpdateCidrs';
import { enableEgressStore } from '../../../helpers/settings';
import { enableEgressStore, isAppStreamEnabled } from '../../../helpers/settings';
import ScEnvironmentEgressStoreDetail from './ScEnvironmentEgressStoreDetail';

const PROCESSING_STATUS_CODE = 'PROCESSING';
Expand Down Expand Up @@ -212,7 +212,7 @@ class ScEnvironmentButtons extends React.Component {
View Detail
</Button>
)}
{state.canTerminate && !state.key.includes('FAILED') && (
{!isAppStreamEnabled && state.canTerminate && !state.key.includes('FAILED') && (
<Button
floated="left"
basic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class CreateInternalEnvForm extends React.Component {
runInAction(() => {
this.form = getCreateInternalEnvForm({
projectIdOptions: this.getProjectIdOptions(),
cidr: this.props.defaultCidr,
cidr: this.isAppStreamEnabled ? undefined : this.props.defaultCidr,
});
});
}
Expand Down Expand Up @@ -148,7 +148,7 @@ class CreateInternalEnvForm extends React.Component {

renderForm() {
const form = this.form;
const askForCidr = !_.isUndefined(this.props.defaultCidr);
const askForCidr = !_.isUndefined(this.props.defaultCidr) && !this.isAppStreamEnabled;
const configurations = this.configurations;

// we show the AppStream configuration warning when the feature is enabled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Parameters:
AccessFromCIDRBlock:
Type: String
Description: The CIDR used to access the ec2 instances.
Default: 10.0.0.0/19
S3Mounts:
Type: String
Description: A JSON array of objects with name, bucket, and prefix properties used to mount data
Expand Down Expand Up @@ -137,35 +138,43 @@ Resources:
FromPort: 0
ToPort: 65535
CidrIp: 0.0.0.0/0
- IpProtocol: icmp
FromPort: -1
ToPort: -1
CidrIp: !Ref AccessFromCIDRBlock
- !If
- AppStreamEnabled
- !Ref "AWS::NoValue"
- IpProtocol: icmp
FromPort: -1
ToPort: -1
CidrIp: !Ref AccessFromCIDRBlock
- !If
- AppStreamEnabled
- DestinationSecurityGroupId:
Fn::ImportValue: !Sub "${SolutionNamespace}-CfnEndpointSecurityGroup"
IpProtocol: '-1'
- !Ref "AWS::NoValue"
SecurityGroupIngress:
- IpProtocol: tcp
FromPort: 22
ToPort: 22
CidrIp: !Ref AccessFromCIDRBlock
- IpProtocol: tcp
FromPort: 80
ToPort: 80
CidrIp: !Ref AccessFromCIDRBlock
- IpProtocol: tcp
FromPort: 443
ToPort: 443
CidrIp: !Ref AccessFromCIDRBlock
- !If
- AppStreamEnabled
- SourceSecurityGroupId:
Fn::ImportValue: !Sub "${SolutionNamespace}-SwbAppStreamSG"
IpProtocol: '-1'
- IpProtocol: tcp
FromPort: 22
ToPort: 22
CidrIp: !Ref AccessFromCIDRBlock
- !If
- AppStreamEnabled
- !Ref "AWS::NoValue"
- IpProtocol: tcp
FromPort: 80
ToPort: 80
CidrIp: !Ref AccessFromCIDRBlock
- !If
- AppStreamEnabled
- !Ref "AWS::NoValue"
- IpProtocol: tcp
FromPort: 443
ToPort: 443
CidrIp: !Ref AccessFromCIDRBlock
Tags:
- Key: Name
Value: !Join ['-', [Ref: Namespace, 'ec2-sg']]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Parameters:
AccessFromCIDRBlock:
Type: String
Description: The CIDR used to access the ec2 instances.
Default: 10.0.0.0/19
S3Mounts:
Type: String
Description: A JSON array of objects with name, bucket, and prefix properties used to mount data
Expand Down Expand Up @@ -159,27 +160,29 @@ Resources:
FromPort: 0
ToPort: 65535
CidrIp: 0.0.0.0/0
- IpProtocol: icmp
FromPort: -1
ToPort: -1
CidrIp: !Ref AccessFromCIDRBlock
- !If
- AppStreamEnabled
- !Ref "AWS::NoValue"
- IpProtocol: icmp
FromPort: -1
ToPort: -1
CidrIp: !Ref AccessFromCIDRBlock
- !If
- AppStreamEnabled
- DestinationSecurityGroupId:
Fn::ImportValue: !Sub "${SolutionNamespace}-CfnEndpointSecurityGroup"
IpProtocol: '-1'
- !Ref "AWS::NoValue"
SecurityGroupIngress:
- IpProtocol: tcp
FromPort: 3389
ToPort: 3389
CidrIp: !Ref AccessFromCIDRBlock
- !If
- AppStreamEnabled
- SourceSecurityGroupId:
Fn::ImportValue: !Sub "${SolutionNamespace}-SwbAppStreamSG"
IpProtocol: '-1'
- !Ref "AWS::NoValue"
- IpProtocol: tcp
FromPort: 3389
ToPort: 3389
CidrIp: !Ref AccessFromCIDRBlock
Tags:
- Key: Name
Value: !Join ['-', [Ref: Namespace, 'ec2-sg']]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ Parameters:
Type: AWS::EC2::Subnet::Id
AccessFromCIDRBlock:
Type: String
Description: The CIDR used to access sagemaker.
Description: The CIDR used to access sagemaker. This parameter is only required when AppStream is disabled
Default: 10.0.0.0/19
S3Mounts:
Type: String
Description: A JSON array of objects with name, bucket and prefix properties used to mount data
Expand Down Expand Up @@ -60,10 +61,13 @@ Resources:
VpcId:
Ref: VPC
SecurityGroupIngress:
- IpProtocol: tcp
FromPort: 443
ToPort: 443
CidrIp: !Ref AccessFromCIDRBlock
- !If
- AppStreamEnabled
- !Ref "AWS::NoValue"
- IpProtocol: tcp
FromPort: 443
ToPort: 443
CidrIp: !Ref AccessFromCIDRBlock

PreSignedURLBoundary:
Type: AWS::IAM::ManagedPolicy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ describe('EnvironmentScCidrService', () => {
let service = null;
let environmentScService = null;
let lockService = null;
let settings = null;

beforeEach(async () => {
const container = new ServicesContainer();
Expand All @@ -69,6 +70,13 @@ describe('EnvironmentScCidrService', () => {
service = await container.find('environmentScCidrService');
lockService = await container.find('lockService');
environmentScService = await container.find('environmentScService');
settings = await container.find('settings');
settings.getBoolean = jest.fn(key => {
if (key === 'isAppStreamEnabled') {
return false;
}
throw Error(`${key} not found`);
});

// Skip authorization by default
service.assertAuthorized = jest.fn();
Expand All @@ -79,6 +87,39 @@ describe('EnvironmentScCidrService', () => {
});

describe('Validation checks', () => {
it('should fail validation check since AppStream is enabled', async () => {
// BUILD
settings.getBoolean = jest.fn(key => {
if (key === 'isAppStreamEnabled') {
return true;
}
throw Error(`${key} not found`);
});
const requestContext = {};
const params = {
id: 'testId',
updateRequest: [
{
protocol: 'tcp',
fromPort: 123,
toPort: 123,
cidrBlocks: ['123.123.123.123/32'],
},
],
};

// OPERATE
try {
await service.update(requestContext, params);
expect.hasAssertions();
} catch (err) {
expect(service.boom.is(err, 'badRequest')).toBe(true);
expect(err.message).toContain('CIDR operation unavailable when AppStream is enabled');
expect(settings.getBoolean).toHaveBeenCalledTimes(1);
expect(settings.getBoolean).toHaveBeenCalledWith('isAppStreamEnabled');
}
});

it('should fail because the updateRequest contains additional properties', async () => {
// BUILD
const params = {
Expand Down
Loading