Harden build.sh script#7086
Conversation
|
|
xlamorlette-datadog
left a comment
There was a problem hiding this comment.
I confirm it's working fine.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c6f82dcd0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 2d990ab | Docs | Datadog PR Page | Give us feedback! |
There was a problem hiding this comment.
Pull request overview
This PR hardens utils/build/build.sh by deferring weblog validation and default-variant resolution until the script is actually building the weblog image, preventing failures when BUILD_IMAGES does not include weblog (e.g., ./build.sh -i runner with TEST_LIBRARY=cpp).
Changes:
- Move library existence checks, default weblog variant resolution, and variant Dockerfile validation into the
weblogimage build branch. - Move remote Docker cache configuration to the
weblogbuild path so it’s not computed whenweblogisn’t being built. - Remove the previous unconditional/global weblog validation block at the end of the script.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [[ ! -d "${SCRIPT_DIR}/docker/${TEST_LIBRARY}" ]]; then | ||
| echo "Library ${TEST_LIBRARY} not found" | ||
| echo "Available libraries: $(echo $(list-libraries))" | ||
| exit 1 |
Motivation
When
./build.sh -i runnerwas executed with an env varTEST_LIBRARY=cpp, it was trying to get the default weblog of cpp (which does not exists) and failed, even if the purpose of the call was not to build the weblog.It was sneaky, as
./build.sh -i runnercan be called by./run.shif thevenvis updated. Forcppfolks, it was breaking run.shIssue raised in #7082
Changes
Move weblog and variant resolution inside the block that actually build the weblog
Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present