Skip to content

Commit

Permalink
NETOBSERV-307: run with capabilities instead of privileged container (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Mario Macias authored May 10, 2022
1 parent 0ac20b2 commit d0a45a1
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 24 deletions.
6 changes: 6 additions & 0 deletions api/v1alpha1/flowcollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ type FlowCollectorEBPF struct {
// in edge debug/support scenarios.
//+optional
Env map[string]string `json:"env,omitempty"`

// Privileged mode for the eBPF Agent container. If false, the operator will add the following
// capabilities to the container, to enable its correct operation:
// BPF, PERFMON, NET_ADMIN, SYS_RESOURCE.
// +optional
Privileged bool `json:"privileged,omitempty"`
}

// FlowCollectorFLP defines the desired flowlogs-pipeline state of FlowCollector
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/flows.netobserv.io_flowcollectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,12 @@ spec:
- fatal
- panic
type: string
privileged:
description: 'Privileged mode for the eBPF Agent container. If
false, the operator will add the following capabilities to the
container, to enable its correct operation: BPF, PERFMON, NET_ADMIN,
SYS_RESOURCE.'
type: boolean
resources:
description: 'Compute Resources required by this container. Cannot
be updated. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
Expand Down
1 change: 1 addition & 0 deletions config/samples/flows_v1alpha1_flowcollector.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ spec:
interfaces: []
excludeInterfaces: ["lo"]
logLevel: info
privileged: false
flowlogsPipeline:
kind: DaemonSet
# kind: Deployment
Expand Down
1 change: 1 addition & 0 deletions config/samples/flows_v1alpha1_flowcollector_versioned.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ spec:
interfaces: []
excludeInterfaces: ["lo"]
logLevel: info
privileged: false
flowlogsPipeline:
kind: DaemonSet
# kind: Deployment
Expand Down
24 changes: 17 additions & 7 deletions controllers/ebpf/agent_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (c *AgentController) Reconcile(
}
}

if err := c.permissions.Reconcile(ctx); err != nil {
if err := c.permissions.Reconcile(ctx, &target.Spec.EBPF); err != nil {
return fmt.Errorf("reconciling permissions: %w", err)
}
desired := c.desired(target)
Expand Down Expand Up @@ -157,12 +157,8 @@ func (c *AgentController) desired(coll *flowsv1alpha1.FlowCollector) *v1.DaemonS
Image: coll.Spec.EBPF.Image,
ImagePullPolicy: corev1.PullPolicy(coll.Spec.EBPF.ImagePullPolicy),
Resources: coll.Spec.EBPF.Resources,
// TODO: other parameters when NETOBSERV-201 is implemented
SecurityContext: &corev1.SecurityContext{
Privileged: pointer.Bool(true),
RunAsUser: pointer.Int64(0),
},
Env: c.envConfig(coll),
SecurityContext: c.securityContext(coll),
Env: c.envConfig(coll),
}},
},
},
Expand Down Expand Up @@ -250,3 +246,17 @@ func (c *AgentController) requiredAction(current, desired *v1.DaemonSet) reconci
}
return actionUpdate
}

func (c *AgentController) securityContext(coll *flowsv1alpha1.FlowCollector) *corev1.SecurityContext {
sc := corev1.SecurityContext{
RunAsUser: pointer.Int64(0),
}

if coll.Spec.EBPF.Privileged {
sc.Privileged = &coll.Spec.EBPF.Privileged
} else {
sc.Capabilities = &corev1.Capabilities{Add: permissions.AllowedCapabilities}
}

return &sc
}
34 changes: 20 additions & 14 deletions controllers/ebpf/internal/permissions/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,23 @@ package permissions
import (
"context"
"fmt"
"reflect"

"github.com/netobserv/network-observability-operator/api/v1alpha1"
"github.com/netobserv/network-observability-operator/controllers/constants"
"github.com/netobserv/network-observability-operator/controllers/reconcilers"
"github.com/netobserv/network-observability-operator/pkg/discover"
"github.com/netobserv/network-observability-operator/pkg/helper"
osv1 "github.com/openshift/api/security/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
)

var AllowedCapabilities = []v1.Capability{"BPF", "PERFMON", "NET_ADMIN", "SYS_RESOURCE"}

// Reconciler reconciles the different resources to enable the privileged operation of the
// Netobserv Agent:
// - Create the privileged namespace with Pod Permissions annotations (for Vanilla K8s)
Expand All @@ -40,7 +43,7 @@ func NewReconciler(
}
}

func (c *Reconciler) Reconcile(ctx context.Context) error {
func (c *Reconciler) Reconcile(ctx context.Context, desired *v1alpha1.FlowCollectorEBPF) error {
log.IntoContext(ctx, log.FromContext(ctx).WithName("permissions"))

if err := c.reconcileNamespace(ctx); err != nil {
Expand All @@ -49,7 +52,7 @@ func (c *Reconciler) Reconcile(ctx context.Context) error {
if err := c.reconcileServiceAccount(ctx); err != nil {
return fmt.Errorf("reconciling service account: %w", err)
}
if err := c.reconcileVendorPermissions(ctx); err != nil {
if err := c.reconcileVendorPermissions(ctx, desired); err != nil {
return fmt.Errorf("reconciling vendor permissions: %w", err)
}
return nil
Expand Down Expand Up @@ -114,23 +117,25 @@ func (c *Reconciler) reconcileServiceAccount(ctx context.Context) error {
return nil
}

func (c *Reconciler) reconcileVendorPermissions(ctx context.Context) error {
func (c *Reconciler) reconcileVendorPermissions(
ctx context.Context, desired *v1alpha1.FlowCollectorEBPF,
) error {
if c.vendor.Vendor(ctx) == discover.VendorOpenShift {
return c.reconcileOpenshiftPermissions(ctx)
return c.reconcileOpenshiftPermissions(ctx, desired)
}
return nil
}

func (c *Reconciler) reconcileOpenshiftPermissions(ctx context.Context) error {
func (c *Reconciler) reconcileOpenshiftPermissions(
ctx context.Context, desired *v1alpha1.FlowCollectorEBPF,
) error {
rlog := log.FromContext(ctx,
"securityContextConstraints", constants.EBPFSecurityContext)
scc := &osv1.SecurityContextConstraints{
ObjectMeta: metav1.ObjectMeta{
Name: constants.EBPFSecurityContext,
},
// TODO: replace by individual capabilities
AllowPrivilegedContainer: true,
AllowHostNetwork: true,
AllowHostNetwork: true,
RunAsUser: osv1.RunAsUserStrategyOptions{
Type: osv1.RunAsUserStrategyRunAsAny,
},
Expand All @@ -141,6 +146,11 @@ func (c *Reconciler) reconcileOpenshiftPermissions(ctx context.Context) error {
"system:serviceaccount:" + c.privilegedNamespace + ":" + constants.EBPFServiceAccount,
},
}
if desired.Privileged {
scc.AllowPrivilegedContainer = true
} else {
scc.AllowedCapabilities = AllowedCapabilities
}
actual := &osv1.SecurityContextConstraints{}
if err := c.client.Get(ctx, client.ObjectKeyFromObject(scc), actual); err != nil {
if errors.IsNotFound(err) {
Expand All @@ -153,11 +163,7 @@ func (c *Reconciler) reconcileOpenshiftPermissions(ctx context.Context) error {
rlog.Info("creating SecurityContextConstraints")
return c.client.CreateOwned(ctx, scc)
}
if scc.AllowPrivilegedContainer != actual.AllowPrivilegedContainer ||
scc.AllowHostNetwork != actual.AllowHostNetwork ||
scc.RunAsUser != actual.RunAsUser ||
scc.SELinuxContext != actual.SELinuxContext ||
!reflect.DeepEqual(scc.Users, actual.Users) {
if !equality.Semantic.DeepDerivative(&scc, &actual) {
rlog.Info("updating SecurityContextConstraints")
return c.client.UpdateOwned(ctx, actual, scc)
}
Expand Down
14 changes: 11 additions & 3 deletions controllers/flowcollector_controller_ebpf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@ func flowCollectorEBPFSpecs() {
Expect(spec.DNSPolicy).To(Equal(v1.DNSClusterFirstWithHostNet))
Expect(spec.ServiceAccountName).To(Equal(constants.EBPFServiceAccount))
Expect(len(spec.Containers)).To(Equal(1))
Expect(spec.Containers[0].SecurityContext.Privileged).To(Not(BeNil()))
Expect(*spec.Containers[0].SecurityContext.Privileged).To(BeTrue())
Expect(spec.Containers[0].SecurityContext.Privileged).To(BeNil())
Expect(spec.Containers[0].SecurityContext.Capabilities.Add).To(ContainElements(
[]v1.Capability{"BPF", "PERFMON", "NET_ADMIN", "SYS_RESOURCE"},
))
Expect(spec.Containers[0].SecurityContext.RunAsUser).To(Not(BeNil()))
Expect(*spec.Containers[0].SecurityContext.RunAsUser).To(Equal(int64(0)))
Expect(spec.Containers[0].Env).To(ContainElements(
Expand Down Expand Up @@ -130,11 +132,12 @@ func flowCollectorEBPFSpecs() {
Expect(k8sClient.Get(ctx, crKey, &updated)).Should(Succeed())
Expect(updated.Spec.EBPF.Sampling).To(Equal(int32(123)))
updated.Spec.EBPF.Sampling = 4
updated.Spec.EBPF.Privileged = true
Expect(k8sClient.Update(ctx, &updated)).Should(Succeed())

ds := appsv1.DaemonSet{}
By("expecting that the daemonset spec has eventually changed")
Eventually(func() interface{} {
ds := appsv1.DaemonSet{}
if err := k8sClient.Get(ctx, agentKey, &ds); err != nil {
return err
}
Expand All @@ -147,6 +150,11 @@ func flowCollectorEBPFSpecs() {
return fmt.Errorf("unexpected env vars: %#v",
ds.Spec.Template.Spec.Containers[0].Env)
}).WithTimeout(timeout).WithPolling(interval).Should(Succeed())

container := ds.Spec.Template.Spec.Containers[0]
Expect(container.SecurityContext.Privileged).To(Not(BeNil()))
Expect(*container.SecurityContext.Privileged).To(BeTrue())
Expect(container.SecurityContext.Capabilities).To(BeNil())
})

It("Should undeploy everything when deleted", func() {
Expand Down
7 changes: 7 additions & 0 deletions docs/FlowCollector.md
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,13 @@ EBPF contains the settings of an eBPF-based flow reporter when the "agent" prop
<i>Default</i>: info<br/>
</td>
<td>false</td>
</tr><tr>
<td><b>privileged</b></td>
<td>boolean</td>
<td>
Privileged mode for the eBPF Agent container. If false, the operator will add the following capabilities to the container, to enable its correct operation: BPF, PERFMON, NET_ADMIN, SYS_RESOURCE.<br/>
</td>
<td>false</td>
</tr><tr>
<td><b><a href="#flowcollectorspecebpfresources">resources</a></b></td>
<td>object</td>
Expand Down

0 comments on commit d0a45a1

Please sign in to comment.