Fix ace-install openshift detection

[ArnobKumarSaha]

No description provided.

[Copilot]

Pull request overview

This PR attempts to fix helper template logic in Helm charts by adding support for global distro configuration and modifying nil-safety handling. However, the changes introduce critical bugs that will cause template rendering failures.

Key changes:

• Modified operator.ubi template in ace chart to support both global and local distro.ubi values with fallback logic
• Removed default wrapper in ace-installer's distro.openshift template when handling .Values.options

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
charts/ace/templates/_helpers.tpl	Adds complex nested logic to support global.distro.ubi with fallback to local distro.ubi, but introduces bugs in the default function parameter order and readability issues
charts/ace-installer/templates/_helpers.tpl	Removes nil-safety wrapper around .Values.options, which will cause template failures when the value is undefined

---

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

[Copilot]

Removing the default wrapper will cause this template to fail when .Values.options is nil or undefined. The dict function requires its argument to be non-nil, but if .Values.options is not set, this will result in an error like "error calling dict: dict requires an even number of arguments". The original (default dict .Values.options) pattern was correct as it ensures a valid dict is passed to the dig function even when .Values.options is not defined.

Suggested change

	{{- or (.Capabilities.APIVersions.Has "project.openshift.io/v1/Project") (dig "distro" "openshift" false (dict .Values.options)) -}}
	{{- or (.Capabilities.APIVersions.Has "project.openshift.io/v1/Project") (dig "distro" "openshift" false (default dict .Values.options)) -}}

[Copilot]

The logic in this expression appears incorrect. The default function takes two arguments: the default value and the value to check. However, this expression has (default (dig "ubi" "" (default dict .Values.distro)) .Values.global.distro.ubi) which will use .Values.global.distro.ubi as the value to check and fall back to the dig result if it's empty. This seems backwards - it should likely be (default .Values.global.distro.ubi (dig "ubi" "" (default dict .Values.distro))) to prioritize the global value and fall back to the local value. Additionally, the has function expects a simple value, but this complex expression may not work as intended with the list membership check.

Suggested change

	{{ ternary "-ubi" "" (list "operator" "all" | has (default (dig "ubi" "" (default dict .Values.distro)) .Values.global.distro.ubi)) }}
	{{ ternary "-ubi" "" (list "operator" "all" | has (default .Values.global.distro.ubi (dig "ubi" "" (default dict .Values.distro)))) }}

[Copilot]

This expression has become very complex with deeply nested function calls that are difficult to read and maintain. Consider breaking this down into multiple steps or using intermediate variables to improve readability. For example, the pattern used in line 216 for distro.openshift which uses or with multiple conditions is more readable: {{- or .Values.global.distro.ubi (and .Values.distro .Values.distro.ubi) -}}

Suggested change

	{{ ternary "-ubi" "" (list "operator" "all" | has (default (dig "ubi" "" (default dict .Values.distro)) .Values.global.distro.ubi)) }}
	{{- $distro := default dict .Values.distro -}}
	{{- $localUbi := dig "ubi" "" $distro -}}
	{{- $ubiValue := default $localUbi .Values.global.distro.ubi -}}
	{{- $useUbi := list "operator" "all" | has $ubiValue -}}
	{{- ternary "-ubi" "" $useUbi -}}