fix: ensure certificate gets updated on reload

[blakepettersson]

Fixes #10707. GetCertificate ensures that the most current version of
a.settings.Certificate is used. It's still a bit of a mystery to me
as to why the reloading of the server does not work for this, since it
should fulfill the same function.

I'm not sure of the best way to test that this works the way as intended,
but I tested this locally with the test suite given in #10707 and this works
fine.

Signed-off-by: Blake Pettersson blake.pettersson@gmail.com

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).

[codecov]

Codecov Report

Base: 47.29% // Head: 47.42% // Increases project coverage by +0.13% 🎉

Coverage data is based on head (cd44be6) compared to base (1ab4026).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12076      +/-   ##
==========================================
+ Coverage   47.29%   47.42%   +0.13%     
==========================================
  Files         245      246       +1     
  Lines       41670    41826     +156     
==========================================
+ Hits        19707    19838     +131     
- Misses      19978    19992      +14     
- Partials     1985     1996      +11     

Impacted Files	Coverage Δ
server/server.go	56.38% <0.00%> (+2.34%)	⬆️
util/oidc/provider.go	34.42% <0.00%> (-20.12%)	⬇️
server/application/terminal.go	19.48% <0.00%> (-1.17%)	⬇️
util/oidc/oidc.go	57.43% <0.00%> (-0.34%)	⬇️
util/kube/portforwarder.go	0.00% <0.00%> (ø)
cmd/argocd/commands/admin/cluster.go	0.00% <0.00%> (ø)
util/security/jwt.go	42.10% <0.00%> (ø)
applicationset/generators/matrix.go	69.74% <0.00%> (+0.25%)	⬆️
controller/appcontroller.go	52.27% <0.00%> (+0.39%)	⬆️
util/settings/settings.go	49.20% <0.00%> (+0.77%)	⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[crenshaw-dev]

I think this is the change that broke it: 9d4c940

I don’t completely understand what causes us to move through the loop. But prior to that change, we’d initialize ArgoCDServer on each iteration. Now we initialize it outside that loop. The certificates are set in NewServer and then never updated.

[blakepettersson]

I don’t completely understand what causes us to move through the loop. But prior to that change, we’d initialize ArgoCDServer on each iteration. Now we initialize it outside that loop. The certificates are set in NewServer and then never updated.

I haven't had time to look into this in depth but this probably means that the webhook reloading doesn't work either. I modified watchSettings to not to break out of the loop on cert changes, but watchSettings itself probably needs an overhaul. IMO this could be handled in another PR though.

[drpaneas]

/lgtm

I've tested this locally and indeed it fixes the regression. I've used the testsuite from @vx-github :

Step 1: Get the current certificate

2023-03-02 11:17:13
subject= /C=NL/O=Argo CD Cert Test number 1/CN=localhost
notBefore=Mar  2 10:16:12 2023 GMT
notAfter=Mar 12 10:16:12 2023 GMT
serial=ECD5483932CD2E8C <--- old

Step 2: Update the certificate secret/argocd-server-tls

2023-03-02 11:17:14
subject= /C=NL/O=Argo CD Cert Test number 2/CN=localhost
notBefore=Mar  2 10:17:14 2023 GMT
notAfter=Mar 12 10:17:14 2023 GMT
serial=B850266BA30BFB68

secret/argocd-server-tls configured

Check if the certificate has indeed changed

2023-03-02 11:17:14
subject= /C=NL/O=Argo CD Cert Test number 2/CN=localhost
notBefore=Mar  2 10:17:14 2023 GMT
notAfter=Mar 12 10:17:14 2023 GMT
serial=B850266BA30BFB68 <--- new

Notice the change now it so fast it happens even within the same second.

This is also visible from the logs:

[argocd-server] time="2023-03-02T10:17:14Z" level=info msg="Loading TLS configuration from secret argocd/argocd-server-tls"
[argocd-server] time="2023-03-02T10:17:14Z" level=info msg="Notifying 1 settings subscribers: [0xc00135a960]"
[argocd-server] time="2023-03-02T10:17:14Z" level=info msg="tls certificate modified. reloading certificate"
[dex] time="2023-03-02T10:17:14Z" level=info msg="Loading TLS configuration from secret argocd/argocd-server-tls"
[dex] time="2023-03-02T10:17:14Z" level=info msg="Notifying 1 settings subscribers: [0xc0001866c0]"
[dex] time="2023-03-02T10:17:14Z" level=info msg="dex config unmodified"
[argocd-applicationset-controller] time="2023-03-02T10:17:14Z" level=info msg="Loading TLS configuration from secret argocd/argocd-server-tls"
[argocd-application-controller] time="2023-03-02T10:17:19Z" level=info msg="Loading TLS configuration from secret argocd/argocd-server-tls

In the previous version, the tlsConfig variable was set with a single certificate using Certificates field.

However, in this PR's updated version, the TlsConfig.GetCertificate method is set instead, which returns the certificate from the a.settings.Certificate variable that contains the latest version of certificates. This hook dynamically reloads the certificate as needed, without restarting the server, which could be more efficient and less disruptive to ongoing operations.

[crenshaw-dev]

/cherry-pick release-2.4

[crenshaw-dev]

/cherry-pick release-2.5

[crenshaw-dev]

/cherry-pick release-2.6