Skip to content

Commit

Permalink
enable default strict security context
Browse files Browse the repository at this point in the history
  • Loading branch information
Haleygo committed Jul 18, 2023
1 parent db2dc44 commit 8173cd6
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 49 deletions.
6 changes: 3 additions & 3 deletions controllers/factory/alertmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,13 +496,13 @@ func makeStatefulSetSpec(cr *victoriametricsv1beta1.VMAlertmanager, c *config.Ba
NodeSelector: cr.Spec.NodeSelector,
PriorityClassName: cr.Spec.PriorityClassName,
TerminationGracePeriodSeconds: &terminationGracePeriod,
InitContainers: cr.Spec.InitContainers,
Containers: containers,
InitContainers: addStrictSecuritySettingsToContainers(cr.Spec.InitContainers, c.EnableStrictSecurity),
Containers: addStrictSecuritySettingsToContainers(containers, c.EnableStrictSecurity),
Volumes: volumes,
RuntimeClassName: cr.Spec.RuntimeClassName,
SchedulerName: cr.Spec.SchedulerName,
ServiceAccountName: cr.GetServiceAccountName(),
SecurityContext: cr.Spec.SecurityContext,
SecurityContext: addStrictSecuritySettingsToPod(cr.Spec.SecurityContext, c.EnableStrictSecurity),
Tolerations: cr.Spec.Tolerations,
Affinity: cr.Spec.Affinity,
HostNetwork: cr.Spec.HostNetwork,
Expand Down
37 changes: 32 additions & 5 deletions controllers/factory/builders.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ package factory
import (
"context"
"fmt"
"strings"

"github.com/VictoriaMetrics/operator/controllers/factory/k8stools"
v2 "k8s.io/api/autoscaling/v2"
"k8s.io/api/autoscaling/v2beta2"
policyv1 "k8s.io/api/policy/v1"
policyv1beta1 "k8s.io/api/policy/v1beta1"
"strings"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
Expand All @@ -20,6 +21,7 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand Down Expand Up @@ -266,7 +268,6 @@ func buildDefaultPDB(cr svcBuilderArgs, spec *victoriametricsv1beta1.EmbeddedPod
}

func reconcilePDBV1(ctx context.Context, rclient client.Client, crdName string, pdb *policyv1.PodDisruptionBudget) error {

currentPdb := &policyv1.PodDisruptionBudget{}
err := rclient.Get(ctx, types.NamespacedName{Namespace: pdb.Namespace, Name: pdb.Name}, currentPdb)
if err != nil {
Expand All @@ -286,7 +287,6 @@ func reconcilePDBV1(ctx context.Context, rclient client.Client, crdName string,
}

func reconcilePDB(ctx context.Context, rclient client.Client, crdName string, pdb *policyv1beta1.PodDisruptionBudget) error {

currentPdb := &policyv1beta1.PodDisruptionBudget{}
err := rclient.Get(ctx, types.NamespacedName{Namespace: pdb.Namespace, Name: pdb.Name}, currentPdb)
if err != nil {
Expand Down Expand Up @@ -392,7 +392,6 @@ func buildProbe(container v1.Container, cr probeCRD) v1.Container {
probe.SuccessThreshold = 1
}
}

}
addMissingFields(lp)
addMissingFields(sp)
Expand Down Expand Up @@ -437,7 +436,6 @@ func buildHPASpec(targetRef v2beta2.CrossVersionObjectReference, spec *victoriam
Behavior: k8stools.MustConvertObjectVersionsJSON[v2beta2.HorizontalPodAutoscalerBehavior, v2.HorizontalPodAutoscalerBehavior](spec.Behaviour, "v2beta/autoscaling/behaviorSpec"),
},
}

}

func reconcileHPA(ctx context.Context, rclient client.Client, targetHPA client.Object) error {
Expand Down Expand Up @@ -525,3 +523,32 @@ func formatContainerImage(globalRepo string, containerImage string) string {
}
return globalRepo + containerImage[len("quay.io/"):]
}

func addStrictSecuritySettingsToPod(p *v1.PodSecurityContext, enableStrictSecurity bool) *v1.PodSecurityContext {
if !enableStrictSecurity || p != nil {
return p
}
return &v1.PodSecurityContext{
RunAsNonRoot: pointer.Bool(true),
// 'nobody' has uid `65534' in alpine image
RunAsUser: pointer.Int64(65534),
RunAsGroup: pointer.Int64(65534),
FSGroup: pointer.Int64(65534),
}
}

func addStrictSecuritySettingsToContainers(containers []v1.Container, enableStrictSecurity bool) []v1.Container {
if !enableStrictSecurity {
return containers
}
for idx := range containers {
container := &containers[idx]
if container.SecurityContext == nil {
container.SecurityContext = &v1.SecurityContext{
ReadOnlyRootFilesystem: pointer.Bool(true),
AllowPrivilegeEscalation: pointer.Bool(false),
}
}
}
return containers
}
124 changes: 122 additions & 2 deletions controllers/factory/builders_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,18 @@ package factory
import (
"context"
"fmt"
"github.com/stretchr/testify/assert"
"testing"

"github.com/stretchr/testify/assert"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/k8stools"
"github.com/go-test/deep"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/pointer"
)

func Test_reconcileServiceForCRD(t *testing.T) {
Expand Down Expand Up @@ -415,7 +417,6 @@ func (t testBuildProbeCR) ProbeScheme() string {
}

func (t testBuildProbeCR) ProbePort() string {

return t.port
}

Expand Down Expand Up @@ -631,3 +632,122 @@ func TestFormatContainerImage(t *testing.T) {
// correct behaviour, user must fix image naming
f("private.github.io", "my-private.registry/victoria-metrics/storage", "private.github.io/my-private.registry/victoria-metrics/storage")
}

func TestAddStrictSecuritySettingsToPod(t *testing.T) {
type args struct {
podSecurityPolicy *v1.PodSecurityContext
enableStrictSecurity bool
exp *v1.PodSecurityContext
}
tests := []struct {
name string
args args
validate func(svc *v1.Service) error
}{
{
name: "enforce strict security",
args: args{
enableStrictSecurity: true,
exp: &v1.PodSecurityContext{
RunAsNonRoot: pointer.Bool(true),
RunAsUser: pointer.Int64(65534),
RunAsGroup: pointer.Int64(65534),
FSGroup: pointer.Int64(65534),
},
},
},
{
name: "disable enableStrictSecurity",
args: args{
enableStrictSecurity: false,
exp: nil,
},
},
{
name: "use custom security",
args: args{
podSecurityPolicy: &v1.PodSecurityContext{
RunAsNonRoot: pointer.Bool(false),
},
enableStrictSecurity: true,
exp: &v1.PodSecurityContext{
RunAsNonRoot: pointer.Bool(false),
},
},
},
}
for _, tt := range tests {
res := addStrictSecuritySettingsToPod(tt.args.podSecurityPolicy, tt.args.enableStrictSecurity)
if diff := deep.Equal(res, tt.args.exp); len(diff) > 0 {
t.Fatalf("got unexpected result: %v, expect: %v", res, tt.args.exp)
}
}
}

func TestAddStrictSecuritySettingsToContainers(t *testing.T) {
type args struct {
containers []v1.Container
enableStrictSecurity bool
exp []v1.Container
}
tests := []struct {
name string
args args
validate func(svc *v1.Service) error
}{
{
name: "enforce strict security",
args: args{
containers: []v1.Container{
{
Name: "c1",
SecurityContext: &v1.SecurityContext{
ReadOnlyRootFilesystem: pointer.Bool(false),
},
},
{
Name: "c2",
},
},
enableStrictSecurity: true,
exp: []v1.Container{
{
Name: "c1",
SecurityContext: &v1.SecurityContext{
ReadOnlyRootFilesystem: pointer.Bool(false),
},
},
{
Name: "c2",
SecurityContext: &v1.SecurityContext{
ReadOnlyRootFilesystem: pointer.Bool(true),
AllowPrivilegeEscalation: pointer.Bool(false),
},
},
},
},
},
{
name: "disable enableStrictSecurity",
args: args{
containers: []v1.Container{
{
Name: "c2",
},
},
enableStrictSecurity: false,
exp: []v1.Container{
{
Name: "c2",
},
},
},
},
}
for _, tt := range tests {
res := addStrictSecuritySettingsToContainers(tt.args.containers, tt.args.enableStrictSecurity)
if diff := deep.Equal(res, tt.args.exp); len(diff) > 0 {
t.Fatalf("got unexpected result: %v, expect: %v", res, tt.args.exp)
}
}
}
5 changes: 3 additions & 2 deletions controllers/factory/vmagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -505,13 +505,14 @@ func makeSpecForVMAgent(cr *victoriametricsv1beta1.VMAgent, c *config.BaseOperat
return nil, fmt.Errorf("cannot apply patch for initContainers: %w", err)
}
}

return &corev1.PodSpec{
NodeSelector: cr.Spec.NodeSelector,
Volumes: volumes,
InitContainers: ic,
Containers: containers,
Containers: addStrictSecuritySettingsToContainers(containers, c.EnableStrictSecurity),
ServiceAccountName: cr.GetServiceAccountName(),
SecurityContext: cr.Spec.SecurityContext,
SecurityContext: addStrictSecuritySettingsToPod(cr.Spec.SecurityContext, c.EnableStrictSecurity),
ImagePullSecrets: cr.Spec.ImagePullSecrets,
Affinity: cr.Spec.Affinity,
SchedulerName: cr.Spec.SchedulerName,
Expand Down
25 changes: 11 additions & 14 deletions controllers/factory/vmalert.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ func createOrUpdateVMAlertSecret(ctx context.Context, rclient client.Client, cr
if len(ba.password) > 0 {
s.Data[buildRemoteSecretKey(sourcePrefix, basicAuthPasswordKey)] = []byte(ba.password)
}

}
if ha.BearerAuth != nil && len(ba.bearerValue) > 0 {
s.Data[buildRemoteSecretKey(sourcePrefix, bearerTokenKey)] = []byte(ba.bearerValue)
Expand Down Expand Up @@ -228,7 +227,6 @@ func CreateOrUpdateVMAlert(ctx context.Context, cr *victoriametricsv1beta1.VMAle

// newDeployForCR returns a busybox pod with the same name/namespace as the cr
func newDeployForVMAlert(cr *victoriametricsv1beta1.VMAlert, c *config.BaseOperatorConf, ruleConfigMapNames []string, remoteSecrets map[string]*authSecret) (*appsv1.Deployment, error) {

if cr.Spec.Image.Repository == "" {
cr.Spec.Image.Repository = c.VMAlertDefault.Image
}
Expand Down Expand Up @@ -267,7 +265,6 @@ func newDeployForVMAlert(cr *victoriametricsv1beta1.VMAlert, c *config.BaseOpera
}

func vmAlertSpecGen(cr *victoriametricsv1beta1.VMAlert, c *config.BaseOperatorConf, ruleConfigMapNames []string, remoteSecrets map[string]*authSecret) (*appsv1.DeploymentSpec, error) {

confReloadArgs := []string{
fmt.Sprintf("-webhook-url=%s", victoriametricsv1beta1.BuildReloadPathWithPort(cr.Spec.ExtraArgs, cr.Spec.Port)),
}
Expand Down Expand Up @@ -427,14 +424,15 @@ func vmAlertSpecGen(cr *victoriametricsv1beta1.VMAlert, c *config.BaseOperatorCo
}
vmalertContainer = buildProbe(vmalertContainer, cr)

vmalertContainers := []corev1.Container{vmalertContainer, {
Name: "config-reloader",
Image: fmt.Sprintf("%s", formatContainerImage(c.ContainerRegistry, c.VMAlertDefault.ConfigReloadImage)),
Args: confReloadArgs,
Resources: resources,
TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
VolumeMounts: reloaderVolumes,
},
vmalertContainers := []corev1.Container{
vmalertContainer, {
Name: "config-reloader",
Image: fmt.Sprintf("%s", formatContainerImage(c.ContainerRegistry, c.VMAlertDefault.ConfigReloadImage)),
Args: confReloadArgs,
Resources: resources,
TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
VolumeMounts: reloaderVolumes,
},
}

containers, err := k8stools.MergePatchContainers(vmalertContainers, cr.Spec.Containers)
Expand Down Expand Up @@ -476,10 +474,10 @@ func vmAlertSpecGen(cr *victoriametricsv1beta1.VMAlert, c *config.BaseOperatorCo
SchedulerName: cr.Spec.SchedulerName,
RuntimeClassName: cr.Spec.RuntimeClassName,
ServiceAccountName: cr.GetServiceAccountName(),
Containers: containers,
Containers: addStrictSecuritySettingsToContainers(containers, c.EnableStrictSecurity),
Volumes: volumes,
PriorityClassName: cr.Spec.PriorityClassName,
SecurityContext: cr.Spec.SecurityContext,
SecurityContext: addStrictSecuritySettingsToPod(cr.Spec.SecurityContext, c.EnableStrictSecurity),
Affinity: cr.Spec.Affinity,
Tolerations: cr.Spec.Tolerations,
HostNetwork: cr.Spec.HostNetwork,
Expand Down Expand Up @@ -508,7 +506,6 @@ func buildHeadersArg(flagName string, src []string, headers []string) []string {
}

func buildVMAlertAuthArgs(args []string, flagPrefix string, ha victoriametricsv1beta1.HTTPAuth, remoteSecrets map[string]*authSecret) []string {

if s, ok := remoteSecrets[flagPrefix]; ok {
// safety checks must be performed by previous code
if ha.BasicAuth != nil {
Expand Down
6 changes: 3 additions & 3 deletions controllers/factory/vmauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,10 @@ func makeSpecForVMAuth(cr *victoriametricsv1beta1.VMAuth, c *config.BaseOperator
Spec: corev1.PodSpec{
NodeSelector: cr.Spec.NodeSelector,
Volumes: volumes,
InitContainers: ic,
Containers: containers,
InitContainers: addStrictSecuritySettingsToContainers(ic, c.EnableStrictSecurity),
Containers: addStrictSecuritySettingsToContainers(containers, c.EnableStrictSecurity),
ServiceAccountName: cr.GetServiceAccountName(),
SecurityContext: cr.Spec.SecurityContext,
SecurityContext: addStrictSecuritySettingsToPod(cr.Spec.SecurityContext, c.EnableStrictSecurity),
ImagePullSecrets: cr.Spec.ImagePullSecrets,
Affinity: cr.Spec.Affinity,
RuntimeClassName: cr.Spec.RuntimeClassName,
Expand Down
Loading

0 comments on commit 8173cd6

Please sign in to comment.