Skip to content

OCPBUGS-29729: Updates default security context behavior for catalog source pods #3206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 2, 2024
Merged

OCPBUGS-29729: Updates default security context behavior for catalog source pods #3206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55720ca
Refactor security context configuration in pod reconciler
bentito Mar 4, 2024
36aea12
Add checking of the namespace PSA restrictions
bentito Apr 4, 2024
b1efd17
Fix linter issues
bentito Apr 5, 2024
27f9246
fixes
Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 160 additions & 5 deletions pkg/controller/operators/catalog/operator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import (
"testing/quick"
"time"

"k8s.io/utils/ptr"

controllerclient "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/controller-runtime/client"

"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -59,7 +63,6 @@ import (
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/solver"
"github.com/operator-framework/operator-lifecycle-manager/pkg/fakes"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/clientfake"
controllerclient "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/controller-runtime/client"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
Expand Down Expand Up @@ -838,6 +841,160 @@ func withStatus(catalogSource v1alpha1.CatalogSource, status v1alpha1.CatalogSou
return copy
}

func TestSyncCatalogSourcesSecurityPolicy(t *testing.T) {
assertLegacySecurityPolicy := func(t *testing.T, pod *corev1.Pod) {
require.Nil(t, pod.Spec.SecurityContext)
require.Equal(t, &corev1.SecurityContext{
ReadOnlyRootFilesystem: ptr.To(false),
}, pod.Spec.Containers[0].SecurityContext)
}

assertRestrictedPolicy := func(t *testing.T, pod *corev1.Pod) {
require.Equal(t, &corev1.PodSecurityContext{
SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault},
RunAsNonRoot: ptr.To(true),
RunAsUser: ptr.To(int64(1001)),
}, pod.Spec.SecurityContext)
require.Equal(t, &corev1.SecurityContext{
ReadOnlyRootFilesystem: ptr.To(false),
AllowPrivilegeEscalation: ptr.To(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
}, pod.Spec.Containers[0].SecurityContext)
}

clockFake := utilclocktesting.NewFakeClock(time.Date(2018, time.January, 26, 20, 40, 0, 0, time.UTC))
tests := []struct {
testName string
namespace *corev1.Namespace
catalogSource *v1alpha1.CatalogSource
check func(*testing.T, *corev1.Pod)
}{
{
testName: "UnlabeledNamespace/NoUserPreference/LegacySecurityPolicy",
namespace: &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "cool-namespace",
},
},
catalogSource: &v1alpha1.CatalogSource{
ObjectMeta: metav1.ObjectMeta{
Name: "cool-catalog",
Namespace: "cool-namespace",
UID: types.UID("catalog-uid"),
},
Spec: v1alpha1.CatalogSourceSpec{
Image: "catalog-image",
SourceType: v1alpha1.SourceTypeGrpc,
},
},
check: assertLegacySecurityPolicy,
}, {
testName: "UnlabeledNamespace/UserPreferenceForRestricted/RestrictedSecurityPolicy",
namespace: &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "cool-namespace",
},
},
catalogSource: &v1alpha1.CatalogSource{
ObjectMeta: metav1.ObjectMeta{
Name: "cool-catalog",
Namespace: "cool-namespace",
UID: types.UID("catalog-uid"),
},
Spec: v1alpha1.CatalogSourceSpec{
Image: "catalog-image",
SourceType: v1alpha1.SourceTypeGrpc,
GrpcPodConfig: &v1alpha1.GrpcPodConfig{
SecurityContextConfig: v1alpha1.Restricted,
},
},
},
check: assertRestrictedPolicy,
}, {
testName: "LabeledNamespace/NoUserPreference/RestrictedSecurityPolicy",
namespace: &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "cool-namespace",
Labels: map[string]string{
// restricted is the default psa policy
"pod-security.kubernetes.io/enforce": "restricted",
},
},
},
catalogSource: &v1alpha1.CatalogSource{
ObjectMeta: metav1.ObjectMeta{
Name: "cool-catalog",
Namespace: "cool-namespace",
UID: types.UID("catalog-uid"),
},
Spec: v1alpha1.CatalogSourceSpec{
Image: "catalog-image",
SourceType: v1alpha1.SourceTypeGrpc,
},
},
check: assertRestrictedPolicy,
}, {
testName: "LabeledNamespace/UserPreferenceForLegacy/LegacySecurityPolicy",
namespace: &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "cool-namespace",
},
},
catalogSource: &v1alpha1.CatalogSource{
ObjectMeta: metav1.ObjectMeta{
Name: "cool-catalog",
Namespace: "cool-namespace",
UID: types.UID("catalog-uid"),
},
Spec: v1alpha1.CatalogSourceSpec{
Image: "catalog-image",
SourceType: v1alpha1.SourceTypeGrpc,
GrpcPodConfig: &v1alpha1.GrpcPodConfig{
SecurityContextConfig: v1alpha1.Legacy,
},
},
},
check: assertLegacySecurityPolicy,
},
}
for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
// Create existing objects
clientObjs := []runtime.Object{tt.catalogSource}

