feat(deps): Normalize Go dependency - #20
Conversation
Render each shared dependency once at its resolved (shallowest BFS) occurrence instead of repeating it under every parent. The resolved node is tagged with the count of other parents and each parent that hid a duplicate gets a trailing "(and N other dependencies)" marker. The model and JSON carry the new other_parents/hidden_duplicates counts. --show-duplicates restores the full per-parent repetition with dup:N tags. Stats, --flat node/edge lists, and deps diff stay on the full uncollapsed graph (collapse is forced off internally for diffs).
Discover local Helm chart directories (Chart.yaml) in addition to Flux
HelmRelease manifests. Each chart becomes a dependency root carrying its
declared subchart dependencies (helm, version recorded verbatim from the
Chart.yaml range) plus the container images referenced in values.yaml and
templates/ (image). Templated values (`{{ ... }}`) are skipped since they
cannot be resolved offline.
Chart discovery is a filesystem walk independent of the git-backed
k8s-manifest scanner, so a chart directory outside a git repo still
resolves; image-discovery errors now degrade to a warning when other
roots resolved. Vendored subcharts under a parent chart's charts/ dir are
skipped. Chart roots render package-style (manager-prefixed children),
distinct from the namespace/kind grouping used for k8s manifest images.
At --depth other than 1 (mirroring go/maven/gradle), dependency scanning now recurses into remote dependencies, backed by a persistent cache under the user cache dir: - Helm charts: each subchart is fetched from its Helm repository (index resolved via SemVer range, chart .tgz loaded with the Helm SDK chart loader), and its own subcharts and values/template images are recursed. - Container images: the base image is resolved from the OCI base-image label and from the Dockerfile FROM directives in the image's source repository (located via the OCI source label or a ghcr/quay heuristic, cloned into the cache), then recursed. The cache (remote_cache.go) keeps immutable artifacts (git clones, chart archives, image config by digest) long and revalidates indexes/tags daily, with negative caching and singleflight. Remote failures degrade to per-node warnings; only cache initialization is fatal. --depth 1 stays fully offline. Helm is pinned to v3.17.x and only pkg/chart/loader is used (index.yaml parsed directly): v3.18+ bumps distribution/v3 past what registry-scanner needs, and pkg/repo transitively requires a k8s API removed in the pinned k8s release.
…s under deps command Consolidate image/helm and package manager (go/npm/pnpm) update logic into a single `deps update` command with unified filtering, version resolution, and interactive selection. The legacy `images update` command now delegates to `deps update --manager image,helm`. Key changes: - Add --latest and --version flags to resolve dependencies non-interactively - Add resource filters (--kind, --namespace, --name, --selector) and name patterns (--image, --chart) for image/helm targets - Make expression argument optional; empty expression matches all dependencies - Support Flux v2 HelmRelease spec.chartRef (OCIRepository/HelmChart) in addition to inline spec.chart - Extract version math, matching, plan rendering, resolution, and prompts into separate files - Add helm credentials support (basic auth, TLS) for private chart repositories - Refactor imageupdate discovery into DiscoverRepoTargets and Filter functions - Add comprehensive tests for --latest, --version, resource filters, and chartRef resolution BREAKING CHANGE: `images update` is deprecated; use `deps update --manager image,helm` instead. The `deps update` expression argument is now optional (was required).
Apply effective Kustomize namespaces to discovered targets, distinguish exact-file scans from directory scans, and report untracked YAML files. Preserve chart source-resolution errors so callers can surface actionable diagnostics.
Expose Kubernetes metadata filters for image and Helm dependencies, while preserving positional paths and root help behavior. Restore implicit scan routing for bare paths, flags, and empty invocations.
Add Kubernetes resource filtering for image and Helm scans, with actionable errors for untracked manifest targets. Reuse published-version lookups across duplicate occurrences while preserving per-occurrence updates. Collapse package-manager duplicates silently by default and retain duplicate analysis only with --show-duplicates. BREAKING CHANGE: Remove Node.other_parents and Node.hidden_duplicates and their rendered markers; duplicate statistics and sections are no longer produced by default.
Expose MatchItem dependency filters through `deps update` and combine them with positional patterns. Deduplicate identical version prompts so one confirmation updates repeated declarations consistently. BREAKING CHANGE: rename `UpdateOptions.Expression` to `Filters` and change `VersionSelector` to receive `UpdateVersionPrompt`.
Add cache warming for CI, sandbox, and air-gapped environments. Use real package managers to populate shared caches, optionally compile dependencies, and verify offline readiness while preserving partial results and actionable command errors.
Replace Argo-specific registry APIs with go-containerregistry to simplify image metadata resolution while preserving Docker credential authentication and OCI label discovery Claude-Session-Id: 86a07da7-fd6f-46e2-bcbd-abaa7cadf735
Claude-Session-Id: 86a07da7-fd6f-46e2-bcbd-abaa7cadf735
Accept GitHub slugs and repository URLs for Go cache warming by canonicalising inputs before invoking the toolchain. Preserve the user's original spec while reporting the canonical module identity, and share dependency-spec parsing across managers. BREAKING CHANGE: manifest.Warmer implementations must implement NormalizeSpec
WalkthroughChangesDependency cache warming
Dependency graph scanning and updates
Kubernetes image and Helm targets
Sequence Diagram(s)sequenceDiagram
participant CLI
participant DiscoverTargets
participant SourceIndex
participant Update
CLI->>DiscoverTargets: scan repository and apply filters
DiscoverTargets->>SourceIndex: index Helm and OCI sources
SourceIndex-->>DiscoverTargets: resolved chart and image targets
DiscoverTargets-->>Update: ordered update candidates
Update-->>CLI: plans, edits, and warnings
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.72.0)Trivy execution failed: 2026-08-09T07:27:07Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: ansible scan error: scan config error: find projects: walk dir: range error: stat .coderabbit-opengrep-fallback.ea56c766-4af7-459a-9a7e-082d50bee691.yml: no such file or directory 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (18)
imageupdate/labels.go (1)
43-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe ST1023 hint on line 43 is a false positive. Keep the explicit type.
authn.Anonymousand the return value ofauthn.FromConfigon line 45 have different concrete types. If you drop theauthn.Authenticatorannotation,authinfers the concrete type ofauthn.Anonymousand the assignment on line 45 fails to compile. Suppress the hint with a//nolint:staticcheck // interface var needs explicit typedirective instead of applying it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@imageupdate/labels.go` around lines 43 - 46, Keep the explicit authn.Authenticator type on the auth variable because authn.Anonymous and authn.FromConfig return different concrete types. Add a //nolint:staticcheck // interface var needs explicit type directive to suppress ST1023 without changing the assignment logic.Source: Linters/SAST tools
cmd/repomap/images_update.go (1)
36-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSet
cmd.Deprecatedinstead of the manual stderr warning.Lines 36-39 mark deprecation in
cmd.Shorttext only. Cobra has aDeprecatedfield. When you set it, Cobra printsCommand "update" is deprecated, <text>on every invocation and marks the command in help output. That replaces the manualfmt.Fprintln(os.Stderr, ...)on line 44 and removes theosimport.♻️ Proposed change
func init() { cmd := clicky.AddNamedCommandWithContext("update", imagesCmd, UpdateImageOptions{}, runUpdateImage) cmd.Short = "(deprecated) Update image tags and Helm chart versions; use 'deps update'" + cmd.Deprecated = "use 'repomap deps update --manager image,helm' instead" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/repomap/images_update.go` around lines 36 - 39, Update the command initialization in init to set cmd.Deprecated with the deprecation guidance instead of embedding it in cmd.Short. Remove the manual stderr warning from runUpdateImage and delete the now-unused os import, preserving the command’s existing deprecation behavior through Cobra.imageupdate/discover_test.go (1)
46-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the scope helpers.
The new tests cover namespace resolution only.
inScanScopeandscanScopedecide which files discovery reads, anddeps updatethen edits those files.untrackedFileTargetdrives a user-facing message. None of the three has a test.A table test over
inScanScopeis cheap and pins the prefix semantics, including that a"kenya/"scope must not match"kenyatta/app.yaml".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@imageupdate/discover_test.go` around lines 46 - 66, Add table-driven tests for the scope helpers, covering prefix matching and rejecting paths outside the scope, especially ensuring inScanScope does not treat "kenyatta/app.yaml" as inside the "kenya/" scope. Also add focused coverage for scanScope and untrackedFileTarget, including their file-selection and user-facing message behavior.imageupdate/discover.go (1)
53-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe discovery pipeline discards every diagnostic across three layers. A malformed or unresolvable manifest is skipped silently at parse time, at extraction time, and again at presentation time.
DiscoverResult.WarningsandTargetWarningalready exist as the channel for these non-fatal problems, but no layer routes into it except chart resolution. The user sees an empty or incomplete result with no reason.
imageupdate/discover.go#L53-L66: append aTargetWarningwhenIndexSourcesfails on line 54 and whenExtractTargetsfails on line 63; move thewarningsdeclaration above the indexing loop.imageupdate/extract.go#L58-L81: line 75 discards theparser.ParseByteserror and skips the document. Return or report that error so a resource that decoded successfully but failed AST parsing is not dropped without a signal.cmd/repomap/images.go#L67-L74: printres.Warningsandres.UntrackedTargetto stderr, or extend thediscoverAndFiltersignature to return them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@imageupdate/discover.go` around lines 53 - 66, Route all non-fatal discovery diagnostics through the existing warning channel and present them to users: in imageupdate/discover.go lines 53-66, move warnings before indexing and append TargetWarning entries for IndexSources and ExtractTargets errors; in imageupdate/extract.go lines 58-81, propagate the parser.ParseBytes error instead of silently skipping the resource; in cmd/repomap/images.go lines 67-74, print DiscoverResult.Warnings and UntrackedTarget to stderr or return them through discoverAndFilter.deps/helm_credentials.go (2)
97-97: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueSet an explicit
MinVersionon the TLS config.The custom
tls.Configdoes not setMinVersion. The Go client default is TLS 1.2, so behavior is acceptable today, but the value is implicit and the linter flags it. Set it explicitly to pin the floor.🛡️ Proposed change
- tlsCfg := &tls.Config{InsecureSkipVerify: cred.InsecureSkipTLSVerify} //nolint:gosec // honors the user's helm repo setting + tlsCfg := &tls.Config{ + MinVersion: tls.VersionTLS12, + InsecureSkipVerify: cred.InsecureSkipTLSVerify, //nolint:gosec // honors the user's helm repo setting + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/helm_credentials.go` at line 97, Update the tls.Config initialization in the Helm credential setup to set MinVersion explicitly to TLS 1.2, while preserving the existing InsecureSkipTLSVerify behavior.Source: Linters/SAST tools
93-112: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the TLS clients instead of building one per request.
httpClientForCredsallocates a newhttp.Transporton every call.httpGetcallsauthorizefor each fetched URL, so each chart index and each.tgzdownload for a credentialed repository gets its own connection pool. The transports are never closed, so idle connections and their file descriptors accumulate during a large scan. Build the client once per repository entry and reuse it.♻️ Proposed refactor sketch
type helmCredentials struct { repos []helmRepoCreds + mu sync.Mutex + clients map[string]*http.Client // keyed by repo URL }- return httpClientForCreds(cred) + return h.clientFor(cred) +} + +func (h *helmCredentials) clientFor(cred helmRepoCreds) *http.Client { + h.mu.Lock() + defer h.mu.Unlock() + if c, ok := h.clients[cred.URL]; ok { + return c + } + c := httpClientForCreds(cred) + if h.clients == nil { + h.clients = map[string]*http.Client{} + } + h.clients[cred.URL] = c + return c }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/helm_credentials.go` around lines 93 - 112, Change the repository-fetch flow so httpClientForCreds is invoked once per repository entry and its *http.Client is reused for all authorize/httpGet requests for that repository, rather than constructing a new transport per URL. Preserve the existing credential-derived TLS configuration and default-client behavior, and ensure the cached client is scoped to each repository entry.deps/collapse.go (1)
26-45: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueSeed
seenwith the root ID.
collapseRootdoes not markroot.IDas seen. A self-referential dependency (a child whose ID equals the root module ID) is therefore kept and re-expanded once. Seeding the set makes the invariant explicit and matches the "each dependency renders once" contract in the doc comment.♻️ Proposed refactor
- seen := map[string]bool{} + seen := map[string]bool{root.ID: true} queue := []*Node{root}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/collapse.go` around lines 26 - 45, Update collapseRoot to initialize seen with root.ID before processing the queue, so self-referential root children are skipped while preserving the existing breadth-first collapsing behavior.deps/remote_cache.go (1)
119-127: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUse
ttlImmutablewhen the reference is digest-pinned.The TTL comment at lines 21-23 states that image config by digest is immutable and kept long.
ImageConfigalways usesttlIndex. A reference such asghcr.io/acme/app@sha256:...is therefore refetched daily even though its config cannot change.♻️ Proposed change
v, err, _ := c.group.Do("img:"+ref, func() (any, error) { path := c.entryPath("imageconfig", ref) - if e, ok := readEntry[ImageConfig](path); ok && !c.expiredAt(e.FetchedAt, e.NotFound, ttlIndex) { + ttl := ttlIndex + if strings.Contains(ref, "`@sha256`:") { + ttl = ttlImmutable + } + if e, ok := readEntry[ImageConfig](path); ok && !c.expiredAt(e.FetchedAt, e.NotFound, ttl) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/remote_cache.go` around lines 119 - 127, Update ImageConfig to select ttlImmutable for digest-pinned references and retain ttlIndex for mutable tag references, using the existing reference-parsing or digest-detection helper if available. Apply the selected TTL when calling expiredAt in the cached image-config lookup.deps/remote_helm_client.go (1)
147-187: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winBound the extracted archive size.
The
//nolint:goseccomment states that chart archives are size-bounded, but nothing enforces a bound.gzip.NewReaderplusio.Copydecompresses whatever the remote repository serves. A malicious or corrupt.tgzcan fill the temporary filesystem. Add a per-entry and total limit.Also consider
hdr.Typeflag. A symlink entry currently becomes an empty regular file, which silently corrupts charts that use symlinks. Skip or reject non-regular entries explicitly.🛡️ Proposed change
+const maxChartBytes = 512 << 20 // 512 MiB total per archive + func extractTarGz(data []byte, dest string) error { gz, err := gzip.NewReader(bytes.NewReader(data)) if err != nil { return err } defer func() { _ = gz.Close() }() tr := tar.NewReader(gz) + var written int64 for { @@ if hdr.FileInfo().IsDir() { if err := os.MkdirAll(target, 0o755); err != nil { return err } continue } + if !hdr.FileInfo().Mode().IsRegular() { + continue // skip symlinks, devices, and other special entries + } @@ - if _, err := io.Copy(f, tr); err != nil { //nolint:gosec // chart archives are size-bounded + n, err := io.Copy(f, io.LimitReader(tr, maxChartBytes-written)) + written += n + if err == nil && written >= maxChartBytes { + err = fmt.Errorf("chart archive exceeds %d bytes", maxChartBytes) + } + if err != nil { _ = f.Close() return err }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/remote_helm_client.go` around lines 147 - 187, Update extractTarGz to enforce both per-entry and cumulative decompressed-size limits while copying archive contents, returning an error before either limit can be exceeded; replace the unsupported size-bound claim in the io.Copy path with real enforcement. Also inspect hdr.Typeflag and explicitly handle non-regular entries, rejecting or intentionally skipping symlinks and other unsupported types rather than creating empty files.deps/scan_chart.go (2)
193-212: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
repoTagImagecan classify non-image maps as images.
repoTagImageis applied to every map in the values tree, not only to maps under animagekey. A block such asgit: {repository: "https://github.com/acme/x", tag: v1}yieldshttps://github.com/acme/x:v1.looksLikeImageaccepts it because the string starts with a letter and contains no rejected character. The result is a spurious image node.Reject references that contain
://inlooksLikeImage.♻️ Proposed change
func looksLikeImage(ref string) bool { if ref == "" || strings.ContainsAny(ref, " \t{}|<>\"'`") || strings.Contains(ref, "{{") { return false } + // A URL is a source repository, not a container image reference. + if strings.Contains(ref, "://") { + return false + } c := ref[0]Also applies to: 276-282
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/scan_chart.go` around lines 193 - 212, Update looksLikeImage to reject any candidate reference containing “://” before accepting it as an image, preventing URL-based repository values from producing image nodes. Preserve existing validation for legitimate image references and leave repoTagImage unchanged.
229-236: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRecord the intentional error swallowing for
nilerr.golangci-lint reports
nilerrat Line 231 and Line 235. Skipping an unreadable template must not abort the walk, so the behavior is intended. Add//nolint:nilerrwith the reason.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/scan_chart.go` around lines 229 - 236, Add targeted //nolint:nilerr annotations with a reason to the intentional nil-error returns in the filepath.WalkDir callback, covering both the walk callback error branch and the os.ReadFile failure branch. Preserve the behavior of skipping unreadable templates without aborting the walk.Source: Linters/SAST tools
deps/image_base.go (2)
129-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSilence
nilerrexplicitly and correct the doc comment.Two points:
- golangci-lint reports
nilerrat Line 138. The behavior is intended: one unreadable entry must not abort discovery. Add a//nolint:nilerrdirective with the reason so the intent is recorded and the pipeline stays green.- The comment says "shallow walk".
filepath.WalkDirrecurses through the whole tree except.gitandignoredDirs. Use accurate wording.♻️ Proposed change
// findDockerfile returns the repo-root Dockerfile if present, else the first -// Dockerfile found in a shallow walk, warning when several exist. +// Dockerfile found in a recursive walk (skipping .git and ignoredDirs), +// warning when several exist. func findDockerfile(root string) (string, []string) { if p := filepath.Join(root, "Dockerfile"); fileExists(p) { return p, nil } var found []string _ = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { if err != nil { + // A single unreadable entry must not abort discovery. + //nolint:nilerr return nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/image_base.go` around lines 129 - 139, Update the findDockerfile comment to describe the full recursive WalkDir traversal, including its existing exclusions, instead of calling it shallow. Add an inline nolint:nilerr directive with a clear reason to the intentional error path in the WalkDir callback, preserving the behavior of skipping unreadable entries.Source: Linters/SAST tools
129-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord the intentional
nilerrsuppression in bothWalkDircallbacks. Both walks deliberately returnnilwhen the callback receives an error, so one unreadable entry does not abort discovery. Neither site records that intent, so golangci-lint reportsnilerrthree times and the pipeline fails.
deps/image_base.go#L129-L139: add//nolint:nilerrwith the reason above thereturn nilat Line 138, and correct the doc comment, which calls the recursivefilepath.WalkDira "shallow walk".deps/scan_chart.go#L229-L236: add//nolint:nilerrwith the reason for thereturn nilat Line 231 and Line 235.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/image_base.go` around lines 129 - 139, In deps/image_base.go lines 129-139, add a reasoned //nolint:nilerr suppression above the intentional error-path return nil in findDockerfile, and correct its documentation to describe filepath.WalkDir as recursive rather than shallow. In deps/scan_chart.go lines 229-236, add the same reasoned //nolint:nilerr suppression above both intentional return nil statements so unreadable entries do not abort discovery.Source: Linters/SAST tools
deps/update_resolve.go (2)
160-168: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInclude the offending output in the parse error.
expected JSON version arraygives no way to diagnose the failure. npm and pnpm sometimes prepend a warning banner to stdout, which makes bothjson.Unmarshalcalls fail. Add a truncated excerpt ofstdoutto the error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/update_resolve.go` around lines 160 - 168, Update the JSON parse failure return in the version-output parsing flow to include a safely truncated excerpt of stdout alongside the existing error message. Preserve the successful array and single-string parsing paths, and ensure the excerpt cannot produce an excessively large error.
64-80: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse the task-supplied context in dependency-version lookups.
group.AddprovidesflanksourceContext.Context, but the callback ignores it and passes the outerctxtoresolveCandidateRawVersions. That keeps network lookups running if the task group context is cancelled or has a deadline, and it bypasses the task context’s logging scope. Use the injected context, or document why the outerctxmust be preserved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/update_resolve.go` around lines 64 - 80, Update the callback registered in the dependency-resolution group to pass its injected flanksourceContext.Context to resolveCandidateRawVersions instead of the outer ctx, preserving task cancellation, deadlines, and logging scope.deps/update_match.go (1)
186-197: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
lessis not a total order over the candidate key.
key()includesDir, butlessdoes not. Two candidates that differ only byDircompare equal in both directions. Ordering then depends on the discovery order of the caller. AddDirto make the order total and the output reproducible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/update_match.go` around lines 186 - 197, Update UpdateCandidate.less to compare the Dir field after Scope and before Name, matching the key fields used by key(). Preserve the existing comparison order for Manager, File, Scope, and Name so candidates differing only by Dir receive a deterministic ordering.deps/update_test.go (1)
682-718: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe lookup counter records only one of two resolver calls.
countingImageVersionResolverincrementscallsinAvailable, butResolveLatestVersionsis also invoked for every image or Helm candidate. Every dedupe assertion built on this counter therefore observes half of the round-trips.
deps/update_test.go#L682-L718: incrementcalls[updateTargetName(target)]inResolveLatestVersionsas well, under the same mutex.deps/update_modes_test.go#L140-L169: after the counter covers both methods, adjust the expected counts inTestUpdate_DedupesVersionLookupsAcrossDuplicatesso it asserts one lookup per image across both resolver methods.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/update_test.go` around lines 682 - 718, Update countingImageVersionResolver.ResolveLatestVersions in deps/update_test.go:682-718 to increment calls[updateTargetName(target)] under the existing mutex, matching Available. Then update expected counts in TestUpdate_DedupesVersionLookupsAcrossDuplicates in deps/update_modes_test.go:140-169 so each image asserts one lookup across both resolver methods.deps/update_modes.go (1)
61-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace
containsStringwithslices.Contains.The module targets Go 1.26.1, so move the string check to standard library
slices.Containsand importslices.♻️ Proposed refactor
-func containsString(values []string, target string) bool { - for _, v := range values { - if v == target { - return true - } - } - return false -}Then update the call site:
if !slices.Contains(available, opts.Version) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/update_modes.go` around lines 61 - 68, Remove the custom containsString helper and import the standard-library slices package. Update its call site in the surrounding version-availability logic to use slices.Contains(available, opts.Version), preserving the existing membership check behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/repomap/images_test_helpers.go`:
- Line 50: Update the Git command invocations in the test helper to use a
context created with context.WithTimeout and exec.CommandContext instead of
exec.Command. Apply this to both commands around the git init operations, and
defer context cancellation so the test cannot block indefinitely.
In `@cmd/repomap/images.go`:
- Around line 13-24: Update versionOnly to remove any digest suffix at “@”
before locating the tag separator. Extract the text after the last “:”, but
return it only when it contains no “/”; otherwise return the digest-stripped
reference unchanged so registry ports without tags are not treated as versions.
In `@deps/dockerfile.go`:
- Around line 118-127: Update parseArgAssign to parse all assignment tokens
rather than only tokens[0], supporting both `NAME=value` assignments and the
space-separated `NAME value` ENV form. Return or expose every parsed assignment
so callers can record multiple values such as `A=1 B=2`; update the caller to
consume the expanded result while preserving trimming and invalid-input
handling.
In `@deps/image_base.go`:
- Around line 82-85: Update the GitRepo resolution around
parseImageRef(ref).Version to try the image tag first, then its v-prefixed
variant when applicable, and finally the repository’s default branch. Preserve
the existing clone error reporting only after all revision candidates fail, so
common and semver container tags resolve successfully.
- Around line 113-127: Restrict normalizeGitURL to label-derived HTTPS URLs on
approved forge hosts, such as GitHub and GitLab, and reject all plaintext
http:// and arbitrary git@ hosts to prevent untrusted cloning targets. Preserve
the existing empty/unsupported behavior, and provide a separate explicit opt-in
path for callers that intentionally need arbitrary hosts rather than broadening
normalizeGitURL.
In `@deps/manager/node/warm.go`:
- Around line 74-80: Update the verification command construction in
deps/manager/node/warm.go lines 74-80 to copy cmds.Offline and append
cmds.IgnoreScripts when !req.Build, while preserving scripts for build
workflows. Update deps/manager/npm/warm_test.go lines 34-44 to expect npm ci
--offline --ignore-scripts for non-build verification and add coverage for
build-plus-verify without --ignore-scripts.
In `@deps/remote_cache.go`:
- Around line 242-255: Update writeEntry to create the staging file with
os.CreateTemp in filepath.Dir(path) using a unique name instead of path +
".tmp"; write the encoded buffer through that file, close it, and rename the
resulting temporary path to path while preserving the existing early-return
behavior on errors.
- Around line 171-196: Update cloneInto so failed git commands preserve and
return the underlying error for transient or authentication-related failures
instead of converting every failure to notFoundError. Retain notFoundError only
when the git failure output identifies a genuinely missing repository, including
the fallback clone path for a non-empty ref; inspect the runner’s error/output
consistently while preserving the existing successful clone flow.
In `@deps/remote_helm_client.go`:
- Around line 19-29: Update the Helm version pin note above helmClient to match
the current go.mod pin of helm.sh/helm/v3 v3.21.3, or remove the stale version
constraint and rationale. Keep the surrounding helmClient documentation accurate
for future dependency upgrades.
In `@deps/resolve_remote.go`:
- Around line 59-85: Update resolveRemote and expandNode so visited tracking is
path-local cycle protection rather than a global suppression set across roots
and sibling branches. Add a resolved fetch-result cache keyed by remoteKey, and
when a key has already been fetched, assign deep-copied cached children adjusted
to the current node depth before recursing; preserve cycle prevention with an
inFlight set and ensure distinct repeated deployments receive their children.
In `@deps/scan_chart.go`:
- Around line 214-223: Update scalarString in deps/scan_chart.go:214-223 to
preserve the original textual representation of numeric YAML scalars instead of
formatting decoded float64 values, while retaining existing handling for nil and
strings. In deps/scan_chart_test.go:28-31, change the fixture tag to a
precision-sensitive value such as 1.10 so the test verifies the image tag
remains unchanged.
- Around line 255-259: Update the image-value parsing near strings.TrimSpace in
the scan flow to remove YAML trailing comments before validation and appending
to refs. Ensure values such as “nginx:1.25 # pinned” become “nginx:1.25”, while
preserving quoted or otherwise valid image references and existing
empty/template filtering behavior.
- Around line 76-78: Update the root node path assignment in the chart-scanning
flow to use the repository-relative path already computed in rel, matching the
paths assigned to subchart and image children. Keep the serialized Node.Path
consistent and avoid storing the absolute chartPath.
- Around line 307-312: Update gitChartFiles and its caller Scan to use the scan
context when executing git ls-files, ensuring the command is cancellable or
subject to the established timeout. Preserve the existing file-discovery
arguments, including --exclude-standard, and keep the current failure result
when the command exits with an error.
In `@deps/scan.go`:
- Around line 86-100: Update the error-handling flow around imageErr and
packageErr so packageErr is appended to warnings when roots have been
discovered, mirroring the existing imageErr handling. Preserve returning
packageErr when len(roots) == 0, while ensuring successful scans with other
roots do not discard the package discovery failure.
In `@deps/update_match.go`:
- Around line 169-184: Update helmSourceKey to return a distinct non-empty
sentinel when target is nil, rather than returning the empty string. Preserve
the existing SourceErr, repository URL, and OCI key behavior for non-nil
targets.
In `@deps/update_version.go`:
- Around line 62-79: The updateableVersions function should return no registry
candidates when normalizeCurrentVersion produces a value that semver.NewVersion
cannot parse. Handle currentErr immediately after parsing current, while
preserving the existing filtering and greater-than comparison for valid current
semantic versions.
In `@imageupdate/chartref.go`:
- Around line 132-136: Update the HelmChart error formatting in the "HelmChart"
branch to use the chartRef object's namespace and name carried by t.Ref, rather
than src.srcRefNamespace and src.chartName. Preserve the existing wrapped lookup
error and ensure the message identifies the HelmChart object the user must fix.
In `@imageupdate/labels.go`:
- Around line 26-37: Update LabelResolver.Labels to parse and resolve the
original ref directly instead of reconstructing a tag from GetFullNameWithoutTag
and tagName. Preserve tag resolution for tag references while retaining any
`@digest`, including digest-only references, so remote.Image receives the pinned
reference and returns its digest and labels.
In `@tracked_yaml.go`:
- Around line 21-23: Update the tracked-file iteration around
strings.Split(result.Stdout, "\n") so valid Git filenames retain leading and
trailing whitespace when passed to the file-type check and subsequent reads.
Remove the unconditional strings.TrimSpace transformation, while still handling
genuinely empty lines appropriately and preserving filepath.ToSlash
normalization.
---
Nitpick comments:
In `@cmd/repomap/images_update.go`:
- Around line 36-39: Update the command initialization in init to set
cmd.Deprecated with the deprecation guidance instead of embedding it in
cmd.Short. Remove the manual stderr warning from runUpdateImage and delete the
now-unused os import, preserving the command’s existing deprecation behavior
through Cobra.
In `@deps/collapse.go`:
- Around line 26-45: Update collapseRoot to initialize seen with root.ID before
processing the queue, so self-referential root children are skipped while
preserving the existing breadth-first collapsing behavior.
In `@deps/helm_credentials.go`:
- Line 97: Update the tls.Config initialization in the Helm credential setup to
set MinVersion explicitly to TLS 1.2, while preserving the existing
InsecureSkipTLSVerify behavior.
- Around line 93-112: Change the repository-fetch flow so httpClientForCreds is
invoked once per repository entry and its *http.Client is reused for all
authorize/httpGet requests for that repository, rather than constructing a new
transport per URL. Preserve the existing credential-derived TLS configuration
and default-client behavior, and ensure the cached client is scoped to each
repository entry.
In `@deps/image_base.go`:
- Around line 129-139: Update the findDockerfile comment to describe the full
recursive WalkDir traversal, including its existing exclusions, instead of
calling it shallow. Add an inline nolint:nilerr directive with a clear reason to
the intentional error path in the WalkDir callback, preserving the behavior of
skipping unreadable entries.
- Around line 129-139: In deps/image_base.go lines 129-139, add a reasoned
//nolint:nilerr suppression above the intentional error-path return nil in
findDockerfile, and correct its documentation to describe filepath.WalkDir as
recursive rather than shallow. In deps/scan_chart.go lines 229-236, add the same
reasoned //nolint:nilerr suppression above both intentional return nil
statements so unreadable entries do not abort discovery.
In `@deps/remote_cache.go`:
- Around line 119-127: Update ImageConfig to select ttlImmutable for
digest-pinned references and retain ttlIndex for mutable tag references, using
the existing reference-parsing or digest-detection helper if available. Apply
the selected TTL when calling expiredAt in the cached image-config lookup.
In `@deps/remote_helm_client.go`:
- Around line 147-187: Update extractTarGz to enforce both per-entry and
cumulative decompressed-size limits while copying archive contents, returning an
error before either limit can be exceeded; replace the unsupported size-bound
claim in the io.Copy path with real enforcement. Also inspect hdr.Typeflag and
explicitly handle non-regular entries, rejecting or intentionally skipping
symlinks and other unsupported types rather than creating empty files.
In `@deps/scan_chart.go`:
- Around line 193-212: Update looksLikeImage to reject any candidate reference
containing “://” before accepting it as an image, preventing URL-based
repository values from producing image nodes. Preserve existing validation for
legitimate image references and leave repoTagImage unchanged.
- Around line 229-236: Add targeted //nolint:nilerr annotations with a reason to
the intentional nil-error returns in the filepath.WalkDir callback, covering
both the walk callback error branch and the os.ReadFile failure branch. Preserve
the behavior of skipping unreadable templates without aborting the walk.
In `@deps/update_match.go`:
- Around line 186-197: Update UpdateCandidate.less to compare the Dir field
after Scope and before Name, matching the key fields used by key(). Preserve the
existing comparison order for Manager, File, Scope, and Name so candidates
differing only by Dir receive a deterministic ordering.
In `@deps/update_modes.go`:
- Around line 61-68: Remove the custom containsString helper and import the
standard-library slices package. Update its call site in the surrounding
version-availability logic to use slices.Contains(available, opts.Version),
preserving the existing membership check behavior.
In `@deps/update_resolve.go`:
- Around line 160-168: Update the JSON parse failure return in the
version-output parsing flow to include a safely truncated excerpt of stdout
alongside the existing error message. Preserve the successful array and
single-string parsing paths, and ensure the excerpt cannot produce an
excessively large error.
- Around line 64-80: Update the callback registered in the dependency-resolution
group to pass its injected flanksourceContext.Context to
resolveCandidateRawVersions instead of the outer ctx, preserving task
cancellation, deadlines, and logging scope.
In `@deps/update_test.go`:
- Around line 682-718: Update countingImageVersionResolver.ResolveLatestVersions
in deps/update_test.go:682-718 to increment calls[updateTargetName(target)]
under the existing mutex, matching Available. Then update expected counts in
TestUpdate_DedupesVersionLookupsAcrossDuplicates in
deps/update_modes_test.go:140-169 so each image asserts one lookup across both
resolver methods.
In `@imageupdate/discover_test.go`:
- Around line 46-66: Add table-driven tests for the scope helpers, covering
prefix matching and rejecting paths outside the scope, especially ensuring
inScanScope does not treat "kenyatta/app.yaml" as inside the "kenya/" scope.
Also add focused coverage for scanScope and untrackedFileTarget, including their
file-selection and user-facing message behavior.
In `@imageupdate/discover.go`:
- Around line 53-66: Route all non-fatal discovery diagnostics through the
existing warning channel and present them to users: in imageupdate/discover.go
lines 53-66, move warnings before indexing and append TargetWarning entries for
IndexSources and ExtractTargets errors; in imageupdate/extract.go lines 58-81,
propagate the parser.ParseBytes error instead of silently skipping the resource;
in cmd/repomap/images.go lines 67-74, print DiscoverResult.Warnings and
UntrackedTarget to stderr or return them through discoverAndFilter.
In `@imageupdate/labels.go`:
- Around line 43-46: Keep the explicit authn.Authenticator type on the auth
variable because authn.Anonymous and authn.FromConfig return different concrete
types. Add a //nolint:staticcheck // interface var needs explicit type directive
to suppress ST1023 without changing the assignment logic.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 15a8bb64-92de-4863-82eb-3d92dca13156
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (77)
.gitignoreREADME.mdcmd/repomap/cache_warm.gocmd/repomap/cache_warm_test.gocmd/repomap/deps.gocmd/repomap/deps_test.gocmd/repomap/images.gocmd/repomap/images_test_helpers.gocmd/repomap/images_update.gocmd/repomap/images_update_test.gocmd/repomap/main.gocmd/repomap/main_test.godeps/cachewarm.godeps/cachewarm_plan.godeps/cachewarm_test.godeps/chart_remote.godeps/collapse.godeps/collapse_test.godeps/compare_scan.godeps/dockerfile.godeps/dockerfile_test.godeps/go_graph_test.godeps/helm_credentials.godeps/helm_credentials_test.godeps/image_base.godeps/manager/gomod/warm.godeps/manager/gomod/warm_test.godeps/manager/node/warm.godeps/manager/node/warm_test.godeps/manager/npm/warm.godeps/manager/npm/warm_test.godeps/manager/pnpm/warm.godeps/manager/pnpm/warm_test.godeps/manifest/command.godeps/manifest/manager.godeps/manifest/warm.godeps/manifest/warm_test.godeps/model.godeps/pretty_tree.godeps/pretty_tree_test.godeps/remote_cache.godeps/remote_cache_test.godeps/remote_helm_client.godeps/remote_helm_client_test.godeps/resolve_remote.godeps/resolve_remote_test.godeps/runner.godeps/scan.godeps/scan_chart.godeps/scan_chart_test.godeps/scan_image.godeps/scan_test.godeps/update.godeps/update_apply.godeps/update_image.godeps/update_match.godeps/update_modes.godeps/update_modes_test.godeps/update_plan.godeps/update_prompt.godeps/update_prompt_test.godeps/update_resolve.godeps/update_test.godeps/update_version.gogo.modimageupdate/chartref.goimageupdate/chartref_test.goimageupdate/discover.goimageupdate/discover_test.goimageupdate/extract.goimageupdate/labels.goimageupdate/sourceref.goimageupdate/sourceref_test.goimageupdate/target.goimageupdate/testdata/manifests/helmrelease-chartref-helmchart.yamlimageupdate/testdata/manifests/helmrelease-chartref-oci.yamltracked_yaml.go
💤 Files with no reviewable changes (1)
- cmd/repomap/images_update_test.go
| func writeRepo(t *testing.T) (*repomap.ArchConf, string) { | ||
| t.Helper() | ||
| dir := t.TempDir() | ||
| if out, err := exec.Command("git", "-C", dir, "init").CombinedOutput(); err != nil { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching images_test_helpers.go:\n'
fd -a 'images_test_helpers\.go$' . || true
file="$(fd 'images_test_helpers\.go$' . | head -n 1 || true)"
if [ -n "${file:-}" ]; then
echo
echo "File: $file"
wc -l "$file"
echo
sed -n '1,90p' "$file" | nl -ba
fi
printf '\nSearch noctx/lint config and usages:\n'
rg -n '"noctx"|noctx|golangci-lint|lint|exec\.Command\(|exec\.CommandContext\(' . -g '!vendor' -g '!.git' | head -n 200 || true
printf '\nGo files in cmd/repomap/images_test_helpers.go and nearby outline:\n'
if [ -f "$file" ]; then
ast-grep outline "$file" || true
fiRepository: flanksource/repomap
Length of output: 376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$(fd 'images_test_helpers\.go$' . | head -n 1 || true)"
echo "file=$file"
wc -l "$file"
echo
cat -n "$file"
printf '\nSearch noctx/lint config and exec.Command usages:\n'
rg -n '"noctx"|noctx|golangci-lint|lint|exec\.Command\(|exec\.CommandContext\(' . -g '!vendor' -g '!.git' | head -n 200 || true
echo
echo "Go imports and command context availability:"
rg -n '(^|[[:space:]])import\s*\(|"context"|golang.org/x/sync/errgroup|exec\.Command("git"|Context\(' "$file" || trueRepository: flanksource/repomap
Length of output: 3951
Use context-bound Git commands.
exec.Command at lines 50 and 57 can block indefinitely if Git stalls. Use context.WithTimeout with exec.CommandContext, and cancel the context when the test ends.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 50-50: os/exec.Command must not be called. use os/exec.CommandContext
(noctx)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/repomap/images_test_helpers.go` at line 50, Update the Git command
invocations in the test helper to use a context created with context.WithTimeout
and exec.CommandContext instead of exec.Command. Apply this to both commands
around the git init operations, and defer context cancellation so the test
cannot block indefinitely.
Source: Linters/SAST tools
| // versionOnly strips an image/chart current value down to its tag/version | ||
| // (dropping any registry/repo prefix and digest suffix). | ||
| func versionOnly(currentValue string) string { | ||
| if i := strings.LastIndex(currentValue, ":"); i >= 0 { | ||
| v := currentValue[i+1:] | ||
| if at := strings.Index(v, "@"); at >= 0 { | ||
| v = v[:at] | ||
| } | ||
| return v | ||
| } | ||
| return currentValue | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
versionOnly returns the wrong value for digest-qualified and port-qualified references.
Line 16 takes the last : in the whole string. Three cases break.
For ghcr.io/acme/app:v1@sha256:abcd, the last : is the one inside sha256:abcd. The function returns abcd instead of v1. The @ strip on lines 18-20 never runs, because the last : always sits to the right of the @ in this form. Those three lines are dead code.
For ghcr.io/acme/app@sha256:abcd, the function returns abcd.
For registry:5000/app with no tag, the last : is the port separator. The function returns 5000/app.
Strip the digest first. Then take the tag after the last :, and reject a match that still contains /, which means the : was a registry port.
🐛 Proposed fix
func versionOnly(currentValue string) string {
- if i := strings.LastIndex(currentValue, ":"); i >= 0 {
- v := currentValue[i+1:]
- if at := strings.Index(v, "@"); at >= 0 {
- v = v[:at]
- }
- return v
- }
- return currentValue
+ // Drop the digest suffix first; it contains its own ":".
+ name := currentValue
+ if at := strings.Index(name, "@"); at >= 0 {
+ name = name[:at]
+ }
+ i := strings.LastIndex(name, ":")
+ if i < 0 {
+ return name
+ }
+ // A ":" followed by a path segment is a registry port, not a tag.
+ if v := name[i+1:]; !strings.Contains(v, "/") {
+ return v
+ }
+ return name
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // versionOnly strips an image/chart current value down to its tag/version | |
| // (dropping any registry/repo prefix and digest suffix). | |
| func versionOnly(currentValue string) string { | |
| if i := strings.LastIndex(currentValue, ":"); i >= 0 { | |
| v := currentValue[i+1:] | |
| if at := strings.Index(v, "@"); at >= 0 { | |
| v = v[:at] | |
| } | |
| return v | |
| } | |
| return currentValue | |
| } | |
| // versionOnly strips an image/chart current value down to its tag/version | |
| // (dropping any registry/repo prefix and digest suffix). | |
| func versionOnly(currentValue string) string { | |
| // Drop the digest suffix first; it contains its own ":". | |
| name := currentValue | |
| if at := strings.Index(name, "@"); at >= 0 { | |
| name = name[:at] | |
| } | |
| i := strings.LastIndex(name, ":") | |
| if i < 0 { | |
| return name | |
| } | |
| // A ":" followed by a path segment is a registry port, not a tag. | |
| if v := name[i+1:]; !strings.Contains(v, "/") { | |
| return v | |
| } | |
| return name | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/repomap/images.go` around lines 13 - 24, Update versionOnly to remove any
digest suffix at “@” before locating the tag separator. Extract the text after
the last “:”, but return it only when it contains no “/”; otherwise return the
digest-stripped reference unchanged so registry ports without tags are not
treated as versions.
| func parseArgAssign(tokens []string) (name, value string, ok bool) { | ||
| if len(tokens) == 0 { | ||
| return "", "", false | ||
| } | ||
| eq := strings.SplitN(tokens[0], "=", 2) | ||
| if len(eq) != 2 { | ||
| return "", "", false | ||
| } | ||
| return eq[0], strings.Trim(eq[1], `"'`), true | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle the space-separated ENV form and multiple assignments.
parseArgAssign reads only tokens[0] and requires an =. Two valid forms are lost:
ENV GO_VERSION 1.22(legacy space form) returnsok == false. A laterFROM golang:$GO_VERSIONthen produces an "unresolved build arg" warning and the base image disappears from the graph.ARG A=1 B=2records onlyA.
🐛 Proposed fix
-func parseArgAssign(tokens []string) (name, value string, ok bool) {
- if len(tokens) == 0 {
- return "", "", false
- }
- eq := strings.SplitN(tokens[0], "=", 2)
- if len(eq) != 2 {
- return "", "", false
- }
- return eq[0], strings.Trim(eq[1], `"'`), true
-}
+// parseArgAssigns reads ARG/ENV assignments. It supports the `KEY=value`
+// form (possibly repeated) and the legacy `ENV KEY value` space form.
+func parseArgAssigns(tokens []string) map[string]string {
+ if len(tokens) == 0 {
+ return nil
+ }
+ if !strings.Contains(tokens[0], "=") {
+ if len(tokens) < 2 {
+ return nil
+ }
+ return map[string]string{tokens[0]: strings.Trim(strings.Join(tokens[1:], " "), `"'`)}
+ }
+ out := map[string]string{}
+ for _, tok := range tokens {
+ key, val, found := strings.Cut(tok, "=")
+ if !found || key == "" {
+ continue
+ }
+ out[key] = strings.Trim(val, `"'`)
+ }
+ return out
+}Update the caller accordingly:
case "ARG", "ENV":
- if name, val, ok := parseArgAssign(fields[1:]); ok {
- args[name] = val
- }
+ for name, val := range parseArgAssigns(fields[1:]) {
+ args[name] = val
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func parseArgAssign(tokens []string) (name, value string, ok bool) { | |
| if len(tokens) == 0 { | |
| return "", "", false | |
| } | |
| eq := strings.SplitN(tokens[0], "=", 2) | |
| if len(eq) != 2 { | |
| return "", "", false | |
| } | |
| return eq[0], strings.Trim(eq[1], `"'`), true | |
| } | |
| // parseArgAssigns reads ARG/ENV assignments. It supports the `KEY=value` | |
| // form (possibly repeated) and the legacy `ENV KEY value` space form. | |
| func parseArgAssigns(tokens []string) map[string]string { | |
| if len(tokens) == 0 { | |
| return nil | |
| } | |
| if !strings.Contains(tokens[0], "=") { | |
| if len(tokens) < 2 { | |
| return nil | |
| } | |
| return map[string]string{tokens[0]: strings.Trim(strings.Join(tokens[1:], " "), `"'`)} | |
| } | |
| out := map[string]string{} | |
| for _, tok := range tokens { | |
| key, val, found := strings.Cut(tok, "=") | |
| if !found || key == "" { | |
| continue | |
| } | |
| out[key] = strings.Trim(val, `"'`) | |
| } | |
| return out | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/dockerfile.go` around lines 118 - 127, Update parseArgAssign to parse
all assignment tokens rather than only tokens[0], supporting both `NAME=value`
assignments and the space-separated `NAME value` ENV form. Return or expose
every parsed assignment so callers can record multiple values such as `A=1 B=2`;
update the caller to consume the expanded result while preserving trimming and
invalid-input handling.
| dir, err := r.cache.GitRepo(ctx, url, parseImageRef(ref).Version) | ||
| if err != nil { | ||
| return nil, []string{fmt.Sprintf("image %s: clone %s: %s", ref, url, err)} | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
The image tag is used directly as a git ref.
parseImageRef(ref).Version passes the container tag to GitRepo as a revision. Common tags do not exist as git refs:
latest,stable, and floating tags have no matching ref.- Semver tags are frequently published as
v1.25.3in git but1.25.3in the registry.
Each mismatch fails the clone and degrades to a warning, so base-image resolution silently returns nothing for most images. Consider trying the tag, then the v-prefixed tag, then the default branch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/image_base.go` around lines 82 - 85, Update the GitRepo resolution
around parseImageRef(ref).Version to try the image tag first, then its
v-prefixed variant when applicable, and finally the repository’s default branch.
Preserve the existing clone error reporting only after all revision candidates
fail, so common and semver container tags resolve successfully.
| // normalizeGitURL turns an OCI source label into a cloneable URL. | ||
| func normalizeGitURL(source string) string { | ||
| source = strings.TrimSpace(source) | ||
| source = strings.TrimPrefix(source, "git+") | ||
| switch { | ||
| case source == "": | ||
| return "" | ||
| case strings.HasPrefix(source, "http://"), strings.HasPrefix(source, "https://"), strings.HasPrefix(source, "git@"): | ||
| return strings.TrimSuffix(source, ".git") | ||
| case strings.HasPrefix(source, "github.com/"), strings.HasPrefix(source, "gitlab.com/"): | ||
| return "https://" + strings.TrimSuffix(source, ".git") | ||
| default: | ||
| return "" | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the RemoteCache contract and the GitRepo clone implementation.
set -euo pipefail
rg -n -C6 'GitRepo\(|ImageConfig\(' --type=go
fd -i 'remote_cache' -e go --exec cat -n {}Repository: flanksource/repomap
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked Go files:\n'
git ls-files '*.go' | sed -n '1,120p'
printf '\nLocate image_base.go and related constants/functions:\n'
fd -i 'image_base|dep|remote.*cache|cache.*remote|git' -e go . -x sh -c 'echo "--- $1"; ast-grep outline "$1" 2>/dev/null | sed -n "1,160p"' sh {}Repository: flanksource/repomap
Length of output: 8369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Search for source-label/git clone related symbols:\n'
rg -n -C5 'Labels|labelSource|normalizeGitURL|sourceRepoHeuristic|source|Source|git+|github.com|gitlab.com|git clone|Exec|cmd\.' --type=goRepository: flanksource/repomap
Length of output: 208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Image resolver labels and source handling:\n'
cat -n deps/image_base.go | sed -n '1,160p'
printf '\nRemote cache GitRepo implementation:\n'
cat -n deps/remote_cache.go | sed -n '1,230p'Repository: flanksource/repomap
Length of output: 14804
Restrict OCI source labels before cloning.
cfg.Labels["org.opencontainers.image.source"] is image metadata, and a non-empty value is sent to cache.GitRepo, which runs git clone. normalizeGitURL currently accepts any http://, https://, or git@ host, so a crafted image label can make repomap clone from an attacker-chosen host, including internal addresses. Allow only known forge hosts for label-derived sources, and reject plaintext http://; leave an explicit opt-in path for arbitrary hosts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/image_base.go` around lines 113 - 127, Restrict normalizeGitURL to
label-derived HTTPS URLs on approved forge hosts, such as GitHub and GitLab, and
reject all plaintext http:// and arbitrary git@ hosts to prevent untrusted
cloning targets. Preserve the existing empty/unsupported behavior, and provide a
separate explicit opt-in path for callers that intentionally need arbitrary
hosts rather than broadening normalizeGitURL.
| // helmSourceKey identifies the chart repository a Helm candidate resolves | ||
| // against; charts sharing a name but a different repo (or an unresolved source) | ||
| // must not collapse onto the same lookup. | ||
| func helmSourceKey(target *imageupdate.UpdateTarget) string { | ||
| if target == nil { | ||
| return "" | ||
| } | ||
| if target.SourceErr != "" { | ||
| return "err:" + target.SourceErr | ||
| } | ||
| key := target.RepoURL | ||
| if target.IsOCI { | ||
| key += "|oci" | ||
| } | ||
| return key | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
A nil Target collapses distinct Helm candidates onto one lookup.
The doc comment states that an unresolved source must not collapse onto the same lookup. helmSourceKey returns "" when target == nil, so every Helm candidate without a target shares one resolution key. Two charts with the same name from different repositories then receive the same published-version list.
Return a distinct sentinel for the nil case.
🐛 Proposed fix
func helmSourceKey(target *imageupdate.UpdateTarget) string {
if target == nil {
- return ""
+ // An absent target is unresolved; keep it out of every shared bucket.
+ return "nil"
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/update_match.go` around lines 169 - 184, Update helmSourceKey to return
a distinct non-empty sentinel when target is nil, rather than returning the
empty string. Preserve the existing SourceErr, repository URL, and OCI key
behavior for non-nil targets.
| func updateableVersions(current string, versions []string) []string { | ||
| current = normalizeCurrentVersion(current) | ||
| currentSemver, currentErr := semver.NewVersion(current) | ||
| out := make([]string, 0, len(versions)) | ||
| for _, version := range versions { | ||
| if version == "" || version == current { | ||
| continue | ||
| } | ||
| versionSemver, versionErr := semver.NewVersion(version) | ||
| if currentErr != nil || versionErr != nil { | ||
| out = append(out, version) | ||
| continue | ||
| } | ||
| if versionSemver.GreaterThan(currentSemver) { | ||
| out = append(out, version) | ||
| } | ||
| } | ||
| return out |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'updateableVersions\(' deps --glob '*.go'
rg -n -C 4 '"(workspace:|file:|git\+|https://|ssh://)' deps --glob '*_test.go' || trueRepository: flanksource/repomap
Length of output: 11666
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- deps/update_version.go ---\n'
cat -n deps/update_version.go | sed -n '1,110p'
printf '\n--- deps/update_resolve.go ---\n'
cat -n deps/update_resolve.go | sed -n '1,80p'
printf '\n--- deps/update_test.go relevant sections ---\n'
cat -n deps/update_test.go | sed -n '1,150p'
printf '\n--- semver module references ---\n'
rg -n 'Masterminds/semver|NewVersion|NormalizeSpec|go.mod' -g '*.go' -g 'go.mod' -g 'deps/*'Repository: flanksource/repomap
Length of output: 248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- deps/update_version.go ---'
cat -n deps/update_version.go | sed -n '1,110p'
printf '%s\n' ''
printf '%s\n' '--- deps/update_resolve.go ---'
cat -n deps/update_resolve.go | sed -n '1,80p'
printf '%s\n' ''
printf '%s\n' '--- deps/update_test.go relevant sections ---'
cat -n deps/update_test.go | sed -n '1,150p'
printf '%s\n' ''
printf '%s\n' '--- semver module references ---'
rg -n 'Masterminds/semver|NewVersion|NormalizeSpec|go.mod' -g '*.go' -g 'go.mod' -g 'deps/*'Repository: flanksource/repomap
Length of output: 12538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repo root files ---'
git ls-files | sed -n '1,80p'
printf '%s\n' ''
printf '%s\n' '--- lockfiles with semver references ---'
for f in $(git ls-files | grep -E '(^|/)go\.mod$|(^|/)go\.sum$|go\.work$'); do
if rg -q 'Masterminds/semver' "$f"; then
echo "## $f"
rg -n 'Masterminds/semver' "$f"
fi
done
printf '%s\n' ''
printf '%s\n' '--- dependency resolver/candidate discovery sources ---'
rg -n -C 3 'type UpdateCandidate|ManagerNPM|file:|workspace:\*|DiscoverUpdateCandidates|resolveCandidateRawVersions' deps --glob '*.go'Repository: flanksource/repomap
Length of output: 31383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- pkg/deps go.mod ---'
cat -n pkg/deps/go.mod | sed -n '1,80p' || true
printf '%s\n' ''
printf '%s\n' '--- semver NewVersion behavior in installed module cache ---'
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cd "$tmpdir"
go mod init probe
printf 'module probe\n\nrequire github.com/Masterminds/semver/v3 v3.5.0\n' > go.mod
cat > probe.go <<'EOF'
package main
import (
"fmt"
"github.com/Masterminds/semver/v3"
)
func main() {
samples := []string{"workspace:*", "workspace:.", "workspace:^1.0.0", "file:../shared", "./local", "git@github.com:acme/lib.git", "https://github.com/acme/lib.git", "1.2.3", "2.0.0", "~1.0.0", "^1.0.0"}
for _, s := range samples {
v, err := semver.NewVersion(s)
if err != nil {
fmt.Printf("%q => parse-error: %s\n", s, err)
continue
}
fmt.Printf("%q => parsed: %s pr: %q patch: %d\n", s, v.String(), v.Prerelease(), v.Patch())
}
}
EOF
go run probe.goRepository: flanksource/repomap
Length of output: 473
🌐 Web query:
Masterminds semver NewVersion non semver returns error parsed only semver documentation
💡 Result:
In the Masterminds/semver Go library, the NewVersion function does not simply ignore non-semver input; it attempts to coerce such input into a valid semantic version by default [1][2]. If the input cannot be parsed or coerced into a valid semantic version, NewVersion returns an error [3][4]. The library provides the following mechanisms for handling this: 1. Default Behavior (Coercion): The package-level variable CoerceNewVersion is set to true by default [1][5]. When true, NewVersion attempts to coerce "SemVer-ish" strings (such as v1.2 or 1.2) into compliant semantic versions (e.g., 1.2.0) [1][6]. 2. Error Handling: If the string is fundamentally unparseable, NewVersion returns a non-nil error [1][3]. It is standard practice to check for this error after calling the function [1][2]: v, err:= semver.NewVersion("invalid-version") if err!= nil { // Handle the error here } 3. Strict Parsing: If you require strict adherence to the SemVer 2 specification without automatic coercion, you should use the StrictNewVersion function instead [1][5]. 4. Detailed Errors: The library includes a package-level variable DetailedNewVersionErrors [1][5]. When CoerceNewVersion is false, setting DetailedNewVersionErrors to true allows the parser to return more specific diagnostic information explaining why a version failed to parse, rather than a generic ErrInvalidSemVer [1][3]. Enabling this feature is generally slower than the default fast path [1][3].
Citations:
- 1: https://pkg.go.dev/github.com/Masterminds/semver/v3
- 2: https://github.com/Masterminds/semver?tab=readme-ov-file
- 3: https://github.com/NVIDIA/nodewright/blob/main/operator/vendor/github.com/Masterminds/semver/v3/version.go
- 4: https://github.com/Masterminds/semver/blob/master/version.go
- 5: https://pkg.go.dev/github.com/masterminds/semver/v3@v3.4.0
- 6: https://github.com/Masterminds/semver/blob/v3.4.0/doc.go
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cd "$tmpdir"
rm -f go.mod go.sum
cat > probe.go <<'EOF'
package main
import (
"fmt"
"math/big"
"strings"
"github.com/Masterminds/semver/v3"
)
func main() {
semver.CoerceNewVersion = true
samples := []string{
"workspace:*", "workspace:.", "workspace:^1.0.0",
"file:../shared", "./local",
"git@github.com:acme/lib.git", "https://github.com/acme/lib.git",
"1.2.3", "2.0.0", "~1.0.0", "^1.0.0",
"unstable-git",
}
for _, s := range samples {
v, err := semver.NewVersion(s)
if err != nil {
fmt.Printf("%q => parse-error: %s\n", s, err)
continue
}
fmt.Printf("%q => parsed %v major=%d minor=%d patch=%d prerelease=%q majorInt=%s\n", s, v, v.Major(), v.Minor(), v.Patch(), v.Prerelease(), v.MajorRaw())
}
}
EOF
cat > go.mod <<'EOF'
module probe
go 1.22
require github.com/Masterminds/semver/v3 v3.5.0
EOF
cat > go.sum <<'EOF'
github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAwZ/1OOE=
github.com/Masterminds/semver/v3 v3.5.0/go.mod h1:4V+yj/TJE1HU9XfppCgVM5q4I8p9XfR9mK4nBSS6E=
EOF
go mod verify
go run probe.goRepository: flanksource/repomap
Length of output: 600
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'cd / && rm -rf "$tmpdir"' EXIT
cd "$tmpdir"
rm -f go.mod go.sum
cat > probe.go <<'EOF'
package main
import (
"fmt"
"github.com/Masterminds/semver/v3"
)
func main() {
semver.CoerceNewVersion = true
samples := []string{
"workspace:*", "workspace:.", "workspace:^1.0.0",
"file:../shared", "./local",
"git@github.com:acme/lib.git", "https://github.com/acme/lib.git",
"1.2.3", "2.0.0", "~1.0.0", "^1.0.0",
"unstable-git",
}
for _, s := range samples {
v, err := semver.NewVersion(s)
if err != nil {
fmt.Printf("%q => parse-error: %s\n", s, err)
continue
}
fmt.Printf("%q => parsed %v major=%d minor=%d patch=%d prerelease=%q majorInt=%s\n", s, v, v.Major(), v.Minor(), v.Patch(), v.Prerelease(), v.MajorRaw())
}
}
EOF
cat > go.mod <<'EOF'
module probe
go 1.22
require github.com/Masterminds/semver/v3 v3.5.0
EOF
go mod download github.com/Masterminds/semver/v3@v3.5.0
go list -mod=readonly -m github.com/Masterminds/semver/v3
go run probe.goRepository: flanksource/repomap
Length of output: 317
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'cd / && rm -rf "$tmpdir"' EXIT
cd "$tmpdir"
rm -f go.mod go.sum
cat > probe.go <<'EOF'
package main
import (
"fmt"
"math/big"
"github.com/Masterminds/semver/v3"
)
func main() {
semver.CoerceNewVersion = true
samples := []string{
"workspace:*", "workspace:.", "workspace:^1.0.0",
"file:../shared", "./local",
"git@github.com:acme/lib.git", "https://github.com/acme/lib.git",
"1.2.3", "2.0.0", "~1.0.0", "^1.0.0",
"unstable-git",
}
for _, s := range samples {
v, err := semver.NewVersion(s)
if err != nil {
fmt.Printf("%q => parse-error: %s\n", s, err)
continue
}
fmt.Printf("%q => parsed %v major=%d minor=%d patch=%d prerelease=%q major=%s\n", s, v, v.Major(), v.Minor(), v.Patch(), v.Prerelease(), v.(*semver.Version).Raw().MarshalJSON())
_ = v.(*semver.Version).Raw().MarshalJSON()
}
}
EOF
cat > go.mod <<'EOF'
module probe
go 1.22
require github.com/Masterminds/semver/v3 v3.5.0
EOF
go mod download github.com/Masterminds/semver/v3@v3.5.0
go run probe.goRepository: flanksource/repomap
Length of output: 419
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'cd / && rm -rf "$tmpdir"' EXIT
cd "$tmpdir"
rm -f go.mod go.sum
cat > probe.go <<'EOF'
package main
import (
"fmt"
"github.com/Masterminds/semver/v3"
)
func main() {
semver.CoerceNewVersion = true
samples := []string{
"workspace:*", "workspace:.", "workspace:^1.0.0",
"file:../shared", "./local",
"git@github.com:acme/lib.git", "https://github.com/acme/lib.git",
"1.2.3", "2.0.0", "~1.0.0", "^1.0.0",
"unstable-git",
}
for _, s := range samples {
v, err := semver.NewVersion(s)
if err != nil {
fmt.Printf("%q => parse-error: %s\n", s, err)
continue
}
fmt.Printf("%q => parsed %v major=%d minor=%d patch=%d prerelease=%q\n", s, v, v.Major(), v.Minor(), v.Patch(), v.Prerelease())
}
}
EOF
cat > go.mod <<'EOF'
module probe
go 1.22
require github.com/Masterminds/semver/v3 v3.5.0
EOF
go mod download github.com/Masterminds/semver/v3@v3.5.0
go run probe.goRepository: flanksource/repomap
Length of output: 862
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- deps/package-lock.go ---'
cat -n deps/package-lock.go | sed -n '1,280p'
printf '%s\n' ''
printf '%s\n' '--- deps/scan.go package lock sections ---'
cat -n deps/scan.go | sed -n '240,320p'
printf '%s\n' ''
printf '%s\n' '--- package lock candidate fields ---'
rg -n -C 5 'addPackageJSONDeps|PackageLock|packageLock|PnpmLock|ParsePackage|ParsePnpm' deps --glob '*.go'Repository: flanksource/repomap
Length of output: 239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package-lock source files ---'
git ls-files deps | sort | grep -E 'package|pnpm|npm|manifest'
printf '%s\n' ''
printf '%s\n' '--- local specifier parser references ---'
rg -n -C 4 'isPackageManager|isLocal|file:|workspace:\*|workspace:"|workspace:' deps --glob '*.go'
printf '%s\n' ''
printf '%s\n' '--- update stage apply ---'
cat -n deps/update_apply.go | sed -n '1,90p'
printf '%s\n' ''
printf '%s\n' '--- update modes available versions tests ---'
cat -n deps/update_modes_test.go | sed -n '1,90p'
cat -n deps/update_modes_test.go | sed -n '200,290p'Repository: flanksource/repomap
Length of output: 18005
No registry update choices for unparseable current specs.
For current values such as workspace:*, file:../shared, or repository URLs, semver.NewVersion returns an error, but currentErr != nil currently offers every registry version. Apply no registry updates when the current value is not valid semantic versioning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/update_version.go` around lines 62 - 79, The updateableVersions function
should return no registry candidates when normalizeCurrentVersion produces a
value that semver.NewVersion cannot parse. Handle currentErr immediately after
parsing current, while preserving the existing filtering and greater-than
comparison for valid current semantic versions.
| case "HelmChart": | ||
| repo, err := idx.lookupHelmRepository(src.srcRefName, chartSourceNamespace(src, wantNS), t.Ref) | ||
| if err != nil { | ||
| return fmt.Errorf("HelmChart %s/%s: %w", src.srcRefNamespace, src.chartName, err) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the HelmChart identity in the error message.
Line 135 formats src.srcRefNamespace and src.chartName. src.srcRefNamespace is the namespace of the HelmChart's spec.sourceRef, not the namespace of the HelmChart object. src.chartName is spec.chart, not the HelmChart object name. When spec.sourceRef.namespace is omitted, the message renders as HelmChart /<chart>: .... This message reaches the user through SourceErr and TargetWarning, so it must name the object the user has to fix.
Use the chartRef identity that the target already carries.
🐛 Proposed fix
case "HelmChart":
repo, err := idx.lookupHelmRepository(src.srcRefName, chartSourceNamespace(src, wantNS), t.Ref)
if err != nil {
- return fmt.Errorf("HelmChart %s/%s: %w", src.srcRefNamespace, src.chartName, err)
+ return fmt.Errorf("HelmChart %s: %w", sourceKey(t.ChartRefNamespace, t.ChartRefName), err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "HelmChart": | |
| repo, err := idx.lookupHelmRepository(src.srcRefName, chartSourceNamespace(src, wantNS), t.Ref) | |
| if err != nil { | |
| return fmt.Errorf("HelmChart %s/%s: %w", src.srcRefNamespace, src.chartName, err) | |
| } | |
| case "HelmChart": | |
| repo, err := idx.lookupHelmRepository(src.srcRefName, chartSourceNamespace(src, wantNS), t.Ref) | |
| if err != nil { | |
| return fmt.Errorf("HelmChart %s: %w", sourceKey(t.ChartRefNamespace, t.ChartRefName), err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@imageupdate/chartref.go` around lines 132 - 136, Update the HelmChart error
formatting in the "HelmChart" branch to use the chartRef object's namespace and
name carried by t.Ref, rather than src.srcRefNamespace and src.chartName.
Preserve the existing wrapped lookup error and ensure the message identifies the
HelmChart object the user must fix.
| // Labels resolves the manifest digest and OCI labels for an image reference of | ||
| // the form registry/repo:tag[@digest]. | ||
| func (l *LabelResolver) Labels(ctx context.Context, ref string) (digest string, labels map[string]string, err error) { | ||
| img := NewContainerImage(ref) | ||
| tagName := "latest" | ||
| if img.ImageTag != nil && img.ImageTag.TagName != "" { | ||
| tagName = img.ImageTag.TagName | ||
| } | ||
| tag, err := name.NewTag(img.GetFullNameWithoutTag() + ":" + tagName) | ||
| if err != nil { | ||
| return "", nil, fmt.Errorf("parse reference %s:%s: %w", img.GetFullNameWithoutTag(), tagName, err) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
go-containerregistry name.ParseReference digest and tag precedence
💡 Result:
In the google/go-containerregistry library, the name.ParseReference function determines the type of a reference based on a specific precedence order defined in its implementation [1][2]. When you call name.ParseReference(s), the function attempts to parse the string as a tag first, and if that fails, it attempts to parse it as a digest [1][2]. 1. Tag Check: It calls NewTag(s). If this succeeds, it returns a Tag object [1][2]. 2. Digest Check: If NewTag(s) fails, it calls NewDigest(s). If this succeeds, it returns a Digest object [1][2]. Regarding references that contain both a tag and a digest (e.g., repo:tag@digest): The library's behavior for these combined references has evolved. Historically, attempting to parse a combined reference like repo:tag@digest through NewTag often fails or produces unexpected results because NewTag expects the tag to be the final component [3]. Consequently, such references are typically successfully parsed as a Digest type [3][4]. Recent updates have clarified these semantics. While the library now explicitly recognizes and handles references with both a tag and a digest as a Digest reference, the tag information is effectively "carried" within the digest object to preserve the original reference string, even though the primary identifier of the returned object is the digest [5]. If you need to access both components, note that the returned Digest object may not provide a direct, simple method like Tag in older versions, though ongoing efforts have been made to improve access to this carried tag information [6]. In summary, the precedence is explicitly Tag first, then Digest [1][2]. If a string fits both formats (or is a combined format that fails the strict Tag parser), it is treated as a Digest [3][4].
Citations:
- 1: https://github.com/google/go-containerregistry/blob/master/pkg/name/ref.go
- 2: https://github.com/google/go-containerregistry/blob/main/pkg/name/ref.go
- 3: question: Clarify the semantics of image references with both tag and digest google/go-containerregistry#2287
- 4: ggcr: name functionally discards tags when a reference has both tag and digest google/go-containerregistry#1768
- 5: Document tag and digest reference semantics google/go-containerregistry#2325
- 6: ggcr: Parsing image name with both tag and digest loses tag info google/go-containerregistry#2069
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== files ==\n'
git ls-files | rg '(^|/)imageupdate/labels\.go$|(^|/)deps/remote_cache\.go$|(^|/)go\.mod$'
printf '\n== labels.go outline ==\n'
ast-grep outline imageupdate/labels.go || true
printf '\n== labels.go relevant lines ==\n'
cat -n imageupdate/labels.go | sed -n '1,120p'
printf '\n== go.mod containerregistry version ==\n'
rg -n 'google/go-containerregistry|containerregistry' go.mod go.sum || true
printf '\n== deps/remote_cache relevant lines ==\n'
cat -n deps/remote_cache.go | sed -n '1,220p'
printf '\n== usages of LabelResolver.Labels ==\n'
rg -n 'Labels\(|LabelResolver' -S .Repository: flanksource/repomap
Length of output: 13002
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe: read the vendored/go-module local source only if available under module cache
# without executing repository code or installing dependencies.
python3 - <<'PY'
import os, pathlib, re
# Try to locate go-containerregistry name source in module cache based on go.mod line.
mod_path = pathlib.Path('go.mod')
mods = None
if mod_path.exists():
text = mod_path.read_text()
m = re.search(r'^module\s+(?P<mod>\S+)\s*$', text, re.M)
v = re.search(r'^\s+github\.com/google/go-containerregistry\s+v(?P<ver>\S+)\s*$', text, re.M)
if m and v:
mods = [('github.com/google/go-containerregistry', v.group('ver'))]
print('go.mod module=', m.group('mod'), 'containerregistry=', v.group('ver'))
cache_root = os.environ.get('GOMODCACHE')
if not cache_root:
cache_home = os.path.expanduser('~/go/pkg/mod')
cache_root = cache_home
print('cache_root=', cache_root)
candidates=[]
if mods:
mod, ver = mods[0]
candidates.append(os.path.join(cache_root, mod+'`@v`'+ver, 'pkg', 'name', 'ref.go'))
# also common installed versions if exact path missing
for d in sorted(pathlib.Path(cache_root).glob('github.com/google/go-containerregistry@*')):
candidates.append(str(d/'pkg/name/ref.go'))
for path in candidates:
p=pathlib.Path(path)
print('candidate=', path, p.exists())
if p.exists():
txt=p.read_text()
for func in ['ParseReference', 'NewTag', 'NewDigest']:
idx=txt.find('func '+func)
print('\n-- '+func+' at', idx)
if idx >= 0:
start=txt.rfind('\n',0,max(0,idx-1000))+1
end=txt.find('\nfunc ', idx+1)
if end==-1: end=len(txt)
print(txt[start:end][:3000])
# Probe package parse behavior if module is cached as source and tools can import nothing.
# Avoid running repository code; just use installed go if present and package exists.
PYRepository: flanksource/repomap
Length of output: 4583
Resolve digest-pinned references by digest.
Labels documents support for registry/repo:tag[@digest], but line 34 builds a name.NewTag from GetFullNameWithoutTag() + ":" + tagName and drops any digest. A manifest pin such as repo:v1@sha256:... will instead walk the v1 tag, and a digest-only reference falls back to :latest. Since deps/remote_cache.go:119-138 caches the result under the original reference, the wrong digest and labels are stored under the digest-pinned key.
Parse the input reference directly so digest references resolve by digest; remote.Image accepts tag or digest references.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@imageupdate/labels.go` around lines 26 - 37, Update LabelResolver.Labels to
parse and resolve the original ref directly instead of reconstructing a tag from
GetFullNameWithoutTag and tagName. Preserve tag resolution for tag references
while retaining any `@digest`, including digest-only references, so remote.Image
receives the pinned reference and returns its digest and labels.
| for _, line := range strings.Split(result.Stdout, "\n") { | ||
| file := filepath.ToSlash(strings.TrimSpace(line)) | ||
| if file == "" || !kubernetes.IsYaml(file) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve tracked filename whitespace.
Line 22 changes valid Git paths such as " deploy.yaml" or "deploy.yaml " before the file read. The scan then skips the tracked manifest.
Proposed fix
for _, line := range strings.Split(result.Stdout, "\n") {
- file := filepath.ToSlash(strings.TrimSpace(line))
+ file := filepath.ToSlash(line)
if file == "" || !kubernetes.IsYaml(file) {
continue
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for _, line := range strings.Split(result.Stdout, "\n") { | |
| file := filepath.ToSlash(strings.TrimSpace(line)) | |
| if file == "" || !kubernetes.IsYaml(file) { | |
| for _, line := range strings.Split(result.Stdout, "\n") { | |
| file := filepath.ToSlash(line) | |
| if file == "" || !kubernetes.IsYaml(file) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tracked_yaml.go` around lines 21 - 23, Update the tracked-file iteration
around strings.Split(result.Stdout, "\n") so valid Git filenames retain leading
and trailing whitespace when passed to the file-type check and subsequent reads.
Remove the unconditional strings.TrimSpace transformation, while still handling
genuinely empty lines appropriately and preserving filepath.ToSlash
normalization.
1 participant
What
Notes
manifest.Warmerimplementations must implementNormalizeSpec.Summary by CodeRabbit
New Features
cache-warmfor Go, npm, and pnpm dependencies, with optional builds and offline verification.Improvements
images updatenow directs users todeps updateand shows a deprecation warning.Documentation
cache-warmusage and supported workflows.