Allowing empty valued global variables in config file

[PereBueno]

Fixes: #2658

Exporting empty variables as an empty string seems not possible. There's a explicit check to avoid it and removing it would result in lots of empty configurations being exported, which is not desired.

Also, export works recursively, so any line being exported is not aware of the existence of any sibling, meaning in

- key: "FOO"
  value: ""

key and value lines are not aware of each other.

Having these, this PR will allow reading a config file that contains variables without a value (as they are exported)

jenkins:
  globalNodeProperties:
  - envVars:
      env:
      - key: VARIABLE1
        value: foo
      - key: EMPTY_VAR

will be accepted, Jenkins will start normally and variable will be visible

Modified integration tests accordingly.

Your checklist for this pull request

🚨 Please review the guidelines for contributing to this repository.

• Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or in Jenkins JIRA
• Link to relevant pull requests, esp. upstream and downstream changes
• Did you provide a test-case? That demonstrates feature works or fixes the issue.

[timja]

Is this needed?

[PereBueno]

Yes, if not using it env is not exported

- \"envVars\"
- toolLocation:    
  - locations:\n    
    - home: "/home/user/bin/git"
    [....]

If I'm not wrong it's because we're extending BaseConfigurator which will take EnvVars as an scalar, while before this configurator it was read as a HeteroDescribableConfigurator.

[timja]

Would extending HeteroDescribableConfigurator work instead?

[PereBueno]

I didn't test that, seems more complex than this approach.
I can do a test and check if we would be simplifying things

[timja]

worth a quick go

[PereBueno]

Sorry, my fault, not Hetero, but DataBoundConfigurator.
Just changed it to use it, slightly simpler.

[timja]

Looks, much simpler and more like what I expected, thanks for sorting.

[timja]

Thanks!

[timja]

Reverted in #2663

See #2662

[jglick]

IMO this is the wrong fix and it is

configuration-as-code-plugin/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java

Lines 660 to 662 in 148f87d

	if (value == null || value.length() == 0) {
	return null;
	}

which should be corrected. If a field is set to the empty string (rather than null), we should export

thatField: ''

regardless of where it is. If some config class neglects to call Util.fixEmpty (or fixEmptyAndTrim etc.) when processing a parameter to a @DataBoundSetter which cannot logically be the empty string, then fix that setter.

[fcojfernandez]

that might have an huge impact on the export. Any single property with an empty String as value will be exported when nowadays is not

If some config class neglects to call Util.fixEmpty (or fixEmptyAndTrim etc.) when processing a parameter to a @DataBoundSetter which cannot logically be the empty string, then fix that setter.

which might mean to fix how many plugins? Although I agree with that being the 100% correct solution, that might be little realistic as users using the export nowadays will see unexpected changes in the export without a (from the user perspective) good reason

[jglick]

which might mean to fix how many plugins?

Not sure, but they need to be fixed anyway even without JCasC.