* [pve-devel] [PATCH storage] CephConfig: ensure newline in $secret parameter
@ 2021-11-26 15:02 Aaron Lauterer
2022-01-17 10:11 ` Aaron Lauterer
2022-01-24 11:26 ` Fabian Ebner
0 siblings, 2 replies; 6+ messages in thread
From: Aaron Lauterer @ 2021-11-26 15:02 UTC (permalink / raw)
To: pve-devel
Ensure that the user provided $secret ends in a newline. Otherwise we
will have Input/output errors from rados_connect.
Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
PVE/CephConfig.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/PVE/CephConfig.pm b/PVE/CephConfig.pm
index 5c94a04..ac28e76 100644
--- a/PVE/CephConfig.pm
+++ b/PVE/CephConfig.pm
@@ -227,6 +227,7 @@ sub ceph_create_keyfile {
eval {
if (defined($secret)) {
mkdir '/etc/pve/priv/ceph';
+ $secret = "${secret}\n" if $secret !~ m/\n$/;
PVE::Tools::file_set_contents($ceph_storage_keyring, $secret, 0400);
} elsif ($type eq 'rbd') {
mkdir '/etc/pve/priv/ceph';
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage] CephConfig: ensure newline in $secret parameter
2021-11-26 15:02 [pve-devel] [PATCH storage] CephConfig: ensure newline in $secret parameter Aaron Lauterer
@ 2022-01-17 10:11 ` Aaron Lauterer
2022-01-24 14:03 ` Thomas Lamprecht
2022-01-24 11:26 ` Fabian Ebner
1 sibling, 1 reply; 6+ messages in thread
From: Aaron Lauterer @ 2022-01-17 10:11 UTC (permalink / raw)
To: pve-devel
Ping? Patch should still apply
On 11/26/21 16:02, Aaron Lauterer wrote:
> Ensure that the user provided $secret ends in a newline. Otherwise we
> will have Input/output errors from rados_connect.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> PVE/CephConfig.pm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/PVE/CephConfig.pm b/PVE/CephConfig.pm
> index 5c94a04..ac28e76 100644
> --- a/PVE/CephConfig.pm
> +++ b/PVE/CephConfig.pm
> @@ -227,6 +227,7 @@ sub ceph_create_keyfile {
> eval {
> if (defined($secret)) {
> mkdir '/etc/pve/priv/ceph';
> + $secret = "${secret}\n" if $secret !~ m/\n$/;
> PVE::Tools::file_set_contents($ceph_storage_keyring, $secret, 0400);
> } elsif ($type eq 'rbd') {
> mkdir '/etc/pve/priv/ceph';
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage] CephConfig: ensure newline in $secret parameter
2021-11-26 15:02 [pve-devel] [PATCH storage] CephConfig: ensure newline in $secret parameter Aaron Lauterer
2022-01-17 10:11 ` Aaron Lauterer
@ 2022-01-24 11:26 ` Fabian Ebner
2022-01-24 14:42 ` Aaron Lauterer
1 sibling, 1 reply; 6+ messages in thread
From: Fabian Ebner @ 2022-01-24 11:26 UTC (permalink / raw)
To: pve-devel, Aaron Lauterer
Am 26.11.21 um 16:02 schrieb Aaron Lauterer:
> Ensure that the user provided $secret ends in a newline. Otherwise we
> will have Input/output errors from rados_connect.
>
Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>
Tested-by: Fabian Ebner <f.ebner@proxmox.com>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> PVE/CephConfig.pm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/PVE/CephConfig.pm b/PVE/CephConfig.pm
> index 5c94a04..ac28e76 100644
> --- a/PVE/CephConfig.pm
> +++ b/PVE/CephConfig.pm
> @@ -227,6 +227,7 @@ sub ceph_create_keyfile {
> eval {
> if (defined($secret)) {
> mkdir '/etc/pve/priv/ceph';
> + $secret = "${secret}\n" if $secret !~ m/\n$/;
> PVE::Tools::file_set_contents($ceph_storage_keyring, $secret, 0400);
> } elsif ($type eq 'rbd') {
> mkdir '/etc/pve/priv/ceph';
Just one thing I'm wondering: AFAIU there is no problem for CephFS
currently, but for consistency/future-proving, we might put a newline
there as well when the $secret is not user-provided. I.e. below,
$cephfs_secret isn't newline-terminated:
} elsif ($type eq 'cephfs') {
my $cephfs_secret =
$ceph_get_key->($ceph_admin_keyring, 'admin');
mkdir '/etc/pve/priv/ceph';
PVE::Tools::file_set_contents($ceph_storage_keyring,
$cephfs_secret, 0400);
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage] CephConfig: ensure newline in $secret parameter
2022-01-17 10:11 ` Aaron Lauterer
@ 2022-01-24 14:03 ` Thomas Lamprecht
2022-01-24 14:18 ` Aaron Lauterer
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Lamprecht @ 2022-01-24 14:03 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
On 17.01.22 11:11, Aaron Lauterer wrote:
> Ping? Patch should still apply
>
> On 11/26/21 16:02, Aaron Lauterer wrote:
>> Ensure that the user provided $secret ends in a newline. Otherwise we
>> will have Input/output errors from rados_connect.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>> PVE/CephConfig.pm | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/PVE/CephConfig.pm b/PVE/CephConfig.pm
>> index 5c94a04..ac28e76 100644
>> --- a/PVE/CephConfig.pm
>> +++ b/PVE/CephConfig.pm
>> @@ -227,6 +227,7 @@ sub ceph_create_keyfile {
>> eval {
>> if (defined($secret)) {
>> mkdir '/etc/pve/priv/ceph';
>> + $secret = "${secret}\n" if $secret !~ m/\n$/;
FWIW, we normally use chomp for this, e.g.:
chomp $secret
file_set_contents($ceph_storage_keyring, "$secret\n", 0400);
https://perldoc.perl.org/functions/chomp
IIRC, that little nit made me not apply+push it immediately and then it seems
like it went under the radar.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage] CephConfig: ensure newline in $secret parameter
2022-01-24 14:03 ` Thomas Lamprecht
@ 2022-01-24 14:18 ` Aaron Lauterer
0 siblings, 0 replies; 6+ messages in thread
From: Aaron Lauterer @ 2022-01-24 14:18 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
On 1/24/22 15:03, Thomas Lamprecht wrote:
> On 17.01.22 11:11, Aaron Lauterer wrote:
>> Ping? Patch should still apply
>>
>> On 11/26/21 16:02, Aaron Lauterer wrote:
>>> Ensure that the user provided $secret ends in a newline. Otherwise we
>>> will have Input/output errors from rados_connect.
>>>
>>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>>> ---
>>> PVE/CephConfig.pm | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/PVE/CephConfig.pm b/PVE/CephConfig.pm
>>> index 5c94a04..ac28e76 100644
>>> --- a/PVE/CephConfig.pm
>>> +++ b/PVE/CephConfig.pm
>>> @@ -227,6 +227,7 @@ sub ceph_create_keyfile {
>>> eval {
>>> if (defined($secret)) {
>>> mkdir '/etc/pve/priv/ceph';
>>> + $secret = "${secret}\n" if $secret !~ m/\n$/;
>
> FWIW, we normally use chomp for this, e.g.:
>
> chomp $secret
> file_set_contents($ceph_storage_keyring, "$secret\n", 0400);
>
> https://perldoc.perl.org/functions/chomp
>
> IIRC, that little nit made me not apply+push it immediately and then it seems
> like it went under the radar.
Okay. I'll incorporate that in a v2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage] CephConfig: ensure newline in $secret parameter
2022-01-24 11:26 ` Fabian Ebner
@ 2022-01-24 14:42 ` Aaron Lauterer
0 siblings, 0 replies; 6+ messages in thread
From: Aaron Lauterer @ 2022-01-24 14:42 UTC (permalink / raw)
To: Fabian Ebner, pve-devel
On 1/24/22 12:26, Fabian Ebner wrote:
> Am 26.11.21 um 16:02 schrieb Aaron Lauterer:
>> Ensure that the user provided $secret ends in a newline. Otherwise we
>> will have Input/output errors from rados_connect.
>>
>
> Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>
> Tested-by: Fabian Ebner <f.ebner@proxmox.com>
>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>> PVE/CephConfig.pm | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/PVE/CephConfig.pm b/PVE/CephConfig.pm
>> index 5c94a04..ac28e76 100644
>> --- a/PVE/CephConfig.pm
>> +++ b/PVE/CephConfig.pm
>> @@ -227,6 +227,7 @@ sub ceph_create_keyfile {
>> eval {
>> if (defined($secret)) {
>> mkdir '/etc/pve/priv/ceph';
>> + $secret = "${secret}\n" if $secret !~ m/\n$/;
>> PVE::Tools::file_set_contents($ceph_storage_keyring, $secret, 0400);
>> } elsif ($type eq 'rbd') {
>> mkdir '/etc/pve/priv/ceph';
>
> Just one thing I'm wondering: AFAIU there is no problem for CephFS currently, but for consistency/future-proving, we might put a newline there as well when the $secret is not user-provided. I.e. below, $cephfs_secret isn't newline-terminated:
>
> } elsif ($type eq 'cephfs') {
> my $cephfs_secret = $ceph_get_key->($ceph_admin_keyring, 'admin');
> mkdir '/etc/pve/priv/ceph';
> PVE::Tools::file_set_contents($ceph_storage_keyring, $cephfs_secret, 0400);
> }
Good idea. I did some initial tests in a user provided (external) storage config and adding creating a CephFS in a hyperconverged setup and did not run into issues.
I will add that in the v2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-24 14:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-26 15:02 [pve-devel] [PATCH storage] CephConfig: ensure newline in $secret parameter Aaron Lauterer
2022-01-17 10:11 ` Aaron Lauterer
2022-01-24 14:03 ` Thomas Lamprecht
2022-01-24 14:18 ` Aaron Lauterer
2022-01-24 11:26 ` Fabian Ebner
2022-01-24 14:42 ` Aaron Lauterer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal