Skip to content

Enable the SPIFFE socket volume and endpoint to be configured #309

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.

Sign up for GitHub

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
mattbates wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Enable the SPIFFE socket volume and endpoint to be configured #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
afc8169
add option to specify volume mount for the Workload API socket
mattbates Sep 3, 2025
8e4147f
add config option for the SPIFFE socket path
mattbates Sep 3, 2025
fc76d24
switch order of vars
mattbates Sep 3, 2025
fb6564f
use endpoint instead of path to be more precise
mattbates Sep 3, 2025
9fdb7a6
fix typo
mattbates Sep 3, 2025
ec8968f
fix typo
mattbates Sep 3, 2025
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
16 changes: 10 additions & 6 deletions cmd/cofidectl/cmd/workload/workload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ This command will display the status of workloads in the Cofide configuration st
`

type StatusOpts struct {
podName string
namespace string
trustZone string
podName string
namespace string
trustZone string
spiffeSocketVolume string
spiffeSocketEndpoint string
}

func (w *WorkloadCommand) GetStatusCommand() *cobra.Command {
Expand Down Expand Up @@ -150,6 +152,8 @@ func (w *WorkloadCommand) GetStatusCommand() *cobra.Command {
f.StringVar(&opts.podName, "pod-name", "", "Pod name for the workload")
f.StringVar(&opts.namespace, "namespace", "", "Namespace for the workload")
f.StringVar(&opts.trustZone, "trust-zone", "", "Trust zone for the workload")
f.StringVar(&opts.spiffeSocketVolume, "spiffe-socket-volume", "spiffe-workload-api", "The volume for the SPIFFE socket (UDS)")
f.StringVar(&opts.spiffeSocketEndpoint, "spiffe-socket-endpoint", "", "The full SPIFFE socket (UDS) endpoint URI, which should be prefixed with the /spiffe-workload-api directory.")
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

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

medium

The help text for spiffe-socket-endpoint is a bit confusing. It mentions a "full... URI" but also that it "should be prefixed with". This can be ambiguous for users. A clearer description explaining that this is a path inside the container and giving an example would be more helpful.

Suggested change
f.StringVar(&opts.spiffeSocketEndpoint, "spiffe-socket-endpoint", "", "The full SPIFFE socket (UDS) endpoint URI, which should be prefixed with the /spiffe-workload-api directory.")
f.StringVar(&opts.spiffeSocketEndpoint, "spiffe-socket-endpoint", "", "Path to the SPIFFE Workload API socket in the debug container. The volume is mounted at /spiffe-workload-api, so the path should be under this directory (e.g. /spiffe-workload-api/agent.sock).")


cobra.CheckErr(cmd.MarkFlagRequired("pod-name"))
cobra.CheckErr(cmd.MarkFlagRequired("namespace"))
Expand All @@ -174,7 +178,7 @@ func (w *WorkloadCommand) status(ctx context.Context, ds datasource.DataSource,
return err
}

statusCh, dataCh := getWorkloadStatus(ctx, client, opts.podName, opts.namespace)
statusCh, dataCh := getWorkloadStatus(ctx, client, opts.podName, opts.namespace, opts.spiffeSocketEndpoint, opts.spiffeSocketVolume)

// Create a spinner to display whilst the debug container is created and executed and logs retrieved
if err := statusspinner.WatchProvisionStatus(ctx, statusCh, false); err != nil {
Expand Down Expand Up @@ -239,14 +243,14 @@ func renderRegisteredWorkloads(ctx context.Context, ds datasource.DataSource, ku
return nil
}

func getWorkloadStatus(ctx context.Context, client *kubeutil.Client, podName string, namespace string) (<-chan *provisionpb.Status, chan string) {
func getWorkloadStatus(ctx context.Context, client *kubeutil.Client, podName, namespace, spiffeSocketEndpoint, spiffeSocketVolume string) (<-chan *provisionpb.Status, chan string) {
statusCh := make(chan *provisionpb.Status)
dataCh := make(chan string, 1)

go func() {
defer close(statusCh)
defer close(dataCh)
workload.GetStatus(ctx, statusCh, dataCh, client, podName, namespace)
workload.GetStatus(ctx, statusCh, dataCh, client, podName, namespace, spiffeSocketEndpoint, spiffeSocketVolume)
}()

return statusCh, dataCh
Expand Down
17 changes: 12 additions & 5 deletions internal/pkg/workload/workload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
)

const debugContainerNamePrefix = "cofidectl-debug"
const debugContainerImage = "ghcr.io/cofide/cofidectl-debug-container:v0.2.1"
const debugContainerImage = "ghcr.io/cofide/cofidectl-debug-container:v0.2.2-dev"
Copy link
Member

@jsnctl jsnctl Sep 4, 2025

Choose a reason for hiding this comment

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

This looks to be causing failures in the CI


type Workload struct {
Name string
Expand Down Expand Up @@ -145,15 +145,15 @@ func GetUnregisteredWorkloads(ctx context.Context, kubeCfgFile string, kubeConte
return unregisteredWorkloads, nil
}

func GetStatus(ctx context.Context, statusCh chan<- *provisionpb.Status, dataCh chan string, client *kubeutil.Client, podName string, namespace string) {
func GetStatus(ctx context.Context, statusCh chan<- *provisionpb.Status, dataCh chan string, client *kubeutil.Client, podName, namespace, spiffeSocketEndpoint, spiffeSocketVolumeMount string) {
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

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

medium

The parameter for the SPIFFE socket volume name is inconsistent across function calls (spiffeSocketVolume, spiffeSocketVolumeMount, spiffeSocketVolumeName). For better readability and maintainability, it's best to use a single, consistent name. spiffeSocketVolumeName, as used in createDebugContainer, is descriptive and would be a good choice to standardize on.

I suggest refactoring to use spiffeSocketVolumeName in:

  • StatusOpts struct in cmd/cofidectl/cmd/workload/workload.go
  • getWorkloadStatus function in cmd/cofidectl/cmd/workload/workload.go
  • This GetStatus function signature and its usage of the parameter.
Suggested change
func GetStatus(ctx context.Context, statusCh chan<- *provisionpb.Status, dataCh chan string, client *kubeutil.Client, podName, namespace, spiffeSocketEndpoint, spiffeSocketVolumeMount string) {
func GetStatus(ctx context.Context, statusCh chan<- *provisionpb.Status, dataCh chan string, client *kubeutil.Client, podName, namespace, spiffeSocketEndpoint, spiffeSocketVolumeName string) {

debugContainerName := fmt.Sprintf("%s-%s", debugContainerNamePrefix, rand.String(5))

statusCh <- provision.StatusOk(
"Creating",
fmt.Sprintf("Waiting for ephemeral debug container to be created in %s", podName),
)

if err := createDebugContainer(ctx, client, podName, namespace, debugContainerName); err != nil {
if err := createDebugContainer(ctx, client, podName, namespace, spiffeSocketEndpoint, spiffeSocketVolumeMount, debugContainerName); err != nil {
statusCh <- provision.StatusError(
"Creating",
fmt.Sprintf("Failed waiting for ephemeral debug container to be created in %s", podName),
Expand Down Expand Up @@ -193,7 +193,7 @@ func GetStatus(ctx context.Context, statusCh chan<- *provisionpb.Status, dataCh
)
}

func createDebugContainer(ctx context.Context, client *kubeutil.Client, podName string, namespace string, debugContainerName string) error {
func createDebugContainer(ctx context.Context, client *kubeutil.Client, podName, namespace, spiffeSocketEndpoint, spiffeSocketVolumeName, debugContainerName string) error {
pod, err := client.Clientset.CoreV1().Pods(namespace).Get(ctx, podName, metav1.GetOptions{})
if err != nil {
return err
Expand All @@ -209,13 +209,20 @@ func createDebugContainer(ctx context.Context, client *kubeutil.Client, podName
VolumeMounts: []v1.VolumeMount{
{
ReadOnly: true,
Name: "spiffe-workload-api",
Name: spiffeSocketVolumeName,
MountPath: "/spiffe-workload-api",
}},
},
TargetContainerName: pod.Spec.Containers[0].Name,
}

if spiffeSocketEndpoint != "" {
debugContainer.Env = append(debugContainer.Env, v1.EnvVar{
Name: "SPIFFE_ENDPOINT_SOCKET",
Copy link
Member

@markgoddard markgoddard Sep 15, 2025

Choose a reason for hiding this comment

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

To check: does this work with the existing image? go-spiffe should support this natively.

Value: spiffeSocketEndpoint,
})
}

pod.Spec.EphemeralContainers = append(pod.Spec.EphemeralContainers, debugContainer)

_, err = client.Clientset.CoreV1().Pods(namespace).UpdateEphemeralContainers(
Expand Down
Loading