Skip to content

WIP:DNM: Bpfman operator APIs review #2221

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.

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
190 changes: 190 additions & 0 deletions bpfman-operator/apis/v1alpha1/bpf_application_state_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
/*
Copyright 2023 The bpfman Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metav1types "k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// +union
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'XDP' ? has(self.xdp) : !has(self.xdp)",message="xdp configuration is required when type is xdp, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'TC' ? has(self.tc) : !has(self.tc)",message="tc configuration is required when type is tc, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'TCX' ? has(self.tcx) : !has(self.tcx)",message="tcx configuration is required when type is TCX, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'UProbe' ? has(self.uprobe) : !has(self.uprobe)",message="uprobe configuration is required when type is uprobe, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'URetProbe' ? has(self.uretprobe) : !has(self.uretprobe)",message="uretprobe configuration is required when type is uretprobe, and forbidden otherwise"
type BpfApplicationProgramState struct {
BpfProgramStateCommon `json:",inline"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally we recommend to not add other fields to unions, and have just the discriminator and the members.

It's been found to be confusing in the past when we have mixed members and non members


// type specifies the provisioned eBPF program type for this program entry.
// Type will be one of:
// TC, TCX, UProbe, URetProbe, XDP
//
// When set to TC, the tc object will be populated with the eBPF program data
// associated with a TC program.
//
// When set to TCX, the tcx object will be populated with the eBPF program
// data associated with a TCX program.
//
// When set to UProbe, the uprobe object will be populated with the eBPF
// program data associated with a UProbe program.
//
// When set to URetProbe, the uretprobe object will be populated with the eBPF
// program data associated with a URetProbe program.
//
// When set to XDP, the xdp object will be populated with the eBPF program data
// associated with a URetProbe program.
// +unionDiscriminator
// +required
// +kubebuilder:validation:Enum:="XDP";"TC";"TCX";"UProbe";"URetProbe"
Type EBPFProgType `json:"type"`

// xdp contains the attachment data for an XDP program when type is set to XDP.
// +unionMember
// +optional
XDP *XdpProgramInfoState `json:"xdp,omitempty"`

// tc contains the attachment data for a TC program when type is set to TC.
// +unionMember
// +optional
TC *TcProgramInfoState `json:"tc,omitempty"`

// tcx contains the attachment data for a TCX program when type is set to TCX.
// +unionMember
// +optional
TCX *TcxProgramInfoState `json:"tcx,omitempty"`

// uprobe contains the attachment data for a UProbe program when type is set to
// UProbe.
// +unionMember
// +optional
UProbe *UprobeProgramInfoState `json:"uprobe,omitempty"`

// uretprobe contains the attachment data for a URetProbe program when type is
// set to URetProbe.
// +unionMember
// +optional
URetProbe *UprobeProgramInfoState `json:"uretprobe,omitempty"`
}

type BpfApplicationStateStatus struct {
// UpdateCount tracks the number of times the BpfApplicationState object has
// been updated. The bpfman agent initializes it to 1 when it creates the
// object, and then increments it before each subsequent update. It serves
// as a lightweight sequence number to verify that the API server is serving
// the most recent version of the object before beginning a new Reconcile
// operation.
UpdateCount int64 `json:"updateCount"`
// node is the name of the Kubernets node for this BpfApplicationState.
Node string `json:"node"`
// appLoadStatus reflects the status of loading the eBPF application on the
// given node.
//
// NotLoaded is a temporary state that is assigned when a
// ClusterBpfApplicationState is created and the initial reconcile is being
// processed.
//
// LoadSuccess is returned if all the programs have been loaded with no
// errors.
//
// LoadError is returned if one or more programs encountered an error and
// were not loaded.
//
// NotSelected is returned if this application did not select to run on this
// Kubernetes node.
//
// UnloadSuccess is returned when all the programs were successfully
// unloaded.
//
// UnloadError is returned if one or more programs encountered an error when
// being unloaded.
AppLoadStatus AppLoadStatus `json:"appLoadStatus"`
// programs is a list of eBPF programs contained in the parent BpfApplication
// instance. Each entry in the list contains the derived program attributes as
// well as the attach status for each program on the given Kubernetes node.
Programs []BpfApplicationProgramState `json:"programs,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing SSATags, needs +listType with a value of atomic or map

// conditions contains the summary state of the BpfApplication for the given
// Kubernetes node. If one or more programs failed to load or attach to the
// designated attachment point, the condition will report the error. If more
// than one error has occurred, condition will contain the first error
// encountered.
// +patchMergeKey=type
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
}

// +genclient
// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:resource:scope=Namespaced

// BpfApplicationState contains the state of a BpfApplication instance for a
// given Kubernetes node. When a user creates a BpfApplication instance, bpfman
// creates a BpfApplicationState instance for each node in a Kubernetes
// cluster.
// +kubebuilder:printcolumn:name="Node",type=string,JSONPath=".status.node"
// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[0].reason`
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
type BpfApplicationState struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I realized mid-review of some of the above fields that this seems to be a separate CRD for conveying the observed state of a BpfApplication to users. Is there a reason why you are using a separate CRD for this instead of utilizing the status subresource on the BpfApplication CRD itself?

metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

// status reflects the status of a BpfApplication instance for the given node.
// appLoadStatus and conditions provide an overall status for the given node,
// while each item in the programs list provides a per eBPF program status for
// the given node.
Status BpfApplicationStateStatus `json:"status,omitempty"`
}

// +kubebuilder:object:root=true
// BpfApplicationStateList contains a list of BpfApplicationState objects
type BpfApplicationStateList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []BpfApplicationState `json:"items"`
}

func (an BpfApplicationState) GetName() string {
return an.Name
}

func (an BpfApplicationState) GetUID() metav1types.UID {
return an.UID
}

func (an BpfApplicationState) GetAnnotations() map[string]string {
return an.Annotations
}

func (an BpfApplicationState) GetLabels() map[string]string {
return an.Labels
}

func (an BpfApplicationState) GetConditions() []metav1.Condition {
return an.Status.Conditions
}

func (an BpfApplicationState) GetClientObject() client.Object {
return &an
}

func (anl BpfApplicationStateList) GetItems() []BpfApplicationState {
return anl.Items
}
201 changes: 201 additions & 0 deletions bpfman-operator/apis/v1alpha1/bpf_application_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
/*
Copyright 2024 The bpfman Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// BpfApplicationProgram defines the desired state of BpfApplication
// +union
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'XDP' ? has(self.xdp) : !has(self.xdp)",message="xdp configuration is required when type is xdp, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'TC' ? has(self.tc) : !has(self.tc)",message="tc configuration is required when type is tc, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'TCX' ? has(self.tcx) : !has(self.tcx)",message="tcx configuration is required when type is TCX, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'UProbe' ? has(self.uprobe) : !has(self.uprobe)",message="uprobe configuration is required when type is uprobe, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'URetProbe' ? has(self.uretprobe) : !has(self.uretprobe)",message="uretprobe configuration is required when type is uretprobe, and forbidden otherwise"
type BpfApplicationProgram struct {
// name is a required field and is the name of the function that is the entry
// point for the eBPF program. name must not be an empty string, must not
// exceed 64 characters in length, must start with alpha characters and must
// only contain alphanumeric characters.
// +required
// +kubebuilder:validation:Pattern="^[a-zA-Z][a-zA-Z0-9_]+."
Copy link
Contributor

Choose a reason for hiding this comment

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

General advice is to use CEL expressions to enforce regex matching now so that you can provide a more user-friendly message than returning a regex pattern to them on failed validations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can u pls share more info on how to do so with cel ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like

// +kubebuilder:validation:XValidation:rule=`self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")`,message=`resource must consist of only lowercase alphanumeric characters and hyphens`

// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=64
Name string `json:"name"`

// type is a required field used to specify the type of the eBPF program.
//
// Allowed values are:
// TC, TCX, UProbe, URetProbe, XDP
//
// When set to TC, the eBPF program can attach to network devices (interfaces).
// The program can be attached on either packet ingress or egress, so the
// program will be called on every incoming or outgoing packet seen by the
// network device. When using the TC program type, the tc field is required.
// See tc for more details on TC programs.
//
// When set to TCX, the eBPF program can attach to network devices
// (interfaces). The program can be attached on either packet ingress or
// egress, so the program will be called on every incoming or outgoing packet
// seen by the network device. When using the TCX program type, the tcx field
// is required. See tcx for more details on TCX programs.
//
// When set to UProbe, the program can attach in user-space. The UProbe is
// attached to a binary, library or function name, and optionally an offset in
// the code. When using the UProbe program type, the uprobe field is required.
// See uprobe for more details on UProbe programs.
//
// When set to URetProbe, the program can attach in user-space.
// The URetProbe is attached to the return of a binary, library or function
// name, and optionally an offset in the code. When using the URetProbe
// program type, the uretprobe field is required. See uretprobe for more
// details on URetProbe programs.
//
// When set to XDP, the eBPF program can attach to network devices (interfaces)
// and will be called on every incoming packet received by the network device.
// When using the XDP program type, the xdp field is required. See xdp for more
// details on XDP programs.
// +unionDiscriminator
// +required
// +kubebuilder:validation:Enum:="XDP";"TC";"TCX";"UProbe";"URetProbe"
Type EBPFProgType `json:"type"`

// xdp is an optional field, but required when the type field is set to XDP.
// xdp defines the desired state of the application's XDP programs. XDP program
// can be attached to network devices (interfaces) and will be called on every
// incoming packet received by the network device. The XDP attachment point is
// just after the packet has been received off the wire, but before the Linux
// kernel has allocated an sk_buff, which is used to pass the packet through
// the kernel networking stack.
// +unionMember
// +optional
XDP *XdpProgramInfo `json:"xdp,omitempty"`

// tc is an optional field, but required when the type field is set to TC. tc
// defines the desired state of the application's TC programs. TC programs are
// attached to network devices (interfaces). The program can be attached on
// either packet ingress or egress, so the program will be called on every
// incoming or outgoing packet seen by the network device. The TC attachment
// point is in Linux's Traffic Control (tc) subsystem, which is after the
// Linux kernel has allocated an sk_buff. TCX is newer implementation of TC
// with enhanced performance and better support for running multiple programs
// on a given network device. This makes TC useful for packet classification
// actions.
// +unionMember
// +optional
TC *TcProgramInfo `json:"tc,omitempty"`

// tcx is an optional field, but required when the type field is set to TCX.
// tcx defines the desired state of the application's TCX programs. TCX
// programs are attached to network devices (interfaces). The program can be
// attached on either packet ingress or egress, so the program will be called
// on every incoming or outgoing packet seen by the network device. The TCX
// attachment point is in Linux's Traffic Control (tc) subsystem, which is
// after the Linux kernel has allocated an sk_buff. This makes TCX useful for
// packet classification actions. TCX is a newer implementation of TC with
// enhanced performance and better support for running multiple programs on a
// given network device.
// +unionMember
// +optional
TCX *TcxProgramInfo `json:"tcx,omitempty"`

// uprobe is an optional field, but required when the type field is set to
// UProbe. uprobe defines the desired state of the application's UProbe
// programs. UProbe programs are user-space probes. A target must be provided,
// which is the library name or absolute path to a binary or library where the
// probe is attached. Optionally, a function name can also be provided to
// provide finer granularity on where the probe is attached. They can be
// attached at any point in the binary, library or function using the optional
// offset field. However, caution must be taken when using the offset, ensuring
// the offset is still in the desired bytecode.
// +unionMember
// +optional
UProbe *UprobeProgramInfo `json:"uprobe,omitempty"`

// uretprobe is an optional field, but required when the type field is set to
// URetProbe. uretprobe defines the desired state of the application's
// URetProbe programs. URetProbe programs are user-space probes. A target must
// be provided, which is the library name or absolute path to a binary or
// library where the probe is attached. Optionally, a function name can also be
// provided to provide finer granularity on where the probe is attached. They
// are attached to the return point of the binary, library or function, but can
// be set anywhere using the optional offset field. However, caution must be
// taken when using the offset, ensuring the offset is still in the desired
// bytecode.
// +unionMember
// +optional
URetProbe *UprobeProgramInfo `json:"uretprobe,omitempty"`
}

// spec defines the desired state of the BpfApplication. The BpfApplication
// describes the set of one or more namespace scoped eBPF programs that should
// be loaded for a given application and attributes for how they should be
// loaded. eBPF programs that are grouped together under the same
// BpfApplication instance can share maps and global data between the eBPF
// programs loaded on the same Kubernetes Node.
type BpfApplicationSpec struct {
BpfAppCommon `json:",inline"`

// programs is a required field and is the list of eBPF programs in a BPF
// Application CRD that should be loaded in kernel memory. At least one entry
// is required. eBPF programs in this list will be loaded on the system based
// the nodeSelector. Even if an eBPF program is loaded in kernel memory, it
// cannot be triggered until an attachment point is provided. The different
// program types have different ways of attaching. The attachment points can be
// added at creation time or modified (added or removed) at a later time to
// activate or deactivate the eBPF program as desired.
// CAUTION: When programs are added or removed from the list, that requires all
// programs in the list to be reloaded, which could be temporarily service
// effecting. For this reason, modifying the list is currently not allowed.
// +required
// +kubebuilder:validation:MinItems:=1
Programs []BpfApplicationProgram `json:"programs,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this meant to be an optional field? What happens if I create a BpfApplication and specify no programs?

Copy link
Contributor

@everettraven everettraven Mar 12, 2025

Choose a reason for hiding this comment

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

Generally for lists we recommend having an upper bound to the number of entries. Is there a reasonable upper bound that can be specified here?

Additionally, what other constraints exist here? Can I have multiple program entries with the same .name value? If not there should be an admission time check that prevents this, likely using a CEL expression. If yes, are there any constraints when specifying it multiple times?

Choose a reason for hiding this comment

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

The purpose of a BpfApplication is to ask bpfman to load and possibly attach eBpf programs. I don't see a use case for a BpfApplication CRD with no programs identified. Regarding the bound, I don't know what an acceptable bound would be. The same goes for links.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, this should probably be a required field. For an upper bound, how many programs would you generally expect a user to specify? What number would surprise you as being a really high number?

Choose a reason for hiding this comment

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

Agreed, it should be required. We'll fix that.

Choose a reason for hiding this comment

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

I didn't want to arbitrarily limit it. However, we'll try to come up with some upper bounds on what's reasonable.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can always increase it in the future if needed. You can never decrease it though.

If you truly can't enforce a reasonable upper bound then that is fine, but usually there is a reasonable one that can be set

}

// +genclient
// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:resource:scope=Namespaced

// BpfApplication is the schema for the namespace scoped BPF Applications API.
// This API allows applications to use bpfman to load and attach one or more
// eBPF programs on a Kubernetes cluster.
//
// The bpfApplication.status field reports the overall status of the
// BpfApplication CRD. A given BpfApplication CRD can result in loading and
// attaching multiple eBPF programs on multiple nodes, so this status is just a
// summary. More granular per-node status details can be found in the
// corresponding BpfApplicationState CRD that bpfman creates for each node.
// +kubebuilder:printcolumn:name="NodeSelector",type=string,JSONPath=`.spec.nodeselector`
// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[0].reason`
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
type BpfApplication struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec BpfApplicationSpec `json:"spec,omitempty"`
Status BpfAppStatus `json:"status,omitempty"`
Comment on lines +191 to +192
Copy link
Contributor

Choose a reason for hiding this comment

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

Both these fields should have godoc and either a +required or +optional marker

}

// +kubebuilder:object:root=true
// BpfApplicationList contains a list of BpfApplications
type BpfApplicationList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []BpfApplication `json:"items"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Field should have godoc and either a +required or +optional marker

}
Loading