Fix unsupported_field! macro to properly handle recursive cases #121

Eeshu-Yadav · 2025-10-21T16:51:24Z

Fixed macro to return Result::<(), GenericError>::Ok(()) correctly
Recursive case now properly propagates return value
Changed behavior from hard error to graceful warning
Unsupported fields like alt_stat_name are now ignored with warnings

This resolves issue #41 where alt_stat_name caused config parsing to fail. The macro now logs a warning and continues parsing instead of crashing.

kmesh-bot · 2025-10-21T16:51:32Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hzxuzhonghu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gemini-code-assist · 2025-10-21T16:51:38Z

Summary of Changes

Hello @Eeshu-Yadav, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of configuration parsing by modifying the unsupported_field! macro. Instead of halting execution with an error when an unrecognized field is encountered, the system will now issue a warning and gracefully continue processing the configuration. This ensures that minor discrepancies in configuration files do not lead to critical failures, improving the overall resilience of the application.

Highlights

Macro Behavior Change: The unsupported_field! macro now logs a warning using tracing::warn! when an unsupported field is encountered, instead of returning a hard error (Err(GenericError::UnsupportedField)).
Recursive Case Handling: The recursive variant of the unsupported_field! macro has been corrected to properly propagate the return value and ensure all fields in a list are processed, preventing early termination.
Consistent Return Value: The macro now consistently returns Result::<(), GenericError>::Ok(()) in all scenarios, ensuring graceful continuation of configuration parsing even when unsupported fields are present.
Issue Resolution: This change resolves issue failed to connect orion to istiod as a waypoint #41, where specific unsupported fields (e.g., alt_stat_name) would cause configuration parsing to fail, by allowing the parser to ignore them with a warning and continue.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

Copilot

Pull Request Overview

This PR fixes the unsupported_field! macro to handle unsupported configuration fields gracefully by logging warnings instead of causing parsing failures.

Key Changes:

Changed macro behavior from returning errors to logging warnings and continuing execution
Fixed the recursive case to properly propagate the return value from nested macro invocations
Ensured both branches consistently return Result::<(), GenericError>::Ok(())

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-21T16:52:05Z

orion-configuration/src/config/common.rs

+                #[allow(dropping_copy_types, clippy::drop_non_drop)]
+                drop($field);


The drop($field) call in the else branch is redundant. Since the field is not used (as verified by the is_used() check), dropping it here serves no purpose and can be removed for clarity.

Copilot · 2025-10-21T16:52:06Z

orion-configuration/src/config/common.rs

+                drop($field);
            }
-        };
+            unsupported_field!($($tail),+)


The recursive call to unsupported_field! is missing a return statement or value propagation. The result of this recursive call should be returned, otherwise the recursion will expand but its result will be discarded. Consider wrapping this in a return or ensuring the value is properly used.

gemini-code-assist

Code Review

The pull request modifies the unsupported_field! macro in orion-configuration/src/config/common.rs to handle recursive cases properly and change the behavior from a hard error to a graceful warning. The macro now logs a warning and continues parsing instead of crashing when encountering unsupported fields. The changes ensure that the macro returns Result::<(), GenericError>::Ok(()) correctly and propagates the return value in recursive cases. I have identified a potential issue with the macro's recursive call and provided a suggestion to ensure the intended behavior.

gemini-code-assist · 2025-10-21T16:53:00Z

orion-configuration/src/config/common.rs

+                drop($field);
            }
-        };
+            unsupported_field!($($tail),+)


The recursive call unsupported_field!($($tail),+) might not return the Result as intended because the outer block will always return Ok(()). Consider returning the result of the recursive call to propagate any potential errors.

I'm not sure if this is the intended behavior, but it's something to consider.

Suggested change

unsupported_field!($($tail),+)

unsupported_field!($($tail),+)?

- Fixed macro to return Result::<(), GenericError>::Ok(()) correctly - Recursive case now properly propagates return value - Changed behavior from hard error to graceful warning - Unsupported fields like alt_stat_name are now ignored with warnings This resolves issue kmesh-net#41 where alt_stat_name caused config parsing to fail. The macro now logs a warning and continues parsing instead of crashing. Signed-off-by: Eeshu-Yadav <[email protected]>

Eeshu-Yadav · 2025-10-24T14:43:50Z

@dawid-nowak this one also

Eeshu-Yadav · 2025-10-27T11:15:53Z

@hzxuzhonghu this one

Copilot AI review requested due to automatic review settings October 21, 2025 16:51

kmesh-bot requested review from dawid-nowak and hzxuzhonghu October 21, 2025 16:51

kmesh-bot added the size/S label Oct 21, 2025

Copilot AI reviewed Oct 21, 2025

View reviewed changes

gemini-code-assist bot reviewed Oct 21, 2025

View reviewed changes

Eeshu-Yadav force-pushed the fix-issue-41-unsupported-field-macro branch from 739e3a3 to 567476d Compare October 21, 2025 16:58

kmesh-bot added size/M and removed size/S labels Oct 21, 2025

Eeshu-Yadav mentioned this pull request Oct 22, 2025

Implement HTTP RBAC for Waypoint Replacement #119

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix unsupported_field! macro to properly handle recursive cases #121

Fix unsupported_field! macro to properly handle recursive cases #121

Eeshu-Yadav commented Oct 21, 2025

Uh oh!

kmesh-bot commented Oct 21, 2025

Uh oh!

gemini-code-assist bot commented Oct 21, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 21, 2025

Uh oh!

Copilot AI Oct 21, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Oct 21, 2025

Uh oh!

Eeshu-Yadav commented Oct 24, 2025

Uh oh!

Eeshu-Yadav commented Oct 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		#[allow(dropping_copy_types, clippy::drop_non_drop)]
		drop($field);

	unsupported_field!($($tail),+)
	unsupported_field!($($tail),+)?

Fix unsupported_field! macro to properly handle recursive cases #121

Are you sure you want to change the base?

Fix unsupported_field! macro to properly handle recursive cases #121

Conversation

Eeshu-Yadav commented Oct 21, 2025

Uh oh!

kmesh-bot commented Oct 21, 2025

Uh oh!

gemini-code-assist bot commented Oct 21, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

Eeshu-Yadav commented Oct 24, 2025

Uh oh!

Eeshu-Yadav commented Oct 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants