fix(t8s-cluster/management-cluster): leave out protocol if nil

[cwrau]

I ran the old code, no idea why it fails with flux?

Summary by CodeRabbit

• Infrastructure
  • Improved security group rule protocol configuration handling to ensure consistent and reliable application of default values in cluster settings.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[gemini-code-assist]

Summary of Changes

Hello @cwrau, 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 refines the Helm template logic for OpenStack security group rules to handle the protocol field more robustly. The modification ensures that the protocol field is only included in the generated configuration when it has a defined, non-nil value, or defaults to 'tcp' if completely unspecified. This change is intended to resolve compatibility issues observed with Flux CD, which may not correctly process explicitly set nil protocol values.

Highlights

• Conditional Protocol Setting: The logic for setting the protocol field in OpenStack security group rules has been updated. The field will now only be explicitly set if it holds a non-nil value, preventing empty or null protocol entries.
• Flux CD Compatibility Fix: This change addresses an issue where explicitly setting a nil or empty protocol value was causing failures when used with Flux CD, by ensuring the field is omitted in such cases.
• Default Protocol Behavior: If the protocol key is entirely absent from the security group rule definition, it will continue to default to tcp.

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.

Footnotes

1. 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. ↩

[coderabbitai]

Walkthrough

A Helm template file is refactored to replace an inline ternary operator with an explicit conditional block for OpenStack protocol defaulting logic, improving clarity of the default value assignment when the protocol key is absent.

Changes

Cohort / File(s)	Change Summary
OpenStack Cluster Template Configuration charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/_openStackClusterTemplateSpec.yaml	Refactored protocol defaulting from inline ternary operator to explicit conditional block; default value of "tcp" now applied via conditional check rather than ternary expression

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

• Single file modification with straightforward logic restructuring
• No functional behavior change, only control flow reorganization
• Low syntactic complexity in templating logic

Poem

🐰 A protocol finds its home,
With clarity now shown—
No ternary tricks to confuse,
Just "tcp" when you need to choose!
hops contentedly 🎩✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: modifying protocol defaulting behavior to omit it when nil, which matches the changeset's core objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/t8s-cluster/leave-out-protocol

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3deff65 and 6bb7072.

📒 Files selected for processing (1)

• charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/_openStackClusterTemplateSpec.yaml (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Agent
• GitHub Check: lint helm chart (t8s-cluster)

🔇 Additional comments (1)

charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/_openStackClusterTemplateSpec.yaml (1)

111-115: Correctly omits protocol field when nil. ✓

The refactored conditional logic properly handles three cases: truthy protocol values (lines 111–112), absent protocol keys (lines 113–114), and nil protocol values (implicitly omitted). This aligns with the PR objective and resolves the Flux failure by preventing invalid nil fields in the rendered YAML output.

The logic is consistent with the codebase's hasKey pattern (e.g., lines 88, 91, 95, 120–125).

---

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 fixes an issue where a protocol key with a nil value was being rendered in the output YAML. The new logic correctly omits the protocol key if its value is nil, provides a default of tcp if the key is not present, and uses the specified value otherwise. The change is correct and addresses the problem. I've added one comment with a suggestion to refactor the logic for improved readability.

[Copilot]

Pull request overview

This PR modifies the OpenStack cluster template to change how the protocol field is handled in security group rules. Previously, the protocol field was always set (even when the value was nil), but now it is conditionally omitted when explicitly set to nil. This change is necessary for Flux compatibility and affects native routing rules where protocol should not be specified.

• Replaced inline protocol field assignment with conditional logic
• Protocol field is now omitted when value is nil (instead of being set to nil)
• Maintains backward compatibility for cases where protocol is not specified (defaults to "tcp") or has an explicit value

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.