Skip to content

ci-operator: add automatic dockerfile inputs detection #4851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Prucek wants to merge 3 commits into
base: main
Choose a base branch
from
+1,057 −0
Open

ci-operator: add automatic dockerfile inputs detection #4851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
921ed96
pkg/dockerfile: add registry-replacer functionality to a separate pkg
Prucek Nov 24, 2025
03baaa0
pkg/default: use pkg/dockerfile in project images for base_images det…
Prucek Nov 24, 2025
966e16b
test/e2e: add dockerfile-inputs
Prucek Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions pkg/defaults/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (

"github.com/openshift/ci-tools/pkg/api"
"github.com/openshift/ci-tools/pkg/api/configresolver"
"github.com/openshift/ci-tools/pkg/dockerfile"
"github.com/openshift/ci-tools/pkg/kubernetes"
"github.com/openshift/ci-tools/pkg/labeledclient"
"github.com/openshift/ci-tools/pkg/lease"
Expand Down Expand Up @@ -1035,9 +1036,128 @@ func runtimeStepConfigsForBuild(

buildSteps = append(buildSteps, getSourceStepsForJobSpec(jobSpec, injectedTest)...)

// Detect Dockerfile inputs for project images and add InputImageTagStepConfiguration entries
dockerfileInputSteps, err := detectDockerfileInputs(config, readFile)
if err != nil {
return nil, fmt.Errorf("failed to detect Dockerfile inputs: %w", err)
}
buildSteps = append(buildSteps, dockerfileInputSteps...)
Comment on lines +1039 to +1044
Copy link

@coderabbitai coderabbitai bot Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Detection errors should not fail the build graph

Per earlier feedback, detectDockerfileInputs errors should be logged as warnings, not propagated as fatal errors. This keeps the auto-detection feature additive without breaking existing configurations.

🔎 Proposed fix 
 	// Detect Dockerfile inputs for project images and add InputImageTagStepConfiguration entries
 	dockerfileInputSteps, err := detectDockerfileInputs(config, readFile)
 	if err != nil {
-		return nil, fmt.Errorf("failed to detect Dockerfile inputs: %w", err)
+		logrus.WithError(err).Warn("Dockerfile inputs detection failed; continuing without auto-detected base images")
+	} else {
+		buildSteps = append(buildSteps, dockerfileInputSteps...)
 	}
-	buildSteps = append(buildSteps, dockerfileInputSteps...)
📝 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.

Suggested change
// Detect Dockerfile inputs for project images and add InputImageTagStepConfiguration entries
dockerfileInputSteps, err := detectDockerfileInputs(config, readFile)
if err != nil {
return nil, fmt.Errorf("failed to detect Dockerfile inputs: %w", err)
}
buildSteps = append(buildSteps, dockerfileInputSteps...)
// Detect Dockerfile inputs for project images and add InputImageTagStepConfiguration entries
dockerfileInputSteps, err := detectDockerfileInputs(config, readFile)
if err != nil {
logrus.WithError(err).Warn("Dockerfile inputs detection failed; continuing without auto-detected base images")
} else {
buildSteps = append(buildSteps, dockerfileInputSteps...)
}
🤖 Prompt for AI Agents 
In @pkg/defaults/defaults.go around lines 1039 - 1044, The call to
detectDockerfileInputs currently propagates errors and aborts graph
construction; change it to treat errors as non-fatal: call
detectDockerfileInputs(config, readFile), and if it returns an error log a
warning (using the package's logger) that includes the error message and
context, do not return the error, and only append returned dockerfileInputSteps
to buildSteps when non-nil/non-empty so auto-detection stays additive rather
than failing the build graph.


Comment on lines +1039 to +1045
Copy link

@coderabbitai coderabbitai bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make Dockerfile-input detection best-effort instead of failing the build graph

Right now any error from detectDockerfileInputs (e.g., an unexpected registry reference that DetectInputsFromDockerfile cannot parse) will cause runtimeStepConfigsForBuild to return an error and abort graph construction. That’s a behavior change compared to today, where unusual but syntactically valid Dockerfiles still run, just without auto-detected inputs.

Given this feature is an additive convenience, it’s safer to degrade gracefully on detection errors and continue without auto-generated InputImageTagStepConfigurations. Otherwise, a single odd-looking reference to registry.ci.openshift.org/quay-proxy.ci.openshift.org can break the entire job.

Consider handling errors like this instead:

-	// Detect Dockerfile inputs for project images and add InputImageTagStepConfiguration entries
-	dockerfileInputSteps, err := detectDockerfileInputs(config, readFile)
-	if err != nil {
-		return nil, fmt.Errorf("failed to detect Dockerfile inputs: %w", err)
-	}
-	buildSteps = append(buildSteps, dockerfileInputSteps...)
+	// Detect Dockerfile inputs for project images and add InputImageTagStepConfiguration entries.
+	// This is best-effort and should not break existing configurations.
+	dockerfileInputSteps, err := detectDockerfileInputs(config, readFile)
+	if err != nil {
+		logrus.WithError(err).Warn("Dockerfile inputs detection failed; continuing without auto-detected base images")
+	} else {
+		buildSteps = append(buildSteps, dockerfileInputSteps...)
+	}

This keeps the new functionality while avoiding regressions for configs with unusual but valid image references.

Also applies to: 1042-1131

🤖 Prompt for AI Agents 
In pkg/defaults/defaults.go around lines 1029 to 1035 (and similarly for
1042–1131), the call to detectDockerfileInputs currently returns an error that
aborts graph construction; make detection best‑effort by catching and logging
the error instead of returning it. Change the code to call
detectDockerfileInputs, and if it returns an error, write a warning-level log or
comment (including the error message) and continue without appending
dockerfileInputSteps; only return on fatal/unexpected errors, not on
parse/registry detection failures. Ensure the same non-fatal behavior is applied
to the other block(s) noted (1042–1131).

return buildSteps, nil
}

// detectDockerfileInputs reads Dockerfiles for project images and detects registry.ci.openshift.org
// references that should be added as base images
func detectDockerfileInputs(config *api.ReleaseBuildConfiguration, readFile readFile) ([]api.StepConfiguration, error) {
var steps []api.StepConfiguration

for i := range config.Images {
image := &config.Images[i]
dockerfileContent, dockerfilePath, err := readDockerfileForImage(image, readFile)
if err != nil {
logrus.WithError(err).WithField("image", image.To).Debug("Failed to read Dockerfile for input detection, skipping")
continue
}
if dockerfileContent == nil {
continue
}

imageSteps, err := processDetectedBaseImages(config, image, dockerfileContent, dockerfilePath)
if err != nil {
return nil, err
}
steps = append(steps, imageSteps...)
}

return steps, nil
}

// readDockerfileForImage reads the Dockerfile content for a given image configuration
// Returns content, a description of the source, and any error
func readDockerfileForImage(image *api.ProjectDirectoryImageBuildStepConfiguration, readFile readFile) ([]byte, string, error) {
Copy link
Contributor

@danilo-gemoli danilo-gemoli Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nesting levels of this body can be decreased by doing so:

func readDockerfileForImage(image *api.ProjectDirectoryImageBuildStepConfiguration, readFile readFile) ([]byte, string, error) {
  if image.DockerfileLiteral != nil {
    return []byte(*image.DockerfileLiteral), "dockerfile_literal"
  }
  // Everything else here, no `else`
}


if image.DockerfileLiteral != nil {
return []byte(*image.DockerfileLiteral), "dockerfile_literal", nil
}
dockerfilePath := "Dockerfile"
if image.DockerfilePath != "" {
dockerfilePath = image.DockerfilePath
}
if image.ContextDir != "" {
dockerfilePath = fmt.Sprintf("%s/%s", image.ContextDir, dockerfilePath)
}

dockerfileContent, err := readFile(dockerfilePath)
if err != nil {
return nil, dockerfilePath, fmt.Errorf("failed to read Dockerfile at %s: %w", dockerfilePath, err)
}

return dockerfileContent, dockerfilePath, nil
}

// processDetectedBaseImages detects base images from a Dockerfile and updates the configuration
func processDetectedBaseImages(
config *api.ReleaseBuildConfiguration,
image *api.ProjectDirectoryImageBuildStepConfiguration,
dockerfileContent []byte,
dockerfilePath string,
) ([]api.StepConfiguration, error) {
detectedBaseImages, err := dockerfile.DetectInputsFromDockerfile(dockerfileContent, image.Inputs)
if err != nil {
return nil, fmt.Errorf("failed to detect inputs from Dockerfile %s: %w", dockerfilePath, err)
}
if len(detectedBaseImages) == 0 {
return nil, nil
}

if image.Inputs == nil {
image.Inputs = make(map[string]api.ImageBuildInputs)
}

var steps []api.StepConfiguration

for alias, baseImage := range detectedBaseImages {
if config.BaseImages == nil {
config.BaseImages = make(map[string]api.ImageStreamTagReference)
}

if presentAlias, presentBaseImage := isBaseImagePresent(config, baseImage); presentAlias != "" {
image.Inputs[presentAlias] = api.ImageBuildInputs{
As: []string{presentBaseImage.As},
}
logrus.Infof("Detected base image for \"%s\" from \"%s\" matches existing base image \"%s\"", image.To, dockerfilePath, presentAlias)
continue
}

config.BaseImages[alias] = baseImage
image.Inputs[alias] = api.ImageBuildInputs{
As: []string{baseImage.As},
}

stepConfig := api.InputImageTagStepConfiguration{
InputImage: api.InputImage{
BaseImage: defaultImageFromReleaseTag(alias, baseImage, config.ReleaseTagConfiguration),
To: api.PipelineImageStreamTagReference(alias),
},
Sources: []api.ImageStreamSource{{SourceType: api.ImageStreamSourceBase, Name: alias}},
}
steps = append(steps, api.StepConfiguration{InputImageTagStepConfiguration: &stepConfig})

logrus.Infof("Detected base image for \"%s\" from \"%s\": will tag into pipeline:%s", image.To, dockerfilePath, alias)
}

return steps, nil
}

func isBaseImagePresent(config *api.ReleaseBuildConfiguration, baseImage api.ImageStreamTagReference) (string, api.ImageStreamTagReference) {
for presentAlias, presentBaseImage := range config.BaseImages {
if baseImage.Name == presentBaseImage.Name && baseImage.Namespace == presentBaseImage.Namespace && baseImage.Tag == presentBaseImage.Tag {
return presentAlias, presentBaseImage
}
}
return "", api.ImageStreamTagReference{}
}

func getSourceStepsForJobSpec(jobSpec *api.JobSpec, injectedTest bool) []api.StepConfiguration {
var sourceSteps []api.StepConfiguration
primaryRef := determinePrimaryRef(jobSpec, injectedTest)
Expand Down
Loading