bug: truncate volume names to 63 characters in BackupController executor#1092
bug: truncate volume names to 63 characters in BackupController executor#1092SyeKlu wants to merge 1 commit intok8up-io:masterfrom
Conversation
69fb3a0 to
668e959
Compare
|
Hello. I am also affected by this. Will it be fixed soon? |
|
Thanks for this contribution! We appreciate you taking the time to submit this PR. To be able to merge it, we need the commits to be signed off (DCO — Developer Certificate of Origin), which is a requirement for all CNCF projects. You can do this by rebasing with: (where N is the number of commits in this PR) and then force-pushing to your branch. If you have any questions about this, feel free to ask! |
668e959 to
c1db2e0
Compare
|
The commit is now signed with DCO and ready. |
arska
left a comment
There was a problem hiding this comment.
Thanks for the update and the sign-off!
The fix is correct but I'd suggest two improvements:
- Extract a helper function instead of the inline anonymous function — it's more readable and testable:
// truncateVolumeName ensures the volume name doesn't exceed the Kubernetes 63-character limit.
func truncateVolumeName(name string) string {
if len(name) > 63 {
return name[:63]
}
return name
}Then use it as:
Name: truncateVolumeName(pvc.Name),- Add a unit test to prevent regressions:
func TestTruncateVolumeName(t *testing.T) {
tests := map[string]struct {
input string
expected string
}{
"short name": {
input: "my-pvc",
expected: "my-pvc",
},
"exactly 63 chars": {
input: strings.Repeat("a", 63),
expected: strings.Repeat("a", 63),
},
"over 63 chars": {
input: strings.Repeat("a", 100),
expected: strings.Repeat("a", 63),
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
assert.Equal(t, tc.expected, truncateVolumeName(tc.input))
})
}
}Would you be able to make these changes? Happy to help if needed.
c1db2e0 to
6a29e85
Compare
|
I've adjusted it |
arska
left a comment
There was a problem hiding this comment.
Looks great — clean helper function with good test coverage. Thank you for the update!
|
CI is failing — missing Please add |
Signed-off-by: Sebastian-Sye Klute <sk@teuto.net>
6a29e85 to
a499bb9
Compare
|
I’ve added the import. |
Summary
This PR fixes volume name handling in the BackupController executor to comply with Kubernetes’ 63-character limit.
volumes[*].name is now automatically truncated to 63 characters, preventing errors caused by longer names.
Other related fields, such as persistentVolumeClaim.claimName and configMap.name, remain unaffected.
Related Issue:
Fixes k8up-io/k8up#862