CNTRLPLANE-3564: test(awsprivatelink): add unit test for NoSuchHostedZone handling dur… by mgencur · Pull Request #8794 · openshift/hypershift

mgencur · 2026-06-22T13:07:13Z

…ing deletion

What this PR does / why we need it:

Cover the code path where FindRecord() returns a NoSuchHostedZone error because the Route53 hosted zone was already deleted externally. The controller correctly treats this as successful deletion and removes the finalizer.

Which issue(s) this PR fixes:

Fixes https://redhat.atlassian.net/browse/CNTRLPLANE-3564

Special notes for your reviewer:

Checklist:

Subject and description added to both, commit and PR.
Relevant issues have been referenced.
This change includes docs.
This change includes unit tests.

Summary by CodeRabbit

Tests
- Improved test coverage for deletion scenarios with external resource removal handling.

…ing deletion Cover the code path where FindRecord() returns a NoSuchHostedZone error because the Route53 hosted zone was already deleted externally. The controller correctly treats this as successful deletion and removes the finalizer. Signed-off-by: Martin Gencur <mgencur@redhat.com>

openshift-merge-bot · 2026-06-22T13:07:15Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

openshift-ci-robot · 2026-06-22T13:07:17Z

@mgencur: This pull request references CNTRLPLANE-3564 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

…ing deletion

What this PR does / why we need it:

Cover the code path where FindRecord() returns a NoSuchHostedZone error because the Route53 hosted zone was already deleted externally. The controller correctly treats this as successful deletion and removes the finalizer.

Which issue(s) this PR fixes:

Fixes https://redhat.atlassian.net/browse/CNTRLPLANE-3564

Special notes for your reviewer:

Checklist:

Subject and description added to both, commit and PR.

Relevant issues have been referenced.

This change includes docs.

This change includes unit tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

coderabbitai · 2026-06-22T13:07:30Z

📝 Walkthrough

Walkthrough

A new table-driven test case, "Route53 hosted zone already deleted externally," is added to TestReconcileDeletion in the AWS Private Link controller test file. The Route53 mock is configured to return a route53types.NoSuchHostedZone error from ListResourceRecordSets. The test asserts that the reconcile deletion path returns no error (expectError: false) and that the AWS finalizer is removed from the AWSEndpointService (expectFinalizer: false).

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: adding a unit test for NoSuchHostedZone error handling in the awsprivatelink controller during deletion.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	New test case in TestReconcileDeletion uses static, deterministic name: "When Route53 hosted zone is already deleted externally it should treat deletion as successful" with no dynamic values, gener...
Test Structure And Quality	✅ Passed	The new test case follows all quality requirements: it has single responsibility (tests NoSuchHostedZone error handling), follows codebase's table-driven test pattern with shared setup/cleanup logi...
Topology-Aware Scheduling Compatibility	✅ Passed	PR only modifies test code (awsprivatelink_controller_test.go), adding a test case for Route53 deletion. No deployment manifests, operator configuration, or scheduling constraints are introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	This PR adds a standard Go unit test (not Ginkgo e2e), so the IPv6/disconnected network check does not apply. The test uses mocked AWS APIs with no IPv4 assumptions or external connectivity.
No-Weak-Crypto	✅ Passed	PR adds unit tests with no weak crypto usage: no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or insecure token comparison patterns detected.
Container-Privileges	✅ Passed	The PR adds unit tests to a Go controller, not container/Kubernetes manifests. No privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings are present.
No-Sensitive-Data-In-Logs	✅ Passed	The PR adds only a unit test case with no new logging statements. The test uses non-sensitive data: Route53 Zone ID (publicly visible), DNS hostname, and test infrastructure names that are not secr...
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-06-22T13:15:29Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.85%. Comparing base (8019810) to head (4806b8d).
⚠️ Report is 440 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8794      +/-   ##
==========================================
+ Coverage   42.09%   46.85%   +4.75%     
==========================================
  Files         766      784      +18     
  Lines       95047   108292   +13245     
==========================================
+ Hits        40012    50741   +10729     
- Misses      52221    54419    +2198     
- Partials     2814     3132     +318

see 140 files with indirect coverage changes

Flag	Coverage Δ
cmd-support	`39.94% <ø> (+4.51%)`	⬆️
cpo-hostedcontrolplane	`49.70% <ø> (+5.21%)`	⬆️
cpo-other	`50.98% <ø> (+6.72%)`	⬆️
hypershift-operator	`56.13% <ø> (+4.20%)`	⬆️
other	`34.45% <ø> (+2.88%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sdminonne

I don't have a lot of things to say except. Thank you!

openshift-merge-bot · 2026-07-13T08:27:22Z

Pipeline controller notification

No second-stage tests were triggered for this PR.

This can happen when:

The changed files don't match any pipeline_run_if_changed patterns
All files match pipeline_skip_if_only_changed patterns
No pipeline-controlled jobs are defined for the main branch

Use /test ? to see all available tests.

jparrill

/approve

openshift-ci · 2026-07-13T08:36:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jparrill, mgencur, sdminonne

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:

~~OWNERS~~ [jparrill]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mgencur · 2026-07-13T08:38:55Z

/verified by unit tests

This PR only adds unit tests.

openshift-ci-robot · 2026-07-13T08:39:06Z

@mgencur: This PR has been marked as verified by unit tests.

Details

In response to this:

/verified by unit tests

This PR only adds unit tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

mgencur · 2026-07-14T06:31:54Z

/test all

mgencur · 2026-07-27T11:22:06Z

/test all

bryan-cox · 2026-07-27T11:25:46Z

/pipeline required

openshift-merge-bot · 2026-07-27T11:25:49Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test unit
/test verify

bryan-cox · 2026-07-27T11:50:42Z

/pipeline required

openshift-merge-bot · 2026-07-27T11:50:45Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test unit
/test verify

bryan-cox · 2026-07-27T11:52:19Z

/lgtm

openshift-ci-robot · 2026-07-27T12:04:14Z

@mgencur: This pull request references CNTRLPLANE-3564 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

…ing deletion

What this PR does / why we need it:

Cover the code path where FindRecord() returns a NoSuchHostedZone error because the Route53 hosted zone was already deleted externally. The controller correctly treats this as successful deletion and removes the finalizer.

Which issue(s) this PR fixes:

Fixes https://redhat.atlassian.net/browse/CNTRLPLANE-3564

Special notes for your reviewer:

Checklist:

Subject and description added to both, commit and PR.

Relevant issues have been referenced.

This change includes docs.

This change includes unit tests.

Summary by CodeRabbit

Tests

Improved test coverage for deletion scenarios with external resource removal handling.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

mgencur · 2026-07-27T12:54:30Z

/retest required

mgencur · 2026-07-27T13:02:28Z

/retest-required

mgencur · 2026-07-28T11:25:07Z

/pipeline required

openshift-merge-bot · 2026-07-28T11:25:10Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test unit
/test verify

mgencur · 2026-07-28T11:25:25Z

/retest

openshift-ci · 2026-07-28T11:52:19Z

@mgencur: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

mgencur · 2026-07-28T13:31:26Z

Created #9141 with the same stuff. It seems the items such as the following don't appear there:

ci/prow/e2e-awsWaiting for status to be reported — Waiting for pipeline condition to trigger this job

mgencur · 2026-07-28T13:40:26Z

Closing in favor of #9141

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 22, 2026

openshift-ci Bot added the do-not-merge/needs-area label Jun 22, 2026

openshift-ci Bot requested review from devguyio and sdminonne June 22, 2026 13:08

openshift-ci Bot added area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/platform/aws PR/issue for AWS (AWSPlatform) platform and removed do-not-merge/needs-area labels Jun 22, 2026

sdminonne approved these changes Jul 13, 2026

View reviewed changes

openshift-ci Bot assigned sdminonne Jul 13, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2026

jparrill reviewed Jul 13, 2026

View reviewed changes

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 13, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jul 13, 2026

openshift-ci Bot assigned bryan-cox Jul 27, 2026

mgencur closed this Jul 27, 2026

mgencur reopened this Jul 27, 2026

mgencur mentioned this pull request Jul 28, 2026

CNTRLPLANE-3564: test(awsprivatelink): add unit test for NoSuchHostedZone handling dur… #9141

Merged

4 tasks

mgencur closed this Jul 28, 2026

Uh oh!

Conversation

mgencur commented Jun 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Checklist:

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented Jun 22, 2026

Uh oh!

openshift-ci-robot commented Jun 22, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Checklist:

Uh oh!

coderabbitai Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

codecov Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sdminonne left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-merge-bot Bot commented Jul 13, 2026

Uh oh!

jparrill left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Jul 13, 2026

Uh oh!

mgencur commented Jul 13, 2026

Uh oh!

openshift-ci-robot commented Jul 13, 2026

Uh oh!

mgencur commented Jul 14, 2026

Uh oh!

mgencur commented Jul 27, 2026

Uh oh!

bryan-cox commented Jul 27, 2026

Uh oh!

openshift-merge-bot Bot commented Jul 27, 2026

Uh oh!

bryan-cox commented Jul 27, 2026

Uh oh!

openshift-merge-bot Bot commented Jul 27, 2026

Uh oh!

bryan-cox commented Jul 27, 2026

Uh oh!

openshift-ci-robot commented Jul 27, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Checklist:

Summary by CodeRabbit

Uh oh!

mgencur commented Jul 27, 2026

Uh oh!

mgencur commented Jul 27, 2026

Uh oh!

mgencur commented Jul 28, 2026

Uh oh!

openshift-merge-bot Bot commented Jul 28, 2026

Uh oh!

mgencur commented Jul 28, 2026

Uh oh!

openshift-ci Bot commented Jul 28, 2026

Uh oh!

mgencur commented Jul 28, 2026

Uh oh!

mgencur commented Jul 28, 2026

Uh oh!

Reviewers

mgencur commented Jun 22, 2026 •

edited by coderabbitai Bot

Loading

openshift-ci-robot commented Jun 22, 2026 •

edited by openshift-ci Bot

Loading

coderabbitai Bot commented Jun 22, 2026 •

edited

Loading

codecov Bot commented Jun 22, 2026 •

edited

Loading

openshift-ci-robot commented Jul 27, 2026 •

edited by openshift-ci Bot

Loading