Merge pull request Azure#32 from jddarby/jl/add-unit-testing

Add unit tests for build and generate config.

src/aosm/README.md

@@ -175,3 +175,25 @@ Delete a published design
 Delete a published design and the publisher, artifact stores and NSD group
 
 `az aosm nsd delete --config-file input.json --clean`
+
+# Development
+
+## Unit tests
+To run unit tests run `azdev test aosm`.
+
+To get code coverage run:
+```bash
+pip install coverage 
+cd src/aosm
+coverage erase
+coverage run -m pytest .
+coverage report --include="*/src/aosm/*" --omit="*/src/aosm/azext_aosm/vendored_sdks/*","*/src/aosm/azext_aosm/tests/*" -m
+```
+
+## Pipelines
+The pipelines for the Azure CLI run in ADO, not in github.
+To trigger a pipeline you need to create a PR against main.
+Until we do the initial merge to main we don't want to have a PR to main for every code review.
+Instead we have a single PR for the `add-aosm-extension` branch: https://github.com/Azure/azure-cli-extensions/pull/6426
+Once you have merged your changes to `add-aosm-extension` then look at the Azure Pipelines under https://github.com/Azure/azure-cli-extensions/pull/6426/checks, click on the link that says `<X> errors / <Y> warnings`.
+

src/aosm/azext_aosm/_configuration.py

@@ -1,3 +1,4 @@
+import json
 import os
 import re
 from dataclasses import dataclass, field
@@ -117,7 +118,32 @@ class ArtifactConfig:
 
 
 @dataclass
