Skip to content

CLID-629: Add integration test for manifest list sub-digests in dry-run mapping.txt - #1491

Open
adolfo-ab wants to merge 1 commit into
openshift:mainfrom
adolfo-ab:CLID-629
Open

CLID-629: Add integration test for manifest list sub-digests in dry-run mapping.txt#1491
adolfo-ab wants to merge 1 commit into
openshift:mainfrom
adolfo-ab:CLID-629

Conversation

@adolfo-ab

@adolfo-ab adolfo-ab commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Github / Jira issue:

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code Improvements (Refactoring, Performance, CI upgrades, etc)
  • Internal repo assets (diagrams / docs on github repo)
  • This change requires a documentation update on openshift docs

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

Expected Outcome

Please describe the outcome expected from the tests.

Summary by CodeRabbit

  • Tests
    • Added integration coverage for dry-run manifest-list mappings.
    • Added validation for mappings between multi-architecture images and their platform-specific sub-manifests.
    • Added test coverage for ARM64 and PPC64LE image variants.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

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: LGTM mode

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

openshift-ci-robot commented Aug 6, 2026

Copy link
Copy Markdown

@adolfo-ab: This pull request references CLID-629 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target either version "5.0." or "openshift-5.0.", but it targets "openshift-5.1" instead.

Details

In response to this:

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Github / Jira issue:

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code Improvements (Refactoring, Performance, CI upgrades, etc)
  • Internal repo assets (diagrams / docs on github repo)
  • This change requires a documentation update on openshift docs

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

Expected Outcome

Please describe the outcome expected from the tests.

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 aguidirh and dorzel August 6, 2026 07:58
@openshift-ci

openshift-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adolfo-ab

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:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 6, 2026
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Walkthrough

The integration tests add a manifest-list dry-run scenario. The test fixture defines a multi-architecture additional image. Helpers parse mapping.txt, validate top-level and sub-manifest mappings, and normalize image references.

Changes

Manifest-list dry-run validation

Layer / File(s) Summary
Manifest-list dry-run scenario
tests/integration/dry_run_test.go, tests/integration/testdata/imagesetconfigs/dry_run/isc-manifest-list.yaml
The fixture and integration test cover pinned manifest-list, ARM64, and PPC64LE sub-manifest digests.
Mapping parsing and reference normalization
tests/integration/dry_run_test.go
The test parses mapping.txt, checks destination repositories and digests, and removes tags or digests without misinterpreting registry ports.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Ipv6 And Disconnected Network Test Compatibility ⚠️ Warning The new Ginkgo It() uses a pinned image from quay.io, and MirrorToDisk passes the source unchanged; it requires public registry access. IPv6 and disconnected network compatibility notice: run /payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-metal-ipi-ovn-ipv6; mirror the image internally or add [Skipped:Disconnected].
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the integration test for manifest-list sub-digests in dry-run mapping.txt.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Stable And Deterministic Test Names ✅ Passed All Ginkgo titles in the changed test use static string literals; none contain runtime values, generated identifiers, dates, node or namespace names, or addresses.
Test Structure And Quality ✅ Passed The new It block has one mapping-focused responsibility, uses existing BeforeEach/AfterEach cleanup, passes the suite-timeout context, has no cluster waits, and adds diagnostic assertion messages.
Microshift Test Compatibility ✅ Passed The new Ginkgo test runs oc-mirror against a local registry and disk workspace; it uses no MicroShift-unavailable OpenShift kube APIs, namespaces, or multi-node assumptions.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The new Ginkgo test only runs oc-mirror against a temporary directory and an in-process local registry; it makes no multi-node or HA assumptions.
Topology-Aware Scheduling Compatibility ✅ Passed The commit adds only an integration test and an ImageSetConfiguration with an additional image; it adds no deployment, controller, operator, or scheduling constraint.
Ote Binary Stdout Contract ✅ Passed The PR adds only Ginkgo test registration, test-body assertions, file parsing, and a YAML fixture; it adds no process-level stdout writes or logging setup changes.
No-Weak-Crypto ✅ Passed The PR adds only SHA-256 image digest strings and mapping helpers; searches found no MD5, SHA-1, DES, RC4, Blowfish, ECB, crypto APIs, custom crypto, or secret comparisons.
Container-Privileges ✅ Passed The two changed files are a Go integration test and an ImageSetConfiguration fixture; the commit contains no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or root security settings.
No-Sensitive-Data-In-Logs ✅ Passed The new test reports only temporary paths and public quay.io image digests; it adds no passwords, tokens, API keys, PII, hostnames, or customer data to logs.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/integration/dry_run_test.go (1)

110-118: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Assert the complete top-level mapping.

The helper checks that topLevelSource exists, then uses the destination read from mapping.txt as the expected base for every sub-digest. A wrong top-level repository can therefore pass if the sub-digest entries use the same wrong repository. Pass the expected top-level destination to expectMappingContainsManifestListSubDigests and assert the complete top-level mapping before deriving destBase.

🤖 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 `@tests/integration/dry_run_test.go` around lines 110 - 118, Update
expectMappingContainsManifestListSubDigests to accept the expected top-level
destination, assert the complete topLevelSource-to-destination mapping with that
value, and only then derive destBase from the expected destination rather than
the mapping file’s value. Update all callers to pass the expected destination.
🤖 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 `@tests/integration/dry_run_test.go`:
- Around line 110-118: Update expectMappingContainsManifestListSubDigests to
accept the expected top-level destination, assert the complete
topLevelSource-to-destination mapping with that value, and only then derive
destBase from the expected destination rather than the mapping file’s value.
Update all callers to pass the expected destination.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f3bef65-7977-4551-800b-4ac7b3e31a91

📥 Commits

Reviewing files that changed from the base of the PR and between 75a5e05 and 8aac391.

📒 Files selected for processing (2)
  • tests/integration/dry_run_test.go
  • tests/integration/testdata/imagesetconfigs/dry_run/isc-manifest-list.yaml

@openshift-ci

openshift-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown

@adolfo-ab: 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.

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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants