Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
203 changes: 203 additions & 0 deletions test/e2e/issuer_acme_dns01_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package e2e

import (
"context"
"encoding/json"
"fmt"
"log"
"os"
Expand All @@ -13,10 +14,16 @@ import (
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/msi/armmsi"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/iam"
"github.com/aws/aws-sdk-go-v2/service/sts"
"github.com/google/uuid"
gcpcrm "google.golang.org/api/cloudresourcemanager/v1"
gcpiam "google.golang.org/api/iam/v1"

Expand Down Expand Up @@ -1386,6 +1393,202 @@ var _ = Describe("ACME Issuer DNS01 solver", Ordered, func() {
})
})

Context("with Azure DNS in Workload Identity environment", Label("Platform:Azure", "CredentialsMode:Manual"), Label("TechPreview"), func() {
var identityClientID string
var subscriptionID, dnsResourceGroupName, hostedZoneName string

BeforeAll(func() {
By("verifying cluster is STS-enabled")
isSTS, err := isSTSCluster(ctx, oseOperatorClient, configClient)
Expect(err).NotTo(HaveOccurred())
if !isSTS {
Skip("Test requires Azure Workload Identity enabled")
}

By("setting up Azure authentication environment variable from credentials file")
if os.Getenv("OPENSHIFT_CI") == "true" {
clusterProfileDir := os.Getenv("CLUSTER_PROFILE_DIR")
Expect(clusterProfileDir).NotTo(BeEmpty(), "CLUSTER_PROFILE_DIR should exist when running in OpenShift CI")
os.Setenv("AZURE_AUTH_LOCATION", filepath.Join(clusterProfileDir, "osServicePrincipal.json"))
} else {
Expect(os.Getenv("AZURE_AUTH_LOCATION")).NotTo(BeEmpty(), "AZURE_AUTH_LOCATION must be set when running locally")
}
azureAuthLocation := os.Getenv("AZURE_AUTH_LOCATION")
data, err := os.ReadFile(azureAuthLocation)
Expect(err).NotTo(HaveOccurred(), "failed to read Azure credentials file")
var sp struct {
ClientID string `json:"clientId"`
ClientSecret string `json:"clientSecret"`
TenantID string `json:"tenantId"`
}
Expect(json.Unmarshal(data, &sp)).To(Succeed(), "failed to parse Azure credentials file")
os.Setenv("AZURE_CLIENT_ID", sp.ClientID)
os.Setenv("AZURE_CLIENT_SECRET", sp.ClientSecret)
os.Setenv("AZURE_TENANT_ID", sp.TenantID)

By("creating Azure SDK credential")
cred, err := azidentity.NewDefaultAzureCredential(nil)
Expect(err).NotTo(HaveOccurred(), "failed to create Azure SDK credential")

// Get DNS zone info (subscriptionID comes from the cluster DNS config)
subscriptionID, dnsResourceGroupName, hostedZoneName = getAzureDNSZoneInfo(ctx)

By("getting cluster resource group from Infrastructure object")
infra, err := configClient.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred(), "failed to get Infrastructure object")
clusterResourceGroup := infra.Status.PlatformStatus.Azure.ResourceGroupName
Expect(clusterResourceGroup).NotTo(BeEmpty(), "Azure resource group should not be empty")

By("getting cluster resource group location")
rgClient, err := armresources.NewResourceGroupsClient(subscriptionID, cred, nil)
Expect(err).NotTo(HaveOccurred(), "failed to create resource groups client")
rg, err := rgClient.Get(ctx, clusterResourceGroup, nil)
Expect(err).NotTo(HaveOccurred(), "failed to get resource group")
location := *rg.Location

Comment on lines +1445 to +1448
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard nil Location before dereference.

Line 1447 can panic if Azure returns a resource group payload without Location.

🛡️ Suggested fix
 			rg, err := rgClient.Get(ctx, clusterResourceGroup, nil)
 			Expect(err).NotTo(HaveOccurred(), "failed to get resource group")
+			Expect(rg.Location).NotTo(BeNil(), "resource group location should be populated")
 			location := *rg.Location
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
rg, err := rgClient.Get(ctx, clusterResourceGroup, nil)
Expect(err).NotTo(HaveOccurred(), "failed to get resource group")
location := *rg.Location
rg, err := rgClient.Get(ctx, clusterResourceGroup, nil)
Expect(err).NotTo(HaveOccurred(), "failed to get resource group")
Expect(rg.Location).NotTo(BeNil(), "resource group location should be populated")
location := *rg.Location
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/issuer_acme_dns01_test.go` around lines 1445 - 1448, The test
dereferences rg.Location without guarding for nil which can panic if Azure
returns no Location; after calling rgClient.Get (and the Expect on err), add a
nil-check for rg.Location and fail the test or use
Expect(rg.Location).NotTo(BeNil(), "resource group Location is missing") before
using *rg.Location, then assign location := *rg.Location; ensure you reference
rg and rgClient.Get in the fix so the check sits immediately after the Get/err
assertion.

By("getting OIDC issuer from Authentication object")
authConfig, err := configClient.Authentications().Get(ctx, "cluster", metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred(), "failed to get Authentication object")
oidcIssuer := authConfig.Spec.ServiceAccountIssuer
Expect(oidcIssuer).NotTo(BeEmpty(), "OIDC issuer not found in Authentication object")

By("creating Azure Managed Identity")
randomSuffix := randomStr(4)
identityName := "e2e-cert-manager-dns01-" + randomSuffix
msiClient, err := armmsi.NewUserAssignedIdentitiesClient(subscriptionID, cred, nil)
Expect(err).NotTo(HaveOccurred(), "failed to create MSI client")

identity, err := msiClient.CreateOrUpdate(ctx, clusterResourceGroup, identityName, armmsi.Identity{
Location: &location,
}, nil)
Expect(err).NotTo(HaveOccurred(), "failed to create Managed Identity")
identityClientID = *identity.Properties.ClientID
identityPrincipalID := *identity.Properties.PrincipalID

DeferCleanup(func(ctx context.Context) {
By("Cleaning up Azure Managed Identity")
_, err := msiClient.Delete(ctx, clusterResourceGroup, identityName, nil)
if err != nil {
fmt.Fprintf(GinkgoWriter, "failed to delete Managed Identity during cleanup: %v\n", err)
}
})

By("granting DNS Zone Contributor role to Managed Identity on the DNS zone")
dnsZoneScope := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/dnszones/%s",
subscriptionID, dnsResourceGroupName, hostedZoneName)

roleDefClient, err := armauthorization.NewRoleDefinitionsClient(cred, nil)
Expect(err).NotTo(HaveOccurred(), "failed to create role definitions client")
roleName := "DNS Zone Contributor"
filter := fmt.Sprintf("roleName eq '%s'", roleName)
pager := roleDefClient.NewListPager(dnsZoneScope, &armauthorization.RoleDefinitionsClientListOptions{Filter: &filter})
Expect(pager.More()).To(BeTrue(), "no role definitions found for %q", roleName)
page, err := pager.NextPage(ctx)
Expect(err).NotTo(HaveOccurred(), "failed to list role definitions")
Expect(page.Value).NotTo(BeEmpty(), "role definition %q not found", roleName)
dnsZoneContributorRoleID := *page.Value[0].ID

authClient, err := armauthorization.NewRoleAssignmentsClient(subscriptionID, cred, nil)
Expect(err).NotTo(HaveOccurred(), "failed to create authorization client")
roleAssignmentName := uuid.New().String()

err = wait.PollUntilContextTimeout(ctx, fastPollInterval, lowTimeout, true, func(context.Context) (bool, error) {
_, assignErr := authClient.Create(ctx, dnsZoneScope, roleAssignmentName, armauthorization.RoleAssignmentCreateParameters{
Properties: &armauthorization.RoleAssignmentProperties{
RoleDefinitionID: &dnsZoneContributorRoleID,
PrincipalID: &identityPrincipalID,
PrincipalType: to.Ptr(armauthorization.PrincipalTypeServicePrincipal),
},
}, nil)
if assignErr != nil {
fmt.Fprintf(GinkgoWriter, "role assignment attempt failed (retrying): %v\n", assignErr)
return false, nil
}
Comment on lines +1495 to +1506
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get exact code at the specified lines
sed -n '1490,1515p' test/e2e/issuer_acme_dns01_test.go | cat -n

Repository: openshift/cert-manager-operator

Length of output: 1493


🏁 Script executed:

#!/bin/bash
# Check for existing imports in the file
head -50 test/e2e/issuer_acme_dns01_test.go | cat -n

Repository: openshift/cert-manager-operator

Length of output: 2060


🏁 Script executed:

#!/bin/bash
# Search for error handling patterns in the file
rg -n 'ResponseError|errors\.As|StatusCode|ErrorCode' test/e2e/issuer_acme_dns01_test.go

Repository: openshift/cert-manager-operator

Length of output: 57


🏁 Script executed:

#!/bin/bash
# Check if azcore is already imported
rg -n 'import|azcore' test/e2e/issuer_acme_dns01_test.go | head -20

Repository: openshift/cert-manager-operator

Length of output: 139


🏁 Script executed:

#!/bin/bash
# Search for other Azure API calls that might handle conflicts
rg -n 'authClient|Create\(' test/e2e/issuer_acme_dns01_test.go | head -20

Repository: openshift/cert-manager-operator

Length of output: 2380


🌐 Web query:

Azure SDK for Go v2 RoleAssignment Create conflict error handling

💡 Result:

In Azure SDK for Go v2 (github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2), RoleAssignmentsClient.Create(...) returns an error that you can unwrap as *azcore.ResponseError when the service responds with a non-2xx code. (pkg.go.dev)

Treat “already exists” as success (409 Conflict / RoleAssignmentExists)

Azure RBAC role assignments are effectively unique by (principal + roleDefinition + scope); if you try to create the same combination again (often with a different GUID for roleAssignmentName), Azure can return 409 Conflict with code RoleAssignmentExists. (learn.microsoft.com)

import (
  "errors"
  "net/http"

  "github.com/Azure/azure-sdk-for-go/sdk/azcore"
)

resp, err := roleAssignmentsClient.Create(ctx, scope, roleAssignmentName, params, nil)
if err != nil {
  var re *azcore.ResponseError
  if errors.As(err, &re) {
    if re.StatusCode == http.StatusConflict && re.ErrorCode == "RoleAssignmentExists" {
      // idempotent success: the assignment is already present
      return nil
    }
  }
  return err
}
// use resp.RoleAssignment ...
_ = resp

Notes:

  • azcore.ResponseError is the standard error type for non-success HTTP status codes, and the SDK expects you to use errors.As() to access it. (pkg.go.dev)
  • The SDK extracts ErrorCode from the response (preferring x-ms-error-code, otherwise parsing the body). (pkg.go.dev)

If you need the existing assignment’s ID

If you must return the existing role assignment resource ID (not just ignore the conflict), you typically need to list role assignments at the scope and match on principalId + roleDefinitionId (since the conflicting create won’t tell you the existing GUID). This is the same approach commonly recommended via CLI for investigating these conflicts. (learn.microsoft.com)

Citations:


🌐 Web query:

Azure SDK for Go azcore ResponseError StatusCode RoleAssignmentExists

💡 Result:

In the Azure SDK for Go, ARM “role assignment already exists” surfaces as an *azcore.ResponseError with:

  • StatusCode == 409 (HTTP Conflict)
  • ErrorCode == "RoleAssignmentExists" (pkg.go.dev)

Handle it by type-asserting via errors.As and checking StatusCode and/or ErrorCode:

import (
  "errors"
  "net/http"

  "github.com/Azure/azure-sdk-for-go/sdk/azcore"
)

if err != nil {
  var re *azcore.ResponseError
  if errors.As(err, &re) {
    if re.StatusCode == http.StatusConflict && re.ErrorCode == "RoleAssignmentExists" {
      // role assignment is already present; treat as success / ignore / reconcile
      return nil
    }
  }
  return err
}

Why it happens: you attempted to create an RBAC role assignment that already exists (same scope + principal + role), so the service returns 409 with code RoleAssignmentExists. (github.com)

Citations:


🏁 Script executed:

#!/bin/bash
# Search for any other Azure error handling patterns in the test file
rg -n 'if.*err|Delete|authClient' test/e2e/issuer_acme_dns01_test.go | tail -30

Repository: openshift/cert-manager-operator

Length of output: 2349


🏁 Script executed:

#!/bin/bash
# Check if the repository has any other conflict handling patterns
rg -r 'ResponseError|StatusConflict|RoleAssignmentExists' test/ --type go

Repository: openshift/cert-manager-operator

Length of output: 50388


Handle role-assignment "already exists" as success in retry loop.

The retry loop currently retries on all errors. When a role assignment succeeds server-side but the client receives a transient error, subsequent retry attempts can return HTTP 409 Conflict with ErrorCode == "RoleAssignmentExists", causing the loop to timeout instead of succeeding.

🔧 Suggested fix
+// import additions needed:
+//   "errors"
+//   "net/http"
+//   "github.com/Azure/azure-sdk-for-go/sdk/azcore"

 			err = wait.PollUntilContextTimeout(ctx, fastPollInterval, lowTimeout, true, func(context.Context) (bool, error) {
 				_, assignErr := authClient.Create(ctx, dnsZoneScope, roleAssignmentName, armauthorization.RoleAssignmentCreateParameters{
 					Properties: &armauthorization.RoleAssignmentProperties{
 						RoleDefinitionID: &dnsZoneContributorRoleID,
 						PrincipalID:      &identityPrincipalID,
 						PrincipalType:    to.Ptr(armauthorization.PrincipalTypeServicePrincipal),
 					},
 				}, nil)
 				if assignErr != nil {
+					var respErr *azcore.ResponseError
+					if errors.As(assignErr, &respErr) &&
+						(respErr.StatusCode == http.StatusConflict && respErr.ErrorCode == "RoleAssignmentExists") {
+						return true, nil
+					}
 					fmt.Fprintf(GinkgoWriter, "role assignment attempt failed (retrying): %v\n", assignErr)
 					return false, nil
 				}
 				return true, nil
 			})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/issuer_acme_dns01_test.go` around lines 1495 - 1506, In the
wait.PollUntilContextTimeout retry loop around authClient.Create (creating
roleAssignmentName in dnsZoneScope), treat a failure where the server reports
the role assignment already exists as a success: after calling authClient.Create
and getting assignErr, detect the 409/Conflict with ErrorCode
"RoleAssignmentExists" (inspect the error type returned by the Azure SDK for the
status code/ErrorCode) and return true,nil so the loop exits successfully;
otherwise continue returning false,nil for transient errors as before.

return true, nil
})
Expect(err).NotTo(HaveOccurred(), "failed to create role assignment")

DeferCleanup(func(ctx context.Context) {
By("Cleaning up role assignment")
_, err := authClient.Delete(ctx, dnsZoneScope, roleAssignmentName, nil)
if err != nil {
fmt.Fprintf(GinkgoWriter, "failed to delete role assignment during cleanup: %v\n", err)
}
})

By("creating Federated Identity Credential for cert-manager ServiceAccount")
ficClient, err := armmsi.NewFederatedIdentityCredentialsClient(subscriptionID, cred, nil)
Expect(err).NotTo(HaveOccurred(), "failed to create Federated Identity Credentials client")

_, err = ficClient.CreateOrUpdate(ctx, clusterResourceGroup, identityName, "cert-manager", armmsi.FederatedIdentityCredential{
Properties: &armmsi.FederatedIdentityCredentialProperties{
Issuer: &oidcIssuer,
Subject: to.Ptr("system:serviceaccount:cert-manager:cert-manager"),
Audiences: []*string{to.Ptr("api://AzureADTokenExchange")},
},
}, nil)
Expect(err).NotTo(HaveOccurred(), "failed to create Federated Identity Credential")

DeferCleanup(func(ctx context.Context) {
By("Cleaning up Federated Identity Credential")
_, err := ficClient.Delete(ctx, clusterResourceGroup, identityName, "cert-manager", nil)
if err != nil {
fmt.Fprintf(GinkgoWriter, "failed to delete Federated Identity Credential during cleanup: %v\n", err)
}
})
})

It("should obtain a valid certificate using ambient credentials through AAD Workload Identity", func() {

By("adding 'azure.workload.identity/use' label to cert-manager controller pods")
err := addOverrideLabels(certmanageroperatorclient, certmanagerControllerDeployment, map[string]string{
"azure.workload.identity/use": "true",
})
Expect(err).NotTo(HaveOccurred(), "failed to add workload identity label to cert-manager controller")

DeferCleanup(func(ctx context.Context) {
By("Removing workload identity label from cert-manager controller pods")
if err := addOverrideLabels(certmanageroperatorclient, certmanagerControllerDeployment, nil); err != nil {
fmt.Fprintf(GinkgoWriter, "failed to remove workload identity label during cleanup: %v\n", err)
}
})

By("waiting for cert-manager deployment to rollout with workload identity label")
err = waitForDeploymentPodLabelAndRollout(ctx, "cert-manager", "cert-manager", "azure.workload.identity/use", "true", 2*time.Minute)
Expect(err).NotTo(HaveOccurred(), "timeout waiting for cert-manager deployment rollout with workload identity label")

By("creating ACME ClusterIssuer with AzureDNS DNS-01 solver using managed identity")
clusterIssuerName := "letsencrypt-dns01-azuredns-wi"
solver := acmev1.ACMEChallengeSolver{
DNS01: &acmev1.ACMEChallengeSolverDNS01{
AzureDNS: &acmev1.ACMEIssuerDNS01ProviderAzureDNS{
SubscriptionID: subscriptionID,
ResourceGroupName: dnsResourceGroupName,
HostedZoneName: hostedZoneName,
ManagedIdentity: &acmev1.AzureManagedIdentity{
ClientID: identityClientID,
},
},
},
}
clusterIssuer := createACMEClusterIssuer(clusterIssuerName, solver)
_, err = certmanagerClient.CertmanagerV1().ClusterIssuers().Create(ctx, clusterIssuer, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred(), "failed to create ClusterIssuer")
DeferCleanup(func(ctx context.Context) {
certmanagerClient.CertmanagerV1().ClusterIssuers().Delete(ctx, clusterIssuerName, metav1.DeleteOptions{})
})

By("waiting for ClusterIssuer to become ready")
err = waitForClusterIssuerReadiness(ctx, clusterIssuerName)
Expect(err).NotTo(HaveOccurred(), "timeout waiting for ClusterIssuer to become Ready")

// Create and verify certificate
certName := "letsencrypt-cert-azuredns-wi"
dnsName := fmt.Sprintf("adazwi-%s.%s", randomStr(3), appsDomain) // acronym for "ACME DNS01 AzureDNS Workload Identity"
createAndVerifyACMECertificate(ctx, certName, ns.Name, dnsName, clusterIssuerName, "ClusterIssuer")
})
})

Context("with IBM Cloud Internet Service Webhook", Label("Platform:IBM"), func() {

// This test uses IBM Cloud Internet Services (CIS) for the DNS-01 challenge.
Expand Down
56 changes: 56 additions & 0 deletions test/e2e/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,48 @@ func verifyDeploymentEnv(k8sclient *kubernetes.Clientset, deploymentName string,
})
}

// addOverrideLabels adds the override labels to specific the cert-manager operand. The update process
// is retried if a conflict error is encountered.
func addOverrideLabels(client *certmanoperatorclient.Clientset, deploymentName string, labels map[string]string) error {
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
operator, err := client.OperatorV1alpha1().CertManagers().Get(context.TODO(), "cluster", metav1.GetOptions{})
if err != nil {
return err
}

updatedOperator := operator.DeepCopy()

switch deploymentName {
case certmanagerControllerDeployment:
cfg := updatedOperator.Spec.ControllerConfig
if cfg == nil {
cfg = &v1alpha1.DeploymentConfig{}
}
cfg.OverrideLabels = labels
updatedOperator.Spec.ControllerConfig = cfg
case certmanagerWebhookDeployment:
cfg := updatedOperator.Spec.WebhookConfig
if cfg == nil {
cfg = &v1alpha1.DeploymentConfig{}
}
cfg.OverrideLabels = labels
updatedOperator.Spec.WebhookConfig = cfg
case certmanagerCAinjectorDeployment:
cfg := updatedOperator.Spec.CAInjectorConfig
if cfg == nil {
cfg = &v1alpha1.DeploymentConfig{}
}
cfg.OverrideLabels = labels
updatedOperator.Spec.CAInjectorConfig = cfg
default:
return fmt.Errorf("unsupported deployment name: %s", deploymentName)
}

_, err = client.OperatorV1alpha1().CertManagers().Update(context.TODO(), updatedOperator, metav1.UpdateOptions{})
return err
})
}

// addOverrideResources adds the override resources to the specific cert-manager operand. The update process
// is retried if a conflict error is encountered.
func addOverrideResources(client *certmanoperatorclient.Clientset, deploymentName string, res v1alpha1.CertManagerResourceRequirements) error {
Expand Down Expand Up @@ -793,6 +835,20 @@ func waitForDeploymentArgAndRollout(ctx context.Context, namespace, deploymentNa
}, timeout)
}

// waitForDeploymentPodLabelAndRollout waits for a deployment's pod template to have
// a specific label and for the rollout to complete. This is useful when waiting for
// operator-driven label changes to propagate to operand deployments,
// as it ensures the expected change has been applied before checking rollout status.
func waitForDeploymentPodLabelAndRollout(ctx context.Context, namespace, deploymentName, labelKey, labelValue string, timeout time.Duration) error {
return waitForDeploymentConditionAndRollout(ctx, namespace, deploymentName, func(deployment *appsv1.Deployment) bool {
labels := deployment.Spec.Template.GetLabels()
if labels == nil {
return false
}
return labels[labelKey] == labelValue
}, timeout)
Comment on lines +842 to +849
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Require label-key presence in pod label matcher.

At Line 848, missing keys currently compare as "", which can incorrectly pass when expected value is empty.

🔧 Suggested fix
 func waitForDeploymentPodLabelAndRollout(ctx context.Context, namespace, deploymentName, labelKey, labelValue string, timeout time.Duration) error {
 	return waitForDeploymentConditionAndRollout(ctx, namespace, deploymentName, func(deployment *appsv1.Deployment) bool {
 		labels := deployment.Spec.Template.GetLabels()
 		if labels == nil {
 			return false
 		}
-		return labels[labelKey] == labelValue
+		actual, ok := labels[labelKey]
+		return ok && actual == labelValue
 	}, timeout)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func waitForDeploymentPodLabelAndRollout(ctx context.Context, namespace, deploymentName, labelKey, labelValue string, timeout time.Duration) error {
return waitForDeploymentConditionAndRollout(ctx, namespace, deploymentName, func(deployment *appsv1.Deployment) bool {
labels := deployment.Spec.Template.GetLabels()
if labels == nil {
return false
}
return labels[labelKey] == labelValue
}, timeout)
func waitForDeploymentPodLabelAndRollout(ctx context.Context, namespace, deploymentName, labelKey, labelValue string, timeout time.Duration) error {
return waitForDeploymentConditionAndRollout(ctx, namespace, deploymentName, func(deployment *appsv1.Deployment) bool {
labels := deployment.Spec.Template.GetLabels()
if labels == nil {
return false
}
actual, ok := labels[labelKey]
return ok && actual == labelValue
}, timeout)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/utils_test.go` around lines 842 - 849, The pod-label matcher in
waitForDeploymentPodLabelAndRollout can falsely succeed when the label key is
missing because labels[labelKey] returns "" for absent keys; modify the
predicate passed to waitForDeploymentConditionAndRollout to verify the key
exists (use the map lookup form, e.g., val, ok := labels[labelKey]) and return
ok && val == labelValue so the matcher requires presence and value equality;
update the anonymous function inside waitForDeploymentPodLabelAndRollout
accordingly (referencing waitForDeploymentPodLabelAndRollout and
waitForDeploymentConditionAndRollout).

}

// waitForClusterIssuerReadiness waits for a ClusterIssuer to become Ready
func waitForClusterIssuerReadiness(ctx context.Context, clusterIssuerName string) error {
return wait.PollUntilContextTimeout(ctx, fastPollInterval, lowTimeout, true,
Expand Down
12 changes: 11 additions & 1 deletion test/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,19 @@ module github.com/openshift/cert-manager-operator/test
go 1.25.0

require (
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.19.1
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.12.0
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2 v2.2.0
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/msi/armmsi v1.3.0
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0
github.com/aws/aws-sdk-go-v2 v1.40.0
github.com/aws/aws-sdk-go-v2/config v1.31.12
github.com/aws/aws-sdk-go-v2/service/iam v1.52.1
github.com/aws/aws-sdk-go-v2/service/sts v1.38.6
github.com/cert-manager/cert-manager v1.19.2
github.com/ghodss/yaml v1.0.0
github.com/google/go-cmp v0.7.0
github.com/google/uuid v1.6.0
github.com/onsi/ginkgo/v2 v2.27.5
github.com/onsi/gomega v1.39.0
github.com/openshift/api v0.0.0-20260105191300-d1c4dc4fd37b
Expand All @@ -32,6 +38,8 @@ require (
cloud.google.com/go/auth v0.16.5 // indirect
cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect
cloud.google.com/go/compute/metadata v0.9.0 // indirect
github.com/Azure/azure-sdk-for-go/sdk/internal v1.11.2 // indirect
github.com/AzureAD/microsoft-authentication-library-for-go v1.5.0 // indirect
github.com/Masterminds/semver/v3 v3.4.0 // indirect
github.com/aws/aws-sdk-go-v2/credentials v1.18.16 // indirect
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.9 // indirect
Expand Down Expand Up @@ -72,17 +80,18 @@ require (
github.com/go-openapi/swag/yamlutils v0.24.0 // indirect
github.com/go-task/slim-sprig/v3 v3.0.0 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang-jwt/jwt/v5 v5.3.0 // indirect
github.com/google/btree v1.1.3 // indirect
github.com/google/gnostic-models v0.7.0 // indirect
github.com/google/pprof v0.0.0-20250903194437-c28834ac2320 // indirect
github.com/google/s2a-go v0.1.9 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect
github.com/googleapis/gax-go/v2 v2.15.0 // indirect
github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/krishicks/yaml-patch v0.0.10 // indirect
github.com/kylelemons/godebug v1.1.0 // indirect
github.com/mailru/easyjson v0.9.0 // indirect
github.com/moby/spdystream v0.5.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
Expand All @@ -92,6 +101,7 @@ require (
github.com/nxadm/tail v1.4.11 // indirect
github.com/onsi/ginkgo v1.16.5 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/prometheus/client_golang v1.23.2 // indirect
github.com/prometheus/client_model v0.6.2 // indirect
Expand Down
Loading