Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions docs/09-Configuration reference/settings.catalog.json
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@
"key": "jobs.\u003cowner-kind\u003e.containers.\u003ccontainer-name\u003e.env-vars",
"valueType": "map[string]string",
"sources": [
"internal/resources/jobs/job.go:122"
"internal/resources/jobs/job.go:124"
]
},
{
Expand All @@ -383,14 +383,14 @@
}
],
"sources": [
"internal/resources/jobs/job.go:66"
"internal/resources/jobs/job.go:68"
]
},
{
"key": "jobs.\u003cowner-kind\u003e.init-containers.\u003ccontainer-name\u003e.env-vars",
"valueType": "map[string]string",
"sources": [
"internal/resources/jobs/job.go:132"
"internal/resources/jobs/job.go:134"
]
},
{
Expand All @@ -408,14 +408,14 @@
}
],
"sources": [
"internal/resources/jobs/job.go:77"
"internal/resources/jobs/job.go:79"
]
},
{
"key": "jobs.\u003cowner-kind\u003e.spec.template.annotations",
"valueType": "map[string]string",
"sources": [
"internal/resources/jobs/job.go:150"
"internal/resources/jobs/job.go:152"
]
},
{
Expand Down
22 changes: 21 additions & 1 deletion internal/resources/jobs/job.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package jobs

import (
"crypto/sha256"
"encoding/hex"
"fmt"
"maps"
"strings"
Expand Down Expand Up @@ -165,6 +167,24 @@ func withSettingAnnotations(ctx core.Context, owner v1beta1.Dependent) HandleJob
})
}

// maxNameLength is the longest Job name the apiserver accepts: the Job
// controller copies the name into the job-name label, capped at 63 bytes.
const maxNameLength = 63

// Name returns the name of the Job that Handle creates for owner.
// Names that fit are left unchanged, so existing Jobs keep their name.
// Longer names are truncated and suffixed with a hash of the full name.
func Name(owner client.Object, jobName string) string {
name := fmt.Sprintf("%s-%s", owner.GetUID(), jobName)
if len(name) <= maxNameLength {
return name
}

hash := sha256.Sum256([]byte(name))
suffix := hex.EncodeToString(hash[:4])
return name[:maxNameLength-len(suffix)-1] + "-" + suffix
}

func Handle(ctx core.Context, owner v1beta1.Dependent, jobName string, container v1.Container, options ...HandleJobOption) error {
configuration := &handleJobConfiguration{}
if options == nil {
Expand All @@ -183,7 +203,7 @@ func Handle(ctx core.Context, owner v1beta1.Dependent, jobName string, container
option(configuration)
}

jobName = fmt.Sprintf("%s-%s", owner.GetUID(), jobName)
jobName = Name(owner, jobName)
job := &batchv1.Job{}
err := ctx.GetClient().Get(ctx, types.NamespacedName{
Namespace: owner.GetStack(),
Expand Down
88 changes: 88 additions & 0 deletions internal/resources/jobs/name_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package jobs

import (
"strings"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"

"github.com/formancehq/operator/v3/api/formance.com/v1beta1"
)

func TestName(t *testing.T) {
t.Parallel()

owner := &v1beta1.Broker{
ObjectMeta: metav1.ObjectMeta{
UID: types.UID("120b264e-5be3-49a8-bd4a-71e2ea097ae3"),
},
}

tests := []struct {
name string
jobName string
want string
}{
{
name: "short name is unchanged",
jobName: "create-stream",
want: "120b264e-5be3-49a8-bd4a-71e2ea097ae3-create-stream",
},
{
name: "63-byte name is unchanged",
jobName: "create-topic-reconciliatio",
want: "120b264e-5be3-49a8-bd4a-71e2ea097ae3-create-topic-reconciliatio",
},
{
name: "64-byte name is shortened",
jobName: "create-topic-reconciliation",
},
{
name: "much longer name is shortened",
jobName: "create-topic-transactionplane-with-a-very-long-suffix",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

got := Name(owner, tt.jobName)
if tt.want != "" && got != tt.want {
t.Fatalf("Name() = %q, want %q", got, tt.want)
}
if len(got) > maxNameLength {
t.Fatalf("Name() = %q is %d bytes, want at most %d", got, len(got), maxNameLength)
}
if errs := validation.IsValidLabelValue(got); len(errs) > 0 {
t.Fatalf("Name() = %q is not a valid label value: %v", got, errs)
}
if errs := validation.IsDNS1123Subdomain(got); len(errs) > 0 {
t.Fatalf("Name() = %q is not a valid object name: %v", got, errs)
}
if !strings.HasPrefix(got, string(owner.GetUID())+"-") {
t.Fatalf("Name() = %q does not start with the owner UID", got)
}
if again := Name(owner, tt.jobName); again != got {
t.Fatalf("Name() is not stable: %q then %q", got, again)
}
})
}
}

func TestNameDistinguishesLongNamesWithSamePrefix(t *testing.T) {
t.Parallel()

owner := &v1beta1.Broker{
ObjectMeta: metav1.ObjectMeta{
UID: types.UID("120b264e-5be3-49a8-bd4a-71e2ea097ae3"),
},
}

first := Name(owner, "create-topic-reconciliation-a")
second := Name(owner, "create-topic-reconciliation-b")
if first == second {
t.Fatalf("Name() returned %q for two different job names", first)
}
}
19 changes: 17 additions & 2 deletions internal/tests/jobs_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,14 @@ var _ = Describe("Job", func() {
})
It("Should have annotations set", func() {
j := &batchv1.Job{}
Expect(LoadResource(stack.Name, fmt.Sprintf("%s-%s", module.GetUID(), job.Name), j)).To(Succeed())
Expect(LoadResource(stack.Name, jobs.Name(module, job.Name), j)).To(Succeed())

Expect(j.Spec.Template.Annotations).ToNot(BeNil())
Expect(j.Spec.Template.Annotations["first"]).To(Equal("second"))
})
It("Should have security context configured with run-as settings", func() {
j := &batchv1.Job{}
Expect(LoadResource(stack.Name, fmt.Sprintf("%s-%s", module.GetUID(), job.Name), j)).To(Succeed())
Expect(LoadResource(stack.Name, jobs.Name(module, job.Name), j)).To(Succeed())

for _, container := range j.Spec.Template.Spec.Containers {
Expect(container.SecurityContext).ToNot(BeNil())
Expand All @@ -171,6 +171,21 @@ var _ = Describe("Job", func() {
Expect(*container.SecurityContext.RunAsGroup).To(Equal(int64(runAS.group)))
}
})
Context("With a name too long for the job-name label", func() {
BeforeEach(func() {
// 36-byte owner UID + "-" + 27 bytes = 64 bytes, one over the label limit.
job.Name = "create-topic-reconciliation"
})
It("Should create the job under a shortened name", func() {
name := jobs.Name(module, job.Name)
Expect(len(module.GetUID()) + 1 + len(job.Name)).To(Equal(64))
Expect(len(name)).To(BeNumerically("<=", 63))

j := &batchv1.Job{}
Expect(LoadResource(stack.Name, name, j)).To(Succeed())
Expect(j).To(BeControlledBy(module))
})
})
AfterEach(func() {
for _, setting := range settings {
Expect(Delete(&setting)).To(Succeed())
Expand Down
Loading