Add kubernetes events - #348
Conversation
Adding Kubernetes events to make attestation results more visable for admins. Signed-off-by: Yair Podemsky <ypodemsk@redhat.com> Assisted-by: AI
Reviewer's GuideThis PR adds Kubernetes event emission across the registration, attestation, key provisioning, and reference value computation flows, introduces a shared event-recording helper, wires recorders into controllers and HTTP services, and adds tests and utilities to validate the new events end‑to‑end. Sequence diagram for machine registration eventssequenceDiagram
participant User
participant RegisterServer
participant KubernetesAPI
participant Recorder
participant Machine
User->>RegisterServer: GET register endpoint
RegisterServer->>KubernetesAPI: create Machine
KubernetesAPI-->>RegisterServer: created Machine
RegisterServer->>Recorder: record_event MachineRegistered
Recorder->>KubernetesAPI: publish Event for Machine
Sequence diagram for attestation key registration and approval eventssequenceDiagram
participant Client
participant KeyRegister as attestation-key-register
participant KubernetesAPI
participant Recorder
participant AKController as ak-controller
participant Machine
participant AttestationKey
Client->>KeyRegister: PUT attestation key
KeyRegister->>KubernetesAPI: create AttestationKey
KubernetesAPI-->>KeyRegister: created AttestationKey
KeyRegister->>Recorder: record_event AttestationKeyRegistered
Recorder->>KubernetesAPI: publish Event for AttestationKey
AKController->>Recorder: record_event AttestationKeyApproved
Recorder->>KubernetesAPI: publish Event for AttestationKey
AKController->>Recorder: record_event AttestationKeyApproved
Recorder->>KubernetesAPI: publish Event for Machine
Sequence diagram for key provisioning eventssequenceDiagram
participant MachineController as keygen-controller
participant KubernetesAPI
participant Trustee
participant Recorder
participant Machine
MachineController->>Trustee: generate_secret
MachineController->>Trustee: send_secret
Trustee-->>MachineController: provisioning result
alt provisioning succeeds
MachineController->>Recorder: record_event KeyProvisioned
Recorder->>KubernetesAPI: publish Event for Machine
else provisioning fails
MachineController->>Recorder: record_event KeyProvisioningFailed
Recorder->>KubernetesAPI: publish Warning Event for Machine
end
Sequence diagram for reference value computation eventssequenceDiagram
participant ImageController as rv-controller
participant KubernetesAPI
participant ComputationJob
participant Recorder
participant ApprovedImage
ImageController->>ImageController: handle_new_image
ImageController->>Recorder: record_event ComputationStarted
Recorder->>KubernetesAPI: publish Event for ApprovedImage
ComputationJob->>ImageController: job_reconcile
ImageController->>KubernetesAPI: delete completed Job
ImageController->>Recorder: record_event ComputationCompleted
Recorder->>KubernetesAPI: publish Event for ApprovedImage
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yairpod The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="api/v1alpha1/crds.go" line_range="36" />
<code_context>
// +kubebuilder:rbac:groups=trusted-execution-clusters.io,resources=trustedexecutionclusters;machines;approvedimages;attestationkeys,verbs=create;delete;get;list;patch;update;watch
// +kubebuilder:rbac:groups=trusted-execution-clusters.io,resources=trustedexecutionclusters/finalizers;machines/finalizers;attestationkeys/finalizers;approvedimages/finalizers,verbs=update
// +kubebuilder:rbac:groups=trusted-execution-clusters.io,resources=trustedexecutionclusters/status;machines/status;approvedimages/status;attestationkeys/status,verbs=get;patch;update
+// +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch
// TrustedExecutionClusterSpec defines the desired state of TrustedExecutionCluster
</code_context>
<issue_to_address>
**issue (bug_risk):** The event recorder publishes `events.k8s.io` Events, but the checked-in operator RBAC grants `create;patch` only for core `events` (`apiGroups: [""]`), not `events.k8s.io`. Every `record_event` call therefore receives a Kubernetes authorization error in deployed clusters, which is only logged and leaves the new events absent.
**Triggers:** When the checked-in RBAC manifests are deployed without regenerating them to add the `events.k8s.io` rule.
**Suggested fix:** Add `apiGroups: ["events.k8s.io"]` with `resources: ["events"]` and `verbs: ["create", "patch"]` to the deployed operator RBAC, and regenerate all packaged manifests.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the event logic is wrong, Kubernetes Event objects can be emitted with incorrect or overly frequent messages and remain after the code is reverted, though they are bounded and can be deleted. The added RBAC grant also changes what these workloads may write in the cluster, but it does not grant access to application data or alter the underlying provisioning decisions.
Blocking findings: api/v1alpha1/crds.go:36
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
144447f to
051366e
Compare
| } | ||
|
|
||
| pub struct ControllerContext { | ||
| pub client: Client, |
There was a problem hiding this comment.
Why not use cache(aka AkContextData) here insted of client?
Add a test for the attastation basic events. Signed-off-by: Yair Podemsky <ypodemsk@redhat.com> Assisted-by: AI
051366e to
abfe429
Compare
|
@yairpod: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| } | ||
|
|
||
| pub struct ControllerContext { | ||
| pub client: Client, |
| } | ||
|
|
||
| pub async fn record_event( | ||
| recorder: Option<&Recorder>, |
Emitting kubernetes events on major registration/attestation flow points.
This will allow cluster admins to follow and debug what happens in confidential clusters.
Summary by Sourcery
Add Kubernetes event reporting throughout the registration, attestation, key management, and reference-value computation flows to improve cluster observability and debugging.
New Features:
Enhancements:
Tests: