Fix mounting secrets

[yini7777]

Tracking issue

Closes: #5053

Why are the changes needed?

Context --> we're upgrading from v1.10.6 to v1.11.0 and nothing has been changed on our end. But got the following error:

│ Error: cannot patch "flytescheduler" with kind Deployment: Deployment.apps "flytescheduler" is invalid: spec.template.spec.initContainers[0].volumeMounts[2].name: Not found: "auth"
│ 
│   with helm_release.flyte-unstable-core,
│   on flyte-unstable.tf line 1, in resource "helm_release" "flyte-unstable-core":
│    1: resource "helm_release" "flyte-unstable-core" {

we set Values.secrets.adminOauthClientCredentials.enabled to false because we don't want Flyte to create the secret, as we have created it via an ExternalSecret . In this case Flyte does not mount the secret to the flytescheduler.

Solution --> Add a new value as Values.secrets.adminOauthClientCredentials.create . We will set this to false and enabled to true.

What changes were proposed in this pull request?

How was this patch tested?

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

[welcome]

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

• Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
• Sign off your commits (Reference: DCO Guide).

[davidmirror-ops]

@yini7777 I think a missing conditional for flytescheduler was added in #5054. Nevertheless I guess you're still facing the same issue as the conditional logic is not met when you set the adminOauthCredentials.enabled to false which not only doesn't create the secret but doesn't mount it.
Is my understanding correct?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.11%. Comparing base (2528de7) to head (b256aaf).
Report is 7 commits behind head on master.

❗ Current head b256aaf differs from pull request most recent head be46ef6. Consider uploading reports for the commit be46ef6 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5063      +/-   ##
==========================================
+ Coverage   56.82%   59.11%   +2.28%     
==========================================
  Files          34      645     +611     
  Lines        2154    55574   +53420     
==========================================
+ Hits         1224    32852   +31628     
- Misses        837    20129   +19292     
- Partials       93     2593    +2500     

Flag	Coverage Δ
unittests	59.11% <ø> (+2.28%)	⬆️

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.

[yini7777]

Nevertheless I guess you're still facing the same issue as the conditional logic is not met when you set the adminOauthCredentials.enabled to false which not only doesn't create the secret but doesn't mount it. Is my understanding correct?

Yes exactly!

[yini7777]

@davidmirror-ops I'm wondering if there is anything needs to be tested or modified in this PR.

[davidmirror-ops]

@yini7777 Yesterday we merged #5072 to fix the lint-and-test-charts test. Also I see the generate Helm test failing

Could you please

1. Rebase to master and merge
2. Run make helm and push the changes

Let me know if you have any question

Thanks!

[yini7777]

@wild-endeavor @davidmirror-ops Thank you for your update and explanation. I've made some modifications to align with your suggestions. Could you please review them?

[davidmirror-ops]

I deployed the changes proposed here and validated the behavior is as expected:

create & mount: specify clientSecret in the values file and enabled: true
don't create, but do mount: just set enabled: true
don't do anything: set enabled: false

[yini7777]

@davidmirror-ops Thanks a lot for testing it out! Just curious about when validating don't create, but do mount: just set enabled: true, did u also overwrite the default clientSecret: foobar with clientSecret: null?

[davidmirror-ops]

@yini7777 I commented out that line entirely tbh

[yini7777]

@davidmirror-ops Got it! I think it’s also crucial to test if overwriting with null works as expected, since from a user perspective it’s not practical to comment it out, I am also open to carry out this test myself.

The reason I’m so cautious about this is due to this issue. If you have any guidance on how to test it effectively, I’m eager to learn and contribute. Thanks!

[davidmirror-ops]

@yini7777 you nailed it. when I set

secrets:
  adminOauthClientCredentials:
    enabled: true
    clientSecret: null

propeller fails to start:

MountVolume.SetUp failed for volume "auth" : secret "flyte-secret-auth" not found

[yini7777]

Awesome! Thanks a lot! Then I'll do a rebase and merge it!

[welcome]

Congrats on merging your first pull request! 🎉