Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][broker]: validation for at least one TlsUrl / nonTlsUrl is present for cluster creation/update #19965

Closed
wants to merge 8 commits into from

Conversation

niharrathod
Copy link
Contributor

@niharrathod niharrathod commented Mar 29, 2023

Fixes #19866

Motivation

Verify either TlsUrl or nonTlsUrl is provided as part of Cluster create and update operation.
This validation is not applicable for "global" cluster create / update operation.

Modifications

  • Added validation either serviceUrl or serviceUrlTls is set as part of cluster creation and update API.
  • Added validation either brokerServiceUrl or brokerServiceUrlTls is set as part of cluster creation and update API.
  • Added unit test cases.
  • Fixed old test-cases.

Core changes are only in two files ClustersBase.java, ClusterDataImpl.java, added unit test w.r.t same in ClusterDataTest.java
rest 115 file diff is because of patching existing test cases.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added can be verified as follows:

  • unit tests added
  • fixed old test cases

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 29, 2023
@niharrathod
Copy link
Contributor Author

At first site this looks big change, but actual changes are only in two files ClustersBase.java, ClusterDataImpl.java where validation is added, unit test w.r.t same in ClusterDataTest.java, rest 115 file diff is because of patching existing test cases, where brokerServiceUrl is set to satisfy the validation.

Kindly requesting for a review. I am happy to discuss & do any improvisation / further work if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verify at least one URL is present for cluster creation
1 participant