Skip to content

test: refactor pull secret helper functions and update go-doc - #6188

Merged
openshift-merge-bot[bot] merged 3 commits into
Azure:mainfrom
mvacula02:mvacula/aro-27529
Jul 31, 2026
Merged

test: refactor pull secret helper functions and update go-doc#6188
openshift-merge-bot[bot] merged 3 commits into
Azure:mainfrom
mvacula02:mvacula/aro-27529

Conversation

@mvacula02

@mvacula02 mvacula02 commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

ARO-27529

What

  • Extract viable functionality from the existing pull secret E2E test case as helper functions.
  • Restructure pull secret helpers to a new separate file (previously in hcp_helper.go)
  • Add Go doc comments explaining behaviour and references to relevant upstream documentation (hypershift)
  • Update existing pull secret E2E test case to use the new functions
  • add unit tests for suitable helper functions

Why

Refactor the existing helper functions related to customer pull secret handling in the E2E test code base to ensure they are modular, reusable, and well-documented. Future test case ARO-27528 will build on this.

Updating documentation as the pull-secret management feature is complex and future readability will greatly benefit from the additional context provided for both the helpers and the verifier.

Testing

This PR modifies existing test framework helper functions and a single E2E test case. No behavioral changes.

Adding unit tests for CreateTestDockerConfigSecret, AddRegistryAuthToSecret (3 scenarios).

PR Checklist

  • PR is scoped to a single task (no mixed concerns)
  • Title follows Conventional Commits format
  • Summary explains the "Why" behind the change
  • Linked to relevant ticket/issue
  • Screenshots included (if graph/UI/metrics changes)
  • Self-reviewed the diff
  • CI/CD checks are passing (ignore Tide)
  • Draft PR used for WIP (if applicable)
  • Commit history is clean (rebased/squashed)
  • Tricky code blocks are commented
  • Specific reviewers tagged
  • All comment threads resolved before merge

If E2E tests are included:

  • E2E tests follow Principles of Good E2E Test Case Design
  • If new E2E use case is covered (via a new test or new check/verifier),
    demonstrate that the test is able to detect a defect/error and fail with
    proper error message and logs which communicates nature of the problem.

@openshift-ci

openshift-ci Bot commented Jul 21, 2026

Copy link
Copy Markdown

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

@mvacula02

mvacula02 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

@mvacula02
mvacula02 marked this pull request as ready for review July 22, 2026 08:17
Copilot AI review requested due to automatic review settings July 22, 2026 08:17
Copilot AI reviewed Jul 22, 2026

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@mvacula02

mvacula02 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

/assign Martin Bukatovic (@mbukatov) Mike Gahagan (@mgahagan73)

mbukatov
Martin Bukatovic (mbukatov) reviewed Jul 24, 2026
View reviewed changes

@mbukatov Martin Bukatovic (mbukatov) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks nice, thanks for the refactoring!

Comment thread test/util/framework/pullsecret_helper.go
Copilot AI review requested due to automatic review settings July 27, 2026 07:33
Copilot AI reviewed Jul 27, 2026

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@mbukatov

Martin Bukatovic (mbukatov) commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

/lgtm

thank you for the fix

@openshift-ci openshift-ci Bot added the lgtm label Jul 27, 2026
@mvacula02

mvacula02 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

@mvacula02

mvacula02 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

/assign Patrik Suba (@patriksuba)

@mvacula02

mvacula02 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

@mgahagan73

Mike Gahagan (mgahagan73) commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

/approve

@openshift-merge-bot

openshift-merge-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD d3b62bb and 2 for PR HEAD 0f8316c in total

@mvacula02

mvacula02 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

3 similar comments
@mvacula02

mvacula02 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

@mvacula02

mvacula02 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

@mvacula02

mvacula02 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

@openshift-merge-bot

openshift-merge-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 9ce5cbc and 1 for PR HEAD 0f8316c in total

@openshift-merge-bot

openshift-merge-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD bf99b44 and 0 for PR HEAD 0f8316c in total

@mvacula02

mvacula02 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

@openshift-merge-bot

openshift-merge-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

/hold

Revision 0f8316c was retested 3 times: holding

@sclarkso

Steven Clarkson (sclarkso) commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

/test e2e-parallel

@mvacula02

mvacula02 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

/hold cancel

@mvacula02

mvacula02 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

@openshift-merge-bot

openshift-merge-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 52090fd and 2 for PR HEAD 0f8316c in total

@openshift-merge-bot

openshift-merge-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD b3e8f6b and 1 for PR HEAD 0f8316c in total

@mvacula02

mvacula02 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

mvacula02 added 3 commits July 31, 2026 13:20
@mvacula02
Refactor pull secret helper functions and update go-doc
b0d2d3b 
refactor existing pull-secret related helpers for reusability. Update documentation with details and upstream references. Update existing E2E test to use the new helper functions.
https://redhat.atlassian.net/browse/ARO-27529
@mvacula02
add unit tests
0444566
@mvacula02
handle nil Auth, add unit test
4c5b0b7
@mvacula02
mvacula02 force-pushed the mvacula/aro-27529 branch from 0f8316c to 4c5b0b7 Compare July 31, 2026 11:24
Copilot AI review requested due to automatic review settings July 31, 2026 11:24
@openshift-ci openshift-ci Bot removed the lgtm label Jul 31, 2026
Copilot AI reviewed Jul 31, 2026

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@mvacula02

mvacula02 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

/test images

mbukatov
Martin Bukatovic (mbukatov) reviewed Jul 31, 2026
View reviewed changes

@mbukatov Martin Bukatovic (mbukatov) left a comment

Copy link
Copy Markdown
Collaborator

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 label Jul 31, 2026
@openshift-ci

openshift-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mbukatov, mgahagan73, mvacula02

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

@mvacula02

mvacula02 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

/test images

1 similar comment
@mvacula02

mvacula02 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

/test images

@mvacula02

mvacula02 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

1 similar comment
@mvacula02

mvacula02 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

/test e2e-parallel

@openshift-merge-bot
openshift-merge-bot Bot merged commit 3ce3dc8 into Azure:main Jul 31, 2026
16 checks passed
@mvacula02
mvacula02 deleted the mvacula/aro-27529 branch August 1, 2026 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@bennerv Ben Vesel (bennerv) Awaiting requested review from bennerv
@deads2k David Eads (deads2k) Awaiting requested review from deads2k
+1 more reviewer
@mbukatov Martin Bukatovic (mbukatov) mbukatov left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@mbukatov Martin Bukatovic (mbukatov)

@mgahagan73 Mike Gahagan (mgahagan73)

@patriksuba Patrik Suba (patriksuba)

Labels

approved lgtm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mvacula02 @mbukatov @mgahagan73 @sclarkso @patriksuba