[pve-devel] [PATCH storage 0/2] RBD/Cephfs: new keyring parameter

public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed

* [pve-devel] [PATCH storage 0/2] RBD/Cephfs: new keyring parameter
@ 2021-07-21 15:13 Aaron Lauterer
  2021-07-21 15:13 ` [pve-devel] [PATCH storage 1/2] CephConfig: add optional $secret parameter Aaron Lauterer
  2021-07-21 15:13 ` [pve-devel] [PATCH storage 2/2] Ceph: add keyring parameter for external clusters Aaron Lauterer
  0 siblings, 2 replies; 4+ messages in thread
From: Aaron Lauterer @ 2021-07-21 15:13 UTC (permalink / raw)
  To: pve-devel

This new parameter allows to set the RBD keyring or CephFS secret for an
external Ceph Cluster right when creating the storage.
Up until now, they had to be manually placed in
/etc/pve/priv/ceph/$file.

In order to reuse as much code as possible, I had to adjust the
PVE::CephConfig::ceph_create_keyfile method to be able to pass the
secret in as parameter.

Aaron Lauterer (2):
  CephConfig: add optional $secret parameter
  Ceph: add keyring parameter for external clusters

 PVE/API2/Storage/Config.pm  |  2 +-
 PVE/CLI/pvesm.pm            | 12 ++++++++++--
 PVE/CephConfig.pm           | 15 +++++++++------
 PVE/Storage/CephFSPlugin.pm | 20 ++++++++++++++------
 PVE/Storage/RBDPlugin.pm    | 24 ++++++++++++++++++------
 5 files changed, 52 insertions(+), 21 deletions(-)

-- 
2.30.2





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

* [pve-devel] [PATCH storage 1/2] CephConfig: add optional $secret parameter
  2021-07-21 15:13 [pve-devel] [PATCH storage 0/2] RBD/Cephfs: new keyring parameter Aaron Lauterer
@ 2021-07-21 15:13 ` Aaron Lauterer
  2021-07-21 15:13 ` [pve-devel] [PATCH storage 2/2] Ceph: add keyring parameter for external clusters Aaron Lauterer
  1 sibling, 0 replies; 4+ messages in thread
From: Aaron Lauterer @ 2021-07-21 15:13 UTC (permalink / raw)
  To: pve-devel

This allows us to manually pass the used RBD keyring or CephFS secret.
Useful mostly when adding external Ceph clusters where we have no other
means to fetch them.

I renamed the previous $secret to $cephfs_secret to be able to use
$secret as parameter.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/CephConfig.pm | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/PVE/CephConfig.pm b/PVE/CephConfig.pm
index 83d72fc..5c94a04 100644
--- a/PVE/CephConfig.pm
+++ b/PVE/CephConfig.pm
@@ -212,7 +212,7 @@ sub ceph_connect_option {
 }
 
 sub ceph_create_keyfile {
-    my ($type, $storeid) = @_;
+    my ($type, $storeid, $secret) = @_;
 
     my $extension = 'keyring';
     $extension = 'secret' if ($type eq 'cephfs');
@@ -221,17 +221,20 @@ sub ceph_create_keyfile {
     my $ceph_storage_keyring = "/etc/pve/priv/ceph/${storeid}.$extension";
 
     die "ceph authx keyring file for storage '$storeid' already exists!\n"
-	if -e $ceph_storage_keyring;
+	if -e $ceph_storage_keyring && !defined($secret);
 
-    if (-e $ceph_admin_keyring) {
+    if (-e $ceph_admin_keyring || defined($secret)) {
 	eval {
-	    if ($type eq 'rbd') {
+	    if (defined($secret)) {
+		mkdir '/etc/pve/priv/ceph';
+		PVE::Tools::file_set_contents($ceph_storage_keyring, $secret, 0400);
+	    } elsif ($type eq 'rbd') {
 		mkdir '/etc/pve/priv/ceph';
 		PVE::Tools::file_copy($ceph_admin_keyring, $ceph_storage_keyring);
 	    } elsif ($type eq 'cephfs') {
-		my $secret = $ceph_get_key->($ceph_admin_keyring, 'admin');
+		my $cephfs_secret = $ceph_get_key->($ceph_admin_keyring, 'admin');
 		mkdir '/etc/pve/priv/ceph';
-		PVE::Tools::file_set_contents($ceph_storage_keyring, $secret, 0400);
+		PVE::Tools::file_set_contents($ceph_storage_keyring, $cephfs_secret, 0400);
 	   }
 	};
 	if (my $err = $@) {
-- 
2.30.2





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

* [pve-devel] [PATCH storage 2/2] Ceph: add keyring parameter for external clusters
  2021-07-21 15:13 [pve-devel] [PATCH storage 0/2] RBD/Cephfs: new keyring parameter Aaron Lauterer
  2021-07-21 15:13 ` [pve-devel] [PATCH storage 1/2] CephConfig: add optional $secret parameter Aaron Lauterer
@ 2021-07-21 15:13 ` Aaron Lauterer
  2021-07-30 13:35   ` Thomas Lamprecht
  1 sibling, 1 reply; 4+ messages in thread
From: Aaron Lauterer @ 2021-07-21 15:13 UTC (permalink / raw)
  To: pve-devel

By adding the keyring for RBD storage or the secret for CephFS ones, it
is possible to add an external Ceph cluster with only one API call.

Previously the keyring / secret file needed to be placed in
/etc/pve/priv/ceph/$storeID.{keyring,secret} manually.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/API2/Storage/Config.pm  |  2 +-
 PVE/CLI/pvesm.pm            | 12 ++++++++++--
 PVE/Storage/CephFSPlugin.pm | 20 ++++++++++++++------
 PVE/Storage/RBDPlugin.pm    | 24 ++++++++++++++++++------
 4 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/PVE/API2/Storage/Config.pm b/PVE/API2/Storage/Config.pm
index ea655c5..bf38df3 100755
--- a/PVE/API2/Storage/Config.pm
+++ b/PVE/API2/Storage/Config.pm
@@ -112,7 +112,7 @@ __PACKAGE__->register_method ({
 	return &$api_storage_config($cfg, $param->{storage});
     }});
 
-my $sensitive_params = [qw(password encryption-key master-pubkey)];
+my $sensitive_params = [qw(password encryption-key master-pubkey keyring)];
 
 __PACKAGE__->register_method ({
     name => 'create',
diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 668170a..190de91 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -64,13 +64,21 @@ sub param_mapping {
 	}
     };
 
+    my $keyring_map = {
+	name => 'keyring',
+	desc => 'file containing the keyring to authenticate in the Ceph cluster',
+	func => sub {
+	    my ($value) = @_;
+	    return PVE::Tools::file_get_contents($value);
+	},
+    };
 
     my $mapping = {
 	'cifsscan' => [ $password_map ],
 	'cifs' => [ $password_map ],
 	'pbs' => [ $password_map ],
-	'create' => [ $password_map, $enc_key_map, $master_key_map ],
-	'update' => [ $password_map, $enc_key_map, $master_key_map ],
+	'create' => [ $password_map, $enc_key_map, $master_key_map, $keyring_map ],
+	'update' => [ $password_map, $enc_key_map, $master_key_map, $keyring_map ],
     };
     return $mapping->{$name};
 }
diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
index 2aaa450..ae02cb8 100644
--- a/PVE/Storage/CephFSPlugin.pm
+++ b/PVE/Storage/CephFSPlugin.pm
@@ -146,6 +146,7 @@ sub options {
 	fuse => { optional => 1 },
 	bwlimit => { optional => 1 },
 	maxfiles => { optional => 1 },
+	keyring => { optional => 1 },
 	'prune-backups' => { optional => 1 },
     };
 }
@@ -163,20 +164,27 @@ sub check_config {
 sub on_add_hook {
     my ($class, $storeid, $scfg, %param) = @_;
 
-    return if defined($scfg->{monhost}); # nothing to do if not pve managed ceph
+    my $secret = $param{keyring} if defined $param{keyring} // undef;
+    PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $secret);
 
-    PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid);
+    return;
+}
+
+sub on_update_hook {
+    my ($class, $storeid, $scfg, %param) = @_;
+
+    if (defined($param{keyring})) {
+	PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $param{keyring});
+    } else {
+	PVE::CephConfig::ceph_remove_keyfile($scfg->{type}, $storeid);
+    }
 
     return;
 }
 
 sub on_delete_hook {
     my ($class, $storeid, $scfg) = @_;
-
-    return if defined($scfg->{monhost}); # nothing to do if not pve managed ceph
-
     PVE::CephConfig::ceph_remove_keyfile($scfg->{type}, $storeid);
-
     return;
 }
 
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index a8d1243..3e1a671 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -305,6 +305,10 @@ sub properties {
 	    description => "Always access rbd through krbd kernel module.",
 	    type => 'boolean',
 	},
+	keyring => {
+	    description => "Client keyring contents (for external clusters).",
+	    type => 'string',
+	},
     };
 }
 
@@ -318,6 +322,7 @@ sub options {
 	username => { optional => 1 },
 	content => { optional => 1 },
 	krbd => { optional => 1 },
+	keyring => { optional => 1 },
 	bwlimit => { optional => 1 },
     };
 }
@@ -327,20 +332,27 @@ sub options {
 sub on_add_hook {
     my ($class, $storeid, $scfg, %param) = @_;
 
-    return if defined($scfg->{monhost}); # nothing to do if not pve managed ceph
+    my $secret = $param{keyring} if defined $param{keyring} // undef;
+    PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $secret);
+
+    return;
+}
+
+sub on_update_hook {
+    my ($class, $storeid, $scfg, %param) = @_;
 
-    PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid);
+    if (defined($param{keyring})) {
+	PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $param{keyring});
+    } else {
+	PVE::CephConfig::ceph_remove_keyfile($scfg->{type}, $storeid);
+    }
 
     return;
 }
 
 sub on_delete_hook {
     my ($class, $storeid, $scfg) = @_;
-
-    return if defined($scfg->{monhost}); # nothing to do if not pve managed ceph
-
     PVE::CephConfig::ceph_remove_keyfile($scfg->{type}, $storeid);
-
     return;
 }
 
-- 
2.30.2





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

* Re: [pve-devel] [PATCH storage 2/2] Ceph: add keyring parameter for external clusters
  2021-07-21 15:13 ` [pve-devel] [PATCH storage 2/2] Ceph: add keyring parameter for external clusters Aaron Lauterer
@ 2021-07-30 13:35   ` Thomas Lamprecht
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2021-07-30 13:35 UTC (permalink / raw)
  To: Proxmox VE development discussion, Aaron Lauterer

On 21/07/2021 17:13, Aaron Lauterer wrote:
> By adding the keyring for RBD storage or the secret for CephFS ones, it
> is possible to add an external Ceph cluster with only one API call.
> 
> Previously the keyring / secret file needed to be placed in
> /etc/pve/priv/ceph/$storeID.{keyring,secret} manually.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  PVE/API2/Storage/Config.pm  |  2 +-
>  PVE/CLI/pvesm.pm            | 12 ++++++++++--
>  PVE/Storage/CephFSPlugin.pm | 20 ++++++++++++++------
>  PVE/Storage/RBDPlugin.pm    | 24 ++++++++++++++++++------
>  4 files changed, 43 insertions(+), 15 deletions(-)
> 

> diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
> index 2aaa450..ae02cb8 100644
> --- a/PVE/Storage/CephFSPlugin.pm
> +++ b/PVE/Storage/CephFSPlugin.pm

> @@ -163,20 +164,27 @@ sub check_config {
>  sub on_add_hook {
>      my ($class, $storeid, $scfg, %param) = @_;
>  
> -    return if defined($scfg->{monhost}); # nothing to do if not pve managed ceph
> +    my $secret = $param{keyring} if defined $param{keyring} // undef;
> +    PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $secret);
>  
> -    PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid);
> +    return;
> +}
> +
> +sub on_update_hook {
> +    my ($class, $storeid, $scfg, %param) = @_;
> +
> +    if (defined($param{keyring})) {
> +	PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $param{keyring});
> +    } else {
> +	PVE::CephConfig::ceph_remove_keyfile($scfg->{type}, $storeid);
> +    }

this is dangerous, you will always delete the key on any update that did not
provided a new one.

Please look in other plugins about how one must handle this, e.g., PBS

if (exists($param{password})) {
    if (defined($param{password})) {
        pbs_set_password($scfg, $storeid, $param{password});
    } else {
        pbs_delete_password($scfg, $storeid);
    }
}

iow, first check if the param is set and only then you can deduct that undefined
means "must be deleted".


> @@ -327,20 +332,27 @@ sub options {
>  sub on_add_hook {
>      my ($class, $storeid, $scfg, %param) = @_;
>  
> -    return if defined($scfg->{monhost}); # nothing to do if not pve managed ceph
> +    my $secret = $param{keyring} if defined $param{keyring} // undef;
> +    PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $secret);
> +
> +    return;
> +}
> +
> +sub on_update_hook {
> +    my ($class, $storeid, $scfg, %param) = @_;
>  
> -    PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid);
> +    if (defined($param{keyring})) {
> +	PVE::CephConfig::ceph_create_keyfile($scfg->{type}, $storeid, $param{keyring});
> +    } else {
> +	PVE::CephConfig::ceph_remove_keyfile($scfg->{type}, $storeid);
> +    }

same here.





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

end of thread, other threads:[~2021-07-30 13:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21 15:13 [pve-devel] [PATCH storage 0/2] RBD/Cephfs: new keyring parameter Aaron Lauterer
2021-07-21 15:13 ` [pve-devel] [PATCH storage 1/2] CephConfig: add optional $secret parameter Aaron Lauterer
2021-07-21 15:13 ` [pve-devel] [PATCH storage 2/2] Ceph: add keyring parameter for external clusters Aaron Lauterer
2021-07-30 13:35   ` 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