[release-1.12] Support for seconds field in PingSource schedule (#7529)

* feat: support for seconds field in PingSource schedule * chore: additional test added * chore: e2e test for configuring pingsource with seconds field in schedule --------- Co-authored-by: SiBell <simon.bell@campbellsci.co.uk>
knative · Jan 3, 2024 · cb5dfda · cb5dfda
1 parent 65bd6b9
commit cb5dfda
Show file tree

Hide file tree

Showing 8 changed files with 137 additions and 7 deletions.
diff --git a/pkg/adapter/mtping/adapter.go b/pkg/adapter/mtping/adapter.go
@@ -58,7 +58,12 @@ func NewEnvConfig() adapter.EnvConfigAccessor {
 
 func NewAdapter(ctx context.Context, env adapter.EnvConfigAccessor, ceClient cloudevents.Client) adapter.Adapter {
 	logger := logging.FromContext(ctx)
-	runner := NewCronJobsRunner(adapter.GetClientConfig(ctx), kubeclient.Get(ctx), logging.FromContext(ctx))
+
+	opts := cron.WithParser(cron.NewParser(
+		cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor,
+	))
+
+	runner := NewCronJobsRunner(adapter.GetClientConfig(ctx), kubeclient.Get(ctx), logging.FromContext(ctx), opts)
 
 	return &mtpingAdapter{
 		logger:    logger,

diff --git a/pkg/apis/sources/v1/ping_validation.go b/pkg/apis/sources/v1/ping_validation.go
@@ -46,7 +46,11 @@ func (cs *PingSourceSpec) Validate(ctx context.Context) *apis.FieldError {
 		schedule = "CRON_TZ=" + cs.Timezone + " " + schedule
 	}
 
-	if _, err := cron.ParseStandard(schedule); err != nil {
+	parser := cron.NewParser(
+		cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor,
+	)
+
+	if _, err := parser.Parse(schedule); err != nil {
 		if strings.HasPrefix(err.Error(), "provided bad location") {
 			fe := apis.ErrInvalidValue(err, "timezone")
 			errs = errs.Also(fe)

diff --git a/pkg/apis/sources/v1/ping_validation_test.go b/pkg/apis/sources/v1/ping_validation_test.go
@@ -56,6 +56,24 @@ func TestPingSourceValidation(t *testing.T) {
 				},
 			},
 			want: nil,
+		},
+		{
+			name: "valid spec with schedule including seconds",
+			source: PingSource{
+				Spec: PingSourceSpec{
+					Schedule: "10 0/5 * * * ?",
+					SourceSpec: duckv1.SourceSpec{
+						Sink: duckv1.Destination{
+							Ref: &duckv1.KReference{
+								APIVersion: "v1",
+								Kind:       "broker",
+								Name:       "default",
+							},
+						},
+					},
+				},
+			},
+			want: nil,
 		}, {
 			name: "valid spec with timezone",
 			source: PingSource{
@@ -107,7 +125,8 @@ func TestPingSourceValidation(t *testing.T) {
 			want: func() *apis.FieldError {
 				return apis.ErrGeneric("expected at least one, got none", "ref", "uri").ViaField("spec.sink")
 			}(),
-		}, {
+		},
+		{
 			name: "invalid schedule",
 			source: PingSource{
 				Spec: PingSourceSpec{
@@ -125,11 +144,12 @@ func TestPingSourceValidation(t *testing.T) {
 			},
 			want: func() *apis.FieldError {
 				var errs *apis.FieldError
-				fe := apis.ErrInvalidValue("expected exactly 5 fields, found 1: [2]", "spec.schedule")
+				fe := apis.ErrInvalidValue("expected 5 to 6 fields, found 1: [2]", "spec.schedule")
 				errs = errs.Also(fe)
 				return errs
 			}(),
-		}, {
+		},
+		{
 			name: "valid spec with data",
 			source: PingSource{
 				Spec: PingSourceSpec{

diff --git a/pkg/apis/sources/v1beta2/ping_validation.go b/pkg/apis/sources/v1beta2/ping_validation.go
@@ -46,7 +46,11 @@ func (cs *PingSourceSpec) Validate(ctx context.Context) *apis.FieldError {
 		schedule = "CRON_TZ=" + cs.Timezone + " " + schedule
 	}
 
-	if _, err := cron.ParseStandard(schedule); err != nil {
+	parser := cron.NewParser(
+		cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor,
+	)
+
+	if _, err := parser.Parse(schedule); err != nil {
 		if strings.HasPrefix(err.Error(), "provided bad location") {
 			fe := apis.ErrInvalidValue(err, "timezone")
 			errs = errs.Also(fe)

diff --git a/pkg/apis/sources/v1beta2/ping_validation_test.go b/pkg/apis/sources/v1beta2/ping_validation_test.go
@@ -56,6 +56,24 @@ func TestPingSourceValidation(t *testing.T) {
 				},
 			},
 			want: nil,
+		},
+		{
+			name: "valid spec with schedule including seconds",
+			source: PingSource{
+				Spec: PingSourceSpec{
+					Schedule: "10 0/5 * * * ?",
+					SourceSpec: duckv1.SourceSpec{
+						Sink: duckv1.Destination{
+							Ref: &duckv1.KReference{
+								APIVersion: "v1",
+								Kind:       "broker",
+								Name:       "default",
+							},
+						},
+					},
+				},
+			},
+			want: nil,
 		}, {
 			name: "valid spec with timezone",
 			source: PingSource{
@@ -125,7 +143,7 @@ func TestPingSourceValidation(t *testing.T) {
 			},
 			want: func() *apis.FieldError {
 				var errs *apis.FieldError
-				fe := apis.ErrInvalidValue("expected exactly 5 fields, found 1: [2]", "spec.schedule")
+				fe := apis.ErrInvalidValue("expected 5 to 6 fields, found 1: [2]", "spec.schedule")
 				errs = errs.Also(fe)
 				return errs
 			}(),

diff --git a/test/rekt/features/pingsource/features.go b/test/rekt/features/pingsource/features.go
@@ -131,6 +131,26 @@ func SendsEventsWithCloudEventData() *feature.Feature {
 	return f
 }
 
+func SendsEventsWithSecondsInSchedule() *feature.Feature {
+	source := feature.MakeRandomK8sName("pingsource")
+	sink := feature.MakeRandomK8sName("sink")
+	f := feature.NewFeature()
+
+	f.Setup("install sink", eventshub.Install(sink, eventshub.StartReceiver))
+
+	f.Requirement("install pingsource", pingsource.Install(source,
+		pingsource.WithSchedule("10 0/1 * * * ?"),
+		pingsource.WithSink(service.AsDestinationRef(sink)),
+	))
+	f.Requirement("pingsource goes ready", pingsource.IsReady(source))
+
+	f.Stable("pingsource as event source").
+		Must("delivers events",
+			assert.OnStore(sink).MatchEvent(test.HasType("dev.knative.sources.ping")).AtLeast(1))
+
+	return f
+}
+
 // SendsEventsWithEventTypes tests pingsource to a ready broker.
 func SendsEventsWithEventTypes() *feature.Feature {
 	source := feature.MakeRandomK8sName("source")

diff --git a/test/rekt/pingsource_test.go b/test/rekt/pingsource_test.go
@@ -89,3 +89,17 @@ func TestPingSourceWithCloudEventData(t *testing.T) {
 
 	env.Test(ctx, t, pingsource.SendsEventsWithCloudEventData())
 }
+
+func TestPingSourceWithSecondsInSchedule(t *testing.T) {
+	t.Parallel()
+
+	ctx, env := global.Environment(
+		knative.WithKnativeNamespace(system.Namespace()),
+		knative.WithLoggingConfig,
+		knative.WithTracingConfig,
+		k8s.WithEventListener,
+		environment.Managed(t),
+	)
+
+	env.Test(ctx, t, pingsource.SendsEventsWithSecondsInSchedule())
+}
diff --git a/test/rekt/resources/pingsource/pingsource_test.go b/test/rekt/resources/pingsource/pingsource_test.go
@@ -146,3 +146,48 @@ func Example_fullbase64() {
 	//     CACerts: |-
 	//       xyz
 }
+
+func Example_schedule_with_secs() {
+	ctx := testlog.NewContext()
+	images := map[string]string{}
+	cfg := map[string]interface{}{
+		"name":        "foo",
+		"namespace":   "bar",
+		"schedule":    "10 0/5 * * * ?",
+		"contentType": "application/json",
+		"data":        `{"message": "Hello world!"}`,
+		"sink": map[string]interface{}{
+			"ref": map[string]string{
+				"kind":       "sinkkind",
+				"namespace":  "sinknamespace",
+				"name":       "sinkname",
+				"apiVersion": "sinkversion",
+			},
+			"uri": "uri/parts",
+		},
+	}
+
+	files, err := manifest.ExecuteYAML(ctx, yaml, images, cfg)
+	if err != nil {
+		panic(err)
+	}
+
+	manifest.OutputYAML(os.Stdout, files)
+	// Output:
+	// apiVersion: sources.knative.dev/v1
+	// kind: PingSource
+	// metadata:
+	//   name: foo
+	//   namespace: bar
+	// spec:
+	//   schedule: '10 0/5 * * * ?'
+	//   contentType: 'application/json'
+	//   data: '{"message": "Hello world!"}'
+	//   sink:
+	//     ref:
+	//       kind: sinkkind
+	//       namespace: sinknamespace
+	//       name: sinkname
+	//       apiVersion: sinkversion
+	//     uri: uri/parts
+}