Skip to content

Commit a465577

Browse files
committed
feat(cloudformation): Add latest parameter validation and drift detection fixes
- Allow empty string in parameter validation (#175) - Enable drift columns with change set level flag (#174)
2 parents c046b42 + cbf2daf commit a465577

File tree

7 files changed

+65
-19
lines changed

7 files changed

+65
-19
lines changed

packages/core/src/awsService/cloudformation/commands/cfnCommands.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,14 @@ export function viewChangeSetCommand(client: LanguageClient, diffProvider: DiffW
142142
stackName: params.stackName,
143143
})
144144

145-
void diffProvider.updateData(params.stackName, describeChangeSetResult.changes, params.changeSetName, true)
145+
void diffProvider.updateData(
146+
params.stackName,
147+
describeChangeSetResult.changes,
148+
params.changeSetName,
149+
true,
150+
[],
151+
describeChangeSetResult.deploymentMode
152+
)
146153
void commands.executeCommand(commandKey('diff.focus'))
147154
} catch (error) {
148155
await handleLspError(error, 'Error viewing change set')

packages/core/src/awsService/cloudformation/stacks/actions/stackActionInputValidation.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,6 @@ export function validateChangeSetName(value: string): string | undefined {
5757
}
5858

5959
export function validateParameterValue(input: string, param: TemplateParameter): string | undefined {
60-
if (!input && !param.Default) {
61-
return `Parameter ${param.name} is required`
62-
}
63-
6460
const actualValue = input ?? param.Default?.toString() ?? ''
6561

6662
// Handle CommaDelimitedList validation

packages/core/src/awsService/cloudformation/stacks/actions/stackActionRequestType.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ export type Failable = {
155155
export type DescribeValidationStatusResult = GetStackActionStatusResult &
156156
Failable & {
157157
ValidationDetails?: ValidationDetail[]
158+
deploymentMode?: DeploymentMode
158159
}
159160

160161
export type DescribeDeploymentStatusResult = GetStackActionStatusResult &
@@ -235,6 +236,7 @@ export type DescribeChangeSetParams = ChangeSetReference
235236
export type DescribeChangeSetResult = ChangeSetInfo & {
236237
stackName: string
237238
changes?: StackChange[]
239+
deploymentMode?: DeploymentMode
238240
}
239241

240242
export type StackInfo = {

packages/core/src/awsService/cloudformation/stacks/actions/validationWorkflow.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
ResourceToImport,
1313
ChangeSetOptionalFlags,
1414
ValidationDetail,
15+
DeploymentMode,
1516
} from './stackActionRequestType'
1617
import { LanguageClient } from 'vscode-languageclient/node'
1718
import { showErrorMessage, showValidationStarted, showValidationSuccess, showValidationFailure } from '../../ui/message'
@@ -114,7 +115,10 @@ export class Validation {
114115
if (validationResult.state === StackActionState.SUCCESSFUL) {
115116
showValidationSuccess(this.stackName)
116117

117-
this.showDiffView(describeValidationStatusResult.ValidationDetails ?? [])
118+
this.showDiffView(
119+
describeValidationStatusResult.ValidationDetails,
120+
describeValidationStatusResult.deploymentMode
121+
)
118122
} else {
119123
showValidationFailure(
120124
this.stackName,
@@ -152,13 +156,14 @@ export class Validation {
152156
}, 1000)
153157
}
154158

155-
private showDiffView(validationDetail?: ValidationDetail[]) {
159+
private showDiffView(validationDetail?: ValidationDetail[], deploymentMode?: DeploymentMode) {
156160
void this.diffProvider.updateData(
157161
this.stackName,
158162
this.changes,
159163
this.changeSetName,
160164
this.shouldEnableDeployment,
161-
validationDetail
165+
validationDetail,
166+
deploymentMode
162167
)
163168
void commands.executeCommand(commandKey('diff.focus'))
164169
}

packages/core/src/awsService/cloudformation/ui/diffWebviewProvider.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55

66
import { WebviewView, WebviewViewProvider, commands, Disposable } from 'vscode'
7-
import { StackChange, ValidationDetail } from '../stacks/actions/stackActionRequestType'
7+
import { DeploymentMode, StackChange, ValidationDetail } from '../stacks/actions/stackActionRequestType'
88
import { DiffViewHelper } from './diffViewHelper'
99
import { commandKey } from '../utils'
1010
import { StackViewCoordinator } from './stackViewCoordinator'
@@ -23,6 +23,7 @@ export class DiffWebviewProvider implements WebviewViewProvider, Disposable {
2323
private totalPages: number = 0
2424
private readonly disposables: Disposable[] = []
2525
private validationDetail: ValidationDetail[] = []
26+
private deploymentMode?: DeploymentMode
2627

2728
constructor(private readonly coordinator: StackViewCoordinator) {
2829
this.disposables.push(
@@ -44,7 +45,8 @@ export class DiffWebviewProvider implements WebviewViewProvider, Disposable {
4445
changes: StackChange[] = [],
4546
changeSetName?: string,
4647
enableDeployments = false,
47-
validationDetail?: ValidationDetail[]
48+
validationDetail?: ValidationDetail[],
49+
deploymentMode?: DeploymentMode
4850
) {
4951
this.stackName = stackName
5052
this.changes = changes
@@ -55,6 +57,7 @@ export class DiffWebviewProvider implements WebviewViewProvider, Disposable {
5557
if (validationDetail) {
5658
this.validationDetail = validationDetail
5759
}
60+
this.deploymentMode = deploymentMode
5861

5962
await this.coordinator.setChangeSetMode(stackName, true)
6063
if (this._view) {
@@ -139,15 +142,18 @@ export class DiffWebviewProvider implements WebviewViewProvider, Disposable {
139142
`
140143
}
141144

142-
// Check if any resource has drift
145+
// Check if REVERT_DRIFT change set or any resource has drift
143146
// TODO: adapt if we do real backend pagination
144-
const hasDrift = changes.some(
145-
(change) =>
146-
change.resourceChange?.resourceDriftStatus ||
147-
change.resourceChange?.details?.some(
148-
(detail) => detail.Target?.Drift || detail.Target?.LiveResourceDrift
149-
)
150-
)
147+
// TODO: remove resource fallback once server is passing deploymentMode
148+
const hasDrift =
149+
this.deploymentMode === DeploymentMode.REVERT_DRIFT ||
150+
changes.some(
151+
(change) =>
152+
change.resourceChange?.resourceDriftStatus ||
153+
change.resourceChange?.details?.some(
154+
(detail) => detail.Target?.Drift || detail.Target?.LiveResourceDrift
155+
)
156+
)
151157

152158
let tableHtml = `
153159
<table style="width: 100%; border-collapse: collapse; border: 1px solid var(--vscode-panel-border);">

packages/core/src/test/awsService/cloudformation/stacks/actions/stackActionInputValidation.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,15 @@ describe('validateParameterValue', function () {
6666
assert.strictEqual(validateParameterValue('2', param), undefined)
6767
assert.strictEqual(validateParameterValue('3', param), 'Value must be one of: 1, 2')
6868
})
69+
70+
it('should handle empty string values', function () {
71+
const param: TemplateParameter = {
72+
name: 'TestParam',
73+
Type: 'String',
74+
}
75+
76+
assert.strictEqual(validateParameterValue('', param), undefined)
77+
})
6978
})
7079

7180
describe('Number parameters', function () {

packages/core/src/test/awsService/cloudformation/ui/diffWebviewProvider.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66
import assert from 'assert'
77
import * as sinon from 'sinon'
88
import { DiffWebviewProvider } from '../../../../awsService/cloudformation/ui/diffWebviewProvider'
9-
import { StackChange } from '../../../../awsService/cloudformation/stacks/actions/stackActionRequestType'
9+
import {
10+
DeploymentMode,
11+
StackChange,
12+
} from '../../../../awsService/cloudformation/stacks/actions/stackActionRequestType'
1013

1114
describe('DiffWebviewProvider', function () {
1215
let sandbox: sinon.SinonSandbox
@@ -288,5 +291,23 @@ describe('DiffWebviewProvider', function () {
288291
assert.ok(html.includes('live-value'))
289292
assert.ok(html.includes('⚠️ Modified'))
290293
})
294+
295+
it('should show drift status column when deploymentMode is REVERT_DRIFT', function () {
296+
const changes: StackChange[] = [
297+
{
298+
resourceChange: {
299+
action: 'Modify',
300+
logicalResourceId: 'Resource',
301+
},
302+
},
303+
]
304+
305+
void provider.updateData('test-stack', changes, undefined, false, undefined, DeploymentMode.REVERT_DRIFT)
306+
const mockWebview = createMockWebview()
307+
provider.resolveWebviewView(mockWebview as any)
308+
const html = mockWebview.webview.html
309+
310+
assert.ok(html.includes('Drift Status'))
311+
})
291312
})
292313
})

0 commit comments

Comments
 (0)