Skip to content

Conversation

@FranciscoKurpiel
Copy link
Collaborator

This commit introduces a new mechanism to allow xtypes to signal that a provided value (such as an empty string) should be treated as if no value was provided at all. This solves a design limitation where the validation logic could not distinguish between a valid empty value and a "not provided" marker for certain types.

Problem:

Previously, an xtype like xtypes.RSAPrivateKey would fail validation when an optional parameter was backed by an environment variable set to an empty string.

  • The validation function (ValueValid) is context-agnostic and does not know if a parameter is optional.
  • If ValueValid returned an error for an empty string, it would incorrectly fail optional parameters.
  • If ValueValid did not return an error, it would incorrectly allow a required parameter to be considered valid while producing a nil value, without MustParse returning an error.

Solution:

This commit implements a sentinel error-based solution:

  1. types.ErrNoValue: A new exported error, ErrNoValue, has been added to the types package.

  2. xtype Adoption: xtypes that need this behavior (like RSAPrivateKey) can now return types.ErrNoValue from their ValueValid function when they encounter a value (like an empty string) that should be treated as "not provided".

  3. Core Logic Update: The central validation function validValue in parsed.go has been updated to recognize ErrNoValue. When it receives this error, it now correctly applies the optional/required logic as if the parameter had not been supplied at all.

Consequences:

  • For Users: This change fixes the bug where optional RSAPrivateKey parameters would fail with empty environment variables. The behavior is now consistent and correct. The default value of an optional parameter is now correctly used when an empty string is provided.
  • For xtype Developers: Developers of new xtypes now have a clear, idiomatic pattern to handle cases where certain string representations should be considered "empty" or "not provided". They can return types.ErrNoValue from ValueValid to trigger this behavior.
  • Testing: A comprehensive test case (TestRSAPrivateKey) has been added to parser_test.go to verify this new behavior and prevent future regressions.

This change makes the validation system more robust and flexible while maintaining safety and correctness.

@FranciscoKurpiel FranciscoKurpiel self-assigned this Sep 20, 2025
@FranciscoKurpiel FranciscoKurpiel added the enhancement New feature or request label Sep 20, 2025
@gemini-code-assist

This comment was marked as resolved.

gemini-code-assist[bot]

This comment was marked as resolved.

@FranciscoKurpiel FranciscoKurpiel force-pushed the cfgenv-optional-empty-value branch 2 times, most recently from 7e75fc5 to 6bb8c4d Compare September 20, 2025 14:58
This commit introduces a new mechanism to allow xtypes to signal that a
provided value (such as an empty string) should be treated as if no value
was provided at all. This solves a design limitation where the validation
logic could not distinguish between a valid empty value and a "not provided"
marker for certain types.

Problem:

Previously, an xtype like `xtypes.RSAPrivateKey` would fail validation
when an optional parameter was backed by an environment variable set to an
empty string.

- The validation function (`ValueValid`) is context-agnostic and does not
  know if a parameter is optional.
- If `ValueValid` returned an error for an empty string, it would
  incorrectly fail optional parameters.
- If `ValueValid` did *not* return an error, it would incorrectly allow a
  required parameter to be considered valid while producing a `nil` value,
  without `MustParse` returning an error.

Solution:

This commit implements a sentinel error-based solution:

1.  **`types.ErrNoValue`**: A new exported error, `ErrNoValue`, has been
    added to the `types` package.

2.  **xtype Adoption**: xtypes that need this behavior (like `RSAPrivateKey`)
    can now return `types.ErrNoValue` from their `ValueValid` function when
    they encounter a value (like an empty string) that should be treated as
    "not provided".

3.  **Core Logic Update**: The central validation function `validValue` in
    `parsed.go` has been updated to recognize `ErrNoValue`. When it
    receives this error, it now correctly applies the `optional`/`required`
    logic as if the parameter had not been supplied at all.

Consequences:

- **For Users:** This change fixes the bug where optional `RSAPrivateKey`
  parameters would fail with empty environment variables. The behavior is
  now consistent and correct. The default value of an optional parameter is
  now correctly used when an empty string is provided.
- **For xtype Developers:** Developers of new xtypes now have a clear,
  idiomatic pattern to handle cases where certain string representations
  should be considered "empty" or "not provided". They can return
  `types.ErrNoValue` from `ValueValid` to trigger this behavior.
- **Testing:** A comprehensive test case (`TestRSAPrivateKey`) has been
  added to `parser_test.go` to verify this new behavior and prevent future
  regressions.

This change makes the validation system more robust and flexible while
maintaining safety and correctness.
@FranciscoKurpiel FranciscoKurpiel force-pushed the cfgenv-optional-empty-value branch from 6bb8c4d to 8a1b1e7 Compare September 20, 2025 15:12
@FranciscoKurpiel
Copy link
Collaborator Author

/gemini review

gemini-code-assist[bot]

This comment was marked as resolved.

@FranciscoKurpiel FranciscoKurpiel merged commit a3a9995 into master Sep 20, 2025
5 checks passed
@FranciscoKurpiel FranciscoKurpiel deleted the cfgenv-optional-empty-value branch September 20, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

2 participants