Fixes for Windows (#58)

* Fix bicep render on Windows

* Fixes for Windows

* python-static-checks ran

src/aosm/README.md

@@ -22,7 +22,7 @@ https://github.com/jddarby/azure-cli-extensions/releases/download/aosm-extension
 To install, download this wheel and run:
 `az extension add --source path/to/aosm-0.2.0-py2.py3-none-any.whl`
 
-You must also have helm installed, instructions can be found here: https://helm.sh/docs/intro/install/#through-package-managers
+For CNFs you will also need helm and docker installed. See [CNFs](#cnfs) below for details.
 
 ## Updating 
 
@@ -92,6 +92,10 @@ For NSDs, you will need to have a Resource Group with a deployed Publisher, Arti
 
 ### Command examples
 
+#### Before you start
+`az login` to login to the Azure CLI.
+`az account set --subscription <subscription>` to choose the subscription you will work on.
+
 #### NFDs
 
 Get help on command arguments

src/aosm/azext_aosm/_configuration.py

@@ -48,7 +48,8 @@
     "artifact_name": "Name of the artifact",
     "file_path": (
         "Optional. File path of the artifact you wish to upload from your local disk. "
-        "Delete if not required."
+        "Delete if not required. Relative paths are relative to the configuration file."
+        "On Windows escape any backslash with another backslash."
     ),
     "blob_sas_url": (
         "Optional. SAS URL of the blob artifact you wish to copy to your Artifact"
@@ -68,7 +69,8 @@
     ),
     "helm_package_name": "Name of the Helm package",
     "path_to_chart": (
-        "File path of Helm Chart on local disk. Accepts .tgz, .tar or .tar.gz"
+        "File path of Helm Chart on local disk. Accepts .tgz, .tar or .tar.gz."
+        " Use Linux slash (/) file separator even if running on Windows."
     ),
     "path_to_mappings": (
         "File path of value mappings on local disk where chosen values are replaced "
@@ -86,7 +88,7 @@
         "image to use for the VM."
     ),
     "source_registry_id": (
-        "Resource ID of the source acr registry from which to pull the image"
+        "Resource ID of the source acr registry from which to pull the image."
     ),
     "source_registry_namespace": (
         "Optional. Namespace of the repository of the source acr registry from which "
@@ -153,9 +155,7 @@ def output_directory_for_build(self) -> Path:
 
     @property
     def acr_manifest_names(self) -> List[str]:
-        """
-        The list of ACR manifest names.
-        """
+        """The list of ACR manifest names."""
         raise NotImplementedError("Subclass must define property")
 
 
@@ -343,9 +343,7 @@ def validate(self):
 
 @dataclass
 class NFDRETConfiguration:
-    """
-    The configuration required for an NFDV that you want to include in an NSDV.
-    """
+    """The configuration required for an NFDV that you want to include in an NSDV."""
 
     publisher: str = PUBLISHER_NAME
     publisher_resource_group: str = PUBLISHER_RESOURCE_GROUP
@@ -406,10 +404,8 @@ def build_output_folder_name(self) -> Path:
 
     @property
     def arm_template(self) -> ArtifactConfig:
-        """
-        Return the parameters of the ARM template for this RET to be uploaded as part of
-        the NSDV.
-        """
+        """Return the parameters of the ARM template for this RET to be uploaded as part of
+        the NSDV."""
         artifact = ArtifactConfig()
         artifact.artifact_name = f"{self.name.lower()}_nf_artifact"
 
@@ -452,9 +448,7 @@ class NSConfiguration(Configuration):
     nsdv_description: str = DESCRIPTION_MAP["nsdv_description"]
 
     def __post_init__(self):
-        """
-        Covert things to the correct format.
-        """
+        """Covert things to the correct format."""
         if self.network_functions and isinstance(self.network_functions[0], dict):
             nf_ret_list = [
                 NFDRETConfiguration(**config) for config in self.network_functions
@@ -506,9 +500,7 @@ def cg_schema_name(self) -> str:
 
     @property
     def acr_manifest_names(self) -> List[str]:
-        """
-        The list of ACR manifest names for all the NF ARM templates.
-        """
+        """The list of ACR manifest names for all the NF ARM templates."""
         return [nf.acr_manifest_name(self.nsd_version) for nf in self.network_functions]
 
 

src/aosm/azext_aosm/custom.py

@@ -103,7 +103,11 @@ def _get_config_from_file(config_file: str, configuration_type: str) -> Configur
 
 
 def _generate_nfd(
-    definition_type: str, config: NFConfiguration, order_params: bool, interactive: bool, force: bool = False
+    definition_type: str,
+    config: NFConfiguration,
+    order_params: bool,
+    interactive: bool,
+    force: bool = False,
 ):
     """Generate a Network Function Definition for the given type and config."""
     nfd_generator: NFDGenerator
@@ -199,7 +203,7 @@ def delete_published_definition(
     definition_type,
     config_file,
     clean=False,
-    force=False
+    force=False,
 ):
     """
     Delete a published definition.
@@ -277,7 +281,9 @@ def _generate_config(configuration_type: str, output_file: str = "input.json"):
         )
 
 
-def build_design(cmd, client: HybridNetworkManagementClient, config_file: str, force: bool = False):
+def build_design(
+    cmd, client: HybridNetworkManagementClient, config_file: str, force: bool = False
+):
     """
     Build a Network Service Design.
 
@@ -386,7 +392,9 @@ def publish_design(
     deployer.deploy_nsd_from_bicep()
 
 
-def _generate_nsd(config: NSConfiguration, api_clients: ApiClients, force: bool = False):
+def _generate_nsd(
+    config: NSConfiguration, api_clients: ApiClients, force: bool = False
+):
     """Generate a Network Service Design for the given config."""
     if config:
         nsd_generator = NSDGenerator(config=config, api_clients=api_clients)

src/aosm/azext_aosm/delete/delete.py

@@ -103,7 +103,9 @@ def delete_nsd(self, clean: bool = False, force: bool = False) -> None:
                 f" {self.config.cg_schema_name}?"
             )
             if clean:
-                print(f"Because of the --clean flag, the NSDG {self.config.nsdg_name} will also be deleted.")
+                print(
+                    f"Because of the --clean flag, the NSDG {self.config.nsdg_name} will also be deleted."
+                )
             print("There is no undo. Type 'delete' to confirm")
             if not input_ack("delete", "Confirm delete:"):
                 print("Not proceeding with delete")

src/aosm/azext_aosm/deploy/artifact.py

@@ -4,6 +4,7 @@
 # pylint: disable=unidiomatic-typecheck
 """A module to handle interacting with artifacts."""
 import subprocess
+import shutil
 from dataclasses import dataclass
 from typing import List, Union
 
@@ -88,20 +89,35 @@ def _upload_helm_to_acr(self, artifact_config: HelmPackageConfig) -> None:
         registry_name = registry.replace(".azurecr.io", "")
 
         # az acr login --name "registry_name"
-        login_command = ["az", "acr", "login", "--name", registry_name]
+        # Note that this uses the user running the CLI's AZ login credentials, not the
+        # manifest credentials retrieved from the ACR. This isn't ideal, but using the
+        # manifest credentials caused problems so we are doing this for now.
+        logger.debug("Logging into %s", registry_name)
+        login_command = [
+            str(shutil.which("az")),
+            "acr",
+            "login",
+            "--name",
+            registry_name,
+        ]
         subprocess.run(login_command, check=True)
 
         try:
             logger.debug("Uploading %s to %s", chart_path, target_registry)
 
             # helm push "$chart_path" "$target_registry"
-            push_command = ["helm", "push", chart_path, target_registry]
+            push_command = [
+                str(shutil.which("helm")),
+                "push",
+                chart_path,
+                target_registry,
+            ]
             subprocess.run(push_command, check=True)
         finally:
             # If we don't logout from the registry, future Artifact uploads to this ACR
             # will fail with an UNAUTHORIZED error. There is no az acr logout command,
             # but it is a wrapper around docker, so a call to docker logout will work.
-            logout_command = ["docker", "logout", registry]
+            logout_command = [str(shutil.which("docker")), "logout", registry]
             subprocess.run(logout_command, check=True)
 
     def _upload_to_storage_account(self, artifact_config: ArtifactConfig) -> None:

src/aosm/azext_aosm/deploy/deploy_with_arm.py

@@ -146,9 +146,7 @@ def deploy_nfd_from_bicep(self) -> None:
         print("Done")
 
     def _vnfd_artifact_upload(self) -> None:
-        """
-        Uploads the VHD and ARM template artifacts
-        """
+        """Uploads the VHD and ARM template artifacts."""
         assert isinstance(self.config, VNFConfiguration)
         storage_account_manifest = ArtifactManifestOperator(
             self.config,
@@ -176,9 +174,7 @@ def _vnfd_artifact_upload(self) -> None:
         arm_template_artifact.upload(self.config.arm_template)
 
     def _cnfd_artifact_upload(self) -> None:
-        """
-        Uploads the Helm chart and any additional images.
-        """
+        """Uploads the Helm chart and any additional images."""
         assert isinstance(self.config, CNFConfiguration)
         acr_properties = self.api_clients.aosm_client.artifact_stores.get(
             resource_group_name=self.config.publisher_resource_group_name,
@@ -291,7 +287,9 @@ def parameters(self) -> Dict[str, Any]:
 
     def construct_parameters(self) -> Dict[str, Any]:
         """
-        Create the parmeters dictionary for vnfdefinitions.bicep. VNF specific.
+        Create the parmeters dictionary for vnfdefinitions.bicep.
+
+        VNF specific.
         """
         if self.resource_type == VNF:
             assert isinstance(self.config, VNFConfiguration)
@@ -448,9 +446,7 @@ def deploy_nsd_from_bicep(self) -> None:
             print("Done")
 
     def deploy_manifest_template(self) -> None:
-        """
-        Deploy the bicep template defining the manifest.
-        """
+        """Deploy the bicep template defining the manifest."""
         print("Deploy bicep template for Artifact manifests")
         logger.debug("Deploy manifest bicep")
 

src/aosm/azext_aosm/generate_nfd/cnf_nfd_generator.py

@@ -43,9 +43,7 @@
 
 @dataclass
 class Artifact:
-    """
-    Information about an artifact.
-    """
+    """Information about an artifact."""
 
     name: str
     version: str
@@ -319,8 +317,8 @@ def _copy_to_output_directory(self) -> None:
         """
         Copy files from the temp directory to the output directory.
 
-        Files are the config mappings, schema and bicep templates (artifact manifest
-        and NFDV).
+        Files are the config mappings, schema and bicep templates (artifact manifest and
+        NFDV).
         """
         assert self._tmp_dir
 

src/aosm/azext_aosm/generate_nsd/nf_ret.py

@@ -19,9 +19,7 @@
 
 
 class NFRETGenerator:
-    """
-    Represents a single network function resource element template withing an NSD.
-    """
+    """Represents a single network function resource element template withing an NSD."""
 
     def __init__(
         self, api_clients: ApiClients, config: NFDRETConfiguration, cg_schema_name: str
@@ -63,7 +61,7 @@ def _get_nfdv(
     @property
     def config_mappings(self) -> Dict[str, Any]:
         """
-                Return the contents of the config mapping file for this RET.
+        Return the contents of the config mapping file for this RET.
 
                 Output will look something like:
         {
@@ -129,9 +127,7 @@ def nf_bicep_substitutions(self) -> Dict[str, Any]:
 
     @property
     def config_schema_snippet(self) -> Dict[str, Any]:
-        """
-        Return the CGS snippet for this NF.
-        """
+        """Return the CGS snippet for this NF."""
         nfdv_version_description_string = (
             f"The version of the {self.config.name} "
             "NFD to use.  This version must be compatible with (have the same "

src/aosm/azext_aosm/generate_nsd/nsd_generator.py

@@ -142,9 +142,7 @@ def _write_config_group_schema_json(self, output_directory) -> None:
         logger.debug("%s created", schema_path)
 
     def _write_config_mapping_files(self, output_directory) -> None:
-        """
-        Write out a config mapping file for each NF.
-        """
+        """Write out a config mapping file for each NF."""
         temp_mappings_folder_path = os.path.join(
             output_directory, CONFIG_MAPPINGS_DIR_NAME
         )

src/aosm/azext_aosm/tests/latest/recording_processors.py

@@ -7,9 +7,7 @@
 # the recordings so that we can avoid checking in secrets to the repo.
 # --------------------------------------------------------------------------------------------
 
-from azure.cli.testsdk.scenario_tests import (
-    RecordingProcessor
-)
+from azure.cli.testsdk.scenario_tests import RecordingProcessor
 from azure.cli.testsdk.scenario_tests.utilities import is_text_payload
 import json
 

src/aosm/azext_aosm/tests/latest/test_aosm_cnf_publish_and_delete.py

@@ -53,7 +53,7 @@
 # class CnfNsdTest(ScenarioTest):
 #     @ResourceGroupPreparer()
 #     def test_cnf_nsd_publish_and_delete(self, resource_group):
-#         # We are overriding a resource group name here because we need to have some 
+#         # We are overriding a resource group name here because we need to have some
 #         # resources predeployed in order to get around the timeout bug in the testing framework.
 #         resource_group = "patrykkulik-test"
 

src/aosm/azext_aosm/tests/latest/test_aosm_vnf_publish_and_delete.py

@@ -98,7 +98,7 @@
 #             f'az aosm nfd build -f "{nfd_input_file_path}" --definition-type vnf --force'
 #         )
 
-#         # There is currently a bug in the CLI testing framework that causes the command 
+#         # There is currently a bug in the CLI testing framework that causes the command
 #         # to fail on timeout. This is a workaround to retry the command if it fails.
 #         retry_attempts = 0
 #         while retry_attempts < 2:

src/aosm/azext_aosm/tests/latest/test_cnf.py

@@ -15,9 +15,7 @@
 
 class TestCNF(unittest.TestCase):
     def test_generate_config(self):
-        """
-        Test generating a config file for a VNF.
-        """
+        """Test generating a config file for a VNF."""
         starting_directory = os.getcwd()
         with TemporaryDirectory() as test_dir:
             os.chdir(test_dir)
@@ -29,9 +27,7 @@ def test_generate_config(self):
                 os.chdir(starting_directory)
 
     def test_build(self):
-        """
-        Test the build command for CNFs.
-        """
+        """Test the build command for CNFs."""
         starting_directory = os.getcwd()
         with TemporaryDirectory() as test_dir:
             os.chdir(test_dir)