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

Add validation for max length of cluster network name #111

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Conversation

rrajendran17
Copy link
Contributor

@rrajendran17 rrajendran17 commented Aug 9, 2024

Problem:
when creating a cluster network using GUI, user specifies a name for cluster network and it is validated for maximum length of 12 at the UI code,but when cluster network with name > 12 chars is created using API, the cluster network is created successfully.

cluster network webhook validator is missing the max length check for cluster network name.
Later it is validated when vlan config is applied on the same cluster network.
But the creation of cluster network should validate it.

Solution:
Added create function in cluster network validator to validate for maximum length of cluster network name.

Related Issue:

harvester/harvester#3826

Test plan:
Manually tested on a harvester node with the changes.

Copy link
Member

@starbops starbops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

Copy link
Contributor

@mingshuoqiu mingshuoqiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.

@rrajendran17 rrajendran17 merged commit 9892fc1 into master Aug 12, 2024
4 checks passed
@rrajendran17 rrajendran17 deleted the br-3826 branch August 12, 2024 16:05
@lanfon72
Copy link
Member

@bk201 does this need backport to old versions?

@bk201
Copy link
Member

bk201 commented Aug 14, 2024

yes, we need a backport. I updated some comments about the workflow and I moved the issue back to review first because the change didn't land in v1.4-head either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants