Skip to content

Commit 73c129a

Browse files
author
Per Goncalves da Silva
committed
fixes
Signed-off-by: Per Goncalves da Silva <[email protected]>
1 parent b1efd17 commit 73c129a

File tree

7 files changed

+451
-279
lines changed

7 files changed

+451
-279
lines changed

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

Lines changed: 165 additions & 9 deletions
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"
@@ -837,6 +841,160 @@ func withStatus(catalogSource v1alpha1.CatalogSource, status v1alpha1.CatalogSou
837841
return copy
838842
}
839843

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+
840998
func TestSyncCatalogSources(t *testing.T) {
841999
clockFake := utilclocktesting.NewFakeClock(time.Date(2018, time.January, 26, 20, 40, 0, 0, time.UTC))
8421000
now := metav1.NewTime(clockFake.Now())
@@ -1164,7 +1322,7 @@ func TestSyncCatalogSources(t *testing.T) {
11641322
if tt.expectedStatus != nil {
11651323
if tt.expectedStatus.GRPCConnectionState != nil {
11661324
updated.Status.GRPCConnectionState.LastConnectTime = now
1167-
// Ignore LastObservedState difference if an expected LastObservedState is no provided
1325+
// Ignore LastObservedState difference if an expected LastObservedState is not provided
11681326
if tt.expectedStatus.GRPCConnectionState.LastObservedState == "" {
11691327
updated.Status.GRPCConnectionState.LastObservedState = ""
11701328
}
@@ -2005,15 +2163,13 @@ func NewFakeOperator(ctx context.Context, namespace string, namespaces []string,
20052163
}
20062164
op.sources = grpc.NewSourceStore(config.logger, 1*time.Second, 5*time.Second, op.syncSourceState)
20072165
if op.reconciler == nil {
2008-
op.reconciler = &fakes.FakeRegistryReconcilerFactory{
2009-
ReconcilerForSourceStub: func(source *v1alpha1.CatalogSource) reconciler.RegistryReconciler {
2010-
return &fakes.FakeRegistryReconciler{
2011-
EnsureRegistryServerStub: func(logger *logrus.Entry, source *v1alpha1.CatalogSource) error {
2012-
return nil
2013-
},
2014-
}
2015-
},
2166+
s := runtime.NewScheme()
2167+
err := k8sfake.AddToScheme(s)
2168+
if err != nil {
2169+
return nil, err
20162170
}
2171+
applier := controllerclient.NewFakeApplier(s, "testowner")
2172+
op.reconciler = reconciler.NewRegistryReconcilerFactory(lister, op.opClient, "test:pod", op.now, applier, 1001, "", "")
20172173
}
20182174

20192175
op.RunInformers(ctx)

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

Lines changed: 37 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ import (
2525
// configMapCatalogSourceDecorator wraps CatalogSource to add additional methods
2626
type configMapCatalogSourceDecorator struct {
2727
*v1alpha1.CatalogSource
28-
Reconciler *ConfigMapRegistryReconciler
29-
runAsUser int64
28+
runAsUser int64
3029
}
3130

3231
const (
@@ -110,32 +109,15 @@ func (s *configMapCatalogSourceDecorator) Service() (*corev1.Service, error) {
110109
return svc, nil
111110
}
112111

113-
func (s *configMapCatalogSourceDecorator) getNamespaceSecurityContextConfig() (v1alpha1.SecurityConfig, error) {
114-
namespace := s.GetNamespace()
115-
if config, ok := s.Reconciler.namespacePSAConfigCache[namespace]; ok {
116-
return config, nil
112+
func (s *configMapCatalogSourceDecorator) Pod(image string, options ...PodOptionFunc) (*corev1.Pod, error) {
113+
// define defaults and apply options
114+
opts := &PodOption{
115+
SecurityContextConfig: v1alpha1.Restricted,
117116
}
118-
// Retrieve the client from the reconciler
119-
client := s.Reconciler.OpClient
120-
121-
ns, err := client.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), s.GetNamespace(), metav1.GetOptions{})
122-
if err != nil {
123-
return "", fmt.Errorf("error fetching namespace: %v", err)
124-
}
125-
// 'pod-security.kubernetes.io/enforce' is the label used for enforcing namespace level security,
126-
// and 'restricted' is the value indicating a restricted security policy.
127-
if val, exists := ns.Labels["pod-security.kubernetes.io/enforce"]; exists && val == "restricted" {
128-
return v1alpha1.Restricted, nil
117+
for _, o := range options {
118+
o(opts)
129119
}
130-
131-
return v1alpha1.Legacy, nil
132-
}
133-
func (s *configMapCatalogSourceDecorator) Pod(image string) (*corev1.Pod, error) {
134-
securityContextConfig, err := s.getNamespaceSecurityContextConfig()
135-
if err != nil {
136-
return nil, err
137-
}
138-
pod, err := Pod(s.CatalogSource, "configmap-registry-server", "", "", image, nil, s.Labels(), s.Annotations(), 5, 5, s.runAsUser, securityContextConfig)
120+
pod, err := Pod(s.CatalogSource, "configmap-registry-server", "", "", image, nil, s.Labels(), s.Annotations(), 5, 5, s.runAsUser, opts.SecurityContextConfig)
139121
if err != nil {
140122
return nil, err
141123
}
@@ -208,12 +190,11 @@ func (s *configMapCatalogSourceDecorator) RoleBinding() *rbacv1.RoleBinding {
208190
}
209191

210192
type ConfigMapRegistryReconciler struct {
211-
now nowFunc
212-
Lister operatorlister.OperatorLister
213-
OpClient operatorclient.ClientInterface
214-
Image string
215-
createPodAsUser int64
216-
namespacePSAConfigCache map[string]v1alpha1.SecurityConfig
193+
now nowFunc
194+
Lister operatorlister.OperatorLister
195+
OpClient operatorclient.ClientInterface
196+
Image string
197+
createPodAsUser int64
217198
}
218199

219200
var _ RegistryEnsurer = &ConfigMapRegistryReconciler{}
@@ -264,8 +245,8 @@ func (c *ConfigMapRegistryReconciler) currentRoleBinding(source configMapCatalog
264245
return roleBinding
265246
}
266247

267-
func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceDecorator, image string) ([]*corev1.Pod, error) {
268-
protoPod, err := source.Pod(image)
248+
func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceDecorator, image string, podSecurityConfig v1alpha1.SecurityConfig) ([]*corev1.Pod, error) {
249+
protoPod, err := source.Pod(image, WithSecurityContextConfig(podSecurityConfig))
269250
if err != nil {
270251
return nil, err
271252
}
@@ -281,8 +262,8 @@ func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceD
281262
return pods, nil
282263
}
283264

284-
func (c *ConfigMapRegistryReconciler) currentPodsWithCorrectResourceVersion(source configMapCatalogSourceDecorator, image string) ([]*corev1.Pod, error) {
285-
protoPod, err := source.Pod(image)
265+
func (c *ConfigMapRegistryReconciler) currentPodsWithCorrectResourceVersion(source configMapCatalogSourceDecorator, image string, podSecurityConfig v1alpha1.SecurityConfig) ([]*corev1.Pod, error) {
266+
protoPod, err := source.Pod(image, WithSecurityContextConfig(podSecurityConfig))
286267
if err != nil {
287268
return nil, err
288269
}
@@ -300,10 +281,7 @@ func (c *ConfigMapRegistryReconciler) currentPodsWithCorrectResourceVersion(sour
300281

301282
// EnsureRegistryServer ensures that all components of registry server are up to date.
302283
func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry, catalogSource *v1alpha1.CatalogSource) error {
303-
if c.namespacePSAConfigCache == nil {
304-
c.namespacePSAConfigCache = make(map[string]v1alpha1.SecurityConfig)
305-
}
306-
source := configMapCatalogSourceDecorator{catalogSource, c, c.createPodAsUser}
284+
source := configMapCatalogSourceDecorator{catalogSource, c.createPodAsUser}
307285

308286
image := c.Image
309287
if source.Spec.SourceType == "grpc" {
@@ -317,6 +295,11 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
317295
overwrite := source.Status.RegistryServiceStatus == nil
318296
overwritePod := overwrite
319297

298+
defaultPodSecurityConfig, err := getNamespaceSecurityContextConfig(c.OpClient, catalogSource.GetNamespace())
299+
if err != nil {
300+
return err
301+
}
302+
320303
if source.Spec.SourceType == v1alpha1.SourceTypeConfigmap || source.Spec.SourceType == v1alpha1.SourceTypeInternal {
321304
// fetch configmap first, exit early if we can't find it
322305
// we use the live client here instead of a lister since our listers are scoped to objects with the olm.managed label,
@@ -340,7 +323,7 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
340323
}
341324

342325
// recreate the pod if no existing pod is serving the latest image
343-
current, err := c.currentPodsWithCorrectResourceVersion(source, image)
326+
current, err := c.currentPodsWithCorrectResourceVersion(source, image, defaultPodSecurityConfig)
344327
if err != nil {
345328
return err
346329
}
@@ -359,11 +342,11 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
359342
if err := c.ensureRoleBinding(source, overwrite); err != nil {
360343
return errors.Wrapf(err, "error ensuring rolebinding: %s", source.RoleBinding().GetName())
361344
}
362-
pod, err := source.Pod(image)
345+
pod, err := source.Pod(image, WithSecurityContextConfig(defaultPodSecurityConfig))
363346
if err != nil {
364347
return err
365348
}
366-
if err := c.ensurePod(source, overwritePod); err != nil {
349+
if err := c.ensurePod(source, defaultPodSecurityConfig, overwritePod); err != nil {
367350
return errors.Wrapf(err, "error ensuring pod: %s", pod.GetName())
368351
}
369352
service, err := source.Service()
@@ -429,12 +412,12 @@ func (c *ConfigMapRegistryReconciler) ensureRoleBinding(source configMapCatalogS
429412
return err
430413
}
431414

432-
func (c *ConfigMapRegistryReconciler) ensurePod(source configMapCatalogSourceDecorator, overwrite bool) error {
433-
pod, err := source.Pod(c.Image)
415+
func (c *ConfigMapRegistryReconciler) ensurePod(source configMapCatalogSourceDecorator, podSecurityConfig v1alpha1.SecurityConfig, overwrite bool) error {
416+
pod, err := source.Pod(c.Image, WithSecurityContextConfig(podSecurityConfig))
434417
if err != nil {
435418
return err
436419
}
437-
currentPods, err := c.currentPods(source, c.Image)
420+
currentPods, err := c.currentPods(source, c.Image, podSecurityConfig)
438421
if err != nil {
439422
return err
440423
}
@@ -478,7 +461,7 @@ func (c *ConfigMapRegistryReconciler) ensureService(source configMapCatalogSourc
478461

479462
// CheckRegistryServer returns true if the given CatalogSource is considered healthy; false otherwise.
480463
func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry, catalogSource *v1alpha1.CatalogSource) (healthy bool, err error) {
481-
source := configMapCatalogSourceDecorator{catalogSource, c, c.createPodAsUser}
464+
source := configMapCatalogSourceDecorator{catalogSource, c.createPodAsUser}
482465

483466
image := c.Image
484467
if source.Spec.SourceType == "grpc" {
@@ -489,6 +472,11 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
489472
return
490473
}
491474

475+
podSecurityConfig, err := getNamespaceSecurityContextConfig(c.OpClient, catalogSource.GetNamespace())
476+
if err != nil {
477+
return false, err
478+
}
479+
492480
if source.Spec.SourceType == v1alpha1.SourceTypeConfigmap || source.Spec.SourceType == v1alpha1.SourceTypeInternal {
493481
// we use the live client here instead of a lister since our listers are scoped to objects with the olm.managed label,
494482
// and this configmap is a user-provided input to the catalog source and will not have that label
@@ -502,7 +490,7 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
502490
}
503491

504492
// recreate the pod if no existing pod is serving the latest image
505-
current, err := c.currentPodsWithCorrectResourceVersion(source, image)
493+
current, err := c.currentPodsWithCorrectResourceVersion(source, image, podSecurityConfig)
506494
if err != nil {
507495
return false, err
508496
}
@@ -518,7 +506,7 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
518506
if err != nil {
519507
return false, err
520508
}
521-
pods, err := c.currentPods(source, c.Image)
509+
pods, err := c.currentPods(source, c.Image, podSecurityConfig)
522510
if err != nil {
523511
return false, err
524512
}

0 commit comments

Comments
 (0)