Skip to content

chore(flyte-binary): remove dead config from values.yaml#7559

Merged
pingsutw merged 4 commits into
mainfrom
chore/flyte-binary-remove-dead-values
Jun 19, 2026
Merged

chore(flyte-binary): remove dead config from values.yaml#7559
pingsutw merged 4 commits into
mainfrom
chore/flyte-binary-remove-dead-values

Conversation

@pingsutw

Copy link
Copy Markdown
Member

Why are the changes needed?

While auditing the flyte-binary chart we found several keys in values.yaml (and one
template block) that no template consumes and the v2 binary never reads. They render
nothing, but they show up in the generated README and mislead operators into thinking
they configure something.

What changes were proposed in this pull request?

Remove dead configuration from the chart:

  • flyteconnector (top-level) — a bare enabled flag left over from v1's bundled
    connector subchart. No template references .Values.flyteconnector.
  • configuration.storage.userDataContainer — the chart only renders
    metadataContainer (as storage.container); userDataContainer is never read, so
    setting it has no effect.
  • Self-hosted auth (selfAuthServer) — the v2 binary has no self-hosted OAuth2
    authorization server. It registers no auth config section; runs/config reads only
    authMetadata.* (external/delegated auth) and advertises a public client via
    authMetadata.flyteClient. So the 014-auth-secrets.yaml block in config-secret.yaml
    (auth.appAuth.selfAuthServer.staticClients) was never consumed. Removing it makes the
    entire configuration.auth block dead, so that is removed too and replaced with a
    pointer comment to flyte-core-components.runs.authMetadata (where v2 auth actually
    lives) and the ingress-level SSO.

README.md is regenerated via helm-docs to match.

No functional change: these values were already ignored, and Helm does not error on
unknown values, so users who set any of them are unaffected.

How was this patch tested?

  • helm template output is byte-identical before and after for default values.
  • The selfAuthServer secret no longer renders even when configuration.auth.enabled,
    enableAuthServer, and internal.clientSecretHash are forced on with --set.
  • helm lint passes.
  • README.md regenerated with helm-docs; only the removed keys disappear from the
    parameter table.

Labels

  • removed

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

pingsutw added 2 commits June 18, 2026 15:23
Two keys in the chart's values.yaml are not referenced by any template:

- flyteconnector (top-level): a bare enabled flag left over from the v1
  bundled-connector subchart; no template consumes .Values.flyteconnector.
- configuration.storage.userDataContainer: the chart only renders
  metadataContainer (as storage.container); userDataContainer is never read,
  so setting it has no effect.

Remove both and regenerate the README. helm template output is byte-identical
before and after, confirming nothing consumed them.

Signed-off-by: Kevin Su <pingsutw@apache.org>
The v2 binary has no self-hosted OAuth2 authorization server. It registers no
'auth' config section; runs/config reads only authMetadata.* (external/delegated
auth) and advertises a public client via authMetadata.flyteClient.

So the chart's '014-auth-secrets.yaml' rendering (auth.appAuth.selfAuthServer.
staticClients) was never consumed, and with it gone the entire configuration.auth
block in values.yaml (oidc, internal, flyteClient, authorizedUris,
clientSecretsExternalSecretRef, enable* gates) is dead. Remove both and replace
the values block with a pointer to flyte-core-components.runs.authMetadata.

helm template output is unchanged for default values, and the selfAuthServer
secret no longer renders even when the old auth.* values are forced on.

Signed-off-by: Kevin Su <pingsutw@apache.org>
Copilot AI review requested due to automatic review settings June 18, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR removes Helm chart values and a template fragment in charts/flyte-binary that are not consumed by any template and are not read by the v2 unified binary, to avoid documenting no-op configuration.

Changes:

  • Removed unused configuration.storage.userDataContainer from values.yaml and the generated README parameters table.
  • Removed the unused configuration.auth block from values.yaml and deleted the corresponding auth Secret rendering block from templates/config-secret.yaml.
  • Removed the unused top-level flyteconnector.enabled value and updated README.md accordingly.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
charts/flyte-binary/values.yaml Deletes dead config keys and replaces the old auth section with a guidance comment.
charts/flyte-binary/templates/config-secret.yaml Removes an unused Secret fragment that rendered selfAuthServer/staticClients auth config.
charts/flyte-binary/README.md Regenerates helm-docs output to drop removed parameters from the table.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread charts/flyte-binary/values.yaml
Sovietaced
Sovietaced previously approved these changes Jun 18, 2026
pingsutw added 2 commits June 18, 2026 15:45
Follow-up to the flyte-binary cleanup: the devbox chart still set and documented
flyte-binary.configuration.storage.userDataContainer, a no-op value. Remove it so
the charts stay consistent. (README edited surgically; the devbox README has other
unrelated drift that is out of scope here and not covered by CI helm-docs checks.)

Signed-off-by: Kevin Su <pingsutw@apache.org>
@pingsutw pingsutw merged commit efe332e into main Jun 19, 2026
22 checks passed
@pingsutw pingsutw deleted the chore/flyte-binary-remove-dead-values branch June 19, 2026 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants