Skip to content

Commit e59a13a

Browse files
thevilledevtodaywasawesome
thevilledev
authored and
todaywasawesome
committed
Merge commit from fork
Fixed a race condition in repository credentials handling by implementing deep copying of secrets before modification. This prevents concurrent map read/write panics when multiple goroutines access the same secret. The fix ensures thread-safe operations by always operating on copies rather than shared objects. Signed-off-by: Ville Vesilehto <[email protected]> Signed-off-by: Dan Garfield <[email protected]>
1 parent 32d555a commit e59a13a

File tree

2 files changed

+297
-117
lines changed

2 files changed

+297
-117
lines changed

‎util/db/repository_secrets.go‎

Lines changed: 122 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ func (s *secretsRepositoryBackend) CreateRepository(ctx context.Context, reposit
3434
},
3535
}
3636

37-
s.repositoryToSecret(repository, repositorySecret)
37+
updatedSecret := s.repositoryToSecret(repository, repositorySecret)
3838

39-
_, err := s.db.createSecret(ctx, repositorySecret)
39+
_, err := s.db.createSecret(ctx, updatedSecret)
4040
if err != nil {
4141
if apierrors.IsAlreadyExists(err) {
4242
hasLabel, err := s.hasRepoTypeLabel(secName)
@@ -142,9 +142,9 @@ func (s *secretsRepositoryBackend) UpdateRepository(ctx context.Context, reposit
142142
return nil, err
143143
}
144144

145-
s.repositoryToSecret(repository, repositorySecret)
145+
updatedSecret := s.repositoryToSecret(repository, repositorySecret)
146146

147-
_, err = s.db.kubeclientset.CoreV1().Secrets(s.db.ns).Update(ctx, repositorySecret, metav1.UpdateOptions{})
147+
_, err = s.db.kubeclientset.CoreV1().Secrets(s.db.ns).Update(ctx, updatedSecret, metav1.UpdateOptions{})
148148
if err != nil {
149149
return nil, err
150150
}
@@ -187,9 +187,9 @@ func (s *secretsRepositoryBackend) CreateRepoCreds(ctx context.Context, repoCred
187187
},
188188
}
189189

190-
repoCredsToSecret(repoCreds, repoCredsSecret)
190+
updatedSecret := repoCredsToSecret(repoCreds, repoCredsSecret)
191191

192-
_, err := s.db.createSecret(ctx, repoCredsSecret)
192+
_, err := s.db.createSecret(ctx, updatedSecret)
193193
if err != nil {
194194
if apierrors.IsAlreadyExists(err) {
195195
return nil, status.Errorf(codes.AlreadyExists, "repository credentials %q already exists", repoCreds.URL)
@@ -237,9 +237,9 @@ func (s *secretsRepositoryBackend) UpdateRepoCreds(ctx context.Context, repoCred
237237
return nil, err
238238
}
239239

240-
repoCredsToSecret(repoCreds, repoCredsSecret)
240+
updatedSecret := repoCredsToSecret(repoCreds, repoCredsSecret)
241241

242-
repoCredsSecret, err = s.db.kubeclientset.CoreV1().Secrets(s.db.ns).Update(ctx, repoCredsSecret, metav1.UpdateOptions{})
242+
repoCredsSecret, err = s.db.kubeclientset.CoreV1().Secrets(s.db.ns).Update(ctx, updatedSecret, metav1.UpdateOptions{})
243243
if err != nil {
244244
return nil, err
245245
}
@@ -323,73 +323,75 @@ func (s *secretsRepositoryBackend) GetAllOCIRepoCreds(_ context.Context) ([]*app
323323
}
324324

325325
func secretToRepository(secret *corev1.Secret) (*appsv1.Repository, error) {
326+
secretCopy := secret.DeepCopy()
327+
326328
repository := &appsv1.Repository{
327-
Name: string(secret.Data["name"]),
328-
Repo: string(secret.Data["url"]),
329-
Username: string(secret.Data["username"]),
330-
Password: string(secret.Data["password"]),
331-
BearerToken: string(secret.Data["bearerToken"]),
332-
SSHPrivateKey: string(secret.Data["sshPrivateKey"]),
333-
TLSClientCertData: string(secret.Data["tlsClientCertData"]),
334-
TLSClientCertKey: string(secret.Data["tlsClientCertKey"]),
335-
Type: string(secret.Data["type"]),
336-
GithubAppPrivateKey: string(secret.Data["githubAppPrivateKey"]),
337-
GitHubAppEnterpriseBaseURL: string(secret.Data["githubAppEnterpriseBaseUrl"]),
338-
Proxy: string(secret.Data["proxy"]),
339-
NoProxy: string(secret.Data["noProxy"]),
340-
Project: string(secret.Data["project"]),
341-
GCPServiceAccountKey: string(secret.Data["gcpServiceAccountKey"]),
342-
}
343-
344-
insecureIgnoreHostKey, err := boolOrFalse(secret, "insecureIgnoreHostKey")
329+
Name: string(secretCopy.Data["name"]),
330+
Repo: string(secretCopy.Data["url"]),
331+
Username: string(secretCopy.Data["username"]),
332+
Password: string(secretCopy.Data["password"]),
333+
BearerToken: string(secretCopy.Data["bearerToken"]),
334+
SSHPrivateKey: string(secretCopy.Data["sshPrivateKey"]),
335+
TLSClientCertData: string(secretCopy.Data["tlsClientCertData"]),
336+
TLSClientCertKey: string(secretCopy.Data["tlsClientCertKey"]),
337+
Type: string(secretCopy.Data["type"]),
338+
GithubAppPrivateKey: string(secretCopy.Data["githubAppPrivateKey"]),
339+
GitHubAppEnterpriseBaseURL: string(secretCopy.Data["githubAppEnterpriseBaseUrl"]),
340+
Proxy: string(secretCopy.Data["proxy"]),
341+
NoProxy: string(secretCopy.Data["noProxy"]),
342+
Project: string(secretCopy.Data["project"]),
343+
GCPServiceAccountKey: string(secretCopy.Data["gcpServiceAccountKey"]),
344+
}
345+
346+
insecureIgnoreHostKey, err := boolOrFalse(secretCopy, "insecureIgnoreHostKey")
345347
if err != nil {
346348
return repository, err
347349
}
348350
repository.InsecureIgnoreHostKey = insecureIgnoreHostKey
349351

350-
insecure, err := boolOrFalse(secret, "insecure")
352+
insecure, err := boolOrFalse(secretCopy, "insecure")
351353
if err != nil {
352354
return repository, err
353355
}
354356
repository.Insecure = insecure
355357

356-
enableLfs, err := boolOrFalse(secret, "enableLfs")
358+
enableLfs, err := boolOrFalse(secretCopy, "enableLfs")
357359
if err != nil {
358360
return repository, err
359361
}
360362
repository.EnableLFS = enableLfs
361363

362-
enableOCI, err := boolOrFalse(secret, "enableOCI")
364+
enableOCI, err := boolOrFalse(secretCopy, "enableOCI")
363365
if err != nil {
364366
return repository, err
365367
}
366368
repository.EnableOCI = enableOCI
367369

368-
insecureOCIForceHTTP, err := boolOrFalse(secret, "insecureOCIForceHttp")
370+
insecureOCIForceHTTP, err := boolOrFalse(secretCopy, "insecureOCIForceHttp")
369371
if err != nil {
370372
return repository, err
371373
}
372374
repository.InsecureOCIForceHttp = insecureOCIForceHTTP
373375

374-
githubAppID, err := intOrZero(secret, "githubAppID")
376+
githubAppID, err := intOrZero(secretCopy, "githubAppID")
375377
if err != nil {
376378
return repository, err
377379
}
378380
repository.GithubAppId = githubAppID
379381

380-
githubAppInstallationID, err := intOrZero(secret, "githubAppInstallationID")
382+
githubAppInstallationID, err := intOrZero(secretCopy, "githubAppInstallationID")
381383
if err != nil {
382384
return repository, err
383385
}
384386
repository.GithubAppInstallationId = githubAppInstallationID
385387

386-
forceBasicAuth, err := boolOrFalse(secret, "forceHttpBasicAuth")
388+
forceBasicAuth, err := boolOrFalse(secretCopy, "forceHttpBasicAuth")
387389
if err != nil {
388390
return repository, err
389391
}
390392
repository.ForceHttpBasicAuth = forceBasicAuth
391393

392-
useAzureWorkloadIdentity, err := boolOrFalse(secret, "useAzureWorkloadIdentity")
394+
useAzureWorkloadIdentity, err := boolOrFalse(secretCopy, "useAzureWorkloadIdentity")
393395
if err != nil {
394396
return repository, err
395397
}
@@ -398,86 +400,92 @@ func secretToRepository(secret *corev1.Secret) (*appsv1.Repository, error) {
398400
return repository, nil
399401
}
400402

401-
func (s *secretsRepositoryBackend) repositoryToSecret(repository *appsv1.Repository, secret *corev1.Secret) {
402-
if secret.Data == nil {
403-
secret.Data = make(map[string][]byte)
404-
}
405-
406-
updateSecretString(secret, "name", repository.Name)
407-
updateSecretString(secret, "project", repository.Project)
408-
updateSecretString(secret, "url", repository.Repo)
409-
updateSecretString(secret, "username", repository.Username)
410-
updateSecretString(secret, "password", repository.Password)
411-
updateSecretString(secret, "bearerToken", repository.BearerToken)
412-
updateSecretString(secret, "sshPrivateKey", repository.SSHPrivateKey)
413-
updateSecretBool(secret, "enableOCI", repository.EnableOCI)
414-
updateSecretBool(secret, "insecureOCIForceHttp", repository.InsecureOCIForceHttp)
415-
updateSecretString(secret, "tlsClientCertData", repository.TLSClientCertData)
416-
updateSecretString(secret, "tlsClientCertKey", repository.TLSClientCertKey)
417-
updateSecretString(secret, "type", repository.Type)
418-
updateSecretString(secret, "githubAppPrivateKey", repository.GithubAppPrivateKey)
419-
updateSecretInt(secret, "githubAppID", repository.GithubAppId)
420-
updateSecretInt(secret, "githubAppInstallationID", repository.GithubAppInstallationId)
421-
updateSecretString(secret, "githubAppEnterpriseBaseUrl", repository.GitHubAppEnterpriseBaseURL)
422-
updateSecretBool(secret, "insecureIgnoreHostKey", repository.InsecureIgnoreHostKey)
423-
updateSecretBool(secret, "insecure", repository.Insecure)
424-
updateSecretBool(secret, "enableLfs", repository.EnableLFS)
425-
updateSecretString(secret, "proxy", repository.Proxy)
426-
updateSecretString(secret, "noProxy", repository.NoProxy)
427-
updateSecretString(secret, "gcpServiceAccountKey", repository.GCPServiceAccountKey)
428-
updateSecretBool(secret, "forceHttpBasicAuth", repository.ForceHttpBasicAuth)
429-
updateSecretBool(secret, "useAzureWorkloadIdentity", repository.UseAzureWorkloadIdentity)
430-
addSecretMetadata(secret, s.getSecretType())
403+
func (s *secretsRepositoryBackend) repositoryToSecret(repository *appsv1.Repository, secret *corev1.Secret) *corev1.Secret {
404+
secretCopy := secret.DeepCopy()
405+
406+
if secretCopy.Data == nil {
407+
secretCopy.Data = make(map[string][]byte)
408+
}
409+
410+
updateSecretString(secretCopy, "name", repository.Name)
411+
updateSecretString(secretCopy, "project", repository.Project)
412+
updateSecretString(secretCopy, "url", repository.Repo)
413+
updateSecretString(secretCopy, "username", repository.Username)
414+
updateSecretString(secretCopy, "password", repository.Password)
415+
updateSecretString(secretCopy, "bearerToken", repository.BearerToken)
416+
updateSecretString(secretCopy, "sshPrivateKey", repository.SSHPrivateKey)
417+
updateSecretBool(secretCopy, "enableOCI", repository.EnableOCI)
418+
updateSecretBool(secretCopy, "insecureOCIForceHttp", repository.InsecureOCIForceHttp)
419+
updateSecretString(secretCopy, "tlsClientCertData", repository.TLSClientCertData)
420+
updateSecretString(secretCopy, "tlsClientCertKey", repository.TLSClientCertKey)
421+
updateSecretString(secretCopy, "type", repository.Type)
422+
updateSecretString(secretCopy, "githubAppPrivateKey", repository.GithubAppPrivateKey)
423+
updateSecretInt(secretCopy, "githubAppID", repository.GithubAppId)
424+
updateSecretInt(secretCopy, "githubAppInstallationID", repository.GithubAppInstallationId)
425+
updateSecretString(secretCopy, "githubAppEnterpriseBaseUrl", repository.GitHubAppEnterpriseBaseURL)
426+
updateSecretBool(secretCopy, "insecureIgnoreHostKey", repository.InsecureIgnoreHostKey)
427+
updateSecretBool(secretCopy, "insecure", repository.Insecure)
428+
updateSecretBool(secretCopy, "enableLfs", repository.EnableLFS)
429+
updateSecretString(secretCopy, "proxy", repository.Proxy)
430+
updateSecretString(secretCopy, "noProxy", repository.NoProxy)
431+
updateSecretString(secretCopy, "gcpServiceAccountKey", repository.GCPServiceAccountKey)
432+
updateSecretBool(secretCopy, "forceHttpBasicAuth", repository.ForceHttpBasicAuth)
433+
updateSecretBool(secretCopy, "useAzureWorkloadIdentity", repository.UseAzureWorkloadIdentity)
434+
addSecretMetadata(secretCopy, s.getSecretType())
435+
436+
return secretCopy
431437
}
432438

433439
func (s *secretsRepositoryBackend) secretToRepoCred(secret *corev1.Secret) (*appsv1.RepoCreds, error) {
440+
secretCopy := secret.DeepCopy()
441+
434442
repository := &appsv1.RepoCreds{
435-
URL: string(secret.Data["url"]),
436-
Username: string(secret.Data["username"]),
437-
Password: string(secret.Data["password"]),
438-
BearerToken: string(secret.Data["bearerToken"]),
439-
SSHPrivateKey: string(secret.Data["sshPrivateKey"]),
440-
TLSClientCertData: string(secret.Data["tlsClientCertData"]),
441-
TLSClientCertKey: string(secret.Data["tlsClientCertKey"]),
442-
Type: string(secret.Data["type"]),
443-
GithubAppPrivateKey: string(secret.Data["githubAppPrivateKey"]),
444-
GitHubAppEnterpriseBaseURL: string(secret.Data["githubAppEnterpriseBaseUrl"]),
445-
GCPServiceAccountKey: string(secret.Data["gcpServiceAccountKey"]),
446-
Proxy: string(secret.Data["proxy"]),
447-
NoProxy: string(secret.Data["noProxy"]),
448-
}
449-
450-
enableOCI, err := boolOrFalse(secret, "enableOCI")
443+
URL: string(secretCopy.Data["url"]),
444+
Username: string(secretCopy.Data["username"]),
445+
Password: string(secretCopy.Data["password"]),
446+
BearerToken: string(secretCopy.Data["bearerToken"]),
447+
SSHPrivateKey: string(secretCopy.Data["sshPrivateKey"]),
448+
TLSClientCertData: string(secretCopy.Data["tlsClientCertData"]),
449+
TLSClientCertKey: string(secretCopy.Data["tlsClientCertKey"]),
450+
Type: string(secretCopy.Data["type"]),
451+
GithubAppPrivateKey: string(secretCopy.Data["githubAppPrivateKey"]),
452+
GitHubAppEnterpriseBaseURL: string(secretCopy.Data["githubAppEnterpriseBaseUrl"]),
453+
GCPServiceAccountKey: string(secretCopy.Data["gcpServiceAccountKey"]),
454+
Proxy: string(secretCopy.Data["proxy"]),
455+
NoProxy: string(secretCopy.Data["noProxy"]),
456+
}
457+
458+
enableOCI, err := boolOrFalse(secretCopy, "enableOCI")
451459
if err != nil {
452460
return repository, err
453461
}
454462
repository.EnableOCI = enableOCI
455463

456-
insecureOCIForceHTTP, err := boolOrFalse(secret, "insecureOCIForceHttp")
464+
insecureOCIForceHTTP, err := boolOrFalse(secretCopy, "insecureOCIForceHttp")
457465
if err != nil {
458466
return repository, err
459467
}
460468
repository.InsecureOCIForceHttp = insecureOCIForceHTTP
461469

462-
githubAppID, err := intOrZero(secret, "githubAppID")
470+
githubAppID, err := intOrZero(secretCopy, "githubAppID")
463471
if err != nil {
464472
return repository, err
465473
}
466474
repository.GithubAppId = githubAppID
467475

468-
githubAppInstallationID, err := intOrZero(secret, "githubAppInstallationID")
476+
githubAppInstallationID, err := intOrZero(secretCopy, "githubAppInstallationID")
469477
if err != nil {
470478
return repository, err
471479
}
472480
repository.GithubAppInstallationId = githubAppInstallationID
473481

474-
forceBasicAuth, err := boolOrFalse(secret, "forceHttpBasicAuth")
482+
forceBasicAuth, err := boolOrFalse(secretCopy, "forceHttpBasicAuth")
475483
if err != nil {
476484
return repository, err
477485
}
478486
repository.ForceHttpBasicAuth = forceBasicAuth
479487

480-
useAzureWorkloadIdentity, err := boolOrFalse(secret, "useAzureWorkloadIdentity")
488+
useAzureWorkloadIdentity, err := boolOrFalse(secretCopy, "useAzureWorkloadIdentity")
481489
if err != nil {
482490
return repository, err
483491
}
@@ -486,31 +494,35 @@ func (s *secretsRepositoryBackend) secretToRepoCred(secret *corev1.Secret) (*app
486494
return repository, nil
487495
}
488496

489-
func repoCredsToSecret(repoCreds *appsv1.RepoCreds, secret *corev1.Secret) {
490-
if secret.Data == nil {
491-
secret.Data = make(map[string][]byte)
492-
}
493-
494-
updateSecretString(secret, "url", repoCreds.URL)
495-
updateSecretString(secret, "username", repoCreds.Username)
496-
updateSecretString(secret, "password", repoCreds.Password)
497-
updateSecretString(secret, "bearerToken", repoCreds.BearerToken)
498-
updateSecretString(secret, "sshPrivateKey", repoCreds.SSHPrivateKey)
499-
updateSecretBool(secret, "enableOCI", repoCreds.EnableOCI)
500-
updateSecretBool(secret, "insecureOCIForceHttp", repoCreds.InsecureOCIForceHttp)
501-
updateSecretString(secret, "tlsClientCertData", repoCreds.TLSClientCertData)
502-
updateSecretString(secret, "tlsClientCertKey", repoCreds.TLSClientCertKey)
503-
updateSecretString(secret, "type", repoCreds.Type)
504-
updateSecretString(secret, "githubAppPrivateKey", repoCreds.GithubAppPrivateKey)
505-
updateSecretInt(secret, "githubAppID", repoCreds.GithubAppId)
506-
updateSecretInt(secret, "githubAppInstallationID", repoCreds.GithubAppInstallationId)
507-
updateSecretString(secret, "githubAppEnterpriseBaseUrl", repoCreds.GitHubAppEnterpriseBaseURL)
508-
updateSecretString(secret, "gcpServiceAccountKey", repoCreds.GCPServiceAccountKey)
509-
updateSecretString(secret, "proxy", repoCreds.Proxy)
510-
updateSecretString(secret, "noProxy", repoCreds.NoProxy)
511-
updateSecretBool(secret, "forceHttpBasicAuth", repoCreds.ForceHttpBasicAuth)
512-
updateSecretBool(secret, "useAzureWorkloadIdentity", repoCreds.UseAzureWorkloadIdentity)
513-
addSecretMetadata(secret, common.LabelValueSecretTypeRepoCreds)
497+
func repoCredsToSecret(repoCreds *appsv1.RepoCreds, secret *corev1.Secret) *corev1.Secret {
498+
secretCopy := secret.DeepCopy()
499+
500+
if secretCopy.Data == nil {
501+
secretCopy.Data = make(map[string][]byte)
502+
}
503+
504+
updateSecretString(secretCopy, "url", repoCreds.URL)
505+
updateSecretString(secretCopy, "username", repoCreds.Username)
506+
updateSecretString(secretCopy, "password", repoCreds.Password)
507+
updateSecretString(secretCopy, "bearerToken", repoCreds.BearerToken)
508+
updateSecretString(secretCopy, "sshPrivateKey", repoCreds.SSHPrivateKey)
509+
updateSecretBool(secretCopy, "enableOCI", repoCreds.EnableOCI)
510+
updateSecretBool(secretCopy, "insecureOCIForceHttp", repoCreds.InsecureOCIForceHttp)
511+
updateSecretString(secretCopy, "tlsClientCertData", repoCreds.TLSClientCertData)
512+
updateSecretString(secretCopy, "tlsClientCertKey", repoCreds.TLSClientCertKey)
513+
updateSecretString(secretCopy, "type", repoCreds.Type)
514+
updateSecretString(secretCopy, "githubAppPrivateKey", repoCreds.GithubAppPrivateKey)
515+
updateSecretInt(secretCopy, "githubAppID", repoCreds.GithubAppId)
516+
updateSecretInt(secretCopy, "githubAppInstallationID", repoCreds.GithubAppInstallationId)
517+
updateSecretString(secretCopy, "githubAppEnterpriseBaseUrl", repoCreds.GitHubAppEnterpriseBaseURL)
518+
updateSecretString(secretCopy, "gcpServiceAccountKey", repoCreds.GCPServiceAccountKey)
519+
updateSecretString(secretCopy, "proxy", repoCreds.Proxy)
520+
updateSecretString(secretCopy, "noProxy", repoCreds.NoProxy)
521+
updateSecretBool(secretCopy, "forceHttpBasicAuth", repoCreds.ForceHttpBasicAuth)
522+
updateSecretBool(secretCopy, "useAzureWorkloadIdentity", repoCreds.UseAzureWorkloadIdentity)
523+
addSecretMetadata(secretCopy, common.LabelValueSecretTypeRepoCreds)
524+
525+
return secretCopy
514526
}
515527

516528
func (s *secretsRepositoryBackend) getRepositorySecret(repoURL, project string, allowFallback bool) (*corev1.Secret, error) {

0 commit comments

Comments
 (0)