Skip to content

Migration of credential operation functions into framework #3635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mvacula02 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Migration of credential operation functions into framework #3635

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50c569a
add helpers for cred ops
mvacula02 Dec 17, 2025
02a237d
implement suggestions
mvacula02 Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions test/e2e/admin_credential_lifecycle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,14 @@ import (

"github.com/openshift-eng/openshift-tests-extension/pkg/util/sets"

hcpsdk20240610preview "github.com/Azure/ARO-HCP/test/sdk/resourcemanager/redhatopenshifthcp/armredhatopenshifthcp"
"github.com/Azure/ARO-HCP/test/util/framework"
"github.com/Azure/ARO-HCP/test/util/labels"
"github.com/Azure/ARO-HCP/test/util/verifiers"
)

var _ = Describe("Customer", func() {

terminalProvisioningStates := sets.New(hcpsdk20240610preview.ProvisioningStateSucceeded, hcpsdk20240610preview.ProvisioningStateFailed, hcpsdk20240610preview.ProvisioningStateCanceled)
terminalProvisioningStates := sets.New(framework.ProvisioningStateSucceeded, framework.ProvisioningStateFailed, framework.ProvisioningStateCanceled)

It("should be able to test admin credentials before cluster ready, then full admin credential lifecycle",
labels.RequireNothing,
Expand Down Expand Up @@ -102,7 +101,7 @@ var _ = Describe("Customer", func() {
By("waiting for cluster to appear and testing admin credentials while in deploying state")
// Poll the cluster state and test admin credentials when we find it deploying
var testedWhileDeploying bool
var previousState hcpsdk20240610preview.ProvisioningState
var previousState framework.ProvisioningState
GinkgoLogr.Info("creating cluster, waiting for it to reach a terminal state")
Eventually(func() bool {
cluster, err := framework.GetHCPCluster(ctx, clusterClient, *resourceGroup.Name, clusterName)
Expand All @@ -125,11 +124,11 @@ var _ = Describe("Customer", func() {
if !testedWhileDeploying && !terminalProvisioningStates.Has(*cluster.Properties.ProvisioningState) {
By("testing admin credentials while cluster is in deploying state")
testedWhileDeploying = true
_, err := clusterClient.BeginRequestAdminCredential(
_, err := framework.RequestAdminCredential(
ctx,
clusterClient,
*resourceGroup.Name,
clusterName,
nil,
)
var respErr *azcore.ResponseError
if err != nil && errors.As(err, &respErr) && http.StatusConflict == respErr.StatusCode {
Expand All @@ -141,15 +140,15 @@ var _ = Describe("Customer", func() {
}

// If cluster is ready, we're done
if *cluster.Properties.ProvisioningState == hcpsdk20240610preview.ProvisioningStateSucceeded {
if *cluster.Properties.ProvisioningState == framework.ProvisioningStateSucceeded {
if !testedWhileDeploying {
Fail("Cluster provisioned too quickly to test 409 behavior - unable to validate admin credentials fail during deployment")
}
return true // Success - cluster is ready
}

// If cluster failed, that's an error
if *cluster.Properties.ProvisioningState == hcpsdk20240610preview.ProvisioningStateFailed {
if *cluster.Properties.ProvisioningState == framework.ProvisioningStateFailed {
Fail("Cluster provisioning failed")
}

Expand Down Expand Up @@ -190,11 +189,7 @@ var _ = Describe("Customer", func() {
}

By("revoking all cluster admin credentials via ARO HCP RP API")
poller, err := clusterClient.BeginRevokeCredentials(ctx, *resourceGroup.Name, clusterName, nil)
Expect(err).NotTo(HaveOccurred())

By("waiting for revocation operation to complete")
_, err = poller.PollUntilDone(ctx, nil)
err = framework.RevokeCredentialsAndWait(ctx, clusterClient, *resourceGroup.Name, clusterName, 10*time.Minute)
Expect(err).NotTo(HaveOccurred())

By("validating all admin credentials now fail after revocation")
Expand Down
121 changes: 95 additions & 26 deletions test/util/framework/hcp_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ import (
hcpsdk20240610preview "github.com/Azure/ARO-HCP/test/sdk/resourcemanager/redhatopenshifthcp/armredhatopenshifthcp"
)

const (
ProvisioningStateSucceeded = hcpsdk20240610preview.ProvisioningStateSucceeded
ProvisioningStateFailed = hcpsdk20240610preview.ProvisioningStateFailed
ProvisioningStateCanceled = hcpsdk20240610preview.ProvisioningStateCanceled
Copy link
Collaborator

@deads2k deads2k Dec 18, 2025

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.

)

type ProvisioningState = hcpsdk20240610preview.ProvisioningState

// checkOperationResult ensures the result model returned by a runtime.Poller
// matches the resource model returned from a GET request.
func checkOperationResult(expectModel, resultModel any) error {
Expand Down Expand Up @@ -70,41 +78,18 @@ func (tc *perItOrDescribeTestContext) GetAdminRESTConfigForHCPCluster(
hcpClusterName string,
timeout time.Duration, // this is a POST request, so keep the timeout as it's async
) (*rest.Config, error) {
ctx, cancel := context.WithTimeoutCause(ctx, timeout, fmt.Errorf("timeout '%f' minutes exceeded during GetAdminRESTConfigForHCPCluster for cluster %s in resource group %s", timeout.Minutes(), hcpClusterName, resourceGroupName))
defer cancel()

startTime := time.Now()
defer func() {
finishTime := time.Now()
tc.RecordTestStep("Collect admin credentials for cluster", startTime, finishTime)
}()

adminCredentialRequestPoller, err := hcpClient.BeginRequestAdminCredential(
ctx,
resourceGroupName,
hcpClusterName,
nil,
)
credentialResponse, err := RequestAdminCredentialAndWait(ctx, hcpClient, resourceGroupName, hcpClusterName, timeout)
if err != nil {
return nil, fmt.Errorf("failed to start credential request: %w", err)
}

operationResult, err := adminCredentialRequestPoller.PollUntilDone(ctx, &runtime.PollUntilDoneOptions{
Frequency: StandardPollInterval,
})
if err != nil {
if errors.Is(err, context.DeadlineExceeded) {
return nil, fmt.Errorf("failed waiting for hcpCluster=%q in resourcegroup=%q to finish getting creds, caused by: %w, error: %w", hcpClusterName, resourceGroupName, context.Cause(ctx), err)
}
return nil, fmt.Errorf("failed waiting for hcpCluster=%q in resourcegroup=%q to finish getting creds: %w", hcpClusterName, resourceGroupName, err)
return nil, err
}

switch m := any(operationResult).(type) {
case hcpsdk20240610preview.HcpOpenShiftClustersClientRequestAdminCredentialResponse:
return readStaticRESTConfig(m.Kubeconfig)
default:
return nil, fmt.Errorf("unknown type %T", m)
}
return readStaticRESTConfig(credentialResponse.Kubeconfig)
}

func readStaticRESTConfig(kubeconfigContent *string) (*rest.Config, error) {
Expand All @@ -128,6 +113,90 @@ func readStaticRESTConfig(kubeconfigContent *string) (*rest.Config, error) {
return ret, nil
}

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

@deads2k deads2k Dec 18, 2025

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.

}

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

@deads2k deads2k Dec 18, 2025

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

}

func RequestAdminCredentialAndWait(
ctx context.Context,
hcpClient *hcpsdk20240610preview.HcpOpenShiftClustersClient,
resourceGroupName string,
hcpClusterName string,
timeout time.Duration,
) (*hcpsdk20240610preview.HcpOpenShiftClustersClientRequestAdminCredentialResponse, error) {
ctx, cancel := context.WithTimeoutCause(ctx, timeout, fmt.Errorf("timeout '%f' minutes exceeded during RequestAdminCredentialAndWait for cluster %s in resource group %s", timeout.Minutes(), hcpClusterName, resourceGroupName))
defer cancel()

poller, err := hcpClient.BeginRequestAdminCredential(ctx, resourceGroupName, hcpClusterName, nil)
if err != nil {
return nil, fmt.Errorf("failed to start credential request for hcpCluster=%q in resourcegroup=%q: %w", hcpClusterName, resourceGroupName, err)
}

operationResult, err := poller.PollUntilDone(ctx, &runtime.PollUntilDoneOptions{
Frequency: StandardPollInterval,
})
if err != nil {
if errors.Is(err, context.DeadlineExceeded) {
return nil, fmt.Errorf("failed waiting for hcpCluster=%q in resourcegroup=%q to finish getting creds, caused by: %w, error: %w", hcpClusterName, resourceGroupName, context.Cause(ctx), err)
}
return nil, fmt.Errorf("failed waiting for hcpCluster=%q in resourcegroup=%q to finish getting creds: %w", hcpClusterName, resourceGroupName, err)
}

switch m := any(operationResult).(type) {
case hcpsdk20240610preview.HcpOpenShiftClustersClientRequestAdminCredentialResponse:
return &m, nil
default:
return nil, fmt.Errorf("unknown type %T", m)
}
}

func RevokeCredentialsAndWait(
ctx context.Context,
hcpClient *hcpsdk20240610preview.HcpOpenShiftClustersClient,
resourceGroupName string,
hcpClusterName string,
timeout time.Duration,
) error {
ctx, cancel := context.WithTimeoutCause(ctx, timeout, fmt.Errorf("timeout '%f' minutes exceeded during RevokeCredentialsAndWait for cluster %s in resource group %s", timeout.Minutes(), hcpClusterName, resourceGroupName))
defer cancel()

poller, err := hcpClient.BeginRevokeCredentials(ctx, resourceGroupName, hcpClusterName, nil)
if err != nil {
return fmt.Errorf("failed to start credential revocation for hcpCluster=%q in resourcegroup=%q: %w", hcpClusterName, resourceGroupName, err)
}

operationResult, err := poller.PollUntilDone(ctx, &runtime.PollUntilDoneOptions{
Frequency: StandardPollInterval,
})
if err != nil {
if errors.Is(err, context.DeadlineExceeded) {
return fmt.Errorf("failed waiting for hcpCluster=%q in resourcegroup=%q to finish revoking creds, caused by: %w, error: %w", hcpClusterName, resourceGroupName, context.Cause(ctx), err)
}
return fmt.Errorf("failed waiting for hcpCluster=%q in resourcegroup=%q to finish revoking creds: %w", hcpClusterName, resourceGroupName, err)
}

switch m := any(operationResult).(type) {
case hcpsdk20240610preview.HcpOpenShiftClustersClientRevokeCredentialsResponse:
return nil
default:
return fmt.Errorf("unknown type %T", m)
}
}

// DeleteHCPCluster deletes an hcp cluster and waits for the operation to complete
func DeleteHCPCluster(
ctx context.Context,
Expand Down