Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hosted_engine_setup: hosted-engine.conf permissions #569

Merged
merged 2 commits into from
Aug 3, 2022

Conversation

michalskrivanek
Copy link
Member

  • hosted_engine_setup: fix HE VM networking on recent CentOS Stream
  • align hosted-engine.conf permissions to host-deploy

host deploy creates hosted-engine conf with restricted perissions since
it may contain sensitive data. We should do the same here in initial
deploy.
@michalskrivanek
Copy link
Member Author

/ost he-basic-suite-master

@michalskrivanek michalskrivanek requested a review from didib August 2, 2022 13:08
@michalskrivanek
Copy link
Member Author

[root@ost-he-basic-suite-master-host-0 ~]# ls -l /etc/ovirt-hosted-engine/
total 12
-rw-r--r--. 1 root root  224 Aug  1 06:49 10-appliance.conf
-r--r-----. 1 vdsm kvm  1033 Aug  2 13:35 hosted-engine.conf
-rw-------. 1 root root  103 Aug  2 02:30 virsh_auth.conf
[root@ost-he-basic-suite-master-host-0 ~]# ls -l /var/run/ovirt-hosted-engine-ha
total 4
-rw-r-----. 1 vdsm kvm 1306 Aug  2 13:35 vm.conf

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@arachmani
Copy link
Member

Don't we need to change permissions also in this task:

How does the first point in the first comment related to this PR -
hosted_engine_setup: fix HE VM networking on recent CentOS Stream ?

@mwperina
Copy link
Member

mwperina commented Aug 3, 2022

Don't we need to change permissions also in this task:

How does the first point in the first comment related to this PR - hosted_engine_setup: fix HE VM networking on recent CentOS Stream ?

Asaf, could you please provide a patch for that?

@michalskrivanek
Copy link
Member Author

Don't we need to change permissions also in this task:

How does the first point in the first comment related to this PR - hosted_engine_setup: fix HE VM networking on recent CentOS Stream ?

is the he_local_vm_dir not being cleared at the end? It has all sorts of temporary files and configurations used during installation, I don't think these permissions are going to solve all of them. We shouldn't leave this behind, and if we indeed don't then it's fine as is

@michalskrivanek
Copy link
Member Author

Don't we need to change permissions also in this task:

How does the first point in the first comment related to this PR - hosted_engine_setup: fix HE VM networking on recent CentOS Stream ?

is the he_local_vm_dir not being cleared at the end? It has all sorts of temporary files and configurations used during installation, I don't think these permissions are going to solve all of them. We shouldn't leave this behind, and if we indeed don't then it's fine as is

it seems to me it's cleaned in all cases - failures as well as full and partial execution.

Copy link
Member

@arachmani arachmani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@arachmani arachmani changed the title hosted-engine.conf permissions hosted_engine_setup: hosted-engine.conf permissions Aug 3, 2022
@arachmani arachmani merged commit 73092e9 into oVirt:master Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants