feat(webhook): support executable-sourced credentials in sidecar injection#1340
feat(webhook): support executable-sourced credentials in sidecar injection#1340yangspirit wants to merge 6 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yangspirit 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.
Code Review
This pull request introduces support for executable-sourced credentials in the workload identity configuration. It modifies the mutating webhook to conditionally mount token volumes or set the GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES environment variable based on the credential source. Feedback suggests adding validation to ensure either a file or an executable is provided to prevent invalid volume paths and refactoring the mountPath calculation for improved clarity.
| if credConfig.CredentialSource.Executable == nil { | ||
| pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{ |
There was a problem hiding this comment.
Since File is now marked as omitempty in the CredentialConfig struct, it can be an empty string. If both Executable and File are missing or empty, the code will proceed to use an empty path in filepath.Base (line 440), which results in . and may lead to an invalid or unintended volume projection path. Adding validation ensures the configuration is correct before usage.
if credConfig.CredentialSource.Executable == nil {
if credConfig.CredentialSource.File == "" {
return "", nil, fmt.Errorf("workload identity credential configuration must have either 'file' or 'executable' set")
}
pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{| mountPath := filepath.Dir(credentialConfig.CredentialSource.File) | ||
|
|
||
| credentialVolumeMounts := []corev1.VolumeMount{ | ||
| {Name: SidecarContainerWICredentialConfigMapVolumeName, MountPath: SidecarContainerWICredentialConfigMapVolumeMountPath}, | ||
| } | ||
| var envVars []corev1.EnvVar | ||
| if credentialConfig.CredentialSource.Executable == nil { | ||
| credentialVolumeMounts = append(credentialVolumeMounts, corev1.VolumeMount{Name: SidecarContainerWITokenVolumeName, MountPath: mountPath}) | ||
| } else { | ||
| envVars = append(envVars, corev1.EnvVar{Name: "GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES", Value: "1"}) | ||
| } |
There was a problem hiding this comment.
The mountPath variable is only used when Executable is nil. Moving its calculation inside the if block improves code clarity and avoids unnecessary processing when an executable-sourced credential is used. Additionally, consider using a constant for the environment variable name GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES to avoid magic strings.
credentialVolumeMounts := []corev1.VolumeMount{
{Name: SidecarContainerWICredentialConfigMapVolumeName, MountPath: SidecarContainerWICredentialConfigMapVolumeMountPath},
}
var envVars []corev1.EnvVar
if credentialConfig.CredentialSource.Executable == nil {
mountPath := filepath.Dir(credentialConfig.CredentialSource.File)
credentialVolumeMounts = append(credentialVolumeMounts, corev1.VolumeMount{Name: SidecarContainerWITokenVolumeName, MountPath: mountPath})
} else {
envVars = append(envVars, corev1.EnvVar{Name: "GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES", Value: "1"})
}b82741f to
19d3b55
Compare
This change adds support for Executable-Sourced Credentials (AIP-4117) in the GCS Fuse sidecar injector webhook.
mutatingwebhook.go, dynamically constructcredentialVolumeMountsbased on whether the credential configuration uses an executable or a file.GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES=1environment variable to enable the Google Auth Library to use the executable.oidc_test.goto verify the correct injection of environment variables and volume mounts for executable credentials.This unblocks integrations that rely on custom binaries to fetch tokens