Skip to content
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

Make DOCKER_TARGET a buid time argument only. #23076

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Feb 13, 2025

Fixes: mozilla/addons#15361

Description

Define TARGET setting reading the docker target from build info.

Context

It is very subtle, but extremely problematic to rely on an environmnet variable for DOCKER_TARGET. This value is mostly relevant during build time, it literally determines which stage of the docker image to build to. The actual state cannot be changed, but environment variables can be freely changed whenever you want.

Since we are relying on this value to know what kind of checks to run or what features might be enabled we must use a value that is hard coded during the build. that is what the build-info.json is meant for.

Testing

Make up with a given target "production" or "development"

make up DOCKER_TARGET=<target>

Shell into the container

make shell

Print the environmnet

Expect DOCKER_TARGET is not defined

printenv

Django shell it up

make djshell

Get that TARGET setting

settings.TARGET

Expect to match <target>

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@KevinMind KevinMind requested review from a team, diox and eviljeff and removed request for a team and diox February 13, 2025 19:12
@KevinMind KevinMind merged commit c862812 into master Feb 14, 2025
41 checks passed
@KevinMind KevinMind deleted the kevinmind/addons/153310-docker-target branch February 14, 2025 10:29
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.

[Task]: Define TARGET based on build info
2 participants