fix: dockerfile registry.ci input detection with baseImages

[deepsm007]

Skip Dockerfile registry.ci inputs already covered by from + baseImages; stop dropping the last unrelated FROM when from names a different base.

https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_continuous-release-jobs/1797/pull-ci-openshift-continuous-release-jobs-master-images/2060040001508347904

Summary

This PR fixes Dockerfile registry reference detection in OpenShift CI's pipeline configuration processing, addressing issues with duplicate input detection when base images are explicitly specified.

Problem Fixed

The CI infrastructure was incorrectly detecting and processing Docker registry references from Dockerfile FROM instructions that were already declared through the baseImages configuration. This caused:

• Duplicate input detection when a base image was both explicitly configured and referenced in the Dockerfile
• Loss of the last FROM instruction in multi-stage builds when the primary from configuration pointed to a different base image
• Overly aggressive removal of FROM directives that were unrelated to the primary build stage

Changes

Core Detection Logic (pkg/dockerfile/inputs.go):

• Extended DetectInputsFromDockerfile() with a new baseImages parameter to enable skipping of Dockerfile registry references that are already covered by explicitly configured base images
• Added matchesFromBaseImage() helper function that compares Dockerfile references against the base images map by namespace, repository, and tag to determine if a reference should be skipped

Simplified Registry Extraction (pkg/dockerfile/extract.go):

• Removed the from parameter from ExtractRegistryReferences()
• The function now unconditionally extracts all registry references without special handling for dropping references based on the from configuration

Updated Callers:

• pkg/defaults/defaults.go: Now passes the baseImages map when detecting inputs during pipeline configuration processing
• cmd/registry-replacer/main.go: Simplified to use the new signature of ExtractRegistryReferences()

Impact

CI operators and users defining multi-stage container builds will see more accurate image input detection. The fix ensures that:

• Base images explicitly declared in configuration are not redundantly detected from Dockerfile instructions
• All FROM instructions in multi-stage builds are properly preserved, regardless of which stages are used as the primary build base
• Registry reference detection correctly handles scenarios where the primary build base differs from secondary image references

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors Dockerfile analysis by simplifying registry reference extraction into a pure function and relocating base image context handling into input detection logic. ExtractRegistryReferences now accepts only Dockerfile bytes; DetectInputsFromDockerfile gains a baseImages map parameter to skip references matching known base images. Call sites and tests are updated accordingly.

Changes

Base Image Context in Dockerfile Input Detection

Layer / File(s)	Summary
Simplify ExtractRegistryReferences contract pkg/dockerfile/extract.go	ExtractRegistryReferences removes the PipelineImageStreamTagReference parameter. Function now unconditionally scans Dockerfile for FROM and COPY lines, extracts registry references via RegistryRegex, and de-duplicates them before returning in encounter order.
Enhance DetectInputsFromDockerfile with baseImages context pkg/dockerfile/inputs.go	DetectInputsFromDockerfile adds a baseImages map parameter and uses it to skip detected references matching the provided from base image. Calls simplified ExtractRegistryReferences, constructs detected entries with base-image key and original Dockerfile reference as As field. New matchesFromBaseImage helper safely compares parsed references against baseImages while handling empty or nil cases.
Update call sites for new function signatures cmd/registry-replacer/main.go, pkg/defaults/defaults.go	ensureReplacement calls ExtractRegistryReferences with single argument. processDetectedBaseImages passes baseImages map into DetectInputsFromDockerfile to activate base image matching.
Test coverage for base image matching logic pkg/dockerfile/inputs_test.go	Test case struct adds baseImages field. New test cases exercise from matching base images (with duplicate skipping), multi-stage builds, unrelated stages, and COPY --from handling. Test harness passes tc.baseImages into DetectInputsFromDockerfile.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 15 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Go Error Handling	⚠️ Warning	The matchesFromBaseImage helper silently ignores OrgRepoTagFromPullString errors without justification, inconsistent with error logging at lines 25-28 in the same file.	Add logging or comment justifying silent error handling in matchesFromBaseImage, or log errors consistently with DetectInputsFromDockerfile.

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly summarizes the main change: fixing Dockerfile registry.ci input detection to account for baseImages parameter.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Test Coverage For New Features	✅ Passed	All modified functions include adequate test coverage. 13 table-driven test cases in TestDetectInputsFromDockerfile, including 4 regression tests for new baseImages parameter functionality.
Stable And Deterministic Test Names	✅ Passed	PR uses standard Go testing package (not Ginkgo). The single modified test file contains only static, deterministic test names without dynamic values, timestamps, UUIDs, or interpolation.
Test Structure And Quality	✅ Passed	Tests use standard Go testing (not Ginkgo), making BeforeEach/timeout checks inapplicable. Table-driven tests show single responsibility. Assertions include meaningful messages.
Microshift Test Compatibility	✅ Passed	This PR modifies standard Go unit tests (using the testing package), not Ginkgo e2e tests. The custom check applies only to Ginkgo e2e tests, which are not present in this PR.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PR contains no Ginkgo e2e tests—only unit test changes to pkg/dockerfile/inputs_test.go. SNO compatibility check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	PR modifies only utility code (Dockerfile processing) without deploying manifests or scheduling constraints. Check not applicable.
Ote Binary Stdout Contract	✅ Passed	PR contains no unauthorized stdout writes. Changes are function call adjustments and library code refactoring with no process-level stdout writes added.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PR adds no Ginkgo e2e tests. Only modified test file (pkg/dockerfile/inputs_test.go) uses standard Go unit tests, not Ginkgo.
No-Weak-Crypto	✅ Passed	No weak cryptography detected. PR modifies Dockerfile parsing for registry input detection; no crypto imports, weak algorithms (MD5/SHA1/DES/RC4), or secret comparisons found.
Container-Privileges	✅ Passed	PR contains only Go source files for Dockerfile processing; no container/Kubernetes manifests with privileged configurations are present.
No-Sensitive-Data-In-Logs	✅ Passed	Logging added in pkg/dockerfile/inputs.go logs only non-sensitive registry references (e.g., 'registry.ci.openshift.org/ocp/4.19:base'), not credentials or PII.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[danilo-gemoli]

/lgtm

[coderabbitai]

🧹 Nitpick comments (1)

pkg/dockerfile/inputs.go (1)

9-12: ⚡ Quick win

Update DetectInputsFromDockerfile docs to include baseImages/from skip semantics.

The exported function comment no longer describes the full contract after the new parameter and behavior change.

As per coding guidelines **/*.go: Go documentation on Classes/Functions/Fields should be written properly.

🤖 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 `@pkg/dockerfile/inputs.go` around lines 9 - 12, Update the
DetectInputsFromDockerfile function comment to document the new parameters and
skip semantics: explain that the baseImages map and the from
api.PipelineImageStreamTagReference are used to skip adding certain registry
references (describe which conditions cause a skip, e.g., when a reference is
already present in baseImages or matches from), state that existingInputs
influences detection, and clarify that the returned map keys are base image
names with ImageStreamTagReference.As containing the original Dockerfile
reference; reference the function name DetectInputsFromDockerfile and the
parameters baseImages and from in the comment so callers understand the full
contract.

🤖 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.

Nitpick comments:
In `@pkg/dockerfile/inputs.go`:
- Around line 9-12: Update the DetectInputsFromDockerfile function comment to
document the new parameters and skip semantics: explain that the baseImages map
and the from api.PipelineImageStreamTagReference are used to skip adding certain
registry references (describe which conditions cause a skip, e.g., when a
reference is already present in baseImages or matches from), state that
existingInputs influences detection, and clarify that the returned map keys are
base image names with ImageStreamTagReference.As containing the original
Dockerfile reference; reference the function name DetectInputsFromDockerfile and
the parameters baseImages and from in the comment so callers understand the full
contract.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: ad71ee8a-0ef6-432e-8c1d-ace93f9e5666

📥 Commits

Reviewing files that changed from the base of the PR and between 7b6e7b8 and 5dce40d.

📒 Files selected for processing (5)

• cmd/registry-replacer/main.go
• pkg/defaults/defaults.go
• pkg/dockerfile/extract.go
• pkg/dockerfile/inputs.go
• pkg/dockerfile/inputs_test.go

[deepsm007]

/test e2e

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danilo-gemoli, deepsm007

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [danilo-gemoli,deepsm007]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[deepsm007]

/override ci/prow/images

not related and this PR is critical for CRT

[openshift-ci]

@deepsm007: Overrode contexts on behalf of deepsm007: ci/prow/images

Details

In response to this:

/override ci/prow/images

not related and this PR is critical for CRT

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[openshift-merge-bot]

Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage.

[openshift-ci]

@deepsm007: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.