chore(ci): Ensure null http override values aren't set via tests

[elizabethhealy]

The values shouldnt be null they should be unset

Summary by CodeRabbit

• Bug Fixes
  • Improved HTTP server configuration output to omit unset timeout fields instead of displaying them as null values, resulting in cleaner configuration rendering.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b777ed9d-a387-4e71-b5b3-fba9e909f3d7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• ✅ Review completed - (🔄 Check again to review again)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dspx-2813-ensure-http-override-vals-not-present

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request updates the HTTP server option tests to assert that 'idleTimeout' and 'readHeaderTimeout' are absent from the configuration. However, feedback indicates that these assertions will fail because the current Helm template renders null values explicitly as 'key: null'. A suggestion was provided to revert the assertions to maintain compatibility with the existing template logic.

[gemini-code-assist]

These assertions will fail with the current Helm template for config.yaml. The template at charts/platform/templates/config.yaml on line 82 uses toYaml on the entire .Values.server.http object. Since idleTimeout and readHeaderTimeout have null as their default value in values.yaml, they will be rendered into the config as idleTimeout: null and readHeaderTimeout: null, causing these NotContains assertions to fail.

To fix this, the template should be updated to filter out null values. If changing the template is not intended in this PR, these assertions should be reverted to check for the presence of key: null.

Suggested change

	s.NotContains(data, "idleTimeout", "server.http.idleTimeout should not be present")
	s.NotContains(data, "readHeaderTimeout", "server.http.readHeaderTimeout should not be present")
	s.Require().Contains(data, "idleTimeout: null", "idleTimeout should be null in the config file")
	s.Require().Contains(data, "readHeaderTimeout: null", "readHeaderTimeout should be null in the config file")

[elizabethhealy]

/gemini you said "To fix this, the template should be updated to filter out null values." -- does this really matter in the context of yaml? whether its null or not present in the map?

[gemini-code-assist]

In many YAML parsers, key: null and the key being absent are functionally equivalent. However, in the context of Go applications (which often use gopkg.in/yaml.v3 or similar), unmarshaling key: null into a struct field can result in the field being set to its zero value (e.g., 0 for an int or time.Duration), whereas omitting the key allows the application to retain its default value or handle the absence explicitly. If your application logic relies on checking if a value was provided (e.g., using a pointer type *time.Duration to distinguish between 'unset' and 'zero'), having key: null in the config can cause unexpected behavior by overriding defaults with zero values. Therefore, omitting the key is generally cleaner and safer for configuration management.

[elizabethhealy]

closing - not necessary as null and empty are evaled the same in the platform code