From 1e4f661b8c29edf0b03eac4adf032735d6a885a7 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Mon, 23 Jun 2025 14:42:05 -0500 Subject: T7499: allow load from internal representation to avoid re-parsing --- data/vyconf.proto | 3 ++- src/session.ml | 12 ++++++++++-- src/session.mli | 2 +- src/vyconf_pbt.ml | 20 ++++++++++++++++++-- src/vyconf_pbt.mli | 2 ++ src/vyconfd.ml | 2 +- 6 files changed, 34 insertions(+), 7 deletions(-) diff --git a/data/vyconf.proto b/data/vyconf.proto index 30f213c..4466837 100644 --- a/data/vyconf.proto +++ b/data/vyconf.proto @@ -85,7 +85,8 @@ message Request { message Load { required string Location = 1; - optional ConfigFormat format = 2; + required bool cached = 2; + optional ConfigFormat format = 3; } message Merge { diff --git a/src/session.ml b/src/session.ml index 9ce2807..72e766d 100644 --- a/src/session.ml +++ b/src/session.ml @@ -127,8 +127,16 @@ let session_changed w s = let del_tree = CT.get_subtree diff ["del"] in (del_tree <> CT.default) || (add_tree <> CT.default) -let load w s file = - let ct = Vyos1x.Config_file.load_config file in +let load w s file cached = + let ct = + if cached then + try + Ok (IC.read_internal file) + with Vyos1x.Internal.Read_error e -> + Error e + else + Vyos1x.Config_file.load_config file + in match ct with | Error e -> raise (Session_error (Printf.sprintf "Error loading config: %s" e)) | Ok config -> diff --git a/src/session.mli b/src/session.mli index 8e5805d..0398bb1 100644 --- a/src/session.mli +++ b/src/session.mli @@ -37,7 +37,7 @@ val discard : world -> session_data -> session_data val session_changed : world -> session_data -> bool -val load : world -> session_data -> string -> session_data +val load : world -> session_data -> string -> bool -> session_data val save : world -> session_data -> string -> session_data diff --git a/src/vyconf_pbt.ml b/src/vyconf_pbt.ml index 0afa0bd..facf9f5 100644 --- a/src/vyconf_pbt.ml +++ b/src/vyconf_pbt.ml @@ -83,6 +83,7 @@ type request_rollback = { type request_load = { location : string; + cached : bool; format : request_config_format option; } @@ -313,9 +314,11 @@ let rec default_request_rollback let rec default_request_load ?location:((location:string) = "") + ?cached:((cached:bool) = false) ?format:((format:request_config_format option) = None) () : request_load = { location; + cached; format; } @@ -567,11 +570,13 @@ let default_request_rollback_mutable () : request_rollback_mutable = { type request_load_mutable = { mutable location : string; + mutable cached : bool; mutable format : request_config_format option; } let default_request_load_mutable () : request_load_mutable = { location = ""; + cached = false; format = None; } @@ -819,6 +824,7 @@ let rec pp_request_rollback fmt (v:request_rollback) = let rec pp_request_load fmt (v:request_load) = let pp_i fmt () = Pbrt.Pp.pp_record_field ~first:true "location" Pbrt.Pp.pp_string fmt v.location; + Pbrt.Pp.pp_record_field ~first:false "cached" Pbrt.Pp.pp_bool fmt v.cached; Pbrt.Pp.pp_record_field ~first:false "format" (Pbrt.Pp.pp_option pp_request_config_format) fmt v.format; in Pbrt.Pp.pp_brk pp_i fmt () @@ -1137,10 +1143,12 @@ let rec encode_pb_request_rollback (v:request_rollback) encoder = let rec encode_pb_request_load (v:request_load) encoder = Pbrt.Encoder.string v.location encoder; Pbrt.Encoder.key 1 Pbrt.Bytes encoder; + Pbrt.Encoder.bool v.cached encoder; + Pbrt.Encoder.key 2 Pbrt.Varint encoder; begin match v.format with | Some x -> encode_pb_request_config_format x encoder; - Pbrt.Encoder.key 2 Pbrt.Varint encoder; + Pbrt.Encoder.key 3 Pbrt.Varint encoder; | None -> (); end; () @@ -1784,6 +1792,7 @@ let rec decode_pb_request_rollback d = let rec decode_pb_request_load d = let v = default_request_load_mutable () in let continue__= ref true in + let cached_is_set = ref false in let location_is_set = ref false in while !continue__ do match Pbrt.Decoder.key d with @@ -1795,15 +1804,22 @@ let rec decode_pb_request_load d = | Some (1, pk) -> Pbrt.Decoder.unexpected_payload "Message(request_load), field(1)" pk | Some (2, Pbrt.Varint) -> begin - v.format <- Some (decode_pb_request_config_format d); + v.cached <- Pbrt.Decoder.bool d; cached_is_set := true; end | Some (2, pk) -> Pbrt.Decoder.unexpected_payload "Message(request_load), field(2)" pk + | Some (3, Pbrt.Varint) -> begin + v.format <- Some (decode_pb_request_config_format d); + end + | Some (3, pk) -> + Pbrt.Decoder.unexpected_payload "Message(request_load), field(3)" pk | Some (_, payload_kind) -> Pbrt.Decoder.skip d payload_kind done; + begin if not !cached_is_set then Pbrt.Decoder.missing_field "cached" end; begin if not !location_is_set then Pbrt.Decoder.missing_field "location" end; ({ location = v.location; + cached = v.cached; format = v.format; } : request_load) diff --git a/src/vyconf_pbt.mli b/src/vyconf_pbt.mli index d9eb8ef..42a4af6 100644 --- a/src/vyconf_pbt.mli +++ b/src/vyconf_pbt.mli @@ -90,6 +90,7 @@ type request_rollback = { type request_load = { location : string; + cached : bool; format : request_config_format option; } @@ -315,6 +316,7 @@ val default_request_rollback : val default_request_load : ?location:string -> + ?cached:bool -> ?format:request_config_format option -> unit -> request_load diff --git a/src/vyconfd.ml b/src/vyconfd.ml index 0b92fd1..379a55e 100644 --- a/src/vyconfd.ml +++ b/src/vyconfd.ml @@ -217,7 +217,7 @@ let discard world token (_req: request_discard) = let load world token (req: request_load) = try - let session = Session.load world (find_session token) req.location + let session = Session.load world (find_session token) req.location req.cached in Hashtbl.replace sessions token session; response_tmpl -- cgit v1.2.3 From 59ebae9198439ddd1e9bbdec61d163f638e1071a Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Tue, 24 Jun 2025 07:07:09 -0500 Subject: T7499: use direct request to vyconfd to avoid re-validating paths Merging before passing to the backend results in re-validation of existing paths in the proposed config; this is unavoidable with the legacy backend. For vyconf, send the merge proposal with the request, so validation is only required on new paths. --- data/vyconf.proto | 3 ++- src/session.ml | 10 ++++++++++ src/session.mli | 2 ++ src/vyconf_pbt.ml | 20 ++++++++++++++++++-- src/vyconf_pbt.mli | 2 ++ src/vyconfd.ml | 9 +++++++++ 6 files changed, 43 insertions(+), 3 deletions(-) diff --git a/data/vyconf.proto b/data/vyconf.proto index 4466837..30f95aa 100644 --- a/data/vyconf.proto +++ b/data/vyconf.proto @@ -91,7 +91,8 @@ message Request { message Merge { required string Location = 1; - optional ConfigFormat format = 2; + required bool destructive = 2; + optional ConfigFormat format = 3; } message Save { diff --git a/src/session.ml b/src/session.ml index 72e766d..5e276c2 100644 --- a/src/session.ml +++ b/src/session.ml @@ -142,6 +142,16 @@ let load w s file cached = | Ok config -> validate_tree w config; {s with proposed_config=config;} +let merge w s file destructive = + let ct = Vyos1x.Config_file.load_config file in + match ct with + | Error e -> raise (Session_error (Printf.sprintf "Error loading config: %s" e)) + | Ok config -> + let () = validate_tree w config in + let merged = CD.tree_merge ~destructive:destructive s.proposed_config config + in + {s with proposed_config=merged;} + let save w s file = let ct = w.running_config in let res = Vyos1x.Config_file.save_config ct file in diff --git a/src/session.mli b/src/session.mli index 0398bb1..1a9b79f 100644 --- a/src/session.mli +++ b/src/session.mli @@ -39,6 +39,8 @@ val session_changed : world -> session_data -> bool val load : world -> session_data -> string -> bool -> session_data +val merge : world -> session_data -> string -> bool -> session_data + val save : world -> session_data -> string -> session_data val get_value : world -> session_data -> string list -> string diff --git a/src/vyconf_pbt.ml b/src/vyconf_pbt.ml index facf9f5..913fcea 100644 --- a/src/vyconf_pbt.ml +++ b/src/vyconf_pbt.ml @@ -89,6 +89,7 @@ type request_load = { type request_merge = { location : string; + destructive : bool; format : request_config_format option; } @@ -324,9 +325,11 @@ let rec default_request_load let rec default_request_merge ?location:((location:string) = "") + ?destructive:((destructive:bool) = false) ?format:((format:request_config_format option) = None) () : request_merge = { location; + destructive; format; } @@ -582,11 +585,13 @@ let default_request_load_mutable () : request_load_mutable = { type request_merge_mutable = { mutable location : string; + mutable destructive : bool; mutable format : request_config_format option; } let default_request_merge_mutable () : request_merge_mutable = { location = ""; + destructive = false; format = None; } @@ -832,6 +837,7 @@ let rec pp_request_load fmt (v:request_load) = let rec pp_request_merge fmt (v:request_merge) = let pp_i fmt () = Pbrt.Pp.pp_record_field ~first:true "location" Pbrt.Pp.pp_string fmt v.location; + Pbrt.Pp.pp_record_field ~first:false "destructive" Pbrt.Pp.pp_bool fmt v.destructive; Pbrt.Pp.pp_record_field ~first:false "format" (Pbrt.Pp.pp_option pp_request_config_format) fmt v.format; in Pbrt.Pp.pp_brk pp_i fmt () @@ -1156,10 +1162,12 @@ let rec encode_pb_request_load (v:request_load) encoder = let rec encode_pb_request_merge (v:request_merge) encoder = Pbrt.Encoder.string v.location encoder; Pbrt.Encoder.key 1 Pbrt.Bytes encoder; + Pbrt.Encoder.bool v.destructive encoder; + Pbrt.Encoder.key 2 Pbrt.Varint encoder; begin match v.format with | Some x -> encode_pb_request_config_format x encoder; - Pbrt.Encoder.key 2 Pbrt.Varint encoder; + Pbrt.Encoder.key 3 Pbrt.Varint encoder; | None -> (); end; () @@ -1826,6 +1834,7 @@ let rec decode_pb_request_load d = let rec decode_pb_request_merge d = let v = default_request_merge_mutable () in let continue__= ref true in + let destructive_is_set = ref false in let location_is_set = ref false in while !continue__ do match Pbrt.Decoder.key d with @@ -1837,15 +1846,22 @@ let rec decode_pb_request_merge d = | Some (1, pk) -> Pbrt.Decoder.unexpected_payload "Message(request_merge), field(1)" pk | Some (2, Pbrt.Varint) -> begin - v.format <- Some (decode_pb_request_config_format d); + v.destructive <- Pbrt.Decoder.bool d; destructive_is_set := true; end | Some (2, pk) -> Pbrt.Decoder.unexpected_payload "Message(request_merge), field(2)" pk + | Some (3, Pbrt.Varint) -> begin + v.format <- Some (decode_pb_request_config_format d); + end + | Some (3, pk) -> + Pbrt.Decoder.unexpected_payload "Message(request_merge), field(3)" pk | Some (_, payload_kind) -> Pbrt.Decoder.skip d payload_kind done; + begin if not !destructive_is_set then Pbrt.Decoder.missing_field "destructive" end; begin if not !location_is_set then Pbrt.Decoder.missing_field "location" end; ({ location = v.location; + destructive = v.destructive; format = v.format; } : request_merge) diff --git a/src/vyconf_pbt.mli b/src/vyconf_pbt.mli index 42a4af6..c9a7530 100644 --- a/src/vyconf_pbt.mli +++ b/src/vyconf_pbt.mli @@ -96,6 +96,7 @@ type request_load = { type request_merge = { location : string; + destructive : bool; format : request_config_format option; } @@ -324,6 +325,7 @@ val default_request_load : val default_request_merge : ?location:string -> + ?destructive:bool -> ?format:request_config_format option -> unit -> request_merge diff --git a/src/vyconfd.ml b/src/vyconfd.ml index 379a55e..a0be019 100644 --- a/src/vyconfd.ml +++ b/src/vyconfd.ml @@ -223,6 +223,14 @@ let load world token (req: request_load) = response_tmpl with Session.Session_error msg -> {response_tmpl with status=Fail; error=(Some msg)} +let merge world token (req: request_merge) = + try + let session = Session.merge world (find_session token) req.location req.destructive + in + Hashtbl.replace sessions token session; + response_tmpl + with Session.Session_error msg -> {response_tmpl with status=Fail; error=(Some msg)} + let save world token (req: request_save) = try let _ = Session.save world (find_session token) req.location @@ -319,6 +327,7 @@ let rec handle_connection world ic oc () = | Some t, Session_changed r -> session_changed world t r | Some t, Get_config r -> get_config world t r | Some t, Load r -> load world t r + | Some t, Merge r -> merge world t r | Some t, Save r -> save world t r | _ -> failwith "Unimplemented" ) |> Lwt.return -- cgit v1.2.3 From 554ac72db109a04b2d53843bc75aa80965d22546 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Sun, 29 Jun 2025 01:47:05 -0500 Subject: T7374: close session to avoid stale data --- src/vyconf_cli.ml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/vyconf_cli.ml b/src/vyconf_cli.ml index ded03df..0d1535e 100644 --- a/src/vyconf_cli.ml +++ b/src/vyconf_cli.ml @@ -29,6 +29,14 @@ let output_format_of_string s = | "json" -> Out_json | _ -> failwith (Printf.sprintf "Unknown output format %s, should be plain or json" s) +let in_cli_config_session () = + let env = Unix.environment () in + let res = Array.find_opt (fun c -> String.starts_with ~prefix:"_OFR_CONFIGURE" c) env + in + match res with + | Some _ -> true + | None -> false + let get_session () = let pid = Int32.of_int (Unix.getppid()) in let socket = "/var/run/vyconfd.sock" in @@ -42,6 +50,13 @@ let get_session () = | Error _ -> setup_session client "vyconf_cli" pid | _ as c -> c |> Lwt.return +let close_session () = + let%lwt client = get_session () in + match client with + | Ok c -> + teardown_session c + | Error e -> Error e |> Lwt.return + let main op path = let%lwt client = get_session () in let%lwt result = @@ -57,6 +72,10 @@ let main op path = end | Error e -> Error e |> Lwt.return in + let () = + if not (in_cli_config_session ()) then + close_session () |> Lwt.ignore_result + in match result with | Ok s -> let%lwt () = Lwt_io.write Lwt_io.stdout s in Lwt.return 0 | Error e -> let%lwt () = Lwt_io.write Lwt_io.stderr (Printf.sprintf "%s\n" e) in Lwt.return 1 -- cgit v1.2.3 From 9ac7c1e3a258c1e41c4c4abd7b45a8f29af7907a Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Mon, 30 Jun 2025 15:48:10 -0500 Subject: T7499: catch Duplicate_value error --- src/session.ml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/session.ml b/src/session.ml index 5e276c2..1ff9bae 100644 --- a/src/session.ml +++ b/src/session.ml @@ -104,8 +104,12 @@ let set w s path = apply_cfg_op op s.proposed_config |> (fun c -> RT.set_tag_data w.reference_tree c path) |> (fun c -> RT.set_leaf_data w.reference_tree c path) - with CT.Useless_set -> + with + | CT.Useless_set -> raise (Session_error (Printf.sprintf "Useless set, path: %s" (string_of_op op))) + | CT.Duplicate_value -> + raise (Session_error (Printf.sprintf "Duplicate value, path: %s" (string_of_op op))) + in {s with proposed_config=config; changeset=(op :: s.changeset)} -- cgit v1.2.3