Merge pull request Azure#72 from jddarby/sunny/credentials-for-cnf-im…

…age-upload

Optional use of listcredentials for uploading CNF images to Artifact Store

src/aosm/HISTORY.rst

@@ -27,6 +27,13 @@ upcoming
 * Re-order publish steps so that artifacts are uploaded before the NFD/NSD is published.
 * Add progress information for VHD upload
 * Change optional argument from `manifest_parameters_json_file` to `manifest_params_file` to appease linter.
+* NB CHANGE TO PREVIOUS CONFIG FILE FORMAT FOR NFDS
+  - Add options for CNF image upload. By default CNF images are copied from a source ACR using `az acr import` which is fast but requires subscription-wide permissions.
+  - If permissions are not available then CNF images can be copies from the source ACR using `docker pull` then `docker push`, which is slower and requires Docker to be installed. This is governed by a new --no-subscription-permissions flag.
+  - Also, if only a single image is required, it can be specified in the config file and uploaded from local docker registry using `docker push`
+  - CNF image config has been moved into an `images` section of the config file. Please run `az aosm nfd generate-config --definition-type cnf` to generate a new config file.
+  - Remove pre-deploy check to check source ACR exists. This will be found at the time that images are copied / accessed.
+  - Change from using ContainerRegistryManagementClient to `az acr import` subprocess call, so that we don't need to know the Resource Group.
 
 0.2.0
 ++++++

src/aosm/azext_aosm/_configuration.py

@@ -7,7 +7,6 @@
 import logging
 import json
 import os
-import re
 from dataclasses import dataclass, field
 from pathlib import Path
 from typing import Any, Dict, List, Optional, Union
@@ -20,7 +19,6 @@
     NSD,
     NSD_OUTPUT_BICEP_PREFIX,
     VNF,
-    SOURCE_ACR_REGEX,
 )
 
 logger = logging.getLogger(__name__)
@@ -87,13 +85,21 @@
         "The parameter name in the VM ARM template which specifies the name of the "
         "image to use for the VM."
     ),
-    "source_registry_id": (
-        "Resource ID of the source acr registry from which to pull the image."
+    "source_registry": (
+        "Optional. Login server of the source acr registry from which to pull the "
+        "image(s). For example sourceacr.azurecr.io. Leave blank if you have set "
+        "source_local_docker_image."
+    ),
+    "source_local_docker_image": (
+        "Optional. Image name of the source docker image from local machine. For "
+        "limited use case where the CNF only requires a single docker image and exists "
+        "in the local docker repository. Set to blank of not required."
     ),
     "source_registry_namespace": (
         "Optional. Namespace of the repository of the source acr registry from which "
         "to pull. For example if your repository is samples/prod/nginx then set this to"
-        " samples/prod . Leave blank if the image is in the root namespace."
+        " samples/prod . Leave blank if the image is in the root namespace or you have "
+        "set source_local_docker_image."
         "See https://learn.microsoft.com/en-us/azure/container-registry/"
         "container-registry-best-practices#repository-namespaces for further details."
     ),
@@ -280,9 +286,17 @@ class HelmPackageConfig:
 
 
 @dataclass
-class CNFConfiguration(NFConfiguration):
-    source_registry_id: str = DESCRIPTION_MAP["source_registry_id"]
+class CNFImageConfig:
+    """CNF Image config settings."""
+
+    source_registry: str = DESCRIPTION_MAP["source_registry"]
     source_registry_namespace: str = DESCRIPTION_MAP["source_registry_namespace"]
+    source_local_docker_image: str = DESCRIPTION_MAP["source_local_docker_image"]
+
+
+@dataclass
+class CNFConfiguration(NFConfiguration):
+    images: Any = CNFImageConfig()
     helm_packages: List[Any] = field(default_factory=lambda: [HelmPackageConfig()])
 
     def __post_init__(self):
@@ -300,6 +314,9 @@ def __post_init__(self):
                     package["path_to_mappings"]
                 )
                 self.helm_packages[package_index] = HelmPackageConfig(**dict(package))
+        if isinstance(self.images, dict):
+            self.images = CNFImageConfig(**self.images)
+            self.validate()
 
     @property
     def output_directory_for_build(self) -> Path:
@@ -312,15 +329,32 @@ def validate(self):
 
         :raises ValidationError: If source registry ID doesn't match the regex
         """
-        if self.source_registry_id == DESCRIPTION_MAP["source_registry_id"]:
-            # Config has not been filled in. Don't validate.
-            return
+        source_reg_set = (
+            self.images.source_registry
+            and self.images.source_registry != DESCRIPTION_MAP["source_registry"]
+        )
+        source_local_set = (
+            self.images.source_local_docker_image
+            and self.images.source_local_docker_image
+            != DESCRIPTION_MAP["source_local_docker_image"]
+        )
+        source_reg_namespace_set = (
+            self.images.source_registry_namespace
+            and self.images.source_registry_namespace
+            != DESCRIPTION_MAP["source_registry_namespace"]
+        )
+
+        # If these are the same, either neither is set or both are, both of which are errors
+        if source_reg_set == source_local_set:
+            raise ValidationError(
+                "Config validation error. Images config must have either a local docker image"
+                " or a source registry, but not both."
+            )
 
-        source_registry_match = re.search(SOURCE_ACR_REGEX, self.source_registry_id)
-        if not source_registry_match or len(source_registry_match.groups()) < 2:
+        if source_reg_namespace_set and not source_reg_set:
             raise ValidationError(
-                "CNF config has an invalid source registry ID. Please run `az aosm "
-                "nfd generate-config` to see the valid formats."
+                "Config validation error. The image source registry namespace should "
+                "only be configured if a source registry is configured."
             )
 
 
@@ -343,7 +377,7 @@ def validate(self):
 
 
 @dataclass
-class NFDRETConfiguration:
+class NFDRETConfiguration:  # pylint: disable=too-many-instance-attributes
     """The configuration required for an NFDV that you want to include in an NSDV."""
 
     publisher: str = PUBLISHER_NAME
@@ -528,22 +562,31 @@ def get_configuration(
     :return: The configuration object
     """
     if config_file:
-        with open(config_file, "r", encoding="utf-8") as f:
-            config_as_dict = json.loads(f.read())
+        try:
+            with open(config_file, "r", encoding="utf-8") as f:
+                config_as_dict = json.loads(f.read())
+        except json.decoder.JSONDecodeError as e:
+            raise InvalidArgumentValueError(
+                f"Config file {config_file} is not valid JSON: {e}"
+            ) from e
     else:
         config_as_dict = {}
 
     config: Configuration
-
-    if configuration_type == VNF:
-        config = VNFConfiguration(config_file=config_file, **config_as_dict)
-    elif configuration_type == CNF:
-        config = CNFConfiguration(config_file=config_file, **config_as_dict)
-    elif configuration_type == NSD:
-        config = NSConfiguration(config_file=config_file, **config_as_dict)
-    else:
+    try:
+        if configuration_type == VNF:
+            config = VNFConfiguration(config_file=config_file, **config_as_dict)
+        elif configuration_type == CNF:
+            config = CNFConfiguration(config_file=config_file, **config_as_dict)
+        elif configuration_type == NSD:
+            config = NSConfiguration(config_file=config_file, **config_as_dict)
+        else:
+            raise InvalidArgumentValueError(
+                "Definition type not recognized, options are: vnf, cnf or nsd"
+            )
+    except TypeError as typeerr:
         raise InvalidArgumentValueError(
-            "Definition type not recognized, options are: vnf, cnf or nsd"
-        )
+            f"Config file {config_file} is not valid: {typeerr}"
+        ) from typeerr
 
     return config

src/aosm/azext_aosm/_params.py

@@ -121,6 +121,21 @@ def load_arguments(self: AzCommandsLoader, _):
                 "container images (for CNFs)."
             ),
         )
+        c.argument(
+            "no_subscription_permissions",
+            options_list=["--no-subscription-permissions", "-u"],
+            arg_type=get_three_state_flag(),
+            help=(
+                "CNF definition_type publish only - ignored for VNF."
+                " Set to True if you do not "
+                "have permission to import to the Publisher subscription (Contributor "
+                "role + AcrPush role, or a custom role that allows the importImage "
+                "action and AcrPush over the "
+                "whole subscription). This means that the image artifacts will be "
+                "pulled to your local machine and then pushed to the Artifact Store. "
+                "Requires Docker to be installed locally."
+            ),
+        )
 
     with self.argument_context("aosm nsd") as c:
         c.argument(

src/aosm/azext_aosm/custom.py

@@ -19,7 +19,7 @@
 from azure.core import exceptions as azure_exceptions
 from knack.log import get_logger
 
-from azext_aosm._client_factory import cf_acr_registries, cf_features, cf_resources
+from azext_aosm._client_factory import cf_features, cf_resources
 from azext_aosm._configuration import (
     CNFConfiguration,
     Configuration,
@@ -202,6 +202,7 @@ def publish_definition(
     manifest_file: Optional[str] = None,
     manifest_params_file: Optional[str] = None,
     skip: Optional[SkipSteps] = None,
+    no_subscription_permissions: bool = False,
 ):
     """
     Publish a generated definition.
@@ -217,16 +218,22 @@ def publish_definition(
     :param definition_type: VNF or CNF
     :param config_file: Path to the config file for the NFDV
     :param definition_file: Optional path to a bicep template to deploy, in case the
-        user wants to edit the built NFDV template.
-        If omitted, the default built NFDV template will be used.
+        user wants to edit the built NFDV template. If omitted, the default built NFDV
+        template will be used.
     :param parameters_json_file: Optional path to a parameters file for the bicep file,
-        in case the user wants to edit the built NFDV template. If omitted,
-        parameters from config will be turned into parameters for the bicep file
+        in case the user wants to edit the built NFDV template. If omitted, parameters
+        from config will be turned into parameters for the bicep file
     :param manifest_file: Optional path to an override bicep template to deploy
         manifests
-    :param manifest_params_file: Optional path to an override bicep parameters
-        file for manifest parameters
+    :param manifest_params_file: Optional path to an override bicep parameters file for
+        manifest parameters
     :param skip: options to skip, either publish bicep or upload artifacts
+    :param no_subscription_permissions:
+            CNF definition_type publish only - ignored for VNF. Causes the image
+            artifact copy from a source ACR to be done via docker pull and push,
+            rather than `az acr import`. This is slower but does not require
+            Contributor (or importImage action) and AcrPush permissions on the publisher
+            subscription. It requires Docker to be installed.
     """
     # Check that the required features are enabled on the subscription
     _check_features_enabled(cmd)
@@ -235,7 +242,6 @@ def publish_definition(
     api_clients = ApiClients(
         aosm_client=client,
         resource_client=cf_resources(cmd.cli_ctx),
-        container_registry_client=cf_acr_registries(cmd.cli_ctx),
     )
 
     if definition_type not in (VNF, CNF):
@@ -258,6 +264,7 @@ def publish_definition(
         manifest_params_file=manifest_params_file,
         skip=skip,
         cli_ctx=cmd.cli_ctx,
+        use_manifest_permissions=no_subscription_permissions,
     )
     deployer.deploy_nfd_from_bicep()