Merge pull request #2229 from simonbaird/support-no-timeout

Support running with no timeout, ignore/deprecate TIMEOUT task param

cmd/root/root_cmd.go

@@ -73,11 +73,18 @@ func NewRootCmd() *cobra.Command {
 			// set a custom message for context.DeadlineExceeded error
 			context.DeadlineExceeded = customDeadlineExceededError{}
 
-			// Create a new context now that flags have been parsed so a custom timeout can be used.
-			ctx, cancel := context.WithTimeout(cmd.Context(), globalTimeout)
+			// Create a new context now that flags have been parsed so a
+			// custom timeout can be used and traces can be added
+			ctx := cmd.Context()
+			var cancel context.CancelFunc
+			if globalTimeout > 0 {
+				ctx, cancel = context.WithTimeout(ctx, globalTimeout)
+				log.Debugf("globalTimeout is %s", time.Duration(globalTimeout))
+			} else {
+				log.Debugf("globalTimeout is %d, no timeout used", globalTimeout)
+			}
 			ctx = tracing.WithTrace(ctx, enabledTraces)
 			cmd.SetContext(ctx)
-			log.Debugf("globalTimeout is %d", globalTimeout)
 
 			var cpuprofile *os.File
 			var tracefile *os.File

docs/modules/ROOT/pages/verify-enterprise-contract.adoc

@@ -64,9 +64,8 @@ paths can be provided by using the `:` separator.
 +
 *Default*: `now`
 *EXTRA_RULE_DATA* (`string`):: Merge additional Rego variables into the policy data. Use syntax "key=value,key2=value2..."
-*TIMEOUT* (`string`):: Timeout setting for `ec validate`.
-+
-*Default*: `5m0s`
+*TIMEOUT* (`string`):: This param is deprecated and will be removed in future. Its value is ignored. EC will be run without a timeout. (If you do want to apply a timeout use the Tekton task timeout.)
+
 *WORKERS* (`string`):: Number of parallel workers to use for policy evaluation.
 +
 *Default*: `1`

features/task_validate_image.feature

@@ -298,7 +298,9 @@ Feature: Verify Enterprise Contract Tekton Tasks
     Then the task should succeed
       And the task logs for step "debug-log" should contain "Using provided effective time 2020-01-01T00:00:00Z"
 
-  Scenario: Timeout is honored
+  # Previously we did allow a custom timeout to be set via the TIMEOUT param, but now it's ignored.
+  # (This test could be removed in the future, but let's keep it for now I guess.)
+  Scenario: Deprecated timeout param is ignored
     Given a working namespace
       And a key pair named "known"
       And an image named "acceptance/timeout"
@@ -314,7 +316,7 @@ Feature: Verify Enterprise Contract Tekton Tasks
       | IGNORE_REKOR         | true                                                                   |
       | TIMEOUT              | 666s                                                                   |
     Then the task should succeed
-      And the task logs for step "debug-log" should contain "globalTimeout is 666000000000"
+      And the task logs for step "debug-log" should contain "globalTimeout is 100h0m0s"
 
   Scenario: SSL_CERT_DIR environment variable is customized
     Given a working namespace

tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml

@@ -127,8 +127,10 @@ spec:
 
     - name: TIMEOUT
       type: string
-      description: Timeout setting for `ec validate`.
-      default: "5m0s"
+      description: >
+        This param is deprecated and will be removed in future. Its value is ignored. EC will
+        be run without a timeout. (If you do want to apply a timeout use the Tekton task timeout.)
+      default: ""
 
     - name: WORKERS
       type: string
@@ -224,7 +226,11 @@ spec:
         - "$(params.WORKERS)"
         # NOTE: The syntax below is required to negate boolean parameters
         - "--info=$(params.INFO)"
-        - "--timeout=$(params.TIMEOUT)"
+        # Fresh versions of ec support "--timeout=0" to indicate no timeout, but this would break
+        # the task if it's used with an older version of ec. In an abundance of caution, let's set
+        # an arbitrary high value instead of using 0 here. In future we can change it to 0.
+        # (The reason to not use an explicit timeout for ec is so Tekton can handle the timeouts).
+        - "--timeout=100h"
         - "--strict=false"
         - "--show-successes"
         - "--effective-time=$(params.EFFECTIVE_TIME)"