Skip to content

Commit 2ba83b0

Browse files
committed
Render the co-pilot Secret only when there is a credential to protect
The gate asked whether the chart could render a complete storage config, not whether that config held anything secret. So S3 with authType=iam, GCS (whose json key the chart hardcodes empty), and Azure without a key all got a Secret and a projected volume on every task pod to protect a region and an endpoint. Gate on the credential instead: S3 accesskey with a literal secretKey, Azure with a key, or an operator-supplied Secret. Ambient auth stays on the command line, where nothing sensitive travels. configuration.inline.storage also qualifies, whatever it holds. The chart cannot see whether an operator put a session token or a service-account key in there, and config-secret.yaml merges it into the Secret, so the safe default is the Secret. secretKeyPath is unchanged: it is credential-bearing but unreadable at template time, so it stays on the command line. Verified the Secret and the plugin key appear or vanish together for every combination, since a mismatch strands task pods in ContainerCreating. Signed-off-by: Kevin Su <pingsutw@apache.org>
1 parent 8090f29 commit 2ba83b0

1 file changed

Lines changed: 19 additions & 9 deletions

File tree

charts/flyte-binary/templates/_helpers.tpl

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -205,24 +205,34 @@ storage:
205205

206206
{{/*
207207
Whether co-pilot reads its storage configuration from a Secret rather than from its command
208-
line. Rendering that Secret and pointing co-pilot at it are both gated on this and must stay
209-
in lockstep: naming a Secret that is never rendered leaves every task pod stuck in
208+
line. The Secret exists to keep credentials out of every task's pod spec, so it is rendered
209+
only when there is a credential to keep out of them. Ambient auth (S3 authType=iam, GCS
210+
workload identity, Azure without a key) puts nothing sensitive on the command line, and a
211+
Secret carrying a region and an endpoint would be machinery protecting nothing.
212+
213+
Rendering the Secret and pointing co-pilot at it are both gated on this and must stay in
214+
lockstep: naming a Secret that is never rendered leaves every task pod stuck in
210215
ContainerCreating.
211216
212-
False only for S3 with secretKeyPath, which names a file living solely in this deployment's
213-
container, so the rendered Secret would carry no usable credentials — and co-pilot takes the
214-
stow config all-or-nothing, making a partial file worse than none. Those deployments keep it
215-
on the command line, or supply their own Secret via storage.copilotStorageSecretRef.
217+
configuration.inline counts, whatever it holds. The chart cannot see whether an operator
218+
put a session token or a GCS service-account key in there, and config-secret.yaml merges
219+
it into the Secret, so err towards the Secret rather than towards the command line.
220+
221+
S3 with only secretKeyPath is the one credential-bearing case that stays on the command
222+
line: the path names a file living solely in this deployment's container, so the chart
223+
cannot read it to render a Secret. Those deployments supply their own via
224+
storage.copilotStorageSecretRef.
216225
*/}}
217226
{{- define "flyte-binary.configuration.copilotStorageFromSecret" -}}
227+
{{- $root := . -}}
218228
{{- with .Values.configuration.storage -}}
219229
{{- if .copilotStorageSecretRef -}}
220230
true
221-
{{- else if ne "s3" .provider -}}
231+
{{- else if and (eq "s3" .provider) (eq "accesskey" .providerConfig.s3.authType) .providerConfig.s3.secretKey -}}
222232
true
223-
{{- else if ne "accesskey" .providerConfig.s3.authType -}}
233+
{{- else if and (eq "azure" .provider) .providerConfig.azure.key -}}
224234
true
225-
{{- else if .providerConfig.s3.secretKey -}}
235+
{{- else if and $root.Values.configuration.inline (hasKey $root.Values.configuration.inline "storage") -}}
226236
true
227237
{{- end -}}
228238
{{- end -}}

0 commit comments

Comments
 (0)