Skip to content

Commit 9b28021

Browse files
perdasilvabentitoPer Goncalves da Silva
authored
OCPBUGS-29729: Updates default security context behavior for catalog source pods (#3206)
* Refactor security context configuration in pod reconciler This change updates the logic for setting security contexts within the OLM pod reconciler. Now, it differentiates between 'Restricted' and 'Legacy' security contexts more explicitly. The 'Restricted' security context applies default security settings unless overridden, while the 'Legacy' context clears all security settings. When no security context is configured, it defaults to restricted. Additionally, the related tests have been updated to reflect these changes and ensure correct behavior. Signed-off-by: btofel <[email protected]> * Add checking of the namespace PSA restrictions Signed-off-by: btofel <[email protected]> * Fix linter issues Signed-off-by: btofel <[email protected]> Signed-off-by: Brett Tofel <[email protected]> * fixes Signed-off-by: Per Goncalves da Silva <[email protected]> --------- Signed-off-by: btofel <[email protected]> Signed-off-by: Brett Tofel <[email protected]> Signed-off-by: Per Goncalves da Silva <[email protected]> Co-authored-by: Brett Tofel <[email protected]> Co-authored-by: Per Goncalves da Silva <[email protected]>
1 parent 01d4a00 commit 9b28021

File tree

7 files changed

+416
-134
lines changed

7 files changed

+416
-134
lines changed

‎pkg/controller/operators/catalog/operator_test.go

+160-5
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ import (
1313
"testing/quick"
1414
"time"
1515

16+
"k8s.io/utils/ptr"
17+
18+
controllerclient "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/controller-runtime/client"
19+
1620
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
1721

1822
"github.com/sirupsen/logrus"
@@ -59,7 +63,6 @@ import (
5963
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/solver"
6064
"github.com/operator-framework/operator-lifecycle-manager/pkg/fakes"
6165
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/clientfake"
62-
controllerclient "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/controller-runtime/client"
6366
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
6467
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
6568
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
@@ -838,6 +841,160 @@ func withStatus(catalogSource v1alpha1.CatalogSource, status v1alpha1.CatalogSou
838841
return copy
839842
}
840843

844+
func TestSyncCatalogSourcesSecurityPolicy(t *testing.T) {
845+
assertLegacySecurityPolicy := func(t *testing.T, pod *corev1.Pod) {
846+
require.Nil(t, pod.Spec.SecurityContext)
847+
require.Equal(t, &corev1.SecurityContext{
848+
ReadOnlyRootFilesystem: ptr.To(false),
849+
}, pod.Spec.Containers[0].SecurityContext)
850+
}
851+
852+
assertRestrictedPolicy := func(t *testing.T, pod *corev1.Pod) {
853+
require.Equal(t, &corev1.PodSecurityContext{
854+
SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault},
855+
RunAsNonRoot: ptr.To(true),
856+
RunAsUser: ptr.To(int64(1001)),
857+
}, pod.Spec.SecurityContext)
858+
require.Equal(t, &corev1.SecurityContext{
859+
ReadOnlyRootFilesystem: ptr.To(false),
860+
AllowPrivilegeEscalation: ptr.To(false),
861+
Capabilities: &corev1.Capabilities{
862+
Drop: []corev1.Capability{"ALL"},
863+
},
864+
}, pod.Spec.Containers[0].SecurityContext)
865+
}
866+
867+
clockFake := utilclocktesting.NewFakeClock(time.Date(2018, time.January, 26, 20, 40, 0, 0, time.UTC))
868+
tests := []struct {
869+
testName string
870+
namespace *corev1.Namespace
871+
catalogSource *v1alpha1.CatalogSource
872+
check func(*testing.T, *corev1.Pod)
873+
}{
874+
{
875+
testName: "UnlabeledNamespace/NoUserPreference/LegacySecurityPolicy",
876+
namespace: &corev1.Namespace{
877+
ObjectMeta: metav1.ObjectMeta{
878+
Name: "cool-namespace",
879+
},
880+
},
881+
catalogSource: &v1alpha1.CatalogSource{
882+
ObjectMeta: metav1.ObjectMeta{
883+
Name: "cool-catalog",
884+
Namespace: "cool-namespace",
885+
UID: types.UID("catalog-uid"),
886+
},
887+
Spec: v1alpha1.CatalogSourceSpec{
888+
Image: "catalog-image",
889+
SourceType: v1alpha1.SourceTypeGrpc,
890+
},
891+
},
892+
check: assertLegacySecurityPolicy,
893+
}, {
894+
testName: "UnlabeledNamespace/UserPreferenceForRestricted/RestrictedSecurityPolicy",
895+
namespace: &corev1.Namespace{
896+
ObjectMeta: metav1.ObjectMeta{
897+
Name: "cool-namespace",
898+
},
899+
},
900+
catalogSource: &v1alpha1.CatalogSource{
901+
ObjectMeta: metav1.ObjectMeta{
902+
Name: "cool-catalog",
903+
Namespace: "cool-namespace",
904+
UID: types.UID("catalog-uid"),
905+
},
906+
Spec: v1alpha1.CatalogSourceSpec{
907+
Image: "catalog-image",
908+
SourceType: v1alpha1.SourceTypeGrpc,
909+
GrpcPodConfig: &v1alpha1.GrpcPodConfig{
910+
SecurityContextConfig: v1alpha1.Restricted,
911+
},
912+
},
913+
},
914+
check: assertRestrictedPolicy,
915+
}, {
916+
testName: "LabeledNamespace/NoUserPreference/RestrictedSecurityPolicy",
917+
namespace: &corev1.Namespace{
918+
ObjectMeta: metav1.ObjectMeta{
919+
Name: "cool-namespace",
920+
Labels: map[string]string{
921+
// restricted is the default psa policy
922+
"pod-security.kubernetes.io/enforce": "restricted",
923+
},
924+
},
925+
},
926+
catalogSource: &v1alpha1.CatalogSource{
927+
ObjectMeta: metav1.ObjectMeta{
928+
Name: "cool-catalog",
929+
Namespace: "cool-namespace",
930+
UID: types.UID("catalog-uid"),
931+
},
932+
Spec: v1alpha1.CatalogSourceSpec{
933+
Image: "catalog-image",
934+
SourceType: v1alpha1.SourceTypeGrpc,
935+
},
936+
},
937+
check: assertRestrictedPolicy,
938+
}, {
939+
testName: "LabeledNamespace/UserPreferenceForLegacy/LegacySecurityPolicy",
940+
namespace: &corev1.Namespace{
941+
ObjectMeta: metav1.ObjectMeta{
942+
Name: "cool-namespace",
943+
},
944+
},
945+
catalogSource: &v1alpha1.CatalogSource{
946+
ObjectMeta: metav1.ObjectMeta{
947+
Name: "cool-catalog",
948+
Namespace: "cool-namespace",
949+
UID: types.UID("catalog-uid"),
950+
},
951+
Spec: v1alpha1.CatalogSourceSpec{
952+
Image: "catalog-image",
953+
SourceType: v1alpha1.SourceTypeGrpc,
954+
GrpcPodConfig: &v1alpha1.GrpcPodConfig{
955+
SecurityContextConfig: v1alpha1.Legacy,
956+
},
957+
},
958+
},
959+
check: assertLegacySecurityPolicy,
960+
},
961+
}
962+
for _, tt := range tests {
963+
t.Run(tt.testName, func(t *testing.T) {
964+
// Create existing objects
965+
clientObjs := []runtime.Object{tt.catalogSource}
966+
967+
// Create test operator
968+
ctx, cancel := context.WithCancel(context.TODO())
969+
defer cancel()
970+
971+
op, err := NewFakeOperator(
972+
ctx,
973+
tt.namespace.GetName(),
974+
[]string{tt.namespace.GetName()},
975+
withClock(clockFake),
976+
withClientObjs(clientObjs...),
977+
)
978+
require.NoError(t, err)
979+
980+
// Because NewFakeOperator creates the namespace, we need to update the namespace to match the test case
981+
// before running the sync function
982+
_, err = op.opClient.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), tt.namespace, metav1.UpdateOptions{})
983+
require.NoError(t, err)
984+
985+
// Run sync
986+
err = op.syncCatalogSources(tt.catalogSource)
987+
require.NoError(t, err)
988+
989+
pods, err := op.opClient.KubernetesInterface().CoreV1().Pods(tt.catalogSource.Namespace).List(context.TODO(), metav1.ListOptions{})
990+
require.NoError(t, err)
991+
require.Len(t, pods.Items, 1)
992+
993+
tt.check(t, &pods.Items[0])
994+
})
995+
}
996+
}
997+
841998
func TestSyncCatalogSources(t *testing.T) {
842999
clockFake := utilclocktesting.NewFakeClock(time.Date(2018, time.January, 26, 20, 40, 0, 0, time.UTC))
8431000
now := metav1.NewTime(clockFake.Now())
@@ -1165,7 +1322,7 @@ func TestSyncCatalogSources(t *testing.T) {
11651322
if tt.expectedStatus != nil {
11661323
if tt.expectedStatus.GRPCConnectionState != nil {
11671324
updated.Status.GRPCConnectionState.LastConnectTime = now
1168-
// Ignore LastObservedState difference if an expected LastObservedState is no provided
1325+
// Ignore LastObservedState difference if an expected LastObservedState is not provided
11691326
if tt.expectedStatus.GRPCConnectionState.LastObservedState == "" {
11701327
updated.Status.GRPCConnectionState.LastObservedState = ""
11711328
}
@@ -2012,7 +2169,6 @@ func NewFakeOperator(ctx context.Context, namespace string, namespaces []string,
20122169
return nil, err
20132170
}
20142171
applier := controllerclient.NewFakeApplier(s, "testowner")
2015-
20162172
op.reconciler = reconciler.NewRegistryReconcilerFactory(lister, op.opClient, "test:pod", op.now, applier, 1001, "", "")
20172173
}
20182174

@@ -2028,7 +2184,6 @@ func NewFakeOperator(ctx context.Context, namespace string, namespaces []string,
20282184

20292185
return op, nil
20302186
}
2031-
20322187
func installPlan(name, namespace string, phase v1alpha1.InstallPlanPhase, names ...string) *v1alpha1.InstallPlan {
20332188
return &v1alpha1.InstallPlan{
20342189
ObjectMeta: metav1.ObjectMeta{
@@ -2175,7 +2330,7 @@ func pod(t *testing.T, s v1alpha1.CatalogSource) *corev1.Pod {
21752330
Name: s.GetName(),
21762331
},
21772332
}
2178-
pod, err := reconciler.Pod(&s, "registry-server", "central-opm", "central-util", s.Spec.Image, serviceAccount, s.GetLabels(), s.GetAnnotations(), 5, 10, 1001)
2333+
pod, err := reconciler.Pod(&s, "registry-server", "central-opm", "central-util", s.Spec.Image, serviceAccount, s.GetLabels(), s.GetAnnotations(), 5, 10, 1001, v1alpha1.Legacy)
21792334
if err != nil {
21802335
t.Fatal(err)
21812336
}

‎pkg/controller/registry/reconciler/configmap.go

+24-14
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ func (s *configMapCatalogSourceDecorator) Service() (*corev1.Service, error) {
109109
return svc, nil
110110
}
111111

112-
func (s *configMapCatalogSourceDecorator) Pod(image string) (*corev1.Pod, error) {
113-
pod, err := Pod(s.CatalogSource, "configmap-registry-server", "", "", image, nil, s.Labels(), s.Annotations(), 5, 5, s.runAsUser)
112+
func (s *configMapCatalogSourceDecorator) Pod(image string, defaultPodSecurityConfig v1alpha1.SecurityConfig) (*corev1.Pod, error) {
113+
pod, err := Pod(s.CatalogSource, "configmap-registry-server", "", "", image, nil, s.Labels(), s.Annotations(), 5, 5, s.runAsUser, defaultPodSecurityConfig)
114114
if err != nil {
115115
return nil, err
116116
}
@@ -238,8 +238,8 @@ func (c *ConfigMapRegistryReconciler) currentRoleBinding(source configMapCatalog
238238
return roleBinding
239239
}
240240

241-
func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceDecorator, image string) ([]*corev1.Pod, error) {
242-
protoPod, err := source.Pod(image)
241+
func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceDecorator, image string, defaultPodSecurityConfig v1alpha1.SecurityConfig) ([]*corev1.Pod, error) {
242+
protoPod, err := source.Pod(image, defaultPodSecurityConfig)
243243
if err != nil {
244244
return nil, err
245245
}
@@ -255,8 +255,8 @@ func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceD
255255
return pods, nil
256256
}
257257

258-
func (c *ConfigMapRegistryReconciler) currentPodsWithCorrectResourceVersion(source configMapCatalogSourceDecorator, image string) ([]*corev1.Pod, error) {
259-
protoPod, err := source.Pod(image)
258+
func (c *ConfigMapRegistryReconciler) currentPodsWithCorrectResourceVersion(source configMapCatalogSourceDecorator, image string, defaultPodSecurityConfig v1alpha1.SecurityConfig) ([]*corev1.Pod, error) {
259+
protoPod, err := source.Pod(image, defaultPodSecurityConfig)
260260
if err != nil {
261261
return nil, err
262262
}
@@ -288,6 +288,11 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
288288
overwrite := source.Status.RegistryServiceStatus == nil
289289
overwritePod := overwrite
290290

291+
defaultPodSecurityConfig, err := getDefaultPodContextConfig(c.OpClient, catalogSource.GetNamespace())
292+
if err != nil {
293+
return err
294+
}
295+
291296
if source.Spec.SourceType == v1alpha1.SourceTypeConfigmap || source.Spec.SourceType == v1alpha1.SourceTypeInternal {
292297
// fetch configmap first, exit early if we can't find it
293298
// we use the live client here instead of a lister since our listers are scoped to objects with the olm.managed label,
@@ -311,7 +316,7 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
311316
}
312317

313318
// recreate the pod if no existing pod is serving the latest image
314-
current, err := c.currentPodsWithCorrectResourceVersion(source, image)
319+
current, err := c.currentPodsWithCorrectResourceVersion(source, image, defaultPodSecurityConfig)
315320
if err != nil {
316321
return err
317322
}
@@ -330,11 +335,11 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
330335
if err := c.ensureRoleBinding(source, overwrite); err != nil {
331336
return errors.Wrapf(err, "error ensuring rolebinding: %s", source.RoleBinding().GetName())
332337
}
333-
pod, err := source.Pod(image)
338+
pod, err := source.Pod(image, defaultPodSecurityConfig)
334339
if err != nil {
335340
return err
336341
}
337-
if err := c.ensurePod(source, overwritePod); err != nil {
342+
if err := c.ensurePod(source, defaultPodSecurityConfig, overwritePod); err != nil {
338343
return errors.Wrapf(err, "error ensuring pod: %s", pod.GetName())
339344
}
340345
service, err := source.Service()
@@ -400,12 +405,12 @@ func (c *ConfigMapRegistryReconciler) ensureRoleBinding(source configMapCatalogS
400405
return err
401406
}
402407

403-
func (c *ConfigMapRegistryReconciler) ensurePod(source configMapCatalogSourceDecorator, overwrite bool) error {
404-
pod, err := source.Pod(c.Image)
408+
func (c *ConfigMapRegistryReconciler) ensurePod(source configMapCatalogSourceDecorator, defaultPodSecurityConfig v1alpha1.SecurityConfig, overwrite bool) error {
409+
pod, err := source.Pod(c.Image, defaultPodSecurityConfig)
405410
if err != nil {
406411
return err
407412
}
408-
currentPods, err := c.currentPods(source, c.Image)
413+
currentPods, err := c.currentPods(source, c.Image, defaultPodSecurityConfig)
409414
if err != nil {
410415
return err
411416
}
@@ -460,6 +465,11 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
460465
return
461466
}
462467

468+
defaultPodSecurityConfig, err := getDefaultPodContextConfig(c.OpClient, catalogSource.GetNamespace())
469+
if err != nil {
470+
return false, err
471+
}
472+
463473
if source.Spec.SourceType == v1alpha1.SourceTypeConfigmap || source.Spec.SourceType == v1alpha1.SourceTypeInternal {
464474
// we use the live client here instead of a lister since our listers are scoped to objects with the olm.managed label,
465475
// and this configmap is a user-provided input to the catalog source and will not have that label
@@ -473,7 +483,7 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
473483
}
474484

475485
// recreate the pod if no existing pod is serving the latest image
476-
current, err := c.currentPodsWithCorrectResourceVersion(source, image)
486+
current, err := c.currentPodsWithCorrectResourceVersion(source, image, defaultPodSecurityConfig)
477487
if err != nil {
478488
return false, err
479489
}
@@ -489,7 +499,7 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
489499
if err != nil {
490500
return false, err
491501
}
492-
pods, err := c.currentPods(source, c.Image)
502+
pods, err := c.currentPods(source, c.Image, defaultPodSecurityConfig)
493503
if err != nil {
494504
return false, err
495505
}

0 commit comments

Comments
 (0)