-
Notifications
You must be signed in to change notification settings - Fork 4k
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(servicecatalogappregistry): initial L2 construct for Application #15140
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
e93bd3e
Initial L2 construct for appregisty application
be2e930
Adding AppRegistry Application construct
27f685c
Merge branch 'master' into appregistry_l2_construct
arcrank 6911452
Merge branch 'master' into appregistry_l2_construct
arcrank 517df1b
Addressing comments from PR
40c8ba4
Merge branch 'master' into appregistry_l2_construct
arcrank 0ab78e0
Fixing input validation tests
854cb01
Merge branch 'appregistry_l2_construct' of github.com:arcrank/aws-cdk…
1ba5eed
removing trailing whitespaces...
5b763b3
Merge branch 'master' into appregistry_l2_construct
arcrank 427e54a
Merge branch 'master' into appregistry_l2_construct
arcrank c652129
Update packages/@aws-cdk/aws-servicecatalogappregistry/README.md
arcrank be8b38b
Update packages/@aws-cdk/aws-servicecatalogappregistry/lib/applicatio…
arcrank 46abb81
Merge branch 'aws:master' into appregistry_l2_construct
arcrank bbfb0f1
updating ARN parsing and error messaging
48ffa72
Merge branch 'master' into appregistry_l2_construct
mergify[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
98 changes: 98 additions & 0 deletions
98
packages/@aws-cdk/aws-servicecatalogappregistry/lib/application.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
import * as cdk from '@aws-cdk/core'; | ||
import { InputValidator } from './private/validation'; | ||
import { CfnApplication } from './servicecatalogappregistry.generated'; | ||
|
||
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main | ||
// eslint-disable-next-line no-duplicate-imports, import/order | ||
import { Construct } from 'constructs'; | ||
|
||
/** | ||
* A Service Catalog AppRegistry Application. | ||
*/ | ||
export interface IApplication extends cdk.IResource { | ||
/** | ||
* The ARN of the application. | ||
* @attribute | ||
*/ | ||
readonly applicationArn: string; | ||
|
||
/** | ||
* The ID of the application. | ||
* @attribute | ||
*/ | ||
readonly applicationId: string; | ||
} | ||
|
||
/** | ||
* Properties for a Service Catalog AppRegistry Application | ||
*/ | ||
export interface ApplicationProps { | ||
/** | ||
* Enforces a particular physical application name. | ||
*/ | ||
readonly applicationName: string; | ||
|
||
/** | ||
* Description for application. | ||
* @default - No description provided | ||
*/ | ||
readonly description?: string; | ||
} | ||
|
||
abstract class ApplicationBase extends cdk.Resource implements IApplication { | ||
public abstract readonly applicationArn: string; | ||
public abstract readonly applicationId: string; | ||
} | ||
skinny85 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* A Service Catalog AppRegistry Application. | ||
*/ | ||
export class Application extends ApplicationBase { | ||
/** | ||
* Imports an Application construct that represents an external application. | ||
* | ||
* @param scope The parent creating construct (usually `this`). | ||
* @param id The construct's name. | ||
* @param applicationArn the Amazon Resource Name of the existing AppRegistry Application | ||
*/ | ||
public static fromApplicationArn(scope: Construct, id: string, applicationArn: string): IApplication { | ||
const arn = cdk.Stack.of(scope).splitArn(applicationArn, cdk.ArnFormat.SLASH_RESOURCE_SLASH_RESOURCE_NAME); | ||
const applicationId = arn.resourceName; | ||
|
||
if (!applicationId) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed this to match our SC construct error check and messaging |
||
throw new Error('Missing required Application ID from Application ARN: ' + applicationArn); | ||
} | ||
|
||
class Import extends ApplicationBase { | ||
public readonly applicationArn = applicationArn; | ||
public readonly applicationId = applicationId!; | ||
} | ||
|
||
return new Import(scope, id, { | ||
environmentFromArn: applicationArn, | ||
}); | ||
} | ||
|
||
public readonly applicationArn: string; | ||
public readonly applicationId: string; | ||
|
||
constructor(scope: Construct, id: string, props: ApplicationProps) { | ||
super(scope, id); | ||
|
||
this.validateApplicationProps(props); | ||
|
||
const application = new CfnApplication(this, 'Resource', { | ||
name: props.applicationName, | ||
arcrank marked this conversation as resolved.
Show resolved
Hide resolved
|
||
description: props.description, | ||
}); | ||
|
||
this.applicationArn = application.attrArn; | ||
this.applicationId = application.attrId; | ||
} | ||
arcrank marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
private validateApplicationProps(props: ApplicationProps) { | ||
InputValidator.validateLength(this.node.path, 'application name', 1, 256, props.applicationName); | ||
InputValidator.validateRegex(this.node.path, 'application name', /^[a-zA-Z0-9-_]+$/, props.applicationName); | ||
InputValidator.validateLength(this.node.path, 'application description', 0, 1024, props.description); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,4 @@ | ||
export * from './application'; | ||
|
||
// AWS::ServiceCatalogAppRegistry CloudFormation Resources: | ||
export * from './servicecatalogappregistry.generated'; |
31 changes: 31 additions & 0 deletions
31
packages/@aws-cdk/aws-servicecatalogappregistry/lib/private/validation.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import * as cdk from '@aws-cdk/core'; | ||
|
||
/** | ||
* Class to validate that inputs match requirements. | ||
*/ | ||
export class InputValidator { | ||
/** | ||
* Validates length is between allowed min and max lengths. | ||
*/ | ||
public static validateLength(resourceName: string, inputName: string, minLength: number, maxLength: number, inputString?: string): void { | ||
if (!cdk.Token.isUnresolved(inputString) && inputString !== undefined && (inputString.length < minLength || inputString.length > maxLength)) { | ||
throw new Error(`Invalid ${inputName} for resource ${resourceName}, must have length between ${minLength} and ${maxLength}, got: '${this.truncateString(inputString, 100)}'`); | ||
} | ||
} | ||
|
||
/** | ||
* Validates a regex. | ||
*/ | ||
public static validateRegex(resourceName: string, inputName: string, regex: RegExp, inputString?: string): void { | ||
if (!cdk.Token.isUnresolved(inputString) && inputString !== undefined && !regex.test(inputString)) { | ||
throw new Error(`Invalid ${inputName} for resource ${resourceName}, must match regex pattern ${regex}, got: '${this.truncateString(inputString, 100)}'`); | ||
} | ||
} | ||
|
||
private static truncateString(string: string, maxLength: number): string { | ||
if (string.length > maxLength) { | ||
return string.substring(0, maxLength) + '[truncated]'; | ||
} | ||
return string; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
132 changes: 132 additions & 0 deletions
132
packages/@aws-cdk/aws-servicecatalogappregistry/test/application.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
import '@aws-cdk/assert-internal/jest'; | ||
import * as cdk from '@aws-cdk/core'; | ||
import * as appreg from '../lib'; | ||
|
||
describe('Application', () => { | ||
let stack: cdk.Stack; | ||
|
||
beforeEach(() => { | ||
stack = new cdk.Stack(); | ||
}); | ||
|
||
test('default application creation', () => { | ||
new appreg.Application(stack, 'MyApplication', { | ||
applicationName: 'testApplication', | ||
}); | ||
|
||
expect(stack).toMatchTemplate({ | ||
Resources: { | ||
MyApplication5C63EC1D: { | ||
Type: 'AWS::ServiceCatalogAppRegistry::Application', | ||
Properties: { | ||
Name: 'testApplication', | ||
}, | ||
}, | ||
}, | ||
}); | ||
}), | ||
|
||
test('application with explicit description', () => { | ||
const description = 'my test application description'; | ||
new appreg.Application(stack, 'MyApplication', { | ||
applicationName: 'testApplication', | ||
description: description, | ||
}); | ||
|
||
expect(stack).toHaveResourceLike('AWS::ServiceCatalogAppRegistry::Application', { | ||
Description: description, | ||
}); | ||
}), | ||
|
||
test('application with application tags', () => { | ||
const application = new appreg.Application(stack, 'MyApplication', { | ||
applicationName: 'testApplication', | ||
}); | ||
|
||
cdk.Tags.of(application).add('key1', 'value1'); | ||
cdk.Tags.of(application).add('key2', 'value2'); | ||
|
||
expect(stack).toHaveResourceLike('AWS::ServiceCatalogAppRegistry::Application', { | ||
Tags: { | ||
key1: 'value1', | ||
key2: 'value2', | ||
}, | ||
}); | ||
}), | ||
|
||
test('for an application imported by ARN', () => { | ||
const application = appreg.Application.fromApplicationArn(stack, 'MyApplication', | ||
'arn:aws:servicecatalog:us-east-1:123456789012:/applications/0aqmvxvgmry0ecc4mjhwypun6i'); | ||
expect(application.applicationId).toEqual('0aqmvxvgmry0ecc4mjhwypun6i'); | ||
}), | ||
|
||
test('fails for application imported by ARN missing applicationId', () => { | ||
expect(() => { | ||
appreg.Application.fromApplicationArn(stack, 'MyApplication', | ||
'arn:aws:servicecatalog:us-east-1:123456789012:/applications/'); | ||
}).toThrow(/Missing required Application ID from Application ARN:/); | ||
}), | ||
|
||
test('application created with a token description does not throw validation error and creates', () => { | ||
const tokenDescription = new cdk.CfnParameter(stack, 'Description'); | ||
|
||
new appreg.Application(stack, 'MyApplication', { | ||
applicationName: 'myApplication', | ||
description: tokenDescription.valueAsString, | ||
}); | ||
|
||
expect(stack).toHaveResourceLike('AWS::ServiceCatalogAppRegistry::Application', { | ||
Description: { | ||
Ref: 'Description', | ||
}, | ||
}); | ||
}), | ||
|
||
test('application created with a token application name does not throw validation error', () => { | ||
const tokenApplicationName= new cdk.CfnParameter(stack, 'ApplicationName'); | ||
|
||
new appreg.Application(stack, 'MyApplication', { | ||
applicationName: tokenApplicationName.valueAsString, | ||
}); | ||
|
||
expect(stack).toHaveResourceLike('AWS::ServiceCatalogAppRegistry::Application', { | ||
Name: { | ||
Ref: 'ApplicationName', | ||
}, | ||
}); | ||
}), | ||
|
||
test('fails for application with description length longer than allowed', () => { | ||
expect(() => { | ||
new appreg.Application(stack, 'MyApplication', { | ||
applicationName: 'testApplication', | ||
description: 'too long description'.repeat(1000), | ||
}); | ||
}).toThrow(/Invalid application description for resource/); | ||
}), | ||
|
||
test('fails for application creation with name too short', () => { | ||
expect(() => { | ||
new appreg.Application(stack, 'MyApplication', { | ||
applicationName: '', | ||
}); | ||
}).toThrow(/Invalid application name for resource/); | ||
}), | ||
|
||
test('fails for application with name too long', () => { | ||
expect(() => { | ||
new appreg.Application(stack, 'MyApplication', { | ||
applicationName: 'testApplication'.repeat(50), | ||
}); | ||
}).toThrow(/Invalid application name for resource/); | ||
}), | ||
|
||
test('fails for application with name of invalid characters', () => { | ||
expect(() => { | ||
new appreg.Application(stack, 'MyApplication', { | ||
applicationName: 'My@ppl!iC@ #', | ||
}); | ||
}).toThrow(/Invalid application name for resource/); | ||
}); | ||
}); | ||
|
||
arcrank marked this conversation as resolved.
Show resolved
Hide resolved
|
11 changes: 11 additions & 0 deletions
11
packages/@aws-cdk/aws-servicecatalogappregistry/test/integ.application.expected.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"Resources": { | ||
"TestApplication2FBC585F": { | ||
"Type": "AWS::ServiceCatalogAppRegistry::Application", | ||
"Properties": { | ||
"Name": "myApplicationtest", | ||
"Description": "my application description" | ||
} | ||
} | ||
} | ||
} |
13 changes: 13 additions & 0 deletions
13
packages/@aws-cdk/aws-servicecatalogappregistry/test/integ.application.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import * as cdk from '@aws-cdk/core'; | ||
import * as appreg from '../lib'; | ||
|
||
const app = new cdk.App(); | ||
const stack = new cdk.Stack(app, 'integ-servicecatalogappregistry-application'); | ||
|
||
new appreg.Application(stack, 'TestApplication', { | ||
applicationName: 'myApplicationtest', | ||
description: 'my application description', | ||
}); | ||
|
||
app.synth(); | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW - you called this
displayName
inPortfolio
. Any reason you went with a different name inApplication
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual cfn term in
Portfolio
isDisplayName
, see here. For application it is justname
henceapplicationName
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't want to make this consistent across the ServiceCatalog constructs that exhibit this behavior (display name + auto-generated ID)?