From 20d348edc3e1e109519718267d61b3a696b4831d Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Mon, 20 Nov 2023 13:58:50 +0100 Subject: [PATCH] Do not run nova services as root Now each podified service runs as nova user. This needed a bit of change how we run our jobs. Now the scripts of the job is also started via kolla_start, as a side effect this PR needed to make sure that the mounted script files are executable by the nova user. As apache is also running with the nova user we needed to set some permissions for the api containers. At the same time we make sure that apache logs everything to its stdout / stderr to make the logs visible. Implements: https://issues.redhat.com/browse/OSPRH-1374 --- pkg/nova/cellmapping.go | 13 +++----- pkg/nova/common.go | 5 ++- .../{host_discovery.go => host_discover.go} | 13 +++----- pkg/nova/volumes.go | 4 ++- pkg/novaapi/deployment.go | 7 ++--- pkg/novacompute/deployment.go | 5 ++- pkg/novaconductor/dbsync.go | 14 +++------ pkg/novaconductor/deployment.go | 5 ++- pkg/novametadata/deployment.go | 7 ++--- pkg/novascheduler/deployment.go | 5 ++- pkg/novncproxy/deployment.go | 5 ++- .../config/cell-mapping-config.json | 24 ++++++++++++++ ...-config.json => host-discover-config.json} | 2 +- templates/novaapi/config/httpd.conf | 2 ++ templates/novaapi/config/nova-api-config.json | 5 +++ .../config/nova-conductor-dbsync-config.json | 31 +++++++++++++++++++ templates/novametadata/config/httpd.conf | 2 ++ .../config/nova-metadata-config.json | 5 +++ .../novaconductor_controller_test.go | 1 - 19 files changed, 103 insertions(+), 52 deletions(-) rename pkg/nova/{host_discovery.go => host_discover.go} (90%) create mode 100644 templates/nova-manage/config/cell-mapping-config.json rename templates/nova-manage/config/{nova-manage-config.json => host-discover-config.json} (91%) create mode 100644 templates/novaconductor/config/nova-conductor-dbsync-config.json diff --git a/pkg/nova/cellmapping.go b/pkg/nova/cellmapping.go index 8d65c2682..0b63ced51 100644 --- a/pkg/nova/cellmapping.go +++ b/pkg/nova/cellmapping.go @@ -4,16 +4,13 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" common "github.com/openstack-k8s-operators/lib-common/modules/common" "github.com/openstack-k8s-operators/lib-common/modules/common/env" novav1 "github.com/openstack-k8s-operators/nova-operator/api/v1beta1" ) -const ( - cellMappingCommand = "/usr/local/bin/kolla_set_configs && /var/lib/openstack/bin/ensure_cell_mapping.sh" -) - func CellMappingJob( instance *novav1.Nova, cell *novav1.NovaCell, @@ -22,13 +19,11 @@ func CellMappingJob( inputHash string, labels map[string]string, ) *batchv1.Job { - runAsUser := int64(0) - args := []string{"-c"} if cell.Spec.Debug.StopJob { args = append(args, common.DebugCommand) } else { - args = append(args, cellMappingCommand) + args = append(args, KollaServiceCommand) } envVars := map[string]env.Setter{} @@ -69,13 +64,13 @@ func CellMappingJob( Args: args, Image: cell.Spec.ConductorServiceTemplate.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ GetConfigVolumeMount(), GetScriptVolumeMount(), - GetKollaConfigVolumeMount("nova-manage"), + GetKollaConfigVolumeMount("cell-mapping"), }, }, }, diff --git a/pkg/nova/common.go b/pkg/nova/common.go index 4e0f0a73d..6f20a5257 100644 --- a/pkg/nova/common.go +++ b/pkg/nova/common.go @@ -24,12 +24,15 @@ import ( const ( // KollaServiceCommand - the command to start the service binary in the kolla container - KollaServiceCommand = "/usr/local/bin/kolla_set_configs && /usr/local/bin/kolla_start" + KollaServiceCommand = "/usr/local/bin/kolla_start" // NovaAPIDatabaseName - the name of the DB to store tha API schema NovaAPIDatabaseName = "nova_api" // NovaCell0DatabaseName - the name of the DB to store the cell schema for // cell0 NovaCell0DatabaseName = "nova_cell0" + // NovaUserID is the linux user ID used by Kolla for the nova user + // in the service containers + NovaUserID int64 = 42436 ) // GetScriptSecretName returns the name of the Secret used for the diff --git a/pkg/nova/host_discovery.go b/pkg/nova/host_discover.go similarity index 90% rename from pkg/nova/host_discovery.go rename to pkg/nova/host_discover.go index 1505eb8bf..c7104cc0e 100644 --- a/pkg/nova/host_discovery.go +++ b/pkg/nova/host_discover.go @@ -22,10 +22,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -const ( - discoverCommand = "/usr/local/bin/kolla_set_configs && /var/lib/openstack/bin/host_discover.sh" + "k8s.io/utils/ptr" ) func HostDiscoveryJob( @@ -35,13 +32,11 @@ func HostDiscoveryJob( inputHash string, labels map[string]string, ) *batchv1.Job { - runAsUser := int64(0) - args := []string{"-c"} if instance.Spec.Debug.StopJob { args = append(args, common.DebugCommand) } else { - args = append(args, discoverCommand) + args = append(args, KollaServiceCommand) } envVars := map[string]env.Setter{} @@ -81,13 +76,13 @@ func HostDiscoveryJob( Args: args, Image: instance.Spec.ConductorServiceTemplate.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ GetConfigVolumeMount(), GetScriptVolumeMount(), - GetKollaConfigVolumeMount("nova-manage"), + GetKollaConfigVolumeMount("host-discover"), }, }, }, diff --git a/pkg/nova/volumes.go b/pkg/nova/volumes.go index 5caf2025a..df01fc08f 100644 --- a/pkg/nova/volumes.go +++ b/pkg/nova/volumes.go @@ -28,7 +28,9 @@ const ( var ( configMode int32 = 0640 - scriptMode int32 = 0740 + // NOTE(gibi): scripts are mounted as root but kolla runs them with + // the nova user. Hence the mode needs to be wide open here. + scriptMode int32 = 0777 ) func GetConfigVolumeMount() corev1.VolumeMount { diff --git a/pkg/novaapi/deployment.go b/pkg/novaapi/deployment.go index 39658c047..2d633f202 100644 --- a/pkg/novaapi/deployment.go +++ b/pkg/novaapi/deployment.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" ) // StatefulSet - returns the StatefulSet definition for the nova-api service @@ -36,8 +37,6 @@ func StatefulSet( labels map[string]string, annotations map[string]string, ) *appsv1.StatefulSet { - runAsUser := int64(0) - // This allows the pod to start up slowly. The pod will only be killed // if it does not succeed a probe in 60 seconds. startupProbe := &corev1.Probe{ @@ -135,7 +134,7 @@ func StatefulSet( Args: []string{"-c", "tail -n+1 -F /var/log/nova/nova-api.log"}, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(nova.NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{nova.GetLogVolumeMount()}, @@ -152,7 +151,7 @@ func StatefulSet( Args: args, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(nova.NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ diff --git a/pkg/novacompute/deployment.go b/pkg/novacompute/deployment.go index cf2450031..abef38d88 100644 --- a/pkg/novacompute/deployment.go +++ b/pkg/novacompute/deployment.go @@ -26,6 +26,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ) // StatefulSet - returns the StatefulSet definition for the nova-compute service @@ -35,8 +36,6 @@ func StatefulSet( labels map[string]string, annotations map[string]string, ) *appsv1.StatefulSet { - runAsUser := int64(0) - // After the first successful startupProbe, livenessProbe takes over livenessProbe := &corev1.Probe{ // TODO might need tuning @@ -122,7 +121,7 @@ func StatefulSet( Args: args, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(nova.NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ diff --git a/pkg/novaconductor/dbsync.go b/pkg/novaconductor/dbsync.go index 36ae4d118..8dc324060 100644 --- a/pkg/novaconductor/dbsync.go +++ b/pkg/novaconductor/dbsync.go @@ -26,11 +26,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -const ( - // cellDBSyncCommand - the command to be used to run db sync for the cell DB - cellDBSyncCommand = "/usr/local/bin/kolla_set_configs && /bin/sh -c /var/lib/openstack/bin/dbsync.sh" + "k8s.io/utils/ptr" ) // CellDBSyncJob - define a batchv1.Job to be run to apply the cel DB schema @@ -39,13 +35,11 @@ func CellDBSyncJob( labels map[string]string, annotations map[string]string, ) *batchv1.Job { - runAsUser := int64(0) - args := []string{"-c"} if instance.Spec.Debug.StopJob { args = append(args, common.DebugCommand) } else { - args = append(args, cellDBSyncCommand) + args = append(args, nova.KollaServiceCommand) } envVars := map[string]env.Setter{} @@ -81,13 +75,13 @@ func CellDBSyncJob( Args: args, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(nova.NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ nova.GetConfigVolumeMount(), nova.GetScriptVolumeMount(), - nova.GetKollaConfigVolumeMount("nova-conductor"), + nova.GetKollaConfigVolumeMount("nova-conductor-dbsync"), }, }, }, diff --git a/pkg/novaconductor/deployment.go b/pkg/novaconductor/deployment.go index 4cf586606..91233bfba 100644 --- a/pkg/novaconductor/deployment.go +++ b/pkg/novaconductor/deployment.go @@ -26,6 +26,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ) // StatefulSet - returns the StatefulSet definition for the nova-api service @@ -35,8 +36,6 @@ func StatefulSet( labels map[string]string, annotations map[string]string, ) *appsv1.StatefulSet { - runAsUser := int64(0) - livenessProbe := &corev1.Probe{ // TODO might need tuning TimeoutSeconds: 5, @@ -128,7 +127,7 @@ func StatefulSet( Args: args, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(nova.NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ diff --git a/pkg/novametadata/deployment.go b/pkg/novametadata/deployment.go index 2d864a9d8..4fb77b6bf 100644 --- a/pkg/novametadata/deployment.go +++ b/pkg/novametadata/deployment.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" ) // StatefulSet - returns the StatefulSet definition for the nova-metadata service @@ -36,8 +37,6 @@ func StatefulSet( labels map[string]string, annotations map[string]string, ) *appsv1.StatefulSet { - runAsUser := int64(0) - // This allows the pod to start up slowly. The pod will only be killed // if it does not succeed a probe in 60 seconds. startupProbe := &corev1.Probe{ @@ -135,7 +134,7 @@ func StatefulSet( Args: []string{"-c", "tail -n+1 -F /var/log/nova/nova-metadata.log"}, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(nova.NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ @@ -154,7 +153,7 @@ func StatefulSet( Args: args, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(nova.NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ diff --git a/pkg/novascheduler/deployment.go b/pkg/novascheduler/deployment.go index 365711028..9724e8a1e 100644 --- a/pkg/novascheduler/deployment.go +++ b/pkg/novascheduler/deployment.go @@ -26,6 +26,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ) // StatefulSet - returns the StatefulSet definition for the nova-scheduler service @@ -35,8 +36,6 @@ func StatefulSet( labels map[string]string, annotations map[string]string, ) *appsv1.StatefulSet { - runAsUser := int64(0) - // This allows the pod to start up slowly. The pod will only be killed // if it does not succeed a probe in 60 seconds. startupProbe := &corev1.Probe{ @@ -137,7 +136,7 @@ func StatefulSet( Args: args, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(nova.NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ diff --git a/pkg/novncproxy/deployment.go b/pkg/novncproxy/deployment.go index 6d284f8bc..e9f89646a 100644 --- a/pkg/novncproxy/deployment.go +++ b/pkg/novncproxy/deployment.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" ) // StatefulSet - returns the StatefulSet definition for the nova-novanovncproxy service @@ -36,8 +37,6 @@ func StatefulSet( labels map[string]string, annotations map[string]string, ) *appsv1.StatefulSet { - runAsUser := int64(0) - // This allows the pod to start up slowly. The pod will only be killed // if it does not succeed a probe in 60 seconds. startupProbe := &corev1.Probe{ @@ -135,7 +134,7 @@ func StatefulSet( Args: args, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(nova.NovaUserID), }, Env: env, VolumeMounts: []corev1.VolumeMount{ diff --git a/templates/nova-manage/config/cell-mapping-config.json b/templates/nova-manage/config/cell-mapping-config.json new file mode 100644 index 000000000..9951f5351 --- /dev/null +++ b/templates/nova-manage/config/cell-mapping-config.json @@ -0,0 +1,24 @@ +{ + "command": "/var/lib/openstack/bin/ensure_cell_mapping.sh", + "config_files": [ + { + "source": "/var/lib/openstack/config/nova-blank.conf", + "dest": "/etc/nova/nova.conf", + "owner": "nova", + "perm": "0600" + }, + { + "source": "/var/lib/openstack/config/01-nova.conf", + "dest": "/etc/nova/nova.conf.d/01-nova.conf", + "owner": "nova", + "perm": "0600" + }, + { + "source": "/var/lib/openstack/config/02-nova-override.conf", + "dest": "/etc/nova/nova.conf.d/02-nova-override.conf", + "owner": "nova", + "perm": "0600", + "optional": true + } + ] +} diff --git a/templates/nova-manage/config/nova-manage-config.json b/templates/nova-manage/config/host-discover-config.json similarity index 91% rename from templates/nova-manage/config/nova-manage-config.json rename to templates/nova-manage/config/host-discover-config.json index 51543e14a..6c02d6600 100644 --- a/templates/nova-manage/config/nova-manage-config.json +++ b/templates/nova-manage/config/host-discover-config.json @@ -1,5 +1,5 @@ { - "command": "", + "command": "/var/lib/openstack/bin/host_discover.sh", "config_files": [ { "source": "/var/lib/openstack/config/nova-blank.conf", diff --git a/templates/novaapi/config/httpd.conf b/templates/novaapi/config/httpd.conf index 428a2214f..17e35370f 100644 --- a/templates/novaapi/config/httpd.conf +++ b/templates/novaapi/config/httpd.conf @@ -18,6 +18,8 @@ LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" combine LogFormat "%{X-Forwarded-For}i %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" proxy SetEnvIf X-Forwarded-For "^.*\..*\..*\..*" forwarded +ErrorLog /dev/stderr +TransferLog /dev/stdout CustomLog /dev/stdout combined env=!forwarded CustomLog /dev/stdout proxy env=forwarded ## set default apache log level to info from warning diff --git a/templates/novaapi/config/nova-api-config.json b/templates/novaapi/config/nova-api-config.json index caa432f61..5d4a9edde 100644 --- a/templates/novaapi/config/nova-api-config.json +++ b/templates/novaapi/config/nova-api-config.json @@ -32,6 +32,11 @@ "path": "/var/log/nova", "owner": "nova:apache", "recurse": true + }, + { + "path": "/etc/httpd/run/", + "owner": "nova:apache", + "recurse": true } ] } diff --git a/templates/novaconductor/config/nova-conductor-dbsync-config.json b/templates/novaconductor/config/nova-conductor-dbsync-config.json new file mode 100644 index 000000000..a534f90bf --- /dev/null +++ b/templates/novaconductor/config/nova-conductor-dbsync-config.json @@ -0,0 +1,31 @@ +{ + "command": "/var/lib/openstack/bin/dbsync.sh", + "config_files": [ + { + "source": "/var/lib/openstack/config/nova-blank.conf", + "dest": "/etc/nova/nova.conf", + "owner": "nova", + "perm": "0600" + }, + { + "source": "/var/lib/openstack/config/01-nova.conf", + "dest": "/etc/nova/nova.conf.d/01-nova.conf", + "owner": "nova", + "perm": "0600" + }, + { + "source": "/var/lib/openstack/config/02-nova-override.conf", + "dest": "/etc/nova/nova.conf.d/02-nova-override.conf", + "owner": "nova", + "perm": "0600", + "optional": true + } + ], + "permissions": [ + { + "path": "/var/log/nova", + "owner": "nova:nova", + "recurse": true + } + ] +} diff --git a/templates/novametadata/config/httpd.conf b/templates/novametadata/config/httpd.conf index ba9040295..b66a707c0 100644 --- a/templates/novametadata/config/httpd.conf +++ b/templates/novametadata/config/httpd.conf @@ -18,6 +18,8 @@ LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" combine LogFormat "%{X-Forwarded-For}i %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" proxy SetEnvIf X-Forwarded-For "^.*\..*\..*\..*" forwarded +ErrorLog /dev/stderr +TransferLog /dev/stdout CustomLog /dev/stdout combined env=!forwarded CustomLog /dev/stdout proxy env=forwarded ## set default apache log level to info from warning diff --git a/templates/novametadata/config/nova-metadata-config.json b/templates/novametadata/config/nova-metadata-config.json index 2b1e7c2b8..d005385e6 100644 --- a/templates/novametadata/config/nova-metadata-config.json +++ b/templates/novametadata/config/nova-metadata-config.json @@ -32,6 +32,11 @@ "path": "/var/log/nova", "owner": "nova:apache", "recurse": true + }, + { + "path": "/etc/httpd/run/", + "owner": "nova:apache", + "recurse": true } ] } diff --git a/test/functional/novaconductor_controller_test.go b/test/functional/novaconductor_controller_test.go index 798174591..75e2e219a 100644 --- a/test/functional/novaconductor_controller_test.go +++ b/test/functional/novaconductor_controller_test.go @@ -226,7 +226,6 @@ var _ = Describe("NovaConductor controller", func() { Expect(job.Spec.Template.Spec.Containers).To(HaveLen(1)) container := job.Spec.Template.Spec.Containers[0] Expect(container.VolumeMounts).To(HaveLen(3)) - Expect(container.Args[1]).To(ContainSubstring("dbsync.sh")) Expect(container.Image).To(Equal(ContainerImage)) })