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 name length validation for driver #98

Closed
wants to merge 1 commit into from
Closed

Conversation

Madhu-1
Copy link
Collaborator

@Madhu-1 Madhu-1 commented Aug 6, 2024

Describe what this PR does

Choosing an arbitrary length which is 56 for the csi driver name and adding a validation to block CR creation i the name has more chars.

Is there anything that requires special attention

Do you have any questions?

No

Is the change backward compatible?

Yes but only to an extent

Are there concerns around backward compatibility?

Yes, the migration not be possible for the existing driver if names are beyond 63 chars, or else we don't have a problem.

Related issues

Fixes: #44

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 6, 2024

Seeking more advice and it's a hard restriction with names and am open to another number as well. @nb-ohad @travisn @BlaineEXE

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 6, 2024

we create svc from the driver name which can be a maximum 63 chars, we also append liveness to it which is 8, with this restriction 54+8=62

Choosing an arbitary length which is
54 for the csi driver name and adding
a validation to block CR creation if
the name is having more chars.

Signed-off-by: Madhu Rajanna <[email protected]>
@nb-ohad
Copy link
Collaborator

nb-ohad commented Aug 6, 2024

/hold
I believe this needs more discussion

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 6, 2024

New discussed approach is to put a restriction on the CRD name to match the length restriction of the kubernetes csidriver object, not the minimal resource length limit and adjust all other dependent resources' names to consider the length and generate the name to ensure that we don't go beyond the kubernetes name restriction for all the resources like deployment/daemonset/service etc.

@BlaineEXE
Copy link

54 chars seems like plenty to me.

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 7, 2024

For me, this makes sense as well and not to think too much about it, if someone wants to have a different solution am going to close it and let others do it.

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 8, 2024

Closing it as am not working on it anymore

@Madhu-1 Madhu-1 closed this Aug 8, 2024
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.

Add length check on the driver CR name
3 participants