Skip to content

Commit

Permalink
fix(ivs): Not a standard physical name pattern (aws#24706)
Browse files Browse the repository at this point in the history
## Summary

- change physical name of Props to `<cfnResource>Name`  (e.g channelName)
- generate physical name by CDK if not present

## Why need this change

Because the alpha package `aws-ivs` does not follow standard CDK physical name conventions. 

CDK expects an auto-generated physical name to be used if the physical name is omitted. This will result in the "Use generated resource names, not physical names " of [Best practice](https://docs.aws.amazon.com/cdk/v2/guide/best-practices.html#best-practices-apps-names) can be complied with. However, the current `aws-ivs` package configuration does not follow that rule, so if omitted, the resource will be created without a physical name. Being created without a physical name is inconvenient for resource management, and many customers are forced to use physical names explicitly.
Also, the standard CDK naming convention for the physical name property should be of the form `<cfnResource>Name` like bucketName, but the IVS package is just `name` and does not conform to the standard.

## Why changes now
Resolving these issues would require breaking changes and should be resolved before migrating to the stable package.

## Related issues
Closes none.

BREAKING CHANGE: Renamed ChannelProps.name to ChannelProps.channelName
* Renamed PlaybackKeyPairProps.name to PlaybackKeyPairProps.playbackKeyPairName
* Channel now generates a physical name if one is not provided
* PlaybackKeyPair now generates a physical name if one is not provided

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
WinterYukky authored and homakk committed Mar 28, 2023
1 parent d6f3996 commit b1b5ed1
Show file tree
Hide file tree
Showing 12 changed files with 282 additions and 158 deletions.
17 changes: 10 additions & 7 deletions packages/@aws-cdk/aws-ivs/lib/channel.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as core from '@aws-cdk/core';
import { Lazy, Names } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { CfnChannel } from './ivs.generated';
import { StreamKey } from './stream-key';
Expand Down Expand Up @@ -93,11 +94,11 @@ export interface ChannelProps {
readonly latencyMode?: LatencyMode;

/**
* Channel name.
* A name for the channel.
*
* @default - None
* @default Automatically generated name
*/
readonly name?: string;
readonly channelName?: string;

/**
* The channel type, which determines the allowable resolution and bitrate.
Expand Down Expand Up @@ -152,17 +153,19 @@ export class Channel extends ChannelBase {

constructor(scope: Construct, id: string, props: ChannelProps = {}) {
super(scope, id, {
physicalName: props.name,
physicalName: props.channelName ?? Lazy.string({
produce: () => Names.uniqueResourceName(this, { maxLength: 128, allowedSpecialCharacters: '-_' }),
}),
});

if (props.name && !core.Token.isUnresolved(props.name) && !/^[a-zA-Z0-9-_]*$/.test(props.name)) {
throw new Error(`name must contain only numbers, letters, hyphens and underscores, got: '${props.name}'`);
if (this.physicalName && !core.Token.isUnresolved(this.physicalName) && !/^[a-zA-Z0-9-_]*$/.test(this.physicalName)) {
throw new Error(`channelName must contain only numbers, letters, hyphens and underscores, got: '${this.physicalName}'`);
}

const resource = new CfnChannel(this, 'Resource', {
authorized: props.authorized,
latencyMode: props.latencyMode,
name: props.name,
name: this.physicalName,
type: props.type,
});

Expand Down
17 changes: 11 additions & 6 deletions packages/@aws-cdk/aws-ivs/lib/playback-key-pair.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as core from '@aws-cdk/core';
import { Lazy, Names } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { CfnPlaybackKeyPair } from './ivs.generated';

Expand Down Expand Up @@ -35,9 +36,9 @@ export interface PlaybackKeyPairProps {
* An arbitrary string (a nickname) assigned to a playback key pair that helps the customer identify that resource.
* The value does not need to be unique.
*
* @default None
* @default Automatically generated name
*/
readonly name?: string;
readonly playbackKeyPairName?: string;
}

/**
Expand All @@ -54,15 +55,19 @@ export class PlaybackKeyPair extends PlaybackKeyPairBase {
public readonly playbackKeyPairFingerprint: string;

constructor(scope: Construct, id: string, props: PlaybackKeyPairProps) {
super(scope, id, {});
super(scope, id, {
physicalName: props.playbackKeyPairName ?? Lazy.string({
produce: () => Names.uniqueResourceName(this, { maxLength: 128, allowedSpecialCharacters: '-_' }),
}),
});

if (props.name && !core.Token.isUnresolved(props.name) && !/^[a-zA-Z0-9-_]*$/.test(props.name)) {
throw new Error(`name must contain only numbers, letters, hyphens and underscores, got: '${props.name}'`);
if (props.playbackKeyPairName && !core.Token.isUnresolved(props.playbackKeyPairName) && !/^[a-zA-Z0-9-_]*$/.test(props.playbackKeyPairName)) {
throw new Error(`playbackKeyPairName must contain only numbers, letters, hyphens and underscores, got: '${props.playbackKeyPairName}'`);
}

const resource = new CfnPlaybackKeyPair(this, 'Resource', {
publicKeyMaterial: props.publicKeyMaterial,
name: props.name,
name: props.playbackKeyPairName,
});

this.playbackKeyPairArn = resource.attrArn;
Expand Down
2 changes: 0 additions & 2 deletions packages/@aws-cdk/aws-ivs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@
},
"awslint": {
"exclude": [
"props-physical-name:@aws-cdk/aws-ivs.ChannelProps",
"props-physical-name:@aws-cdk/aws-ivs.StreamKeyProps",
"props-physical-name:@aws-cdk/aws-ivs.PlaybackKeyPairProps",
"from-method:@aws-cdk/aws-ivs.StreamKey",
"from-method:@aws-cdk/aws-ivs.PlaybackKeyPair"
]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "30.0.0",
"version": "31.0.0",
"files": {
"71d02f468f201d8c79d33a72cd8debac3b538f4e0efb492c6c06aca0055029f8": {
"fef396e89f5ce4eed5b6b04937e297b9abeab6a5db8cd808498a20aca8c61ef3": {
"source": {
"path": "aws-cdk-ivs.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "71d02f468f201d8c79d33a72cd8debac3b538f4e0efb492c6c06aca0055029f8.json",
"objectKey": "fef396e89f5ce4eed5b6b04937e297b9abeab6a5db8cd808498a20aca8c61ef3.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
{
"Resources": {
"PlaybackKeyPairBE17315B": {
"DefaultPropertiesPlaybackKeyPairaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaB5D4BE27": {
"Type": "AWS::IVS::PlaybackKeyPair",
"Properties": {
"PublicKeyMaterial": "-----BEGIN PUBLIC KEY-----\nMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEHBm/D9UFf1z4czcAFuM7w+tstxxzoLVo\nfa1OT0gQjRYsy/YTcrKI5FS7ur3NZIcmiwqerr7dP0wSZjfEMNe82W1zWdkxHJ6Y\n73g9gZDxwGdjowZjEOIvAeH2Of6NeDOo\n-----END PUBLIC KEY-----"
}
},
"DefaultPropertiesChannelaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa201FBD46": {
"Type": "AWS::IVS::Channel",
"Properties": {
"Name": "aws-cdk-ivsDefaultPropertiesChannel-_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaEDEAEDA9"
}
},
"AllPropertiesPlaybackKeyPair96291E97": {
"Type": "AWS::IVS::PlaybackKeyPair",
"Properties": {
"Name": "IVSIntegrationTestPlaybackKeyPair",
"PublicKeyMaterial": "-----BEGIN PUBLIC KEY-----\nMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEs6k8Xf6WyFq3yZXoup8G/gH6DntSATqD\nYfo83eX0GJCKxJ8fr09h9LP9HDGof8/bo66P+SGHeAARGF/O9WPAQVUgSlm/KMFX\nEPtPtOm1s0GR9k1ydU5hkI++f9CoZ5lM\n-----END PUBLIC KEY-----"
}
},
"Channel4048F119": {
"AllPropertiesChannel737C871D": {
"Type": "AWS::IVS::Channel",
"Properties": {
"Authorized": true,
Expand All @@ -16,39 +28,39 @@
"Type": "BASIC"
}
},
"StreamKey9F296F4F": {
"AllPropertiesStreamKey2A169FFE": {
"Type": "AWS::IVS::StreamKey",
"Properties": {
"ChannelArn": {
"Fn::GetAtt": [
"Channel4048F119",
"AllPropertiesChannel737C871D",
"Arn"
]
}
}
}
},
"Outputs": {
"PlaybackKeyPairArn": {
"AllPropertiesPlaybackKeyPairArn9C29D23B": {
"Value": {
"Fn::GetAtt": [
"PlaybackKeyPairBE17315B",
"AllPropertiesPlaybackKeyPair96291E97",
"Arn"
]
}
},
"ChannelArn": {
"AllPropertiesChannelArn97A102C5": {
"Value": {
"Fn::GetAtt": [
"Channel4048F119",
"AllPropertiesChannel737C871D",
"Arn"
]
}
},
"StreamKeyArn": {
"AllPropertiesStreamKeyArnB62C0761": {
"Value": {
"Fn::GetAtt": [
"StreamKey9F296F4F",
"AllPropertiesStreamKey2A169FFE",
"Arn"
]
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"30.0.0"}
{"version":"31.0.0"}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "30.0.0",
"version": "31.0.0",
"testCases": {
"ivs-test/DefaultTest": {
"stacks": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "30.0.0",
"version": "31.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
Expand Down
40 changes: 26 additions & 14 deletions packages/@aws-cdk/aws-ivs/test/integ.ivs.js.snapshot/manifest.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "30.0.0",
"version": "31.0.0",
"artifacts": {
"aws-cdk-ivs.assets": {
"type": "cdk:asset-manifest",
Expand All @@ -17,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/71d02f468f201d8c79d33a72cd8debac3b538f4e0efb492c6c06aca0055029f8.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/fef396e89f5ce4eed5b6b04937e297b9abeab6a5db8cd808498a20aca8c61ef3.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand All @@ -33,40 +33,52 @@
"aws-cdk-ivs.assets"
],
"metadata": {
"/aws-cdk-ivs/PlaybackKeyPair/Resource": [
"/aws-cdk-ivs/DefaultProperties/PlaybackKeyPair-_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "PlaybackKeyPairBE17315B"
"data": "DefaultPropertiesPlaybackKeyPairaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaB5D4BE27"
}
],
"/aws-cdk-ivs/Channel/Resource": [
"/aws-cdk-ivs/DefaultProperties/Channel-_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "Channel4048F119"
"data": "DefaultPropertiesChannelaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa201FBD46"
}
],
"/aws-cdk-ivs/StreamKey/Resource": [
"/aws-cdk-ivs/AllProperties/PlaybackKeyPair/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "StreamKey9F296F4F"
"data": "AllPropertiesPlaybackKeyPair96291E97"
}
],
"/aws-cdk-ivs/PlaybackKeyPairArn": [
"/aws-cdk-ivs/AllProperties/Channel/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "PlaybackKeyPairArn"
"data": "AllPropertiesChannel737C871D"
}
],
"/aws-cdk-ivs/ChannelArn": [
"/aws-cdk-ivs/AllProperties/StreamKey/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "ChannelArn"
"data": "AllPropertiesStreamKey2A169FFE"
}
],
"/aws-cdk-ivs/StreamKeyArn": [
"/aws-cdk-ivs/AllProperties/PlaybackKeyPairArn": [
{
"type": "aws:cdk:logicalId",
"data": "StreamKeyArn"
"data": "AllPropertiesPlaybackKeyPairArn9C29D23B"
}
],
"/aws-cdk-ivs/AllProperties/ChannelArn": [
{
"type": "aws:cdk:logicalId",
"data": "AllPropertiesChannelArn97A102C5"
}
],
"/aws-cdk-ivs/AllProperties/StreamKeyArn": [
{
"type": "aws:cdk:logicalId",
"data": "AllPropertiesStreamKeyArnB62C0761"
}
],
"/aws-cdk-ivs/BootstrapVersion": [
Expand Down
Loading

0 comments on commit b1b5ed1

Please sign in to comment.