Skip to content

CXX-3265 Update release instructions to use Amazon ECR instead of Artifactory #1425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 17, 2025

Conversation

eramongodb
Copy link
Contributor

Resolves CXX-3265. Applies changes similar to those in mongodb/mongo-c-driver#2058.

  • Release instructions are updated to authenticate with and use Amazon ECR instead of Artifactory.
  • Dockerfiles under extras/docker are updated to support setting a DEFAULT_SEARCH_REGISTRY environment variable (pass to make) which is forwarded as a Docker default_search_registry argument, similar to default_search_registry for the Earthfile in CDRIVER-5971 Use Amazon ECR to obtain OCI images in EVG mongo-c-driver#2058. (Note: Earthfile syntax slightly differs from Dockerfile syntax.)

The generate.py script and related files has been drive-by improved for better consistency (e.g. trailing newlines) and with inline script metadata to support uv run.

@eramongodb eramongodb requested a review from kevinAlbs July 16, 2025 17:11
@eramongodb eramongodb self-assigned this Jul 16, 2025
@eramongodb eramongodb requested a review from a team as a code owner July 16, 2025 17:11
@eramongodb eramongodb changed the title Update release instructions to use Amazon ECR instead of Artifactory CXX-3265 Update release instructions to use Amazon ECR instead of Artifactory Jul 16, 2025
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with minor comments.


RUN apk update && apk upgrade && apk add --no-cache openssl3 libstdc++ libc6-compat

COPY --from=builder /opt/mongocxx /usr/local

RUN true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove stray RUN true?

"alpine3.19": "",
"bookworm": "\nRUN ldconfig\n",
"noble": "\nRUN ldconfig\n",
"alpine3.19": "\nRUN true",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"alpine3.19": "\nRUN true",
"alpine3.19": "",

Remove unnecessary RUN true? I expect these changes were for newline consistency. Since a newline is later appended, I expect this can be empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annoyingly, if the expansion is empty (prior value of ""), the Dockerfile for alpine3.19 ends up with a trailing newline:

  COPY --from=builder /opt/mongocxx /usr/local
+ (empty line)
  (EOF newline)

I could not find a way to coerce Jinja2 to avoid expanding {{ post_install_commands }} without an extra newline given a value of "" or None. For typical Jinja2 expansions, one would use % (e.g. {% var %}), but that doesn't seem to apply to {{ var }} expansions... 😕

Comment on lines 6 to 7
: "${ARTIFACTORY_USER:?}"
: "${ARTIFACTORY_PASSWORD:?}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer used:

Suggested change
: "${ARTIFACTORY_USER:?}"
: "${ARTIFACTORY_PASSWORD:?}"

Comment on lines 74 to 75
'ARTIFACTORY_PASSWORD',
'ARTIFACTORY_USER',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer used:

Suggested change
'ARTIFACTORY_PASSWORD',
'ARTIFACTORY_USER',

@eramongodb eramongodb merged commit 83063e5 into mongodb:master Jul 17, 2025
1 check was pending
@eramongodb eramongodb deleted the cxx-3265 branch July 17, 2025 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants