Skip to content

[CLOUDP-352383] Add a build variant to release chart to OCI registry #542

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

Merged
merged 6 commits into from
Oct 24, 2025
Merged

[CLOUDP-352383] Add a build variant to release chart to OCI registry #542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d0ae384
Add a build variant to release chart to OCI registry
viveksinghggits Oct 20, 2025
9459e3c
Run precommit
viveksinghggits Oct 20, 2025
8011687
Early return after logging in to release workflow
viveksinghggits Oct 21, 2025
e6f7e34
Rename `version_id` to `version`
viveksinghggits Oct 23, 2025
f09afa8
Address review comments
viveksinghggits Oct 24, 2025
b863e04
Fix semgrep issues
viveksinghggits Oct 24, 2025
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
3 changes: 3 additions & 0 deletions .evergreen-functions.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ functions:
type: setup
params:
working_dir: src/github.com/mongodb/mongodb-kubernetes
include_expansions_in_env:
- quay_prod_username
- quay_prod_robot_token
add_to_path:
- ${workdir}/bin
- ${PROJECT_DIR}/bin
Expand Down
19 changes: 19 additions & 0 deletions .evergreen-release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,16 @@ tasks:
- func: clone
- func: python_venv
- func: create_chart_release_pr

- name: release_chart_to_oci_registry
tags: [ "release_chart_to_oci_registry" ]
commands:
- func: clone
- func: python_venv
- func: setup_kubectl
- func: setup_aws
- func: helm_registry_login
- func: publish_helm_chart

### Release build variants
buildvariants:
Expand Down Expand Up @@ -255,3 +265,12 @@ buildvariants:
allowed_requesters: [ "patch", "github_tag" ]
tasks:
- name: create_chart_release_pr

- name: release_chart_to_oci_registry
display_name: release_chart_to_oci_registry
tags: [ "release" ]
run_on:
- release-ubuntu2404-small # This is required for CISA attestation https://jira.mongodb.org/browse/DEVPROD-17780
allowed_requesters: [ "patch", "github_tag" ]
tasks:
- name: release_chart_to_oci_registry
48 changes: 46 additions & 2 deletions scripts/release/helm_registry_login.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,18 @@
import os
import subprocess
import sys
from shlex import quote

from lib.base_logger import logger
from scripts.release.build.build_info import load_build_info

BUILD_SCENARIO_RELEASE = "release"
QUAY_USERNAME_ENV_VAR = "quay_prod_username"
QUAY_PASSWORD_ENV_VAR = "quay_prod_robot_token"
QUAY_REGISTRY = "quay.io"

def helm_registry_login(helm_registry: str, region: str):

def helm_registry_login_to_ecr(helm_registry: str, region: str):
logger.info(f"Attempting to log into ECR registry: {helm_registry}, using helm registry login.")

aws_command = ["aws", "ecr", "get-login-password", "--region", region]
Expand Down Expand Up @@ -67,7 +73,45 @@ def main():

registry = build_info.helm_charts["mongodb-kubernetes"].registry
region = build_info.helm_charts["mongodb-kubernetes"].region
return helm_registry_login(registry, region)

if registry == QUAY_REGISTRY:
return helm_registry_login_to_quay(registry)

return helm_registry_login_to_ecr(registry, region)


def helm_registry_login_to_quay(registry):
username = os.environ.get(QUAY_USERNAME_ENV_VAR)
password = os.environ.get(QUAY_PASSWORD_ENV_VAR)

if not username:
raise Exception(f"Env var {QUAY_USERNAME_ENV_VAR} must be set with the quay username.")
if not password:
raise Exception(f"Env var {QUAY_PASSWORD_ENV_VAR} must be set with the quay password.")

# using quote will help us avoid command injectin issues, was reported by semggrep in PR
command = ["helm", "registry", "login", "--username", quote(username), "--password-stdin", quote(registry)]

try:
result = subprocess.run(
command,
input=quote(password), # Pass the password as input bytes
capture_output=True,
text=True,
check=False, # Do not raise an exception on non-zero exit code
)

if result.returncode == 0:
logger.info(f"Successfully logged into helm continer registry {registry}.")
else:
raise Exception(
f"Helm registry login failed to {registry}. Stdout: {result.stderr.strip()}, Stderr: {result.stderr.strip()}"
)

except FileNotFoundError:
raise Exception("Error: 'helm' command not found. Ensure Helm CLI is installed and in your PATH.")
except Exception as e:
raise Exception(f"An unexpected error occurred during execution: {e}")


if __name__ == "__main__":
Expand Down
26 changes: 17 additions & 9 deletions scripts/release/publish_helm_chart.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from lib.base_logger import logger
from scripts.release.build.build_info import *
from scripts.release.helm_registry_login import BUILD_SCENARIO_RELEASE

CHART_DIR = "helm_chart"

Expand All @@ -28,17 +29,14 @@ def run_command(command: list[str]):

# update_chart_and_get_metadata updates the helm chart's Chart.yaml and sets the version
# to either evg patch id or commit which is set in OPERATOR_VERSION.
def update_chart_and_get_metadata(chart_dir: str) -> tuple[str, str]:
def update_chart_and_get_metadata(chart_dir: str, build_scenario) -> tuple[str, str]:
chart_path = os.path.join(chart_dir, "Chart.yaml")
version_id = os.environ.get("OPERATOR_VERSION")
if not version_id:
version = os.environ.get("OPERATOR_VERSION")
if not version:
raise ValueError(
"Error: Environment variable 'OPERATOR_VERSION' must be set to determine the chart version to publish."
)

new_version = f"0.0.0+{version_id}"
logger.info(f"New helm chart version will be: {new_version}")

if not os.path.exists(chart_path):
raise FileNotFoundError(
f"Error: Chart.yaml not found in directory '{chart_dir}'. "
Expand All @@ -52,7 +50,17 @@ def update_chart_and_get_metadata(chart_dir: str) -> tuple[str, str]:
chart_name = data.get("name")
if not chart_name:
raise ValueError("Chart.yaml is missing required 'name' field.")
except Exception as e:
raise Exception(f"Unable to load Chart.yaml from dir {chart_path}")

# if build_scenario is release, the chart.yaml would already have correct chart version
if build_scenario == BUILD_SCENARIO_RELEASE:
Copy link
Collaborator

@MaciejKaras MaciejKaras Oct 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

why not hide this logic in build_info.json? You could specify for example new field version_prefix as 0.0.0+ in patch and staging build scenarios. If it is specified just concatenate it with actual version_id, otherwise use plain version_id

Copy link
Contributor Author

@viveksinghggits viveksinghggits Oct 23, 2025

Choose a reason for hiding this comment

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

So, there are quite a few things here. First is, this change of figuring out the version of dev/staging is not really part of this PR, it was merged as part of the change in this PR.
Having said that, I can make the change (in new PR) but I would like to understand the benefit of doing that. According to me if we have the build_scenario and with that if we can figure something out without looking into build_info.json, does it make sense to look into build_info.json?
Having the eventual version dependent on build scenario makes the code more readable, if it's dev/staging we are appending 0.0.0. But if we read the prefix from build_info, like you said we are hiding away why we are appending things based on prefix.

Copy link
Contributor Author

@viveksinghggits viveksinghggits Oct 24, 2025

Choose a reason for hiding this comment

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

Or in other words, does it make sense to make something configurable, which doesn't need to be. We know that we are not going to change 0.0.0 to something else.

Copy link
Collaborator

@MaciejKaras MaciejKaras Oct 24, 2025

Choose a reason for hiding this comment

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

According to me if we have the build_scenario and with that if we can figure something out without looking into build_info.json, does it make sense to look into build_info.json?

If we follow this logic trail, we could just have the build_scenario variable and figure out everything in the code and don't read configuration in build_info.json at all. The reason for having build_info.json is to be able to configure different behaviour for different build scenarios and not have conditional logic in the code. Additional benefit is that we can read the build_info.json configuration and know how it is configured for each build scenario.

Copy link
Collaborator

@MaciejKaras MaciejKaras Oct 24, 2025

Choose a reason for hiding this comment

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

Having the eventual version dependent on build scenario makes the code more readable, if it's dev/staging we are appending 0.0.0. But if we read the prefix from build_info, like you said we are hiding away why we are appending things based on prefix.

I think we can still add comment about this in build_info.json next to dev and staging scenarios.

Copy link
Collaborator

@MaciejKaras MaciejKaras Oct 24, 2025

Choose a reason for hiding this comment

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

Or in other words, does it make sense to make something configurable, which doesn't need to be. We know that we are not going to change 0.0.0 to something else.

That's true, to build configuration will rarely change, but it doesn't mean this is the only benefit of having it - to change the configuration. The biggest gain in my opinion is to move abstract conditional logic from the code and make it simple, readable and testable.

Copy link
Contributor Author

@viveksinghggits viveksinghggits Oct 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

ok, I can make the change. Is it ok if I do it in the new PR?
The reason is, I want to focus one PR in just one change, don't want to do multiple changes in single PR.

Copy link
Collaborator

@MaciejKaras MaciejKaras Oct 24, 2025

Choose a reason for hiding this comment

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

Sure, it can be done separately.

return chart_name, version

new_version = f"0.0.0+{version}"
logger.info(f"New helm chart version will be: {new_version}")

try:
data["version"] = new_version

with open(chart_path, "w") as f:
Expand All @@ -79,10 +87,10 @@ def get_oci_registry(chart_info: HelmChartInfo) -> str:
return oci_registry


def publish_helm_chart(chart_info: HelmChartInfo):
def publish_helm_chart(chart_info: HelmChartInfo, build_scenario):
try:
oci_registry = get_oci_registry(chart_info)
chart_name, chart_version = update_chart_and_get_metadata(CHART_DIR)
chart_name, chart_version = update_chart_and_get_metadata(CHART_DIR, build_scenario)
tgz_filename = f"{chart_name}-{chart_version}.tgz"

logger.info(f"Packaging chart: {chart_name} with Version: {chart_version}")
Expand All @@ -108,7 +116,7 @@ def main():
build_scenario = args.build_scenario
build_info = load_build_info(build_scenario)

return publish_helm_chart(build_info.helm_charts["mongodb-kubernetes"])
return publish_helm_chart(build_info.helm_charts["mongodb-kubernetes"], build_scenario)


if __name__ == "__main__":
Expand Down