Skip to content

Commit 9a4b426

Browse files
Merge pull request #4819 from linuxfoundation/unicron-fix-health-svc-panic-main
Unicron fix health svc panic main
2 parents d29be2c + 747cd68 commit 9a4b426

File tree

9 files changed

+187
-116
lines changed

9 files changed

+187
-116
lines changed

cla-backend-go/cmd/gitlab/auth/main.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"net/url"
1111
"os"
1212
"strconv"
13+
"sync"
1314

1415
"github.com/gin-gonic/gin"
1516
"github.com/go-resty/resty/v2"
@@ -39,6 +40,8 @@ var passingUsers = map[string]bool{
3940
4041
}
4142

43+
var passingUsersMutex sync.RWMutex // Protects passingUsers map
44+
4245
func main() {
4346
r := gin.Default()
4447
r.GET("/gitlab/sign", func(c *gin.Context) {
@@ -265,7 +268,11 @@ func setCommitStatus(projectID interface{}, commitSha string, userEmail string,
265268
setState := gitlab.Failed
266269

267270
if forceState == "" {
268-
if passingUsers[userEmail] {
271+
passingUsersMutex.RLock()
272+
isPassingUser := passingUsers[userEmail]
273+
passingUsersMutex.RUnlock()
274+
275+
if isPassingUser {
269276
setState = gitlab.Success
270277
}
271278
} else {

cla-backend-go/cmd/migrate_approval_list/main.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ var ghOrgRepo github_organizations.Repository
5050
var gerritService gerrits.Service
5151
var eventsByType []*v1Models.Event
5252
var toUpdateApprovalItems []approvals.ApprovalItem
53+
var toUpdateApprovalItemsMutex sync.Mutex // Protects toUpdateApprovalItems slice
5354

5455
type combinedRepo struct {
5556
users.UserRepository
@@ -252,7 +253,9 @@ func updateApprovalsTable(signature *signatures.ItemSignature) error {
252253
Active: true,
253254
}
254255

256+
toUpdateApprovalItemsMutex.Lock()
255257
toUpdateApprovalItems = append(toUpdateApprovalItems, approvalItem)
258+
toUpdateApprovalItemsMutex.Unlock()
256259
}
257260
}
258261

cla-backend-go/health/service.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ func (s Service) HealthCheck(ctx context.Context) (*models.Health, error) {
7878
// getDynamoTableStatus queries the dynamodb tables and reports if it is healthy
7979
func getDynamoTableStatus() []*models.HealthStatus {
8080
var allStatus []*models.HealthStatus
81+
var mu sync.Mutex
8182

8283
tableNames := []string{
8384
"cla-" + ini.GetStage() + "-ccla-whitelist-requests",
@@ -114,7 +115,9 @@ func getDynamoTableStatus() []*models.HealthStatus {
114115
Name: "EasyCLA - Dynamodb - " + tableName,
115116
Duration: dynamoDuration.String()}
116117

118+
mu.Lock()
117119
allStatus = append(allStatus, &dy)
120+
mu.Unlock()
118121
}(tableName)
119122
}
120123

cla-backend-go/template/repository.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"sort"
1111
"strings"
12+
"sync"
1213
"time"
1314

1415
"github.com/linuxfoundation/easycla/cla-backend-go/utils"
@@ -33,6 +34,7 @@ var (
3334
ApacheStyleTemplateID = "fb4cc144-a76c-4c17-8a52-c648f158fded"
3435
// ASWFStyleTemplateID is template id for ASWF
3536
ASWFStyleTemplateID = "18b8ad08-d7d4-4d75-ad25-30bbfffd59cf"
37+
templateMapMutex sync.RWMutex // Protects templateMap
3638
)
3739

3840
// RepositoryInterface interface functions
@@ -117,9 +119,12 @@ func (r Repository) GetTemplates(ctx context.Context) ([]models.Template, error)
117119

118120
log.WithFields(f).Debug("Loading templates...")
119121
var templates []models.Template
122+
123+
templateMapMutex.RLock()
120124
for _, template := range templateMap {
121125
templates = append(templates, template)
122126
}
127+
templateMapMutex.RUnlock()
123128

124129
// Sort the template list based on the name
125130
log.WithFields(f).Debug("Sorting templates...")
@@ -139,6 +144,8 @@ func (r Repository) GetTemplateName(ctx context.Context, templateID string) (str
139144
}
140145

141146
// For each template...
147+
templateMapMutex.RLock()
148+
defer templateMapMutex.RUnlock()
142149
for _, template := range templateMap {
143150
// If we have a match
144151
if template.ID == templateID {
@@ -152,7 +159,10 @@ func (r Repository) GetTemplateName(ctx context.Context, templateID string) (str
152159

153160
// GetTemplate returns the template based on the template ID
154161
func (r Repository) GetTemplate(templateID string) (models.Template, error) {
162+
templateMapMutex.RLock()
155163
template, ok := templateMap[templateID]
164+
templateMapMutex.RUnlock()
165+
156166
if !ok {
157167
return models.Template{}, ErrTemplateNotFound
158168
}
@@ -162,7 +172,9 @@ func (r Repository) GetTemplate(templateID string) (models.Template, error) {
162172

163173
// CLAGroupTemplateExists return true if the specified template ID exists, false otherwise
164174
func (r Repository) CLAGroupTemplateExists(ctx context.Context, templateID string) bool {
175+
templateMapMutex.RLock()
165176
_, ok := templateMap[templateID]
177+
templateMapMutex.RUnlock()
166178
return ok
167179
}
168180

cla-backend-go/token/token.go

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package token
66
import (
77
"errors"
88
"fmt"
9+
"sync"
910
"time"
1011

1112
"github.com/sirupsen/logrus"
@@ -25,6 +26,7 @@ var (
2526
oauthTokenURL string
2627
token string
2728
expiry time.Time
29+
tokenMutex sync.RWMutex // Protects token and expiry variables
2830
)
2931

3032
type tokenGen struct {
@@ -55,9 +57,11 @@ func Init(paramClientID, paramClientSecret, paramAuth0URL, paramAudience string)
5557
audience = paramAudience
5658
oauthTokenURL = paramAuth0URL
5759

60+
tokenMutex.Lock()
5861
if expiry.Year() == 1 {
5962
expiry = time.Now()
6063
}
64+
tokenMutex.Unlock()
6165

6266
go retrieveToken() //nolint
6367
}
@@ -95,17 +99,23 @@ func retrieveToken() error {
9599
return err
96100
}
97101

98-
//token = tr.TokenType + " " + tr.AccessToken
99-
token = tr.AccessToken
100102
if tr.AccessToken == "" || tr.TokenType == "" {
101103
err = errors.New("error fetching authentication token - response value is empty")
102104
log.WithFields(f).WithError(err).Warn("empty response from auth server")
103105
return err
104106
}
105107

108+
tokenMutex.Lock()
109+
//token = tr.TokenType + " " + tr.AccessToken
110+
token = tr.AccessToken
106111
expiry = time.Now()
112+
tokenMutex.Unlock()
113+
107114
tokenExpiry := time.Now().Add(time.Second * time.Duration(tr.ExpiresIn))
108-
log.WithFields(f).Debugf("retrieved token: %s... expires: %s", token[0:8], tokenExpiry.UTC().String())
115+
tokenMutex.RLock()
116+
tokenForLog := token
117+
tokenMutex.RUnlock()
118+
log.WithFields(f).Debugf("retrieved token: %s... expires: %s", tokenForLog[0:8], tokenExpiry.UTC().String())
109119

110120
return nil
111121
}
@@ -116,15 +126,24 @@ func GetToken() (string, error) {
116126
"functionName": "token.GetToken",
117127
}
118128

129+
tokenMutex.RLock()
130+
currentToken := token
131+
currentExpiry := expiry
132+
tokenMutex.RUnlock()
133+
119134
// set 2.75 hrs duration for new token
120-
if (time.Now().Unix()-expiry.Unix()) > 9900 || token == "" {
135+
if (time.Now().Unix()-currentExpiry.Unix()) > 9900 || currentToken == "" {
121136
log.WithFields(f).Debug("token is either empty or expired, retrieving new token")
122137
err := retrieveToken()
123138
if err != nil {
124139
log.WithFields(f).WithError(err).Warn("unable to retrieve a new token")
125140
return "", err
126141
}
142+
143+
tokenMutex.RLock()
144+
currentToken = token
145+
tokenMutex.RUnlock()
127146
}
128147

129-
return token, nil
148+
return currentToken, nil
130149
}

cla-backend-go/v2/signatures/zip_builder.go

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ package signatures
55

66
import (
77
"bytes"
8+
"errors"
89
"fmt"
10+
"net/http"
911
"strings"
1012
"sync"
1113

@@ -29,7 +31,7 @@ import (
2931

3032
// constants
3133
const (
32-
ParallelDownloader = 100
34+
ParallelDownloader = 10
3335
)
3436

3537
// Zipper implements ZipBuilder interface
@@ -111,8 +113,9 @@ func (z *Zipper) buildPDFZip(claType string, claGroupID string) error {
111113
}
112114
var zipUpdated bool
113115
log.WithFields(f).Debug("getting s3 files")
114-
downloaderInputChan := make(chan *DownloadFileInput)
115-
downloaderOutputChan := make(chan *FileContent)
116+
downloaderInputChan := make(chan *DownloadFileInput, 16)
117+
downloaderOutputChan := make(chan *FileContent, 16)
118+
listErrCh := make(chan error, 1)
116119
var wg sync.WaitGroup
117120
wg.Add(ParallelDownloader)
118121
for i := 1; i <= ParallelDownloader; i++ {
@@ -123,7 +126,7 @@ func (z *Zipper) buildPDFZip(claType string, claGroupID string) error {
123126
close(downloaderOutputChan)
124127
}()
125128
go func() {
126-
err = z.s3.ListObjectsPages(&s3.ListObjectsInput{
129+
localErr := z.s3.ListObjectsPages(&s3.ListObjectsInput{
127130
Bucket: aws.String(z.bucketName),
128131
Prefix: aws.String(s3ZipPrefix(claType, claGroupID)),
129132
}, func(output *s3.ListObjectsOutput, b bool) bool {
@@ -147,12 +150,19 @@ func (z *Zipper) buildPDFZip(claType string, claGroupID string) error {
147150
return true
148151
})
149152
close(downloaderInputChan)
153+
listErrCh <- localErr
150154
}()
151155
zipUpdated = writeFileToZip(writer, downloaderOutputChan)
152-
if err != nil {
153-
return err
156+
if listErr := <-listErrCh; listErr != nil {
157+
if cerr := writer.Close(); cerr != nil {
158+
log.Warnf("zip writer close failed: %v", cerr)
159+
return errors.Join(listErr, cerr)
160+
}
161+
return listErr
162+
}
163+
if cerr := writer.Close(); cerr != nil {
164+
return cerr
154165
}
155-
writer.Close()
156166
if zipUpdated {
157167
remoteZipFileKey := s3ZipFilepath(claType, claGroupID)
158168
log.Debugf("Uploading zip file %s", remoteZipFileKey)
@@ -264,13 +274,16 @@ func getZipWriter(buff *bytes.Buffer) (*zip.Writer, error) {
264274
func (z *Zipper) getZipFileFromS3(claType string, claGroupID string) (*bytes.Buffer, error) {
265275
var buff aws.WriteAtBuffer
266276
remoteFileKey := s3ZipFilepath(claType, claGroupID)
267-
_, err := z.s3.GetObject(&s3.GetObjectInput{
277+
_, err := z.s3.HeadObject(&s3.HeadObjectInput{
268278
Bucket: aws.String(z.bucketName),
269279
Key: aws.String(remoteFileKey),
270280
})
271281
if err != nil {
272-
aerr, ok := err.(awserr.Error)
273-
if ok && aerr.Code() == s3.ErrCodeNoSuchKey {
282+
if rf, ok := err.(awserr.RequestFailure); ok && rf.StatusCode() == http.StatusNotFound {
283+
log.Debugf("zip file %s does not exist on s3", remoteFileKey)
284+
return bytes.NewBuffer(buff.Bytes()), nil
285+
}
286+
if aerr, ok := err.(awserr.Error); ok && (aerr.Code() == s3.ErrCodeNoSuchKey || aerr.Code() == "NotFound") {
274287
log.Debugf("zip file %s does not exist on s3", remoteFileKey)
275288
return bytes.NewBuffer(buff.Bytes()), nil
276289
}

0 commit comments

Comments
 (0)