ceph-dev-pipeline: use function arguments instead of environment variables

[shraddhaag]

This commit makes substantial changes in the ceph-dev-pipeline to move
away from using environment variables. Instead, we use explicitly
declared function arguments in all the helper functions in the pipeline.
This makes variable mutation to be explicit, which is helpful for code
readability and maintainability.

This PR depends on #2621. The main commit for this PR is be41014.

[perezjosibm]

LGTM, I asked the LLM/copilot for risk assesment, I included the result as comment, tbh I am not familiar with groovy so please take this with a large pinch of salt.

Top risks are behavioral regressions from changed state flow, null/typing edge cases, and secret-handling/logging side effects.

1) State propagation changed from implicit global env to explicit return values

This PR is good architecturally, but it changes execution semantics:

• doSourceDistribution() now returns setupBuildId/setupBuildUrl and caller writes to env.
• doCopyArtifactsStage() now returns sha1/version/extraEnv; caller applies extraEnv into env.

Risk: Any missed caller assignment (now or in future edits) can silently break downstream stages that still read env.*.

---

2) Potential SHA1 logging mismatch bug

In doCopyArtifactsStage:

• sha1_trimmed is computed from sha1_input
• sha1 may be replaced from artifact when input is empty
• but log prints println "SHA1=${sha1_trimmed}"

Risk: If no SHA1 input was provided, log may print null (or stale value) even though effective SHA1 is set from artifact. This can mislead debugging and auditability.

---

3) test mode + SHA1 substring for tag creation

In doUploadPackagesStage:

• sha1 = args.test?.toBoolean() ? 'test' : args.sha1
• later sha1[0..6] is used for ceph_builder_tag.

Risk: With TEST=true, slicing 'test'[0..6] can throw index errors in Groovy (StringIndexOutOfBoundsException) depending on runtime behavior. This is a likely functional break in test-release flows.

---

4) Type/format assumptions on args now matter more

Many helpers assume non-empty strings:

• args.sha1[0..6]
• args.version.trim()
• URL construction with args.setupJob, args.jenkinsUrl
• path usage with args.workspace, args.home

Risk: Previously env values were often guaranteed by Jenkins declarations; now missing arg wiring at call sites can cause runtime exceptions earlier and in less obvious places.

---

5) withEnv of credential-derived vars may widen exposure surface

doBuilderContainerStage and doContainerStage push credentials via withEnv([...]) and shell interpolation.

Risk: If any command later echoes env or runs with shell tracing (set -x), secrets may appear in logs. Jenkins masking usually helps, but this refactor increases explicit secret variable plumbing and therefore review burden.

---

6) extraEnv override precedence could alter UI links/metadata unexpectedly

doCopyArtifactsStage now lets dist/other_envvars override BRANCH, CEPH_REPO, SHA1 for UI rendering/description.

Risk: Build description and shaman/github links may no longer reflect input parameters, causing confusion or mis-attribution when artifact metadata differs from requested values.

---

7) Behavior coupling to dependent PR (#2621)

PR body says this depends on #2621.

Risk: If merged/validated out of order, assumptions about available params/env setup or helper behavior may be invalid and produce false negatives/positives in CI.

---

Suggested focused checks before merge

1. Run matrix with:
   • explicit SHA1
   • empty SHA1 (artifact-provided)
   • TEST=true
   • PULP_UPLOAD=true/false
2. Add guards:
   • validate required args at top of each helper
   • safe short SHA: sha1.take(7) instead of [0..6]
3. Fix SHA1 log to print effective value.
4. Confirm Jenkins masking for all credential vars introduced in withEnv.
5. Add one pipeline unit/integration smoke test around returned-map propagation (out.* -> env.*).

[djgalloway]

I don't know how worthwhile this PR is anymore now that #2621 didn't have the intended effect of avoiding the 64K limitation.

[zmc]

I don't know how worthwhile this PR is anymore now that #2621 didn't have the intended effect of avoiding the 64K limitation.

agreed; do we want to back that change out?

[djgalloway]

I don't know how worthwhile this PR is anymore now that #2621 didn't have the intended effect of avoiding the 64K limitation.

agreed; do we want to back that change out?

I don't know if it's worthwhile. After spending some time with it since then, it is a bit easier to read with all the stages split out as functions and not having to worry about the indentation while updating the function code.

[shraddhaag]

Hey folks, if we are going to keep the helper functions, do we want to adds this PR in as well? If so, I can rebase and get it ready for testing! @djgalloway @zmc