public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH storage 0/4] OCI registry pull fixes
@ 2025-11-17 17:13 Filip Schauer
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 1/4] api: storage status: pull OCI image to temporary file Filip Schauer
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Filip Schauer @ 2025-11-17 17:13 UTC (permalink / raw)
  To: pve-devel

Fix some bugs that came up when pulling OCI images from registries and
add an optional API parameter for a custom destination file name for the
OCI image.

Filip Schauer (4):
  api: storage status: pull OCI image to temporary file
  api: storage status: do not pull OCI image if file already exists
  api: storage status: add optional OCI image filename parameter
  api: storage status: oci-registry-pull: fix incomplete reference regex

 src/PVE/API2/Storage/Status.pm | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH storage 1/4] api: storage status: pull OCI image to temporary file
  2025-11-17 17:13 [pve-devel] [PATCH storage 0/4] OCI registry pull fixes Filip Schauer
@ 2025-11-17 17:13 ` Filip Schauer
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 2/4] api: storage status: do not pull OCI image if file already exists Filip Schauer
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2025-11-17 17:13 UTC (permalink / raw)
  To: pve-devel

Pull the OCI image to a temporary file first. Once it has finished,
rename it, which is an atomic operation. This prevents accidental usage
of an incomplete OCI image file for container creation.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/PVE/API2/Storage/Status.pm | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index 5abf53b..96832b9 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -959,14 +959,26 @@ __PACKAGE__->register_method({
         die "storage '$storage' is not configured for content-type 'vztmpl'\n"
             if !$scfg->{content}->{vztmpl};
 
-        my $filename = PVE::Storage::normalize_content_filename($reference);
+        my $filename = PVE::Storage::normalize_content_filename($reference) . ".tar";
+        my $tmp_filename = "$filename.tmp.$$";
         my $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
         PVE::Storage::activate_storage($cfg, $storage);
 
+        local $SIG{INT} = sub {
+            unlink "$path/$tmp_filename"
+                or warn "could not cleanup temporary file: $!"
+                if -e "$path/$tmp_filename";
+            die "got interrupted by signal\n";
+        };
+
         my $worker = sub {
             PVE::Tools::run_command(
-                ["skopeo", "copy", "docker://$reference", "oci-archive:$path/$filename.tar"],
+                [
+                    "skopeo", "copy", "docker://$reference", "oci-archive:$path/$tmp_filename",
+                ],
             );
+            rename("$path/$tmp_filename", "$path/$filename")
+                or die "unable to rename temporary file: $!\n";
         };
 
         my $worker_id = PVE::Tools::encode_text($filename); # must not pass : or the like as w-ID
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH storage 2/4] api: storage status: do not pull OCI image if file already exists
  2025-11-17 17:13 [pve-devel] [PATCH storage 0/4] OCI registry pull fixes Filip Schauer
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 1/4] api: storage status: pull OCI image to temporary file Filip Schauer
@ 2025-11-17 17:13 ` Filip Schauer
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 3/4] api: storage status: add optional OCI image filename parameter Filip Schauer
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2025-11-17 17:13 UTC (permalink / raw)
  To: pve-devel

This ensures that the API call fails early, before pulling the OCI image
from the registry and then failing to rename the temporary file.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/PVE/API2/Storage/Status.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index 96832b9..9fb6141 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -964,6 +964,8 @@ __PACKAGE__->register_method({
         my $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
         PVE::Storage::activate_storage($cfg, $storage);
 
+        die "refusing to override existing file '$filename'\n" if (-f "$path/$filename");
+
         local $SIG{INT} = sub {
             unlink "$path/$tmp_filename"
                 or warn "could not cleanup temporary file: $!"
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH storage 3/4] api: storage status: add optional OCI image filename parameter
  2025-11-17 17:13 [pve-devel] [PATCH storage 0/4] OCI registry pull fixes Filip Schauer
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 1/4] api: storage status: pull OCI image to temporary file Filip Schauer
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 2/4] api: storage status: do not pull OCI image if file already exists Filip Schauer
@ 2025-11-17 17:13 ` Filip Schauer
  2025-11-17 18:58   ` Filip Schauer
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 4/4] api: storage status: oci-registry-pull: fix incomplete reference regex Filip Schauer
  2025-11-17 20:56 ` [pve-devel] applied: [PATCH storage 0/4] OCI registry pull fixes Thomas Lamprecht
  4 siblings, 1 reply; 7+ messages in thread
From: Filip Schauer @ 2025-11-17 17:13 UTC (permalink / raw)
  To: pve-devel

Give the user the ability to choose a custom destination file name for
the OCI image.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/PVE/API2/Storage/Status.pm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index 9fb6141..16436a3 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -933,6 +933,13 @@ __PACKAGE__->register_method({
                     . '(?:\.(?:[a-zA-Z\d]|[a-zA-Z\d][a-zA-Z\d-]*[a-zA-Z\d]))*(?::\d+)?/)?[a-z\d]+'
                     . '(?:/[a-z\d]+(?:(?:(?:[._]|__|[-]*)[a-z\d]+)+)?)*:\w[\w.-]{0,127}$',
             },
+            filename => {
+                description =>
+                    "Custom destination file name of the OCI image. Caution: This will be normalized!",
+                optional => 1,
+                maxLength => 255,
+                type => 'string',
+            },
         },
     },
     returns => {
@@ -959,7 +966,8 @@ __PACKAGE__->register_method({
         die "storage '$storage' is not configured for content-type 'vztmpl'\n"
             if !$scfg->{content}->{vztmpl};
 
-        my $filename = PVE::Storage::normalize_content_filename($reference) . ".tar";
+        my $filename =
+            PVE::Storage::normalize_content_filename($param->{filename} // $reference) . ".tar";
         my $tmp_filename = "$filename.tmp.$$";
         my $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
         PVE::Storage::activate_storage($cfg, $storage);
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH storage 4/4] api: storage status: oci-registry-pull: fix incomplete reference regex
  2025-11-17 17:13 [pve-devel] [PATCH storage 0/4] OCI registry pull fixes Filip Schauer
                   ` (2 preceding siblings ...)
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 3/4] api: storage status: add optional OCI image filename parameter Filip Schauer
@ 2025-11-17 17:13 ` Filip Schauer
  2025-11-17 20:56 ` [pve-devel] applied: [PATCH storage 0/4] OCI registry pull fixes Thomas Lamprecht
  4 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2025-11-17 17:13 UTC (permalink / raw)
  To: pve-devel

Previously trying to pull an OCI image with a '-' character in its name
failed because the regex did not match on it. This commit fixes this by
basing the regex on the one used in 'query-oci-repo-tags'.

Reported-by: Nicolas Frey <n.frey@proxmox.com>
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/PVE/API2/Storage/Status.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index 16436a3..c29a991 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -931,7 +931,8 @@ __PACKAGE__->register_method({
                 pattern =>
                     '^(?:(?:[a-zA-Z\d]|[a-zA-Z\d][a-zA-Z\d-]*[a-zA-Z\d])'
                     . '(?:\.(?:[a-zA-Z\d]|[a-zA-Z\d][a-zA-Z\d-]*[a-zA-Z\d]))*(?::\d+)?/)?[a-z\d]+'
-                    . '(?:/[a-z\d]+(?:(?:(?:[._]|__|[-]*)[a-z\d]+)+)?)*:\w[\w.-]{0,127}$',
+                    . '(?:(?:[._]|__|[-]*)[a-z\d]+)*(?:/[a-z\d]+(?:(?:[._]|__|[-]*)[a-z\d]+)*)*'
+                    . ':\w[\w.-]{0,127}$',
             },
             filename => {
                 description =>
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH storage 3/4] api: storage status: add optional OCI image filename parameter
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 3/4] api: storage status: add optional OCI image filename parameter Filip Schauer
@ 2025-11-17 18:58   ` Filip Schauer
  0 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2025-11-17 18:58 UTC (permalink / raw)
  To: pve-devel

On 17/11/2025 18:15, Filip Schauer wrote:
> + filename => {
> + description =>
> + "Custom destination file name of the OCI image. Caution: This will 
> be normalized!",
> + optional => 1,
> + maxLength => 255,

Add a

minLength => 1,

here



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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] applied:  [PATCH storage 0/4] OCI registry pull fixes
  2025-11-17 17:13 [pve-devel] [PATCH storage 0/4] OCI registry pull fixes Filip Schauer
                   ` (3 preceding siblings ...)
  2025-11-17 17:13 ` [pve-devel] [PATCH storage 4/4] api: storage status: oci-registry-pull: fix incomplete reference regex Filip Schauer
@ 2025-11-17 20:56 ` Thomas Lamprecht
  4 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2025-11-17 20:56 UTC (permalink / raw)
  To: pve-devel, Filip Schauer

On Mon, 17 Nov 2025 18:13:13 +0100, Filip Schauer wrote:
> Fix some bugs that came up when pulling OCI images from registries and
> add an optional API parameter for a custom destination file name for the
> OCI image.
> 
> Filip Schauer (4):
>   api: storage status: pull OCI image to temporary file
>   api: storage status: do not pull OCI image if file already exists
>   api: storage status: add optional OCI image filename parameter
>   api: storage status: oci-registry-pull: fix incomplete reference regex
> 
> [...]

Applied, with squashing in adding minLength to 3/4 as you noted and two other
small follow-ups for code style, thanks!

[1/4] api: storage status: pull OCI image to temporary file
      commit: fe3b2915f2ac5a5b1e4842c67dd2056c7b8f6327
[2/4] api: storage status: do not pull OCI image if file already exists
      commit: 0ed80aaf31551f66fd05753dbdd7d8a398684193
[3/4] api: storage status: add optional OCI image filename parameter
      commit: 21584f8350aa6ba35eb60d10fd058055caa6fba2
[4/4] api: storage status: oci-registry-pull: fix incomplete reference regex
      commit: 007c700af146cba6f0c506ddfc9e0b90c0f8ed1e


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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-11-17 20:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-17 17:13 [pve-devel] [PATCH storage 0/4] OCI registry pull fixes Filip Schauer
2025-11-17 17:13 ` [pve-devel] [PATCH storage 1/4] api: storage status: pull OCI image to temporary file Filip Schauer
2025-11-17 17:13 ` [pve-devel] [PATCH storage 2/4] api: storage status: do not pull OCI image if file already exists Filip Schauer
2025-11-17 17:13 ` [pve-devel] [PATCH storage 3/4] api: storage status: add optional OCI image filename parameter Filip Schauer
2025-11-17 18:58   ` Filip Schauer
2025-11-17 17:13 ` [pve-devel] [PATCH storage 4/4] api: storage status: oci-registry-pull: fix incomplete reference regex Filip Schauer
2025-11-17 20:56 ` [pve-devel] applied: [PATCH storage 0/4] OCI registry pull fixes Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal