Skip to content

Commit

Permalink
Fix jobsink name generator + add unit and fuzz tests
Browse files Browse the repository at this point in the history
Signed-off-by: Pierangelo Di Pilato <[email protected]>
  • Loading branch information
pierDipi authored and knative-prow-robot committed Nov 19, 2024
1 parent 4fb3666 commit 317556d
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 11 deletions.
21 changes: 10 additions & 11 deletions cmd/jobsink/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"crypto/md5" //nolint:gosec
"crypto/tls"
"encoding/hex"
"fmt"
"log"
"net/http"
Expand Down Expand Up @@ -232,11 +233,11 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

id := toIdHashLabelValue(event.Source(), event.ID())
logger.Debug("Getting job for event", zap.String("URI", r.RequestURI), zap.String("id", id))
jobName := toJobName(ref.Name, event.Source(), event.ID())
logger.Debug("Getting job for event", zap.String("URI", r.RequestURI), zap.String("jobName", jobName))

jobs, err := h.k8s.BatchV1().Jobs(js.GetNamespace()).List(r.Context(), metav1.ListOptions{
LabelSelector: jobLabelSelector(ref, id),
LabelSelector: jobLabelSelector(ref, jobName),
Limit: 1,
})
if err != nil {
Expand All @@ -257,16 +258,14 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

jobName := kmeta.ChildName(ref.Name, id)

logger.Debug("Creating job for event", zap.String("URI", r.RequestURI), zap.String("jobName", jobName))

job := js.Spec.Job.DeepCopy()
job.Name = jobName
if job.Labels == nil {
job.Labels = make(map[string]string, 4)
}
job.Labels[sinks.JobSinkIDLabel] = id
job.Labels[sinks.JobSinkIDLabel] = jobName
job.Labels[sinks.JobSinkNameLabel] = ref.Name
job.OwnerReferences = append(job.OwnerReferences, metav1.OwnerReference{
APIVersion: sinksv.SchemeGroupVersion.String(),
Expand Down Expand Up @@ -333,7 +332,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Name: jobName,
Namespace: ref.Namespace,
Labels: map[string]string{
sinks.JobSinkIDLabel: id,
sinks.JobSinkIDLabel: jobName,
sinks.JobSinkNameLabel: ref.Name,
},
OwnerReferences: []metav1.OwnerReference{
Expand Down Expand Up @@ -398,8 +397,7 @@ func (h *Handler) handleGet(ctx context.Context, w http.ResponseWriter, r *http.
eventSource := parts[6]
eventID := parts[8]

id := toIdHashLabelValue(eventSource, eventID)
jobName := kmeta.ChildName(ref.Name, id)
jobName := toJobName(ref.Name, eventSource, eventID)

job, err := h.k8s.BatchV1().Jobs(ref.Namespace).Get(r.Context(), jobName, metav1.GetOptions{})
if err != nil {
Expand Down Expand Up @@ -452,6 +450,7 @@ func jobLabelSelector(ref types.NamespacedName, id string) string {
return fmt.Sprintf("%s=%s,%s=%s", sinks.JobSinkIDLabel, id, sinks.JobSinkNameLabel, ref.Name)
}

func toIdHashLabelValue(source, id string) string {
return utils.ToDNS1123Subdomain(fmt.Sprintf("%s", md5.Sum([]byte(fmt.Sprintf("%s-%s", source, id))))) //nolint:gosec
func toJobName(js string, source, id string) string {
h := md5.Sum([]byte(source + id)) //nolint:gosec
return kmeta.ChildName(js+"-", utils.ToDNS1123Subdomain(hex.EncodeToString(h[:])))
}
90 changes: 90 additions & 0 deletions cmd/jobsink/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package main

import (
"testing"

"k8s.io/apimachinery/pkg/api/validation"

"knative.dev/eventing/pkg/utils"
)

type testCase struct {
JobSinkName string
Source string
Id string
}

func TestToJobName(t *testing.T) {
testcases := []testCase{
{
JobSinkName: "job-sink-success",
Source: "mysource3/myservice",
Id: "2234-5678",
},
{
JobSinkName: "a",
Source: "0",
Id: "0",
},
}

for _, tc := range testcases {
t.Run(tc.JobSinkName+"_"+tc.Source+"_"+tc.Id, func(t *testing.T) {
if errs := validation.NameIsDNS1035Label(tc.JobSinkName, false); len(errs) != 0 {
t.Errorf("Invalid JobSinkName: %v", errs)
}

name := toJobName(tc.JobSinkName, tc.Source, tc.Id)
doubleName := toJobName(tc.JobSinkName, tc.Source, tc.Id)
if name != doubleName {
t.Errorf("Before: %q, after: %q", name, doubleName)
}

if got := utils.ToDNS1123Subdomain(name); got != name {
t.Errorf("ToDNS1123Subdomain(Want) returns a different result, Want: %q, Got: %q", name, got)
}

if errs := validation.NameIsDNS1035Label(name, false); len(errs) != 0 {
t.Errorf("toJobName produced invalid name %q given %q, %q, %q: errors: %#v", name, tc.JobSinkName, tc.Source, tc.Id, errs)
}
})
}
}

func FuzzToJobName(f *testing.F) {
testcases := []testCase{
{
JobSinkName: "job-sink-success",
Source: "mysource3/myservice",
Id: "2234-5678",
},
{
JobSinkName: "a",
Source: "0",
Id: "0",
},
}

for _, tc := range testcases {
f.Add(tc.JobSinkName, tc.Source, tc.Id) // Use f.Add to provide a seed corpus
}
f.Fuzz(func(t *testing.T, js, source, id string) {
if errs := validation.NameIsDNSLabel(js, false); len(errs) != 0 {
t.Skip("Prerequisite: invalid jobsink name")
}

name := toJobName(js, source, id)
doubleName := toJobName(js, source, id)
if name != doubleName {
t.Errorf("Before: %q, after: %q", name, doubleName)
}

if got := utils.ToDNS1123Subdomain(name); got != name {
t.Errorf("ToDNS1123Subdomain(Want) returns a different result, Want: %q, Got: %q", name, got)
}

if errs := validation.NameIsDNSLabel(name, false); len(errs) != 0 {
t.Errorf("toJobName produced invalid name %q given %q, %q, %q: errors: %#v", name, js, source, id, errs)
}
})
}

0 comments on commit 317556d

Please sign in to comment.