Files
devops-lib-gcp/review.md
T
2026-08-26 02:02:24 +05:30

10 KiB

How to Review in This Service

Domain invariants

  • Whitelist gates in constructParam.groovy:getWhitelistedRepos clone Meesho/whitelists fresh on every call (no caching). This is deliberate — each clone captures latest state so a DevOps whitelist change takes effect on the next build without a library release. Reject any caching/memoization that doesn't replace the freshness story.
  • The literal "ringmaster-bot" userId in deployRingmaster.groovy:run is the only signal that distinguishes Ringmaster callbacks from Turbo-Turtle callbacks. Reject any rename of the string without explicit Ringmaster-team coordination.
  • buildObjHelper.groovy:run falls through to defaultBuild silently when no toolchain case matches — a mis-spelled toolchain (golang vs go) produces a no-op build that "succeeds" with no artifact. Flag any new builder added without a match arm, and any toolchain string the dispatcher doesn't recognise.
  • JVM -Xmx/-Xms are auto-calculated in deployArgoCD.groovy:update_helm_repo from memory_request (xmx = memory_request * 0.75, xms = xmx * 0.5). Reject hardcoded -Xmx in JAVA_OPTS unless jvm_memory_override: true is also set in the service's deployment.yaml.
  • Services with priority_v2: sp0 or up0 MUST have canary configured — deployArgoCD.groovy:run blocks non-canary prd deploys for these priorities and there is no bypass. Reject any diff that adds one.
  • environment_map / branch_param_map in constructParam.groovy and deployArgoCD.groovy may only contain permanent branches (master, main, develop, gcp-main). Flag any feature/... entry — these propagate to every consumer (P1_FEATURE_BRANCH_IN_ENVIRONMENT_MAP, PR #707).
  • constructTemplate.groovy:_construct() is @NonCPS because it uses Java regex and string interpolation that aren't CPS-serialisable. Reject removal of the annotation or moves of its logic into a CPS context (inlining into stage bodies or parallel closures).
  • The four-step ArgoCD deploy order in deployArgoCD.groovy (update_argo_repo → refresh_app_of_apps → update_helm_repo → refresh_and_sync) is load-bearing — first-deploy services rely on refresh_app_of_apps creating the Application object. Steps 2 and 4 are not interchangeable.

Critical workflows

  • vars/buildPipeline.groovy:call is the primary entry point invoked by every consuming Jenkinsfile. The chain is constructParam.run → checkOut.run → buildObjHelper.run(build_tool) → notify.run, wrapped in try/catch/finally. The finally block is the only thing that guarantees Slack notification on failure — trace any change to ordering, exception handling, or the surrounding try/catch/finally.
  • vars/gkeCICD.groovy, vars/eksCICD.groovy, vars/cloudFunctionCICD.groovy, vars/gcpMigration.groovy, vars/onlyPushtoJfrog.groovy are sibling entry points — verify any change to a shared helper (constructParam, notify, deployArgoCD, buildObjHelper) is consistent across all of them.
  • deployArgoCD.groovy:run is the prd-deploy critical path: priority check → 4-step deploy. A break here breaks every service's prd rollout — trace canary enforcement and the deploy-step ordering.
  • deployRingmaster.groovy:run is the prd callback path: every prd deploy depends on the curl-to-Turbo-Turtle / Ringmaster handoff. Payload is written to a temp file and sent via curl -d @<file> to avoid bash escaping — never inline new fields into the curl command.
  • constructParam.groovy:getWhitelistedRepos is consulted for five gates (skip-sonar, app-config-disabled, multizone-enabled, allowedNonDevelopPrDeployment, ValidateCacConfig). Any change to gate semantics must be traced end-to-end through all five callers.

Known failure patterns

  • Debug tunnel hostnames (*.lhr.life, *.ngrok.io, *.tunnel.*) merged into deployRingmaster.groovy after local testing (PRs #596, #664) — flag any tunnel literal in any .groovy or YAML (P0_DEBUG_URL_IN_PRODUCTION_CONFIG).
  • SSH private-key contents printed to Jenkins console via cat ./id_github_jenkins in addSSHKey.groovy (PR #634). Jenkins console is readable by anyone with job-read access — flag any cat/echo/print/ls -al of credential paths (P0_SENSITIVE_DEBUG_PRINT_IN_PIPELINE).
  • Bare IPs (172.x.x.x, 10.x.x.x) shipped as build-callback endpoints in notify.groovy (PR #681). IPs break silently when infra is rebalanced — flag any IP literal, replace with DNS (P0_HARDCODED_IP_IN_PIPELINE).
  • Dockerfiles mutated at build time via sh "echo '...' >> Dockerfile-${artifactId}" (PR #707). Runtime mutation makes the actual image instructions invisible in code review — use ARG/multi-stage or a dedicated template (P1_DYNAMIC_DOCKERFILE_MUTATION_VIA_SHELL).
  • Test failures swallowed via if (testExitCode != 0) { echo 'pipeline continues' } patterns (PR #643). Broken code packaged and deployed while CI appears to pass — flag any test-exit-code suppression (P1_TEST_FAILURE_MUST_FAIL_PIPELINE).
  • Shipped typos and copy-paste bugs that Groovy/Jenkins doesn't catch at compile time: chekoutSubmodule in checkOut.groovy:22, catch (Exceptione) in helmGenerator.groovy:102, duplicate buildRegistry map key in buildPython.groovy:76-79, hard-coded branch_name = 'repo' in buildGradle.groovy:252. Reviewer is the only gate — read identifier names and map keys carefully.
  • New language support created by copy-pasting an existing language's template without auditing every field (PR #650 left GOMAXPROCS in rust-values.yaml; PR #707 duplicated validate_configs_v2.py into validate_configs_node.py). Flag any new template file that mirrors an existing one — every env var, ARG, and shared script must be audited.

Performance considerations

  • getWhitelistedRepos does up to 5 fresh git clone of Meesho/whitelists per build under GitHub rate-limiting. Do not add a sixth gate without coordinating with infra; do not add caching without a replacement freshness story (see Domain invariants).
  • Per-build downloads of sonar-scanner, Go toolchain, pnpm, librdkafka add external-network dependency to the critical path and re-fetch on every build (PRs #306, #643). Bake into the builder base image (P1_TOOLS_IN_BASE_IMAGE_NOT_BUILD_STEP, P1_EXTERNAL_BINARY_DOWNLOAD_IN_BUILD).
  • Dockerfile layer caching: dependency manifests (Cargo.toml/Cargo.lock, package.json/package-lock.yaml, go.mod/go.sum, pom.xml) must be COPY-ed and dependencies fetched before COPY . . in every Dockerfile under resources/com/meesho/ (P1_DOCKER_LAYER_CACHING_DEPENDENCY_FIRST, PR #650).
  • Non-prd node pools are BU-shared (nodePoolSelection.groovy:run{BU}-shared). A memory leak or CPU spike on one staging/dev service degrades every other service in the same BU — flag any material increase in requests/limits on a non-prd values.yaml without BU-owner sign-off.
  • Any externally downloaded binary must have hash verification (sha256sum -c <expected>) or be replaced by an Artifactory-hosted artifact with controlled provenance (P1_DOWNLOADED_BINARY_HASH_VERIFICATION, PR #643).
  • Registry base URLs (asia-southeast1-docker.pkg.dev/meesho-central-dev-0622/toolchain) must be referenced via a shared constant in src/com/meesho/utilities/, not repeated inline across stage files (P1_SHARED_CONSTANTS_FOR_REGISTRY_PATHS, PR #681).

Integration boundaries

  • ArgoCD (deployArgoCD.groovy): 4-step deploy order is load-bearing; argocd login --password ${ARGO_PASSWORD} leaks the password via process list — prefer stdin or env-var injection. Flag any new --password <literal> style invocation.
  • Turbo-Turtle / Ringmaster callbacks (deployRingmaster.groovy): canonical internal DNS endpoint (e.g. turbo-turtle.meeshogcp.in); payload sent via curl -d @<tempfile>; discriminated by getCause(UserIdCause).getUserId() == "ringmaster-bot"; temp file deleted in a finally block.
  • GitHub SCM: env.CHANGE_ID from the GitHub Branch Source plugin is the canonical PR-build detector; never use env.BRANCH_NAME =~ /PR-/ — it breaks on non-GitHub SCMs and on re-triggered builds. Meesho/whitelists is cloned freshly per gate.
  • JFrog / S3 artifact targets: push_to_jfrog / push_to_s3 are the boolean params that allow non-master branches to push — verify any new pusher honours both flags.
  • Sonar: skip_sonar whitelist via skip-sonar-whitelist.yaml; uses the per-call Meesho/whitelists clone. Never overload skip_sonar (or hot_fix) for unrelated semantics — introduce a new, clearly named flag instead (P1_SEMANTIC_FLAG_REUSE, PRs #495, #578).
  • Slack #ci-cd-status (default; override via notify_channel): notification fired from notify.run inside the top-level finally block of buildPipeline.groovy. Verify the maintainer Slack username is honoured on failure paths.
  • GCP Artifact Registry asia-southeast1-docker.pkg.dev/meesho-central-dev-0622/toolchain: canonical toolchain registry. A project or region migration requires updating one constant — flag any inline duplicate of the URL.