Skip to content

Commit abccea9

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #788 from perdasilva/perdasilva/34979
[release-4.16] OCPBUGS-34979: Updates default security context behavior for catalog source pods
2 parents 555e529 + 096fed5 commit abccea9

File tree

18 files changed

+533
-217
lines changed

18 files changed

+533
-217
lines changed

‎manifests/0000_50_olm_00-catalogsources.crd.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -613,9 +613,8 @@ spec:
613613
description: If specified, indicates the pod's priority. If not specified, the pod priority will be default or zero if there is no default.
614614
type: string
615615
securityContextConfig:
616-
description: "SecurityContextConfig can be one of `legacy` or `restricted`. The CatalogSource's pod is either injected with the right pod.spec.securityContext and pod.spec.container[*].securityContext values to allow the pod to run in Pod Security Admission (PSA) `restricted` mode, or doesn't set these values at all, in which case the pod can only be run in PSA `baseline` or `privileged` namespaces. Currently if the SecurityContextConfig is unspecified, the default value of `legacy` is used. Specifying a value other than `legacy` or `restricted` result in a validation error. When using older catalog images, which could not be run in `restricted` mode, the SecurityContextConfig should be set to `legacy`. \n In a future version will the default will be set to `restricted`, catalog maintainers should rebuild their catalogs with a version of opm that supports running catalogSource pods in `restricted` mode to prepare for these changes. \n More information about PSA can be found here: https://kubernetes.io/docs/concepts/security/pod-security-admission/'"
616+
description: "SecurityContextConfig can be one of `legacy` or `restricted`. The CatalogSource's pod is either injected with the right pod.spec.securityContext and pod.spec.container[*].securityContext values to allow the pod to run in Pod Security Admission (PSA) `restricted` mode, or doesn't set these values at all, in which case the pod can only be run in PSA `baseline` or `privileged` namespaces. If the SecurityContextConfig is unspecified, the mode will be determined by the namespace's PSA configuration. If the namespace is enforcing `restricted` mode, then the pod will be configured as if `restricted` was specified. Otherwise, it will be configured as if `legacy` was specified. Specifying a value other than `legacy` or `restricted` result in a validation error. When using older catalog images, which can not run in `restricted` mode, the SecurityContextConfig should be set to `legacy`. \n More information about PSA can be found here: https://kubernetes.io/docs/concepts/security/pod-security-admission/'"
617617
type: string
618-
default: legacy
619618
enum:
620619
- legacy
621620
- restricted

‎microshift-manifests/0000_50_olm_00-catalogsources.crd.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -613,9 +613,8 @@ spec:
613613
description: If specified, indicates the pod's priority. If not specified, the pod priority will be default or zero if there is no default.
614614
type: string
615615
securityContextConfig:
616-
description: "SecurityContextConfig can be one of `legacy` or `restricted`. The CatalogSource's pod is either injected with the right pod.spec.securityContext and pod.spec.container[*].securityContext values to allow the pod to run in Pod Security Admission (PSA) `restricted` mode, or doesn't set these values at all, in which case the pod can only be run in PSA `baseline` or `privileged` namespaces. Currently if the SecurityContextConfig is unspecified, the default value of `legacy` is used. Specifying a value other than `legacy` or `restricted` result in a validation error. When using older catalog images, which could not be run in `restricted` mode, the SecurityContextConfig should be set to `legacy`. \n In a future version will the default will be set to `restricted`, catalog maintainers should rebuild their catalogs with a version of opm that supports running catalogSource pods in `restricted` mode to prepare for these changes. \n More information about PSA can be found here: https://kubernetes.io/docs/concepts/security/pod-security-admission/'"
616+
description: "SecurityContextConfig can be one of `legacy` or `restricted`. The CatalogSource's pod is either injected with the right pod.spec.securityContext and pod.spec.container[*].securityContext values to allow the pod to run in Pod Security Admission (PSA) `restricted` mode, or doesn't set these values at all, in which case the pod can only be run in PSA `baseline` or `privileged` namespaces. If the SecurityContextConfig is unspecified, the mode will be determined by the namespace's PSA configuration. If the namespace is enforcing `restricted` mode, then the pod will be configured as if `restricted` was specified. Otherwise, it will be configured as if `legacy` was specified. Specifying a value other than `legacy` or `restricted` result in a validation error. When using older catalog images, which can not run in `restricted` mode, the SecurityContextConfig should be set to `legacy`. \n More information about PSA can be found here: https://kubernetes.io/docs/concepts/security/pod-security-admission/'"
617617
type: string
618-
default: legacy
619618
enum:
620619
- legacy
621620
- restricted

‎staging/api/crds/operators.coreos.com_catalogsources.yaml

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -989,19 +989,15 @@ spec:
989989
SecurityContextConfig can be one of `legacy` or `restricted`. The CatalogSource's pod is either injected with the
990990
right pod.spec.securityContext and pod.spec.container[*].securityContext values to allow the pod to run in Pod
991991
Security Admission (PSA) `restricted` mode, or doesn't set these values at all, in which case the pod can only be
992-
run in PSA `baseline` or `privileged` namespaces. Currently if the SecurityContextConfig is unspecified, the default
993-
value of `legacy` is used. Specifying a value other than `legacy` or `restricted` result in a validation error.
994-
When using older catalog images, which could not be run in `restricted` mode, the SecurityContextConfig should be
995-
set to `legacy`.
996-
997-
998-
In a future version will the default will be set to `restricted`, catalog maintainers should rebuild their catalogs
999-
with a version of opm that supports running catalogSource pods in `restricted` mode to prepare for these changes.
992+
run in PSA `baseline` or `privileged` namespaces. If the SecurityContextConfig is unspecified, the mode will be
993+
determined by the namespace's PSA configuration. If the namespace is enforcing `restricted` mode, then the pod
994+
will be configured as if `restricted` was specified. Otherwise, it will be configured as if `legacy` was
995+
specified. Specifying a value other than `legacy` or `restricted` result in a validation error. When using older
996+
catalog images, which can not run in `restricted` mode, the SecurityContextConfig should be set to `legacy`.
1000997
1001998
1002999
More information about PSA can be found here: https://kubernetes.io/docs/concepts/security/pod-security-admission/'
10031000
type: string
1004-
default: legacy
10051001
enum:
10061002
- legacy
10071003
- restricted

‎staging/api/crds/zz_defs.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎staging/api/pkg/operators/v1alpha1/catalogsource_types.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,15 @@ type GrpcPodConfig struct {
133133
// SecurityContextConfig can be one of `legacy` or `restricted`. The CatalogSource's pod is either injected with the
134134
// right pod.spec.securityContext and pod.spec.container[*].securityContext values to allow the pod to run in Pod
135135
// Security Admission (PSA) `restricted` mode, or doesn't set these values at all, in which case the pod can only be
136-
// run in PSA `baseline` or `privileged` namespaces. Currently if the SecurityContextConfig is unspecified, the default
137-
// value of `legacy` is used. Specifying a value other than `legacy` or `restricted` result in a validation error.
138-
// When using older catalog images, which could not be run in `restricted` mode, the SecurityContextConfig should be
139-
// set to `legacy`.
140-
//
141-
// In a future version will the default will be set to `restricted`, catalog maintainers should rebuild their catalogs
142-
// with a version of opm that supports running catalogSource pods in `restricted` mode to prepare for these changes.
136+
// run in PSA `baseline` or `privileged` namespaces. If the SecurityContextConfig is unspecified, the mode will be
137+
// determined by the namespace's PSA configuration. If the namespace is enforcing `restricted` mode, then the pod
138+
// will be configured as if `restricted` was specified. Otherwise, it will be configured as if `legacy` was
139+
// specified. Specifying a value other than `legacy` or `restricted` result in a validation error. When using older
140+
// catalog images, which can not run in `restricted` mode, the SecurityContextConfig should be set to `legacy`.
143141
//
144142
// More information about PSA can be found here: https://kubernetes.io/docs/concepts/security/pod-security-admission/'
145143
// +optional
146144
// +kubebuilder:validation:Enum=legacy;restricted
147-
// +kubebuilder:default:=legacy
148145
SecurityContextConfig SecurityConfig `json:"securityContextConfig,omitempty"`
149146

150147
// MemoryTarget configures the $GOMEMLIMIT value for the gRPC catalog Pod. This is a soft memory limit for the server,

‎staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go

Lines changed: 160 additions & 5 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"
@@ -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"
@@ -844,6 +847,160 @@ func withStatus(catalogSource v1alpha1.CatalogSource, status v1alpha1.CatalogSou
844847
return copy
845848
}
846849

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

@@ -2034,7 +2190,6 @@ func NewFakeOperator(ctx context.Context, namespace string, namespaces []string,
20342190

20352191
return op, nil
20362192
}
2037-
20382193
func installPlan(name, namespace string, phase v1alpha1.InstallPlanPhase, names ...string) *v1alpha1.InstallPlan {
20392194
return &v1alpha1.InstallPlan{
20402195
ObjectMeta: metav1.ObjectMeta{
@@ -2181,7 +2336,7 @@ func pod(t *testing.T, s v1alpha1.CatalogSource) *corev1.Pod {
21812336
Name: s.GetName(),
21822337
},
21832338
}
2184-
pod, err := reconciler.Pod(&s, "registry-server", "central-opm", "central-util", s.Spec.Image, serviceAccount, s.GetLabels(), s.GetAnnotations(), 5, 10, 1001)
2339+
pod, err := reconciler.Pod(&s, "registry-server", "central-opm", "central-util", s.Spec.Image, serviceAccount, s.GetLabels(), s.GetAnnotations(), 5, 10, 1001, v1alpha1.Legacy)
21852340
if err != nil {
21862341
t.Fatal(err)
21872342
}

0 commit comments

Comments
 (0)