diff --git a/cmd/jobsink/main.go b/cmd/jobsink/main.go index 65c678b1c76..99f6b1f1739 100644 --- a/cmd/jobsink/main.go +++ b/cmd/jobsink/main.go @@ -18,8 +18,10 @@ package main import ( "context" - "crypto/md5" //nolint:gosec + "crypto/md5" + //nolint:gosec "crypto/tls" + "encoding/hex" "fmt" "log" "net/http" @@ -231,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 { @@ -256,8 +258,6 @@ 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() @@ -265,7 +265,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { 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(), @@ -332,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{ @@ -396,8 +396,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 { @@ -450,6 +449,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)) + return kmeta.ChildName(js, "-" + utils.ToDNS1123Subdomain(hex.EncodeToString(h[:]))) } diff --git a/cmd/jobsink/main_test.go b/cmd/jobsink/main_test.go new file mode 100644 index 00000000000..0862d401ffe --- /dev/null +++ b/cmd/jobsink/main_test.go @@ -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.NameIsDNS1035Label(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.NameIsDNS1035Label(name, false); len(errs) != 0 { + t.Errorf("toJobName produced invalid name %q given %q, %q, %q: errors: %#v", name, js, source, id, errs) + } + }) +}