|
| 1 | +# Fix: Registry Resource Download |
| 2 | + |
| 3 | +## Problem |
| 4 | + |
| 5 | +Current implementation in `src/devfile-registry/devfileRegistryWrapper.ts` hard-codes GitHub URLs when downloading stack resources (kubernetes manifests, dockerfiles, etc.): |
| 6 | + |
| 7 | +```typescript |
| 8 | +// Line 370-371 in downloadFromRegistryRepo() |
| 9 | +const url = `https://raw.githubusercontent.com/devfile/registry/main/stacks/${stackName}/${version}/${uriPath}`; |
| 10 | +return DevfileRegistry._get(url); |
| 11 | +``` |
| 12 | + |
| 13 | +**This breaks for:** |
| 14 | +- Non-GitHub registries (https://registry.devfile.io, https://registry.stage.devfile.io) |
| 15 | +- Custom/private registries |
| 16 | +- GitLab, Bitbucket, or other hosting platforms |
| 17 | + |
| 18 | +**Test failure:** HTTP 429 (Too Many Requests) from GitHub's rate limiting. |
| 19 | + |
| 20 | +## Root Cause |
| 21 | + |
| 22 | +The code assumes ALL devfile registries use GitHub, which is incorrect. Each registry can be hosted anywhere. |
| 23 | + |
| 24 | +## How ODO Does It (Correct Approach) |
| 25 | + |
| 26 | +ODO delegates to the **devfile library (Go)** which implements proper URI resolution: |
| 27 | + |
| 28 | +### Key Code from `devfile/library/v2/pkg/devfile/parser/parse.go` |
| 29 | + |
| 30 | +When `ConvertKubernetesContentInUri = true`, the library calls `getKubernetesDefinitionFromUri()`: |
| 31 | + |
| 32 | +```go |
| 33 | +// Line 854-890 |
| 34 | +func getKubernetesDefinitionFromUri(uri string, d devfileCtx.DevfileCtx, devfileUtilsClient parserUtil.DevfileUtils) ([]byte, error) { |
| 35 | + absoluteURL := strings.HasPrefix(uri, "http://") || strings.HasPrefix(uri, "https://") |
| 36 | + |
| 37 | + // Case 1: Relative path on disk |
| 38 | + if !absoluteURL && d.GetAbsPath() != "" { |
| 39 | + newUri = path.Join(path.Dir(d.GetAbsPath()), uri) |
| 40 | + data = fs.ReadFile(newUri) |
| 41 | + } |
| 42 | + |
| 43 | + // Case 2: Relative path to a URL (KEY INSIGHT!) |
| 44 | + else if d.GetURL() != "" { |
| 45 | + u = parse(d.GetURL()) // e.g., "https://registry.devfile.io/devfiles/go/2.6.0" |
| 46 | + u.Path = path.Join(path.Dir(u.Path), uri) // "kubernetes/deploy.yaml" |
| 47 | + newUri = u.String() // "https://registry.devfile.io/devfiles/go/2.6.0/kubernetes/deploy.yaml" |
| 48 | + data = DownloadInMemory(newUri) |
| 49 | + } |
| 50 | + |
| 51 | + // Case 3: Absolute HTTP URL |
| 52 | + else if absoluteURL { |
| 53 | + newUri = uri |
| 54 | + data = DownloadInMemory(newUri) |
| 55 | + } |
| 56 | + |
| 57 | + return data |
| 58 | +} |
| 59 | +``` |
| 60 | + |
| 61 | +### The Key Insight |
| 62 | + |
| 63 | +**Relative URIs are resolved against the devfile's source URL!** |
| 64 | + |
| 65 | +If devfile came from: |
| 66 | +``` |
| 67 | +https://registry.devfile.io/devfiles/go/2.6.0 |
| 68 | +``` |
| 69 | + |
| 70 | +And has: |
| 71 | +```yaml |
| 72 | +components: |
| 73 | + - name: deploy |
| 74 | + kubernetes: |
| 75 | + uri: kubernetes/deploy.yaml |
| 76 | +``` |
| 77 | +
|
| 78 | +It resolves to: |
| 79 | +``` |
| 80 | +https://registry.devfile.io/devfiles/go/2.6.0/kubernetes/deploy.yaml |
| 81 | +``` |
| 82 | + |
| 83 | +**This IS the registry REST API pattern!** |
| 84 | + |
| 85 | +## Solution |
| 86 | + |
| 87 | +### Phase 1: Store Devfile Source URL |
| 88 | + |
| 89 | +When we fetch a devfile from a registry, store its source URL: |
| 90 | + |
| 91 | +```typescript |
| 92 | +// In DevfileResolver or similar |
| 93 | +interface ResolvedDevfile { |
| 94 | + devfile: Data; |
| 95 | + sourceUrl?: string; // NEW: Track where this came from |
| 96 | +} |
| 97 | +``` |
| 98 | + |
| 99 | +### Phase 2: Implement Proper URI Resolution |
| 100 | + |
| 101 | +Create a utility similar to the Go library: |
| 102 | + |
| 103 | +```typescript |
| 104 | +export class DevfileUriResolver { |
| 105 | + /** |
| 106 | + * Resolves a URI from a devfile component (kubernetes.uri, dockerfile.uri, etc.) |
| 107 | + * following the same logic as devfile/library Go implementation |
| 108 | + */ |
| 109 | + public static async resolveUri( |
| 110 | + uri: string, |
| 111 | + devfileSourceUrl?: string, |
| 112 | + devfileAbsPath?: string |
| 113 | + ): Promise<string> { |
| 114 | + const absoluteURL = uri.startsWith('http://') || uri.startsWith('https://'); |
| 115 | + |
| 116 | + // Case 1: Relative path on disk |
| 117 | + if (!absoluteURL && devfileAbsPath) { |
| 118 | + return path.join(path.dirname(devfileAbsPath), uri); |
| 119 | + } |
| 120 | + |
| 121 | + // Case 2: Relative path to a URL (resolve against registry endpoint) |
| 122 | + if (!absoluteURL && devfileSourceUrl) { |
| 123 | + const u = new URL(devfileSourceUrl); |
| 124 | + u.pathname = path.join(path.dirname(u.pathname), uri); |
| 125 | + return u.toString(); |
| 126 | + } |
| 127 | + |
| 128 | + // Case 3: Absolute URL |
| 129 | + if (absoluteURL) { |
| 130 | + return uri; |
| 131 | + } |
| 132 | + |
| 133 | + throw new Error(`Cannot resolve URI '${uri}': no devfile source context available`); |
| 134 | + } |
| 135 | + |
| 136 | + /** |
| 137 | + * Downloads content from a resolved URI |
| 138 | + */ |
| 139 | + public static async downloadContent(resolvedUri: string): Promise<string> { |
| 140 | + if (resolvedUri.startsWith('http://') || resolvedUri.startsWith('https://')) { |
| 141 | + // Use got library (via DownloadUtil) or simple http.get |
| 142 | + // Add retry logic with exponential backoff for 429 responses |
| 143 | + return this.downloadWithRetry(resolvedUri); |
| 144 | + } |
| 145 | + |
| 146 | + // Local file |
| 147 | + return fs.readFile(resolvedUri, 'utf-8'); |
| 148 | + } |
| 149 | + |
| 150 | + private static async downloadWithRetry(url: string, retries = 3): Promise<string> { |
| 151 | + for (let attempt = 0; attempt < retries; attempt++) { |
| 152 | + try { |
| 153 | + // Use got library or http.get |
| 154 | + return await this.httpGet(url); |
| 155 | + } catch (err) { |
| 156 | + if (err.statusCode === 429 && attempt < retries - 1) { |
| 157 | + // Exponential backoff: 1s, 2s, 4s |
| 158 | + await sleep(Math.pow(2, attempt) * 1000); |
| 159 | + continue; |
| 160 | + } |
| 161 | + throw err; |
| 162 | + } |
| 163 | + } |
| 164 | + } |
| 165 | +} |
| 166 | +``` |
| 167 | + |
| 168 | +### Phase 3: Update downloadStackExtraFiles() |
| 169 | + |
| 170 | +Replace the hard-coded GitHub logic: |
| 171 | + |
| 172 | +```typescript |
| 173 | +// In devfileRegistryWrapper.ts |
| 174 | +private async downloadFromRegistryRepo( |
| 175 | + registryUrl: string, |
| 176 | + stackName: string, |
| 177 | + version: string, |
| 178 | + uriPath: string |
| 179 | +): Promise<string> { |
| 180 | + // Build the proper registry endpoint URL |
| 181 | + const devfileSourceUrl = `${registryUrl}/devfiles/${stackName}/${version}`; |
| 182 | + |
| 183 | + // Resolve URI against registry endpoint |
| 184 | + const resolvedUrl = await DevfileUriResolver.resolveUri( |
| 185 | + uriPath, |
| 186 | + devfileSourceUrl |
| 187 | + ); |
| 188 | + |
| 189 | + // Download with retry logic |
| 190 | + return DevfileUriResolver.downloadContent(resolvedUrl); |
| 191 | +} |
| 192 | +``` |
| 193 | + |
| 194 | +### Phase 4: Handle Edge Cases |
| 195 | + |
| 196 | +1. **Registry doesn't serve files (404):** |
| 197 | + - Could fall back to GitHub for devfile.io registries (but log a warning) |
| 198 | + - Or fail with clear error message |
| 199 | + |
| 200 | +2. **Rate limiting (429):** |
| 201 | + - Retry with exponential backoff |
| 202 | + - Add caching (already implemented in `getStackFile()`) |
| 203 | + |
| 204 | +3. **Authentication:** |
| 205 | + - Support auth tokens like the Go library does |
| 206 | + |
| 207 | +## Testing Strategy |
| 208 | + |
| 209 | +### Unit Tests |
| 210 | + |
| 211 | +1. URI resolution: |
| 212 | + - Absolute HTTP URLs |
| 213 | + - Relative paths with devfile source URL |
| 214 | + - Relative paths with local file |
| 215 | + - Error cases (no context) |
| 216 | + |
| 217 | +2. Download with retry: |
| 218 | + - Successful download |
| 219 | + - 429 response → retry → success |
| 220 | + - Non-retriable error → fail immediately |
| 221 | + |
| 222 | +### Integration Tests |
| 223 | + |
| 224 | +1. Download from registry.devfile.io |
| 225 | +2. Download from registry.stage.devfile.io |
| 226 | +3. Handle 404 gracefully |
| 227 | +4. Handle 429 with retry |
| 228 | + |
| 229 | +## Files to Modify |
| 230 | + |
| 231 | +1. **src/devfile/devfileUriResolver.ts** (NEW) |
| 232 | + - URI resolution logic |
| 233 | + - Download with retry |
| 234 | + |
| 235 | +2. **src/devfile-registry/devfileRegistryWrapper.ts** |
| 236 | + - Replace `downloadFromRegistryRepo()` implementation |
| 237 | + - Store devfile source URL when fetching from registry |
| 238 | + |
| 239 | +3. **src/devfile/devfileResolver.ts** |
| 240 | + - Track source URL in resolved devfiles |
| 241 | + - Optionally: implement `ConvertKubernetesContentInUri`-like functionality |
| 242 | + |
| 243 | +4. **test/unit/devfile/devfileUriResolver.test.ts** (NEW) |
| 244 | + - URI resolution tests |
| 245 | + - Download retry tests |
| 246 | + |
| 247 | +## References |
| 248 | + |
| 249 | +- [Devfile Library Go Implementation](https://github.com/devfile/library) |
| 250 | +- [Devfile API Issue #46](https://github.com/devfile/api/issues/46#issuecomment-1093301551) |
| 251 | +- [Registry REST API](https://github.com/devfile/registry-support/blob/main/index/server/registry-REST-API.adoc) |
| 252 | +- ODO source: `/home/jeremy/projects/redhat-developer/odo/vendor/github.com/devfile/library/v2/pkg/devfile/parser/parse.go:847-890` |
| 253 | + |
| 254 | +## Timeline |
| 255 | + |
| 256 | +- URI resolver implementation: 2-3 hours |
| 257 | +- Update registry wrapper: 1 hour |
| 258 | +- Unit tests: 2 hours |
| 259 | +- Integration tests: 1 hour |
| 260 | +- Manual testing: 1 hour |
| 261 | + |
| 262 | +**Total: ~7-8 hours** |
0 commit comments