Allow using empty strings as job parameters

[kanterov]

Changes

Allow using an empty string as the "default" value for job parameters, otherwise, such jobs are rejected because "default" field is required.

Remove "omitempty" modifier because both fields are required.

Tests

Verified manually with Terraform

• make test run locally
• relevant change in docs/ folder
• covered with integration tests in internal/acceptance
• relevant acceptance tests are passing
• using Go SDK

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (acc1cea) 84.29% compared to head (c5d5983) 83.14%.
Report is 23 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3158      +/-   ##
==========================================
- Coverage   84.29%   83.14%   -1.15%     
==========================================
  Files         162      168       +6     
  Lines       14405    14811     +406     
==========================================
+ Hits        12143    12315     +172     
- Misses       1565     1771     +206     
- Partials      697      725      +28     

Files	Coverage Δ
jobs/resource_job.go	88.51% <ø> (ø)

... and 27 files with indirect coverage changes

[pietern]

@kanterov @gaborratky-db Isn't it valid to define a job where some job parameters are required? I.e. you have to specify the parameters when running it? If so, then this approach does not allow that use case.

I believe this fix needs to use the ForceSendFields feature to mark the default as set or not. That way it is possible to both define parameters with no default (such that specifying them for a run is mandatory) as well as parameters with the default being an empty string.

[kanterov]

@pietern, we don't really have 'required' job parameters in this case. But, the 'default' field in the API is required. If it's set to an empty string, it gets skipped in the API request because of the 'omitempty' thing, and that's causing a 400 error. So, we've got to make sure we include this 'default' field every time, even if it's just an empty string.

[pietern]

@kanterov Please add a unit test to confirm that empty strings are now specified. There are two cases to consider; a user explicitly passes an empty string (good), and a user doesn't specify the default at all, which would default to an empty string. In the latter case, we could see configuration drift on the TF side.

[pietern]

@kanterov This is a map, so only one of these will end up on the backend.

Does that impact the validity of the integration test?

[kanterov]

Oh... my bad, we need to fix it. It shouldn't matter because we don't run it, and I've tried empty value manually. But let's fix the test.