* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox