From 62201478aa44921d3a596f469432211c9adbb38a Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Mon, 15 Jul 2024 10:48:33 +0800 Subject: [PATCH 01/14] CP-49212: Update datamodel for non-CDN update Add a field "origin" in "repository" to indicate the origin of the repository. It's an enum type with 2 values: "remote" and "bundle". Signed-off-by: Bengang Yuan --- ocaml/idl/datamodel_repository.ml | 38 +++++++++++++++++++++++-- ocaml/idl/schematest.ml | 2 +- ocaml/xapi-cli-server/cli_frontend.ml | 11 ++++++- ocaml/xapi-cli-server/cli_operations.ml | 15 ++++++++-- ocaml/xapi-cli-server/record_util.ml | 2 ++ ocaml/xapi-cli-server/records.ml | 5 ++++ ocaml/xapi/message_forwarding.ml | 6 ++++ ocaml/xapi/repository.ml | 35 +++++++++++++++++++---- ocaml/xapi/repository.mli | 6 ++++ ocaml/xapi/repository_helpers.ml | 16 +++++++---- ocaml/xapi/xapi_globs.ml | 4 +++ 11 files changed, 123 insertions(+), 17 deletions(-) diff --git a/ocaml/idl/datamodel_repository.ml b/ocaml/idl/datamodel_repository.ml index 02b17f509e0..114242d913f 100644 --- a/ocaml/idl/datamodel_repository.ml +++ b/ocaml/idl/datamodel_repository.ml @@ -18,10 +18,19 @@ open Datamodel_roles let lifecycle = [(Lifecycle.Published, "1.301.0", "")] +let origin = + Enum + ( "origin" + , [ + ("remote", "The origin of the repository is a remote one") + ; ("bundle", "The origin of the repository is a local bundle file") + ] + ) + let introduce = call ~name:"introduce" ~in_oss_since:None ~lifecycle:[(Published, "1.301.0", "")] - ~doc:"Add the configuration for a new repository" + ~doc:"Add the configuration for a new remote repository" ~versioned_params: [ { @@ -73,6 +82,18 @@ let introduce = ~allowed_roles:(_R_POOL_OP ++ _R_CLIENT_CERT) () +let introduce_bundle = + call ~name:"introduce_bundle" ~in_oss_since:None ~lifecycle:[] + ~doc:"Add the configuration for a new bundle repository" + ~params: + [ + (String, "name_label", "The name of the repository") + ; (String, "name_description", "The description of the repository") + ] + ~result:(Ref _repository, "The ref of the created repository record.") + ~allowed_roles:(_R_POOL_OP ++ _R_CLIENT_CERT) + () + let forget = call ~name:"forget" ~in_oss_since:None ~lifecycle:[(Published, "1.301.0", "")] @@ -148,7 +169,15 @@ let t = ~lifecycle:[(Published, "1.301.0", "")] ~persist:PersistEverything ~in_oss_since:None ~messages_default_allowed_roles:(_R_POOL_OP ++ _R_CLIENT_CERT) - ~messages:[introduce; forget; apply; set_gpgkey_path; apply_livepatch] + ~messages: + [ + introduce + ; introduce_bundle + ; forget + ; apply + ; set_gpgkey_path + ; apply_livepatch + ] ~contents: [ uid _repository ~lifecycle:[(Published, "1.301.0", "")] @@ -187,5 +216,10 @@ let t = ; field ~qualifier:StaticRO ~lifecycle:[] ~ty:String ~default_value:(Some (VString "")) "gpgkey_path" "The file name of the GPG public key of this repository" + ; field ~qualifier:StaticRO ~lifecycle:[] ~ty:origin "origin" + ~default_value:(Some (VEnum "remote")) + "The origin of the repository. 'remote' if the origin of the \ + repository is a remote one, 'bundle' if the origin of the \ + repository is a local bundle file." ] () diff --git a/ocaml/idl/schematest.ml b/ocaml/idl/schematest.ml index 4ba16fbfe1c..6091444dcc9 100644 --- a/ocaml/idl/schematest.ml +++ b/ocaml/idl/schematest.ml @@ -3,7 +3,7 @@ let hash x = Digest.string x |> Digest.to_hex (* BEWARE: if this changes, check that schema has been bumped accordingly in ocaml/idl/datamodel_common.ml, usually schema_minor_vsn *) -let last_known_schema_hash = "7885f7b085e4a5e32977a4b222030412" +let last_known_schema_hash = "2a6baa01032827a321845b264c6aaae4" let current_schema_hash : string = let open Datamodel_types in diff --git a/ocaml/xapi-cli-server/cli_frontend.ml b/ocaml/xapi-cli-server/cli_frontend.ml index 1c71177c3c8..e735d4793ca 100644 --- a/ocaml/xapi-cli-server/cli_frontend.ml +++ b/ocaml/xapi-cli-server/cli_frontend.ml @@ -3659,11 +3659,20 @@ let rec cmdtable_data : (string * cmd_spec) list = , { reqd= ["name-label"; "binary-url"; "source-url"; "update"] ; optn= ["name-description"; "gpgkey-path"] - ; help= "Add the configuration for a new repository." + ; help= "Add the configuration for a new remote repository." ; implementation= No_fd Cli_operations.Repository.introduce ; flags= [] } ) + ; ( "repository-introduce-bundle" + , { + reqd= ["name-label"] + ; optn= ["name-description"] + ; help= "Add the configuration for a new bundle repository." + ; implementation= No_fd Cli_operations.Repository.introduce_bundle + ; flags= [] + } + ) ; ( "repository-forget" , { reqd= ["uuid"] diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index 83f10a7a46d..93d0c69f41f 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -1318,6 +1318,7 @@ let gen_cmds rpc session_id = ; "hash" ; "up-to-date" ; "gpgkey-path" + ; "origin" ] rpc session_id ) @@ -7934,9 +7935,7 @@ end module Repository = struct let introduce printer rpc session_id params = let name_label = List.assoc "name-label" params in - let name_description = - try List.assoc "name-description" params with Not_found -> "" - in + let name_description = get_param params "name-description" ~default:"" in let binary_url = List.assoc "binary-url" params in let source_url = List.assoc "source-url" params in let update = get_bool_param params "update" in @@ -7948,6 +7947,16 @@ module Repository = struct let uuid = Client.Repository.get_uuid ~rpc ~session_id ~self:ref in printer (Cli_printer.PList [uuid]) + let introduce_bundle printer rpc session_id params = + let name_label = List.assoc "name-label" params in + let name_description = get_param params "name-description" ~default:"" in + let ref = + Client.Repository.introduce_bundle ~rpc ~session_id ~name_label + ~name_description + in + let uuid = Client.Repository.get_uuid ~rpc ~session_id ~self:ref in + printer (Cli_printer.PList [uuid]) + let forget _printer rpc session_id params = let ref = Client.Repository.get_by_uuid ~rpc ~session_id diff --git a/ocaml/xapi-cli-server/record_util.ml b/ocaml/xapi-cli-server/record_util.ml index 105615fedfd..8e0bb1aaef2 100644 --- a/ocaml/xapi-cli-server/record_util.ml +++ b/ocaml/xapi-cli-server/record_util.ml @@ -1145,3 +1145,5 @@ let vm_placement_policy_of_string a = `anti_affinity | s -> record_failure "Invalid VM placement policy, got %s" s + +let repo_origin_to_string = function `remote -> "remote" | `bundle -> "bundle" diff --git a/ocaml/xapi-cli-server/records.ml b/ocaml/xapi-cli-server/records.ml index abcd5f3fb1c..f75809bf592 100644 --- a/ocaml/xapi-cli-server/records.ml +++ b/ocaml/xapi-cli-server/records.ml @@ -5370,6 +5370,11 @@ let repository_record rpc session_id repository = ~value:x ) () + ; make_field ~name:"origin" + ~get:(fun () -> + Record_util.repo_origin_to_string (x ()).API.repository_origin + ) + () ] } diff --git a/ocaml/xapi/message_forwarding.ml b/ocaml/xapi/message_forwarding.ml index 34e420259b8..716274759ee 100644 --- a/ocaml/xapi/message_forwarding.ml +++ b/ocaml/xapi/message_forwarding.ml @@ -6560,6 +6560,12 @@ functor Local.Repository.introduce ~__context ~name_label ~name_description ~binary_url ~source_url ~update ~gpgkey_path + let introduce_bundle ~__context ~name_label ~name_description = + info "Repository.introduce_bundle: name = '%s'; name_description = '%s'" + name_label name_description ; + Local.Repository.introduce_bundle ~__context ~name_label + ~name_description + let forget ~__context ~self = info "Repository.forget: self = '%s'" (repository_uuid ~__context self) ; Local.Repository.forget ~__context ~self diff --git a/ocaml/xapi/repository.ml b/ocaml/xapi/repository.ml index bd63984c0a1..b3c2266d8ae 100644 --- a/ocaml/xapi/repository.ml +++ b/ocaml/xapi/repository.ml @@ -44,7 +44,22 @@ let introduce ~__context ~name_label ~name_description ~binary_url ~source_url ) ) ; create_repository_record ~__context ~name_label ~name_description ~binary_url - ~source_url ~update ~gpgkey_path + ~source_url ~update ~gpgkey_path ~origin:`remote + +let introduce_bundle ~__context ~name_label ~name_description = + Db.Repository.get_all ~__context + |> List.iter (fun ref -> + if + name_label = Db.Repository.get_name_label ~__context ~self:ref + || Db.Repository.get_origin ~__context ~self:ref = `bundle + then + raise + Api_errors.( + Server_error (repository_already_exists, [Ref.string_of ref]) + ) + ) ; + create_repository_record ~__context ~name_label ~name_description + ~binary_url:"" ~source_url:"" ~update:true ~gpgkey_path:"" ~origin:`bundle let forget ~__context ~self = let pool = Helpers.get_pool ~__context in @@ -112,8 +127,18 @@ let sync ~__context ~self ~token ~token_id = try let repo_name = get_remote_repository_name ~__context ~self in remove_repo_conf_file repo_name ; - let binary_url = Db.Repository.get_binary_url ~__context ~self in - let source_url = Db.Repository.get_source_url ~__context ~self in + let binary_url, source_url = + match Db.Repository.get_origin ~__context ~self with + | `remote -> + ( Db.Repository.get_binary_url ~__context ~self + , Some (Db.Repository.get_source_url ~__context ~self) + ) + | `bundle -> + let uri = + Uri.make ~scheme:"file" ~path:!Xapi_globs.bundle_repository_dir () + in + (Uri.to_string uri, None) + in let gpgkey_path = match Db.Repository.get_gpgkey_path ~__context ~self with | "" -> @@ -122,8 +147,8 @@ let sync ~__context ~self ~token ~token_id = s in let write_initial_yum_config () = - write_yum_config ~source_url:(Some source_url) ~binary_url - ~repo_gpgcheck:true ~gpgkey_path ~repo_name + write_yum_config ~source_url ~binary_url ~repo_gpgcheck:true ~gpgkey_path + ~repo_name in write_initial_yum_config () ; clean_yum_cache repo_name ; diff --git a/ocaml/xapi/repository.mli b/ocaml/xapi/repository.mli index 8b8ee7e09cd..e7bddad8bad 100644 --- a/ocaml/xapi/repository.mli +++ b/ocaml/xapi/repository.mli @@ -22,6 +22,12 @@ val introduce : -> gpgkey_path:string -> [`Repository] API.Ref.t +val introduce_bundle : + __context:Context.t + -> name_label:string + -> name_description:string + -> [`Repository] API.Ref.t + val forget : __context:Context.t -> self:[`Repository] API.Ref.t -> unit val cleanup_all_pool_repositories : unit -> unit diff --git a/ocaml/xapi/repository_helpers.ml b/ocaml/xapi/repository_helpers.ml index f67e8647822..71bf53ed1e3 100644 --- a/ocaml/xapi/repository_helpers.ml +++ b/ocaml/xapi/repository_helpers.ml @@ -134,11 +134,12 @@ module GuidanceSet = struct end let create_repository_record ~__context ~name_label ~name_description - ~binary_url ~source_url ~update ~gpgkey_path = + ~binary_url ~source_url ~update ~gpgkey_path ~origin = let ref = Ref.make () in let uuid = Uuidx.(to_string (make ())) in Db.Repository.create ~__context ~ref ~uuid ~name_label ~name_description - ~binary_url ~source_url ~update ~hash:"" ~up_to_date:false ~gpgkey_path ; + ~binary_url ~source_url ~update ~hash:"" ~up_to_date:false ~gpgkey_path + ~origin ; ref module DomainNameIncludeIP = struct @@ -377,9 +378,14 @@ let get_repository_name ~__context ~self = Db.Repository.get_uuid ~__context ~self let get_remote_repository_name ~__context ~self = - !Xapi_globs.remote_repository_prefix - ^ "-" - ^ get_repository_name ~__context ~self + let prefix = + match Db.Repository.get_origin ~__context ~self with + | `remote -> + !Xapi_globs.remote_repository_prefix + | `bundle -> + !Xapi_globs.bundle_repository_prefix + in + prefix ^ "-" ^ get_repository_name ~__context ~self let get_local_repository_name ~__context ~self = !Xapi_globs.local_repository_prefix diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index 1e03882ead1..a21226cf075 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -925,6 +925,8 @@ let yum_repos_config_dir = ref "/etc/yum.repos.d" let remote_repository_prefix = ref "remote" +let bundle_repository_prefix = ref "bundle" + let local_repository_prefix = ref "local" let yum_config_manager_cmd = ref "/usr/bin/yum-config-manager" @@ -945,6 +947,8 @@ let repository_gpgkey_name = ref "" let repository_gpgcheck = ref true +let bundle_repository_dir = ref "/var/xapi/bundle-repo" + let observer_config_dir = "/etc/xensource/observer" let ignore_vtpm_unimplemented = ref false From 4824a91fe311a00ea40f55f29e2eb3daae4759f3 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Sat, 13 Jul 2024 11:33:44 +0800 Subject: [PATCH 02/14] CP-49212: Add UT for update datamodel for non-CDN update Signed-off-by: Bengang Yuan --- ocaml/tests/test_repository.ml | 40 ++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/ocaml/tests/test_repository.ml b/ocaml/tests/test_repository.ml index e30cf71362a..860dc63a950 100644 --- a/ocaml/tests/test_repository.ml +++ b/ocaml/tests/test_repository.ml @@ -14,7 +14,7 @@ module T = Test_common -let test_introduce_duplicate_name () = +let test_introduce_duplicate_repo_name () = let __context = T.make_test_database () in let name_label = "name" in let name_description = "description" in @@ -28,13 +28,20 @@ let test_introduce_duplicate_name () = Repository.introduce ~__context ~name_label ~name_description ~binary_url ~source_url ~update:true ~gpgkey_path in - Alcotest.check_raises "test_introduce_duplicate_name" + Alcotest.check_raises "test_introduce_duplicate_repo_name_1" Api_errors.(Server_error (repository_already_exists, [Ref.string_of ref])) (fun () -> Repository.introduce ~__context ~name_label ~name_description:name_description_1 ~binary_url:binary_url_1 ~source_url:source_url_1 ~update:true ~gpgkey_path |> ignore + ) ; + Alcotest.check_raises "test_introduce_duplicate_repo_name_2" + Api_errors.(Server_error (repository_already_exists, [Ref.string_of ref])) + (fun () -> + Repository.introduce_bundle ~__context ~name_label + ~name_description:name_description_1 + |> ignore ) let test_introduce_duplicate_binary_url () = @@ -51,7 +58,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_name" + Alcotest.check_raises "test_introduce_duplicate_binary_url" Api_errors.(Server_error (repository_already_exists, [Ref.string_of ref])) (fun () -> Repository.introduce ~__context ~binary_url ~name_label:name_label_1 @@ -83,9 +90,30 @@ let test_introduce_invalid_gpgkey_path () = |> ignore ) +let test_introduce_duplicate_bundle_repo () = + let __context = T.make_test_database () in + let name_label = "name" in + let name_label_1 = "name1" in + let name_description = "description" in + let name_description_1 = "description1" in + let ref = + Repository.introduce_bundle ~__context ~name_label ~name_description + in + + Alcotest.check_raises "test_introduce_duplicate_bundle_repo" + Api_errors.(Server_error (repository_already_exists, [Ref.string_of ref])) + (fun () -> + Repository.introduce_bundle ~__context ~name_label:name_label_1 + ~name_description:name_description_1 + |> ignore + ) + let test = [ - ("test_introduce_duplicate_name", `Quick, test_introduce_duplicate_name) + ( "test_introduce_duplicate_repo_name" + , `Quick + , test_introduce_duplicate_repo_name + ) ; ( "test_introduce_duplicate_binary_url" , `Quick , test_introduce_duplicate_binary_url @@ -94,6 +122,10 @@ let test = , `Quick , test_introduce_invalid_gpgkey_path ) + ; ( "test_introduce_duplicate_bundle_repo" + , `Quick + , test_introduce_duplicate_bundle_repo + ) ] let () = From 459f68370d542802d2e9dd731d36ad06e01d5dc2 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Mon, 1 Jul 2024 09:56:00 +0800 Subject: [PATCH 03/14] CP-49213: Add new tar unpacking module Add a module Tar_ext to unpack a tar file. During the unpacking process, verify the tar file, containing total file size, file type, file path, file integrity, etc. Signed-off-by: Bengang Yuan --- ocaml/xapi/tar_ext.ml | 173 +++++++++++++++++++++++++++++++++++++++++ ocaml/xapi/tar_ext.mli | 42 ++++++++++ 2 files changed, 215 insertions(+) create mode 100644 ocaml/xapi/tar_ext.ml create mode 100644 ocaml/xapi/tar_ext.mli diff --git a/ocaml/xapi/tar_ext.ml b/ocaml/xapi/tar_ext.ml new file mode 100644 index 00000000000..3595cbee683 --- /dev/null +++ b/ocaml/xapi/tar_ext.ml @@ -0,0 +1,173 @@ +(* + * Copyright (c) Cloud Software Group, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + *) + +open Helpers + +module D = Debug.Make (struct let name = __MODULE__ end) + +open D + +let dir_perm = 0o755 + +let dir_size = 4096L + +let inode_size = 4096L + +let ( ++ ) = Int64.add + +let ( // ) = Filename.concat + +type unpack_error = + | Illegal_file_path of string + | Unsupported_file_type of string + | Unpacked_exceeds_max_size_limit of Int64.t + | File_size_mismatch of { + path: string + ; expected_size: Int64.t + ; actual_size: Int64.t + } + | File_incomplete + | File_corrupted + | Unpacking_failure + +let unpack_error_to_string = function + | Illegal_file_path path -> + Printf.sprintf "Illegal file path: %s" path + | Unsupported_file_type t -> + Printf.sprintf "Unsupported file type: %s" t + | Unpacked_exceeds_max_size_limit size -> + Printf.sprintf + "Stop unpacking, otherwise unpacked files exceed max size limit: %s" + (Int64.to_string size) + | File_size_mismatch {path; expected_size; actual_size} -> + Printf.sprintf + "Unpacked file size mismatch, path: %s, expected size: %s, actual \ + size: %s" + path + (Int64.to_string expected_size) + (Int64.to_string actual_size) + | File_incomplete -> + "File incompete" + | File_corrupted -> + "File corrupted" + | Unpacking_failure -> + "Unpacking failure" + +type unpack_result = Next of Int64.t + +(* Unpack one of entries in the tar file to a specified directory. The file's + information is defined in the file header. It includes the following parameters: + dir: which directory to unpack the tar file. + max_size_limit: the maximum size limitation of all the files in the tar. + acc_size: the current accumulated unpacked files size. + ifd: the tar file's descriptor. + head: the header of the file to be unpacked. +*) +let unpack dir max_size_limit acc_size ifd head = + (* Check if the entry's filename is legal. Including: + Check if starting with '/' + Check if including '.' + Check if including '..' + *) + let assert_file_path_is_legal file_path = + let file_list = String.split_on_char '/' file_path in + if + String.starts_with ~prefix:"/" file_path + || List.exists (String.equal ".") file_list + || List.exists (String.equal "..") file_list + then + Error (Illegal_file_path file_path) + else + Ok () + in + let ( let* ) = Result.bind in + (* Check if the accumulated files size plus the current file size exceeds the + maximum size limitation. If so, return an error. *) + let assert_file_not_exceed_max_size acc_size = + if Int64.compare acc_size max_size_limit > 0 then + Error (Unpacked_exceeds_max_size_limit max_size_limit) + else + Ok () + in + let file_name = head.Tar.Header.file_name in + debug "%s: Unpacking tar file: %s" __FUNCTION__ file_name ; + let* () = assert_file_path_is_legal file_name in + let path = dir // file_name in + match head.Tar.Header.link_indicator with + | Tar.Header.Link.Normal -> + let expected_size = head.Tar.Header.file_size in + let acc_size' = acc_size ++ expected_size ++ inode_size in + let* () = assert_file_not_exceed_max_size acc_size' in + Unixext.with_file path [Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC] + head.Tar.Header.file_mode (fun ofd -> + let actual_size = Unixext.copy_file ~limit:expected_size ifd ofd in + if actual_size <> expected_size then + Error (File_size_mismatch {path; expected_size; actual_size}) + else + Ok (Next acc_size') + ) + | Tar.Header.Link.Directory -> + let acc_size' = acc_size ++ dir_size ++ inode_size in + let* () = assert_file_not_exceed_max_size acc_size' in + Unixext.mkdir_rec path head.Tar.Header.file_mode ; + Ok (Next acc_size') + | Hard + | Symbolic + | Character + | Block + | FIFO + | GlobalExtendedHeader + | PerFileExtendedHeader + | LongLink -> + Error + (Unsupported_file_type + (Tar.Header.Link.to_string head.Tar.Header.link_indicator) + ) + +(* It will call function 'unpack' for every entry in the tar file. Each entry + contains a header and the file self. The header include the destination file + name, file type, file size, etc. The header will be passed to the function + 'unpack' as a parameter, then the function will verify the file size, file + path, etc, and then unpack to the directory which is the other parameter of + this function. +*) +let unpack_tar_file ~dir ~ifd ~max_size_limit = + debug "%s: Unpacking to %s" __FUNCTION__ dir ; + Unixext.rm_rec dir ; + Unixext.mkdir_rec dir dir_perm ; + let rec unpack_all_files f acc_size = + match Tar_unix.Archive.with_next_file ifd (f acc_size) with + | exception Tar.Header.End_of_stream -> + debug "%s: Unpacked to %s successfully" __FUNCTION__ dir ; + Ok () + | Ok (Next size) -> + unpack_all_files f size + | Error _ as err -> + err + | exception End_of_file -> + Error File_incomplete + | exception Tar.Header.Checksum_mismatch -> + Error File_corrupted + | exception e -> + error "%s: Unpacking failure: %s" __FUNCTION__ (Printexc.to_string e) ; + Error Unpacking_failure + in + let unpack' = unpack dir max_size_limit in + unpack_all_files unpack' 0L + |> Result.map_error (fun err -> + error "%s: Failed to unpack to %s due to error: %s" __FUNCTION__ dir + (unpack_error_to_string err) ; + Unixext.rm_rec dir ; + err + ) diff --git a/ocaml/xapi/tar_ext.mli b/ocaml/xapi/tar_ext.mli new file mode 100644 index 00000000000..c56cf23a8be --- /dev/null +++ b/ocaml/xapi/tar_ext.mli @@ -0,0 +1,42 @@ +(* + * Copyright (c) Cloud Software Group, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + *) + +type unpack_error = + | Illegal_file_path of string + | Unsupported_file_type of string + | Unpacked_exceeds_max_size_limit of Int64.t + | File_size_mismatch of { + path: string + ; expected_size: Int64.t + ; actual_size: Int64.t + } + | File_incomplete + | File_corrupted + | Unpacking_failure + +val unpack_error_to_string : unpack_error -> string + +val unpack_tar_file : + dir:string + -> ifd:Unix.file_descr + -> max_size_limit:int64 + -> (unit, unpack_error) result +(** [unpack_tar_file dir ifd max_size_limit] unpacks a tar file from file + descriptor [ifd] to specific directory [dir]. The total size of all unpacked + files should not exceed the maximum size limitation [max_size_limit]. + The function result is: + {ul + {- [Ok ()] if successful.} + {- [Error unpack_error] otherwise. [unpack_error] indicates the + failing reason.}} *) From a9b0ef7f95e88c6912c5465c3a8f8987bc073e35 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Mon, 1 Jul 2024 14:00:34 +0800 Subject: [PATCH 04/14] CP-49213: UT for add new tar unpacking module Signed-off-by: Bengang Yuan --- ocaml/tests/dune | 7 +- .../tests/test_data/gen_tar_ext_test_file.sh | 50 ++++++ ocaml/tests/test_tar_ext.ml | 162 ++++++++++++++++++ 3 files changed, 216 insertions(+), 3 deletions(-) create mode 100755 ocaml/tests/test_data/gen_tar_ext_test_file.sh create mode 100644 ocaml/tests/test_tar_ext.ml diff --git a/ocaml/tests/dune b/ocaml/tests/dune index ef0778ce51c..f792e954b81 100644 --- a/ocaml/tests/dune +++ b/ocaml/tests/dune @@ -8,7 +8,7 @@ test_vm_placement test_vm_helpers test_repository test_repository_helpers test_ref test_livepatch test_rpm test_updateinfo test_storage_smapiv1_wrapper test_storage_quicktest test_observer - test_pool_periodic_update_sync)) + test_pool_periodic_update_sync test_tar_ext)) (libraries alcotest angstrom @@ -61,15 +61,16 @@ (tests (names test_vm_helpers test_vm_placement test_network_sriov test_vdi_cbt test_clustering test_pusb test_daemon_manager test_repository test_repository_helpers - test_livepatch test_rpm test_updateinfo test_pool_periodic_update_sync) + test_livepatch test_rpm test_updateinfo test_pool_periodic_update_sync test_tar_ext) (package xapi) (modes exe) (modules test_vm_helpers test_vm_placement test_network_sriov test_vdi_cbt test_event test_clustering test_cluster_host test_cluster test_pusb test_daemon_manager test_repository test_repository_helpers test_livepatch test_rpm - test_updateinfo test_pool_periodic_update_sync) + test_updateinfo test_pool_periodic_update_sync test_tar_ext) (libraries alcotest + bos fmt ptime result diff --git a/ocaml/tests/test_data/gen_tar_ext_test_file.sh b/ocaml/tests/test_data/gen_tar_ext_test_file.sh new file mode 100755 index 00000000000..673013a5c31 --- /dev/null +++ b/ocaml/tests/test_data/gen_tar_ext_test_file.sh @@ -0,0 +1,50 @@ +#!/bin/bash + +test_file_dir=$1 +mkdir -p "${test_file_dir}" +cd "${test_file_dir}" || exit + +echo "========= Generating regular tar file =========" +echo "This is file-1" > file1.txt +echo "This is file-2" > file2.txt +tar -cvf test_tar_ext_regular.tar file1.txt file2.txt + +echo "========= Generating tar file with illegal path =========" +mkdir test_illegal_dir +touch test_illegal_dir/file +tar --absolute-names -cvf test_tar_ext_illegal_path.tar test_illegal_dir/../file1.txt + +echo "========= Generating tar file trying to escape the current dir =========" +mkdir current_dir +mkdir another_dir +touch current_dir/file +touch another_dir/escaped_file +tar --absolute-names -cvf current_dir/test_tar_ext_trying_to_escape.tar current_dir/../another_dir/escaped_file + +echo "========= Generating tar file with absolute path starting from '/' =========" +tar --absolute-names -cvf test_tar_ext_absolute_path.tar /usr/bin/ls + +echo "========= Generating tar file with unsupported file type =========" +ln -s file1.txt link +tar -cvf test_tar_ext_unsupported_file_type.tar link + +echo "========= Generating tar file unpacked exceeds max size limit =========" +dd if=/dev/zero of=file1 bs=1M count=1 +dd if=/dev/zero of=file2 bs=1M count=1 +dd if=/dev/zero of=file3 bs=1M count=1 +tar -cvf test_tar_ext_unpacked_exceeds_max_size.tar file1 file2 file3 + +echo "========= Generating size mismatch tar file =========" +split -b 100000 test_tar_ext_unpacked_exceeds_max_size.tar test_tar_ext_file_size_mismatch. +mv test_tar_ext_file_size_mismatch.aa test_tar_ext_file_size_mismatch.tar + +echo "========= Generating incomplete tar file =========" +mv file1.txt test_tar_ext_file_incomplete.tar + +echo "========= Generating corrupted tar file =========" +cp test_tar_ext_regular.tar test_tar_ext_corrupted_file.tar +sed -i 's/file1.txt/file3.txt/g' test_tar_ext_corrupted_file.tar + +echo "========= Generating unpacking failure file =========" +cp test_tar_ext_regular.tar test_tar_ext_unpacking_failure.tar +sed -i 's/file1.txt/file.txt/g' test_tar_ext_unpacking_failure.tar diff --git a/ocaml/tests/test_tar_ext.ml b/ocaml/tests/test_tar_ext.ml new file mode 100644 index 00000000000..49cb0edb63f --- /dev/null +++ b/ocaml/tests/test_tar_ext.ml @@ -0,0 +1,162 @@ +(* + * Copyright (c) Cloud Software Group, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + *) + +open Helpers +open Tar_ext +open Bos + +let ( // ) = Filename.concat + +let gen_test_file_script = "test_data" // "gen_tar_ext_test_file.sh" + +let max_size_limit = 2000000L + +let create_temp_dir () = + let mktemp = Cmd.v "mktemp" in + let mktemp' = Cmd.(mktemp % "-d") in + let result = OS.Cmd.(run_out mktemp' |> to_string) in + match result with + | Ok path -> + path + | Error (`Msg s) -> + Alcotest.fail + (Printf.sprintf "Test tar_ext creating temp dir failure: %s" s) + +let test_file_dir = create_temp_dir () + +let unpack_dir = test_file_dir // "output" + +type test_case = { + description: string + ; test_file: string + ; expected: (unit, unpack_error) result +} + +let test_cases = + [ + { + description= "Test regular tar file" + ; test_file= "test_tar_ext_regular.tar" + ; expected= Ok () + } + ; { + description= "Test tar file with illegal path" + ; test_file= "test_tar_ext_illegal_path.tar" + ; expected= Error (Illegal_file_path "test_illegal_dir/../file1.txt") + } + ; { + description= "Test tar file trying to escape the current dir" + ; test_file= "current_dir" // "test_tar_ext_trying_to_escape.tar" + ; expected= + Error (Illegal_file_path "current_dir/../another_dir/escaped_file") + } + ; { + description= "Test tar file with absolute path starting from '/'" + ; test_file= "test_tar_ext_absolute_path.tar" + ; expected= Error (Illegal_file_path "/usr/bin/ls") + } + ; { + description= "Test tar file with unsupported file type" + ; test_file= "test_tar_ext_unsupported_file_type.tar" + ; expected= Error (Unsupported_file_type "Symbolic") + } + ; { + description= "Test unpacked exceeds max size limit" + ; test_file= "test_tar_ext_unpacked_exceeds_max_size.tar" + ; expected= Error (Unpacked_exceeds_max_size_limit 2000000L) + } + ; { + description= "Test unpacked file size mismatch" + ; test_file= "test_tar_ext_file_size_mismatch.tar" + ; expected= + Error + (File_size_mismatch + { + path= unpack_dir // "file1" + ; expected_size= 1048576L + ; actual_size= 99488L + } + ) + } + ; { + description= "Test file incomplete" + ; test_file= "test_tar_ext_file_incomplete.tar" + ; expected= Error File_incomplete + } + ; { + description= "Test corrupted tar file" + ; test_file= "test_tar_ext_corrupted_file.tar" + ; expected= Error File_corrupted + } + ; { + description= "Test file unpacking failure" + ; test_file= "test_tar_ext_unpacking_failure.tar" + ; expected= Error Unpacking_failure + } + ] + +let prepare_env () = + let bash = Cmd.v "bash" in + let gen = Cmd.(bash % gen_test_file_script % test_file_dir) in + let result = OS.Cmd.(run_out gen |> out_null |> success) in + match result with + | Ok () -> + () + | Error (`Msg s) -> + Alcotest.fail (Printf.sprintf "Test tar_ext preparing failure: %s" s) + +let test {test_file; expected; _} () = + let unpack () = + Unixext.with_file (test_file_dir // test_file) [Unix.O_RDONLY] 0o644 + (fun ifd -> Tar_ext.unpack_tar_file ~dir:unpack_dir ~ifd ~max_size_limit + ) + in + let result = unpack () in + match (expected, result) with + | Ok (), Ok () -> + let file1_content = Unixext.string_of_file (unpack_dir // "file1.txt") in + let file2_content = Unixext.string_of_file (unpack_dir // "file2.txt") in + Alcotest.(check string) + "Unpacking file inconsistent" "This is file-1\n" file1_content ; + Alcotest.(check string) + "Unpacking file inconsistent" "This is file-2\n" file2_content + | Error exp, Error acl -> + Alcotest.(check string) + "Unpacking Error inconsistent" + (unpack_error_to_string exp) + (unpack_error_to_string acl) + | Ok (), Error acl -> + Alcotest.fail + (Printf.sprintf + "Unpacking result inconsistent, expected: Ok, actual: %s" + (unpack_error_to_string acl) + ) + | Error exp, Ok () -> + Alcotest.fail + (Printf.sprintf + "Unpacking result inconsistent, expected: %s, actual: Success" + (unpack_error_to_string exp) + ) + +let clean_env () = Unixext.rm_rec test_file_dir + +let generate_tests case = (case.description, `Quick, test case) + +let tests = + (("prepare_env", `Quick, prepare_env) :: List.map generate_tests test_cases) + @ [("clean_env", `Quick, clean_env)] + +let () = + Suite_init.harness_init () ; + Alcotest.run "Test Tar_ext suite" [("Test_tar_ext", tests)] From 7dfcd28d4febfa123e7d5f0758f59ebfce39b393 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Wed, 24 Jul 2024 10:23:36 +0800 Subject: [PATCH 05/14] CP-49214: Upload and sync bundle file Upload a bundle file and unpack it with module Tar_ext. After that, sync the bundle repository. Signed-off-by: Bengang Yuan --- ocaml/idl/datamodel.ml | 1 + ocaml/idl/datamodel_errors.ml | 13 +++ ocaml/xapi-cli-server/cli_frontend.ml | 11 +++ ocaml/xapi-cli-server/cli_operations.ml | 30 +++++++ ocaml/xapi-cli-server/cli_util.ml | 19 +++++ ocaml/xapi-consts/api_errors.ml | 12 +++ ocaml/xapi-consts/constants.ml | 2 + ocaml/xapi/repository.ml | 11 ++- ocaml/xapi/tar_ext.ml | 2 +- ocaml/xapi/xapi.ml | 1 + ocaml/xapi/xapi_globs.ml | 3 + ocaml/xapi/xapi_pool.ml | 108 +++++++++++++++++++++--- ocaml/xapi/xapi_pool.mli | 2 + 13 files changed, 199 insertions(+), 16 deletions(-) diff --git a/ocaml/idl/datamodel.ml b/ocaml/idl/datamodel.ml index eca871fa6d5..580ca92ddbb 100644 --- a/ocaml/idl/datamodel.ml +++ b/ocaml/idl/datamodel.ml @@ -8432,6 +8432,7 @@ let http_actions = , [] ) ) + ; ("put_bundle", (Put, Constants.put_bundle_uri, true, [], _R_POOL_OP, [])) ] (* these public http actions will NOT be checked by RBAC *) diff --git a/ocaml/idl/datamodel_errors.ml b/ocaml/idl/datamodel_errors.ml index 04d56597ea8..4fb61ebbd24 100644 --- a/ocaml/idl/datamodel_errors.ml +++ b/ocaml/idl/datamodel_errors.ml @@ -1895,6 +1895,19 @@ let _ = ~doc:"The GPG public key file name in the repository is invalid." () ; error Api_errors.repository_already_exists ["ref"] ~doc:"The repository already exists." () ; + error Api_errors.bundle_repository_already_exists ["ref"] + ~doc:"The bundle repository already exists." () ; + error Api_errors.bundle_unpack_failed ["error"] + ~doc:"Failed to unpack bundle file." () ; + error Api_errors.bundle_repo_not_enabled [] + ~doc:"Cannot sync bundle as the bundle repository is not enabled." () ; + error Api_errors.can_not_sync_updates [] + ~doc:"Cannot sync updates as the bundle repository is enabled." () ; + error Api_errors.bundle_repo_should_be_single_enabled [] + ~doc: + "If the bundle repository is enabled, it should be the only one enabled \ + repository of the pool." + () ; error Api_errors.repository_is_in_use [] ~doc:"The repository is in use." () ; error Api_errors.repository_cleanup_failed [] ~doc:"Failed to clean up local repository on coordinator." () ; diff --git a/ocaml/xapi-cli-server/cli_frontend.ml b/ocaml/xapi-cli-server/cli_frontend.ml index e735d4793ca..89b7718044b 100644 --- a/ocaml/xapi-cli-server/cli_frontend.ml +++ b/ocaml/xapi-cli-server/cli_frontend.ml @@ -3091,6 +3091,17 @@ let rec cmdtable_data : (string * cmd_spec) list = ; flags= [] } ) + ; ( "pool-sync-bundle" + , { + reqd= ["filename"] + ; optn= [] + ; help= + "Upload and unpack a bundle file, after that, sync the bundle \ + repository." + ; implementation= With_fd Cli_operations.pool_sync_bundle + ; flags= [] + } + ) ; ( "host-ha-xapi-healthcheck" , { reqd= [] diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index 4c0c8f07811..1e049f034b2 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -6769,6 +6769,36 @@ let pool_get_guest_secureboot_readiness printer rpc session_id params = (Record_util.pool_guest_secureboot_readiness_to_string result) ) +let pool_sync_bundle fd _printer rpc session_id params = + let filename_opt = List.assoc_opt "filename" params in + match filename_opt with + | Some filename -> + let make_command task_id = + let master = get_master ~rpc ~session_id in + let master_address = + Client.Host.get_address ~rpc ~session_id ~self:master + in + let uri = + Uri.( + make ~scheme:"http" ~host:master_address + ~path:Constants.put_bundle_uri + ~query: + [ + ("session_id", [Ref.string_of session_id]) + ; ("task_id", [Ref.string_of task_id]) + ] + () + |> to_string + ) + in + debug "%s: requesting HttpPut('%s','%s')" __FUNCTION__ filename uri ; + HttpPut (filename, uri) + in + ignore + (track_http_operation fd rpc session_id make_command "upload bundle") + | None -> + failwith "Required parameter not found: filename" + let host_restore fd _printer rpc session_id params = let filename = List.assoc "file-name" params in let op _ host = diff --git a/ocaml/xapi-cli-server/cli_util.ml b/ocaml/xapi-cli-server/cli_util.ml index 51fa3357d7b..48fd9392ef5 100644 --- a/ocaml/xapi-cli-server/cli_util.ml +++ b/ocaml/xapi-cli-server/cli_util.ml @@ -26,6 +26,14 @@ open Client let finally = Xapi_stdext_pervasives.Pervasiveext.finally +let internal_error fmt = + Printf.ksprintf + (fun str -> + error "%s" str ; + raise Api_errors.(Server_error (internal_error, [str])) + ) + fmt + let log_exn_continue msg f x = try f x with e -> debug "Ignoring exception: %s while %s" (Printexc.to_string e) msg @@ -334,3 +342,14 @@ let error_of_exn e = let string_of_exn exn = let e, l = error_of_exn exn in Printf.sprintf "%s: [ %s ]" e (String.concat "; " l) + +let get_pool ~rpc ~session_id = + match Client.Pool.get_all ~rpc ~session_id with + | [] -> + internal_error "Remote host does not belong to a pool." + | pool :: _ -> + pool + +let get_master ~rpc ~session_id = + let pool = get_pool ~rpc ~session_id in + Client.Pool.get_master ~rpc ~session_id ~self:pool diff --git a/ocaml/xapi-consts/api_errors.ml b/ocaml/xapi-consts/api_errors.ml index 3998068378a..9d11674e7a0 100644 --- a/ocaml/xapi-consts/api_errors.ml +++ b/ocaml/xapi-consts/api_errors.ml @@ -1311,6 +1311,18 @@ let invalid_gpgkey_path = add_error "INVALID_GPGKEY_PATH" let repository_already_exists = add_error "REPOSITORY_ALREADY_EXISTS" +let bundle_repository_already_exists = + add_error "BUNDLE_REPOSITORY_ALREADY_EXISTS" + +let bundle_unpack_failed = add_error "BUNDLE_UNPACK_FAILED" + +let bundle_repo_not_enabled = add_error "BUNDLE_REPO_NOT_ENABLED" + +let can_not_sync_updates = add_error "CAN_NOT_SYNC_UPDATES" + +let bundle_repo_should_be_single_enabled = + add_error "BUNDLE_REPO_SHOULD_BE_SINGLE_ENABLED" + let repository_is_in_use = add_error "REPOSITORY_IS_IN_USE" let repository_cleanup_failed = add_error "REPOSITORY_CLEANUP_FAILED" diff --git a/ocaml/xapi-consts/constants.ml b/ocaml/xapi-consts/constants.ml index 356c6ac6914..2c7fc49e179 100644 --- a/ocaml/xapi-consts/constants.ml +++ b/ocaml/xapi-consts/constants.ml @@ -159,6 +159,8 @@ let get_host_updates_uri = "/host_updates" (* ocaml/xapi/repository.ml *) let get_updates_uri = "/updates" (* ocaml/xapi/repository.ml *) +let put_bundle_uri = "/bundle" (* ocaml/xapi/xapi_pool.ml *) + let default_usb_speed = -1. let use_compression = "use_compression" diff --git a/ocaml/xapi/repository.ml b/ocaml/xapi/repository.ml index fb77d41488b..95007999782 100644 --- a/ocaml/xapi/repository.ml +++ b/ocaml/xapi/repository.ml @@ -51,13 +51,16 @@ let introduce ~__context ~name_label ~name_description ~binary_url ~source_url let introduce_bundle ~__context ~name_label ~name_description = Db.Repository.get_all ~__context |> List.iter (fun ref -> - if - name_label = Db.Repository.get_name_label ~__context ~self:ref - || Db.Repository.get_origin ~__context ~self:ref = `bundle - then + if name_label = Db.Repository.get_name_label ~__context ~self:ref then raise Api_errors.( Server_error (repository_already_exists, [Ref.string_of ref]) + ) ; + if Db.Repository.get_origin ~__context ~self:ref = `bundle then + raise + Api_errors.( + Server_error + (bundle_repository_already_exists, [Ref.string_of ref]) ) ) ; create_repository_record ~__context ~name_label ~name_description diff --git a/ocaml/xapi/tar_ext.ml b/ocaml/xapi/tar_ext.ml index 3595cbee683..35c60719070 100644 --- a/ocaml/xapi/tar_ext.ml +++ b/ocaml/xapi/tar_ext.ml @@ -58,7 +58,7 @@ let unpack_error_to_string = function (Int64.to_string expected_size) (Int64.to_string actual_size) | File_incomplete -> - "File incompete" + "File incomplete" | File_corrupted -> "File corrupted" | Unpacking_failure -> diff --git a/ocaml/xapi/xapi.ml b/ocaml/xapi/xapi.ml index 26659a55801..b568ecded15 100644 --- a/ocaml/xapi/xapi.ml +++ b/ocaml/xapi/xapi.ml @@ -858,6 +858,7 @@ let common_http_handlers () = , Http_svr.FdIO Xapi_pool_update.pool_update_download_handler ) ; ("get_host_updates", Http_svr.FdIO Xapi_host.get_host_updates_handler) + ; ("put_bundle", Http_svr.FdIO Xapi_pool.put_bundle_handler) ] in if !Xapi_globs.disable_webserver then diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index ee2b83df171..cdb1cc40144 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -957,6 +957,9 @@ let ignore_vtpm_unimplemented = ref false let evacuation_batch_size = ref 10 +(* Max size limit of bundle file: 500 MB*) +let bundle_max_size_limit = ref (Int64.of_int (500 * 1024 * 1024)) + type xapi_globs_spec = | Float of float ref | Int of int ref diff --git a/ocaml/xapi/xapi_pool.ml b/ocaml/xapi/xapi_pool.ml index f0cd7c49bfc..d02cfa185c8 100644 --- a/ocaml/xapi/xapi_pool.ml +++ b/ocaml/xapi/xapi_pool.ml @@ -25,6 +25,7 @@ let finally = Xapi_stdext_pervasives.Pervasiveext.finally let with_lock = Xapi_stdext_threads.Threadext.Mutex.execute open Network +open Http module L = Debug.Make (struct let name = "license" end) @@ -3347,10 +3348,30 @@ let enable_tls_verification ~__context = | Some self -> Xapi_cluster_host.set_tls_config ~__context ~self ~verify:true +let contains_bundle_repo ~__context ~repos = + List.exists + (fun repo -> Db.Repository.get_origin ~__context ~self:repo = `bundle) + repos + +let assert_single_bundle_repo_can_be_enabled ~__context ~repos = + if List.length repos > 1 && contains_bundle_repo ~__context ~repos then + raise Api_errors.(Server_error (bundle_repo_should_be_single_enabled, [])) + +let assert_not_bundle_repo ~__context ~repos = + if contains_bundle_repo ~__context ~repos then + raise Api_errors.(Server_error (can_not_sync_updates, [])) + +let disable_auto_update_sync_for_bundle_repo ~__context ~self ~repos = + if contains_bundle_repo ~__context ~repos then ( + Pool_periodic_update_sync.set_enabled ~__context ~value:false ; + Db.Pool.set_update_sync_enabled ~__context ~self ~value:false + ) + let set_repositories ~__context ~self ~value = Xapi_pool_helpers.with_pool_operation ~__context ~self ~doc:"pool.set_repositories" ~op:`configure_repositories @@ fun () -> + assert_single_bundle_repo_can_be_enabled ~__context ~repos:value ; let existings = Db.Pool.get_repositories ~__context ~self in (* To be removed *) List.iter @@ -3373,7 +3394,8 @@ let set_repositories ~__context ~self ~value = value ; Db.Pool.set_repositories ~__context ~self ~value ; if Db.Pool.get_repositories ~__context ~self = [] then - Db.Pool.set_last_update_sync ~__context ~self ~value:Date.epoch + Db.Pool.set_last_update_sync ~__context ~self ~value:Date.epoch ; + disable_auto_update_sync_for_bundle_repo ~__context ~self ~repos:value let add_repository ~__context ~self ~value = Xapi_pool_helpers.with_pool_operation ~__context ~self @@ -3381,11 +3403,15 @@ let add_repository ~__context ~self ~value = @@ fun () -> let existings = Db.Pool.get_repositories ~__context ~self in if not (List.mem value existings) then ( + assert_single_bundle_repo_can_be_enabled ~__context + ~repos:(value :: existings) ; Db.Pool.add_repositories ~__context ~self ~value ; Db.Repository.set_hash ~__context ~self:value ~value:"" ; Repository.reset_updates_in_cache () ; Db.Pool.set_last_update_sync ~__context ~self ~value:Date.epoch - ) + ) ; + disable_auto_update_sync_for_bundle_repo ~__context ~self + ~repos:(value :: existings) let remove_repository ~__context ~self ~value = Xapi_pool_helpers.with_pool_operation ~__context ~self @@ -3403,13 +3429,9 @@ let remove_repository ~__context ~self ~value = if Db.Pool.get_repositories ~__context ~self = [] then Db.Pool.set_last_update_sync ~__context ~self ~value:Date.epoch -let sync_updates ~__context ~self ~force ~token ~token_id = - Pool_features.assert_enabled ~__context ~f:Features.Updates ; +let sync_repos ~__context ~self ~repos ~force ~token ~token_id = let open Repository in - Xapi_pool_helpers.with_pool_operation ~__context ~self - ~doc:"pool.sync_updates" ~op:`sync_updates - @@ fun () -> - Repository_helpers.get_enabled_repositories ~__context + repos |> List.iter (fun repo -> if force then cleanup_pool_repo ~__context ~self:repo ; sync ~__context ~self:repo ~token ~token_id ; @@ -3422,6 +3444,15 @@ let sync_updates ~__context ~self ~force ~token ~token_id = Db.Pool.set_last_update_sync ~__context ~self ~value:(Date.now ()) ; checksum +let sync_updates ~__context ~self ~force ~token ~token_id = + Pool_features.assert_enabled ~__context ~f:Features.Updates ; + Xapi_pool_helpers.with_pool_operation ~__context ~self + ~doc:"pool.sync_updates" ~op:`sync_updates + @@ fun () -> + let repos = Repository_helpers.get_enabled_repositories ~__context in + assert_not_bundle_repo ~__context ~repos ; + sync_repos ~__context ~self ~repos ~force ~token ~token_id + let check_update_readiness ~__context ~self:_ ~requires_reboot = (* Pool license check *) Pool_features.assert_enabled ~__context ~f:Features.Updates ; @@ -3696,9 +3727,15 @@ let configure_update_sync ~__context ~self ~update_sync_frequency Pool_periodic_update_sync.set_enabled ~__context ~value:true let set_update_sync_enabled ~__context ~self ~value = - if value && Db.Pool.get_repositories ~__context ~self = [] then ( - error "Cannot enable automatic update syncing if there are no repositories." ; - raise Api_errors.(Server_error (no_repositories_configured, [])) + ( if value then + match Db.Pool.get_repositories ~__context ~self with + | [] -> + error + "Cannot enable automatic update syncing if there are no \ + repositories." ; + raise Api_errors.(Server_error (no_repositories_configured, [])) + | repos -> + assert_not_bundle_repo ~__context ~repos ) ; Pool_periodic_update_sync.set_enabled ~__context ~value ; Db.Pool.set_update_sync_enabled ~__context ~self ~value @@ -3722,3 +3759,52 @@ let get_guest_secureboot_readiness ~__context ~self:_ = `ready_no_dbx | _, _, _, _ -> `not_ready + +let put_bundle_handler (req : Request.t) s _ = + req.Request.close <- true ; + Xapi_http.with_context "Sync bundle" req s (fun __context -> + (* This is the signal to say we've taken responsibility from the CLI server + for completing the task *) + (* The GUI can deal with this itself, but the CLI is complicated by the thin + cli/cli server split *) + TaskHelper.set_progress ~__context 0.0 ; + Pool_features.assert_enabled ~__context ~f:Features.Updates ; + let pool = Helpers.get_pool ~__context in + Xapi_pool_helpers.with_pool_operation ~__context ~self:pool + ~doc:"pool.sync_updates" ~op:`sync_updates + @@ fun () -> + Http_svr.headers s (Http.http_200_ok ()) ; + let repo = + Repository_helpers.get_single_enabled_update_repository ~__context + in + match Db.Repository.get_origin ~__context ~self:repo with + | `bundle -> ( + let result = + Tar_ext.unpack_tar_file + ~dir:!Xapi_globs.bundle_repository_dir + ~ifd:s + ~max_size_limit:!Xapi_globs.bundle_max_size_limit + in + match result with + | Ok () -> + TaskHelper.set_progress ~__context 0.8 ; + finally + (fun () -> + sync_repos ~__context ~self:pool ~repos:[repo] ~force:true + ~token:"" ~token_id:"" + |> ignore + ) + (fun () -> Unixext.rm_rec !Xapi_globs.bundle_repository_dir) + | Error e -> + error "%s: Failed to unpack bundle with error %s" __FUNCTION__ + (Tar_ext.unpack_error_to_string e) ; + TaskHelper.failed ~__context + Api_errors.( + Server_error + (bundle_unpack_failed, [Tar_ext.unpack_error_to_string e]) + ) ; + Http_svr.headers s (Http.http_400_badrequest ()) + ) + | `remote -> + raise Api_errors.(Server_error (bundle_repo_not_enabled, [])) + ) diff --git a/ocaml/xapi/xapi_pool.mli b/ocaml/xapi/xapi_pool.mli index 39b023810cd..5fc33c66cad 100644 --- a/ocaml/xapi/xapi_pool.mli +++ b/ocaml/xapi/xapi_pool.mli @@ -422,3 +422,5 @@ val get_guest_secureboot_readiness : __context:Context.t -> self:API.ref_pool -> API.pool_guest_secureboot_readiness + +val put_bundle_handler : Http.Request.t -> Unix.file_descr -> 'a -> unit From 3dff387a154cb48827db7e02ea05ffab53200927 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Wed, 24 Jul 2024 10:25:01 +0800 Subject: [PATCH 06/14] CP-49214: Allowed operations for sync bundle Add an allowed_operations for sync bundle. Signed-off-by: Bengang Yuan --- ocaml/idl/datamodel_errors.ml | 5 +++++ ocaml/idl/datamodel_pool.ml | 3 +++ ocaml/idl/schematest.ml | 2 +- ocaml/xapi-cli-server/record_util.ml | 2 ++ ocaml/xapi-consts/api_errors.ml | 2 ++ ocaml/xapi/xapi_pool.ml | 2 +- ocaml/xapi/xapi_pool_helpers.ml | 1 + 7 files changed, 15 insertions(+), 2 deletions(-) diff --git a/ocaml/idl/datamodel_errors.ml b/ocaml/idl/datamodel_errors.ml index 4fb61ebbd24..921a289f04d 100644 --- a/ocaml/idl/datamodel_errors.ml +++ b/ocaml/idl/datamodel_errors.ml @@ -1920,6 +1920,11 @@ let _ = "The operation could not be performed because syncing updates is in \ progress." () ; + error Api_errors.sync_bundle_in_progress [] + ~doc: + "The operation could not be performed because syncing bundle is in \ + progress." + () ; error Api_errors.reposync_failed [] ~doc:"Syncing with remote YUM repository failed." () ; error Api_errors.invalid_repomd_xml [] ~doc:"The repomd.xml is invalid." () ; diff --git a/ocaml/idl/datamodel_pool.ml b/ocaml/idl/datamodel_pool.ml index 4e7336dc2d6..cdc830add08 100644 --- a/ocaml/idl/datamodel_pool.ml +++ b/ocaml/idl/datamodel_pool.ml @@ -21,6 +21,9 @@ let operations = ; ( "sync_updates" , "Indicates this pool is in the process of syncing updates" ) + ; ( "sync_bundle" + , "Indicates this pool is in the process of syncing bundle" + ) ; ( "get_updates" , "Indicates this pool is in the process of getting updates" ) diff --git a/ocaml/idl/schematest.ml b/ocaml/idl/schematest.ml index 6091444dcc9..e81d05ee0ab 100644 --- a/ocaml/idl/schematest.ml +++ b/ocaml/idl/schematest.ml @@ -3,7 +3,7 @@ let hash x = Digest.string x |> Digest.to_hex (* BEWARE: if this changes, check that schema has been bumped accordingly in ocaml/idl/datamodel_common.ml, usually schema_minor_vsn *) -let last_known_schema_hash = "2a6baa01032827a321845b264c6aaae4" +let last_known_schema_hash = "4417b0087b481c3038e73f170b7d4d01" let current_schema_hash : string = let open Datamodel_types in diff --git a/ocaml/xapi-cli-server/record_util.ml b/ocaml/xapi-cli-server/record_util.ml index 92b395185a9..2c98955fffd 100644 --- a/ocaml/xapi-cli-server/record_util.ml +++ b/ocaml/xapi-cli-server/record_util.ml @@ -201,6 +201,8 @@ let pool_operation_to_string = function "configure_repositories" | `sync_updates -> "sync_updates" + | `sync_bundle -> + "sync_bundle" | `get_updates -> "get_updates" | `apply_updates -> diff --git a/ocaml/xapi-consts/api_errors.ml b/ocaml/xapi-consts/api_errors.ml index 9d11674e7a0..0ade8d9cdbf 100644 --- a/ocaml/xapi-consts/api_errors.ml +++ b/ocaml/xapi-consts/api_errors.ml @@ -1334,6 +1334,8 @@ let multiple_update_repositories_enabled = let sync_updates_in_progress = add_error "SYNC_UPDATES_IN_PROGRESS" +let sync_bundle_in_progress = add_error "SYNC_BUNDLE_IN_PROGRESS" + let reposync_failed = add_error "REPOSYNC_FAILED" let createrepo_failed = add_error "CREATEREPO_FAILED" diff --git a/ocaml/xapi/xapi_pool.ml b/ocaml/xapi/xapi_pool.ml index d02cfa185c8..6176729caa4 100644 --- a/ocaml/xapi/xapi_pool.ml +++ b/ocaml/xapi/xapi_pool.ml @@ -3771,7 +3771,7 @@ let put_bundle_handler (req : Request.t) s _ = Pool_features.assert_enabled ~__context ~f:Features.Updates ; let pool = Helpers.get_pool ~__context in Xapi_pool_helpers.with_pool_operation ~__context ~self:pool - ~doc:"pool.sync_updates" ~op:`sync_updates + ~doc:"pool.sync_bundle" ~op:`sync_bundle @@ fun () -> Http_svr.headers s (Http.http_200_ok ()) ; let repo = diff --git a/ocaml/xapi/xapi_pool_helpers.ml b/ocaml/xapi/xapi_pool_helpers.ml index d023cce84d1..16309c7bd51 100644 --- a/ocaml/xapi/xapi_pool_helpers.ml +++ b/ocaml/xapi/xapi_pool_helpers.ml @@ -36,6 +36,7 @@ let blocking_ops = ; (`tls_verification_enable, Api_errors.tls_verification_enable_in_progress) ; (`configure_repositories, Api_errors.configure_repositories_in_progress) ; (`sync_updates, Api_errors.sync_updates_in_progress) + ; (`sync_bundle, Api_errors.sync_bundle_in_progress) ; (`apply_updates, Api_errors.apply_updates_in_progress) ] From 8f4c71bdc5488daabcb9c6c10b7ec23a34656e05 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Wed, 24 Jul 2024 10:25:20 +0800 Subject: [PATCH 07/14] CP-49214: UT for upload and sync bundle file Signed-off-by: Bengang Yuan --- ocaml/tests/dune | 6 +- ocaml/tests/test_pool_repository.ml | 116 ++++++++++++++++++++++++++++ ocaml/tests/test_repository.ml | 4 +- 3 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 ocaml/tests/test_pool_repository.ml diff --git a/ocaml/tests/dune b/ocaml/tests/dune index 2bd666ff4c0..816b18577c4 100644 --- a/ocaml/tests/dune +++ b/ocaml/tests/dune @@ -9,7 +9,7 @@ test_vm_placement test_vm_helpers test_repository test_repository_helpers test_ref test_vm_group test_livepatch test_rpm test_updateinfo test_storage_smapiv1_wrapper test_storage_quicktest test_observer - test_pool_periodic_update_sync test_pkg_mgr test_tar_ext)) + test_pool_periodic_update_sync test_pkg_mgr test_tar_ext test_pool_repository)) (libraries alcotest angstrom @@ -79,13 +79,13 @@ (tests (names test_vm_helpers test_vm_placement test_network_sriov test_vdi_cbt test_clustering test_pusb test_daemon_manager test_repository test_repository_helpers - test_livepatch test_rpm test_updateinfo test_pool_periodic_update_sync test_pkg_mgr test_tar_ext) + test_livepatch test_rpm test_updateinfo test_pool_periodic_update_sync test_pkg_mgr test_tar_ext test_pool_repository) (package xapi) (modes exe) (modules test_vm_helpers test_vm_placement test_network_sriov test_vdi_cbt test_event test_clustering test_cluster_host test_cluster test_pusb test_daemon_manager test_repository test_repository_helpers test_livepatch test_rpm - test_updateinfo test_pool_periodic_update_sync test_pkg_mgr test_tar_ext) + test_updateinfo test_pool_periodic_update_sync test_pkg_mgr test_tar_ext test_pool_repository) (libraries alcotest bos diff --git a/ocaml/tests/test_pool_repository.ml b/ocaml/tests/test_pool_repository.ml new file mode 100644 index 00000000000..bdfcc314e20 --- /dev/null +++ b/ocaml/tests/test_pool_repository.ml @@ -0,0 +1,116 @@ +(* + * Copyright (c) Cloud Software Group, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + *) + +module T = Test_common + +let test_set_remote_and_bundle_repos () = + let __context = T.make_test_database () in + let name_label = "remote" in + let name_description = "remote" in + let binary_url = "https://repo.example.com" in + let source_url = "https://repo-src.example.com" in + let gpgkey_path = "" in + let ref_remote = + Repository.introduce ~__context ~name_label ~name_description ~binary_url + ~source_url ~update:true ~gpgkey_path + in + let ref_bundle = + Repository.introduce_bundle ~__context ~name_label:"bundle" + ~name_description:"bundle" + in + let self = Helpers.get_pool ~__context in + Alcotest.check_raises "test_set_remote_and_bundle_repos" + Api_errors.(Server_error (bundle_repo_should_be_single_enabled, [])) + (fun () -> + Xapi_pool.set_repositories ~__context ~self + ~value:[ref_remote; ref_bundle] + ) + +let test_add_bundle_repo () = + let __context = T.make_test_database () in + let name_label = "remote" in + let name_description = "remote" in + let binary_url = "https://repo.example.com" in + let source_url = "https://repo-src.example.com" in + let gpgkey_path = "" in + let ref_remote = + Repository.introduce ~__context ~name_label ~name_description ~binary_url + ~source_url ~update:true ~gpgkey_path + in + let ref_bundle = + Repository.introduce_bundle ~__context ~name_label:"bundle" + ~name_description:"bundle" + in + let self = Helpers.get_pool ~__context in + Alcotest.check_raises "test_add_bundle_repo" + Api_errors.(Server_error (bundle_repo_should_be_single_enabled, [])) + (fun () -> + Xapi_pool.set_repositories ~__context ~self ~value:[ref_remote] ; + Xapi_pool.add_repository ~__context ~self ~value:ref_bundle + ) + +let test_add_remote_repo () = + let __context = T.make_test_database () in + let name_label = "remote" in + let name_description = "remote" in + let binary_url = "https://repo.example.com" in + let source_url = "https://repo-src.example.com" in + let gpgkey_path = "" in + let ref_remote = + Repository.introduce ~__context ~name_label ~name_description ~binary_url + ~source_url ~update:true ~gpgkey_path + in + let ref_bundle = + Repository.introduce_bundle ~__context ~name_label:"bundle" + ~name_description:"bundle" + in + let self = Helpers.get_pool ~__context in + Alcotest.check_raises "test_add_remote_repo" + Api_errors.(Server_error (bundle_repo_should_be_single_enabled, [])) + (fun () -> + Xapi_pool.set_repositories ~__context ~self ~value:[ref_bundle] ; + Xapi_pool.add_repository ~__context ~self ~value:ref_remote + ) + +let test_can_not_enable_bundle_repo_auto_sync () = + let __context = T.make_test_database () in + let ref_bundle = + Repository.introduce_bundle ~__context ~name_label:"bundle" + ~name_description:"bundle" + in + let self = Helpers.get_pool ~__context in + Alcotest.check_raises "test_can_not_enable_bundle_repo_auto_sync" + Api_errors.(Server_error (can_not_sync_updates, [])) + (fun () -> + Xapi_pool.set_repositories ~__context ~self ~value:[ref_bundle] ; + Xapi_pool.set_update_sync_enabled ~__context ~self ~value:true + ) + +let test = + [ + ( "test_set_remote_and_bundle_repos" + , `Quick + , test_set_remote_and_bundle_repos + ) + ; ("test_add_bundle_repo", `Quick, test_add_bundle_repo) + ; ("test_add_remote_repo", `Quick, test_add_remote_repo) + ; ( "test_can_not_enable_bundle_repo_auto_sync" + , `Quick + , test_can_not_enable_bundle_repo_auto_sync + ) + ] + +let () = + Suite_init.harness_init () ; + Alcotest.run "Test Pool Repository suite" [("Test_pool_repository", test)] diff --git a/ocaml/tests/test_repository.ml b/ocaml/tests/test_repository.ml index 860dc63a950..59008a61272 100644 --- a/ocaml/tests/test_repository.ml +++ b/ocaml/tests/test_repository.ml @@ -101,7 +101,9 @@ let test_introduce_duplicate_bundle_repo () = in Alcotest.check_raises "test_introduce_duplicate_bundle_repo" - Api_errors.(Server_error (repository_already_exists, [Ref.string_of ref])) + Api_errors.( + Server_error (bundle_repository_already_exists, [Ref.string_of ref]) + ) (fun () -> Repository.introduce_bundle ~__context ~name_label:name_label_1 ~name_description:name_description_1 From c870b266b07313f9283966e710e7074822ae3fc1 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Wed, 24 Jul 2024 10:27:53 +0800 Subject: [PATCH 08/14] CP-49214: Refactor cli_operations 1. Replace all `Client.Pool.get_all` and `Client.Pool.get_master` with self-defined function `cli_util.get_pool` and `cli_util.get_master` so that we can reduce the use of `List.hd`. 2. Remove some unused getting pool_master code in function `host_careful_op`. Signed-off-by: Bengang Yuan --- ocaml/xapi-cli-server/cli_operations.ml | 17 +++++------------ quality-gate.sh | 2 +- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index 1e049f034b2..a64b850df40 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -2462,8 +2462,7 @@ let parse_host_uuid ?(default_master = true) rpc session_id params = let hosts = Client.Host.get_all ~rpc ~session_id in let standalone = List.length hosts = 1 in if standalone || default_master then - let pool = List.hd (Client.Pool.get_all ~rpc ~session_id) in - Client.Pool.get_master ~rpc ~session_id ~self:pool + get_master ~rpc ~session_id else failwith "Required parameter not found: host-uuid" @@ -3989,7 +3988,7 @@ let vm_install_real printer rpc session_id template name description params = , [Features.name_of_feature Features.PCI_device_for_auto_update] ) in - let pool = List.hd (Client.Pool.get_all ~rpc ~session_id) in + let pool = get_pool ~rpc ~session_id in let policy_vendor_device_is_ok = not (Client.Pool.get_policy_no_vendor_device ~rpc ~session_id ~self:pool) in @@ -5133,11 +5132,6 @@ let vm_cd_insert printer rpc session_id params = let host_careful_op op warnings fd _printer rpc session_id params = let uuid = List.assoc "uuid" params in let host = Client.Host.get_by_uuid ~rpc ~session_id ~uuid in - let pool = List.hd (Client.Pool.get_all ~rpc ~session_id) in - let _ (* unused variable 'pool_master' *) = - Client.Pool.get_master ~rpc ~session_id ~self:pool - in - (* if pool_master = host then failwith "Cannot forget pool master"; *) let force = get_bool_param params "force" in let go () = ignore (op ~rpc ~session_id ~self:host) in if force then @@ -6605,11 +6599,11 @@ let host_disable_local_storage_caching _printer rpc session_id params = ) let pool_enable_local_storage_caching _printer rpc session_id _params = - let pool = List.hd (Client.Pool.get_all ~rpc ~session_id) in + let pool = get_pool ~rpc ~session_id in Client.Pool.enable_local_storage_caching ~rpc ~session_id ~self:pool let pool_disable_local_storage_caching _printer rpc session_id _params = - let pool = List.hd (Client.Pool.get_all ~rpc ~session_id) in + let pool = get_pool ~rpc ~session_id in Client.Pool.disable_local_storage_caching ~rpc ~session_id ~self:pool let pool_apply_edition printer rpc session_id params = @@ -6692,8 +6686,7 @@ let host_backup fd _printer rpc session_id params = let pool_dump_db fd _printer rpc session_id params = let filename = List.assoc "file-name" params in let make_command task_id = - let pool = List.hd (Client.Pool.get_all ~rpc ~session_id) in - let master = Client.Pool.get_master ~rpc ~session_id ~self:pool in + let master = get_master ~rpc ~session_id in let master_address = Client.Host.get_address ~rpc ~session_id ~self:master in diff --git a/quality-gate.sh b/quality-gate.sh index 8f761718627..2cadb6a8d0e 100755 --- a/quality-gate.sh +++ b/quality-gate.sh @@ -3,7 +3,7 @@ set -e list-hd () { - N=302 + N=296 LIST_HD=$(git grep -r --count 'List.hd' -- **/*.ml | cut -d ':' -f 2 | paste -sd+ - | bc) if [ "$LIST_HD" -eq "$N" ]; then echo "OK counted $LIST_HD List.hd usages" From 4ac5decc0ea28c1b65b26eb743a14965e2b56631 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Fri, 2 Aug 2024 09:36:19 +0100 Subject: [PATCH 09/14] CP-49526: Resolve non-CDN design comments 1. Change the max total bundle files size limit to 1G. 2. Change HTTP error code for the following error to 400: 1) bundle_repo_not_enabled. 2) no_repository_enabled. 3) multiple_update_repositories_enabled. Signed-off-by: Bengang Yuan --- ocaml/xapi/xapi_globs.ml | 4 +- ocaml/xapi/xapi_pool.ml | 80 ++++++++++++++++++++++++---------------- 2 files changed, 50 insertions(+), 34 deletions(-) diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index cdb1cc40144..02ce727a6a1 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -957,8 +957,8 @@ let ignore_vtpm_unimplemented = ref false let evacuation_batch_size = ref 10 -(* Max size limit of bundle file: 500 MB*) -let bundle_max_size_limit = ref (Int64.of_int (500 * 1024 * 1024)) +(* Max size limit of bundle file: 1 GB*) +let bundle_max_size_limit = ref (Int64.of_int (1024 * 1024 * 1024)) type xapi_globs_spec = | Float of float ref diff --git a/ocaml/xapi/xapi_pool.ml b/ocaml/xapi/xapi_pool.ml index 6176729caa4..dcf87d0b503 100644 --- a/ocaml/xapi/xapi_pool.ml +++ b/ocaml/xapi/xapi_pool.ml @@ -3774,37 +3774,53 @@ let put_bundle_handler (req : Request.t) s _ = ~doc:"pool.sync_bundle" ~op:`sync_bundle @@ fun () -> Http_svr.headers s (Http.http_200_ok ()) ; - let repo = - Repository_helpers.get_single_enabled_update_repository ~__context - in - match Db.Repository.get_origin ~__context ~self:repo with - | `bundle -> ( - let result = - Tar_ext.unpack_tar_file - ~dir:!Xapi_globs.bundle_repository_dir - ~ifd:s - ~max_size_limit:!Xapi_globs.bundle_max_size_limit + let repo_opt = + try + let repo = + Repository_helpers.get_single_enabled_update_repository ~__context in - match result with - | Ok () -> - TaskHelper.set_progress ~__context 0.8 ; - finally - (fun () -> - sync_repos ~__context ~self:pool ~repos:[repo] ~force:true - ~token:"" ~token_id:"" - |> ignore - ) - (fun () -> Unixext.rm_rec !Xapi_globs.bundle_repository_dir) - | Error e -> - error "%s: Failed to unpack bundle with error %s" __FUNCTION__ - (Tar_ext.unpack_error_to_string e) ; - TaskHelper.failed ~__context - Api_errors.( - Server_error - (bundle_unpack_failed, [Tar_ext.unpack_error_to_string e]) - ) ; - Http_svr.headers s (Http.http_400_badrequest ()) - ) - | `remote -> - raise Api_errors.(Server_error (bundle_repo_not_enabled, [])) + Some repo + with e -> + TaskHelper.failed ~__context e ; + Http_svr.headers s (Http.http_400_badrequest ()) ; + None + in + match repo_opt with + | Some repo -> ( + match Db.Repository.get_origin ~__context ~self:repo with + | `bundle -> ( + let result = + Tar_ext.unpack_tar_file + ~dir:!Xapi_globs.bundle_repository_dir + ~ifd:s + ~max_size_limit:!Xapi_globs.bundle_max_size_limit + in + match result with + | Ok () -> + TaskHelper.set_progress ~__context 0.8 ; + finally + (fun () -> + sync_repos ~__context ~self:pool ~repos:[repo] ~force:true + ~token:"" ~token_id:"" + |> ignore + ) + (fun () -> Unixext.rm_rec !Xapi_globs.bundle_repository_dir) + | Error e -> + error "%s: Failed to unpack bundle with error %s" __FUNCTION__ + (Tar_ext.unpack_error_to_string e) ; + TaskHelper.failed ~__context + Api_errors.( + Server_error + (bundle_unpack_failed, [Tar_ext.unpack_error_to_string e]) + ) ; + Http_svr.headers s (Http.http_400_badrequest ()) + ) + | `remote -> + error "%s: Bundle repo is not enabled" __FUNCTION__ ; + TaskHelper.failed ~__context + Api_errors.(Server_error (bundle_repo_not_enabled, [])) ; + Http_svr.headers s (Http.http_400_badrequest ()) + ) + | None -> + () ) From 65e35bc553e5cb771c5fcab1c9e40c5f605394fb Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Fri, 2 Aug 2024 09:48:31 +0100 Subject: [PATCH 10/14] CA-396540: Add API error for bundle syncing failure Add a new API error for bundle syncing failure: Syncing with bundle repository failed. Signed-off-by: Bengang Yuan --- ocaml/idl/datamodel_errors.ml | 2 ++ ocaml/xapi-consts/api_errors.ml | 2 ++ ocaml/xapi/xapi_pool.ml | 9 ++++++--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ocaml/idl/datamodel_errors.ml b/ocaml/idl/datamodel_errors.ml index 921a289f04d..3071a4add47 100644 --- a/ocaml/idl/datamodel_errors.ml +++ b/ocaml/idl/datamodel_errors.ml @@ -1927,6 +1927,8 @@ let _ = () ; error Api_errors.reposync_failed [] ~doc:"Syncing with remote YUM repository failed." () ; + error Api_errors.bundle_sync_failed [] + ~doc:"Syncing with bundle repository failed." () ; error Api_errors.invalid_repomd_xml [] ~doc:"The repomd.xml is invalid." () ; error Api_errors.invalid_updateinfo_xml [] ~doc:"The updateinfo.xml is invalid." () ; diff --git a/ocaml/xapi-consts/api_errors.ml b/ocaml/xapi-consts/api_errors.ml index 0ade8d9cdbf..53d9684561f 100644 --- a/ocaml/xapi-consts/api_errors.ml +++ b/ocaml/xapi-consts/api_errors.ml @@ -1338,6 +1338,8 @@ let sync_bundle_in_progress = add_error "SYNC_BUNDLE_IN_PROGRESS" let reposync_failed = add_error "REPOSYNC_FAILED" +let bundle_sync_failed = add_error "BUNDLE_SYNC_FAILED" + let createrepo_failed = add_error "CREATEREPO_FAILED" let invalid_updateinfo_xml = add_error "INVALID_UPDATEINFO_XML" diff --git a/ocaml/xapi/xapi_pool.ml b/ocaml/xapi/xapi_pool.ml index dcf87d0b503..5eae8360b7d 100644 --- a/ocaml/xapi/xapi_pool.ml +++ b/ocaml/xapi/xapi_pool.ml @@ -3800,9 +3800,12 @@ let put_bundle_handler (req : Request.t) s _ = TaskHelper.set_progress ~__context 0.8 ; finally (fun () -> - sync_repos ~__context ~self:pool ~repos:[repo] ~force:true - ~token:"" ~token_id:"" - |> ignore + try + sync_repos ~__context ~self:pool ~repos:[repo] ~force:true + ~token:"" ~token_id:"" + |> ignore + with _ -> + raise Api_errors.(Server_error (bundle_sync_failed, [])) ) (fun () -> Unixext.rm_rec !Xapi_globs.bundle_repository_dir) | Error e -> From 15f2a17bf727cdc9f29505900a9111e6418d3e22 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Mon, 12 Aug 2024 07:45:47 +0100 Subject: [PATCH 11/14] CP-49217: Update datamodel_lifecycle Update the auto-generated changes in `datamodel_lifecycle.ml` before the code is merged into master. Signed-off-by: Bengang Yuan --- ocaml/idl/datamodel_lifecycle.ml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml index bfb6ce0cf2c..1a101ead83b 100644 --- a/ocaml/idl/datamodel_lifecycle.ml +++ b/ocaml/idl/datamodel_lifecycle.ml @@ -25,6 +25,8 @@ let prototyped_of_field = function Some "23.14.0" | "Observer", "uuid" -> Some "23.14.0" + | "Repository", "origin" -> + Some "24.21.0-next" | "Repository", "gpgkey_path" -> Some "22.12.0" | "Certificate", "fingerprint_sha1" -> @@ -123,6 +125,8 @@ let prototyped_of_message = function Some "22.20.0" | "Repository", "set_gpgkey_path" -> Some "22.12.0" + | "Repository", "introduce_bundle" -> + Some "24.21.0-next" | "PCI", "get_dom0_access_status" -> Some "24.14.0" | "PCI", "enable_dom0_access" -> From a61b8532f02d5c785d90716fe62568c2dffc5081 Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Mon, 12 Aug 2024 07:54:29 +0100 Subject: [PATCH 12/14] CP-49217: Update schem in Cli_operations.pool_sync_bundle Update `scheme` to `https` in `Cli_operations.pool_sync_bundle` as `http` can't be used in newcli and will be executed on slave. Signed-off-by: Bengang Yuan --- ocaml/xapi-cli-server/cli_operations.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index a64b850df40..7c693a7a25c 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -6773,7 +6773,7 @@ let pool_sync_bundle fd _printer rpc session_id params = in let uri = Uri.( - make ~scheme:"http" ~host:master_address + make ~scheme:"https" ~host:master_address ~path:Constants.put_bundle_uri ~query: [ From 1a1d5ce51519dcf5f8d9e3daac9d4ae0aecbf3bb Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Wed, 14 Aug 2024 07:05:04 +0100 Subject: [PATCH 13/14] CP-49217: Bump up schema vsn Bump `schema_minor_vsn` as this feature adds a new field repository.origin. Signed-off-by: Bengang Yuan --- ocaml/idl/datamodel_common.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocaml/idl/datamodel_common.ml b/ocaml/idl/datamodel_common.ml index 9afd7bd37c0..ec7e2d7fdb2 100644 --- a/ocaml/idl/datamodel_common.ml +++ b/ocaml/idl/datamodel_common.ml @@ -10,7 +10,7 @@ open Datamodel_roles to leave a gap for potential hotfixes needing to increment the schema version.*) let schema_major_vsn = 5 -let schema_minor_vsn = 780 +let schema_minor_vsn = 781 (* Historical schema versions just in case this is useful later *) let rio_schema_major_vsn = 5 From d8ff15b3d42986424bacae041d1bee052d754efc Mon Sep 17 00:00:00 2001 From: Bengang Yuan Date: Mon, 19 Aug 2024 12:16:53 +0100 Subject: [PATCH 14/14] CP-49217: Refine test_tar_ext and add copyright 1. Refine test_tar_ext for integer format and add some comments to describe the relationship of the 'max_size_limit' between UT and 'gen_tar_ext_test_file.sh'. 2. Add copyright in 'gen_tar_ext_test_file.sh'. Signed-off-by: Bengang Yuan --- ocaml/tests/test_data/gen_tar_ext_test_file.sh | 12 ++++++++++++ ocaml/tests/test_tar_ext.ml | 12 ++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ocaml/tests/test_data/gen_tar_ext_test_file.sh b/ocaml/tests/test_data/gen_tar_ext_test_file.sh index 673013a5c31..39c0deaba2a 100755 --- a/ocaml/tests/test_data/gen_tar_ext_test_file.sh +++ b/ocaml/tests/test_data/gen_tar_ext_test_file.sh @@ -1,4 +1,16 @@ #!/bin/bash +# +# Copyright (c) Cloud Software Group, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published +# by the Free Software Foundation; version 2.1 only. with the special +# exception on linking described in file LICENSE. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. test_file_dir=$1 mkdir -p "${test_file_dir}" diff --git a/ocaml/tests/test_tar_ext.ml b/ocaml/tests/test_tar_ext.ml index 49cb0edb63f..cb16410126c 100644 --- a/ocaml/tests/test_tar_ext.ml +++ b/ocaml/tests/test_tar_ext.ml @@ -20,7 +20,11 @@ let ( // ) = Filename.concat let gen_test_file_script = "test_data" // "gen_tar_ext_test_file.sh" -let max_size_limit = 2000000L +(* The test file generating script 'gen_tar_ext_test_file.sh' will create a tar + file 'test_tar_ext_unpacked_exceeds_max_size.tar' of 3MB. Setting + 'max_size_limit' to 2MB will trigger the error 'Unpacked_exceeds_max_size_limit'. +*) +let max_size_limit = 2 * 1024 * 1024 |> Int64.of_int let create_temp_dir () = let mktemp = Cmd.v "mktemp" in @@ -74,7 +78,7 @@ let test_cases = ; { description= "Test unpacked exceeds max size limit" ; test_file= "test_tar_ext_unpacked_exceeds_max_size.tar" - ; expected= Error (Unpacked_exceeds_max_size_limit 2000000L) + ; expected= Error (Unpacked_exceeds_max_size_limit max_size_limit) } ; { description= "Test unpacked file size mismatch" @@ -84,8 +88,8 @@ let test_cases = (File_size_mismatch { path= unpack_dir // "file1" - ; expected_size= 1048576L - ; actual_size= 99488L + ; expected_size= 1_048_576L + ; actual_size= 99_488L } ) }