// Create test operator
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

op, err := NewFakeOperator(
ctx,
tt.namespace.GetName(),
[]string{tt.namespace.GetName()},
withClock(clockFake),
withClientObjs(clientObjs...),
)
require.NoError(t, err)

// Because NewFakeOperator creates the namespace, we need to update the namespace to match the test case
// before running the sync function
_, err = op.opClient.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), tt.namespace, metav1.UpdateOptions{})
require.NoError(t, err)

// Run sync
err = op.syncCatalogSources(tt.catalogSource)
require.NoError(t, err)

pods, err := op.opClient.KubernetesInterface().CoreV1().Pods(tt.catalogSource.Namespace).List(context.TODO(), metav1.ListOptions{})
require.NoError(t, err)
require.Len(t, pods.Items, 1)

tt.check(t, &pods.Items[0])
})
}
}

func TestSyncCatalogSources(t *testing.T) {
clockFake := utilclocktesting.NewFakeClock(time.Date(2018, time.January, 26, 20, 40, 0, 0, time.UTC))
now := metav1.NewTime(clockFake.Now())
Expand Down Expand Up @@ -1165,7 +1322,7 @@ func TestSyncCatalogSources(t *testing.T) {
if tt.expectedStatus != nil {
if tt.expectedStatus.GRPCConnectionState != nil {
updated.Status.GRPCConnectionState.LastConnectTime = now
// Ignore LastObservedState difference if an expected LastObservedState is no provided
// Ignore LastObservedState difference if an expected LastObservedState is not provided
if tt.expectedStatus.GRPCConnectionState.LastObservedState == "" {
updated.Status.GRPCConnectionState.LastObservedState = ""
}
Expand Down Expand Up @@ -2012,7 +2169,6 @@ func NewFakeOperator(ctx context.Context, namespace string, namespaces []string,
return nil, err
}
applier := controllerclient.NewFakeApplier(s, "testowner")

op.reconciler = reconciler.NewRegistryReconcilerFactory(lister, op.opClient, "test:pod", op.now, applier, 1001, "", "")
}

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

return op, nil
}

func installPlan(name, namespace string, phase v1alpha1.InstallPlanPhase, names ...string) *v1alpha1.InstallPlan {
return &v1alpha1.InstallPlan{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -2175,7 +2330,7 @@ func pod(t *testing.T, s v1alpha1.CatalogSource) *corev1.Pod {
Name: s.GetName(),
},
}
pod, err := reconciler.Pod(&s, "registry-server", "central-opm", "central-util", s.Spec.Image, serviceAccount, s.GetLabels(), s.GetAnnotations(), 5, 10, 1001)
pod, err := reconciler.Pod(&s, "registry-server", "central-opm", "central-util", s.Spec.Image, serviceAccount, s.GetLabels(), s.GetAnnotations(), 5, 10, 1001, v1alpha1.Legacy)
if err != nil {
t.Fatal(err)
}
Expand Down
38 changes: 24 additions & 14 deletions pkg/controller/registry/reconciler/configmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ func (s *configMapCatalogSourceDecorator) Service() (*corev1.Service, error) {
return svc, nil
}

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

func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceDecorator, image string) ([]*corev1.Pod, error) {
protoPod, err := source.Pod(image)
func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceDecorator, image string, defaultPodSecurityConfig v1alpha1.SecurityConfig) ([]*corev1.Pod, error) {
protoPod, err := source.Pod(image, defaultPodSecurityConfig)
if err != nil {
return nil, err
}
Expand All @@ -255,8 +255,8 @@ func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceD
return pods, nil
}

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

defaultPodSecurityConfig, err := getDefaultPodContextConfig(c.OpClient, catalogSource.GetNamespace())
if err != nil {
return err
}

if source.Spec.SourceType == v1alpha1.SourceTypeConfigmap || source.Spec.SourceType == v1alpha1.SourceTypeInternal {
// fetch configmap first, exit early if we can't find it
// we use the live client here instead of a lister since our listers are scoped to objects with the olm.managed label,
Expand All @@ -311,7 +316,7 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
}

// recreate the pod if no existing pod is serving the latest image
current, err := c.currentPodsWithCorrectResourceVersion(source, image)
current, err := c.currentPodsWithCorrectResourceVersion(source, image, defaultPodSecurityConfig)
if err != nil {
return err
}
Expand All @@ -330,11 +335,11 @@ func (c *ConfigMapRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry,
if err := c.ensureRoleBinding(source, overwrite); err != nil {
return errors.Wrapf(err, "error ensuring rolebinding: %s", source.RoleBinding().GetName())
}
pod, err := source.Pod(image)
pod, err := source.Pod(image, defaultPodSecurityConfig)
if err != nil {
return err
}
if err := c.ensurePod(source, overwritePod); err != nil {
if err := c.ensurePod(source, defaultPodSecurityConfig, overwritePod); err != nil {
return errors.Wrapf(err, "error ensuring pod: %s", pod.GetName())
}
service, err := source.Service()
Expand Down Expand Up @@ -400,12 +405,12 @@ func (c *ConfigMapRegistryReconciler) ensureRoleBinding(source configMapCatalogS
return err
}

func (c *ConfigMapRegistryReconciler) ensurePod(source configMapCatalogSourceDecorator, overwrite bool) error {
pod, err := source.Pod(c.Image)
func (c *ConfigMapRegistryReconciler) ensurePod(source configMapCatalogSourceDecorator, defaultPodSecurityConfig v1alpha1.SecurityConfig, overwrite bool) error {
pod, err := source.Pod(c.Image, defaultPodSecurityConfig)
if err != nil {
return err
}
currentPods, err := c.currentPods(source, c.Image)
currentPods, err := c.currentPods(source, c.Image, defaultPodSecurityConfig)
if err != nil {
return err
}
Expand Down Expand Up @@ -460,6 +465,11 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
return
}

defaultPodSecurityConfig, err := getDefaultPodContextConfig(c.OpClient, catalogSource.GetNamespace())
if err != nil {
return false, err
}

if source.Spec.SourceType == v1alpha1.SourceTypeConfigmap || source.Spec.SourceType == v1alpha1.SourceTypeInternal {
// we use the live client here instead of a lister since our listers are scoped to objects with the olm.managed label,
// and this configmap is a user-provided input to the catalog source and will not have that label
Expand All @@ -473,7 +483,7 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
}

// recreate the pod if no existing pod is serving the latest image
current, err := c.currentPodsWithCorrectResourceVersion(source, image)
current, err := c.currentPodsWithCorrectResourceVersion(source, image, defaultPodSecurityConfig)
if err != nil {
return false, err
}
Expand All @@ -489,7 +499,7 @@ func (c *ConfigMapRegistryReconciler) CheckRegistryServer(logger *logrus.Entry,
if err != nil {
return false, err
}
pods, err := c.currentPods(source, c.Image)
pods, err := c.currentPods(source, c.Image, defaultPodSecurityConfig)
if err != nil {
return false, err
}
Expand Down
Loading