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.

Sign up for GitHub

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

Jump to bottom

feat(codepipeline): change default pipeline type to V2 (under feature flag) #29096

Merged
merged 23 commits into from
Mar 12, 2024
Merged

feat(codepipeline): change default pipeline type to V2 (under feature flag) #29096

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f579c37
feat(codepipeline): change default value for pipelineType to V2 (unde…
go-to-k Feb 14, 2024
fed918e
integ test
go-to-k Feb 14, 2024
aeca2c8
change existing integ tests
go-to-k Feb 14, 2024
96dbaad
postCliContext
go-to-k Feb 14, 2024
9c80945
add to change rest integ tests
go-to-k Feb 14, 2024
dc60a16
appconfig integ
go-to-k Feb 14, 2024
5e7e47b
integ.codepipeline-start-pipeline-execution.ts
go-to-k Feb 14, 2024
1223806
Merge branch 'main' into codepipeline-pipeline-type-default-v2
go-to-k Feb 17, 2024
a28c548
README
go-to-k Mar 3, 2024
5a23e30
Merge branch 'main' of https://github.com/go-to-k/aws-cdk into codepi…
go-to-k Mar 11, 2024
599243b
change unit tests
go-to-k Mar 11, 2024
3724ba6
aws-appconfig/test/integ.configuration
go-to-k Mar 11, 2024
ae27b58
integ.pipeline-type-v2-by-default
go-to-k Mar 11, 2024
aae1861
README
go-to-k Mar 12, 2024
79cd494
test for app context
go-to-k Mar 12, 2024
ce29920
test postCliContext vs context
go-to-k Mar 12, 2024
31c5b13
Revert "test for app context"
go-to-k Mar 12, 2024
85f008e
change postCliContext to context
go-to-k Mar 12, 2024
8ad99f0
change postCliContext added newly to context
go-to-k Mar 12, 2024
2621600
Revert "change postCliContext added newly to context"
go-to-k Mar 12, 2024
6e0689d
Revert "change postCliContext to context"
go-to-k Mar 12, 2024
0abe694
Revert "test postCliContext vs context"
go-to-k Mar 12, 2024
b55237b
Merge branch 'main' into codepipeline-pipeline-type-default-v2
mergify[bot] Mar 12, 2024
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
20 changes: 19 additions & 1 deletion packages/@aws-cdk/cx-api/FEATURE_FLAGS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ Flags come in three types:
| [@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource](#aws-cdkaws-codepipeline-actionsusenewdefaultbranchforcodecommitsource) | When enabled, the CodeCommit source action is using the default branch name 'main'. | 2.103.1 | (fix) |
| [@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction](#aws-cdkaws-cloudwatch-actionschangelambdapermissionlogicalidforlambdaaction) | When enabled, the logical ID of a Lambda permission for a Lambda action includes an alarm ID. | 2.124.0 | (fix) |
| [@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse](#aws-cdkaws-codepipelinecrossaccountkeysdefaultvaluetofalse) | Enables Pipeline to set the default value for crossAccountKeys to false. | V2NEXT | (default) |
| [@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2](#aws-cdkaws-codepipelinedefaultpipelinetypetov2) | Enables Pipeline to set the default pipeline type to V2. | V2NEXT | (default) |

<!-- END table -->

Expand Down Expand Up @@ -120,7 +121,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-rds:preventRenderingDeprecatedCredentials": true,
"@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource": true,
"@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction": true,
"@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse": true
"@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse": true,
"@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2": true
}
}
```
Expand Down Expand Up @@ -1231,4 +1233,20 @@ construct, the construct automatically defaults the value of this property to fa
**Compatibility with old behavior:** Pass `crossAccountKeys: true` to `Pipeline` construct to restore the previous behavior.


### @aws-cdk/aws-codepipeline:defaultPipelineTypeToV2

*Enables Pipeline to set the default pipeline type to V2.* (default)

When this feature flag is enabled, and the `pipelineType` property is not provided in a `Pipeline`
construct, the construct automatically defaults the value of this property to `PipelineType.V2`.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| V2NEXT | `false` | `true` |

**Compatibility with old behavior:** Pass `pipelineType: PipelineType.V1` to `Pipeline` construct to restore the previous behavior.


<!-- END details -->
11 changes: 6 additions & 5 deletions packages/aws-cdk-lib/aws-codepipeline/lib/pipeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ export interface PipelineProps {
/**
* Type of the pipeline.
*
* @default - PipelineType.V1
* @default - PipelineType.V2 if the feature flag `CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2`
* is true, PipelineType.V1 otherwise
*
* @see https://docs.aws.amazon.com/codepipeline/latest/userguide/pipeline-types-planning.html
*/
Expand Down Expand Up @@ -489,11 +490,11 @@ export class Pipeline extends PipelineBase {
assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com'),
});

// TODO: Change the default value of `pipelineType` to V2 under a feature flag.
if (props.pipelineType === undefined) {
const isDefaultV2 = FeatureFlags.of(this).isEnabled(cxapi.CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2);
if (!isDefaultV2 && props.pipelineType === undefined) {
Annotations.of(this).addWarningV2('@aws-cdk/aws-codepipeline:unspecifiedPipelineType', 'V1 pipeline type is implicitly selected when `pipelineType` is not set. If you want to use V2 type, set `PipelineType.V2`.');
}
this.pipelineType = props.pipelineType ?? PipelineType.V1;
this.pipelineType = props.pipelineType ?? (isDefaultV2 ? PipelineType.V2 : PipelineType.V1);

this.codePipeline = new CfnPipeline(this, 'Resource', {
artifactStore: Lazy.any({ produce: () => this.renderArtifactStoreProperty() }),
Expand All @@ -502,7 +503,7 @@ export class Pipeline extends PipelineBase {
disableInboundStageTransitions: Lazy.any({ produce: () => this.renderDisabledTransitions() }, { omitEmptyArray: true }),
roleArn: this.role.roleArn,
restartExecutionOnUpdate: props && props.restartExecutionOnUpdate,
pipelineType: props.pipelineType,
pipelineType: props.pipelineType ?? (isDefaultV2 ? PipelineType.V2 : undefined),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: seems redundant check since on line 536, you've already used null coalescing operator ??.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line 536 is the following:

this.pipelineType = props.pipelineType ?? (isDefaultV2 ? PipelineType.V2 : PipelineType.V1);

The difference between the line 536 and the code is whether it sets undefined or PipelineType.V1 when props.pipelineType is undefined and the isDefaultV2 is false.

Without this code (not this.pipelineType), if someone who did not originally specify props.pipelineType upgrades to the version that includes this change, the PipelineType.V1 will be explicitly set to L1 if the feature flag is left off. This is by no means a breaking change, but it does mean that the version upgrade will cause the snapshot test to fail. In order to avoid changing the behavior with previous versions, it was intended that the template would be the same as the existing one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh thanks for the explanation.

variables: Lazy.any({ produce: () => this.renderVariables() }, { omitEmptyArray: true }),
triggers: Lazy.any({ produce: () => this.renderTriggers() }, { omitEmptyArray: true }),
name: this.physicalName,
Expand Down
30 changes: 29 additions & 1 deletion packages/aws-cdk-lib/aws-codepipeline/test/pipeline.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,8 @@ describe('', () => {
test.each([
[codepipeline.PipelineType.V1, 'V1'],
[codepipeline.PipelineType.V2, 'V2'],
])('can specify pipeline type %s', (type, expected) => {
[undefined, Match.absent()],
])('can specify pipeline type %s when feature flag is not set', (type, expected) => {
const stack = new cdk.Stack();
const pipeline = new codepipeline.Pipeline(stack, 'Pipeline', {
pipelineType: type,
Expand All @@ -559,6 +560,33 @@ describe('', () => {
PipelineType: expected,
});
});

test.each([
[codepipeline.PipelineType.V1, 'V1'],
[codepipeline.PipelineType.V2, 'V2'],
[undefined, 'V2'],
])('can specify pipeline type %s when feature flag is enabled', (type, expected) => {
const stack = new cdk.Stack();
stack.node.setContext(cxapi.CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2, true);
const pipeline = new codepipeline.Pipeline(stack, 'Pipeline', {
pipelineType: type,
});

const sourceArtifact = new codepipeline.Artifact();
const sourceActions = [new FakeSourceAction({
actionName: 'FakeSource',
output: sourceArtifact,
})];
const buildActions = [new FakeBuildAction({
actionName: 'FakeBuild',
input: sourceArtifact,
})];
testPipelineSetup(pipeline, sourceActions, buildActions);

Template.fromStack(stack).hasResourceProperties('AWS::CodePipeline::Pipeline', {
PipelineType: expected,
});
});
});

describe('cross account key alias name tests', () => {
Expand Down
20 changes: 19 additions & 1 deletion packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ Flags come in three types:
| [@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource](#aws-cdkaws-codepipeline-actionsusenewdefaultbranchforcodecommitsource) | When enabled, the CodeCommit source action is using the default branch name 'main'. | 2.103.1 | (fix) |
| [@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction](#aws-cdkaws-cloudwatch-actionschangelambdapermissionlogicalidforlambdaaction) | When enabled, the logical ID of a Lambda permission for a Lambda action includes an alarm ID. | 2.124.0 | (fix) |
| [@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse](#aws-cdkaws-codepipelinecrossaccountkeysdefaultvaluetofalse) | Enables Pipeline to set the default value for crossAccountKeys to false. | 2.127.0 | (default) |
| [@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2](#aws-cdkaws-codepipelinedefaultpipelinetypetov2) | Enables Pipeline to set the default pipeline type to V2. | V2NEXT | (default) |

<!-- END table -->

Expand Down Expand Up @@ -120,7 +121,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-rds:preventRenderingDeprecatedCredentials": true,
"@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource": true,
"@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction": true,
"@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse": true
"@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse": true,
"@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2": true
}
}
```
Expand Down Expand Up @@ -1231,4 +1233,20 @@ construct, the construct automatically defaults the value of this property to fa
**Compatibility with old behavior:** Pass `crossAccountKeys: true` to `Pipeline` construct to restore the previous behavior.


### @aws-cdk/aws-codepipeline:defaultPipelineTypeToV2

*Enables Pipeline to set the default pipeline type to V2.* (default)

When this feature flag is enabled, and the `pipelineType` property is not provided in a `Pipeline`
construct, the construct automatically defaults the value of this property to `PipelineType.V2`.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| V2NEXT | `false` | `true` |

**Compatibility with old behavior:** Pass `pipelineType: PipelineType.V1` to `Pipeline` construct to restore the previous behavior.


<!-- END details -->
17 changes: 17 additions & 0 deletions packages/aws-cdk-lib/cx-api/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,20 @@ _cdk.json_
}
}
```

* `@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2`

Enables Pipeline to set the default pipeline type to V2.

When this feature flag is enabled, and the `pipelineType` property is not provided in a `Pipeline`
construct, the construct automatically defaults the value of this property to `PipelineType.V2`.

_cdk.json_

```json
{
"context": {
"@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2": true
}
}
```
14 changes: 14 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export const APPSYNC_ENABLE_USE_ARN_IDENTIFIER_SOURCE_API_ASSOCIATION = '@aws-cd
export const CODECOMMIT_SOURCE_ACTION_DEFAULT_BRANCH_NAME = '@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource';
export const LAMBDA_PERMISSION_LOGICAL_ID_FOR_LAMBDA_ACTION = '@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction';
export const CODEPIPELINE_CROSS_ACCOUNT_KEYS_DEFAULT_VALUE_TO_FALSE = '@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse';
export const CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2 = '@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1007,6 +1008,19 @@ export const FLAGS: Record<string, FlagInfo> = {
recommendedValue: true,
compatibilityWithOldBehaviorMd: 'Pass `crossAccountKeys: true` to `Pipeline` construct to restore the previous behavior.',
},

//////////////////////////////////////////////////////////////////////
[CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2]: {
type: FlagType.ApiDefault,
summary: 'Enables Pipeline to set the default pipeline type to V2.',
detailsMd: `
When this feature flag is enabled, and the \`pipelineType\` property is not provided in a \`Pipeline\`
construct, the construct automatically defaults the value of this property to \`PipelineType.V2\`.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
compatibilityWithOldBehaviorMd: 'Pass `pipelineType: PipelineType.V1` to `Pipeline` construct to restore the previous behavior.',
},
};

const CURRENT_MV = 'v2';
Expand Down
Loading