Skip to content

Commit 1c1646e

Browse files
committed
Fix devfile init flow to match odo behavior
**Problem:** - materializeStarterProjects() called BEFORE parent resolution - nodejs-basic has no starter projects (they're in parent nodejs:2.2.0) - Result: starter materialization failed - Starter devfiles were loaded but NOT resolved (parents not merged) - Missing run commands from parent devfiles caused validation failures **Root cause:** 1. ODO doesn't download URIs during init - they're inlined later during dev/deploy 2. Starter project devfiles were replacing resolved registry devfiles WITHOUT being resolved themselves **Solution:** 1. Resolve parents FIRST to get starter projects from parent chain 2. THEN materialize starter (git clone) 3. Remove downloadResources mode entirely (URIs stay as-is in devfile) 4. Remove .git directory after cloning (matches odo, makes component compact) 5. Resolve starter devfiles when they override registry devfiles (FIX!) **Flow changes:** BEFORE: fetch → materialize starter (FAIL) → resolve + download URIs (404) AFTER: fetch → resolve (merge parents) → materialize starter → resolve starter devfile → URIs unchanged URIs will be inlined later during dev/deploy using inlineResources mode. **Files changed:** - src/devfile/init.ts: Fix parent resolution order, resolve starter devfiles, add .git removal - src/devfile/devfileResolver.ts: Remove downloadResources option, keep only inlineResources - src/devfile/devfileUriResolver.ts: NEW - URI resolution matching odo/devfile-library - src/devfile-registry/devfileRegistryWrapper.ts: Remove dead downloadStackExtraFiles code (~260 lines) - test/unit/devfile/devfileResolver.test.ts: Remove downloadResources tests - test/integration/devfileResolver.test.ts: Remove downloadResources test - test/unit/devfile/devfileUriResolver.test.ts: NEW - URI resolution tests Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com> Assisted-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 05b4fdf commit 1c1646e

12 files changed

Lines changed: 978 additions & 663 deletions

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,4 @@ dist/
1111
public/
1212
test-resources/
1313
.DS_Store
14-
14+
.claude/

src/devfile-registry/devfileRegistryWrapper.ts

Lines changed: 0 additions & 272 deletions
Original file line numberDiff line numberDiff line change
@@ -10,68 +10,6 @@ import { OdoPreference } from '../odo/odoPreference';
1010
import { ExecutionContext } from '../util/utils';
1111
import { DevfileData, DevfileInfo } from './devfileInfo';
1212

13-
// Extension ID is used for cache directory naming
14-
// Using a constant allows easy updates if extension name changes
15-
const EXTENSION_CACHE_DIR = 'vs-openshift-tools';
16-
17-
/**
18-
* Get the base directory for devfile registry cache.
19-
* Can be overridden via VSCODE_OPENSHIFT_CACHE_ROOT environment variable.
20-
* Default: ~/.local/state/vs-openshift-tools/devfile-registry-cache
21-
*
22-
* Returns null if cache directory cannot be created or accessed.
23-
*/
24-
async function getDevfileCacheBaseDir(): Promise<string | null> {
25-
const os = require('os');
26-
const path = require('path');
27-
28-
// Get cache root - can be overridden via environment variable
29-
// This allows the same env var to be used by tools.ts and other cache consumers
30-
const cacheRoot = process.env.VSCODE_OPENSHIFT_CACHE_ROOT ||
31-
path.join(os.homedir(), '.local', 'state');
32-
33-
const cacheDir = path.join(
34-
cacheRoot,
35-
EXTENSION_CACHE_DIR,
36-
'devfile-registry-cache'
37-
);
38-
39-
// Validate cache directory
40-
try {
41-
const fs = await import('fs/promises');
42-
43-
// Try to create the directory if it doesn't exist
44-
await fs.mkdir(cacheDir, { recursive: true });
45-
46-
// Check if it's actually a directory
47-
const stats = await fs.stat(cacheDir);
48-
if (!stats.isDirectory()) {
49-
// Path exists but is a file, not a directory - try fallback
50-
const fallback = path.join(os.tmpdir(), EXTENSION_CACHE_DIR, 'devfile-registry-cache');
51-
await fs.mkdir(fallback, { recursive: true });
52-
return fallback;
53-
}
54-
55-
// Test write permissions by creating a temporary test file
56-
const testFile = path.join(cacheDir, '.write-test');
57-
await fs.writeFile(testFile, '', 'utf-8');
58-
await fs.unlink(testFile);
59-
60-
return cacheDir;
61-
} catch (error) {
62-
// Try fallback to temp directory
63-
try {
64-
const fallback = path.join(os.tmpdir(), EXTENSION_CACHE_DIR, 'devfile-registry-cache');
65-
const fs = await import('fs/promises');
66-
await fs.mkdir(fallback, { recursive: true });
67-
return fallback;
68-
} catch {
69-
// No cache available - return null
70-
return null;
71-
}
72-
}
73-
}
74-
7513
export const DEVFILE_VERSION_LATEST: string = 'latest';
7614

7715
/**
@@ -81,7 +19,6 @@ export class DevfileRegistry {
8119
private static instance: DevfileRegistry;
8220

8321
private executionContext: ExecutionContext = new ExecutionContext();
84-
private cacheWarningShown: boolean = false;
8522

8623
public static get Instance(): DevfileRegistry {
8724
if (!DevfileRegistry.instance) {
@@ -244,215 +181,6 @@ export class DevfileRegistry {
244181
});
245182
}
246183

247-
/**
248-
* Download extra stack files (Dockerfile, Kubernetes manifests, etc.) referenced in the devfile.
249-
* These files are stored in the devfile registry GitHub repository alongside the devfile.
250-
*
251-
* Files are cached in the filesystem to avoid repeated downloads.
252-
*
253-
* @param registryUrl Devfile Registry URL
254-
* @param stackName Stack name (e.g., 'go', 'nodejs')
255-
* @param version Stack version (e.g., '2.6.0')
256-
* @param resolvedDevfile The resolved devfile (with parents merged)
257-
* @param projectPath The project path where files should be written
258-
* @returns Promise that resolves when all files are downloaded and written
259-
* @throws Error if any file download or write fails
260-
*/
261-
public async downloadStackExtraFiles(
262-
registryUrl: string,
263-
stackName: string,
264-
version: string,
265-
resolvedDevfile: any,
266-
projectPath: string
267-
): Promise<void> {
268-
const uriPaths = this.extractUriPaths(resolvedDevfile);
269-
270-
if (uriPaths.length === 0) {
271-
return;
272-
}
273-
274-
// Download and write each file, collecting any errors
275-
const results = await Promise.allSettled(
276-
uriPaths.map(async (uriPath) => {
277-
const content = await this.getStackFile(registryUrl, stackName, version, uriPath);
278-
if (!content) {
279-
throw new Error(`Failed to download ${uriPath} from ${stackName}:${version}`);
280-
}
281-
await this.writeStackFile(projectPath, uriPath, content);
282-
return uriPath;
283-
})
284-
);
285-
286-
// Check for failures and report them
287-
const failures = results.filter((r): r is PromiseRejectedResult => r.status === 'rejected');
288-
if (failures.length > 0) {
289-
const errorMessages = failures.map(f => f.reason?.message || String(f.reason)).join('; ');
290-
throw new Error(`Failed to download ${failures.length} file(s): ${errorMessages}`);
291-
}
292-
}
293-
294-
/**
295-
* Extract all URI paths from devfile components (dockerfile.uri, kubernetes.uri, etc.)
296-
*/
297-
private extractUriPaths(devfile: any): string[] {
298-
const uriPaths: string[] = [];
299-
300-
if (!devfile.components) {
301-
return uriPaths;
302-
}
303-
304-
for (const component of devfile.components) {
305-
// Check for dockerfile uri in image components
306-
if (component.image?.dockerfile?.uri) {
307-
uriPaths.push(component.image.dockerfile.uri);
308-
}
309-
310-
// Check for kubernetes/openshift uri
311-
if (component.kubernetes?.uri) {
312-
uriPaths.push(component.kubernetes.uri);
313-
}
314-
315-
if (component.openshift?.uri) {
316-
uriPaths.push(component.openshift.uri);
317-
}
318-
}
319-
320-
return uriPaths;
321-
}
322-
323-
/**
324-
* Get a stack file from cache or download from GitHub registry repository
325-
* @throws Error if download fails
326-
*/
327-
private async getStackFile(
328-
registryUrl: string,
329-
stackName: string,
330-
version: string,
331-
uriPath: string
332-
): Promise<string | null> {
333-
const cacheKey = ExecutionContext.key(`stack-file:${registryUrl}:${stackName}:${version}:${uriPath}`);
334-
335-
// Check memory cache first
336-
if (this.executionContext && this.executionContext.has(cacheKey)) {
337-
return this.executionContext.get(cacheKey);
338-
}
339-
340-
// Check filesystem cache
341-
const cachedContent = await this.getFromFileCache(stackName, version, uriPath);
342-
if (cachedContent) {
343-
this.executionContext.set(cacheKey, cachedContent);
344-
return cachedContent;
345-
}
346-
347-
// Download from GitHub registry repository
348-
try {
349-
const content = await this.downloadFromRegistryRepo(stackName, version, uriPath);
350-
351-
// Cache in memory and filesystem
352-
this.executionContext.set(cacheKey, content);
353-
await this.saveToFileCache(stackName, version, uriPath, content);
354-
355-
return content;
356-
} catch (error) {
357-
const errorMessage = error instanceof Error ? error.message : String(error);
358-
throw new Error(`Failed to download ${uriPath}: ${errorMessage}`);
359-
}
360-
}
361-
362-
/**
363-
* Download a file from the devfile registry GitHub repository
364-
*/
365-
private async downloadFromRegistryRepo(
366-
stackName: string,
367-
version: string,
368-
uriPath: string
369-
): Promise<string> {
370-
const url = `https://raw.githubusercontent.com/devfile/registry/main/stacks/${stackName}/${version}/${uriPath}`;
371-
return DevfileRegistry._get(url);
372-
}
373-
374-
/**
375-
* Get cached file content from filesystem.
376-
* Cache location can be overridden via VSCODE_OPENSHIFT_CACHE_ROOT env var.
377-
* Default: ~/.local/state/vs-openshift-tools/devfile-registry-cache
378-
*/
379-
private async getFromFileCache(
380-
stackName: string,
381-
version: string,
382-
uriPath: string
383-
): Promise<string | null> {
384-
try {
385-
const fs = await import('fs/promises');
386-
const path = await import('path');
387-
388-
const cacheBaseDir = await getDevfileCacheBaseDir();
389-
if (!cacheBaseDir) {
390-
// Warn user once that caching is unavailable
391-
if (!this.cacheWarningShown) {
392-
this.cacheWarningShown = true;
393-
// eslint-disable-next-line no-console
394-
console.warn('[vscode-openshift-tools] Devfile registry cache is unavailable. Files will be downloaded without caching. Set VSCODE_OPENSHIFT_CACHE_ROOT to a writable directory to enable caching.');
395-
}
396-
return null;
397-
}
398-
399-
const cacheDir = path.join(cacheBaseDir, stackName, version);
400-
const filePath = path.join(cacheDir, uriPath);
401-
402-
const content = await fs.readFile(filePath, 'utf-8');
403-
return content;
404-
} catch {
405-
return null;
406-
}
407-
}
408-
409-
/**
410-
* Save file content to filesystem cache.
411-
* Cache location can be overridden via VSCODE_OPENSHIFT_CACHE_ROOT env var.
412-
* Default: ~/.local/state/vs-openshift-tools/devfile-registry-cache
413-
*/
414-
private async saveToFileCache(
415-
stackName: string,
416-
version: string,
417-
uriPath: string,
418-
content: string
419-
): Promise<void> {
420-
try {
421-
const fs = await import('fs/promises');
422-
const path = await import('path');
423-
424-
const cacheBaseDir = await getDevfileCacheBaseDir();
425-
if (!cacheBaseDir) {
426-
// No cache available - silently skip (warning already shown in getFromFileCache)
427-
return;
428-
}
429-
430-
const cacheDir = path.join(cacheBaseDir, stackName, version);
431-
const filePath = path.join(cacheDir, uriPath);
432-
433-
await fs.mkdir(path.dirname(filePath), { recursive: true });
434-
await fs.writeFile(filePath, content, 'utf-8');
435-
} catch {
436-
// Ignore cache write errors - cache is optional
437-
}
438-
}
439-
440-
/**
441-
* Write a stack file to the project directory
442-
*/
443-
private async writeStackFile(
444-
projectPath: string,
445-
uriPath: string,
446-
content: string
447-
): Promise<void> {
448-
const fs = await import('fs/promises');
449-
const path = await import('path');
450-
451-
const fullPath = path.join(projectPath, uriPath);
452-
await fs.mkdir(path.dirname(fullPath), { recursive: true });
453-
await fs.writeFile(fullPath, content, 'utf-8');
454-
}
455-
456184
/**
457185
* Clears the Execution context as well as all cached data
458186
*/

0 commit comments

Comments
 (0)