Skip to content

(psa) allow legacy Catalogsources to run in non-restrcted namespaces #2845

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 2 commits into from
Sep 14, 2022
Merged

(psa) allow legacy Catalogsources to run in non-restrcted namespaces #2845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e69145
(chore) vendor in o-f/api v0.17.1
anik120 Sep 13, 2022
acaf375
(psa) allow legacy Catalogsources to run in non-restrcted namespaces
anik120 Sep 13, 2022
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
7 changes: 7 additions & 0 deletions deploy/chart/crds/0000_50_olm_00-catalogsources.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ spec:
priorityClassName:
description: If specified, indicates the pod's priority. If not specified, the pod priority will be default or zero if there is no default.
type: string
securityContextConfig:
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) controller''s `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. By default, SecurityContextConfig is set to `restricted`. If the value is unspecified, the default value of `restricted` is used. Specifying any other value will 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`. More information about PSA can be found here: https://kubernetes.io/docs/concepts/security/pod-security-admission/'''
type: string
default: restricted
enum:
- legacy
- restricted
tolerations:
description: Tolerations are the catalog source's pod's tolerations.
type: array
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ require (
github.com/onsi/gomega v1.18.1
github.com/openshift/api v0.0.0-20200331152225-585af27e34fd
github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0
github.com/operator-framework/api v0.16.0
github.com/operator-framework/api v0.17.1
github.com/operator-framework/operator-registry v1.17.5
github.com/otiai10/copy v1.2.0
github.com/pkg/errors v0.9.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,8 +1078,8 @@ github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0/go.mod h1:uUQ4
github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJc5AZX7/PBEpw=
github.com/operator-framework/api v0.7.1/go.mod h1:L7IvLd/ckxJEJg/t4oTTlnHKAJIP/p51AvEslW3wYdY=
github.com/operator-framework/api v0.16.0 h1:swUOhVv7QDszxBTwYM8QAtyeqI4EQHNVAiKMS+xjakY=
github.com/operator-framework/api v0.16.0/go.mod h1:kk8xJahHJR3bKqrA+A+1VIrhOTmyV76k+ARv+iV+u1Q=
github.com/operator-framework/api v0.17.1 h1:J/6+Xj4IEV8C7hcirqUFwOiZAU3PbnJhWvB0/bB51c4=
github.com/operator-framework/api v0.17.1/go.mod h1:kk8xJahHJR3bKqrA+A+1VIrhOTmyV76k+ARv+iV+u1Q=
github.com/operator-framework/operator-registry v1.17.5 h1:LR8m1rFz5Gcyje8WK6iYt+gIhtzqo52zMRALdmTYHT0=
github.com/operator-framework/operator-registry v1.17.5/go.mod h1:sRQIgDMZZdUcmHltzyCnM6RUoDF+WS8Arj1BQIARDS8=
github.com/otiai10/copy v1.2.0 h1:HvG945u96iNadPoG2/Ja2+AUJeW5YuFQMixq9yirC+k=
Expand Down
37 changes: 24 additions & 13 deletions pkg/controller/registry/reconciler/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,7 @@ func Pod(source *operatorsv1alpha1.CatalogSource, name string, image string, saN
},
},
SecurityContext: &corev1.SecurityContext{
ReadOnlyRootFilesystem: pointer.Bool(false),
AllowPrivilegeEscalation: pointer.Bool(false),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, because we need to read root file system, they away to grant permissions to that is setting true to AllowPrivilegeEscalation

And RunAsUser=0 is not required ( we need an user that have permissions to do what needs to be done as before, and if the bt setting true there the user will be part of the root group and have the same permissions.

Btw, RunAsUser=0 might not work on OCP (it will depends of the namespace and it probably will raise the error invalid value in the range)

c/c @perdasilva

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does "AllowPrivilegeEscalation" have to do w/ reading the root filesystem?

allow privilege escalation means you can run a setuid binary or otherwise allow a process to "become root", does the opm registry binary do so?

I would simply expect the index.db to be readable (by any uid), or in general we should be ensuring that the container is run w/ the appropriate uid and fsgroup to allow us to read (and write, if necessary) to the appropriate directories. None of that should require privilege escalation.

allowPrivilegeEscalation: Controls whether a process can gain more privileges than its parent process.

https://kubernetes.io/docs/tasks/configure-pod-container/security-context/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd mean we don't need to specify AllowPrivilegeEscalation at all then, since opm doesn't require to be root to read or write to the sqllite db, and also in 4.10 we didn't specify AllowPrivilegeEscalation either (and opm not requiring to be root hasn't changed since 4.10, and is unlikely to change either).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fyi was updating the tests to use image hosted in the quay.io/olmtest registry instead of my personal registry, so took that opportunity to remove AllowPrivilegeEscalation:true too. cc: @perdasilva now I'm happy with the PR going in :)

Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
ReadOnlyRootFilesystem: pointer.Bool(false),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to read the SQL Index DB which is the root file system so that needs to be true (ihmo)

@perdasilva

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

readOnlyRootFilesystem: Mounts the container's root filesystem as read-only.

this needs to be false if we need to write to it (which i think we do, because i think the legacy catalogs write to the / path?).

It does not mean "set this to true if you need to read from the filesystem", you set it to true when you explicitly want to ensure nothing writes to the root filesystem. Which is (aiui) not the case for legacy catalogs (we need to write to the root filesystem)

},
ImagePullPolicy: pullPolicy,
TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
Expand All @@ -195,19 +191,18 @@ func Pod(source *operatorsv1alpha1.CatalogSource, name string, image string, saN
NodeSelector: map[string]string{
"kubernetes.io/os": "linux",
},
SecurityContext: &corev1.PodSecurityContext{
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
},
ServiceAccountName: saName,
},
}

if runAsUser > 0 {
pod.Spec.SecurityContext.RunAsUser = &runAsUser
pod.Spec.SecurityContext.RunAsNonRoot = pointer.Bool(true)
if source.Spec.GrpcPodConfig != nil {
if source.Spec.GrpcPodConfig.SecurityContextConfig == operatorsv1alpha1.Restricted {
addSecurityContext(pod, runAsUser)
}
} else {
addSecurityContext(pod, runAsUser)
}

// Override scheduling options if specified
if source.Spec.GrpcPodConfig != nil {
grpcPodConfig := source.Spec.GrpcPodConfig
Expand Down Expand Up @@ -256,3 +251,19 @@ func hashPodSpec(spec corev1.PodSpec) string {
hashutil.DeepHashObject(hasher, &spec)
return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32()))
}

func addSecurityContext(pod *corev1.Pod, runAsUser int64) {
pod.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation = pointer.Bool(false)
pod.Spec.Containers[0].SecurityContext.Capabilities = &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
}
pod.Spec.SecurityContext = &corev1.PodSecurityContext{
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
}
if runAsUser > 0 {
pod.Spec.SecurityContext.RunAsUser = &runAsUser
pod.Spec.SecurityContext.RunAsNonRoot = pointer.Bool(true)
}
}
146 changes: 145 additions & 1 deletion test/e2e/catalog_e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
)

BeforeEach(func() {
namespaceName := genName("catsrc-e2e-")
// In OPC, PSA labels for any namespace created that is not prefixed with "openshift-" is overriden to enforce
// PSA restricted. This test namespace needs to prefixed with openshift- so that baseline/privileged enforcement
// for the PSA specific tests are not overridden,
// Change it only after https://github.com/operator-framework/operator-lifecycle-manager/issues/2859 is closed.
namespaceName := genName("openshift-catsrc-e2e-")
og := operatorsv1.OperatorGroup{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-operatorgroup", namespaceName),
Expand Down Expand Up @@ -1396,7 +1400,147 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
})
})
})
When("The namespace is labled as Pod Security Admission policy enforce:restricted", func() {
BeforeEach(func() {
var err error
testNS := &corev1.Namespace{}
Eventually(func() error {
testNS, err = c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), ns.GetName(), metav1.GetOptions{})
if err != nil {
return err
}
return nil
}).Should(BeNil())

testNS.ObjectMeta.Labels = map[string]string{
"pod-security.kubernetes.io/enforce": "restricted",
"pod-security.kubernetes.io/enforce-version": "latest",
}

Eventually(func() error {
_, err := c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
if err != nil {
return err
}
return nil
}).Should(BeNil())
})
When("A CatalogSource built with opm v1.21.0 (<v1.23.2)is created without spec.GrpcPodConfig.SecurityContextConfig set to legacy", func() {
var sourceName string
BeforeEach(func() {
sourceName = genName("catalog-")
source := &v1alpha1.CatalogSource{
TypeMeta: metav1.TypeMeta{
Kind: v1alpha1.CatalogSourceKind,
APIVersion: v1alpha1.CatalogSourceCRDAPIVersion,
},
ObjectMeta: metav1.ObjectMeta{
Name: sourceName,
Namespace: ns.GetName(),
Labels: map[string]string{"olm.catalogSource": sourceName},
},
Spec: v1alpha1.CatalogSourceSpec{
SourceType: v1alpha1.SourceTypeGrpc,
Image: "quay.io/olmtest/old-opm-catsrc:v1.21.0",
},
}

Eventually(func() error {
_, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.Background(), source, metav1.CreateOptions{})
return err
}).Should(Succeed())
})
It("The registry pod fails to become come up because of lack of permission", func() {
Eventually(func() (bool, error) {
podList, err := c.KubernetesInterface().CoreV1().Pods(ns.GetName()).List(context.TODO(), metav1.ListOptions{})
if err != nil {
return false, err
}
for _, pod := range podList.Items {
if pod.ObjectMeta.OwnerReferences != nil && pod.ObjectMeta.OwnerReferences[0].Name == sourceName {
if pod.Status.ContainerStatuses != nil && pod.Status.ContainerStatuses[0].State.Terminated != nil {
return true, nil
}
}
}
return false, nil
}).Should(BeTrue())
})
})
})
When("The namespace is labled as Pod Security Admission policy enforce:baseline", func() {
BeforeEach(func() {
var err error
testNS := &corev1.Namespace{}
Eventually(func() error {
testNS, err = c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), ns.GetName(), metav1.GetOptions{})
if err != nil {
return err
}
return nil
}).Should(BeNil())

testNS.ObjectMeta.Labels = map[string]string{
"pod-security.kubernetes.io/enforce": "baseline",
"pod-security.kubernetes.io/enforce-version": "latest",
}

Eventually(func() error {
_, err := c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
if err != nil {
return err
}
return nil
}).Should(BeNil())
})
When("A CatalogSource built with opm v1.21.0 (<v1.23.2)is created with spec.GrpcPodConfig.SecurityContextConfig set to legacy", func() {
var sourceName string
BeforeEach(func() {
sourceName = genName("catalog-")
source := &v1alpha1.CatalogSource{
TypeMeta: metav1.TypeMeta{
Kind: v1alpha1.CatalogSourceKind,
APIVersion: v1alpha1.CatalogSourceCRDAPIVersion,
},
ObjectMeta: metav1.ObjectMeta{
Name: sourceName,
Namespace: ns.GetName(),
Labels: map[string]string{"olm.catalogSource": sourceName},
},
Spec: v1alpha1.CatalogSourceSpec{
GrpcPodConfig: &v1alpha1.GrpcPodConfig{
SecurityContextConfig: operatorsv1alpha1.Legacy,
},
SourceType: v1alpha1.SourceTypeGrpc,
Image: "quay.io/olmtest/old-opm-catsrc:v1.21.0",
},
}

Eventually(func() error {
_, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.Background(), source, metav1.CreateOptions{})
return err
}).Should(Succeed())
})
It("The registry pod comes up successfully", func() {
Eventually(func() (bool, error) {
podList, err := c.KubernetesInterface().CoreV1().Pods(ns.GetName()).List(context.TODO(), metav1.ListOptions{})
if err != nil {
return false, err
}
for _, pod := range podList.Items {
if pod.ObjectMeta.OwnerReferences != nil && pod.ObjectMeta.OwnerReferences[0].Name == sourceName {
if pod.Status.ContainerStatuses != nil {
if *pod.Status.ContainerStatuses[0].Started == true {
return true, nil
}
}
}
}
return false, nil
}).Should(BeTrue())
})
})
})
})

func getOperatorDeployment(c operatorclient.ClientInterface, namespace string, operatorLabels labels.Set) (*appsv1.Deployment, error) {
Expand Down
7 changes: 7 additions & 0 deletions vendor/github.com/operator-framework/api/crds/operators.coreos.com_catalogsources.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/github.com/operator-framework/api/crds/zz_defs.go
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions vendor/github.com/operator-framework/api/pkg/manifests/bundleloader.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 22 additions & 1 deletion vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/catalogsource_types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ github.com/openshift/client-go/config/informers/externalversions/config
github.com/openshift/client-go/config/informers/externalversions/config/v1
github.com/openshift/client-go/config/informers/externalversions/internalinterfaces
github.com/openshift/client-go/config/listers/config/v1
# github.com/operator-framework/api v0.16.0
# github.com/operator-framework/api v0.17.1
## explicit; go 1.18
github.com/operator-framework/api/crds
github.com/operator-framework/api/pkg/constraints
Expand Down