[pve-devel] [PATCH installer] post-hook: add `$version` field describing document schema version

[pve-devel] [PATCH installer] post-hook: add `$version` field describing document schema version

[Christoph Heiss]

This adds a metadata-field `$version` to the post-hook json, indicating
which schema version (and thus structure) this document uses.

The field is of format "<major>.<minor>", following the semantic
versioning meaning for both the major and minor number. A patch version
is left out here, as it doesn't make much sense in this context.

This was suggested by Thomas when originally introducing the post-hook
functionality in [0].

[0] https://lore.proxmox.com/pve-devel/00019f8d-e4f2-420e-892a-b89e8b886748@proxmox.com/

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-post-hook/src/main.rs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index 8558da2..fb51ba9 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -133,6 +133,17 @@ struct CpuInfo {
 #[derive(Serialize)]
 #[serde(rename_all = "kebab-case")]
 struct PostHookInfo {
+    /// major.minor version describing the schema version of this document, in a semanticy-version
+    /// way.
+    ///
+    /// major: Incremented for incompatible/breaking API changes, e.g. removing an existing
+    /// field.
+    /// minor: Incremented when adding functionality in a backwards-compatible matter, e.g.
+    /// adding a new field.
+    // This field is prefixed by `$` on purpose, to indicate that it is document metadata and not
+    // part of the actual content itself. (E.g. JSON Schema uses a similar naming scheme)
+    #[serde(rename = "$version")]
+    schema_version: String,
     /// major.minor version of Debian as installed, retrieved from /etc/debian_version
     debian_version: String,
     /// PVE/PMG/PBS version as reported by `pveversion`, `pmgversion` or
@@ -166,6 +177,8 @@ struct PostHookInfo {
 const SIZE_GIB: usize = 1024 * 1024 * 1024;
 
 impl PostHookInfo {
+    const SCHEMA_VERSION: &str = "1.0";
+
     /// Gathers all needed information about the newly installed system for sending
     /// it to a specified server.
     ///
@@ -215,6 +228,7 @@ impl PostHookInfo {
         };
 
         Ok(Self {
+            schema_version: Self::SCHEMA_VERSION.to_owned(),
             debian_version: read_file("/etc/debian_version")?,
             product: Self::gather_product_info(&setup_info, &run_cmd)?,
             iso: setup_info.iso_info.clone(),
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH installer] post-hook: add `$version` field describing document schema version

[Thomas Lamprecht]

Am 12.11.24 um 14:18 schrieb Christoph Heiss:
> This adds a metadata-field `$version` to the post-hook json, indicating
> which schema version (and thus structure) this document uses.
> 
> The field is of format "<major>.<minor>", following the semantic
> versioning meaning for both the major and minor number. A patch version
> is left out here, as it doesn't make much sense in this context.
> 
> This was suggested by Thomas when originally introducing the post-hook
> functionality in [0].
> 
> [0] https://lore.proxmox.com/pve-devel/00019f8d-e4f2-420e-892a-b89e8b886748@proxmox.com/
> 

looks fine, one naming nit/question inline

> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  proxmox-post-hook/src/main.rs | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
> index 8558da2..fb51ba9 100644
> --- a/proxmox-post-hook/src/main.rs
> +++ b/proxmox-post-hook/src/main.rs
> @@ -133,6 +133,17 @@ struct CpuInfo {
>  #[derive(Serialize)]
>  #[serde(rename_all = "kebab-case")]
>  struct PostHookInfo {
> +    /// major.minor version describing the schema version of this document, in a semanticy-version
> +    /// way.
> +    ///
> +    /// major: Incremented for incompatible/breaking API changes, e.g. removing an existing
> +    /// field.
> +    /// minor: Incremented when adding functionality in a backwards-compatible matter, e.g.
> +    /// adding a new field.
> +    // This field is prefixed by `$` on purpose, to indicate that it is document metadata and not
> +    // part of the actual content itself. (E.g. JSON Schema uses a similar naming scheme)
> +    #[serde(rename = "$version")]

maybe still use $format-version or $hook-version or the like for the
serialized key? I.e., to even better convey that this is, e.g., not some
version from the installer or product.

Albeit a hook-version would be probably sent along as HTTP header, as
that then would even bigger change, but as a) it's rather unlikely that
we change away from JSON and b) the absence of such a header can be used
as v0, it is probably overkill now.

> +    schema_version: String,
>      /// major.minor version of Debian as installed, retrieved from /etc/debian_version
>      debian_version: String,
>      /// PVE/PMG/PBS version as reported by `pveversion`, `pmgversion` or
> @@ -166,6 +177,8 @@ struct PostHookInfo {
>  const SIZE_GIB: usize = 1024 * 1024 * 1024;
>  
>  impl PostHookInfo {
> +    const SCHEMA_VERSION: &str = "1.0";
> +
>      /// Gathers all needed information about the newly installed system for sending
>      /// it to a specified server.
>      ///
> @@ -215,6 +228,7 @@ impl PostHookInfo {
>          };
>  
>          Ok(Self {
> +            schema_version: Self::SCHEMA_VERSION.to_owned(),
>              debian_version: read_file("/etc/debian_version")?,
>              product: Self::gather_product_info(&setup_info, &run_cmd)?,
>              iso: setup_info.iso_info.clone(),



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH installer] post-hook: add `$version` field describing document schema version

[Christoph Heiss]

On Tue, Nov 12, 2024 at 02:44:48PM +0100, Thomas Lamprecht wrote:
> Am 12.11.24 um 14:18 schrieb Christoph Heiss:
> > [..]
> > --- a/proxmox-post-hook/src/main.rs
> > +++ b/proxmox-post-hook/src/main.rs
> > @@ -133,6 +133,17 @@ struct CpuInfo {
> >  #[derive(Serialize)]
> >  #[serde(rename_all = "kebab-case")]
> >  struct PostHookInfo {
> > +    /// major.minor version describing the schema version of this document, in a semanticy-version
> > +    /// way.
> > +    ///
> > +    /// major: Incremented for incompatible/breaking API changes, e.g. removing an existing
> > +    /// field.
> > +    /// minor: Incremented when adding functionality in a backwards-compatible matter, e.g.
> > +    /// adding a new field.
> > +    // This field is prefixed by `$` on purpose, to indicate that it is document metadata and not
> > +    // part of the actual content itself. (E.g. JSON Schema uses a similar naming scheme)
> > +    #[serde(rename = "$version")]
>
> maybe still use $format-version or $hook-version or the like for the
> serialized key? I.e., to even better convey that this is, e.g., not some
> version from the installer or product.

What do you think about

    "$hook": { "version": "1.0" }`

maybe, thereby making it a bit more structured from the start?
Esp. when thinking about future expansions, if we ever want to add
additional metadata fields.

As we're already on the topic of future changes.

But no hard feelings, otherwise I'd just go with `$hook-version`.

>
> Albeit a hook-version would be probably sent along as HTTP header, as
> that then would even bigger change, but as a) it's rather unlikely that
> we change away from JSON and b) the absence of such a header can be used
> as v0, it is probably overkill now.

Also, sending as HTTP header would most likely complicate implementing
a server somewhat, depending on the tool/framework used. IOW, having it
in the document directly is quite useful IMO.

But it's a nice idea for the future to have, if we ever need something
like this.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH installer] post-hook: add `$version` field describing document schema version

[Thomas Lamprecht]

Am 12.11.24 um 15:06 schrieb Christoph Heiss:
> On Tue, Nov 12, 2024 at 02:44:48PM +0100, Thomas Lamprecht wrote:
>> maybe still use $format-version or $hook-version or the like for the
>> serialized key? I.e., to even better convey that this is, e.g., not some
>> version from the installer or product.
> 
> What do you think about
> 
>     "$hook": { "version": "1.0" }`
> 
> maybe, thereby making it a bit more structured from the start?
> Esp. when thinking about future expansions, if we ever want to add
> additional metadata fields.
> 
> As we're already on the topic of future changes.
> 
> But no hard feelings, otherwise I'd just go with `$hook-version`.

both sound fine to me, I first thought $hook-version is more greppable,
but that is not really relevant here as the tool processing this needs
to know json already anyway to do so in a sensible manner, so your
proposal seems good to me too, feel free to chose whatever you prefer.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel