Skip to content

Conversation

mhotan
Copy link
Contributor

@mhotan mhotan commented Aug 28, 2024

  • Make adminOauthClientCredentials secret name configurable.
  • This makes it possible to manage the Kubernetes secret outside the flyte-core helm chart.

Tracking issue

N/A

Why are the changes needed?

Union manages flyte-core helm release via ArgoCD. We identified that managing the oath client secret was appropriate outside of the flyte-core helm release and, thus, outside of a Git repository watched by ArgoCD. This situation will likely surface for other flyte users. We are thus backporting these changes from Union to Flyte OSS.

How was this patch tested?

This change was tested and release to all Union managed data planes.

Setup process

Screenshots

Check all the applicable boxes

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

Related PRs

Docs link

Summary by Bito

This pull request introduces a feature improvement by making the admin OAuth client credentials' secret name configurable, allowing for external management by tools like ArgoCD. It also updates shared secrets and checksum values in several YAML manifest files.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 2

@mhotan mhotan force-pushed the mhotan/configurable-secret-name branch from 75c3fcb to 9b7a65c Compare August 28, 2024 19:31
@mhotan mhotan changed the title Make adminOauthClientCredentials secret name configurable (#417) Make it possible to configure flyte with an externally managed Secret Aug 28, 2024
Copy link

codecov bot commented Aug 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.06%. Comparing base (8125ae1) to head (706502d).
Report is 109 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5700      +/-   ##
==========================================
- Coverage   37.07%   37.06%   -0.01%     
==========================================
  Files        1318     1318              
  Lines      132652   132652              
==========================================
- Hits        49175    49169       -6     
- Misses      79228    79234       +6     
  Partials     4249     4249              
Flag Coverage Δ
unittests-datacatalog 51.58% <ø> (ø)
unittests-flyteadmin 54.31% <ø> (-0.03%) ⬇️
unittests-flytecopilot 30.99% <ø> (ø)
unittests-flytectl 62.29% <ø> (ø)
unittests-flyteidl 7.23% <ø> (ø)
unittests-flyteplugins 53.87% <ø> (ø)
unittests-flytepropeller 42.70% <ø> (ø)
unittests-flytestdlib 55.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

davidmirror-ops
davidmirror-ops previously approved these changes Nov 5, 2024
Copy link
Contributor

@davidmirror-ops davidmirror-ops left a comment

Choose a reason for hiding this comment

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

Great stuff thank you!
Sorry for the delays in the review. Could you address the conflicts to merge?

@eapolinario eapolinario merged commit d40b616 into flyteorg:master Jan 23, 2025
58 checks passed
@flyte-bot
Copy link
Collaborator

flyte-bot commented Jan 23, 2025

Code Review Agent Run #0b34fd

Actionable Suggestions - 0
Review Details
  • Files reviewed - 7 · Commit Range: 9b7a65c..706502d
    • charts/flyte-core/templates/clusterresourcesync/deployment.yaml
    • charts/flyte-core/templates/common/secret-auth.yaml
    • charts/flyte-core/templates/propeller/deployment.yaml
    • charts/flyte-core/values.yaml
    • docker/sandbox-bundled/manifests/complete-agent.yaml
    • docker/sandbox-bundled/manifests/complete.yaml
    • docker/sandbox-bundled/manifests/dev.yaml
  • Files skipped - 1
    • charts/flyte-core/README.md - Reason: Filter setting
  • Tools
    • Golangci-lint (Linter) - ✖︎ Failed
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

AI Code Review powered by Bito Logo

@flyte-bot
Copy link
Collaborator

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
Feature Improvement - Configurable Secret Name for OAuth Client

deployment.yaml - Updated secret name to be configurable via values.

secret-auth.yaml - Modified secret name to use a configurable value.

deployment.yaml - Changed secret name to be configurable through values.

values.yaml - Added a new field for configurable secret name.

Other Improvements - Update Shared Secrets and Checksums

complete-agent.yaml - Updated haSharedSecret and checksum/secret values.

complete.yaml - Modified haSharedSecret and checksum/secret values.

dev.yaml - Changed haSharedSecret and checksum/secret values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants