From 120df4573d461ab40b4845bb74178ff6304a429b Mon Sep 17 00:00:00 2001 From: Igor Soares Date: Fri, 29 Sep 2023 18:45:22 -0300 Subject: [PATCH] Enforce Helm chart version uniqueness During application upload if the incoming chart version already exists in the target Helm repository but the implementation is different then reject the upload. Also, if the incoming chart version exists and the incoming chart is identical to the existing version then just skip the upload. This relies on the digest and version checks performed by the helm-upload script. Helm repository management capabilities have been included to the Application Framework in order to support applications' lifecycle. When an application is removed, its Helm charts are also removed from the repository. Similarlly, when an application is updated, the charts related to the previous version are also removed from the repository. Repositories are re-indexed when charts are deleted and their index files are updated accordingly. Test Plan: PASS: build-pkgs && build-image PASS: AIO-SX fresh install PASS: Run "system application-upload vault-1.0-54.tgz" Check if the chart was correctly uploaded to the repository Run "system application-delete vault" Check if the chart was correctly deleted from the repository Check if the index.yaml file was regenerated accordingly PASS: Run "system application-upload vault-1.0-54.tgz" Run "system application-apply vault" Bump chart version, bump app version and repackage Run "system application-update vault-1.0-55.tgz" Check if the chart of the previous version was correctly removed from the Helm repository Check if the index.yaml file was regenerated accordingly PASS: Run "system application-upload vault-1.0-54.tgz" Run "system application-apply vault" Bump app version to vault-1.0-55 on metadata.yaml and repackage system application-update vault-1.0-55.tgz Check that the app was updated and the chart remained untouched PASS: Run "system application-upload vault-1.0-54.tgz" Run "system application-apply vault" Change image tag on chart, keep chart version, bump app version on metadata and repackage Run "system application-update vault-1.0-55.tgz" Check that the app update failed Story: 2010929 Task: 48882 Depends-On: https://review.opendev.org/c/starlingx/integ/+/896870 Change-Id: I421482969f4cc9fd3789309c5c69b9a3f233053a Signed-off-by: Igor Soares --- .../sysinv/sysinv/conductor/kube_app.py | 68 ++++++++++++++++++- sysinv/sysinv/sysinv/sysinv/helm/common.py | 1 + sysinv/sysinv/sysinv/sysinv/helm/utils.py | 59 ++++++++++++++++ 3 files changed, 126 insertions(+), 2 deletions(-) diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py index c9612abbc9..c493ad94e8 100644 --- a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py +++ b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py @@ -76,6 +76,10 @@ LOCK_NAME_PROCESS_APP_METADATA = 'process_app_metadata' STX_APP_PLUGIN_PATH = '/var/stx_app/plugins' +CHART_UPLOAD_COPY_ERROR_CODE = 1 +CHART_UPLOAD_FILE_EXISTS_ERROR_CODE = 2 +CHART_UPLOAD_VERSION_EXISTS_ERROR_CODE = 3 + # Helper functions def generate_install_manifest_fqpn(app_name, app_version, manifest_filename): @@ -119,7 +123,8 @@ def get_app_install_root_path_ownership(): FluxCDChart = namedtuple('FluxCDChart', 'metadata_name name namespace location ' 'release chart_os_path chart_label ' - 'helm_repo_name') + 'helm_repo_name filesystem_location ' + 'chart_version') class AppOperator(object): @@ -541,6 +546,24 @@ class AppOperator(object): """ Retrieve the namespace of a top level kustomization """ return root_kustomization_yaml.get("namespace", constants.FLUXCD_K8S_FALLBACK_NAMESPACE) + @staticmethod + def remove_app_charts_from_repo(app_charts): + """ Remove application charts from Helm repository""" + + repo_set = set() + for chart in app_charts: + try: + os.remove(chart.filesystem_location) + repo_set.add(chart.helm_repo_name) + except OSError: + LOG.error("Error while removing chart {} from repository". + format(chart.filesystem_location)) + + # Re-index repositories + for repo_name in repo_set: + helm_utils.index_repo(os.path.join(common.HELM_REPO_BASE_PATH, + repo_name)) + def _get_image_tags_by_charts_fluxcd(self, app_images_file, manifest, overrides_dir): app_imgs = [] images_file = None @@ -899,6 +922,27 @@ class AppOperator(object): # Make sure any helm repo changes are reflected for the users helm_utils.refresh_helm_repo_information() + except subprocess.CalledProcessError as e: + if e.returncode == CHART_UPLOAD_COPY_ERROR_CODE: + reason = "Error while copying chart file %s to %s repository" \ + % (chart, helm_repo) + elif e.returncode == CHART_UPLOAD_FILE_EXISTS_ERROR_CODE: + # If the exact same chart already exists then just log a + # warning and proceed with the upload process. + LOG.warning("Chart %s already exists in the %s repository. " + "Skipping upload." % + (os.path.basename(chart), helm_repo)) + elif e.returncode == CHART_UPLOAD_VERSION_EXISTS_ERROR_CODE: + reason = "The incoming chart %s matches the same version of " \ + "an existing chart in the % repository that " \ + "has a different implementation." \ + % (os.path.basename(chart), helm_repo) + else: + reason = str(e) + + if e.returncode != CHART_UPLOAD_FILE_EXISTS_ERROR_CODE: + raise exception.KubeAppUploadFailure( + name=app.name, version=app.version, reason=reason) except Exception as e: raise exception.KubeAppUploadFailure( name=app.name, version=app.version, reason=str(e)) @@ -1164,22 +1208,30 @@ class AppOperator(object): metadata_name = helmrelease_yaml["metadata"]["name"] chart_spec = helmrelease_yaml["spec"]["chart"] chart_name = chart_spec["spec"]["chart"] + chart_version = chart_spec["spec"]["version"] location = "%s/%s-%s%s" % (helm_repo_url.rstrip("/"), chart_name, chart_spec["spec"]["version"], ".tgz") + filesystem_location = helm_utils.get_chart_tarball_path( + os.path.join(common.HELM_REPO_BASE_PATH, helm_repo_name), + chart_name, + chart_version) release = helmrelease_yaml["spec"]["releaseName"] # Dunno if we need to return these in order respecting dependsOn? - # dependencies = [dep["name"] for dep in helmrelease_yaml["spec"].get(["dependsOn"], [])] + # dependencies = [dep["name"] for dep in helmrelease_yaml["spec"]. + # get(["dependsOn"], [])] chart_obj = FluxCDChart( metadata_name=metadata_name, name=metadata_name, namespace=namespace, location=location, + filesystem_location=filesystem_location, release=release, chart_os_path=chart_path, chart_label=chart_name, + chart_version=chart_version, helm_repo_name=helm_repo_name ) charts.append(chart_obj) @@ -2775,6 +2827,7 @@ class AppOperator(object): from_app.charts = self._get_list_of_charts(from_app) to_app_charts = [c.release for c in to_app.charts] deployed_releases = helm_utils.retrieve_helm_releases() + charts_to_delete = [] for from_chart in from_app.charts: # Cleanup the releases in the old application version # but are not in the new application version @@ -2794,7 +2847,13 @@ class AppOperator(object): LOG.info("Helm release %s for Application %s (%s) deleted" % (from_chart.release, from_app.name, from_app.version)) + for to_app_chart in to_app.charts: + if from_chart.chart_label == to_app_chart.chart_label \ + and from_chart.chart_version \ + != to_app_chart.chart_version: + charts_to_delete.append(from_chart) + AppOperator.remove_app_charts_from_repo(charts_to_delete) self._cleanup(from_app, app_dir=False) self._utils._patch_report_app_dependencies( from_app.name + '-' + from_app.version) @@ -3043,6 +3102,7 @@ class AppOperator(object): self._plugins.deactivate_plugins(app) self._dbapi.kube_app_destroy(app.name) + app.charts = self._get_list_of_charts(app) self._cleanup(app) self._utils._patch_report_app_dependencies(app.name + '-' + app.version) # One last check of app alarm, should be no-op unless the @@ -3052,6 +3112,10 @@ class AppOperator(object): # Remove the deleted app from _apps_metadata, since it's # not in the system anymore. self._remove_from_metadata_dict(app.name) + + # Remove charts from Helm repository + AppOperator.remove_app_charts_from_repo(app.charts) + LOG.info("Application (%s) has been purged from the system." % app.name) msg = None diff --git a/sysinv/sysinv/sysinv/sysinv/helm/common.py b/sysinv/sysinv/sysinv/sysinv/helm/common.py index 4d1cc92804..d9428a29d5 100644 --- a/sysinv/sysinv/sysinv/sysinv/helm/common.py +++ b/sysinv/sysinv/sysinv/sysinv/helm/common.py @@ -18,6 +18,7 @@ LOG = logging.getLogger(__name__) HELM_OVERRIDES_PATH = os.path.join(tsconfig.PLATFORM_PATH, 'helm', tsconfig.SW_VERSION) # Supported chart repositories +HELM_REPO_BASE_PATH = '/var/www/pages/helm_charts' HELM_REPO_FOR_APPS = 'starlingx' HELM_REPO_FOR_PLATFORM = 'stx-platform' diff --git a/sysinv/sysinv/sysinv/sysinv/helm/utils.py b/sysinv/sysinv/sysinv/sysinv/helm/utils.py index cae430ab8d..febeb193c4 100644 --- a/sysinv/sysinv/sysinv/sysinv/helm/utils.py +++ b/sysinv/sysinv/sysinv/sysinv/helm/utils.py @@ -13,6 +13,7 @@ import base64 import os import psutil import ruamel.yaml as yaml +import io import tempfile import random import string @@ -288,3 +289,61 @@ def build_overrides_filename(chart_name, namespace=None): filename = namespace + "-" + filename return filename + + +def get_chart_tarball_path(repo_path, chart_name, chart_version): + """ Get the path of a chart tarball available in a Helm repository. + + :param repo_path: Filesystem path to the Helm repository + :param chart_name: Name of the Helm chart + :param chart_version: Version of the Helm chart + :return: string + Full path of the chart tarball in the repository if a + matching chart/version is found. Otherwise returns None. + """ + + repo_index_file = os.path.join(repo_path, "index.yaml") + with io.open(repo_index_file, "r", encoding="utf-8") as f: + root_index_yaml = next(yaml.safe_load_all(f)) + chart_versions = root_index_yaml["entries"][chart_name] + + for chart in chart_versions: + if chart["version"] == chart_version and len(chart["urls"]) > 0: + return os.path.join(repo_path, chart["urls"][0]) + + +def index_repo(repo_path): + """ Index a given Helm repository. + + :param repo_path: Filesystem path to the Helm repository + """ + + helm_cmd = ['helm', 'repo', 'index', repo_path] + env = os.environ.copy() + env['KUBECONFIG'] = kubernetes.KUBERNETES_ADMIN_CONF + + process = subprocess.Popen( + helm_cmd, + env=env, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + universal_newlines=True) + timer = threading.Timer(20, kill_process_and_descendants, [process]) + + try: + timer.start() + _, err = process.communicate() + + if process.returncode == 0 and err: + LOG.warning("Command: %s; %s" % (' '.join(helm_cmd), err)) + elif err: + LOG.error("Failed to index repository {}".format(repo_path)) + raise exception.HelmFailure(reason=err) + else: + err_msg = "Timeout while indexing repository {}".format(repo_path) + raise exception.HelmFailure(reason=err_msg) + except Exception as e: + err_msg = "Failed to execute Helm command: {}".format(e) + LOG.error(err_msg) + raise exception.HelmFailure(reason=err_msg) + finally: + timer.cancel()