Skip to content

OCPBUGS-56492: Fix CatalogSource image check when unauthorized #6192

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.

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

Merged
merged 1 commit into from
May 28, 2025

Conversation

jparrill
Copy link
Contributor

@jparrill jparrill commented May 26, 2025

What this PR does / why we need it

This PR fixes 3 issues:

  • Failing and blocking the HostedCluster provisioning when a needed image is unauthorized to be pulled
  • Overriding the registry once an entry matches just the registry root on the catalogSources
  • Fallback on the original ImageReference once the registryOverrides does not work as expected.

It also includes the test case testing the fallback of a unauthorized pull of an image and the refactor of a test function to be mantible and readable

Which issue(s) this PR fixes

@openshift-ci-robot openshift-ci-robot added jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels May 26, 2025
@openshift-ci-robot
Copy link

@jparrill: This pull request references Jira Issue OCPBUGS-56492, which is invalid:

  • expected the bug to target the "4.20.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.

In response to this:

What this PR does / why we need it

This PR fixes 2 issues:

  • The one failing and blocking the HostedCluster provisioning when a registry is unauthorized
  • An issue overriding the registry once an entry matches just the registry root
  • Refactored the function test to be more readable

Which issue(s) this PR fixes

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.

@openshift-ci openshift-ci bot requested review from hasueki and rtheis May 26, 2025 17:27
@openshift-ci openshift-ci bot added area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/needs-area labels May 26, 2025
@jparrill
Copy link
Contributor Author

/jira refresh

@openshift-ci-robot 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 May 26, 2025
@openshift-ci-robot
Copy link

