Skip to content

fix(tasks): version bump for helm charts #7419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: stable
Choose a base branch
from
+21 −16
Open

fix(tasks): version bump for helm charts #7419

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 21 additions & 16 deletions tasks/release.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,11 @@ def update_helm_chart(context: Context, chart_repo: str | None = "helm/") -> Non
"""Update helm/Chart.yaml with the current version from pyproject.toml."""
print(" - [release] Update Helm chart")

# Import here to not require installing packaging when running invoke without installing dependencies.
from packaging.version import Version

# Get the app version directly from pyproject.toml
app_version = get_version_from_pyproject() # Returns a string like '1.1.0a1'
app_version = Version(get_version_from_pyproject()) # Returns a string like '1.1.0a1'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add exception handling for malformed version strings.

If get_version_from_pyproject() returns a non-PEP 440 compliant version string, Version() will raise InvalidVersion, crashing the task. Consider wrapping in a try-except block to provide a clearer error message.

Apply this diff to add error handling:

-    app_version = Version(get_version_from_pyproject())  # Returns a string like '1.1.0a1'
+    try:
+        app_version = Version(get_version_from_pyproject())  # Returns a string like '1.1.0a1'
+    except Exception as e:
+        raise ValueError(f"Invalid version in pyproject.toml: {e}") from e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
app_version = Version(get_version_from_pyproject()) # Returns a string like '1.1.0a1'
try:
app_version = Version(get_version_from_pyproject()) # Returns a string like '1.1.0a1'
except Exception as e:
raise ValueError(f"Invalid version in pyproject.toml: {e}") from e
🤖 Prompt for AI Agents 
In tasks/release.py around line 107, the call creating
Version(get_version_from_pyproject()) can raise packaging.version.InvalidVersion
for malformed/non-PEP440 strings; wrap that call in a try-except that catches
InvalidVersion, include the offending version string in a clear error message,
and exit or re-raise a more descriptive exception so the task fails with
actionable output; also ensure packaging.version.InvalidVersion is imported or
referenced correctly.


for chart in ["infrahub", "infrahub-enterprise"]:
# Initialize YAML and load the Chart.yaml file
Expand All @@ -112,43 +115,45 @@ def update_helm_chart(context: Context, chart_repo: str | None = "helm/") -> Non
if "appVersion" not in chart_yaml:
raise ValueError(f"appVersion not found in {str(chart_path)}; no updates made.")

old_app_version = chart_yaml.get("appVersion", "")
old_app_version = Version(chart_yaml.get("appVersion", ""))
if old_app_version == app_version:
print(
f"{str(chart_path)} updates not required, `appVersion` of {old_app_version} matches current from `pyproject.toml`"
)
return
Comment on lines +118 to 123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify early return doesn't skip remaining charts.

Line 123 uses return within a loop over multiple charts. If the first chart doesn't need updating, the function exits without processing remaining charts. Verify this is intentional, or consider using continue instead.

Run the following script to check how charts are processed:

🏁 Script executed:

#!/bin/bash
# Description: Verify behavior when first chart doesn't need updates

# Check the loop structure and return/continue usage
ast-grep --pattern $'for chart in $$$:
  $$$
  if $COND:
    $$$
    return
  $$$'

Length of output: 0

🏁 Script executed:

#!/bin/bash
sed -n '90,140p' tasks/release.py

Length of output: 2231

Replace return with continue to process all charts. The return inside the loop at tasks/release.py (around line 123) exits the function after the first matching chart, skipping the rest; use continue instead.

🤖 Prompt for AI Agents 
In tasks/release.py around lines 118 to 123, the loop currently uses return when
a chart's appVersion already matches the pyproject version, which exits the
entire function and skips remaining charts; change that return to continue so
the loop proceeds to the next chart and all charts are processed. Ensure no
other control flow relies on the function exiting at that point and preserve the
existing print message before continuing.


# Handle Helm chart version increment
old_helm_version = chart_yaml.get("version", "")
old_helm_version = Version(chart_yaml.get("version", ""))
if not old_helm_version:
raise ValueError(f"Helm chart `version` not found in {str(chart_path)}; no updates made.")

# Split the Helm chart version into components for increment logic
major, minor, patch = map(int, old_helm_version.split("."))
new_helm_version = f"{major}.{minor}.{patch}"
new_helm_version = old_helm_version

# Determine the appropriate increment
try:
if app_version > old_app_version:
if int(app_version.split(".")[0]) > int(old_app_version.split(".")[0]):
new_helm_version = f"{major + 1}.0.0"
elif int(app_version.split(".")[1]) > int(old_app_version.split(".")[1]):
new_helm_version = f"{major}.{minor + 1}.0"
elif int(app_version.split(".")[2].split("a")[0]) > int(
old_app_version.split(".")[2].split("a")[0]
): # For alpha, beta handling
new_helm_version = f"{major}.{minor}.{patch + 1}"
if not app_version.is_prerelease and app_version > old_app_version:
if app_version.major > old_app_version.major:
new_helm_version = Version(f"{new_helm_version.major + 1}.0.0")
elif app_version.minor > old_app_version.minor:
new_helm_version = Version(f"{new_helm_version.major}.{new_helm_version.minor + 1}.0")
elif app_version.micro > old_app_version.micro:
new_helm_version = Version(
f"{new_helm_version.major}.{new_helm_version.minor}.{new_helm_version.micro + 1}"
)
except Exception:
# Fallback in case app_version has non-standard format for Helm comparison
print(f"Warning: Unable to strictly compare versions, using default Helm chart version: {new_helm_version}")

# Convert Version to str before passing to yaml
app_version = str(app_version)
new_helm_version = str(new_helm_version)

# Update the YAML
chart_yaml["appVersion"] = app_version
chart_yaml["version"] = new_helm_version

if chart == "infrahub":
dependency_version = new_helm_version
dependency_version = str(new_helm_version)

yaml_values: YAML = init_yaml_obj()
values_path = Path(chart_repo) / "charts" / Path(chart) / "values.yaml"
Expand Down