Skip to content

Conversation

@GautamBytes
Copy link
Contributor

What's in this PR?

This PR significantly improves the quality and reliability of the ResourceDistribution generator by introducing two comprehensive test suites and fixing a critical bug that was uncovered during the process.

The Bug Fix in targets.go

The most important change is a fix to the newNameListRNode function in targets.go.

Problem: The function was generating an invalid YAML structure for namespace lists that would be rejected by the Kubernetes API server.

# Incorrect (Old) Output
includedNamespaces:
  list:
  - name: ns1
  - name: ns2

Solution: The function has been corrected to produce a simple list of strings, which is the valid schema required by the ResourceDistribution CRD.

# Correct (New) Output
includedNamespaces:
  list:
  - ns1
  - ns2

New Test Coverage
Two new test files have been added to validate the generator's behavior:

resourcedistribution_test.go: A high-level test for the main MakeResourceDistribution function.
targets_test.go: A granular unit test for the complex validation and transformation logic in targets.go.

These tests cover all key scenarios, including:
Data generation via literalSources.
Applying options to resources via resourceOptions.
Combining all targeting mechanisms (allNamespaces, included/excluded lists, label selectors).
All error and validation paths.

Signed-off-by: GautamBytes <[email protected]>
@kruise-bot kruise-bot requested review from FillZpp and hantmac July 12, 2025 18:51
@kruise-bot
Copy link

[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 furykerry for approval by writing /assign @furykerry in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

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

@GautamBytes
Copy link
Contributor Author

@furykerry please take a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants