test(e2e): add operator initData validation event test by manmathbh · Pull Request #7392 · karmada-io/karmada

manmathbh · 2026-04-15T04:10:55Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces an End-to-End (E2E) test for the Karmada operator to verify validation events. Specifically, it asserts that the operator correctly emits a ValidationError event when initialized with invalid initData up front.
Note: Following review feedback, the test was refactored to validate failure at creation time with proper nil-pointer safeguards, rather than mutating the object post-readiness.

Which issue(s) this PR fixes:

Fixes #7252

Special notes for your reviewer:

Opening this initially as a Draft PR to leverage the CI pipeline's kind cluster for E2E validation.

Does this PR introduce a user-facing change?:

NONE

gemini-code-assist · 2026-04-15T04:11:09Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the testing infrastructure for the Karmada operator by introducing an end-to-end test case. The goal is to ensure that the operator provides appropriate feedback, specifically through Kubernetes events and status conditions, when it encounters invalid configuration data during reconciliation.

Highlights

New E2E Test Suite: Added a new Ginkgo-based test suite to verify validation event handling in the Karmada operator.
Validation Logic Verification: Implemented a test case that mutates the CRD tarball URL to an invalid value, ensuring the operator correctly updates the Ready condition and emits a warning event.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new E2E test suite to verify that the Karmada operator correctly handles validation failures by emitting warning events and updating the resource status to a non-ready state. The review feedback identifies a potential nil pointer dereference when accessing nested spec fields and points out a discrepancy between the test implementation and the PR's stated scope regarding which fields are being validated.

gemini-code-assist · 2026-04-15T04:14:42Z

+			karmada, err := operatorClient.OperatorV1alpha1().Karmadas(testNamespace).Get(context.TODO(), karmadaName, metav1.GetOptions{})
+			gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
+
+			karmada.Spec.CRDTarball.HTTPSource.URL = "bad-url"


Potential nil pointer dereference. If karmada.Spec.CRDTarball or karmada.Spec.CRDTarball.HTTPSource is not initialized (e.g., by InitializeKarmadaInstance or via defaults), accessing the URL field directly will cause a panic. It is safer to ensure these nested pointers are initialized before assignment.

if karmada.Spec.CRDTarball == nil { karmada.Spec.CRDTarball = &operatorv1alpha1.CRDTarball{} } if karmada.Spec.CRDTarball.HTTPSource == nil { karmada.Spec.CRDTarball.HTTPSource = &operatorv1alpha1.HTTPSource{} } karmada.Spec.CRDTarball.HTTPSource.URL = "bad-url"

gemini-code-assist · 2026-04-15T04:14:42Z

+	operatorresource "github.com/karmada-io/karmada/test/e2e/framework/resource/operator"
+)
+
+var _ = ginkgo.Describe("Karmada validation event testing", func() {


The test implementation and its scope do not align with the PR title and description. The PR description states that the test verifies validation events for missing initData (such as spec.components.karmadaAPIServer or spec.hostCluster.networking.dnsDomain), but the code actually tests an update operation on spec.crdTarball.httpSource.url. Please update the PR description or the test cases to ensure consistency.

References

It is acceptable to merge code with incomplete or placeholder logic for new features when it is an intentional part of the development process.

codecov-commenter · 2026-04-15T04:27:11Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.14%. Comparing base (3424bc7) to head (1bf3a88).
⚠️ Report is 66 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7392      +/-   ##
==========================================
+ Coverage   42.03%   42.14%   +0.10%     
==========================================
  Files         874      875       +1     
  Lines       53551    53603      +52     
==========================================
+ Hits        22511    22590      +79     
+ Misses      29349    29312      -37     
- Partials     1691     1701      +10

Flag	Coverage Δ
unittests	`42.14% <ø> (+0.10%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

mohamedawnallah · 2026-04-15T06:27:06Z

Thanks @manmathbh for the PR! Whenever you have it in a reviewable state, just ping me and I'll take a look as soon as I'm available

manmathbh · 2026-04-15T08:36:44Z

Hi @mohamedawnallah thanks for checking in!

I have refactored the test based on the initial bot feedback:

Added proper nil-pointer safeguards.
Restructured the test to assert invalid initData during the initial object creation, rather than doing a post-readiness mutation (which better aligns with the PR's intent).

The CI suite is completely green. This is now ready for your review. PTAL!

Copilot

Pull request overview

Adds an operator E2E test case to verify that creating a Karmada instance with invalid CRD tarball init data triggers a ValidationError warning event and sets the Ready condition to False, improving event-related E2E coverage (Fixes #7252).

Changes:

Introduce a new operator suite E2E test that creates a Karmada CR with an invalid spec.crdTarball.httpSource.url.
Assert the operator updates status.conditions[Ready] to False with reason ValidationError.
Assert a Warning event with reason ValidationError is emitted containing the expected validation message substring.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-15T08:40:29Z

+		operatorresource.DeleteKarmadaInstance(operatorClient, testNamespace, karmadaName)
+	})
+
+	ginkgo.It("should emit warning event and set Ready condition false when initData is invalid", func() {


The test case title mentions "initData" but the scenario being validated is an invalid CRD tarball HTTP URL (spec.crdTarball.httpSource.url). Consider renaming the It() description to match the actual invalid input so failures are self-explanatory in CI output.

Suggested change

ginkgo.It("should emit warning event and set Ready condition false when initData is invalid", func() {

ginkgo.It("should emit warning event and set Ready condition false when CRD tarball HTTP URL is invalid", func() {

Copilot · 2026-04-15T08:40:30Z

+			if karmada.Spec.CRDTarball == nil {
+				karmada.Spec.CRDTarball = &operatorv1alpha1.CRDTarball{}
+			}
+			if karmada.Spec.CRDTarball.HTTPSource == nil {
+				karmada.Spec.CRDTarball.HTTPSource = &operatorv1alpha1.HTTPSource{}
+			}


helper.NewKarmada() already initializes Spec.CRDTarball and Spec.CRDTarball.HTTPSource as non-nil (test/helper/karmada.go). The nil-guard branches here are therefore redundant and add noise; you can simplify this step by directly setting karmada.Spec.CRDTarball.HTTPSource.URL.

Suggested change

if karmada.Spec.CRDTarball == nil {

karmada.Spec.CRDTarball = &operatorv1alpha1.CRDTarball{}

}

if karmada.Spec.CRDTarball.HTTPSource == nil {

karmada.Spec.CRDTarball.HTTPSource = &operatorv1alpha1.HTTPSource{}

}

Copilot · 2026-04-15T08:40:30Z

+		ginkgo.By("Step 3: assert Warning ValidationError event is emitted", func() {
+			framework.WaitEventFitWith(hostClient, testNamespace, karmadaName, func(event corev1.Event) bool {
+				return event.Type == corev1.EventTypeWarning &&
+					event.Reason == karmadacontroller.ValidationErrorReason &&
+					strings.Contains(event.Message, validationErrorMessageSubstring)
+			})


Step 2 uses the operator suite's configurable pollTimeout/pollInterval, but Step 3 calls framework.WaitEventFitWith(), which uses framework.PollTimeout/PollInterval (hard-coded 420s/5s). This makes the test's effective timeout inconsistent with the suite flags and can unnecessarily prolong failures. Prefer waiting for the event with the suite's pollTimeout/pollInterval (e.g., via a local Eventually) or extend the helper to accept timeout/interval.

Signed-off-by: manmathbh <manmathcode@gmail.com>

karmada-bot · 2026-04-15T09:03:10Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ikaven1024, xishanyongye-chang 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:

hack/OWNERS
test/OWNERS

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

mohamedawnallah

Thanks @manmathbh for the PR! Left some comments, otherwise LGTM

mohamedawnallah · 2026-04-15T20:41:54Z


 # wait karmada-operator ready
-kubectl --kubeconfig="${KUBECONFIG}" --context="${CONTEXT_NAME}" wait --for=condition=Ready --timeout=30s pods -l app.kubernetes.io/name=karmada-operator -n ${KARMADA_SYSTEM_NAMESPACE}
+kubectl --kubeconfig="${KUBECONFIG}" --context="${CONTEXT_NAME}" rollout status deployment/karmada-operator --timeout=90s -n ${KARMADA_SYSTEM_NAMESPACE}


Do we need to make those changes?

mohamedawnallah · 2026-04-15T20:42:10Z

+)
+
+var _ = ginkgo.Describe("Karmada validation event testing", func() {
+	const validationErrorMessageSubstring = "invalid CRDs remote URL"


Let's have a sentinel error perhaps with a concise name in

karmada/operator/pkg/controller/karmada/validating.go

Line 34 in 2ae3c07

and we can reuse it here

mohamedawnallah · 2026-04-15T20:42:28Z

+	ginkgo.It("should emit warning event and set Ready condition false when CRD tarball HTTP URL is invalid", func() {
+		ginkgo.By("Step 1: create the Karmada instance with an invalid CRD tarball URL", func() {
+			karmada := helper.NewKarmada(testNamespace, karmadaName)
+			karmada.Spec.CRDTarball.HTTPSource.URL = "bad-url"


nit:

Suggested change

karmada.Spec.CRDTarball.HTTPSource.URL = "bad-url"

karmada.Spec.CRDTarball.HTTPSource.URL = "invalid-url"

karmada-bot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Apr 15, 2026

karmada-bot requested review from Vacant2333 and mohamedawnallah April 15, 2026 04:11

karmada-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 15, 2026

gemini-code-assist Bot reviewed Apr 15, 2026

View reviewed changes

manmathbh force-pushed the test/operator-validation-events-e2e branch from b8bac34 to 519aa82 Compare April 15, 2026 07:05

manmathbh marked this pull request as ready for review April 15, 2026 08:35

Copilot AI review requested due to automatic review settings April 15, 2026 08:35

karmada-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 15, 2026

karmada-bot requested a review from XiShanYongYe-Chang April 15, 2026 08:35

Copilot started reviewing on behalf of manmathbh April 15, 2026 08:36 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

manmathbh force-pushed the test/operator-validation-events-e2e branch from 519aa82 to 3e436ae Compare April 15, 2026 08:49

test(e2e): add operator initData validation event test

1bf3a88

Signed-off-by: manmathbh <manmathcode@gmail.com>

manmathbh force-pushed the test/operator-validation-events-e2e branch from 3e436ae to 1bf3a88 Compare April 15, 2026 09:03

mohamedawnallah reviewed Apr 15, 2026

View reviewed changes

	ginkgo.It("should emit warning event and set Ready condition false when initData is invalid", func() {
	ginkgo.It("should emit warning event and set Ready condition false when CRD tarball HTTP URL is invalid", func() {

	karmada.Spec.CRDTarball.HTTPSource.URL = "bad-url"
	karmada.Spec.CRDTarball.HTTPSource.URL = "invalid-url"

Conversation

manmathbh commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Uh oh!

gemini-code-assist Bot commented Apr 15, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mohamedawnallah commented Apr 15, 2026

Uh oh!

manmathbh commented Apr 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

karmada-bot commented Apr 15, 2026

Uh oh!

mohamedawnallah left a comment

Choose a reason for hiding this comment

Uh oh!

mohamedawnallah Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

mohamedawnallah Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

mohamedawnallah Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

manmathbh commented Apr 15, 2026 •

edited

Loading

codecov-commenter commented Apr 15, 2026 •

edited

Loading