Update Jenkinsfile_API #441
Conversation
WalkthroughUpdated Jenkins pipeline script to use double-quoted shell command for rollback, enabling interpolation of ${params.namespace} so kubectl receives the actual namespace. No other logic or flow modifications. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
deployment/stage/mom-api/Jenkinsfile_API (2)
79-82: Fix kubeconfig user mismatch (sudo gcloud vs non‑sudo kubectl).Running gcloud with sudo writes kubeconfig under root, but kubectl runs as the Jenkins user and won’t see those creds. Run gcloud without sudo (or run kubectl with sudo). Prefer removing sudo here.
Apply this diff:
- sudo gcloud auth activate-service-account --key-file=${GOOGLE_APPLICATION_CREDENTIALS} - sudo gcloud container clusters get-credentials ${GKE_CLUSTER} --zone ${GKE_ZONE} --project ${GCP_PROJECT} + gcloud auth activate-service-account --key-file="${GOOGLE_APPLICATION_CREDENTIALS}" + gcloud container clusters get-credentials "${GKE_CLUSTER}" --zone "${GKE_ZONE}" --project "${GCP_PROJECT}" --quietAlso applies to: 116-118
108-113: Validate/quote namespace to avoid shell injection; pass via env.User‑supplied params.namespace is interpolated directly into sh. Validate and pass via withEnv; also add rollout timeout.
Apply this diff:
def imageDeploySucceeded = false def imageTag = env.GIT_COMMIT_HASH - echo "this is the fetched docker image tag: ${imageTag}" + echo "this is the fetched docker image tag: ${imageTag}" + // Validate and safely pass namespace + def ns = params.namespace?.trim() + if (!(ns ==~ /^[a-z0-9]([-a-z0-9]*[a-z0-9])?$/)) { + error("Invalid namespace: ${ns}") + } try { - sh """ - kubectl set image deployment/mom-api-deployment mom-api=${DOCKER_REGISTRY}/mom-api:${imageTag} -n ${params.namespace} - kubectl rollout status deployment/mom-api-deployment -n ${params.namespace} - """ + withEnv(["NS=${ns}", "IMAGE_TAG=${imageTag}"]) { + sh ''' + kubectl set image deployment/mom-api-deployment mom-api="$DOCKER_REGISTRY"/mom-api:"$IMAGE_TAG" -n "$NS" + kubectl rollout status deployment/mom-api-deployment -n "$NS" --timeout=300s + ''' + } imageDeploySucceeded = true } catch (Exception e) { echo "Deployment failed: ${e}" } if (!imageDeploySucceeded) { echo 'Rolling back to previous revision...' - sh "kubectl rollout undo deployment/mom-api-deployment -n ${params.namespace}" + withEnv(["NS=${ns}"]) { + sh 'kubectl rollout undo deployment/mom-api-deployment -n "$NS"' + } } ... - echo "checking the deployment status" && kubectl get pods -n ${params.namespace} + withEnv(["NS=${ns}"]) { + echo "checking the deployment status" && kubectl get pods -n "$NS" + }Also applies to: 115-118, 126-126, 141-142
🧹 Nitpick comments (6)
deployment/stage/mom-api/Jenkinsfile_API (6)
58-58: Avoid hard‑coded workspace path in docker build.Use WORKSPACE or “.” to make this portable across agents/jobs.
Apply this diff:
- sh "sudo docker build -f deployment/stage/mom-api/api.Dockerfile -t ${DOCKER_REGISTRY}/mom-api:${imageTag} /var/lib/jenkins/workspace/mom_api-pipeline" + sh "sudo docker build -f deployment/stage/mom-api/api.Dockerfile -t ${DOCKER_REGISTRY}/mom-api:${imageTag} ${WORKSPACE}"
56-57: Don’t echo potential secret values.If DOCKER_REGISTRY is a secret text credential, this logs it. Remove these lines.
Apply this diff:
- echo "Printing the saved docker registry from env:" - echo "${dockerRegistry}" + // Omit logging of dockerRegistry (may be sensitive)
21-29: Use BRANCH_NAME and fix ENV var typo.Use env.BRANCH_NAME and consistently set ENVIRONMENT (current code sets ENVIORNMENT on main).
Apply this diff:
- def branch = env.GIT_BRANCH + def branch = env.BRANCH_NAME ?: env.GIT_BRANCH - if (branch == "origin/main"){ - env.ENVIORNMENT = 'main' - } else if (branch == "origin/custom-agents-production-release") { - env.ENVIRONMENT = 'custom-agents-production-release' + if (branch == "main" || branch == "origin/main"){ + env.ENVIRONMENT = 'main' + } else if (branch == "custom-agents-production-release" || branch == "origin/custom-agents-production-release") { + env.ENVIRONMENT = 'custom-agents-production-release'
68-71: Remove noisy debug output.These were likely used during bring‑up; they add little value now.
Apply this diff:
- echo "printing the user here" - sh "whoami && pwd" + // debug removed
158-160: Make docker rmi consistent with earlier sudo usage.Build/push use sudo; rmi should too, otherwise cleanup may silently fail and leak disk.
Apply this diff:
- docker rmi ${DOCKER_REGISTRY}/mom-api:${imageTag} || true + sudo docker rmi ${DOCKER_REGISTRY}/mom-api:${imageTag} || true
44-45: Run gcloud configure-docker non‑interactively.Add --quiet to avoid prompts in CI.
Apply this diff:
- sudo gcloud auth configure-docker ${registryHost} + sudo gcloud auth configure-docker ${registryHost} --quiet
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
deployment/stage/mom-api/Jenkinsfile_API(1 hunks)
🔇 Additional comments (1)
deployment/stage/mom-api/Jenkinsfile_API (1)
126-126: Rollback now interpolates namespace correctly.Switching to double quotes ensures Groovy interpolates ${params.namespace}. Good fix.
Summary by CodeRabbit