Skip to content

Conversation

@mvacula02
Copy link
Collaborator

ARO-23093

What

Add helper wrappers for HCP SDK credential operation functions to the test framework:

  • BeginRequestAdminCredential()

  • BeginRevokeCredentials()

Implement both wait and no-wait variants of both functions for future use.

Refactor admin credential lifecycle test, which uses credential operation functions.

Why

Explained in the JIRA ticket.

Add helper function wrappers for HCP SDK credential operations. Implement both wait and no-wait variants of functions. Refactor admin credential lifecycle test case.
https://issues.redhat.com/browse/ARO-23093
@openshift-ci
Copy link

openshift-ci bot commented Dec 17, 2025

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

@mgahagan73
Copy link
Collaborator

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Dec 18, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mgahagan73, mvacula02
Once this PR has been reviewed and has the lgtm label, please assign bennerv for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@mgahagan73
Copy link
Collaborator

/test e2e-parallel

resourceGroupName string,
hcpClusterName string,
) (*runtime.Poller[hcpsdk20240610preview.HcpOpenShiftClustersClientRequestAdminCredentialResponse], error) {
return hcpClient.BeginRequestAdminCredential(ctx, resourceGroupName, hcpClusterName, nil)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this appears to simply wrap a one-line call in a different one-line call. This is a bad pattern.

resourceGroupName string,
hcpClusterName string,
) (*runtime.Poller[hcpsdk20240610preview.HcpOpenShiftClustersClientRevokeCredentialsResponse], error) {
return hcpClient.BeginRevokeCredentials(ctx, resourceGroupName, hcpClusterName, nil)
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment about questionable alias value and indirection

Comment on lines 44 to 46
ProvisioningStateSucceeded = hcpsdk20240610preview.ProvisioningStateSucceeded
ProvisioningStateFailed = hcpsdk20240610preview.ProvisioningStateFailed
ProvisioningStateCanceled = hcpsdk20240610preview.ProvisioningStateCanceled
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't vanity alias a constant.

@openshift-ci openshift-ci bot removed the lgtm label Dec 19, 2025
@openshift-ci
Copy link

openshift-ci bot commented Dec 19, 2025

New changes are detected. LGTM label has been removed.

@mvacula02 mvacula02 requested a review from deads2k December 19, 2025 16:28
@mgahagan73
Copy link
Collaborator

/test e2e-parallel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants