Skip to content

Commit 66856c2

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

File tree

7 files changed

+433
-286
lines changed

7 files changed

+433
-286
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: 32 additions & 51 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,8 @@ 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
117-
}
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
129-
}
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)
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)
139114
if err != nil {
140115
return nil, err
141116
}
@@ -208,12 +183,11 @@ func (s *configMapCatalogSourceDecorator) RoleBinding() *rbacv1.RoleBinding {
208183
}
209184

210185
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
186+
now nowFunc
187+
Lister operatorlister.OperatorLister
188+
OpClient operatorclient.ClientInterface
189+
Image string
190+
createPodAsUser int64
217191
}
218192

219193
var _ RegistryEnsurer = &ConfigMapRegistryReconciler{}
@@ -264,8 +238,8 @@ func (c *ConfigMapRegistryReconciler) currentRoleBinding(source configMapCatalog
264238
return roleBinding
265239
}
266240

267-
func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceDecorator, image string) ([]*corev1.Pod, error) {
268-
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)
269243
if err != nil {
270244
return nil, err
271245
}
@@ -281,8 +255,8 @@ func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceD
281255
return pods, nil
282256
}
283257

284-
func (c *ConfigMapRegistryReconciler) currentPodsWithCorrectResourceVersion(source configMapCatalogSourceDecorator, image string) ([]*corev1.Pod, error) {
285-
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)
286260
if err != nil {
287261
return nil, err
288262
}
@@ -300,10 +274,7 @@ func (c *ConfigMapRegistryReconciler) currentPodsWithCorrectResourceVersion(sour
300274

301275
// EnsureRegistryServer ensures that all components of registry server are up to date.
302276
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}
277+
source := configMapCatalogSourceDecorator{catalogSource, c.createPodAsUser}
307278

308279
image := c.Image
309280
if source.Spec.SourceType == "grpc" {
@@ -317,6 +288,11 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
317288
overwrite := source.Status.RegistryServiceStatus == nil
318289
overwritePod := overwrite
319290

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

342318
// recreate the pod if no existing pod is serving the latest image
343-
current, err := c.currentPodsWithCorrectResourceVersion(source, image)
319+
current, err := c.currentPodsWithCorrectResourceVersion(source, image, defaultPodSecurityConfig)
344320
if err != nil {
345321
return err
346322
}
@@ -359,11 +335,11 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
359335
if err := c.ensureRoleBinding(source, overwrite); err != nil {
360336
return errors.Wrapf(err, "error ensuring rolebinding: %s", source.RoleBinding().GetName())
361337
}
362-
pod, err := source.Pod(image)
338+
pod, err := source.Pod(image, defaultPodSecurityConfig)
363339
if err != nil {
364340
return err
365341
}
366-
if err := c.ensurePod(source, overwritePod); err != nil {
342+
if err := c.ensurePod(source, defaultPodSecurityConfig, overwritePod); err != nil {
367343
return errors.Wrapf(err, "error ensuring pod: %s", pod.GetName())
368344
}
369345
service, err := source.Service()
@@ -429,12 +405,12 @@ func (c *ConfigMapRegistryReconciler) ensureRoleBinding(source configMapCatalogS
429405
return err
430406
}
431407

432-
func (c *ConfigMapRegistryReconciler) ensurePod(source configMapCatalogSourceDecorator, overwrite bool) error {
433-
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)
434410
if err != nil {
435411
return err
436412
}
437-
currentPods, err := c.currentPods(source, c.Image)
413+
currentPods, err := c.currentPods(source, c.Image, defaultPodSecurityConfig)
438414
if err != nil {
439415
return err
440416
}
@@ -478,7 +454,7 @@ func (c *ConfigMapRegistryReconciler) ensureService(source configMapCatalogSourc
478454

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

483459
image := c.Image
484460
if source.Spec.SourceType == "grpc" {
@@ -489,6 +465,11 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
489465
return
490466
}
491467

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

504485
// recreate the pod if no existing pod is serving the latest image
505-
current, err := c.currentPodsWithCorrectResourceVersion(source, image)
486+
current, err := c.currentPodsWithCorrectResourceVersion(source, image, defaultPodSecurityConfig)
506487
if err != nil {
507488
return false, err
508489
}
@@ -518,7 +499,7 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
518499
if err != nil {
519500
return false, err
520501
}
521-
pods, err := c.currentPods(source, c.Image)
502+
pods, err := c.currentPods(source, c.Image, defaultPodSecurityConfig)
522503
if err != nil {
523504
return false, err
524505
}

0 commit comments

Comments
 (0)