OCPBUGS-54298: Installer should validate mirror registry pull secret #10239

yunjiang29 · 2026-01-20T02:06:01Z

Log a warning if mirror registry host not present in pullSecret:

time="2026-01-20T16:18:59+08:00" level=warning msg="Mirror registry \"not-in-pullsecret-mirror.example.com:5000\" is not found in pullSecret"

openshift-ci-robot · 2026-01-20T02:06:07Z

@yunjiang29: This pull request references Jira Issue OCPBUGS-54298, which is invalid:

expected the bug to target the "4.22.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Log a warning if mirror registry host not present in pullSecret.

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 openshift-eng/jira-lifecycle-plugin repository.

yunjiang29 · 2026-01-20T02:07:13Z

/jira refresh

openshift-ci-robot · 2026-01-20T02:07:21Z

@yunjiang29: This pull request references Jira Issue OCPBUGS-54298, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.22.0) matches configured target version for branch (4.22.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @gpei

Details

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

yunjiang29 · 2026-01-20T05:07:22Z

/payload-job periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-ipi-disc-priv-sts-ep-fips-f14

openshift-ci · 2026-01-20T05:07:28Z

@yunjiang29: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-ipi-disc-priv-sts-ep-fips-f14

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/e7df98e0-f5bd-11f0-84b6-fc38317e86bc-0

yunjiang29 · 2026-01-20T08:18:00Z

periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-ipi-disc-priv-sts-ep-fips-f14 install succeeded, this job enabled imageContentSources and no warning printed in the install log.

openshift-ci-robot · 2026-01-20T08:20:01Z

@yunjiang29: This pull request references Jira Issue OCPBUGS-54298, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.22.0) matches configured target version for branch (4.22.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @gpei

Details

In response to this:

Log a warning if mirror registry host not present in pullSecret:
time="2026-01-20T16:18:59+08:00" level=warning msg="Mirror registry \"not-in-pullsecret-mirror.example.com:5000\" is not found in pullSecret"

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 openshift-eng/jira-lifecycle-plugin repository.

gpei · 2026-01-22T09:55:35Z

/verified by @gpei

Validation handles multiple mirrors correctly

pullSecret: '{"auths":{"mirror1.example.com":{"auth":"test1"},"mirror2.example.com":{"auth":"test2"}}}'
imageContentSources:
- mirrors:
  - mirror1.example.com/ocp/release
  source: quay.io/ocp/release
- mirrors:
  - mirror2.example.com/ocp/release
  source: quay.io/ocp/release
- mirrors:
  - mirror3.example.com/ocp/release
  source: quay.io/ocp/release
- mirrors:
  - mirror4.example.com/ocp/release
  source: quay.io/ocp/release

$ ./openshift-install create manifests --dir 0122e
WARNING Mirror registry "mirror3.example.com" is not found in pullSecret 
WARNING Mirror registry "mirror4.example.com" is not found in pullSecret

The same mirror registry only generates one warning even if used multiple times

   pullSecret: '{"auths":{"other.example.com":{"auth":"test"}}}'
   imageContentSources:
   - mirrors:
     - duplicate-mirror.example.com/ocp/release1
     source: quay.io/ocp/release1
   - mirrors:
     - duplicate-mirror.example.com/ocp/release2
     source: quay.io/ocp/release2
   - mirrors:
     - duplicate-mirror.example.com/ocp/release3
     source: quay.io/ocp/release3

$ ./openshift-install create manifests --dir 0122h
WARNING Mirror registry "duplicate-mirror.example.com" is not found in pullSecret

imageDigestSources Validation


pullSecret: '{"auths":{"mirror1.example.com":{"auth":"test"}}}'
imageDigestSources:
- mirrors:
  - mirror1.example.com/ocp/release
  source: quay.io/ocp/release3
- mirrors:
  - missing-digest.example.com/ocp/release
  source: quay.io/ocp/release4

$ ./openshift-install create manifests --dir 0122d
WARNING Mirror registry "missing-digest.example.com" is not found in pullSecret

Verify validation works with IP address-based and port number mirror registry

pullSecret: '{"auths":{"192.168.1.100:5000":{"auth":"test"}}}'
imageContentSources:
- mirrors:
  - 192.168.1.100:5000/ocp/release
  source: quay.io/ocp/release

No warning msg from installer.

openshift-ci-robot · 2026-01-22T09:55:47Z

@gpei: This PR has been marked as verified by @gpei.

Details

In response to this:

/verified by @gpei

Validation handles multiple mirrors correctly

pullSecret: '{"auths":{"mirror1.example.com":{"auth":"test1"},"mirror2.example.com":{"auth":"test2"}}}'
imageContentSources:
- mirrors:
 - mirror1.example.com/ocp/release
 source: quay.io/ocp/release
- mirrors:
 - mirror2.example.com/ocp/release
 source: quay.io/ocp/release
- mirrors:
 - mirror3.example.com/ocp/release
 source: quay.io/ocp/release
- mirrors:
 - mirror4.example.com/ocp/release
 source: quay.io/ocp/release

$ ./openshift-install create manifests --dir 0122e
WARNING Mirror registry "mirror3.example.com" is not found in pullSecret 
WARNING Mirror registry "mirror4.example.com" is not found in pullSecret

The same mirror registry only generates one warning even if used multiple times

  pullSecret: '{"auths":{"other.example.com":{"auth":"test"}}}'
  imageContentSources:
  - mirrors:
    - duplicate-mirror.example.com/ocp/release1
    source: quay.io/ocp/release1
  - mirrors:
    - duplicate-mirror.example.com/ocp/release2
    source: quay.io/ocp/release2
  - mirrors:
    - duplicate-mirror.example.com/ocp/release3
    source: quay.io/ocp/release3

$ ./openshift-install create manifests --dir 0122h
WARNING Mirror registry "duplicate-mirror.example.com" is not found in pullSecret

imageDigestSources Validation


pullSecret: '{"auths":{"mirror1.example.com":{"auth":"test"}}}'
imageDigestSources:
- mirrors:
 - mirror1.example.com/ocp/release
 source: quay.io/ocp/release3
- mirrors:
 - missing-digest.example.com/ocp/release
 source: quay.io/ocp/release4

$ ./openshift-install create manifests --dir 0122d
WARNING Mirror registry "missing-digest.example.com" is not found in pullSecret

Verify validation works with IP address-based and port number mirror registry

pullSecret: '{"auths":{"192.168.1.100:5000":{"auth":"test"}}}'
imageContentSources:
- mirrors:
 - 192.168.1.100:5000/ocp/release
 source: quay.io/ocp/release

No warning msg from installer.

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 openshift-eng/jira-lifecycle-plugin repository.

tthvo

Great, thank you! I just have a few suggestions :D

tthvo · 2026-01-24T00:04:55Z

pkg/types/validation/installconfig.go

+	if errors.Is(err, dockerref.ErrNameNotCanonical) {
+		host, port, err := net.SplitHostPort(repository)
+		if err != nil {
+			return repository, nil
+		}
+		return net.JoinHostPort(host, port), nil
+	}


I think this part eventually returns the registry value itself, right?

If registry is host:port, split successfully but then rejoin to the same original value.

Otherwise fails, and return the original value.

How about simplifying it a tiny bit?

// extractRegistryHost extracts the registry host (with port if any) from a repository string. // For example: "registry.example.com:5000/namespace/repo" -> "registry.example.com:5000". // Returns an error if the repository string cannot be parsed as either a named reference or a host. func extractRegistryHost(repository string) (string, error) { ref, err := dockerref.ParseNamed(repository) if err != nil { // ErrNameNotCanonical indicates the input is not a fully-qualified repository reference // (e.g., "registry.example.com:5000" without a path, or short names like "ocp/release"). // In these cases, return the input as-is. if errors.Is(err, dockerref.ErrNameNotCanonical) { return repository, nil } return "", err } return dockerref.Domain(ref), nil }

tthvo · 2026-01-24T04:04:35Z

pkg/types/validation/installconfig.go

+	var ps imagePullSecret
+	validPullSecret := false
+	mirrorHostsMissingSecret := []string{}
+
+	if err := validate.ImagePullSecret(pullSecret); err == nil {
+		if err := json.Unmarshal([]byte(pullSecret), &ps); err == nil {
+			validPullSecret = true
+		}
+	}


I notice we have duplicate logic for validateImageContentSources and validateImageDigestSources :D

How about extracting it into its own validation func? This way, if we want to return error later, we can just easily do so.

// validateMirrorCredentials checks if mirror registry hosts are present in the pull secret func validateMirrorCredentials(mirrors []string, pullSecret string) field.ErrorList { allErrs := field.ErrorList{} var ps imagePullSecret if err := validate.ImagePullSecret(pullSecret); err != nil { return allErrs } if err := json.Unmarshal([]byte(pullSecret), &ps); err != nil { return allErrs } missingHosts := sets.New[string]() for _, mirror := range mirrors { mirrorHost, err := extractRegistryHost(mirror) if err != nil { continue // Skip if we can't extract the host } if _, found := ps.Auths[mirrorHost]; !found { missingHosts.Insert(mirrorHost) } } for host := range missingHosts { // Log warnings for registries without credentials // FIXME: We should instead report it as errors logrus.Warnf("Mirror registry %q is not found in pullSecret", host) } return allErrs }

Then we can call the function, for example:

func validateImageDigestSources(groups []types.ImageDigestSource, pullSecret string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} var allMirrors []string for gidx, group := range groups { // ... code-omitted ... for midx, mirror := range group.Mirrors { // ... code-omitted ... allMirrors = append(allMirrors, mirror) } // ... code-omitted ... } allErrs = append(allErrs, validateMirrorCredentials(allMirrors, pullSecret)...) return allErrs }

@tthvo thanks for you detailed feedback, I've incorporated your suggestions

// Log warnings for registries without credentials // FIXME: We should instead report it as errors logrus.Warnf("Mirror registry %q is not found in pullSecret", host)

As per's Zane's suggestion described in the bug, a warning is good for the first version (we can upgrade to an error later), the warning is sufficient to help us debugging.

Thank you! Sounds good to me!

tthvo · 2026-01-24T04:16:14Z

pkg/types/validation/installconfig_test.go

+			}(),
+		},
+		{
+			name: "valid imageContentSources with mirror not in pull secret",


nit: I think we might want to be clear that it does not throw the error and it just shows warning, but technically invalid right? How about naming them like:

imageContentSources with mirror not in pull secret - warning only

tthvo

/approve

LGTM! We need to fix some strict golint complaints though 👇

 pkg/types/validation/installconfig.go:1723:1: Comment should end in a period (godot)
// validateMirrorCredentials checks if mirror registry hosts are present in the pull secret
^

tthvo · 2026-01-26T20:35:34Z

/cc @patrickdillon @zaneb

openshift-ci · 2026-01-26T20:35:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tthvo

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~~ [tthvo]

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

Log a warning if mirror registry host not present in pullSecret.

openshift-ci · 2026-01-27T06:29:22Z

@yunjiang29: 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.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 20, 2026

openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Jan 20, 2026

openshift-ci bot requested review from jhixson74 and tthvo January 20, 2026 02:06

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jan 20, 2026

openshift-ci bot requested a review from gpei January 20, 2026 02:07

yunjiang29 force-pushed the valid-mirror-host-in-ps branch 2 times, most recently from b6c199f to e902733 Compare January 20, 2026 02:34

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jan 22, 2026

tthvo reviewed Jan 24, 2026

View reviewed changes

yunjiang29 force-pushed the valid-mirror-host-in-ps branch from e902733 to f7d69af Compare January 26, 2026 10:42

openshift-ci-robot removed the verified Signifies that the PR passed pre-merge verification criteria label Jan 26, 2026

tthvo reviewed Jan 26, 2026

View reviewed changes

openshift-ci bot requested review from patrickdillon and zaneb January 26, 2026 20:35

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 26, 2026

OCPBUGS-54298: Installer should validate mirror registry pull secret

ad04ec1

Log a warning if mirror registry host not present in pullSecret.

yunjiang29 force-pushed the valid-mirror-host-in-ps branch from f7d69af to ad04ec1 Compare January 27, 2026 02:53

OCPBUGS-54298: Installer should validate mirror registry pull secret #10239

Are you sure you want to change the base?

OCPBUGS-54298: Installer should validate mirror registry pull secret #10239

Conversation

yunjiang29 commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jan 20, 2026

Uh oh!

yunjiang29 commented Jan 20, 2026

Uh oh!

openshift-ci-robot commented Jan 20, 2026

Uh oh!

yunjiang29 commented Jan 20, 2026

Uh oh!

openshift-ci bot commented Jan 20, 2026

Uh oh!

yunjiang29 commented Jan 20, 2026

Uh oh!

openshift-ci-robot commented Jan 20, 2026

Uh oh!

gpei commented Jan 22, 2026

Uh oh!

openshift-ci-robot commented Jan 22, 2026

Uh oh!

tthvo left a comment

Choose a reason for hiding this comment

Uh oh!

tthvo Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

tthvo Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

yunjiang29 Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

tthvo Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

tthvo Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

tthvo left a comment

Choose a reason for hiding this comment

Uh oh!

tthvo commented Jan 26, 2026

Uh oh!

openshift-ci bot commented Jan 26, 2026

Uh oh!

openshift-ci bot commented Jan 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yunjiang29 commented Jan 20, 2026 •

edited

Loading