Skip to content

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

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
mgencur:CNTRLPLANE-3564_aws_zone_missing_2
Jul 28, 2026
Merged

CNTRLPLANE-3564: test(awsprivatelink): add unit test for NoSuchHostedZone handling dur…#9141
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
mgencur:CNTRLPLANE-3564_aws_zone_missing_2

Conversation

@mgencur

@mgencur mgencur commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

…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

  • Bug Fixes
    • Improved cleanup handling when an AWS Route 53 hosted zone has already been deleted.
    • Endpoint service deletion now completes successfully and removes its finalizer instead of reporting an error.

…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

Copy link
Copy Markdown
Contributor

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 openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 28, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 28, 2026

Copy link
Copy Markdown

@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

Same stuff as in #8794 , checking if the PR will still be blocked by CI checks not running.

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

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

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The AWS PrivateLink deletion test suite adds a table-driven scenario for an externally deleted Route53 hosted zone. The test configures an AWSEndpointService with DNS metadata, mocks ListResourceRecordSets to return NoSuchHostedZone, and verifies reconciliation succeeds without an error while removing the resource finalizer.

Possibly related PRs

Suggested reviewers: bryan-cox

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
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 The added test case uses a static, descriptive name; no dynamic or unstable identifiers were introduced by this PR.
Test Structure And Quality ✅ Passed PASS: The new table-driven unit case is isolated, follows existing fake-client/mock patterns, and has no cluster setup, waits, or cleanup gaps.
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The PR only adds a unit-test case in awsprivatelink_controller_test.go; no deployment manifests, replicas, nodeSelectors, affinity, or scheduling logic changed.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PASS: This PR only adds a unit test in awsprivatelink_controller_test.go; it’s not a Ginkgo e2e test and the new case uses mocked Route53/EC2, not external connectivity.
No-Weak-Crypto ✅ Passed The added test only mocks Route53 deletion paths; I found no MD5/SHA1/DES/RC4/custom crypto or secret/token comparisons in the touched code.
Container-Privileges ✅ Passed Only a Go unit test file changed, and the diff scan found no privileged/hostPID/hostNetwork/allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs ✅ Passed No new logging was added in the PR; the change only adds a deletion test with fixture IDs and a mocked NoSuchHostedZone error, not emitted logs.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the change: adding an awsprivatelink unit test for NoSuchHostedZone deletion handling.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@mgencur

mgencur commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

/pipeline required

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

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

@openshift-ci
openshift-ci Bot requested review from Nirshal and muraee July 28, 2026 13:26
@openshift-ci 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 Jul 28, 2026
@mgencur mgencur changed the title [TEST ONLY] CNTRLPLANE-3564: test(awsprivatelink): add unit test for NoSuchHostedZone handling dur… CNTRLPLANE-3564: test(awsprivatelink): add unit test for NoSuchHostedZone handling dur… Jul 28, 2026
@bryan-cox

Copy link
Copy Markdown
Member

/approve

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.61%. Comparing base (5b60ff2) to head (84bd23b).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9141   +/-   ##
=======================================
  Coverage   44.61%   44.61%           
=======================================
  Files         774      774           
  Lines       97098    97098           
=======================================
+ Hits        43317    43323    +6     
+ Misses      50785    50780    -5     
+ Partials     2996     2995    -1     

see 1 file with indirect coverage changes

Flag Coverage Δ
cmd-support 38.39% <ø> (ø)
cpo-hostedcontrolplane 47.22% <ø> (ø)
cpo-other 45.29% <ø> (+0.04%) ⬆️
hypershift-operator 54.55% <ø> (ø)
other 33.30% <ø> (ø)

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.

@mgencur

mgencur commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

/verified by unit tests

This PR only adds unit tests.

@openshift-ci-robot

Copy link
Copy Markdown

@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.

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

openshift-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox, mgencur

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

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

Copy link
Copy Markdown
Contributor

/lgtm

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

Copy link
Copy Markdown
Contributor

Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage.

@openshift-ci

openshift-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

@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.

@openshift-merge-bot
openshift-merge-bot Bot merged commit 4b22cfd into openshift:main Jul 28, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. 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 jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants