Skip to content

Image analyzer#337

Open
Parthiba-Hazra wants to merge 7 commits into
mainfrom
ph/image-analysis
Open

Image analyzer#337
Parthiba-Hazra wants to merge 7 commits into
mainfrom
ph/image-analysis

Conversation

@Parthiba-Hazra
Copy link
Copy Markdown
Collaborator

@Parthiba-Hazra Parthiba-Hazra commented Mar 24, 2026

[Title]

📚 Description of Changes

Provide an overview of your changes and why they’re needed. Link to any related issues (e.g., "Fixes #123"). If your PR fixes a bug, resolves a feature request, or updates documentation, please explain how.

  • What Changed:
    (Describe the modifications, additions, or removals.)

  • Why This Change:
    (Explain the problem this PR addresses or the improvement it provides.)

  • Affected Components:
    (Which component does this change affect? - put x for all components)

  • Compose

  • K8s

  • Other (please specify)

❓ Motivation and Context

Why is this change required? What problem does it solve?

  • Context:
    (Provide background information or link to related discussions/issues.)

  • Relevant Tasks/Issues:
    (e.g., Fixes: #GitHub Issue)

🔍 Types of Changes

Indicate which type of changes your code introduces (check all that apply):

  • BUGFIX: Non-breaking fix for an issue.
  • NEW FEATURE: Non-breaking addition of functionality.
  • BREAKING CHANGE: Fix or feature that causes existing functionality to not work as expected.
  • ENHANCEMENT: Improvement to existing functionality.
  • CHORE: Changes that do not affect production (e.g., documentation, build tooling, CI).

🔬 QA / Verification Steps

Describe the steps a reviewer should take to verify your changes:

  1. (Step one: e.g., "Run make test to verify all tests pass.")
  2. (Step two: e.g., "Deploy to a Kind cluster with make create-kind && make deploy.")
  3. (Additional steps as needed.)

✅ Global Checklist

Please check all boxes that apply:

  • I have read and followed the CONTRIBUTING guidelines.
  • My code follows the code style of this project.
  • I have updated the documentation as needed.
  • I have added tests that cover my changes.
  • All new and existing tests have passed locally.
  • I have run this code in a local environment to verify functionality.
  • I have considered the security implications of this change.

Summary by Gitar

  • New CRD and API types:
    • Added ImageAnalysisResult CRD for storing dive analysis metrics, layer breakdown, and workload references
    • Defined spec/status fields including efficiency, wasted space, and file analysis data
  • Image analysis controller system:
    • Implemented periodic scan cycle with discovery, planning, job submission, and result collection phases
    • Added smart diff to skip recently analyzed images within scan window
    • Integrated scan state tracking and leader election support
  • Image discovery engine:
    • Scans cluster pods to extract container images, deduplicated by digest and grouped by node
    • Resolves workload owners (Deployment, StatefulSet, DaemonSet, Job, CronJob) from pod references
    • Filters by namespace and image patterns
  • Batch job orchestration:
    • Plans and submits Kubernetes batch Jobs with concurrency limits (cluster and per-node)
    • Builds Job specs with containerd socket mounting, workspace volumes, and configurable resources
    • Manages job lifecycle including TTL cleanup and old scan job deletion
  • Result collection and CRD persistence:
    • Parses NDJSON output from analyzer pods to extract dive results
    • Maps dive metrics to ImageAnalysisResult CRDs with threshold evaluation
    • Includes workload references, layer details, and wasted file listings (capped at 50)
  • Configuration and helpers:
    • Added environment-based config loader with validation for resource quantities and tolerations
    • Analyzer container (Dockerfile, bash script) for batch image analysis using dive and crane
    • Helm chart values and RBAC updates for image analysis permissions

This will update automatically on new commits.

Comment thread internal/controller/image_job_manager.go Fixed
@Parthiba-Hazra Parthiba-Hazra marked this pull request as ready for review March 30, 2026 17:37
// Prevent overflow and unreasonable retry counts.
retries = 10
}
backoffLimit := int32(retries)

Check failure

Code scanning / CodeQL

Incorrect conversion between integer types High

Incorrect conversion of an integer with architecture-dependent bit size from
strconv.Atoi
to a lower bit size type int32 without an upper bound check.

Copilot Autofix

AI about 1 month ago

General approach: Avoid converting from an unbounded, architecture-dependent int that comes from strconv.Atoi into a smaller fixed-width integer type without a clear, type-appropriate bound. Either (1) keep the value as int32 all the way from parsing to use, or (2) add explicit bounds based on the target type. In this code, backoffLimit is already logically constrained to [1, 10], so the safest, minimal-change fix is to ensure the configuration field used here is itself a small, fixed-width type (e.g., int32) and to avoid any further narrowing conversion in buildJobObject.

Best concrete fix with minimal behavior change:

  • In buildJobObject, instead of computing backoffLimit as int32(retries) from an int, derive it directly as an int32 value and keep all intermediate variables in int32. Because we can’t change types in the ImageAnalysisConfig struct here (its definition isn’t shown) and can only modify shown snippets, the simplest local change is:
    • Change the temporary retries variable from int to int32.
    • Cast the config value once, with an explicit bound check that also satisfies CodeQL’s requirement for constant bounds, before any further logic.
    • Then operate purely on int32 values, and assign directly to backoffLimit as an int32 without an additional narrowing cast.

Concretely in internal/controller/image_job_manager.go inside buildJobObject:

  1. Introduce a temporary int32 with safe clamping from the possibly large int in m.config.MaxRetries. Use explicit constants within the int32 range (e.g., 0, 10) for clarity.
  2. Use that int32 variable directly as backoffLimit without another cast.

Since the code already enforces 1 <= retries <= 10, we preserve the same logic, but we cast once after a constant-bound check, making it clear there’s no risk of exceeding int32 limits.

No new imports or helper methods are necessary; we can rely solely on basic language constructs.

Suggested changeset 1
internal/controller/image_job_manager.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/internal/controller/image_job_manager.go b/internal/controller/image_job_manager.go
--- a/internal/controller/image_job_manager.go
+++ b/internal/controller/image_job_manager.go
@@ -319,14 +319,15 @@
 	activeDeadlineSeconds := int64(m.config.JobTimeoutMinutes * 60)
 
 	// backoffLimit from config.
-	retries := m.config.MaxRetries
-	if retries <= 0 {
-		retries = 1
-	} else if retries > 10 {
+	rawRetries := m.config.MaxRetries
+	// Clamp to a safe range before converting to int32.
+	if rawRetries <= 0 {
+		rawRetries = 1
+	} else if rawRetries > 10 {
 		// Prevent overflow and unreasonable retry counts.
-		retries = 10
+		rawRetries = 10
 	}
-	backoffLimit := int32(retries)
+	backoffLimit := int32(rawRetries)
 
 	ttl := int32(ttlSecondsAfterFinished)
 
EOF
@@ -319,14 +319,15 @@
activeDeadlineSeconds := int64(m.config.JobTimeoutMinutes * 60)

// backoffLimit from config.
retries := m.config.MaxRetries
if retries <= 0 {
retries = 1
} else if retries > 10 {
rawRetries := m.config.MaxRetries
// Clamp to a safe range before converting to int32.
if rawRetries <= 0 {
rawRetries = 1
} else if rawRetries > 10 {
// Prevent overflow and unreasonable retry counts.
retries = 10
rawRetries = 10
}
backoffLimit := int32(retries)
backoffLimit := int32(rawRetries)

ttl := int32(ttlSecondsAfterFinished)

Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants