feat: image maturity indicator and image update indicators#181
feat: image maturity indicator and image update indicators#181
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis update introduces a comprehensive image maturity checking and indicator system. It adds backend logic for polling and checking Docker image maturity, exposes maturity data via new API endpoints, and updates the UI to display maturity status and update availability for each image. Supporting types, error handling, and store management are also added or extended. Changes
Assessment against linked issues
Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
src/lib/types/errors.type.ts (1)
3-16:⚠️ Potential issueEnum diverges from legacy
errors.ts– expect import collisions
ApiErrorCodehere re-defines several constants that already exist insrc/lib/types/errors.ts(e.g.VALIDATION_ERROR,NOT_FOUND) but also renamesUNAUTHENTICATED→UNAUTHORIZED.
Because other modules still import from botherrors.tsand this new file (seeimage-service.ts), the two enums will never be===equal andswitch/ifcomparisons will silently fail.Action:
- Keep a single source-of-truth for the enum (delete the obsolete file or re-export from one place).
- Preserve the original constant names to avoid breaking API contracts (
UNAUTHENTICATEDvsUNAUTHORIZED).- Add a unit-test that verifies
Object.values(ApiErrorCode)contains no duplicates.
🧹 Nitpick comments (10)
src/routes/api/settings/+server.ts (1)
118-129: LGTM: Added proper validation for the maturity threshold setting.The validation ensures the maturity threshold is between 1 and 365 days, which is a reasonable range. The parsing and error handling are consistent with the validation patterns used for other settings.
However, there's no code to restart the maturity polling scheduler when settings are updated, unlike how the auto-update scheduler is handled in lines 161-164.
Consider adding maturity polling scheduler reinitialization on settings update:
// Re-initialize services that depend on settings await tryCatch(initComposeService()); await tryCatch(stopAutoUpdateScheduler()); if (newSettingsData.autoUpdate) { await tryCatch(initAutoUpdateScheduler()); } +// Reset maturity polling scheduler to apply new settings +await tryCatch(initMaturityPollingScheduler());src/lib/types/docker/image.type.ts (1)
13-18: LGTM: Well-defined ImageMaturity interface.The interface properly represents maturity-related metadata for Docker images with clear property types. The use of a union type for the status field provides good type safety.
Consider adding documentation comments to improve code maintainability:
+/** + * Represents maturity and update information for a Docker image + */ export interface ImageMaturity { version: string; date: string; status: 'Matured' | 'Not Matured' | 'Unknown'; updatesAvailable: boolean; }src/routes/settings/tabs/app-settings.svelte (1)
24-38:$stateinitialisation runs before the store is populatedBecause the
$settingsStoresubscription happens asynchronously,
$settingsStore.stacksDirectorymay still beundefinedwhen these states are
created, causing the input to flash empty before the$effectcopies the real
settings.To avoid the flicker initialise from
data.settings(already available) or
lazy-initialise inside the$effectonce the store is ready.src/lib/components/form/form-input.svelte (1)
34-34:idgeneration is not guaranteed unique or safe
label?.toLowerCase().replace(/ /g, '-')collides when:
- Two different forms use the same label.
- The label contains characters that are not valid in an HTML
id.- The label is changed dynamically.
Generate a stable unique ID once instead:
-const id = label?.toLowerCase().replace(/ /g, '-'); +const id = (label + ? label.toLowerCase().replace(/[^a-z0-9-_:.]/g, '-') + : '') + '-' + crypto.randomUUID();src/lib/stores/maturity-store.ts (2)
37-42: Race-condition risk when several maturity checks run in parallelIf two concurrent checks overlap:
setMaturityChecking(true)→isChecking = true- First check finishes →
setMaturityChecking(false)sets
lastChecked = new Date()- Second check still running, but
isCheckingis alreadyfalse; when it
finishes it will overwritelastCheckedwith an older timestamp.Consider using a counter or a request-ID stack so
isCheckingonly resets when
all running checks are done, or pass the timestamp as an argument.
18-34:updateImageMaturitymutates through a shallow copy – prefer immutable mergeAlthough you copy
maturityData, the outerstateobject is still reused. To
avoid accidental reactive-comparison issues adopt a fully-immutable update:return { - ...state, - maturityData: newData + ...state, + maturityData: { ...state.maturityData, ...(maturity ? { [imageId]: maturity } : {}) } };Or, if deleting, build the new object with
Object.fromEntries.src/routes/images/+page.svelte (2)
426-439: Icon dimensions break layout & accessibilityIcons are rendered in a
w-4 h-4container but usew-10 h-10, overflowing the cell and confusing screen-reader hit-targets.-<CircleCheck class="w-10 h-10 text-green-500" … /> +<CircleCheck class="w-4 h-4 text-green-500" … />Repeat for the other two variants.
584-635: Generated CSS arrows leak global scopeSelectors like
:global(.tooltip-with-arrow[data-side='left']::before)are fine, but the un-scoped variables--popoverand--borderassume Tailwind/CM CSS variables exist everywhere.If this component is consumed in isolation (e.g. storybook) the background may render transparent.
Consider fallback colors:background-color: var(--popover, #fff); border: 1px solid var(--border, #d1d5db);src/lib/services/docker/image-service.ts (2)
67-86: Sequentialawaitloop is IO-bound hot-spot
runMaturityChecksawaits eachcheckImageMaturitythen sleeps 200 ms. On servers with hundreds of images this will run for minutes.Reuse the concurrency pool approach suggested for the frontend, but keep a low limit (e.g. 5) to respect registry rate limits. Logically identical update counts can then be aggregated after
Promise.all.
399-474: Registry API fetch lacks timeout / abort handlingA slow or hung registry call can stall the whole maturity check. Consider an
AbortControllerwith a sensible timeout (10-15 s) and surface aRegistryApiTimeoutError.const ctrl = new AbortController(); const t = setTimeout(() => ctrl.abort(), 15_000); try { const res = await fetch(tagsUrl, { headers, signal: ctrl.signal }); … } finally { clearTimeout(t); }🧰 Tools
🪛 Biome (1.9.4)
[error] 421-421: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/hooks.server.ts(1 hunks)src/lib/components/form/form-input.svelte(1 hunks)src/lib/components/ui/tooltip/index.ts(1 hunks)src/lib/components/ui/tooltip/tooltip-content.svelte(1 hunks)src/lib/services/api/image-api-service.ts(1 hunks)src/lib/services/docker/image-service.ts(4 hunks)src/lib/services/settings-service.ts(1 hunks)src/lib/stores/maturity-store.ts(1 hunks)src/lib/stores/settings-store.ts(1 hunks)src/lib/types/docker/image.type.ts(1 hunks)src/lib/types/errors.type.ts(2 hunks)src/lib/types/form.type.ts(1 hunks)src/lib/types/settings.type.ts(1 hunks)src/routes/api/images/[id]/maturity/+server.ts(1 hunks)src/routes/api/settings/+server.ts(1 hunks)src/routes/images/+page.server.ts(2 hunks)src/routes/images/+page.svelte(7 hunks)src/routes/images/pull-image-dialog.svelte(0 hunks)src/routes/settings/tabs/app-settings.svelte(2 hunks)
💤 Files with no reviewable changes (1)
- src/routes/images/pull-image-dialog.svelte
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/routes/api/settings/+server.ts (2)
src/lib/types/errors.type.ts (1)
ApiErrorResponse(18-24)src/lib/types/errors.ts (1)
ApiErrorCode(4-17)
src/hooks.server.ts (1)
src/lib/services/docker/image-service.ts (1)
initMaturityPollingScheduler(15-38)
src/routes/images/+page.server.ts (1)
src/lib/services/docker/image-service.ts (1)
checkImageMaturity(285-328)
src/lib/stores/maturity-store.ts (1)
src/lib/types/docker/image.type.ts (1)
ImageMaturity(13-18)
src/lib/types/errors.type.ts (1)
src/lib/types/errors.ts (1)
ApiErrorCode(4-17)
🪛 Biome (1.9.4)
src/lib/services/docker/image-service.ts
[error] 421-421: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 605-605: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 626-626: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 707-707: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (21)
src/lib/types/form.type.ts (1)
1-7: Well-structured generic form input typeThis is a clean implementation of a generic form input state type that covers the essential aspects of form input management: value, validation state, interaction tracking, and error handling. The generic approach allows for type-safe usage across different input types.
The separation of
error(string | null) anderrors(string[]) provides flexibility for both simple and complex validation scenarios.src/lib/services/settings-service.ts (1)
26-27: Adding maturity threshold setting with appropriate default valueThe addition of
maturityThresholdDays: 30as a default setting is appropriate for implementing the image maturity indicator feature. The 30-day threshold is a reasonable default that balances caution with practicality for container image maturity assessment.src/lib/types/settings.type.ts (1)
48-48: Proper type definition for maturity thresholdAdding the
maturityThresholdDaysproperty to theSettingsinterface maintains type safety throughout the application. Making it a required property (not optional) is the right choice since it has a default value in the settings service.src/lib/services/api/image-api-service.ts (1)
20-23: Consistent API method implementation for maturity checkingThe
checkMaturitymethod follows the established patterns in the class, making a POST request to the appropriate endpoint and returning the response data. This implementation maintains consistency with other API methods in the service.src/lib/stores/settings-store.ts (1)
42-43: LGTM: Added new maturity threshold setting.The new
maturityThresholdDaysproperty with a default value of 30 is a good addition to support the image maturity checking feature.src/hooks.server.ts (2)
9-9: LGTM: Import for new maturity polling service.Good addition of the import for the maturity polling scheduler initialization function.
16-16: LGTM: Initialized maturity polling scheduler.Correctly added the maturity polling scheduler initialization to the parallel startup sequence alongside other critical services.
src/lib/types/docker/image.type.ts (1)
22-22: LGTM: Properly enhanced image info with maturity data.Good addition of the optional maturity property to the EnhancedImageInfo type to support the new image maturity feature.
src/routes/images/+page.server.ts (3)
2-2: Import updated to include maturity checking function.The import statement now correctly includes the
checkImageMaturityfunction that will be used to enhance image data.
21-31: Well-implemented maturity check with proper error handling.The maturity check implementation is robust as it:
- Only checks images with valid repository and tag values
- Uses try-catch to prevent maturity errors from affecting the main flow
- Logs errors appropriately
- Maintains backward compatibility with existing code
This approach ensures the application remains stable even if maturity checks fail for some images.
35-36: Clean integration of maturity data with existing image properties.The maturity information is properly added to the enhanced image object alongside the existing
inUseproperty without disrupting the original structure.src/routes/api/images/[id]/maturity/+server.ts (4)
1-7: Well-structured imports for the new API endpoint.The imports include all necessary dependencies for JSON responses, type definitions, error handling, and the core maturity checking functionality.
8-19: Strong parameter validation with appropriate error responses.The endpoint properly validates the image ID parameter and returns a well-structured 400 Bad Request response when validation fails, following API best practices.
20-32: Comprehensive error handling with detailed logging.The implementation:
- Uses the
tryCatchutility for consistent error handling- Logs errors with appropriate context
- Extracts Docker-specific error messages for better user experience
- Returns typed error responses with status code 500
- Includes detailed error information for debugging
This approach ensures robust error handling and good developer/user experience.
34-38: Clean success response format.The success response follows a consistent pattern with a success flag and the result data, making it easy for clients to process.
src/lib/components/ui/tooltip/tooltip-content.svelte (2)
1-11: Well-structured component with typed props.The component correctly:
- Imports the necessary Tooltip primitive
- Uses utility function for class merging
- Defines typed props with sensible defaults
- Implements bindable reference
- Uses Svelte's modern syntax for props and bindings
This provides a type-safe and flexible foundation for tooltip content.
13-21: Comprehensive styling with Tailwind CSS.The component wraps the primitive with extensive Tailwind CSS classes that handle:
- Background and text colors
- Animations for showing/hiding
- Positioning transitions based on tooltip side
- Visual styling (border, shadow, padding)
- Proper z-index and overflow handling
This ensures consistent tooltip styling throughout the application while still allowing customization via the class prop.
src/lib/components/ui/tooltip/index.ts (2)
1-7: Clean extraction of tooltip components.The code correctly imports the tooltip primitive and local content component, then extracts individual components from the primitive for easier consumption.
8-18: Well-structured exports with alternative naming.The export structure:
- Provides the base components (Root, Trigger, Content, Provider)
- Also exports them with more descriptive names (Tooltip, TooltipTrigger, etc.)
- Uses clear commenting to separate the export groups
This gives consumers flexibility in how they import and use the components while maintaining a consistent interface.
src/lib/services/docker/image-service.ts (2)
90-98: Browser-onlywindowreference in server context
runMaturityChecksexecutes on the server yet referenceswindowbehind atypeofguard to dispatch an event. This block will never execute in Node, so consumers on the client will not receive an update.If you intend to push results to the UI, emit a custom event over a websocket or use SvelteKit’s event-stream endpoint instead.
817-848:findNewerVersionsOfSameTag– inaccurate when tags contain multiple numeric chunksCurrent regex grabs only the first numeric group, so
2023.04.01-rc1turns into2023.
Consider usingsemverparsing or at least splitting on non-digit boundaries to compare full versions.
1 participant
fixes: #180
fixes: #149
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores