Skip to content

Commit

Permalink
Use random UID and GID when running on OpenShift
Browse files Browse the repository at this point in the history
When running on OpenShift, allow OpenShift to assign a random UID and
GID for the Gatekeeper containers. When it's not OpenShift, fallback to
running as a non-privileged user and group.

Additionally, for backwards compatibility with OpenShift 4.10, seccomp
profile is left unset. See the following for this recommendation:
https://connect.redhat.com/en/blog/important-openshift-changes-pod-security-standards

Signed-off-by: mprahl <[email protected]>
  • Loading branch information
mprahl committed Sep 19, 2023
1 parent 89a85bf commit 0585710
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -493,14 +493,6 @@ spec:
- patch
- update
- watch
- apiGroups:
- security.openshift.io
resourceNames:
- anyuid
resources:
- securitycontextconstraints
verbs:
- use
serviceAccountName: gatekeeper-operator-controller-manager
strategy: deployment
installModes:
Expand Down
21 changes: 0 additions & 21 deletions config/gatekeeper/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,9 @@ resources:
- v1_secret_gatekeeper-webhook-server-cert.yaml
- v1_serviceaccount_gatekeeper-admin.yaml
- v1_service_gatekeeper-webhook-service.yaml
# Add annotations for backwards compatibility
# Add OpenShift specific RBAC
# Remove --disable-cert-rotation
# Set a CPU limit
patches:
- path: patches/apps_v1_deployment_gatekeeper-audit.yaml
- path: patches/apps_v1_deployment_gatekeeper-controller-manager.yaml
- patch: |-
- op: add
path: /rules/-
value:
apiGroups:
- security.openshift.io
resourceNames:
- anyuid
resources:
- securitycontextconstraints
verbs:
- use
target:
group: rbac.authorization.k8s.io
kind: Role
name: gatekeeper-manager-role
version: v1
- patch: |-
- op: remove
path: /spec/template/spec/containers/0/args/5
Expand Down
10 changes: 0 additions & 10 deletions config/gatekeeper/patches/apps_v1_deployment_gatekeeper-audit.yaml

This file was deleted.

This file was deleted.

8 changes: 0 additions & 8 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,3 @@ rules:
- patch
- update
- watch
- apiGroups:
- security.openshift.io
resourceNames:
- anyuid
resources:
- securitycontextconstraints
verbs:
- use
41 changes: 40 additions & 1 deletion controllers/gatekeeper_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ const (
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,namespace="system",resources=roles;rolebindings,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=apps,namespace="system",resources=deployments,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=policy,namespace="system",resources=poddisruptionbudgets,verbs=create;delete;update;use
// +kubebuilder:rbac:groups=security.openshift.io,namespace="system",resources=securitycontextconstraints,resourceNames=anyuid,verbs=use

// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
Expand Down Expand Up @@ -499,6 +498,10 @@ func crOverrides(gatekeeper *operatorv1alpha1.Gatekeeper, asset string, obj *uns
if err := removeAnnotations(obj); err != nil {
return err
}

if err := openShiftDeploymentOverrides(obj); err != nil {
return err
}
}
// webhook overrides
case WebhookFile:
Expand All @@ -512,6 +515,10 @@ func crOverrides(gatekeeper *operatorv1alpha1.Gatekeeper, asset string, obj *uns
if err := removeAnnotations(obj); err != nil {
return err
}

if err := openShiftDeploymentOverrides(obj); err != nil {
return err
}
}
// ValidatingWebhookConfiguration overrides
case ValidatingWebhookConfiguration:
Expand Down Expand Up @@ -742,6 +749,38 @@ func removeAnnotations(obj *unstructured.Unstructured) error {
return nil
}

// openShiftDeploymentOverrides will remove runAsUser, runAsGroup, and seccompProfile on every container in the
// Deployment manifest. The seccompProfile is removed for backwards compatibility with OpenShift <= v4.10. Setting
// seccompProfile=runtime/default in such versions explicitly disqualified the workload from the restricted SCC.
// In OpenShift v4.11+, any workload running in a namespace prefixed with "openshift-*" must use the "restricted"
// profile unless there is a ClusterServiceVersion present, which is not the case for the Gatekeeper operand namespace.
func openShiftDeploymentOverrides(obj *unstructured.Unstructured) error {
containers, _, err := unstructured.NestedSlice(obj.Object, "spec", "template", "spec", "containers")
if err != nil {
return errors.Wrapf(err, "Failed to parse the deployment's containers")
}

for i := range containers {
container, ok := containers[i].(map[string]interface{})
if !ok {
continue
}

unstructured.RemoveNestedField(container, "securityContext", "runAsUser")
unstructured.RemoveNestedField(container, "securityContext", "runAsGroup")
unstructured.RemoveNestedField(container, "securityContext", "seccompProfile")

containers[i] = container
}

err = unstructured.SetNestedField(obj.Object, containers, "spec", "template", "spec", "containers")
if err != nil {
return errors.Wrapf(err, "Failed to set the OpenShift overrides")
}

return nil
}

func setLogLevel(obj *unstructured.Unstructured, logLevel *operatorv1alpha1.LogLevelMode) error {
if logLevel != nil {
return setContainerArg(obj, managerContainer, LogLevelArg, string(*logLevel), false)
Expand Down
62 changes: 61 additions & 1 deletion controllers/gatekeeper_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,64 @@ func assertAffinity(g *WithT, expected *corev1.Affinity, current interface{}) {
g.Expect(util.ToMap(expected)).To(BeEquivalentTo(util.ToMap(current)))
}

func TestOpenShiftOverrides(t *testing.T) {
g := NewWithT(t)
gatekeeper := &operatorv1alpha1.Gatekeeper{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
}

auditObj, err := util.GetManifestObject(AuditFile)
g.Expect(err).ToNot(HaveOccurred())

webhookObj, err := util.GetManifestObject(WebhookFile)
g.Expect(err).ToNot(HaveOccurred())

// Test that no OpenShift overrides take place when it's not OpenShift
err = crOverrides(gatekeeper, AuditFile, auditObj, namespace, false, false)
g.Expect(err).ToNot(HaveOccurred())
assertOverrides(g, auditObj, true)

err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, false, false)
g.Expect(err).ToNot(HaveOccurred())
assertOverrides(g, webhookObj, true)

// Test that OpenShift overrides take place
err = crOverrides(gatekeeper, AuditFile, auditObj, namespace, true, false)
g.Expect(err).ToNot(HaveOccurred())
assertOverrides(g, auditObj, false)

err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, true, false)
g.Expect(err).ToNot(HaveOccurred())
assertOverrides(g, webhookObj, false)
}

func assertOverrides(g *WithT, current *unstructured.Unstructured, isSet bool) {
containers, _, err := unstructured.NestedSlice(current.Object, "spec", "template", "spec", "containers")
g.ExpectWithOffset(1, err).ToNot(HaveOccurred())
g.ExpectWithOffset(1, containers).ToNot(BeEmpty())

for i := range containers {
container, ok := containers[i].(map[string]interface{})
if !ok {
continue
}

_, runAsUserFound, err := unstructured.NestedInt64(container, "securityContext", "runAsUser")
g.ExpectWithOffset(1, err).ToNot(HaveOccurred())
g.ExpectWithOffset(1, runAsUserFound).To(Equal(isSet))

_, runAsGroupFound, err := unstructured.NestedInt64(container, "securityContext", "runAsGroup")
g.ExpectWithOffset(1, err).ToNot(HaveOccurred())
g.ExpectWithOffset(1, runAsGroupFound).To(Equal(isSet))

_, seccompProfileFound, err := unstructured.NestedMap(container, "securityContext", "seccompProfile")
g.ExpectWithOffset(1, err).ToNot(HaveOccurred())
g.ExpectWithOffset(1, seccompProfileFound).To(Equal(isSet))
}
}

func TestNodeSelector(t *testing.T) {
g := NewWithT(t)
nodeSelector := map[string]string{
Expand Down Expand Up @@ -395,10 +453,12 @@ func assertPodAnnotations(g *WithT, obj *unstructured.Unstructured, expected map
g.Expect(obj).NotTo(BeNil())
current, found, err := unstructured.NestedStringMap(obj.Object, "spec", "template", "metadata", "annotations")
g.Expect(err).ToNot(HaveOccurred())
g.Expect(found).To(BeTrue())

if expected == nil {
g.Expect(found).To(BeFalse())
g.Expect(test.DefaultDeployment.PodAnnotations).To(BeEquivalentTo(current))
} else {
g.Expect(found).To(BeTrue())
g.Expect(expected).To(BeEquivalentTo(current))
}
}
Expand Down
12 changes: 0 additions & 12 deletions pkg/bindata/bindata.go

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

3 changes: 0 additions & 3 deletions test/e2e/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ var DefaultDeployment = defaultConfig{
NodeSelector: map[string]string{
"kubernetes.io/os": "linux",
},
PodAnnotations: map[string]string{
"container.seccomp.security.alpha.kubernetes.io/manager": "runtime/default",
},
Resources: &corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1000m"),
Expand Down

0 comments on commit 0585710

Please sign in to comment.