Skip to content

Conversation

@AntonioCeppellini
Copy link

Fixes #15024

Motivation

The field http.timeoutSeconds only accepts int values.
When a templated parameter is used (ex. {{= asInt(...) }}), unmarshalling fails with a type error.
This prevents users from templating timeoutSeconds in WorkflowTemplates and Workflows.

Modifications

  1. Added a custom UnmarshalJSON implementation for HTTP in http_types.go.
  2. The custom UnmarshalJSON accepts numeric and string JSON values (30 and "30")
  3. Added unit tests covering:
  • numeric timeout
  • string timeout
  • empty/null timeout
  • invalid string timeout

Verification

Tested: go test ./pkg/apis/workflow/v1alpha1 -v

Documentation

.features/fix-http-timeoutseconds-templating.yaml

@AntonioCeppellini AntonioCeppellini force-pushed the fix-http-timeoutseconds-templating branch 2 times, most recently from a6205e5 to 423658c Compare November 19, 2025 17:42
scottmelhop and others added 2 commits November 19, 2025 18:52
Signed-off-by: Scott Melhop <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Co-authored-by: Alan Clucas <[email protected]>
Signed-off-by: Antonio Ceppellini <[email protected]>
@AntonioCeppellini AntonioCeppellini force-pushed the fix-http-timeoutseconds-templating branch from 450432e to ecbf7df Compare November 19, 2025 17:54
Copy link
Member

@Joibel Joibel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty sure that with the full CRDs installed this won't work, you'll still be restricted to an int in that field. You'd be able to see this in the test environment in codespaces/devcontainer or by writing an e2e test in test/e2e. You do have to change it to intOrString and run make codegen to change the CRDs.

@AntonioCeppellini AntonioCeppellini marked this pull request as draft November 21, 2025 21:24
@AntonioCeppellini
Copy link
Author

I am pretty sure that with the full CRDs installed this won't work, you'll still be restricted to an int in that field. You'd be able to see this in the test environment in codespaces/devcontainer or by writing an e2e test in test/e2e. You do have to change it to intOrString and run make codegen to change the CRDs.

I changed it following your suggestion and then i tried run make codeged but it fails giving me the following error:

INFO [runner] linters took 2.118984129s with stages: goanalysis_metalinter: 2.118566947s 
version.go:7:7: could not import github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1 (-: # github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1
pkg/apis/workflow/v1alpha1/generated.pb.go:8457:45: cannot convert *m.TimeoutSeconds (variable of struct type intstr.IntOrString) to type uint64
pkg/apis/workflow/v1alpha1/generated.pb.go:15591:32: cannot convert *m.TimeoutSeconds (variable of struct type intstr.IntOrString) to type uint64
pkg/apis/workflow/v1alpha1/generated.pb.go:30080:23: cannot use &v (value of type *int64) as *intstr.IntOrString value in assignment
pkg/apis/workflow/v1alpha1/zz_generated.deepcopy.go:1556:10: cannot use new(int64) (value of type *int64) as *intstr.IntOrString value in assignment) (typecheck)
	wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
	     ^
1 issues:
* typecheck: 1
INFO File cache stats: 1 entries of total size 1.8KiB 
INFO Memory: 42 samples, avg is 174.5MB, max is 535.2MB 
INFO Execution took 4.060364824s                  
make: *** [Makefile:571: lint-go] Error 1

i'm quite blocked, i think that i should touch something at codegen level but i'm not sure about it.

generated file still treat timeoutSeconds as an int (i think that's normal if the codegen fails)

@Joibel
Copy link
Member

Joibel commented Nov 25, 2025

I am pretty sure that with the full CRDs installed this won't work, you'll still be restricted to an int in that field. You'd be able to see this in the test environment in codespaces/devcontainer or by writing an e2e test in test/e2e. You do have to change it to intOrString and run make codegen to change the CRDs.

I changed it following your suggestion and then i tried run make codeged but it fails giving me the following error:

Try removing the words lint-go from

pkg/apis/workflow/v1alpha1/generated.proto: $(TOOL_GO_TO_PROTOBUF) $(PROTO_BINARIES) $(TYPES) $(GOPATH)/src/github.com/gogo/protobuf lint-go
and try again. Also rm -rf vendor if you get stuck running this step.

@AntonioCeppellini
Copy link
Author

I am pretty sure that with the full CRDs installed this won't work, you'll still be restricted to an int in that field. You'd be able to see this in the test environment in codespaces/devcontainer or by writing an e2e test in test/e2e. You do have to change it to intOrString and run make codegen to change the CRDs.

I changed it following your suggestion and then i tried run make codeged but it fails giving me the following error:

Try removing the words lint-go from

pkg/apis/workflow/v1alpha1/generated.proto: $(TOOL_GO_TO_PROTOBUF) $(PROTO_BINARIES) $(TYPES) $(GOPATH)/src/github.com/gogo/protobuf lint-go

and try again. Also rm -rf vendor if you get stuck running this step.

Thank you very much! :D

Right now i'm facing a new problem when make codegen :

go install github.com/go-swagger/go-swagger/cmd/[email protected]
# recurl will only fetch if the file doesn't exist, so delete it
rm -f dist/kubernetes.swagger.json
./hack/recurl.sh dist/kubernetes.swagger.json https://raw.githubusercontent.com/kubernetes/kubernetes/v1.33.1/api/openapi-spec/swagger.json
+ file=dist/kubernetes.swagger.json
+ url=https://raw.githubusercontent.com/kubernetes/kubernetes/v1.33.1/api/openapi-spec/swagger.json
+ '[' '!' -f dist/kubernetes.swagger.json ']'
+ curl -L --fail -o dist/kubernetes.swagger.json -- https://raw.githubusercontent.com/kubernetes/kubernetes/v1.33.1/api/openapi-spec/swagger.json
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 3623k  100 3623k    0     0  1713k      0  0:00:02  0:00:02 --:--:-- 1714k
+ chmod +x dist/kubernetes.swagger.json
rm -Rf v3 vendor
# We have `hack/api/swagger` so that most hack script do not depend on the whole code base and are therefore slow.
go run ./hack/api/swagger secondaryswaggergen
# github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1
pkg/apis/workflow/v1alpha1/zz_generated.deepcopy.go:1556:10: cannot use new(int64) (value of type *int64) as *intstr.IntOrString value in assignment
make: *** [Makefile:783: pkg/apiclient/_.secondary.swagger.json] Error 1

i followed your instructions, removed lint-go and also vendor (i think that vendor is also removed during make codegen)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTTP Template: Can't use parameter for timeoutSeconds

3 participants