CP-51836: Restrict/check binary_url of remote_pool repository (#6089)

1. Add an assertion to restrict `binary_url` of remote_pool repository
to be in the format of `https://<coordinator-ip>/repository/enabled`.
2. Add UT for restrict/check `binary_url` of remote_pool repository.

ocaml/tests/test_repository.ml

@@ -21,6 +21,7 @@ let test_introduce_duplicate_repo_name () =
   let name_description_1 = "description1" in
   let binary_url = "https://repo.example.com" in
   let binary_url_1 = "https://repo1.example.com" in
+  let binary_url_2 = "https://1.1.1.1/repository/enabled" in
   let source_url = "https://repo-src.example.com" in
   let source_url_1 = "https://repo-src1.example.com" in
   let gpgkey_path = "" in
@@ -42,6 +43,14 @@ let test_introduce_duplicate_repo_name () =
       Repository.introduce_bundle ~__context ~name_label
         ~name_description:name_description_1
       |> ignore
+    ) ;
+  Alcotest.check_raises "test_introduce_duplicate_repo_name_3"
+    Api_errors.(Server_error (repository_already_exists, [Ref.string_of ref]))
+    (fun () ->
+      Repository.introduce_remote_pool ~__context ~name_label
+        ~name_description:name_description_1 ~binary_url:binary_url_2
+        ~certificate:""
+      |> ignore
     )
 
 let test_introduce_duplicate_binary_url () =
@@ -58,7 +67,7 @@ let test_introduce_duplicate_binary_url () =
     Repository.introduce ~__context ~name_label ~name_description ~binary_url
       ~source_url ~update:true ~gpgkey_path
   in
-  Alcotest.check_raises "test_introduce_duplicate_binary_url"
+  Alcotest.check_raises "test_introduce_duplicate_binary_url_1"
     Api_errors.(Server_error (repository_already_exists, [Ref.string_of ref]))
     (fun () ->
       Repository.introduce ~__context ~binary_url ~name_label:name_label_1
@@ -110,6 +119,59 @@ let test_introduce_duplicate_bundle_repo () =
       |> ignore
     )
 
+let test_introduce_invalid_remote_pool_repo_url () =
+  let __context = T.make_test_database () in
+  let name_label = "name" in
+  let name_description = "description" in
+  let invalid_url_1 = "http://1.1.1.1/repository/enabled" in
+  let invalid_url_2 = "https://1.1.1.257/repository/enabled" in
+  let invalid_url_3 = "https://test.com/repository/enabled" in
+  let invalid_url_4 = "https://1.1.1.1/other" in
+  let invalid_url_5 = "https://1.1.1.1" in
+  let invalid_url_6 = "non-url" in
+  Alcotest.check_raises "test_introduce_invalid_remote_pool_repo_url_1"
+    Api_errors.(Server_error (invalid_base_url, [invalid_url_1]))
+    (fun () ->
+      Repository.introduce_remote_pool ~__context ~name_label ~name_description
+        ~binary_url:invalid_url_1 ~certificate:""
+      |> ignore
+    ) ;
+  Alcotest.check_raises "test_introduce_invalid_remote_pool_repo_url_2"
+    Api_errors.(Server_error (invalid_base_url, [invalid_url_2]))
+    (fun () ->
+      Repository.introduce_remote_pool ~__context ~name_label ~name_description
+        ~binary_url:invalid_url_2 ~certificate:""
+      |> ignore
+    ) ;
+  Alcotest.check_raises "test_introduce_invalid_remote_pool_repo_url_3"
+    Api_errors.(Server_error (invalid_base_url, [invalid_url_3]))
+    (fun () ->
+      Repository.introduce_remote_pool ~__context ~name_label ~name_description
+        ~binary_url:invalid_url_3 ~certificate:""
+      |> ignore
+    ) ;
+  Alcotest.check_raises "test_introduce_invalid_remote_pool_repo_url_4"
+    Api_errors.(Server_error (invalid_base_url, [invalid_url_4]))
+    (fun () ->
+      Repository.introduce_remote_pool ~__context ~name_label ~name_description
+        ~binary_url:invalid_url_4 ~certificate:""
+      |> ignore
+    ) ;
+  Alcotest.check_raises "test_introduce_invalid_remote_pool_repo_url_5"
+    Api_errors.(Server_error (invalid_base_url, [invalid_url_5]))
+    (fun () ->
+      Repository.introduce_remote_pool ~__context ~name_label ~name_description
+        ~binary_url:invalid_url_5 ~certificate:""
+      |> ignore
+    ) ;
+  Alcotest.check_raises "test_introduce_invalid_remote_pool_repo_url_6"
+    Api_errors.(Server_error (invalid_base_url, [invalid_url_6]))
+    (fun () ->
+      Repository.introduce_remote_pool ~__context ~name_label ~name_description
+        ~binary_url:invalid_url_6 ~certificate:""
+      |> ignore
+    )
+
 let test =
   [
     ( "test_introduce_duplicate_repo_name"
@@ -128,6 +190,10 @@ let test =
     , `Quick
     , test_introduce_duplicate_bundle_repo
     )
+  ; ( "test_introduce_invalid_remote_pool_repo_url"
+    , `Quick
+    , test_introduce_invalid_remote_pool_repo_url
+    )
   ]
 
 let () =

ocaml/xapi-consts/constants.ml

@@ -155,6 +155,9 @@ let get_pool_update_download_uri = "/update/"
 
 let get_repository_uri = "/repository" (* ocaml/xapi/repository.ml *)
 
+let get_enabled_repository_uri =
+  "/repository/enabled" (* ocaml/xapi/repository.ml *)
+
 let get_host_updates_uri = "/host_updates" (* ocaml/xapi/repository.ml *)
 
 let get_updates_uri = "/updates" (* ocaml/xapi/repository.ml *)

ocaml/xapi/repository.ml

@@ -69,6 +69,7 @@ let introduce_bundle ~__context ~name_label ~name_description =
 
 let introduce_remote_pool ~__context ~name_label ~name_description ~binary_url
     ~certificate =
+  assert_remote_pool_url_is_valid ~url:binary_url ;
   Db.Repository.get_all ~__context
   |> List.iter (fun ref ->
          if

ocaml/xapi/repository_helpers.ml

@@ -231,6 +231,18 @@ let assert_gpgkey_path_is_valid path =
     raise Api_errors.(Server_error (invalid_gpgkey_path, [path]))
   )
 
+let assert_remote_pool_url_is_valid ~url =
+  let uri = Uri.of_string url in
+  match (Uri.scheme uri, Uri.host uri, Uri.path uri) with
+  | Some "https", Some host, path
+    when path = Constants.get_enabled_repository_uri
+         && Helpers.is_valid_ip `ipv4or6 host ->
+      ()
+  | _ ->
+      error "Invalid url: %s, expected url format: %s" url
+        ("https://<coordinator-ip>" ^ Constants.get_enabled_repository_uri) ;
+      raise Api_errors.(Server_error (invalid_base_url, [url]))
+
 let with_pool_repositories f =
   Xapi_stdext_pervasives.Pervasiveext.finally
     (fun () ->