Skip to content

Commit a7fec04

Browse files
stoneshi-yunifystoneshi-yunify
authored
Merge pull request #1071 from stoneshi-yunify/master
fix: various bug fixes for error handling and nil checks
2 parents 353f62c + de5bc91 commit a7fec04

File tree

8 files changed

+66
-11
lines changed

8 files changed

+66
-11
lines changed

‎cmd/tools/doc-gen/main.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ func generateSwaggerJson() []byte {
9999

100100
swagger := restfulspec.BuildSwagger(config)
101101

102-
data, _ := json.MarshalIndent(swagger, "", " ")
102+
data, err := json.MarshalIndent(swagger, "", " ")
103+
if err != nil {
104+
log.Fatal(err)
105+
}
103106
err = os.WriteFile(output, data, 0644)
104107
if err != nil {
105108
log.Fatal(err)

‎controllers/gitrepository/pull_request_status_controller.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,22 @@ func (r *PullRequestStatusReconciler) getTimeSinceFinished(time *metav1.Time) (s
288288
timeUnit := compileRegex.FindStringSubmatch(duringTime)
289289

290290
sinceTimeList := compileRegex.Split(duringTime, 2)
291+
292+
// Check bounds to prevent panic
293+
if len(sinceTimeList) == 0 || len(timeUnit) == 0 {
294+
return time.Format("2006-01-02")
295+
}
296+
291297
timeNum := strings.Split(sinceTimeList[0], ".")
298+
if len(timeNum) == 0 {
299+
return time.Format("2006-01-02")
300+
}
292301

293302
sinceTime = timeNum[0] + timeUnit[0]
294-
timeToInt, _ := strconv.Atoi(timeNum[0])
303+
timeToInt, err := strconv.Atoi(timeNum[0])
304+
if err != nil {
305+
return time.Format("2006-01-02")
306+
}
295307

296308
if timeUnit[0] == "h" {
297309
if timeToInt >= 24 && timeToInt < 720 {

‎controllers/jenkins/pipeline/json_converter.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ func (r *JenkinsfileReconciler) updateAnnotations(annotations map[string]string,
133133
}
134134

135135
// update annotations
136+
if pipeline.Annotations == nil {
137+
pipeline.Annotations = make(map[string]string)
138+
}
136139
pipeline.Annotations[v1alpha3.PipelineJenkinsfileValueAnnoKey] = annotations[v1alpha3.PipelineJenkinsfileValueAnnoKey]
137140
pipeline.Annotations[v1alpha3.PipelineJenkinsfileEditModeAnnoKey] = annotations[v1alpha3.PipelineJenkinsfileEditModeAnnoKey]
138141
pipeline.Annotations[v1alpha3.PipelineJenkinsfileValidateAnnoKey] = annotations[v1alpha3.PipelineJenkinsfileValidateAnnoKey]
@@ -176,6 +179,9 @@ func (r *JenkinsfileReconciler) updateAnnotationsAndJenkinsfile(annotations map[
176179
}
177180

178181
// update annotations
182+
if pipeline.Annotations == nil {
183+
pipeline.Annotations = make(map[string]string)
184+
}
179185
pipeline.Annotations[v1alpha3.PipelineJenkinsfileEditModeAnnoKey] = annotations[v1alpha3.PipelineJenkinsfileEditModeAnnoKey]
180186
pipeline.Annotations[v1alpha3.PipelineJenkinsfileValidateAnnoKey] = annotations[v1alpha3.PipelineJenkinsfileValidateAnnoKey]
181187
pipeline.Spec.Pipeline.Jenkinsfile = jenkinsfile

‎controllers/jenkins/pipeline/pipeline_controller.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,9 @@ func (c *Controller) updatePipeline(ctx context.Context, name string, nsName str
338338
}
339339
if pipeline.Annotations != nil {
340340
// update annotations
341+
if newPipeline.Annotations == nil {
342+
newPipeline.Annotations = make(map[string]string)
343+
}
341344
newPipeline.Annotations[devopsv1alpha3.PipelineSyncStatusAnnoKey] = pipeline.Annotations[devopsv1alpha3.PipelineSyncStatusAnnoKey]
342345
newPipeline.Annotations[devopsv1alpha3.PipelineSpecHash] = pipeline.Annotations[devopsv1alpha3.PipelineSpecHash]
343346
}

‎controllers/jenkins/pipeline/pipeline_metadata_controller.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ func (r *Reconciler) obtainAndUpdatePipelineMetadata(pipeline *v1alpha3.Pipeline
108108
return err
109109
}
110110
// update annotation
111+
if pipeline.Annotations == nil {
112+
pipeline.Annotations = make(map[string]string)
113+
}
111114
pipeline.Annotations[v1alpha3.PipelineJenkinsMetadataAnnoKey] = string(metadataJSON)
112115
return nil
113116
}
@@ -136,6 +139,9 @@ func (r *Reconciler) obtainAndUpdatePipelineBranches(pipeline *v1alpha3.Pipeline
136139
}
137140

138141
// update annotation
142+
if pipeline.Annotations == nil {
143+
pipeline.Annotations = make(map[string]string)
144+
}
139145
pipeline.Annotations[v1alpha3.PipelineJenkinsBranchesAnnoKey] = string(branchesJSON)
140146
return nil
141147
}

‎pkg/kapis/devops/v1alpha2/devops.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,11 @@ func (h *ProjectPipelineHandler) SubmitInputStep(req *restful.Request, resp *res
450450
"message": fmt.Sprintf("%v", err),
451451
}
452452

453-
response, _ = json.Marshal(msg)
453+
response, err = json.Marshal(msg)
454+
if err != nil {
455+
parseErr(err, resp)
456+
return
457+
}
454458
} else {
455459
response, err = h.devopsOperator.SubmitInputStep(projectName, pipelineName, runId, nodeId, stepId, req.Request)
456460
if err != nil {
@@ -656,7 +660,11 @@ func (h *ProjectPipelineHandler) SubmitBranchInputStep(req *restful.Request, res
656660
"message": fmt.Sprintf("%v", err),
657661
}
658662

659-
response, _ = json.Marshal(msg)
663+
response, err = json.Marshal(msg)
664+
if err != nil {
665+
parseErr(err, resp)
666+
return
667+
}
660668
} else {
661669
response, err = h.devopsOperator.SubmitBranchInputStep(projectName, pipelineName, branchName, runId, nodeId, stepId, req.Request)
662670
if err != nil {

‎pkg/kapis/devops/v1alpha3/scm/utils.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ func getAuthMethod(repoURL, username, password string, sshKey []byte) (transport
3030
if err != nil {
3131
return nil, fmt.Errorf("failed to create SSH auth: %w", err)
3232
}
33+
// TODO: Implement proper host key verification instead of insecurely ignoring host keys.
34+
// This is a temporary measure. Proper host key verification should be implemented
35+
// by providing known hosts or using a custom HostKeyCallback that validates host keys.
3336
publicKeys.HostKeyCallback = gossh.InsecureIgnoreHostKey()
3437

3538
return publicKeys, nil

‎pkg/models/devops/devops.go‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -647,8 +647,12 @@ func (d devopsOperator) GetNodesDetail(projectName, pipelineName, runId string,
647647
return nil, err
648648
}
649649

650-
Nodes, err := json.Marshal(respNodes)
651-
err = json.Unmarshal(Nodes, &nodesDetails)
650+
nodes, marshalErr := json.Marshal(respNodes)
651+
if marshalErr != nil {
652+
klog.Error(marshalErr)
653+
return nil, marshalErr
654+
}
655+
err = json.Unmarshal(nodes, &nodesDetails)
652656
if err != nil {
653657
klog.Error(err)
654658
return nil, err
@@ -658,6 +662,7 @@ func (d devopsOperator) GetNodesDetail(projectName, pipelineName, runId string,
658662
for i, v := range respNodes {
659663
wg.Add(1)
660664
go func(nodeId string, index int) {
665+
defer wg.Done()
661666
// We have to clone the request to prevent concurrent header writes in the next process
662667
Steps, err := d.GetNodeSteps(projectName, pipelineName, runId, nodeId, req.Clone(context.TODO()))
663668
if err != nil {
@@ -666,7 +671,6 @@ func (d devopsOperator) GetNodesDetail(projectName, pipelineName, runId string,
666671
}
667672

668673
stepChan <- &devops.NodesStepsIndex{Id: index, Steps: Steps}
669-
wg.Done()
670674
}(v.ID, i)
671675
}
672676

@@ -820,7 +824,11 @@ func (d devopsOperator) GetBranchNodesDetail(projectName, pipelineName, branchNa
820824
klog.Error(err)
821825
return nil, err
822826
}
823-
Nodes, err := json.Marshal(respNodes)
827+
Nodes, marshalErr := json.Marshal(respNodes)
828+
if marshalErr != nil {
829+
klog.Error(marshalErr)
830+
return nil, marshalErr
831+
}
824832
err = json.Unmarshal(Nodes, &nodesDetails)
825833
if err != nil {
826834
klog.Error(err)
@@ -831,14 +839,14 @@ func (d devopsOperator) GetBranchNodesDetail(projectName, pipelineName, branchNa
831839
for i, v := range nodesDetails {
832840
wg.Add(1)
833841
go func(nodeId string, index int) {
842+
defer wg.Done()
834843
Steps, err := d.GetBranchNodeSteps(projectName, pipelineName, branchName, runId, nodeId, req)
835844
if err != nil {
836845
klog.Error(err)
837846
return
838847
}
839848

840849
stepChan <- &devops.NodesStepsIndex{Id: index, Steps: Steps}
841-
wg.Done()
842850
}(v.ID, i)
843851
}
844852

@@ -1082,9 +1090,15 @@ func getInputReqBody(reqBody io.ReadCloser) (newReqBody io.ReadCloser, err error
10821090
if checkBody.Abort != true && checkBody.Parameters == nil {
10831091
workRound.Parameters = []devops.CheckPlayloadParameters{}
10841092
workRound.ID = checkBody.ID
1085-
jsonBody, _ = json.Marshal(workRound)
1093+
jsonBody, err = json.Marshal(workRound)
1094+
if err != nil {
1095+
return nil, err
1096+
}
10861097
} else {
1087-
jsonBody, _ = json.Marshal(checkBody)
1098+
jsonBody, err = json.Marshal(checkBody)
1099+
if err != nil {
1100+
return nil, err
1101+
}
10881102
}
10891103

10901104
newReqBody = parseBody(bytes.NewBuffer(jsonBody))

0 commit comments

Comments
 (0)