-class NFConfiguration:
+class Configuration:
+    config_file: Optional[str] = None
+
+    def path_from_cli_dir(self, path: str) -> str:
+        """
+        Convert path from config file to path from current directory.
+
+        We assume that the path supplied in the config file is relative to the
+        configuration file.  That isn't the same as the path relative to where ever the
+        CLI is being run from.  This function fixes that up.
+
+        :param path: The path relative to the config file.
+        """
+        # If no path has been supplied we shouldn't try to update it.
+        if path == "":
+            return ""
+
+        # If it is an absolute path then we don't need to monkey around with it.
+        if os.path.isabs(path):
+            return path
+
+        return os.path.join(os.path.dirname(self.config_file), path)
+
+
+@dataclass
+class NFConfiguration(Configuration):
     publisher_name: str = DESCRIPTION_MAP["publisher_name"]
     publisher_resource_group_name: str = DESCRIPTION_MAP[
         "publisher_resource_group_name"
@@ -140,7 +166,7 @@ def acr_manifest_name(self) -> str:
 
 
 @dataclass
-class NSConfiguration:
+class NSConfiguration(Configuration):
     # pylint: disable=too-many-instance-attributes
     location: str = DESCRIPTION_MAP["location"]
     publisher_name: str = DESCRIPTION_MAP["publisher_name_nsd"]
@@ -274,9 +300,13 @@ def __post_init__(self):
         Used when creating VNFConfiguration object from a loaded json config file.
         """
         if isinstance(self.arm_template, dict):
+            self.arm_template["file_path"] = self.path_from_cli_dir(
+                self.arm_template["file_path"]
+            )
             self.arm_template = ArtifactConfig(**self.arm_template)
 
         if isinstance(self.vhd, dict):
+            self.vhd["file_path"] = self.path_from_cli_dir(self.vhd["file_path"])
             self.vhd = ArtifactConfig(**self.vhd)
             self.validate()
 
@@ -359,6 +389,10 @@ def __post_init__(self):
         """
         for package_index, package in enumerate(self.helm_packages):
             if isinstance(package, dict):
+                package["path_to_chart"] = self.path_from_cli_dir(package["path_to_chart"])
+                package["path_to_mappings"] = self.path_from_cli_dir(
+                    package["path_to_mappings"]
+                )
                 self.helm_packages[package_index] = HelmPackageConfig(**dict(package))
 
     @property
@@ -384,24 +418,27 @@ def validate(self):
 
 
 def get_configuration(
-    configuration_type: str, config_as_dict: Optional[Dict[Any, Any]] = None
+    configuration_type: str, config_file: str = None
 ) -> NFConfiguration or NSConfiguration:
     """
     Return the correct configuration object based on the type.
 
     :param configuration_type: The type of configuration to return
-    :param config_as_dict: The configuration as a dictionary
+    :param config_file: The path to the config file
     :return: The configuration object
     """
-    if config_as_dict is None:
+    if config_file:
+        with open(config_file, "r", encoding="utf-8") as f:
+            config_as_dict = json.loads(f.read())
+    else:
         config_as_dict = {}
 
     if configuration_type == VNF:
-        config = VNFConfiguration(**config_as_dict)
+        config = VNFConfiguration(config_file=config_file, **config_as_dict)
     elif configuration_type == CNF:
-        config = CNFConfiguration(**config_as_dict)
+        config = CNFConfiguration(config_file=config_file, **config_as_dict)
     elif configuration_type == NSD:
-        config = NSConfiguration(**config_as_dict)
+        config = NSConfiguration(config_file=config_file, **config_as_dict)
     else:
         raise InvalidArgumentValueError(
             "Definition type not recognized, options are: vnf, cnf or nsd"

src/aosm/azext_aosm/_help.py

@@ -4,7 +4,7 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
-from knack.help_files import helps  # pylint: disable=unused-import
+from knack.help_files import helps
 
 helps[
     "aosm"

src/aosm/azext_aosm/_params.py

@@ -110,9 +110,7 @@ def load_arguments(self: AzCommandsLoader, _):
                 " alternative parameters."
             ),
         )
-        c.argument(
-            "skip", arg_type=skip_steps, help="Optional skip steps"
-        )
+        c.argument("skip", arg_type=skip_steps, help="Optional skip steps")
 
     with self.argument_context("aosm nsd") as c:
         c.argument(
@@ -122,6 +120,4 @@ def load_arguments(self: AzCommandsLoader, _):
             completer=FilesCompleter(allowednames="*.json"),
             help="The path to the configuration file.",
         )
-        c.argument(
-            "skip", arg_type=skip_steps, help="Optional skip steps"
-        )
+        c.argument("skip", arg_type=skip_steps, help="Optional skip steps")

src/aosm/azext_aosm/custom.py

@@ -95,9 +95,7 @@ def _get_config_from_file(
             " path."
         )
 
-    with open(config_file, "r", encoding="utf-8") as f:
-        config_as_dict = json.loads(f.read())
-    config = get_configuration(configuration_type, config_as_dict)
+    config = get_configuration(configuration_type, config_file)
     return config
 
 
@@ -251,8 +249,13 @@ def _generate_config(configuration_type: str, output_file: str = "input.json"):
     :param output_file: path to output config file, defaults to "input.json"
     :type output_file: str, optional
     """
-    config = get_configuration(configuration_type)
-    config_as_dict = json.dumps(asdict(config), indent=4)
+    # Config file is a special parameter on the configuration objects.  It is the path
+    # to the configuration file, rather than an input parameter.  It therefore shouldn't
+    # be included here.
+    config = asdict(get_configuration(configuration_type))
+    config.pop("config_file")
+
+    config_as_dict = json.dumps(config, indent=4)
 
     if Path(output_file).exists():
         carry_on = input(

src/aosm/azext_aosm/deploy/artifact_manifest.py

@@ -139,9 +139,7 @@ def _get_artifact_client(
 
             # For AOSM to work VHD blobs must have the suffix .vhd
             if artifact.artifact_name.endswith("-vhd"):
-                blob_name = (
-                    f"{artifact.artifact_name[:-4].replace('-', '')}-{artifact.artifact_version}.vhd"
-                )
+                blob_name = f"{artifact.artifact_name[:-4].replace('-', '')}-{artifact.artifact_version}.vhd"
             else:
                 blob_name = container_name
 

src/aosm/azext_aosm/deploy/deploy_with_arm.py

@@ -89,7 +89,7 @@ def deploy_vnfd_from_bicep(
         parameters_json_file: Optional[str] = None,
         manifest_bicep_path: Optional[str] = None,
         manifest_parameters_json_file: Optional[str] = None,
-        skip: Optional[str] = None
+        skip: Optional[str] = None,
     ) -> None:
         """
         Deploy the bicep template defining the VNFD.
@@ -274,7 +274,7 @@ def deploy_cnfd_from_bicep(
         parameters_json_file: Optional[str] = None,
         manifest_bicep_path: Optional[str] = None,
         manifest_parameters_json_file: Optional[str] = None,
-        skip: Optional[str] = None
+        skip: Optional[str] = None,
     ) -> None:
         """
         Deploy the bicep template defining the CNFD.
@@ -457,7 +457,9 @@ def deploy_nsd_from_bicep(
                     manifest_parameters_json_file, manifest_bicep_path, NSD
                 )
             else:
-                print(f"Artifact manifests {self.config.acr_manifest_name} already exists")
+                print(
+                    f"Artifact manifests {self.config.acr_manifest_name} already exists"
+                )
 
             message = (
                 f"Deploy bicep template for NSDV {self.config.nsd_version} "
@@ -487,7 +489,9 @@ def deploy_nsd_from_bicep(
 
         # Convert the NF bicep to ARM
         arm_template_artifact_json = self.convert_bicep_to_arm(
-            os.path.join(self.config.build_output_folder_name, NF_DEFINITION_BICEP_FILENAME)
+            os.path.join(
+                self.config.build_output_folder_name, NF_DEFINITION_BICEP_FILENAME
+            )
         )
 
         with open(self.config.arm_template.file_path, "w", encoding="utf-8") as file:

src/aosm/azext_aosm/deploy/pre_deploy.py

@@ -14,7 +14,7 @@
     NFConfiguration,
     NSConfiguration,
     VNFConfiguration,
-    CNFConfiguration
+    CNFConfiguration,
 )
 from azext_aosm.util.constants import SOURCE_ACR_REGEX
 from azext_aosm.util.management_clients import ApiClients
@@ -154,12 +154,16 @@ def ensure_config_source_registry_exists(self) -> None:
         )
 
         # Match the source registry format
-        source_registry_match = re.search(SOURCE_ACR_REGEX, self.config.source_registry_id)
+        source_registry_match = re.search(
+            SOURCE_ACR_REGEX, self.config.source_registry_id
+        )
         # Config validation has already checked and raised an error if the regex doesn't
         # match
         if source_registry_match and len(source_registry_match.groups()) > 1:
-            source_registry_resource_group_name = source_registry_match.group('resource_group')
-            source_registry_name = source_registry_match.group('registry_name')
+            source_registry_resource_group_name = source_registry_match.group(
+                "resource_group"
+            )
+            source_registry_name = source_registry_match.group("registry_name")
 
             # This will raise an error if the registry does not exist
             self.api_clients.container_registry_client.get(

src/aosm/azext_aosm/generate_nfd/cnf_nfd_generator.py

@@ -191,7 +191,7 @@ def _generate_chart_value_mappings(self, helm_package: HelmPackageConfig) -> Non
         logger.debug(
             "Creating chart value mappings file for %s", helm_package.path_to_chart
         )
-        print("Creating chart value mappings file for %s", helm_package.path_to_chart)
+        print(f"Creating chart value mappings file for {helm_package.path_to_chart}.")
 
         # Get all the values files in the chart
         top_level_values_yaml = self._read_top_level_values_yaml(helm_package)

src/aosm/azext_aosm/generate_nsd/nsd_generator.py

@@ -27,6 +27,7 @@
     NSD_DEFINITION_JINJA2_SOURCE_TEMPLATE,
     SCHEMAS_DIR_NAME,
     TEMPLATES_DIR_NAME,
+    VNF,
 )
 from azext_aosm.util.management_clients import ApiClients
 from azext_aosm.vendored_sdks.models import NetworkFunctionDefinitionVersion, NFVIType
@@ -61,9 +62,7 @@ def __init__(self, api_clients: ApiClients, config: NSConfiguration):
         self.nsd_bicep_template_name = NSD_DEFINITION_JINJA2_SOURCE_TEMPLATE
         self.nf_bicep_template_name = NF_TEMPLATE_JINJA2_SOURCE_TEMPLATE
         self.nsd_bicep_output_name = NSD_BICEP_FILENAME
-        self.nfdv_parameter_name = (
-            f"{self.config.network_function_definition_group_name.replace('-', '_')}_nfd_version"
-        )
+        self.nfdv_parameter_name = f"{self.config.network_function_definition_group_name.replace('-', '_')}_nfd_version"
         self.build_folder_name = self.config.build_output_folder_name
         nfdv = self._get_nfdv(config, api_clients)
         print("Finding the deploy parameters of the NFDV resource")
@@ -138,7 +137,7 @@ def config_group_schema_dict(self) -> Dict[str, Any]:
             "type": "string",
             "description": description_string,
         }
-        cgs_dict["required"].append(self.nfdv_parameter_name)
+        cgs_dict.setdefault("required", []).append(self.nfdv_parameter_name)
 
         managed_identity_description_string = (
             "The managed identity to use to deploy NFs within this SNS.  This should "
@@ -161,8 +160,10 @@ def config_group_schema_dict(self) -> Dict[str, Any]:
                 "/{resourceGroupName}/providers/microsoft.extendedlocation/"
                 "customlocations/{customLocationName}'"
             )
-            cgs_dict["properties"]["customLocationId"] = \
-                {"type": "string", "description": custom_location_description_string}
+            cgs_dict["properties"]["customLocationId"] = {
+                "type": "string",
+                "description": custom_location_description_string,
+            }
             cgs_dict["required"].append("customLocationId")
 
         return cgs_dict
@@ -233,9 +234,8 @@ def write_nf_bicep(self) -> None:
             # location is sometimes part of deploy_properties.
             # We want to avoid having duplicate params in the bicep template
             logger.debug(
-                "Adding deploy parameter key: %s, value: %s to nf template",
-                key,
-                value)
+                "Adding deploy parameter key: %s, value: %s to nf template", key, value
+            )
             if key != "location":
                 bicep_type = (
                     NFV_TO_BICEP_PARAM_TYPES.get(value["type"]) or value["type"]
@@ -301,7 +301,7 @@ def write_nsd_manifest(self) -> None:
     def generate_bicep(self,
                        template_name: str,
                        output_file_name: str,
-                       params: Dict[Any,Any]) -> None:
+                       params: Dict[Any, Any]) -> None:
         """
         Render the bicep templates with the correct parameters and copy them into the build output folder.
 

src/aosm/azext_aosm/tests/latest/mock_cnf/helm-charts/nf-agent-cnf/.helmignore

@@ -0,0 +1,23 @@
+# Patterns to ignore when building packages.
+# This supports shell glob matching, relative path matching, and
+# negation (prefixed with !). Only one pattern per line.
+.DS_Store
+# Common VCS dirs
+.git/
+.gitignore
+.bzr/
+.bzrignore
+.hg/
+.hgignore
+.svn/
+# Common backup files
+*.swp
+*.bak
+*.tmp
+*.orig
+*~
+# Various IDEs
+.project
+.idea/
+*.tmproj
+.vscode/

src/aosm/azext_aosm/tests/latest/mock_cnf/helm-charts/nf-agent-cnf/Chart.yaml

@@ -0,0 +1,24 @@
+apiVersion: v2
+name: nf-agent-cnf
+description: A Helm chart for Kubernetes
+
+# A chart can be either an 'application' or a 'library' chart.
+#
+# Application charts are a collection of templates that can be packaged into versioned archives
+# to be deployed.
+#
+# Library charts provide useful utilities or functions for the chart developer. They're included as
+# a dependency of application charts to inject those utilities and functions into the rendering
+# pipeline. Library charts do not define any templates and therefore cannot be deployed.
+type: application
+
+# This is the chart version. This version number should be incremented each time you make changes
+# to the chart and its templates, including the app version.
+# Versions are expected to follow Semantic Versioning (https://semver.org/)
+version: 0.1.0
+
+# This is the version number of the application being deployed. This version number should be
+# incremented each time you make changes to the application. Versions are not expected to
+# follow Semantic Versioning. They should reflect the version the application is using.
+# It is recommended to use it with quotes.
+appVersion: "1.16.0"