@jparrill: This pull request references Jira Issue OCPBUGS-56492, 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.20.0) matches configured target version for branch (4.20.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira ([email protected]), skipping review request.

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.

@cwbotbot
Copy link

cwbotbot commented May 26, 2025

Test Results

e2e-aws

e2e-aks

@sdminonne
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2025
@jparrill
Copy link
Contributor Author

/hold

Until the fallback image of the OLM catalogs is set to the original one.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 27, 2025
This PR fixes 3 issues:

- Failing and blocking the HostedCluster provisioning when a needed
  image is unauthorized to be pulled
- Overriding the registry once an entry matches just the registry root
  on the catalogSources
- Fallback on the original ImageReference once the registryOverrides
  does not work as expected.

Also includes the test case testing the fallback of a unauthorized pull
of an image and the refactor of a test function to be mantible and readable

Signed-off-by: Juan Manuel Parrilla Madrid <[email protected]>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 27, 2025
@openshift-ci-robot
Copy link

@jparrill: This pull request references Jira Issue OCPBUGS-56492, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.20.0) matches configured target version for branch (4.20.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira ([email protected]), skipping review request.

In response to this:

What this PR does / why we need it

This PR fixes 3 issues:

  • Failing and blocking the HostedCluster provisioning when a needed image is unauthorized to be pulled
  • Overriding the registry once an entry matches just the registry root on the catalogSources
  • Fallback on the original ImageReference once the registryOverrides does not work as expected.

It also includes the test case testing the fallback of a unauthorized pull of an image and the refactor of a test function to be mantible and readable

Which issue(s) this PR fixes

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.

Copy link
Contributor

@sdminonne sdminonne left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2025
Copy link
Contributor

openshift-ci bot commented May 27, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jparrill, sdminonne

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@jparrill
Copy link
Contributor Author

jparrill commented May 27, 2025

/retest

Konflux and AKS...

@jparrill
Copy link
Contributor Author

jparrill commented May 27, 2025

/retest

Konflux

@jparrill
Copy link
Contributor Author

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 27, 2025
Copy link
Contributor

openshift-ci bot commented May 27, 2025

@jparrill: all tests passed!

Full PR test history. Your PR dashboard.

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.

@jparrill
Copy link
Contributor Author

jparrill commented May 27, 2025

/retest

Konflux again...

@mgencur
Copy link
Contributor

mgencur commented May 28, 2025

/lgtm

@jparrill
Copy link
Contributor Author

/retest

Comment on lines +127 to +128
defaultRegistryURL := "registry.redhat.io"
defaultRegistryNamespace := "redhat"
Copy link
Contributor

Choose a reason for hiding this comment

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

can we move those to a constant at the beginning of the file for visibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do in a follow up PR. This blocks the disconnected testing on QE

Copy link
Contributor

Choose a reason for hiding this comment

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

sure!

@jparrill
Copy link
Contributor Author

/retest

@celebdor
Copy link
Collaborator

/override Red Hat Konflux / hypershift-operator-main-on-pull-request

Copy link
Contributor

openshift-ci bot commented May 28, 2025

@celebdor: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

  • /
  • Hat
  • Konflux
  • Red
  • hypershift-operator-main-on-pull-request

Only the following failed contexts/checkruns were expected:

  • Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main
  • Red Hat Konflux / hypershift-operator-main-on-pull-request
  • ci/prow/e2e-aks
  • ci/prow/e2e-aws
  • ci/prow/e2e-aws-upgrade-hypershift-operator
  • ci/prow/e2e-kubevirt-aws-ovn-reduced
  • ci/prow/images
  • ci/prow/okd-scos-e2e-aws-ovn
  • ci/prow/security
  • ci/prow/unit
  • ci/prow/verify
  • ci/prow/verify-deps
  • pull-ci-openshift-hypershift-main-e2e-aks
  • pull-ci-openshift-hypershift-main-e2e-aws
  • pull-ci-openshift-hypershift-main-e2e-aws-upgrade-hypershift-operator
  • pull-ci-openshift-hypershift-main-e2e-kubevirt-aws-ovn-reduced
  • pull-ci-openshift-hypershift-main-images
  • pull-ci-openshift-hypershift-main-okd-scos-e2e-aws-ovn
  • pull-ci-openshift-hypershift-main-security
  • pull-ci-openshift-hypershift-main-unit
  • pull-ci-openshift-hypershift-main-verify
  • pull-ci-openshift-hypershift-main-verify-deps
  • tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override Red Hat Konflux / hypershift-operator-main-on-pull-request

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.

@celebdor
Copy link
Collaborator

/override "Red Hat Konflux / hypershift-operator-main-on-pull-request"

Copy link
Contributor

openshift-ci bot commented May 28, 2025

@celebdor: Overrode contexts on behalf of celebdor: Red Hat Konflux / hypershift-operator-main-on-pull-request

In response to this:

/override "Red Hat Konflux / hypershift-operator-main-on-pull-request"

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.

@celebdor
Copy link
Collaborator

/override "Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main"

Copy link
Contributor

openshift-ci bot commented May 28, 2025

@celebdor: Overrode contexts on behalf of celebdor: Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main

In response to this:

/override "Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main"

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 openshift-merge-bot bot merged commit c256d32 into openshift:main May 28, 2025
17 of 19 checks passed
@openshift-ci-robot
Copy link

@jparrill: Jira Issue OCPBUGS-56492: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-56492 has been moved to the MODIFIED state.

In response to this:

What this PR does / why we need it

This PR fixes 3 issues:

  • Failing and blocking the HostedCluster provisioning when a needed image is unauthorized to be pulled
  • Overriding the registry once an entry matches just the registry root on the catalogSources
  • Fallback on the original ImageReference once the registryOverrides does not work as expected.

It also includes the test case testing the fallback of a unauthorized pull of an image and the refactor of a test function to be mantible and readable

Which issue(s) this PR fixes

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.

@jparrill
Copy link
Contributor Author

/jira backport release-4.19

@openshift-ci-robot
Copy link

@jparrill: The following backport issues have been created:

Queuing cherrypicks to the requested branches to be created after this PR merges:
/cherrypick release-4.19

In response to this:

/jira backport release-4.19

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.

@openshift-cherrypick-robot

@openshift-ci-robot: new pull request created: #6198

In response to this:

@jparrill: The following backport issues have been created:

Queuing cherrypicks to the requested branches to be created after this PR merges:
/cherrypick release-4.19

In response to this:

/jira backport release-4.19

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.

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-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: hypershift
This PR has been included in build ose-hypershift-container-v4.20.0-202505281454.p0.gc256d32.assembly.stream.el9.
All builds following this will include this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants