Skip to content

Commit 62dcb16

Browse files
committed
Move copilotStorageSecretRef to configuration.co-pilot.storageSecretRef
The value configures co-pilot, not the deployment's object store, so it sat under the wrong section and had to carry a copilot prefix to say so. Under the co-pilot section it is just storageSecretRef, the same way the Go field became StorageConfigSecretName once it lived in FlyteCoPilotConfig. The section name has a hyphen, so reads go through a helper rather than repeating the index dance at four call sites. Renders are byte-identical for every valid combination, and the gate matrix is unchanged: a supplied ref still suppresses the chart's own Secret, and the install still fails when combined with external configuration. Signed-off-by: Kevin Su <pingsutw@apache.org>
1 parent 2ba83b0 commit 62dcb16

5 files changed

Lines changed: 26 additions & 17 deletions

File tree

charts/flyte-binary/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Chart for basic single Flyte executable deployment
1919
| configuration.annotations | object | `{}` | |
2020
| configuration.co-pilot.image.repository | string | `"cr.flyte.org/flyteorg/flyte-binary-v2"` | |
2121
| configuration.co-pilot.image.tag | string | `"latest"` | |
22+
| configuration.co-pilot.storageSecretRef | string | `""` | |
2223
| configuration.connectorService.defaultConnector.defaultTimeout | string | `"10s"` | |
2324
| configuration.connectorService.defaultConnector.endpoint | string | `"k8s://flyteconnector.flyte:8000"` | |
2425
| configuration.connectorService.defaultConnector.insecure | bool | `true` | |
@@ -51,7 +52,6 @@ Chart for basic single Flyte executable deployment
5152
| configuration.logging.plugins.kubernetes.templateUri | string | `""` | |
5253
| configuration.logging.plugins.stackdriver.enabled | bool | `false` | |
5354
| configuration.logging.plugins.stackdriver.templateUri | string | `""` | |
54-
| configuration.storage.copilotStorageSecretRef | string | `""` | |
5555
| configuration.storage.metadataContainer | string | `"my-organization-flyte-container"` | |
5656
| configuration.storage.provider | string | `"s3"` | |
5757
| configuration.storage.providerConfig.azure.account | string | `"storage-account-name"` | |

charts/flyte-binary/templates/_helpers.tpl

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,22 @@ Flag to use external configuration.
110110
{{- or .Values.configuration.externalConfigMap .Values.configuration.externalSecretRef -}}
111111
{{- end -}}
112112

113+
{{/*
114+
The Secret an operator supplied for co-pilot, if any. Its own key rather than a field on
115+
configuration.storage, since it configures co-pilot rather than the deployment's storage,
116+
and the section name means it needs the index dance.
117+
*/}}
118+
{{- define "flyte-binary.configuration.copilotStorageSecretRef" -}}
119+
{{- (index .Values.configuration "co-pilot").storageSecretRef -}}
120+
{{- end -}}
121+
113122
{{/*
114123
Fail on value combinations the chart would otherwise ignore in silence. Called from
115124
deployment.yaml, which renders whatever else is switched off.
116125
*/}}
117126
{{- define "flyte-binary.validateValues" -}}
118-
{{- if and .Values.configuration.storage.copilotStorageSecretRef (include "flyte-binary.configuration.externalConfiguration" .) -}}
119-
{{- fail "configuration.storage.copilotStorageSecretRef has no effect while configuration.externalConfigMap or configuration.externalSecretRef is set: the chart renders no ConfigMap, and that is what would carry plugins.k8s.co-pilot.storage-config-secret-name. Set that key in your own configuration instead, then unset copilotStorageSecretRef." -}}
127+
{{- if and (include "flyte-binary.configuration.copilotStorageSecretRef" .) (include "flyte-binary.configuration.externalConfiguration" .) -}}
128+
{{- fail "configuration.co-pilot.storageSecretRef has no effect while configuration.externalConfigMap or configuration.externalSecretRef is set: the chart renders no ConfigMap, and that is what would carry plugins.k8s.co-pilot.storage-config-secret-name. Set that key in your own configuration instead, then unset storageSecretRef." -}}
120129
{{- end -}}
121130
{{- end -}}
122131

@@ -221,12 +230,12 @@ it into the Secret, so err towards the Secret rather than towards the command li
221230
S3 with only secretKeyPath is the one credential-bearing case that stays on the command
222231
line: the path names a file living solely in this deployment's container, so the chart
223232
cannot read it to render a Secret. Those deployments supply their own via
224-
storage.copilotStorageSecretRef.
233+
co-pilot.storageSecretRef.
225234
*/}}
226235
{{- define "flyte-binary.configuration.copilotStorageFromSecret" -}}
227236
{{- $root := . -}}
228237
{{- with .Values.configuration.storage -}}
229-
{{- if .copilotStorageSecretRef -}}
238+
{{- if include "flyte-binary.configuration.copilotStorageSecretRef" $root -}}
230239
true
231240
{{- else if and (eq "s3" .provider) (eq "accesskey" .providerConfig.s3.authType) .providerConfig.s3.secretKey -}}
232241
true

charts/flyte-binary/templates/config-secret.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ stringData:
4343
{{- end }}
4444
{{- end }}
4545
{{- end }}
46-
{{- if and (include "flyte-binary.configuration.copilotStorageFromSecret" .) (not .Values.configuration.storage.copilotStorageSecretRef) }}
46+
{{- if and (include "flyte-binary.configuration.copilotStorageFromSecret" .) (not (include "flyte-binary.configuration.copilotStorageSecretRef" .)) }}
4747
---
4848
{{/*
4949
Co-pilot's storage configuration. Separate from the configuration Secret above because that

charts/flyte-binary/templates/configmap.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ data:
5050
# Co-pilot reads its storage configuration from this Secret instead of its command
5151
# line, keeping the credentials out of every task's pod spec. Omitted for the
5252
# deployments that cannot use it; see copilotStorageFromSecret.
53-
storage-config-secret-name: {{ .Values.configuration.storage.copilotStorageSecretRef | default (include "flyte-binary.configuration.copilotStorageSecretName" .) | quote }}
53+
storage-config-secret-name: {{ include "flyte-binary.configuration.copilotStorageSecretRef" . | default (include "flyte-binary.configuration.copilotStorageSecretName" .) | quote }}
5454
{{- end }}
5555
# Env vars injected into every task pod, so it can reach the control plane
5656
# to enqueue child actions and watch their state.

charts/flyte-binary/values.yaml

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,6 @@ configuration:
9191
storage:
9292
# metadataContainer Bucket to store Flyte metadata
9393
metadataContainer: "my-organization-flyte-container"
94-
# copilotStorageSecretRef Name of an existing Secret holding co-pilot's complete storage
95-
# configuration — stow settings and credentials — in one or more .yaml keys. The whole
96-
# Secret is mounted into the co-pilot containers and every .yaml key in it is read, so
97-
# it must hold nothing else. Leave empty to have the chart render that Secret itself;
98-
# set it when using s3.secretKeyPath, which names a file that exists only in this
99-
# deployment's container and so cannot be read from a task pod.
100-
# Cannot be combined with externalConfigMap or externalSecretRef: the chart renders no
101-
# ConfigMap to carry the co-pilot key then, so set that key in your own configuration
102-
# instead. The chart fails the install rather than ignoring this value in silence.
103-
copilotStorageSecretRef: ""
10494
# provider Object store provider (Supported values: s3, gcs)
10595
provider: s3
10696
# providerConfig Additional object store provider-specific configuration
@@ -170,6 +160,16 @@ configuration:
170160
repository: cr.flyte.org/flyteorg/flyte-binary-v2 # FLYTE_IMAGE
171161
# tag CoPilot sidecar image tag
172162
tag: latest # FLYTE_TAG
163+
# storageSecretRef Name of an existing Secret holding co-pilot's complete storage
164+
# configuration — stow settings and credentials — in one or more .yaml keys. The whole
165+
# Secret is mounted into the co-pilot containers and every .yaml key in it is read, so
166+
# it must hold nothing else. Leave empty to have the chart render that Secret itself;
167+
# set it when using storage.providerConfig.s3.secretKeyPath, which names a file that
168+
# exists only in this deployment's container and so cannot be read from a task pod.
169+
# Cannot be combined with externalConfigMap or externalSecretRef: the chart renders no
170+
# ConfigMap to carry the co-pilot key then, so set that key in your own configuration
171+
# instead. The chart fails the install rather than ignoring this value in silence.
172+
storageSecretRef: ""
173173
# connectorService Flyte Connector configuration
174174
connectorService:
175175
defaultConnector:

0 commit comments

Comments
 (0)