❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

• 🚀 /bns:deploy to deploy the environment

Codecov Report

❌ Patch coverage is 93.61702% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.16%. Comparing base (a4919ed) to head (9220f82).
⚠️ Report is 3 commits behind head on master.

@@            Coverage Diff             @@
##           master   #24080      +/-   ##
==========================================
+ Coverage   60.15%   60.16%   +0.01%     
==========================================
  Files         348      348              
  Lines       59889    59905      +16     
==========================================
+ Hits        36024    36040      +16     
+ Misses      20980    20975       -5     
- Partials     2885     2890       +5     

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

@mtbennett-godaddy is there any reason why we couldn't do something like

// externalServerTLSCertificate will try and load a TLS certificate from an
// external secret, instead of tls.crt and tls.key in argocd-secret. If both
// return values are nil, no external secret has been configured.
func (mgr *SettingsManager) externalServerTLSCertificate(existingCert *tls.Certificate) (*tls.Certificate, error) {
	var cert tls.Certificate
	secret, err := mgr.GetSecretByName(externalServerTLSSecretName)
	if err != nil {
		if apierrors.IsNotFound(err) {
			return nil, nil
		}
	}

	if secret.ResourceVersion != mgr.tlsCertCacheSecretVersion || existingCert == nil {
		log.Infof("Loading TLS configuration from secret %s/%s", mgr.namespace, externalServerTLSSecretName)

		tlsCert, certOK := secret.Data[settingServerCertificate]
		tlsKey, keyOK := secret.Data[settingServerPrivateKey]
		if certOK && keyOK {
			cert, err = tls.X509KeyPair(tlsCert, tlsKey)
			if err != nil {
				return nil, err
			}
		}

		mgr.tlsCertCacheSecretVersion = secret.ResourceVersion
		return &cert, nil
	}

	return existingCert, nil
}

@blakepettersson Commited some refactors before I saw your comment. The refactor adds a loadTLSCertificateFromSecret function that gets called with the external cert and if there’s nothing there, it calls it again with the argocd-secret. The caching logic is in that function so it applies to both cert sources, not just the external one.

However, there are sporadic unit test failures in the GH action runs that I haven’t been able to reproduce locally. Still trying to figure that out.

@blakepettersson Ready for review. I figured out the unit test issue… I was calling invalidateTLSCertCache unnecessarily in the informer event handler, which was being triggered more frequently in the action-run tests than it was locally for some reason.

Seems like this should be cherry-picked back to 3.0 and 3.1 (once this PR gets approved and merged)

@mtbennett-godaddy thanks a lot! Do you mind creating cherry-pick PRs for 3.0 and 3.1?

I'm new to the ArgoCD project and don't really understand the release process. Has this been released? @blakepettersson

@ivanpedersen we haven't cherry-picked this back, so it'll go out in 3.2.0-rc1 which is released in ~2 weeks.

We have cherry-picked and released #24235, which I'm hoping will sufficiently mitigate the logs and performance issues.