Check-pr creates and uses SP. (#1504)

* Check-pr creates and uses SP.

* flake8.

* flake8.

* Fixing var name.

* Fixing deploy.py

* Looking for client_secret

* Change to check_output.

* mypy fix.

* Fixing check-pr

* working version.

* lint

* Updating arg text.

* Removing redundant functionality.

* Changing register codepath and adding flag.

* Removing pycache file.

* Fixing unattended flag.

* Adding space.

* Fixing a few calls.

* Removing file.

* Removing python3.

* Removing old file.

* Adding wait into registration.py

* Formatting registration.py.

* Removing space.

* Adding retry logic to check-pr.

* Formatting.

* Retriggering.

* Retriggering.

* Calling sp_create and adding retry to authorize.

* Fixing syntax.

* Removing comments.

* Adding another retry.

* Retriggering.

* Retriggering.

* Retriggering.

* Trying to find error.

* Adding retry logic.

* Increasing sleep.

* Fixing formatting.

* Retriggering.

* Removing bad file.

* Trying out retry for logger.

* typevar issue?

* Re-adding.

* Retriggering.

* retriggering.

* Retriggering.

Co-authored-by: nharper285 <nharper285@gmail.com>
Co-authored-by: Cheick Keita <kcheick@gmail.com>

src/cli/onefuzz/api.py

@@ -1651,6 +1651,7 @@ class Onefuzz:
         self,
         endpoint: Optional[str] = None,
         client_id: Optional[str] = None,
+        client_secret: Optional[str] = None,
         authority: Optional[str] = None,
         tenant_domain: Optional[str] = None,
     ) -> None:
@@ -1661,6 +1662,8 @@ class Onefuzz:
             self._backend.config.authority = authority
         if client_id is not None:
             self._backend.config.client_id = client_id
+        if client_secret is not None:
+            self._backend.config.client_secret = client_secret
         if tenant_domain is not None:
             self._backend.config.tenant_domain = tenant_domain
 

src/deployment/deploylib/registration.py

@@ -131,11 +131,16 @@ def retry(
                 logger.info(f"failed '{description}' missing required resource")
             else:
                 logger.warning(f"failed '{description}': {err.message}")
-
+        except Exception as exc:
+            exception = exc
+            logger.error(f"failed '{description}'. logging stack trace.")
+            logger.error(exc)
         count += 1
         if count >= tries:
             if error:
                 raise error
+            elif exception:
+                raise exception
             else:
                 raise Exception(f"failed '{description}'")
         else:
@@ -270,19 +275,54 @@ def create_application_registration(
         "appId": registered_app["appId"],
     }
 
-    query_microsoft_graph(
+    def try_sp_create() -> None:
-        method="POST",
+        error: Optional[Exception] = None
-        resource="servicePrincipals",
+        for _ in range(10):
-        body=service_principal_params,
+            try:
-        subscription=subscription_id,
+                query_microsoft_graph(
-    )
+                    method="POST",
+                    resource="servicePrincipals",
+                    body=service_principal_params,
+                    subscription=subscription_id,
+                )
+                return
+            except GraphQueryError as err:
+                # work around timing issue when creating service principal
+                # https://github.com/Azure/azure-cli/issues/14767
+                if (
+                    "service principal being created must in the local tenant"
+                    not in str(err)
+                ):
+                    raise err
+            logger.warning(
+                "creating service principal failed with an error that occurs "
+                "due to AAD race conditions"
+            )
+            time.sleep(60)
+        if error is None:
+            raise Exception("service principal creation failed")
+        else:
+            raise error
+
+    try_sp_create()
+
+    registered_app_id = registered_app["appId"]
+    app_id = app["appId"]
+
+    def try_authorize_application(data: Any) -> None:
+        authorize_application(
+            UUID(registered_app_id),
+            UUID(app_id),
+            subscription_id=subscription_id,
+        )
+
+    retry(try_authorize_application, "authorize application")
+
+    def try_assign_instance_role(data: Any) -> None:
+        assign_instance_app_role(onefuzz_instance_name, name, subscription_id, approle)
+
+    retry(try_assign_instance_role, "assingn role")
 
-    authorize_application(
-        UUID(registered_app["appId"]),
-        UUID(app["appId"]),
-        subscription_id=subscription_id,
-    )
-    assign_instance_app_role(onefuzz_instance_name, name, subscription_id, approle)
     return registered_app
 
 
@@ -745,12 +785,12 @@ def main() -> None:
 
     subparsers = parser.add_subparsers(title="commands", dest="command")
     subparsers.add_parser("update_pool_registration", parents=[parent_parser])
-    role_assignment_parser = subparsers.add_parser(
+    scaleset_role_assignment_parser = subparsers.add_parser(
         "assign_scaleset_role",
         parents=[parent_parser],
     )
-    role_assignment_parser.add_argument(
+    scaleset_role_assignment_parser.add_argument(
-        "scaleset_name",
+        "--scaleset_name",
         help="the name of the scaleset",
     )
     cli_registration_parser = subparsers.add_parser(

src/integration-tests/integration-test.py

@@ -22,9 +22,10 @@ import logging
 import os
 import re
 import sys
+import time
 from enum import Enum
 from shutil import which
-from typing import Dict, List, Optional, Set, Tuple
+from typing import Any, Callable, Dict, List, Optional, Set, Tuple, TypeVar
 from uuid import UUID, uuid4
 
 import requests
@@ -220,6 +221,35 @@ TARGETS: Dict[str, Integration] = {
     ),
 }
 
+OperationResult = TypeVar("OperationResult")
+
+def retry(
+    operation: Callable[[Any], OperationResult],
+    description: str,
+    tries: int = 10,
+    wait_duration: int = 10,
+    data: Any = None,
+) -> OperationResult:
+    logger = logging.Logger
+    count = 0
+    while True:
+        try:
+            return operation(data)
+        except Exception as exc:
+            exception = exc
+            logger.error(f"failed '{description}'. logging stack trace.")
+            logger.error(exc)
+        count += 1
+        if count >= tries:
+            if exception:
+                raise exception
+            else:
+                raise Exception(f"failed '{description}'")
+        else:
+            logger.info(
+                f"waiting {wait_duration} seconds before retrying '{description}'"
+            )
+            time.sleep(wait_duration)
 
 class TestOnefuzz:
     def __init__(self, onefuzz: Onefuzz, logger: logging.Logger, test_id: UUID) -> None:
@@ -836,10 +866,14 @@ class Run(Command):
         test_id: UUID,
         *,
         endpoint: Optional[str],
+        client_id: Optional[str],
+        client_secret: Optional[str],
         poll: bool = False,
         stop_on_complete_check: bool = False,
     ) -> None:
-        self.onefuzz.__setup__(endpoint=endpoint)
+        self.onefuzz.__setup__(
+            endpoint=endpoint, client_id=client_id, client_secret=client_secret
+        )
         tester = TestOnefuzz(self.onefuzz, self.logger, test_id)
         result = tester.check_jobs(
             poll=poll, stop_on_complete_check=stop_on_complete_check
@@ -847,8 +881,17 @@ class Run(Command):
         if not result:
             raise Exception("jobs failed")
 
-    def check_repros(self, test_id: UUID, *, endpoint: Optional[str]) -> None:
+    def check_repros(
-        self.onefuzz.__setup__(endpoint=endpoint)
+        self,
+        test_id: UUID,
+        *,
+        endpoint: Optional[str],
+        client_id: Optional[str],
+        client_secret: Optional[str],
+    ) -> None:
+        self.onefuzz.__setup__(
+            endpoint=endpoint, client_id=client_id, client_secret=client_secret
+        )
         tester = TestOnefuzz(self.onefuzz, self.logger, test_id)
         launch_result, repros = tester.launch_repro()
         result = tester.check_repro(repros)
@@ -860,6 +903,8 @@ class Run(Command):
         samples: Directory,
         *,
         endpoint: Optional[str] = None,
+        client_id: Optional[str] = None,
+        client_secret: Optional[str] = None,
         pool_size: int = 10,
         region: Optional[Region] = None,
         os_list: List[OS] = [OS.linux, OS.windows],
@@ -871,19 +916,43 @@ class Run(Command):
             test_id = uuid4()
         self.logger.info("launching test_id: %s", test_id)
         
-        self.onefuzz.__setup__(endpoint=endpoint)
+        def try_setup(data: Any) -> None:
+            self.onefuzz.__setup__(
+                endpoint=endpoint, client_id=client_id, client_secret=client_secret
+            )
+
+        retry(try_setup, "trying to configure")
+
         tester = TestOnefuzz(self.onefuzz, self.logger, test_id)
         tester.setup(region=region, pool_size=pool_size, os_list=os_list)
         tester.launch(samples, os_list=os_list, targets=targets, duration=duration)
         return test_id
 
-    def cleanup(self, test_id: UUID, *, endpoint: Optional[str]) -> None:
+    def cleanup(
-        self.onefuzz.__setup__(endpoint=endpoint)
+        self,
+        test_id: UUID,
+        *,
+        endpoint: Optional[str],
+        client_id: Optional[str],
+        client_secret: Optional[str],
+    ) -> None:
+        self.onefuzz.__setup__(
+            endpoint=endpoint, client_id=client_id, client_secret=client_secret
+        )
         tester = TestOnefuzz(self.onefuzz, self.logger, test_id=test_id)
         tester.cleanup()
 
-    def check_logs(self, test_id: UUID, *, endpoint: Optional[str]) -> None:
+    def check_logs(
-        self.onefuzz.__setup__(endpoint=endpoint)
+        self,
+        test_id: UUID,
+        *,
+        endpoint: Optional[str],
+        client_id: Optional[str],
+        client_secret: Optional[str],
+    ) -> None:
+        self.onefuzz.__setup__(
+            endpoint=endpoint, client_id=client_id, client_secret=client_secret
+        )
         tester = TestOnefuzz(self.onefuzz, self.logger, test_id=test_id)
         tester.check_logs_for_errors()
 
@@ -892,6 +961,8 @@ class Run(Command):
         samples: Directory,
         *,
         endpoint: Optional[str] = None,
+        client_id: Optional[str] = None,
+        client_secret: Optional[str] = None,
         pool_size: int = 15,
         region: Optional[Region] = None,
         os_list: List[OS] = [OS.linux, OS.windows],
@@ -907,6 +978,8 @@ class Run(Command):
             self.launch(
                 samples,
                 endpoint=endpoint,
+                client_id=client_id,
+                client_secret=client_secret,
                 pool_size=pool_size,
                 region=region,
                 os_list=os_list,
@@ -915,15 +988,30 @@ class Run(Command):
                 duration=duration,
             )
             self.check_jobs(
-                test_id, endpoint=endpoint, poll=True, stop_on_complete_check=True
+                test_id,
+                endpoint=endpoint,
+                client_id=client_id,
+                client_secret=client_secret,
+                poll=True,
+                stop_on_complete_check=True,
             )
 
             if skip_repro:
                 self.logger.warning("not testing crash repro")
             else:
-                self.check_repros(test_id, endpoint=endpoint)
+                self.check_repros(
+                    test_id,
+                    endpoint=endpoint,
+                    client_id=client_id,
+                    client_secret=client_secret,
+                )
 
-            self.check_logs(test_id, endpoint=endpoint)
+            self.check_logs(
+                test_id,
+                endpoint=endpoint,
+                client_id=client_id,
+                client_secret=client_secret,
+            )
 
         except Exception as e:
             self.logger.error("testing failed: %s", repr(e))
@@ -934,7 +1022,12 @@ class Run(Command):
             success = False
 
         try:
-            self.cleanup(test_id, endpoint=endpoint)
+            self.cleanup(
+                test_id,
+                endpoint=endpoint,
+                client_id=client_id,
+                client_secret=client_secret,
+            )
         except Exception as e:
             self.logger.error("testing failed: %s", repr(e))
             error = e

src/utils/check-pr/check-pr.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 #
 # Copyright (c) Microsoft Corporation.
 # Licensed under the MIT License.
@@ -22,7 +22,7 @@ A = TypeVar("A")
 
 def wait(func: Callable[[], Tuple[bool, str, A]], frequency: float = 1.0) -> A:
     """
-    Wait until the provided func returns True
+    Wait until the provided func returns True.
 
     Provides user feedback via a spinner if stdout is a TTY.
     """
@@ -170,6 +170,7 @@ class Deployer:
         skip_tests: bool,
         test_args: List[str],
         repo: str,
+        unattended: bool,
     ):
         self.downloader = Downloader()
         self.pr = pr
@@ -180,6 +181,9 @@ class Deployer:
         self.skip_tests = skip_tests
         self.test_args = test_args or []
         self.repo = repo
+        self.unattended = unattended
+        self.client_id = ""
+        self.client_secret = ""
 
     def merge(self) -> None:
         if self.pr:
@@ -188,9 +192,9 @@ class Deployer:
     def deploy(self, filename: str) -> None:
         print(f"deploying {filename} to {self.instance}")
         venv = "deploy-venv"
-        subprocess.check_call(f"python -mvenv {venv}", shell=True)
+        subprocess.check_call(f"python3 -mvenv {venv}", shell=True)
         pip = venv_path(venv, "pip")
-        py = venv_path(venv, "python")
+        py = venv_path(venv, "python3")
         config = os.path.join(os.getcwd(), "config.json")
         commands = [
             ("extracting release-artifacts", f"unzip -qq {filename}"),
@@ -198,7 +202,7 @@ class Deployer:
             ("installing wheel", f"{pip} install -q wheel"),
             ("installing prereqs", f"{pip} install -q -r requirements.txt"),
             (
-                "running deployment",
+                "running deploment",
                 (
                     f"{py} deploy.py {self.region} "
                     f"{self.instance} {self.instance} cicd {config}"
@@ -210,20 +214,71 @@ class Deployer:
             print(msg)
             subprocess.check_call(cmd, shell=True)
 
+        if self.unattended:
+            self.register()
+
+    def register(self) -> None:
+        sp_name = "sp_" + self.instance
+        print(f"registering {sp_name} to {self.instance}")
+
+        venv = "deploy-venv"
+        pip = venv_path(venv, "pip")
+        py = venv_path(venv, "python3")
+
+        az_cmd = ["az", "account", "show", "--query", "id", "-o", "tsv"]
+        subscription_id = subprocess.check_output(az_cmd, encoding="UTF-8")
+        subscription_id = subscription_id.strip()
+
+        commands = [
+            ("installing prereqs", f"{pip} install -q -r requirements.txt"),
+            (
+                "running cli registration",
+                (
+                    f"{py} ./deploylib/registration.py create_cli_registration "
+                    f"{self.instance} {subscription_id}"
+                    f" --registration_name {sp_name}"
+                ),
+            ),
+        ]
+
+        for (msg, cmd) in commands:
+            print(msg)
+            output = subprocess.check_output(cmd, shell=True, encoding="UTF-8")
+            if "client_id" in output:
+                output_list = output.split("\n")
+                for line in output_list:
+                    if "client_id" in line:
+                        line_list = line.split(":")
+                        client_id = line_list[1].strip()
+                        self.client_id = client_id
+                        print(("client_id: " + client_id))
+                    if "client_secret" in line:
+                        line_list = line.split(":")
+                        client_secret = line_list[1].strip()
+                        self.client_secret = client_secret
+        time.sleep(30)
+        return
+
     def test(self, filename: str) -> None:
         venv = "test-venv"
-        subprocess.check_call(f"python -mvenv {venv}", shell=True)
+        subprocess.check_call(f"python3 -mvenv {venv}", shell=True)
-        py = venv_path(venv, "python")
+        py = venv_path(venv, "python3")
         test_dir = "integration-test-artifacts"
         script = "integration-test.py"
         endpoint = f"https://{self.instance}.azurewebsites.net"
         test_args = " ".join(self.test_args)
+        unattended_args = (
+            f"--client_id {self.client_id} --client_secret {self.client_secret}"
+            if self.unattended
+            else ""
+        )
+
         commands = [
             (
                 "extracting integration-test-artifacts",
                 f"unzip -qq {filename} -d {test_dir}",
             ),
-            ("test venv", f"python -mvenv {venv}"),
+            ("test venv", f"python3 -mvenv {venv}"),
             ("installing wheel", f"./{venv}/bin/pip install -q wheel"),
             ("installing sdk", f"./{venv}/bin/pip install -q sdk/*.whl"),
             (
@@ -231,7 +286,7 @@ class Deployer:
                 (
                     f"{py} {test_dir}/{script} test {test_dir} "
                     f"--region {self.region} --endpoint {endpoint} "
-                    f"{test_args}"
+                    f"{unattended_args} {test_args}"
                 ),
             ),
         ]
@@ -274,6 +329,7 @@ class Deployer:
         )
 
         self.deploy(release_filename)
+
         if not self.skip_tests:
             self.test(test_filename)
 
@@ -306,6 +362,7 @@ def main() -> None:
     parser.add_argument("--merge-on-success", action="store_true")
     parser.add_argument("--subscription_id")
     parser.add_argument("--test_args", nargs=argparse.REMAINDER)
+    parser.add_argument("--unattended", action="store_true")
     args = parser.parse_args()
 
     if not args.branch and not args.pr:
@@ -320,6 +377,7 @@ def main() -> None:
         skip_tests=args.skip_tests,
         test_args=args.test_args,
         repo=args.repo,
+        unattended=args.unattended,
     )
     with tempfile.TemporaryDirectory() as directory:
         os.chdir(directory)