From c6dfcba06a4d237eda67d3c679b7983ff2ac391b Mon Sep 17 00:00:00 2001 From: Pavel Novitskiy Date: Fri, 9 May 2025 16:42:38 +0200 Subject: [PATCH 1/3] fix generating incorrect names for Jobs --- pkg/sync/sync_context.go | 2 +- pkg/sync/sync_context_test.go | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index 3420a351c..d0898e094 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -754,7 +754,7 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) { if targetObj.GetName() == "" { var syncRevision string if len(sc.revision) >= 8 { - syncRevision = sc.revision[0:7] + syncRevision = strings.Trim(sc.revision[0:7], ".") } else { syncRevision = sc.revision } diff --git a/pkg/sync/sync_context_test.go b/pkg/sync/sync_context_test.go index 5cf77db32..edb31cdff 100644 --- a/pkg/sync/sync_context_test.go +++ b/pkg/sync/sync_context_test.go @@ -13,6 +13,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" @@ -977,6 +978,24 @@ func TestUnnamedHooksGetUniqueNames(t *testing.T) { assert.Contains(t, tasks[1].name(), "foobar-postsync-") assert.Equal(t, "", pod.GetName()) }) + + t.Run("Long revision with dots", func(t *testing.T) { + syncCtx := newTestSyncCtx(nil) + pod := testingutils.NewPod() + pod.SetName("") + pod.SetAnnotations(map[string]string{synccommon.AnnotationKeyHook: "PreSync,PostSync"}) + syncCtx.hooks = []*unstructured.Unstructured{pod} + syncCtx.revision = "f.ooba.r" + tasks, successful := syncCtx.getSyncTasks() + + assert.True(t, successful) + assert.Len(t, tasks, 2) + assert.Contains(t, tasks[0].name(), "f.ooba-presync-") + assert.Contains(t, tasks[1].name(), "f.ooba-postsync-") + assert.Equal(t, "", pod.GetName()) + assert.Len(t, validation.IsDNS1123Subdomain(tasks[0].name()), 0) + assert.Len(t, validation.IsDNS1123Subdomain(tasks[1].name()), 0) + }) } func TestManagedResourceAreNotNamed(t *testing.T) { From e9b4195da86e1ac92f2ed63ea530bec406ed2aeb Mon Sep 17 00:00:00 2001 From: Pavel Novitskiy Date: Fri, 9 May 2025 16:51:00 +0200 Subject: [PATCH 2/3] use assert.Empty --- pkg/sync/sync_context_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/sync/sync_context_test.go b/pkg/sync/sync_context_test.go index edb31cdff..63a4936f7 100644 --- a/pkg/sync/sync_context_test.go +++ b/pkg/sync/sync_context_test.go @@ -993,8 +993,8 @@ func TestUnnamedHooksGetUniqueNames(t *testing.T) { assert.Contains(t, tasks[0].name(), "f.ooba-presync-") assert.Contains(t, tasks[1].name(), "f.ooba-postsync-") assert.Equal(t, "", pod.GetName()) - assert.Len(t, validation.IsDNS1123Subdomain(tasks[0].name()), 0) - assert.Len(t, validation.IsDNS1123Subdomain(tasks[1].name()), 0) + assert.Empty(t, validation.IsDNS1123Subdomain(tasks[0].name())) + assert.Empty(t, validation.IsDNS1123Subdomain(tasks[1].name())) }) } From fc2c758b63289864be16daf731b95d5183d0456d Mon Sep 17 00:00:00 2001 From: Pavel Novitskiy Date: Fri, 9 May 2025 17:04:51 +0200 Subject: [PATCH 3/3] use TrimFunc to keep only letters,numbers and dash --- pkg/sync/sync_context.go | 5 ++++- pkg/sync/sync_context_test.go | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index d0898e094..b823a88dd 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -8,6 +8,7 @@ import ( "strings" "sync" "time" + "unicode" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -754,7 +755,9 @@ func (sc *syncContext) getSyncTasks() (_ syncTasks, successful bool) { if targetObj.GetName() == "" { var syncRevision string if len(sc.revision) >= 8 { - syncRevision = strings.Trim(sc.revision[0:7], ".") + syncRevision = strings.TrimFunc(sc.revision[0:7], func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsNumber(r) && r != '-' + }) } else { syncRevision = sc.revision } diff --git a/pkg/sync/sync_context_test.go b/pkg/sync/sync_context_test.go index 63a4936f7..0cb9f0862 100644 --- a/pkg/sync/sync_context_test.go +++ b/pkg/sync/sync_context_test.go @@ -993,8 +993,8 @@ func TestUnnamedHooksGetUniqueNames(t *testing.T) { assert.Contains(t, tasks[0].name(), "f.ooba-presync-") assert.Contains(t, tasks[1].name(), "f.ooba-postsync-") assert.Equal(t, "", pod.GetName()) - assert.Empty(t, validation.IsDNS1123Subdomain(tasks[0].name())) - assert.Empty(t, validation.IsDNS1123Subdomain(tasks[1].name())) + assert.Empty(t, validation.IsDNS1123Subdomain(tasks[0].name())) + assert.Empty(t, validation.IsDNS1123Subdomain(tasks[1].name())) }) }