From 02df460b2c56a10b98716d1f8bc0d6ad8fcd7cac Mon Sep 17 00:00:00 2001 From: patrykkulik-microsoft <116072282+patrykkulik-microsoft@users.noreply.github.com> Date: Tue, 30 Jan 2024 11:04:21 +0000 Subject: [PATCH] Validate that helm chart values exist (#138) * add validation of the default values for helm charts * Add unit tests * update HISTORY.rst * Markups --- src/aosm/HISTORY.rst | 1 + .../cli_handlers/onboarding_cnf_handler.py | 7 +++ src/aosm/azext_aosm/common/exceptions.py | 2 +- .../azext_aosm/inputs/helm_chart_input.py | 51 +++++++++--------- .../nf-agent-cnf-invalid/values.yaml | 48 ----------------- .../latest/unit_test/test_helm_chart_input.py | 54 +++++++++++++++++++ 6 files changed, 89 insertions(+), 74 deletions(-) delete mode 100644 src/aosm/azext_aosm/tests/latest/mock_cnf/helm-charts/nf-agent-cnf-invalid/values.yaml diff --git a/src/aosm/HISTORY.rst b/src/aosm/HISTORY.rst index 6e41b2c4a2e..da97191c546 100644 --- a/src/aosm/HISTORY.rst +++ b/src/aosm/HISTORY.rst @@ -8,6 +8,7 @@ Unreleased * Add `publisher` command group for management of publisher resources. * Changed the name of the `path_to_mappings` parameter in the CNF input file to `default_values` * Added a `helm template` validation step to the `az aosm nfd build` command for the `cnf` definition type +* Added validation of the values file for helm charts when using the `az aosm nfd build` command for the `cnf` definition type * Fixed helm chart image parsing in the `az aosm nfd build` command for the `cnf` definition type. This means that the images can now be extracted correctly from the helm chart. * Fixed: infinite loop bug when retrying failed artifact uploads to the ACR diff --git a/src/aosm/azext_aosm/cli_handlers/onboarding_cnf_handler.py b/src/aosm/azext_aosm/cli_handlers/onboarding_cnf_handler.py index 79882dbc852..4ed1f02cdba 100644 --- a/src/aosm/azext_aosm/cli_handlers/onboarding_cnf_handler.py +++ b/src/aosm/azext_aosm/cli_handlers/onboarding_cnf_handler.py @@ -152,9 +152,16 @@ def _validate_helm_template(self): raise ValidationError(error_message) + def _validate_helm_values(self): + for helm_processor in self.processors: + helm_processor.input_artifact.validate_values() + def pre_validate_build(self): """Run all validation functions required before building the cnf.""" logger.debug("Pre-validating build") + + self._validate_helm_values() + if self.skip != HELM_TEMPLATE: self._validate_helm_template() diff --git a/src/aosm/azext_aosm/common/exceptions.py b/src/aosm/azext_aosm/common/exceptions.py index 34d3d23c7c1..f26bed907db 100644 --- a/src/aosm/azext_aosm/common/exceptions.py +++ b/src/aosm/azext_aosm/common/exceptions.py @@ -17,5 +17,5 @@ class SchemaGetOrGenerateError(Exception): """Raised when the schema cannot be generated or retrieved""" -class DefaultValuesNotFoundError(Exception): +class DefaultValuesNotFoundError(UserFault): """Raised when the default values file cannot be found""" diff --git a/src/aosm/azext_aosm/inputs/helm_chart_input.py b/src/aosm/azext_aosm/inputs/helm_chart_input.py index eb792dd58d4..08ebfa30df5 100644 --- a/src/aosm/azext_aosm/inputs/helm_chart_input.py +++ b/src/aosm/azext_aosm/inputs/helm_chart_input.py @@ -196,30 +196,37 @@ def validate_template(self) -> None: ) return error_message - def get_defaults(self) -> Dict[str, Any]: + def validate_values(self) -> None: """ - Retrieves the default values for the Helm chart. + Confirm that the values.yaml file exists in the Helm chart directory + or that the default values are provided. - :return: The default values for the Helm chart. - :rtype: Dict[str, Any] - :raises DefaultValuesNotFoundError: If no default values were found for the Helm chart. + :raises DefaultValuesNotFoundError: If the values.yaml and default values do not exist. """ - logger.info("Getting default values for Helm chart input") - try: - default_config = self.default_config or self._read_values_yaml() - logger.debug( - "Default values for Helm chart input: %s", - json.dumps(default_config, indent=4), - ) - return copy.deepcopy(default_config) - except FileNotFoundError as error: - logger.error("No default values found for Helm chart '%s'", self.chart_path) + default_config = self.default_config or self._read_values_yaml() + + if not default_config: + logger.error("No values found for Helm chart '%s'", self.chart_path) raise DefaultValuesNotFoundError( "ERROR: No default values found for the Helm chart" f" '{self.chart_path}'. Please provide default values" " or add a values.yaml file to the Helm chart." - ) from error + ) + + def get_defaults(self) -> Dict[str, Any]: + """ + Retrieves the default values for the Helm chart. + + :return: The default values for the Helm chart. + :rtype: Dict[str, Any] + """ + default_config = self.default_config or self._read_values_yaml() + logger.debug( + "Default values for Helm chart input: %s", + json.dumps(default_config, indent=4), + ) + return copy.deepcopy(default_config) def get_schema(self) -> Dict[str, Any]: """ @@ -244,10 +251,10 @@ def get_schema(self) -> Dict[str, Any]: schema = json.load(schema_file) if not schema: - # Otherwise, generate a schema from the default values in values.yaml. - logger.debug("Generating schema from values.yaml") + # Otherwise, generate a schema from the default values or values in values.yaml. + logger.debug("Generating schema from default values") built_schema = genson.Schema() - built_schema.add_object(self._read_values_yaml()) + built_schema.add_object(self.get_defaults()) schema = built_schema.to_dict() logger.debug( @@ -412,11 +419,5 @@ def _read_values_yaml(self) -> Dict[str, Any]: content = yaml_processor.load(f) return content - logger.error("No values.yaml file found in Helm chart '%s'", self.chart_path) - raise FileNotFoundError( - f"ERROR: No values file was found in the Helm chart" - f" '{self.chart_path}'." - ) - def __del__(self): shutil.rmtree(self._temp_dir_path) diff --git a/src/aosm/azext_aosm/tests/latest/mock_cnf/helm-charts/nf-agent-cnf-invalid/values.yaml b/src/aosm/azext_aosm/tests/latest/mock_cnf/helm-charts/nf-agent-cnf-invalid/values.yaml deleted file mode 100644 index 33e888d45db..00000000000 --- a/src/aosm/azext_aosm/tests/latest/mock_cnf/helm-charts/nf-agent-cnf-invalid/values.yaml +++ /dev/null @@ -1,48 +0,0 @@ -# Default values for nf-agent-cnf. -# This is a YAML-formatted file. -# Declare variables to be passed into your templates. - -replicaCount: 1 - -image: - # NFDV defines that AOSM overwrites image.repository with the Articfact Store ACR - repository: sunnyclipubsunnynfagentacr2dd56aed266.azurecr.io - pullPolicy: IfNotPresent - # Overrides the image tag whose default is the chart appVersion. - tag: "879624" - -# NFDV defines that AOSM overwrites imagePullSecrets with whatever is needed for the Artifact Store ACR -imagePullSecrets: [] -nameOverride: "" -fullnameOverride: "" - -serviceAccount: - # Specifies whether a service account should be created - create: true - # Annotations to add to the service account - annotations: {} - # The name of the service account to use. - # If not set and create is true, a name is generated using the fullname template - name: "" - -podAnnotations: {} - -autoscaling: - enabled: false - minReplicas: 1 - maxReplicas: 100 - targetCPUUtilizationPercentage: 80 - # targetMemoryUtilizationPercentage: 80 - -nodeSelector: {} - -tolerations: [] - -affinity: {} - -nfagent: - namespace: sb-uowvjfivpyuow - identity: 041db2eb-36e0-42cd-ac13-03ae8e997cd1 - topic: simpl - # name of pod identity - not using this any more - # podIdentity: mypeapod diff --git a/src/aosm/azext_aosm/tests/latest/unit_test/test_helm_chart_input.py b/src/aosm/azext_aosm/tests/latest/unit_test/test_helm_chart_input.py index 002f0cfcb40..98f34185b0d 100644 --- a/src/aosm/azext_aosm/tests/latest/unit_test/test_helm_chart_input.py +++ b/src/aosm/azext_aosm/tests/latest/unit_test/test_helm_chart_input.py @@ -2,13 +2,16 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- +import logging from unittest import TestCase from pathlib import Path import os +import sys from azext_aosm.inputs.helm_chart_input import ( HelmChartInput, ) +from azext_aosm.common.exceptions import DefaultValuesNotFoundError code_directory = os.path.dirname(__file__) parent_directory = os.path.abspath(os.path.join(code_directory, "..")) @@ -21,6 +24,9 @@ class TestHelmChartInput(TestCase): """Test the HelmChartInput class.""" + def setUp(self): + logging.basicConfig(level=logging.INFO, stream=sys.stdout) + def test_validate_template_valid_chart(self): """Test validating a valid Helm chart using helm template.""" @@ -56,3 +62,51 @@ def test_validate_template_invalid_chart(self): output = helm_chart_input.validate_template() assert output != "" + + def test_validate_values(self): + """Test validating whether values exist in a helm chart.""" + + helm_chart_input = HelmChartInput( + artifact_name="test-invalid", + artifact_version="1.0.0", + chart_path=Path( + os.path.join( + helm_charts_directory, + VALID_CHART_NAME, + ) + ), + ) + + helm_chart_input.validate_values() + + # Use an Invalid chart (because it does not have values.yaml in it), + # but provide a default config parameter which will override those values. + helm_chart_input_with_default_values = HelmChartInput( + artifact_name="test-default-values", + artifact_version="1.0.0", + chart_path=Path( + os.path.join( + helm_charts_directory, + INVALID_CHART_NAME, + ) + ), + default_config={"test": "test"}, + ) + + helm_chart_input_with_default_values.validate_values() + + def test_validate_invalid_values(self): + """Test validating a helm chart with values.yaml missing.""" + helm_chart_input = HelmChartInput( + artifact_name="test-invalid", + artifact_version="1.0.0", + chart_path=Path( + os.path.join( + helm_charts_directory, + INVALID_CHART_NAME, + ) + ), + ) + + with self.assertRaises(DefaultValuesNotFoundError): + helm_chart_input.validate_values()