Skip to content

Commit 7788b10

Browse files
rongyamazonrongyamazon
committed
Styling fix for artifact uploader
1 parent b5bca48 commit 7788b10

File tree

10 files changed

+99
-122
lines changed

10 files changed

+99
-122
lines changed

‎src/artifactexporter/ArtifactExporter.ts‎

Lines changed: 10 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,13 @@
1-
import { readFileSync } from 'fs';
21
import { resolve, dirname, isAbsolute } from 'path';
3-
import { fileURLToPath, pathToFileURL } from 'url';
4-
import { load } from 'js-yaml';
5-
import { TopLevelSection, IntrinsicFunction } from '../context/ContextType';
6-
import { Document, DocumentType } from '../document/Document';
7-
import { detectDocumentType } from '../document/DocumentUtils';
2+
import { fileURLToPath } from 'url';
3+
import { TopLevelSection } from '../context/ContextType';
4+
import { normalizeIntrinsicFunctionAndCondition } from '../context/semantic/Intrinsics';
5+
import { DocumentType } from '../document/Document';
6+
import { parseDocumentContent } from '../document/DocumentUtils';
87
import { S3Service } from '../services/S3Service';
98
import { Artifact } from '../stacks/actions/StackActionRequestType';
109
import { isS3Url, RESOURCE_EXPORTER_MAP } from './ResourceExporters';
1110

12-
const INTRINSIC_FUNCTION_MAP = new Map<string, string>([
13-
['!Ref', IntrinsicFunction.Ref],
14-
['!GetAtt', IntrinsicFunction.GetAtt],
15-
['!Join', IntrinsicFunction.Join],
16-
['!Sub', IntrinsicFunction.Sub],
17-
['!Base64', IntrinsicFunction.Base64],
18-
['!GetAZs', IntrinsicFunction.GetAZs],
19-
['!ImportValue', IntrinsicFunction.ImportValue],
20-
['!Select', IntrinsicFunction.Select],
21-
['!Split', IntrinsicFunction.Split],
22-
['!FindInMap', IntrinsicFunction.FindInMap],
23-
['!Equals', IntrinsicFunction.Equals],
24-
['!If', IntrinsicFunction.If],
25-
['!Not', IntrinsicFunction.Not],
26-
['!And', IntrinsicFunction.And],
27-
['!Or', IntrinsicFunction.Or],
28-
['!Cidr', IntrinsicFunction.Cidr],
29-
['!Transform', IntrinsicFunction.Transform],
30-
['!Condition', 'Condition'],
31-
]);
32-
3311
export type ArtifactWithProperty = {
3412
resourceType: string;
3513
resourcePropertyDict: Record<string, unknown>;
@@ -39,30 +17,14 @@ export type ArtifactWithProperty = {
3917

4018
export class ArtifactExporter {
4119
private readonly templateDict: unknown;
42-
private readonly templateUri: string;
43-
private readonly templateType: DocumentType;
4420

4521
constructor(
4622
private readonly s3Service: S3Service,
47-
private readonly document?: Document,
48-
private readonly templateAbsPath?: string,
23+
private readonly templateType: DocumentType,
24+
private readonly templateUri: string,
25+
templateContent: string,
4926
) {
50-
if (this.document) {
51-
this.templateDict = this.document.getParsedDocumentContent();
52-
this.templateUri = this.document.uri;
53-
this.templateType = this.document.documentType;
54-
} else if (this.templateAbsPath) {
55-
const content = readFileSync(this.templateAbsPath, 'utf8');
56-
this.templateUri = pathToFileURL(this.templateAbsPath).href;
57-
this.templateType = detectDocumentType(this.templateUri, content).type;
58-
if (this.templateType === DocumentType.YAML) {
59-
this.templateDict = load(content);
60-
} else {
61-
this.templateDict = JSON.parse(content);
62-
}
63-
} else {
64-
throw new Error('Either document or absolutePath must be provided');
65-
}
27+
this.templateDict = parseDocumentContent(templateUri, templateContent);
6628
}
6729

6830
private getResourceMapWithArtifact(): Record<string, ArtifactWithProperty[]> {
@@ -148,7 +110,7 @@ export class ArtifactExporter {
148110
const objDict = obj as Record<string, unknown>;
149111

150112
for (const [key, value] of Object.entries(objDict)) {
151-
const newKey = INTRINSIC_FUNCTION_MAP.get(key) ?? key;
113+
const newKey = normalizeIntrinsicFunctionAndCondition(key);
152114
result[newKey] = this.convertIntrinsicFunctionKeys(value);
153115
}
154116

‎src/artifactexporter/ResourceExporters.ts‎

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,15 @@
1-
import {
2-
existsSync,
3-
mkdtempSync,
4-
copyFileSync,
5-
rmSync,
6-
createWriteStream,
7-
statSync,
8-
openSync,
9-
readSync,
10-
closeSync,
11-
} from 'fs';
1+
import { existsSync, mkdtempSync, copyFileSync, rmSync, createWriteStream, statSync, readFileSync } from 'fs';
122
import { tmpdir } from 'os';
133
import path, { join, basename } from 'path';
4+
import { pathToFileURL } from 'url';
145
import archiver from 'archiver';
156
import { dump } from 'js-yaml';
7+
import { detectDocumentType } from '../document/DocumentUtils';
168
import { S3Service } from '../services/S3Service';
179
import { ArtifactExporter } from './ArtifactExporter';
1810

1911
export function isS3Url(url: string): boolean {
20-
return typeof url === 'string' && /^s3:\/\/[^/]+\/.+/.test(url);
12+
return /^s3:\/\/[^/]+\/.+/.test(url);
2113
}
2214

2315
export function isLocalFile(filePath: string): boolean {
@@ -31,26 +23,9 @@ function isLocalFolder(path: string): boolean {
3123
function isArchiveFile(filePath: string) {
3224
// Quick extension check
3325
const ext = path.extname(filePath).toLowerCase();
34-
const archiveExts = ['.zip', '.rar', '.7z', '.tar', '.gz', '.tgz'];
35-
36-
if (!archiveExts.includes(ext)) return false;
37-
38-
// Verify with magic numbers
39-
try {
40-
const fd = openSync(filePath, 'r');
41-
const buffer = Buffer.alloc(8);
42-
readSync(fd, buffer, 0, 8, 0);
43-
closeSync(fd);
44-
45-
return (
46-
(buffer[0] === 0x50 && buffer[1] === 0x4b) || // ZIP
47-
buffer.toString('ascii', 0, 4) === 'Rar!' || // RAR
48-
(buffer[0] === 0x37 && buffer[1] === 0x7a) || // 7Z
49-
(buffer[0] === 0x1f && buffer[1] === 0x8b) // GZIP
50-
);
51-
} catch {
52-
return false;
53-
}
26+
const archiveExts = ['.zip', '.rar', '.7z', '.tar', '.gz', '.tgz', '.zst', '.war'];
27+
28+
return archiveExts.includes(ext);
5429
}
5530

5631
function copyToTempDir(filePath: string): string {
@@ -278,7 +253,11 @@ export class CloudFormationStackResource extends Resource {
278253
throw new Error(`Invalid template path: ${templateAbsPath}`);
279254
}
280255

281-
const template = new ArtifactExporter(this.s3Service, undefined, templateAbsPath);
256+
const templateUri = pathToFileURL(templateAbsPath).href;
257+
const content = readFileSync(templateAbsPath, 'utf8');
258+
const templateType = detectDocumentType(templateUri, content).type;
259+
260+
const template = new ArtifactExporter(this.s3Service, templateType, templateUri, content);
282261
const exportedTemplateDict = await template.export(bucketName, s3KeyPrefix);
283262
const exportedTemplateStr = dump(exportedTemplateDict);
284263

‎src/context/semantic/Intrinsics.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,7 @@ export function normalizeIntrinsicFunction(text: string): string {
55
}
66
return text;
77
}
8+
9+
export function normalizeIntrinsicFunctionAndCondition(text: string): string {
10+
return text === '!Condition' ? 'Condition' : normalizeIntrinsicFunction(text);
11+
}

‎src/document/Document.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { DefaultSettings } from '../settings/Settings';
44
import { LoggerFactory } from '../telemetry/LoggerFactory';
55
import { DocumentMetadata } from './DocumentProtocol';
66
import { detectDocumentType, uriToPath } from './DocumentUtils';
7-
import { parseValidYaml } from './YamlParser';
87

98
export class Document {
109
private readonly log = LoggerFactory.getLogger(Document);

‎src/document/DocumentUtils.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { extname, parse } from 'path';
22
import { Edit, Point } from 'tree-sitter';
33
import { DocumentType } from './Document';
4+
import { parseValidYaml } from './YamlParser';
45

56
export function getIndexFromPoint(content: string, point: Point): number {
67
const contentInLines = content.split('\n');
@@ -91,3 +92,11 @@ export function detectDocumentType(uri: string, content: string): { extension: s
9192
export function uriToPath(uri: string) {
9293
return parse(uri);
9394
}
95+
96+
export function parseDocumentContent(uri: string, content: string): unknown {
97+
const documentType = detectDocumentType(uri, content).type;
98+
if (documentType === DocumentType.JSON) {
99+
return JSON.parse(content);
100+
}
101+
return parseValidYaml(content);
102+
}

‎src/handlers/StackHandler.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,12 @@ export function getTemplateArtifactsHandler(
9595
throw new Error(`Cannot retrieve file with uri: ${params}`);
9696
}
9797

98-
const template = new ArtifactExporter(components.s3Service, document);
98+
const template = new ArtifactExporter(
99+
components.s3Service,
100+
document.documentType,
101+
document.uri,
102+
document.contents(),
103+
);
99104
const artifacts = template.getTemplateArtifacts();
100105
return { artifacts };
101106
} catch (error) {

‎src/stacks/actions/StackActionOperations.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export async function processChangeSet(
9494
try {
9595
if (params.s3Bucket) {
9696
const s3KeyPrefix = params.s3Key ? params.s3Key.slice(0, params.s3Key.lastIndexOf('/')) : undefined;
97-
const template = new ArtifactExporter(s3Service, document);
97+
const template = new ArtifactExporter(s3Service, document.documentType, document.uri, document.contents());
9898

9999
const exportedTemplate = await template.export(params.s3Bucket, s3KeyPrefix);
100100

‎tst/unit/artifactexporter/ArtifactExporter.test.ts‎

Lines changed: 15 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,41 @@
1-
import { readFileSync } from 'fs';
2-
import { load } from 'js-yaml';
31
import { describe, it, expect, vi, beforeEach } from 'vitest';
42
import { ArtifactExporter } from '../../../src/artifactexporter/ArtifactExporter';
5-
import { Document, DocumentType } from '../../../src/document/Document';
3+
import { DocumentType } from '../../../src/document/Document';
64
import { S3Service } from '../../../src/services/S3Service';
75

86
vi.mock('../../../src/services/S3Service');
9-
vi.mock('fs', () => ({
10-
readFileSync: vi.fn(),
11-
}));
12-
vi.mock('js-yaml', () => ({
13-
load: vi.fn(),
14-
Type: vi.fn(),
15-
DEFAULT_SCHEMA: {
16-
extend: vi.fn().mockReturnValue({}),
17-
},
18-
}));
197

208
describe('ArtifactExporter', () => {
219
let mockS3Service: S3Service;
22-
let mockDocument: Document;
2310

2411
beforeEach(() => {
2512
vi.clearAllMocks();
2613
mockS3Service = {
2714
putObjectContent: vi.fn(),
2815
putObject: vi.fn(),
2916
} as any;
30-
mockDocument = {
31-
getParsedDocumentContent: vi.fn().mockReturnValue({ Resources: {} }),
32-
documentType: DocumentType.YAML,
33-
} as any;
3417
});
3518

3619
describe('ArtifactExporter', () => {
37-
it('should create template with document', () => {
38-
const template = new ArtifactExporter(mockS3Service, mockDocument);
20+
it('should create template with valid parameters', () => {
21+
const template = new ArtifactExporter(
22+
mockS3Service,
23+
DocumentType.YAML,
24+
'file:///path/to/template.yaml',
25+
'Resources:\n Bucket:\n Type: AWS::S3::Bucket',
26+
);
3927
expect(template).toBeDefined();
4028
});
4129

42-
it('should create template with absolute path', () => {
43-
vi.mocked(readFileSync).mockReturnValue('Resources:\n Bucket:\n Type: AWS::S3::Bucket');
44-
vi.mocked(load).mockReturnValue({ Resources: { Bucket: { Type: 'AWS::S3::Bucket' } } });
45-
46-
const template = new ArtifactExporter(mockS3Service, undefined, '/path/to/template.yaml');
47-
expect(template).toBeDefined();
48-
expect(readFileSync).toHaveBeenCalledWith('/path/to/template.yaml', 'utf8');
49-
});
50-
51-
it('should throw error when neither document nor path provided', () => {
52-
expect(() => {
53-
new ArtifactExporter(mockS3Service);
54-
}).toThrow('Either document or absolutePath must be provided');
55-
});
56-
5730
it('should export template', async () => {
58-
const template = new ArtifactExporter(mockS3Service, mockDocument);
31+
const template = new ArtifactExporter(
32+
mockS3Service,
33+
DocumentType.YAML,
34+
'file:///path/to/template.yaml',
35+
'Resources:\n Bucket:\n Type: AWS::S3::Bucket',
36+
);
5937
const result = await template.export('test-bucket');
60-
expect(result).toEqual({ Resources: {} });
38+
expect(result).toBeDefined();
6139
});
6240
});
6341
});

‎tst/unit/context/semantic/Intrinsics.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { describe, it, expect } from 'vitest';
2+
import {
3+
normalizeIntrinsicFunction,
4+
normalizeIntrinsicFunctionAndCondition,
5+
} from '../../../../src/context/semantic/Intrinsics';
6+
7+
describe('Intrinsics', () => {
8+
describe('normalizeIntrinsicFunction', () => {
9+
it('should normalize Ref shorthand', () => {
10+
expect(normalizeIntrinsicFunction('!Ref')).toBe('Ref');
11+
});
12+
13+
it('should normalize other function shorthands', () => {
14+
expect(normalizeIntrinsicFunction('!GetAtt')).toBe('Fn::GetAtt');
15+
expect(normalizeIntrinsicFunction('!Join')).toBe('Fn::Join');
16+
expect(normalizeIntrinsicFunction('!Sub')).toBe('Fn::Sub');
17+
});
18+
19+
it('should return unchanged for non-shorthand functions', () => {
20+
expect(normalizeIntrinsicFunction('Ref')).toBe('Ref');
21+
expect(normalizeIntrinsicFunction('Fn::GetAtt')).toBe('Fn::GetAtt');
22+
expect(normalizeIntrinsicFunction('someProperty')).toBe('someProperty');
23+
});
24+
});
25+
26+
describe('normalizeIntrinsicFunctionAndCondition', () => {
27+
it('should handle Condition specially', () => {
28+
expect(normalizeIntrinsicFunctionAndCondition('!Condition')).toBe('Condition');
29+
});
30+
31+
it('should delegate to normalizeIntrinsicFunction for other cases', () => {
32+
expect(normalizeIntrinsicFunctionAndCondition('!Ref')).toBe('Ref');
33+
expect(normalizeIntrinsicFunctionAndCondition('!GetAtt')).toBe('Fn::GetAtt');
34+
expect(normalizeIntrinsicFunctionAndCondition('someProperty')).toBe('someProperty');
35+
});
36+
});
37+
});

‎tst/unit/handlers/StackHandler.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,11 @@ describe('StackActionHandler', () => {
164164
describe('getTemplateArtifactsHandler', () => {
165165
it('should return artifacts when document is available', () => {
166166
const templateUri: TemplateUri = 'test://template.yaml';
167-
const mockDocument = { uri: templateUri } as unknown as Document;
167+
const mockDocument = {
168+
uri: templateUri,
169+
documentType: 'yaml',
170+
contents: vi.fn().mockReturnValue('Resources:\n Bucket:\n Type: AWS::S3::Bucket'),
171+
} as unknown as Document;
168172
const mockArtifacts = [
169173
{ resourceType: 'AWS::Lambda::Function', filePath: './src/handler.js' },
170174
{ resourceType: 'AWS::S3::Bucket', filePath: './assets/data.json' },

0 commit comments

Comments
 (0)