diff --git a/charts/vela-core/templates/defwithtemplate/notification.yaml b/charts/vela-core/templates/defwithtemplate/notification.yaml index 0f0d1f83c..452b15110 100644 --- a/charts/vela-core/templates/defwithtemplate/notification.yaml +++ b/charts/vela-core/templates/defwithtemplate/notification.yaml @@ -161,6 +161,14 @@ spec: body: string } } + // +usage=The timeout of HTTP notifications (Go duration string, e.g. "30s", "2m", "500ms"). Defaults to 3s when omitted. Invalid values fail when the step runs. + timeout?: string & =~"^(0|(([0-9]+(\\.[0-9]*)?|\\.[0-9]+)(ns|us|µs|μs|ms|s|m|h))+)$" + } + + httpRequestOpts: { + if parameter.timeout != _|_ { + timeout: parameter.timeout + } } block: { @@ -222,7 +230,7 @@ spec: request: { body: json.Marshal(parameter.dingding.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -246,7 +254,7 @@ spec: request: { body: json.Marshal(parameter.dingding.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -263,7 +271,7 @@ spec: request: { body: json.Marshal(parameter.lark.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -287,7 +295,7 @@ spec: request: { body: json.Marshal(parameter.lark.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } @@ -305,7 +313,7 @@ spec: request: { body: json.Marshal(parameter.slack.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -329,7 +337,7 @@ spec: request: { body: json.Marshal(parameter.slack.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } diff --git a/charts/vela-core/templates/defwithtemplate/request.yaml b/charts/vela-core/templates/defwithtemplate/request.yaml index b6767d853..7c6b22b66 100644 --- a/charts/vela-core/templates/defwithtemplate/request.yaml +++ b/charts/vela-core/templates/defwithtemplate/request.yaml @@ -29,6 +29,9 @@ spec: if parameter.header != _|_ { header: parameter.header } + if parameter.timeout != _|_ { + timeout: parameter.timeout + } } } } @@ -53,5 +56,7 @@ spec: method: *"GET" | "POST" | "PUT" | "DELETE" body?: {...} header?: [string]: string + // +usage=The timeout of this request (Go duration string, e.g. "30s", "2m", "500ms"). Defaults to 3s when omitted. Invalid values fail when the step runs. + timeout?: string & =~"^(0|(([0-9]+(\\.[0-9]*)?|\\.[0-9]+)(ns|us|µs|μs|ms|s|m|h))+)$" } diff --git a/charts/vela-core/templates/defwithtemplate/webhook.yaml b/charts/vela-core/templates/defwithtemplate/webhook.yaml index 3bb8179a0..80f54e10a 100644 --- a/charts/vela-core/templates/defwithtemplate/webhook.yaml +++ b/charts/vela-core/templates/defwithtemplate/webhook.yaml @@ -37,6 +37,11 @@ spec: value: json.Marshal(parameter.data) } } + httpRequestOpts: { + if parameter.timeout != _|_ { + timeout: parameter.timeout + } + } webhook: { if parameter.url.value != _|_ { req: http.#HTTPDo & { @@ -46,7 +51,7 @@ spec: request: { body: data.value header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -70,7 +75,7 @@ spec: request: { body: data.value header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -90,5 +95,7 @@ spec: }) // +usage=Specify the data you want to send data?: {...} + // +usage=The timeout of this request (Go duration string, e.g. "30s", "2m", "500ms"). Defaults to 3s when omitted. Invalid values fail when the step runs. + timeout?: string & =~"^(0|(([0-9]+(\\.[0-9]*)?|\\.[0-9]+)(ns|us|µs|μs|ms|s|m|h))+)$" } diff --git a/docs/examples/workflow/notification/app.yaml b/docs/examples/workflow/notification/app.yaml index 6b6af22f6..44c22feb6 100644 --- a/docs/examples/workflow/notification/app.yaml +++ b/docs/examples/workflow/notification/app.yaml @@ -21,6 +21,7 @@ spec: - name: notification type: notification properties: + timeout: 30s dingding: # directly specify the webhook url url: diff --git a/pkg/workflow/template/http_timeout_step_test.go b/pkg/workflow/template/http_timeout_step_test.go new file mode 100644 index 000000000..4893dab9f --- /dev/null +++ b/pkg/workflow/template/http_timeout_step_test.go @@ -0,0 +1,92 @@ +/* +Copyright 2021 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package template + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func workflowstepDefDir(t *testing.T) string { + t.Helper() + _, filename, _, ok := runtime.Caller(0) + require.True(t, ok) + // pkg/workflow/template -> repo root + root := filepath.Clean(filepath.Join(filepath.Dir(filename), "../../..")) + return filepath.Join(root, "vela-templates/definitions/internal/workflowstep") +} + +func readWorkflowStepCue(t *testing.T, name string) string { + t.Helper() + path := filepath.Join(workflowstepDefDir(t), name) + data, err := os.ReadFile(path) + require.NoError(t, err, "read %s", path) + return string(data) +} + +func assertTimeoutPlumbing(t *testing.T, content, step string) { + t.Helper() + assert.Contains(t, content, `timeout?: string & =~"^(0|(([0-9]+(\\.[0-9]*)?|\\.[0-9]+)(ns|us|µs|μs|ms|s|m|h))+)$"`, + "%s should expose optional timeout parameter with Go ParseDuration-compatible validation", step) + assert.Contains(t, content, "Invalid values fail when the step runs", + "%s should document runtime failure for invalid timeout values", step) +} + +func assertSharedHTTPRequestOpts(t *testing.T, content, step string, mergeCount int) { + t.Helper() + assert.Contains(t, content, "httpRequestOpts:", + "%s should define shared httpRequestOpts for timeout forwarding", step) + assert.Equal(t, 1, strings.Count(content, "if parameter.timeout != _|_"), + "%s should forward timeout in one shared httpRequestOpts block", step) + assert.Equal(t, mergeCount, strings.Count(content, "& httpRequestOpts"), + "%s should merge httpRequestOpts into each HTTPDo request block", step) +} + +func TestRequestStepExposesHTTPTimeout(t *testing.T) { + content := readWorkflowStepCue(t, "request.cue") + assertTimeoutPlumbing(t, content, "request") + assert.Contains(t, content, "if parameter.timeout != _|_", + "request should forward timeout in its HTTPDo request block") + assert.Contains(t, content, "timeout: parameter.timeout", + "request should forward timeout to http.#HTTPDo as request.timeout") + assert.Equal(t, 1, strings.Count(content, "if parameter.timeout != _|_"), + "request should forward timeout in a single HTTPDo request block") +} + +func TestWebhookStepExposesHTTPTimeout(t *testing.T) { + content := readWorkflowStepCue(t, "webhook.cue") + assertTimeoutPlumbing(t, content, "webhook") + assertSharedHTTPRequestOpts(t, content, "webhook", 2) +} + +func TestNotificationStepExposesHTTPTimeout(t *testing.T) { + content := readWorkflowStepCue(t, "notification.cue") + assertTimeoutPlumbing(t, content, "notification") + assertSharedHTTPRequestOpts(t, content, "notification", 6) + // Email uses email.#SendEmail and must not merge HTTP timeout opts. + emailIdx := strings.Index(content, "email0:") + require.Greater(t, emailIdx, 0) + emailSection := content[emailIdx:] + assert.NotContains(t, emailSection, "& httpRequestOpts", + "email path should not merge HTTP timeout opts") +} diff --git a/references/docgen/def-doc/workflowstep/notification.eg.md b/references/docgen/def-doc/workflowstep/notification.eg.md index 423b45c9f..3f59b7316 100644 --- a/references/docgen/def-doc/workflowstep/notification.eg.md +++ b/references/docgen/def-doc/workflowstep/notification.eg.md @@ -22,6 +22,7 @@ spec: - name: dingtalk-message type: notification properties: + timeout: 30s dingding: # the DingTalk webhook address, please refer to: https://developers.dingtalk.com/document/robots/custom-robot-access url: @@ -35,6 +36,7 @@ spec: - name: slack-message type: notification properties: + timeout: 30s slack: # the Slack webhook address, please refer to: https://api.slack.com/messaging/webhooks url: diff --git a/references/docgen/def-doc/workflowstep/request.eg.md b/references/docgen/def-doc/workflowstep/request.eg.md index 9f4e1ab24..cfdea073e 100644 --- a/references/docgen/def-doc/workflowstep/request.eg.md +++ b/references/docgen/def-doc/workflowstep/request.eg.md @@ -12,6 +12,7 @@ spec: type: request properties: url: https://api.github.com/repos/kubevela/workflow + timeout: 30s outputs: - name: stars valueFrom: | diff --git a/references/docgen/def-doc/workflowstep/webhook.eg.md b/references/docgen/def-doc/workflowstep/webhook.eg.md index c6095a3d7..0cce21261 100644 --- a/references/docgen/def-doc/workflowstep/webhook.eg.md +++ b/references/docgen/def-doc/workflowstep/webhook.eg.md @@ -20,4 +20,5 @@ spec: properties: url: value: -``` \ No newline at end of file + timeout: 30s +``` diff --git a/vela-templates/definitions/internal/workflowstep/notification.cue b/vela-templates/definitions/internal/workflowstep/notification.cue index b8cd241fd..ce23a0d8f 100644 --- a/vela-templates/definitions/internal/workflowstep/notification.cue +++ b/vela-templates/definitions/internal/workflowstep/notification.cue @@ -158,6 +158,14 @@ template: { body: string } } + // +usage=The timeout of HTTP notifications (Go duration string, e.g. "30s", "2m", "500ms"). Defaults to 3s when omitted. Invalid values fail when the step runs. + timeout?: string & =~"^(0|(([0-9]+(\\.[0-9]*)?|\\.[0-9]+)(ns|us|µs|μs|ms|s|m|h))+)$" + } + + httpRequestOpts: { + if parameter.timeout != _|_ { + timeout: parameter.timeout + } } block: { @@ -219,7 +227,7 @@ template: { request: { body: json.Marshal(parameter.dingding.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -245,7 +253,7 @@ template: { request: { body: json.Marshal(parameter.dingding.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -262,7 +270,7 @@ template: { request: { body: json.Marshal(parameter.lark.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -288,7 +296,7 @@ template: { request: { body: json.Marshal(parameter.lark.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } @@ -306,7 +314,7 @@ template: { request: { body: json.Marshal(parameter.slack.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -332,7 +340,7 @@ template: { request: { body: json.Marshal(parameter.slack.message) header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } diff --git a/vela-templates/definitions/internal/workflowstep/request.cue b/vela-templates/definitions/internal/workflowstep/request.cue index 6d0aaa7b9..380e0986b 100644 --- a/vela-templates/definitions/internal/workflowstep/request.cue +++ b/vela-templates/definitions/internal/workflowstep/request.cue @@ -27,6 +27,9 @@ template: { if parameter.header != _|_ { header: parameter.header } + if parameter.timeout != _|_ { + timeout: parameter.timeout + } } } } @@ -51,5 +54,7 @@ template: { method: *"GET" | "POST" | "PUT" | "DELETE" body?: {...} header?: [string]: string + // +usage=The timeout of this request (Go duration string, e.g. "30s", "2m", "500ms"). Defaults to 3s when omitted. Invalid values fail when the step runs. + timeout?: string & =~"^(0|(([0-9]+(\\.[0-9]*)?|\\.[0-9]+)(ns|us|µs|μs|ms|s|m|h))+)$" } } diff --git a/vela-templates/definitions/internal/workflowstep/webhook.cue b/vela-templates/definitions/internal/workflowstep/webhook.cue index 4db3b4831..204c01780 100644 --- a/vela-templates/definitions/internal/workflowstep/webhook.cue +++ b/vela-templates/definitions/internal/workflowstep/webhook.cue @@ -35,6 +35,11 @@ template: { value: json.Marshal(parameter.data) } } + httpRequestOpts: { + if parameter.timeout != _|_ { + timeout: parameter.timeout + } + } webhook: { if parameter.url.value != _|_ { req: http.#HTTPDo & { @@ -44,7 +49,7 @@ template: { request: { body: data.value header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -70,7 +75,7 @@ template: { request: { body: data.value header: "Content-Type": "application/json" - } + } & httpRequestOpts } } } @@ -90,5 +95,7 @@ template: { }) // +usage=Specify the data you want to send data?: {...} + // +usage=The timeout of this request (Go duration string, e.g. "30s", "2m", "500ms"). Defaults to 3s when omitted. Invalid values fail when the step runs. + timeout?: string & =~"^(0|(([0-9]+(\\.[0-9]*)?|\\.[0-9]+)(ns|us|µs|μs|ms|s|m|h))+)$" } }