From 0839d9a37a4a87c78b1fba61ccb45a390119cc14 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Thu, 16 Nov 2023 15:36:35 +0100 Subject: [PATCH] Do not run placement service as root This did not removed the root usage from the init container. We should get rid of the init container instead. (See #64) Implements: https://issues.redhat.com/browse/OSPRH-1374 --- go.mod | 2 +- pkg/placement/const.go | 6 ++++++ pkg/placement/dbsync.go | 14 ++++---------- pkg/placement/deployment.go | 14 ++++---------- pkg/placement/volumes.go | 4 ++-- templates/placementapi/config/httpd.conf | 2 ++ .../config/placement-api-config.json | 13 +++++++++---- .../config/placement-dbsync-config.json | 17 +++++++++++++++++ .../functional/placementapi_controller_test.go | 1 - .../kuttl/common/assert_sample_deployment.yaml | 2 +- 10 files changed, 46 insertions(+), 29 deletions(-) create mode 100644 templates/placementapi/config/placement-dbsync-config.json diff --git a/go.mod b/go.mod index ecb55607..16821571 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( k8s.io/api v0.26.11 k8s.io/apimachinery v0.26.11 k8s.io/client-go v0.26.11 + k8s.io/utils v0.0.0-20231127182322-b307cd553661 sigs.k8s.io/controller-runtime v0.14.7 ) @@ -79,7 +80,6 @@ require ( k8s.io/component-base v0.26.11 //indirect k8s.io/klog/v2 v2.100.1 // indirect k8s.io/kube-openapi v0.0.0-20230308215209-15aac26d736a //indirect - k8s.io/utils v0.0.0-20231127182322-b307cd553661 //indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd //indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect sigs.k8s.io/yaml v1.3.0 // indirect diff --git a/pkg/placement/const.go b/pkg/placement/const.go index c5d950b3..318cf8e1 100644 --- a/pkg/placement/const.go +++ b/pkg/placement/const.go @@ -25,4 +25,10 @@ const ( PlacementPublicPort int32 = 8778 // PlacementInternalPort - PlacementInternalPort int32 = 8778 + + KollaServiceCommand = "/usr/local/bin/kolla_start" + + // PlacementUserID is the linux user ID used by Kolla for the placement + // user in the service containers + PlacementUserID int64 = 42482 ) diff --git a/pkg/placement/dbsync.go b/pkg/placement/dbsync.go index 517595f6..87e28b59 100644 --- a/pkg/placement/dbsync.go +++ b/pkg/placement/dbsync.go @@ -24,11 +24,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -const ( - // DBSyncCommand - - DBSyncCommand = "/usr/local/bin/kolla_set_configs && su -s /bin/sh -c \"placement-manage db sync\" placement" + "k8s.io/utils/ptr" ) // DbSyncJob func @@ -37,13 +33,11 @@ func DbSyncJob( labels map[string]string, annotations map[string]string, ) *batchv1.Job { - runAsUser := int64(0) - args := []string{"-c"} if instance.Spec.Debug.DBSync { args = append(args, common.DebugCommand) } else { - args = append(args, DBSyncCommand) + args = append(args, KollaServiceCommand) } envVars := map[string]env.Setter{} @@ -73,10 +67,10 @@ func DbSyncJob( Args: args, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(PlacementUserID), }, Env: env.MergeEnvs([]corev1.EnvVar{}, envVars), - VolumeMounts: getVolumeMounts(), + VolumeMounts: getVolumeMounts("dbsync"), }, }, }, diff --git a/pkg/placement/deployment.go b/pkg/placement/deployment.go index 09535db3..08bb80ed 100644 --- a/pkg/placement/deployment.go +++ b/pkg/placement/deployment.go @@ -26,11 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" -) - -const ( - // ServiceCommand - - ServiceCommand = "/usr/local/bin/kolla_set_configs && /usr/local/bin/kolla_start" + "k8s.io/utils/ptr" ) // Deployment func @@ -40,8 +36,6 @@ func Deployment( labels map[string]string, annotations map[string]string, ) *appsv1.Deployment { - runAsUser := int64(0) - livenessProbe := &corev1.Probe{ // TODO might need tuning TimeoutSeconds: 5, @@ -70,7 +64,7 @@ func Deployment( }, } } else { - args = append(args, ServiceCommand) + args = append(args, KollaServiceCommand) // // https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ // @@ -112,10 +106,10 @@ func Deployment( Args: args, Image: instance.Spec.ContainerImage, SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, + RunAsUser: ptr.To(PlacementUserID), }, Env: env.MergeEnvs([]corev1.EnvVar{}, envVars), - VolumeMounts: getVolumeMounts(), + VolumeMounts: getVolumeMounts("api"), Resources: instance.Spec.Resources, ReadinessProbe: readinessProbe, LivenessProbe: livenessProbe, diff --git a/pkg/placement/volumes.go b/pkg/placement/volumes.go index db907e4d..eec541fc 100644 --- a/pkg/placement/volumes.go +++ b/pkg/placement/volumes.go @@ -79,7 +79,7 @@ func getInitVolumeMounts() []corev1.VolumeMount { } // getVolumeMounts - general VolumeMounts -func getVolumeMounts() []corev1.VolumeMount { +func getVolumeMounts(serviceName string) []corev1.VolumeMount { return []corev1.VolumeMount{ { Name: "scripts", @@ -94,7 +94,7 @@ func getVolumeMounts() []corev1.VolumeMount { { Name: "config-data-merged", MountPath: "/var/lib/kolla/config_files/config.json", - SubPath: "placement-api-config.json", + SubPath: "placement-" + serviceName + "-config.json", ReadOnly: true, }, } diff --git a/templates/placementapi/config/httpd.conf b/templates/placementapi/config/httpd.conf index f176577c..a42952e2 100644 --- a/templates/placementapi/config/httpd.conf +++ b/templates/placementapi/config/httpd.conf @@ -23,6 +23,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 diff --git a/templates/placementapi/config/placement-api-config.json b/templates/placementapi/config/placement-api-config.json index 08e38a25..d7db9837 100644 --- a/templates/placementapi/config/placement-api-config.json +++ b/templates/placementapi/config/placement-api-config.json @@ -16,20 +16,25 @@ { "source": "/var/lib/config-data/merged/httpd.conf", "dest": "/etc/httpd/conf/httpd.conf", - "owner": "root", + "owner": "apache", "perm": "0644" }, { "source": "/var/lib/config-data/merged/logging.conf", "dest": "/etc/placement/logging.conf", - "owner": "root", - "perm": "0644" + "owner": "placement", + "perm": "0600" } ], "permissions": [ { "path": "/var/log/placement", - "owner": "placement:placement", + "owner": "placement:apache", + "recurse": true + }, + { + "path": "/etc/httpd/run/", + "owner": "placement:apache", "recurse": true } ] diff --git a/templates/placementapi/config/placement-dbsync-config.json b/templates/placementapi/config/placement-dbsync-config.json new file mode 100644 index 00000000..4a7ea0d6 --- /dev/null +++ b/templates/placementapi/config/placement-dbsync-config.json @@ -0,0 +1,17 @@ +{ + "command": "placement-manage db sync", + "config_files": [ + { + "source": "/var/lib/config-data/merged/placement.conf", + "dest": "/etc/placement/placement.conf", + "owner": "placement", + "perm": "0600" + }, + { + "source": "/var/lib/config-data/merged/custom.conf", + "dest": "/etc/placement/placement.conf.d/custom.conf", + "owner": "placement", + "perm": "0600" + } + ] +} diff --git a/tests/functional/placementapi_controller_test.go b/tests/functional/placementapi_controller_test.go index 54e0fdb7..ca9d6dd6 100644 --- a/tests/functional/placementapi_controller_test.go +++ b/tests/functional/placementapi_controller_test.go @@ -350,7 +350,6 @@ var _ = Describe("PlacementAPI controller", func() { container := job.Spec.Template.Spec.Containers[0] Expect(container.VolumeMounts).To(HaveLen(3)) - Expect(container.Args[1]).To(ContainSubstring("placement-manage db sync")) Expect(container.Image).To(Equal("quay.io/podified-antelope-centos9/openstack-placement-api:current-podified")) th.SimulateJobSuccess(names.DBSyncJobName) diff --git a/tests/kuttl/common/assert_sample_deployment.yaml b/tests/kuttl/common/assert_sample_deployment.yaml index c65fbf64..fa0adcc4 100644 --- a/tests/kuttl/common/assert_sample_deployment.yaml +++ b/tests/kuttl/common/assert_sample_deployment.yaml @@ -103,7 +103,7 @@ spec: containers: - args: - -c - - /usr/local/bin/kolla_set_configs && /usr/local/bin/kolla_start + - /usr/local/bin/kolla_start command: - /bin/bash imagePullPolicy: IfNotPresent