Skip to content

ROX-32714: Update groovy tests for ImageV2#19972

Open
charmik-redhat wants to merge 2 commits intomasterfrom
ROX-32714/update-groovy-tests-to-work-with-imagev2
Open

ROX-32714: Update groovy tests for ImageV2#19972
charmik-redhat wants to merge 2 commits intomasterfrom
ROX-32714/update-groovy-tests-to-work-with-imagev2

Conversation

@charmik-redhat
Copy link
Copy Markdown
Contributor

Description

Update groovy E2E tests under qa-tests-backend/ to work correctly when
FlattenImageData is enabled (ImageV2).

When FlattenImageData is enabled, images are stored in a new images_v2
table and their IDs change from the image digest (SHA) to a deterministic
UUIDv5 derived from the full image name and digest. These changes update
the tests to handle both ID schemes:

  • Add Helpers.generateImageV2ID() and newV5FromNonUUIDs() to mirror the
    Go-side pkg/images/utils.NewImageV2ID UUID generation logic
  • Add flattenImageDataEnabled flag to BaseSpecification (read from
    ROX_FLATTEN_IMAGE_DATA env var) and a pre-computed TEST_IMAGE_V2_ID
  • Update AdmissionControllerTest, ImageManagementTest, and CSVTest to
    use the V2 image ID when the flag is enabled
  • Update VulnMgmtTest to pass image name alongside digest so it can
    compute the correct image ID for GraphQL queries
  • Update SACTest to expect an additional search result (IMAGES_V2 category)
  • Update GlobalSearch to include the IMAGES_V2 search category
  • Add retry logic in UpgradesTest for getImages and GraphQL queries,
    since the reprocessor may take time to migrate images to images_v2
    after an upgrade

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

Ran tests with and without the FlattenImageData enabled

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 13, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.61%. Comparing base (ac171d9) to head (b5cae73).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19972      +/-   ##
==========================================
+ Coverage   49.56%   49.61%   +0.04%     
==========================================
  Files        2764     2765       +1     
  Lines      208442   208596     +154     
==========================================
+ Hits       103321   103499     +178     
+ Misses      97466    97440      -26     
- Partials     7655     7657       +2     
Flag Coverage Δ
go-unit-tests 49.61% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 13, 2026

🚀 Build Images Ready

Images are ready for commit b5cae73. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-638-gb5cae7379d

@charmik-redhat charmik-redhat marked this pull request as ready for review April 14, 2026 05:40
@charmik-redhat charmik-redhat requested a review from janisz as a code owner April 14, 2026 05:40
@charmik-redhat charmik-redhat requested a review from a team April 14, 2026 05:40
Comment on lines +202 to +215
byte[] sha256 = MessageDigest.getInstance("SHA-256").digest(ns.getBytes("UTF-8"))
long msb = ByteBuffer.wrap(sha256, 0, 8).getLong()
long lsb = ByteBuffer.wrap(sha256, 8, 8).getLong()
UUID nsUUID = new UUID(msb, lsb)

MessageDigest sha1 = MessageDigest.getInstance("SHA-1")
sha1.update(uuidToBytes(nsUUID))
sha1.update(name.getBytes("UTF-8"))
byte[] hash = sha1.digest()

hash[6] = (byte) ((hash[6] & 0x0F) | 0x50) // version 5
hash[8] = (byte) ((hash[8] & 0x3F) | 0x80) // variant RFC 4122
long msbResult = ByteBuffer.wrap(hash, 0, 8).getLong()
long lsbResult = ByteBuffer.wrap(hash, 8, 8).getLong()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we can use a lib to handle that e.g. https://github.com/f4b6a3/uuid-creator

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, updated to use that library.

Comment on lines +119 to +121
def retryResultRet = gqlService.Call(getQuery(resourceType), [ query: searchQuery ])
assert retryResultRet.getCode() == 200
assert retryResultRet.getValue() != null
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will ignore the previous result. Maybe we don't need to barnach on flattenImageDataEnabled and always retry to have cleaner test. On non faltten images it will not retry at all unless a failure

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I will update it.

if (flattenImageDataEnabled) {
// When FlattenImageData is enabled, the reprocessor moves images from the
// images table to images_v2 after upgrade, which may take some time.
evaluateWithRetry(30, 10) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need evaluateWithRetry or just withRetry as we are not interested in the return value

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we not simply add the retry logic without the feature flag branch?

Copy link
Copy Markdown
Contributor Author

@charmik-redhat charmik-redhat Apr 14, 2026

Choose a reason for hiding this comment

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

Right, withRetry is enough. Also, updated it to always do the retry regardless of feature flag

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 14, 2026

@charmik-redhat: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ocp-4-21-nongroovy-e2e-tests b5cae73 link false /test ocp-4-21-nongroovy-e2e-tests

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants