Skip to content

bug: truncate volume names to 63 characters in BackupController executor#1092

Open
SyeKlu wants to merge 1 commit intok8up-io:masterfrom
SyeKlu:fix-pvc-name-longer-63
Open

bug: truncate volume names to 63 characters in BackupController executor#1092
SyeKlu wants to merge 1 commit intok8up-io:masterfrom
SyeKlu:fix-pvc-name-longer-63

Conversation

@SyeKlu
Copy link
Copy Markdown

@SyeKlu SyeKlu commented Oct 27, 2025

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

@SyeKlu SyeKlu requested a review from a team as a code owner October 27, 2025 15:11
@SyeKlu SyeKlu requested review from tobru and zugao and removed request for a team October 27, 2025 15:11
@SyeKlu SyeKlu force-pushed the fix-pvc-name-longer-63 branch from 69fb3a0 to 668e959 Compare October 27, 2025 15:11
@aw-talos
Copy link
Copy Markdown

aw-talos commented Feb 5, 2026

Hello. I am also affected by this. Will it be fixed soon?

@arska
Copy link
Copy Markdown
Member

arska commented Mar 22, 2026

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:

git rebase --signoff HEAD~N

(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!

@SyeKlu SyeKlu force-pushed the fix-pvc-name-longer-63 branch from 668e959 to c1db2e0 Compare March 23, 2026 07:14
@SyeKlu
Copy link
Copy Markdown
Author

SyeKlu commented Mar 23, 2026

The commit is now signed with DCO and ready.

Copy link
Copy Markdown
Member

@arska arska left a comment

Choose a reason for hiding this comment

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

Thanks for the update and the sign-off!

The fix is correct but I'd suggest two improvements:

  1. 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),
  1. 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.

@SyeKlu SyeKlu force-pushed the fix-pvc-name-longer-63 branch from c1db2e0 to 6a29e85 Compare March 23, 2026 09:43
@SyeKlu
Copy link
Copy Markdown
Author

SyeKlu commented Mar 23, 2026

I've adjusted it

Copy link
Copy Markdown
Member

@arska arska left a comment

Choose a reason for hiding this comment

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

Looks great — clean helper function with good test coverage. Thank you for the update!

@arska
Copy link
Copy Markdown
Member

arska commented Mar 23, 2026

CI is failing — missing strings import in the test file:

vet: operator/backupcontroller/backup_utils_test.go:93:14: undefined: strings

Please add "strings" to the imports in backup_utils_test.go and push.

Signed-off-by: Sebastian-Sye Klute <sk@teuto.net>
@SyeKlu SyeKlu force-pushed the fix-pvc-name-longer-63 branch from 6a29e85 to a499bb9 Compare March 24, 2026 06:39
@SyeKlu
Copy link
Copy Markdown
Author

SyeKlu commented Mar 24, 2026

I’ve added the import.

@SyeKlu SyeKlu requested a review from arska March 26, 2026 06:58
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.

3 participants