Skip to content

ROX-27014: Cert refresh testing cleanup - extract verifySecrets#13650

Merged
vladbologa merged 3 commits intomasterfrom
vb/certrefresh-extract-verify-secrets
Jan 20, 2025
Merged

ROX-27014: Cert refresh testing cleanup - extract verifySecrets#13650
vladbologa merged 3 commits intomasterfrom
vb/certrefresh-extract-verify-secrets

Conversation

@vladbologa
Copy link
Contributor

@vladbologa vladbologa commented Dec 20, 2024 •
edited
Loading

Description

This PR extract similar code from the local scanner and the secured cluster certificate refresh tests, into a common verifySecrets function.

This is also a prefactor for #13596, where this function will be used in more tests.

User-facing documentation

  • CHANGELOG is updated OR update is not needed
  • documentation PR is created and is linked above OR is not needed

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

Testing change only, CI is sufficient.

@vladbologa vladbologa requested a review from a team as a code owner December 20, 2024 13:21
@rhacs-bot
Copy link
Contributor

rhacs-bot commented Dec 20, 2024 •
edited
Loading

Images are ready for the commit at 531569f.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.7.x-430-g531569f488.

Base automatically changed from vb/cert-refresh-channels to master January 7, 2025 11:08
@Maddosaurus
Copy link
Contributor

Maddosaurus commented Jan 9, 2025

Hey Vlad!
I noticed that this PR has been open for some time. It has now merge conflicts - could you resolve these before we start the review?
Thanks!

@vladbologa vladbologa marked this pull request as draft January 9, 2025 13:52
@vladbologa
Copy link
Contributor Author

vladbologa commented Jan 9, 2025

Hey Vlad! I noticed that this PR has been open for some time. It has now merge conflicts - could you resolve these before we start the review? Thanks!

I'll make it draft for now as it's outdated in its current form.

@vladbologa vladbologa force-pushed the vb/certrefresh-extract-verify-secrets branch from 961239c to 8868472 Compare January 9, 2025 15:39
@vikin91
Copy link
Contributor

vikin91 commented Jan 9, 2025

@vladbologa do you plan to work on it and make it reviewable soon?

I am asking, as we have several large PRs to review and we must find the right priorities for them.

@vladbologa
Copy link
Contributor Author

vladbologa commented Jan 9, 2025

@vladbologa do you plan to work on it and make it reviewable soon?

I am asking, as we have several large PRs to review and we must find the right priorities for them.

Actually it's ready now. It's a small change, it only extracts some common testing code.

I have to put the follow-up #13596 on hold until I finish another task, though.

@vladbologa vladbologa marked this pull request as ready for review January 9, 2025 15:49
@stackrox stackrox deleted a comment from openshift-ci bot Jan 9, 2025
@vladbologa vladbologa force-pushed the vb/certrefresh-extract-verify-secrets branch from 8868472 to a0b435f Compare January 15, 2025 08:56
@vladbologa
Copy link
Contributor Author

vladbologa commented Jan 15, 2025

/retest

vikin91
vikin91 approved these changes Jan 16, 2025
View reviewed changes
Copy link
Contributor

@vikin91 vikin91 left a comment

Choose a reason for hiding this comment

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

Looks okay as a prefactor. I am curious how it would play together with the follow-up PR.

Please check my comment about missing entry for scanner-v4-matcher before merging.

@vladbologa
Copy link
Contributor Author

vladbologa commented Jan 20, 2025

/test ocp-4-12-nongroovy-e2e-tests

@vladbologa vladbologa merged commit d9bb76a into master Jan 20, 2025
76 checks passed
@vladbologa vladbologa deleted the vb/certrefresh-extract-verify-secrets branch January 20, 2025 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vikin91 vikin91 vikin91 approved these changes

Assignees

No one assigned

Labels

area/sensor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vladbologa @rhacs-bot @Maddosaurus @vikin91