Skip to content

Explicitly set fields while responding from agentk to rails

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

MR: Pending

Description

What

All fields sent from agentk to rails in the reconciliation request payload should be included, even if they are explicitly null or an empty array/object.

Similar to Explicitly set all fields while responding from... (#406261 - closed) (which I think we have already addressed on Rails and can be closed)

When agentk sends information to Rails, it sends the following fields

type WorkspaceAgentInfo struct {
	Name                    string                 `json:"name"`
	Namespace               string                 `json:"namespace,omitempty"`
	LatestK8sDeploymentInfo map[string]interface{} `json:"latest_k8s_deployment_info,omitempty"`
	TerminationProgress     TerminationProgress    `json:"termination_progress,omitempty"`
}

We always send the Name. Everything else is not always sent. This makes it annoying to understand when a field would be present in information received by Rails from agentk. We should ensure all fields are explicitly sent all times with configured defaults.

How

AFAIK, we only generate the information we need to send to Rails in the function generateWorkspaceAgentInfos

  1. For workspaces that are recently updated in the cluster and agentk has received information about it, we are not sending TerminationProgress. Let's send that with an NotApplicable value. In the future, we can add parameter validation on Rails.
  2. For workspaces that are Terminating/Terminated, we are not sending Namespace and LatestK8sDeploymentInfo. For LatestK8sDeploymentInfo, let's send a nil or empty map. Also, send the Namespace in this case as well.

Why sending namespace is important

Assume you have created a workspace called example-workspace in namespace example-ns. Now when we are sending information about the workspaces to rails and skip the namespace, it can be problematic and a bad actor can mess up our logic.

Imagine the user creating a deployment called example-workspace with the exact same configuration as the above workspace in their namespace (let's say personal-ns). The informer will catch this and send us this information. This would start messing up the state of the workspace in our Postgres DB because we would be tracking the details of the example-workspace in the namespace personal-ns.

Followups

On the rails side, while querying a workspace in the DB, we'll use both the name and the namespace. This will ensure we are always updating the right workspace even if there are any kind of malicious/unwarranted instances from the user having access to the kubernetes cluster.

Update:

With #409806 (closed), changes will be made to set namespace when terminating workspaces. Since this issue is bigger in scope and the rest of the requirements are of lower prioirty, it will picked up later in the future

Acceptance Criteria

  • All fields sent from the agent to rails in the reconciliation request should be explicitly set.
Edited by 🤖 GitLab Bot 🤖