Skip to content

Commit 98feda1

Browse files
committed
fix(server): rebuild extension proxy registry when secrets change
The watchSettings() loop only called UpdateExtensionRegistry() when ExtensionConfig (ConfigMap data) changed, but not when settings.Secrets changed. This meant that if secrets were populated after initial startup (e.g. by External Secrets Operator), the proxy registry was never rebuilt with resolved secret values. UpdateExtensionRegistry() runs parseAndValidateConfig() which calls ReplaceMapSecrets() to substitute $key references with actual secret values. Without rebuilding the registry on secret changes, proxies continued sending literal strings like $openai-api-key to backends, causing 401 errors. Add a check for settings.Secrets changes alongside ExtensionConfig changes, and update prevSecrets accordingly. Also add a test that verifies proxy headers are updated when secrets change. Signed-off-by: Said Sef <saidsef@gmail.com>
1 parent e75b8e0 commit 98feda1

2 files changed

Lines changed: 177 additions & 1 deletion

File tree

server/extension/extension_test.go

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,3 +874,177 @@ extensions:
874874
- name: some-header-name
875875
`
876876
}
877+
878+
func getExtensionConfigWithSecret(name, url, secretRef string) string {
879+
cfg := `
880+
extensions:
881+
- name: %s
882+
backend:
883+
services:
884+
- url: %s
885+
headers:
886+
- name: Authorization
887+
value: '%s'
888+
`
889+
return fmt.Sprintf(cfg, name, url, secretRef)
890+
}
891+
892+
func TestUpdateExtensionRegistryWithSecrets(t *testing.T) {
893+
t.Parallel()
894+
895+
defaultServerNamespace := "control-plane-ns"
896+
defaultProjectName := "project-name"
897+
898+
setup := func() (*mocks.ApplicationGetter, *mocks.SettingsGetter, *mocks.RbacEnforcer, *mocks.ProjectGetter, *mocks.ExtensionMetricsRegistry, *mocks.UserGetter, *extension.Manager, *http.ServeMux) {
899+
appMock := &mocks.ApplicationGetter{}
900+
settMock := &mocks.SettingsGetter{}
901+
rbacMock := &mocks.RbacEnforcer{}
902+
projMock := &mocks.ProjectGetter{}
903+
metricsMock := &mocks.ExtensionMetricsRegistry{}
904+
userMock := &mocks.UserGetter{}
905+
906+
dbMock := &dbmocks.ArgoDB{}
907+
dbMock.EXPECT().GetClusterServersByName(mock.Anything, mock.Anything).Return([]string{"cluster1"}, nil).Maybe()
908+
dbMock.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(&v1alpha1.Cluster{Server: "some-url", Name: "cluster1"}, nil).Maybe()
909+
910+
logger, _ := test.NewNullLogger()
911+
logEntry := logger.WithContext(t.Context())
912+
m := extension.NewManager(logEntry, defaultServerNamespace, settMock, appMock, projMock, dbMock, rbacMock, userMock)
913+
m.AddMetricsRegistry(metricsMock)
914+
915+
mux := http.NewServeMux()
916+
extHandler := http.HandlerFunc(m.CallExtension())
917+
mux.Handle(extension.URLPrefix+"/", extHandler)
918+
919+
return appMock, settMock, rbacMock, projMock, metricsMock, userMock, m, mux
920+
}
921+
922+
newExtensionRequest := func(t *testing.T, method, url string) *http.Request {
923+
t.Helper()
924+
r, err := http.NewRequestWithContext(t.Context(), method, url, http.NoBody)
925+
require.NoError(t, err, "error initializing request")
926+
r.Header.Add(extension.HeaderArgoCDApplicationName, "namespace:app-name")
927+
r.Header.Add(extension.HeaderArgoCDProjectName, defaultProjectName)
928+
return r
929+
}
930+
931+
getApp := func(destName, destServer, projName string) *v1alpha1.Application {
932+
return &v1alpha1.Application{
933+
TypeMeta: metav1.TypeMeta{},
934+
ObjectMeta: metav1.ObjectMeta{},
935+
Spec: v1alpha1.ApplicationSpec{
936+
Destination: v1alpha1.ApplicationDestination{
937+
Name: destName,
938+
Server: destServer,
939+
},
940+
Project: projName,
941+
},
942+
Status: v1alpha1.ApplicationStatus{
943+
Resources: []v1alpha1.ResourceStatus{
944+
{
945+
Group: "apps",
946+
Version: "v1",
947+
Kind: "Pod",
948+
Namespace: "default",
949+
Name: "some-pod",
950+
},
951+
},
952+
},
953+
}
954+
}
955+
956+
getProjectWithDestinations := func(prjName string, destNames []string, destURLs []string) *v1alpha1.AppProject {
957+
destinations := []v1alpha1.ApplicationDestination{}
958+
for _, destName := range destNames {
959+
destination := v1alpha1.ApplicationDestination{
960+
Name: destName,
961+
}
962+
destinations = append(destinations, destination)
963+
}
964+
for _, destURL := range destURLs {
965+
destination := v1alpha1.ApplicationDestination{
966+
Server: destURL,
967+
}
968+
destinations = append(destinations, destination)
969+
}
970+
return &v1alpha1.AppProject{
971+
ObjectMeta: metav1.ObjectMeta{
972+
Name: prjName,
973+
},
974+
Spec: v1alpha1.AppProjectSpec{
975+
Destinations: destinations,
976+
},
977+
}
978+
}
979+
980+
t.Run("will update proxy headers when secrets change", func(t *testing.T) {
981+
t.Parallel()
982+
appMock, settMock, rbacMock, projMock, metricsMock, userMock, manager, mux := setup()
983+
984+
extName := "secret-test"
985+
oldToken := "Bearer old-token"
986+
newToken := "Bearer new-token"
987+
988+
backendSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
989+
auth := r.Header.Get("Authorization")
990+
w.Header().Set("X-Received-Auth", auth)
991+
fmt.Fprintln(w, "ok")
992+
}))
993+
defer backendSrv.Close()
994+
995+
// Initial settings with old secret
996+
secrets := map[string]string{"my.secret": oldToken}
997+
initialSettings := &settings.ArgoCDSettings{
998+
ExtensionConfig: map[string]string{
999+
"": getExtensionConfigWithSecret(extName, backendSrv.URL, "$my.secret"),
1000+
},
1001+
Secrets: secrets,
1002+
}
1003+
settMock.EXPECT().Get().Return(initialSettings, nil).Maybe()
1004+
1005+
// RBAC allow all
1006+
rbacMock.EXPECT().EnforceErr(mock.Anything, rbac.ResourceApplications, rbac.ActionGet, mock.Anything).Return(nil).Maybe()
1007+
rbacMock.EXPECT().EnforceErr(mock.Anything, rbac.ResourceExtensions, rbac.ActionInvoke, mock.Anything).Return(nil).Maybe()
1008+
1009+
userMock.EXPECT().GetUserId(mock.Anything).Return("some-user-id").Maybe()
1010+
userMock.EXPECT().GetUsername(mock.Anything).Return("some-user").Maybe()
1011+
userMock.EXPECT().GetGroups(mock.Anything).Return([]string{"group1"}).Maybe()
1012+
1013+
projMock.EXPECT().Get(defaultProjectName).Return(getProjectWithDestinations("project-name", nil, []string{"some-url"}), nil).Maybe()
1014+
1015+
metricsMock.EXPECT().IncExtensionRequestCounter(mock.Anything, mock.Anything).Maybe()
1016+
metricsMock.EXPECT().ObserveExtensionRequestDuration(mock.Anything, mock.Anything).Maybe()
1017+
1018+
err := manager.RegisterExtensions()
1019+
require.NoError(t, err)
1020+
1021+
ts := httptest.NewServer(mux)
1022+
defer ts.Close()
1023+
1024+
// First request - should get old token
1025+
r1 := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
1026+
appMock.EXPECT().Get(mock.Anything, mock.Anything).Return(getApp("", "some-url", defaultProjectName), nil).Maybe()
1027+
1028+
resp1, err := http.DefaultClient.Do(r1)
1029+
require.NoError(t, err)
1030+
require.Equal(t, http.StatusOK, resp1.StatusCode)
1031+
assert.Equal(t, oldToken, resp1.Header.Get("X-Received-Auth"))
1032+
1033+
// Update settings with new secret
1034+
newSettings := &settings.ArgoCDSettings{
1035+
ExtensionConfig: initialSettings.ExtensionConfig,
1036+
Secrets: map[string]string{"my.secret": newToken},
1037+
}
1038+
err = manager.UpdateExtensionRegistry(newSettings)
1039+
require.NoError(t, err)
1040+
1041+
// Second request - should get new token
1042+
r2 := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
1043+
appMock.EXPECT().Get(mock.Anything, mock.Anything).Return(getApp("", "some-url", defaultProjectName), nil).Maybe()
1044+
1045+
resp2, err := http.DefaultClient.Do(r2)
1046+
require.NoError(t, err)
1047+
require.Equal(t, http.StatusOK, resp2.StatusCode)
1048+
assert.Equal(t, newToken, resp2.Header.Get("X-Received-Auth"))
1049+
})
1050+
}

server/server.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,7 @@ func (server *ArgoCDServer) watchSettings() {
798798
prevBitbucketServerSecret := server.settings.GetWebhookBitbucketServerSecret()
799799
prevGogsSecret := server.settings.GetWebhookGogsSecret()
800800
prevExtConfig := server.settings.ExtensionConfig
801+
prevSecrets := server.settings.Secrets
801802
var prevCert, prevCertKey string
802803
if server.settings.Certificate != nil && !server.Insecure {
803804
prevCert, prevCertKey = tlsutil.EncodeX509KeyPairString(*server.settings.Certificate)
@@ -844,8 +845,9 @@ func (server *ArgoCDServer) watchSettings() {
844845
log.Infof("gogs secret modified. restarting")
845846
break
846847
}
847-
if !reflect.DeepEqual(prevExtConfig, server.settings.ExtensionConfig) {
848+
if !reflect.DeepEqual(prevExtConfig, server.settings.ExtensionConfig) || !reflect.DeepEqual(prevSecrets, server.settings.Secrets) {
848849
prevExtConfig = server.settings.ExtensionConfig
850+
prevSecrets = server.settings.Secrets
849851
log.Infof("extensions configs modified. Updating proxy registry...")
850852
err := server.extensionManager.UpdateExtensionRegistry(server.settings)
851853
if err != nil {

0 commit comments

Comments
 (0)