From cb5dfdac159a096e51ec839563d75ed5000091f7 Mon Sep 17 00:00:00 2001 From: Knative Prow Robot Date: Wed, 3 Jan 2024 18:38:11 +0000 Subject: [PATCH] [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 --- pkg/adapter/mtping/adapter.go | 7 ++- pkg/apis/sources/v1/ping_validation.go | 6 ++- pkg/apis/sources/v1/ping_validation_test.go | 26 +++++++++-- pkg/apis/sources/v1beta2/ping_validation.go | 6 ++- .../sources/v1beta2/ping_validation_test.go | 20 ++++++++- test/rekt/features/pingsource/features.go | 20 +++++++++ test/rekt/pingsource_test.go | 14 ++++++ .../resources/pingsource/pingsource_test.go | 45 +++++++++++++++++++ 8 files changed, 137 insertions(+), 7 deletions(-) diff --git a/pkg/adapter/mtping/adapter.go b/pkg/adapter/mtping/adapter.go index 589ee80501c..bbec93abccf 100644 --- 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 index 30345e3c783..5bed8c7b66d 100644 --- 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 index dc7f15e5472..9ba4aed730e 100644 --- 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 index 2f5a088f7fd..de43bf2e90e 100644 --- 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 index 45b509d73d2..779e3d97883 100644 --- 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 index 06cb09515e2..720f1fae983 100644 --- 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 index fda0c7e9e79..26f47da33ef 100644 --- 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 index fc4dd192397..f49a8e168e9 100644 --- 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 +}