Skip to content

Commit

Permalink
Decouple DEBUG from DEV_MODE to enable running dev/prod mode independ…
Browse files Browse the repository at this point in the history
…ent of debug.
  • Loading branch information
KevinMind committed Oct 16, 2024
1 parent daee988 commit a7565c3
Show file tree
Hide file tree
Showing 25 changed files with 134 additions and 80 deletions.
5 changes: 5 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ rm -rf /deps/build/*
${PIP_COMMAND} install --progress-bar=off --no-deps --exists-action=w -r requirements/pip.txt
EOF

# Expose the DOCKER_TARGET variable to all subsequent stages
# This value is used to determine if we are building for production or development
ARG DOCKER_TARGET
ENV DOCKER_TARGET=${DOCKER_TARGET}

# Define production dependencies as a single layer
# let's the rest of the stages inherit prod dependencies
# and makes copying the /deps dir to the final layer easy.
Expand Down
7 changes: 6 additions & 1 deletion Makefile-docker
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ check_debian_packages: ## check the existence of multiple debian packages

.PHONY: check_pip_packages
check_pip_packages: ## check the existence of multiple python packages
./scripts/check_pip_packages.sh prod.txt dev.txt
@ ./scripts/check_pip_packages.sh prod.txt
# "production" corresponds to the "propduction" DOCKER_TARGET defined in the Dockerfile
# When the target is "production" it means we cannot expect dev.txt dependencies to be installed.
@if [ "$(DOCKER_TARGET)" != "production" ]; then \
./scripts/check_pip_packages.sh dev.txt; \
fi

.PHONY: check_files
check_files: ## check the existence of multiple files
Expand Down
1 change: 1 addition & 0 deletions docker-bake.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ target "web" {
DOCKER_COMMIT = "${DOCKER_COMMIT}"
DOCKER_VERSION = "${DOCKER_VERSION}"
DOCKER_BUILD = "${DOCKER_BUILD}"
DOCKER_TARGET = "${DOCKER_TARGET}"
}
pull = true

Expand Down
8 changes: 0 additions & 8 deletions docker-compose.ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,12 @@ services:
worker:
environment:
- HOST_UID=9500
- DEBUG=
volumes:
- /data/olympia

web:
extends:
service: worker
volumes:
- data_site_static:/data/olympia/site-static

nginx:
volumes:
- data_site_static:/srv/site-static

volumes:
data_olympia:
data_site_static:
7 changes: 4 additions & 3 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ x-olympia: &olympia
entrypoint: ["/data/olympia/docker/entrypoint.sh"]

services:
worker: &worker
worker:
<<: *olympia
command: [
"DJANGO_SETTINGS_MODULE=settings",
Expand Down Expand Up @@ -84,7 +84,8 @@ services:
- autograph

web:
<<: *worker
extends:
service: worker
healthcheck:
test: ["CMD-SHELL", "curl --fail --show-error --include --location http://127.0.0.1:8002/__version__"]
retries: 3
Expand All @@ -98,7 +99,7 @@ services:
image: nginx
volumes:
- ./docker/nginx/addons.conf:/etc/nginx/conf.d/addons.conf
- ./static:/srv/site-static
- ./static:/srv/static
- storage:/srv/user-media
ports:
- "80:80"
Expand Down
2 changes: 1 addition & 1 deletion docker/nginx/addons.conf
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ server {
}

location /static/ {
alias /srv/site-static/;
alias /srv/static/;

# Fallback to the uwsgi server if the file is not found in the static files directory.
# This will happen for vendor files from pytnon or npm dependencies that won't be available
Expand Down
48 changes: 27 additions & 21 deletions docs/topics/development/static-files.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Static Files in addons-server

This document explains how static files are served in the addons-server project during local development.
This document explains how static files are served in the addons-server project during local development. In production,
static files are served directly from a CDN.

## Overview

Expand All @@ -10,6 +11,7 @@ These files come from multiple sources:
1. The `./static` folder in the project
2. Python dependencies
3. npm dependencies
4. Compressed/minified files built by `update_assets`

## Static File Servers

Expand All @@ -29,6 +31,30 @@ The `web` container exposes the `site-static` directory to nginx that includes t
## Static File Sources

The rendering path for static files is as follows:

1. Nginx tries to serve the file if it is available in the `./static` directory.
2. If the file is not found, the request is forwarded to django and served by the static file server.

The static file serve uses our defined `STATICFILES_STORAGE` setting to determine the URL for static files as well as their underlying source file.
During development, we use the `StaticFilesStorage` class which does not map the hashed file names back to their original file names.
Otherwise we use the same `ManifestStaticFilesStorage` class that is used in production, expecting to serve the files from the `STATIC_ROOT` directory.

This allows us to skip `update_assets` in dev mode, speeding up the development process, while still enabling production-like behavior
when configured to do so. The long term goal is to run CI in production mode always to ensure all tests verify against the production
static file build.

To better visualize the impact of the various settings, here is a reference:

Given a static file 'js/devhub/my-file.js':

In `DEV_MODE` the url will look like `/static/js/devhub/my-file.js` no matter what.
However, in production, if `DEBUG` is `False`, the url will append the content hash like this,
`/static/js/devhub/my-file.1234567890.js`. Finally, if `DEBUG` is true, this file will be minified and concatenated with other files and probably look something like this `/static/js/devhub-all.min.1234567890.js`.

The true `production` mode is then when `DEBUG` is `False` and `DEV_MODE` is `False`. But it makes sense
to make these individually toggleable so you can better "debug" js files from a production image.

### Project Static Files

Static files specific to the addons-server project are stored in the `./static` directory. These include CSS, JavaScript, images, and other assets used by the application.
Expand Down Expand Up @@ -59,23 +85,3 @@ During development they are served by the django development server.

We have a (complex) set of npm static assets that are built by the `compress_assets` management command.
During development, these assets are served directly from the node_modules directory using a custom static finder.

## DEBUG Property and Static File Serving

The behavior of static file serving can be controlled using the `DEBUG` environment variable or via setting it directly in
the `local_settings.py` file. Be careful directly setting this value, if DEBUG is set to false, and you don't have sufficient
routing setup to serve files fron nginx only, it can cause failure to serve some static files.

It is best to use the compose file to control DEBUG.a

This is set in the environment, and in CI environments, it's controlled by the `docker-compose.ci.yml` file.

The `DEBUG` property is what is used by django to determine if it should serve static files or not. In development,
you can manually override this in the make up command, but in general, you should rely on the `docker-compose.ci.yml` file
to set the correct value as this will also set appropriate file mounts.

```bash
make up COMPOSE_FILE=docker-compose.yml:docker-compose.ci.yml
```

This will run addons-server in production mode, serving files from the `site-static` directory.
16 changes: 16 additions & 0 deletions docs/topics/development/troubleshooting_and_debugging.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,22 @@

Effective troubleshooting and debugging practices are essential for maintaining and improving the **addons-server** project. This section covers common issues, their solutions, and tools for effective debugging.

## DEV_MODE vs DEBUG

In our project, `DEV_MODE` and `DEBUG` serve distinct but complementary purposes.
`DEV_MODE` is directly tied to the `DOCKER_TARGET` environment variable and is used to enable or disable behaviors
based on whether we are running a production image or not.

For instance, production images always disables certain features like using fake fxa authentication. Additionally,
certain dependencies are only installed in [dev.txt](../../../requirements/dev.txt) and so must be disabled in production.

Conversely, `DEBUG` controls the activation of debugging tools and utilities, such as the debug_toolbar,
which are useful for troubleshooting. Unlike DEV_MODE, DEBUG is independent
and can be toggled in both development and production environments as needed.

This separation ensures that essential behaviors are managed according to the deployment target (DEV_MODE),
while allowing flexibility to enable or disable debugging features (DEBUG) in production or development images.

## Common Issues and Solutions

1. **Containers Not Starting**:
Expand Down
17 changes: 10 additions & 7 deletions settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@

INTERNAL_ROUTES_ALLOWED = True

# Always
SERVE_STATIC_FILES = True

# These apps are great during development.
INSTALLED_APPS += (
'olympia.landfill',
'dbbackup',
)
INSTALLED_APPS += ('olympia.landfill',)

DBBACKUP_STORAGE = 'django.core.files.storage.FileSystemStorage'

Expand Down Expand Up @@ -53,8 +53,11 @@ def insert_debug_toolbar_middleware(middlewares):
return tuple(ret_middleware)


if DEBUG:
INSTALLED_APPS += ('debug_toolbar',)
if DEV_MODE:
INSTALLED_APPS += (
'debug_toolbar',
'dbbackup',
)
MIDDLEWARE = insert_debug_toolbar_middleware(MIDDLEWARE)

DEBUG_TOOLBAR_CONFIG = {
Expand Down Expand Up @@ -107,7 +110,7 @@ def insert_debug_toolbar_middleware(middlewares):
FXA_OAUTH_HOST = 'https://oauth.stage.mozaws.net/v1'
FXA_PROFILE_HOST = 'https://profile.stage.mozaws.net/v1'

# When USE_FAKE_FXA_AUTH and settings.DEBUG are both True, we serve a fake
# When USE_FAKE_FXA_AUTH and settings.DEV_MODE are both True, we serve a fake
# authentication page, bypassing FxA. To disable this behavior, set
# USE_FAKE_FXA = False in your local settings.
# You will also need to specify `client_id` and `client_secret` in your
Expand Down
2 changes: 2 additions & 0 deletions settings_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
IN_TEST_SUITE = True

DEBUG = False
# We should default to production mode unless otherwiser specified
DEV_MODE = False

# We won't actually send an email.
SEND_REAL_EMAIL = True
Expand Down
2 changes: 1 addition & 1 deletion src/olympia/accounts/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ def test_redirect_for_login_with_2fa_enforced_and_config():
assert request.session['enforce_2fa'] is True


@override_settings(DEBUG=True, USE_FAKE_FXA_AUTH=True)
@override_settings(DEV_MODE=True, USE_FAKE_FXA_AUTH=True)
def test_fxa_login_url_when_faking_fxa_auth():
path = '/en-US/addons/abp/?source=ddg'
request = RequestFactory().get(path)
Expand Down
2 changes: 1 addition & 1 deletion src/olympia/accounts/tests/test_verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def test_with_id_token(self):
self.get_profile.assert_called_with('cafe')


@override_settings(USE_FAKE_FXA_AUTH=False, DEBUG=True, VERIFY_FXA_ACCESS_TOKEN=True)
@override_settings(USE_FAKE_FXA_AUTH=False, DEV_MODE=True, VERIFY_FXA_ACCESS_TOKEN=True)
class TestCheckAndUpdateFxaAccessToken(TestCase):
def setUp(self):
super().setUp()
Expand Down
6 changes: 3 additions & 3 deletions src/olympia/accounts/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def has_cors_headers(response, origin='https://addons-frontend'):


class TestLoginStartView(TestCase):
@override_settings(DEBUG=True, USE_FAKE_FXA_AUTH=True)
@override_settings(DEV_MODE=True, USE_FAKE_FXA_AUTH=True)
def test_redirect_url_fake_fxa_auth(self):
response = self.client.get(reverse_ns('accounts.login_start'))
assert response.status_code == 302
Expand Down Expand Up @@ -700,7 +700,7 @@ def test_waffle_flag_off_enforced_2fa_should_have_no_effect(self):
self.request.session['enforce_2fa'] = True
self._test_should_continue_without_redirect_for_two_factor_auth()

@override_settings(DEBUG=True, USE_FAKE_FXA_AUTH=True)
@override_settings(DEV_MODE=True, USE_FAKE_FXA_AUTH=True)
def test_fake_fxa_auth(self):
self.user = user_factory()
self.find_user.return_value = self.user
Expand All @@ -721,7 +721,7 @@ def test_fake_fxa_auth(self):
assert kwargs['next_path'] == '/a/path/?'
assert self.fxa_identify.call_count == 0

@override_settings(DEBUG=True, USE_FAKE_FXA_AUTH=True)
@override_settings(DEV_MODE=True, USE_FAKE_FXA_AUTH=True)
def test_fake_fxa_auth_with_2fa(self):
self.user = user_factory()
self.find_user.return_value = self.user
Expand Down
8 changes: 4 additions & 4 deletions src/olympia/amo/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ def test_allow_mozilla_collections(self):

@pytest.mark.django_db
def test_fake_fxa_authorization_correct_values_passed():
with override_settings(DEBUG=True): # USE_FAKE_FXA_AUTH is already True
with override_settings(DEV_MODE=True): # USE_FAKE_FXA_AUTH is already True
url = reverse('fake-fxa-authorization')
response = test.Client().get(url, {'state': 'foobar'})
assert response.status_code == 200
Expand All @@ -528,15 +528,15 @@ def test_fake_fxa_authorization_correct_values_passed():
@pytest.mark.django_db
def test_fake_fxa_authorization_deactivated():
url = reverse('fake-fxa-authorization')
with override_settings(DEBUG=False, USE_FAKE_FXA_AUTH=False):
with override_settings(DEV_MODE=False, USE_FAKE_FXA_AUTH=False):
response = test.Client().get(url)
assert response.status_code == 404

with override_settings(DEBUG=False, USE_FAKE_FXA_AUTH=True):
with override_settings(DEV_MODE=False, USE_FAKE_FXA_AUTH=True):
response = test.Client().get(url)
assert response.status_code == 404

with override_settings(DEBUG=True, USE_FAKE_FXA_AUTH=False):
with override_settings(DEV_MODE=True, USE_FAKE_FXA_AUTH=False):
response = test.Client().get(url)
assert response.status_code == 404

Expand Down
2 changes: 1 addition & 1 deletion src/olympia/amo/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@ def extract_colors_from_image(path):
def use_fake_fxa():
"""Return whether or not to use a fake FxA server for authentication.
Should always return False in production"""
return settings.DEBUG and settings.USE_FAKE_FXA_AUTH
return settings.DEV_MODE or settings.USE_FAKE_FXA_AUTH


class AMOJSONEncoder(JSONEncoder):
Expand Down
2 changes: 1 addition & 1 deletion src/olympia/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def get_versioned_api_routes(version, url_patterns):
routes = url_patterns

# For now, this feature is only enabled in dev mode
if settings.DEBUG:
if settings.DEV_MODE:
routes.extend(
[
re_path(
Expand Down
3 changes: 3 additions & 0 deletions src/olympia/core/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ def static_check(app_configs, **kwargs):
errors = []
output = StringIO()

if settings.DEV_MODE:
return []

try:
call_command('compress_assets', dry_run=True, stdout=output)
file_paths = output.getvalue().strip().split('\n')
Expand Down
4 changes: 2 additions & 2 deletions src/olympia/landfill/management/commands/fetch_prod_addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ def add_arguments(self, parser):
)

def handle(self, *args, **options):
if not settings.DEBUG:
if not settings.DEV_MODE:
raise CommandError(
'As a safety precaution this command only works if DEBUG=True.'
'As a safety precaution this command only works in DEV_MODE.'
)
self.fetch_addon_data(options)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ def add_arguments(self, parser):
)

def handle(self, *args, **options):
if not settings.DEBUG:
if not settings.DEV_MODE:
raise CommandError(
'As a safety precaution this command only works if DEBUG=True.'
'As a safety precaution this command only works in DEV_MODE.'
)
self.options = options
self.fetch_versions_data()
Expand Down
6 changes: 2 additions & 4 deletions src/olympia/landfill/management/commands/generate_addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,8 @@ def add_arguments(self, parser):
)

def handle(self, *args, **kwargs):
if not settings.DEBUG:
raise CommandError(
'You can only run this command with your DEBUG setting set to True.'
)
if not settings.DEV_MODE:
raise CommandError('You can only run this command in DEV_MODE.')

num = int(kwargs.get('num'))
email = kwargs.get('email')
Expand Down
6 changes: 2 additions & 4 deletions src/olympia/landfill/management/commands/generate_themes.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ def add_arguments(self, parser):
)

def handle(self, *args, **kwargs):
if not settings.DEBUG:
raise CommandError(
'You can only run this command with your DEBUG setting set to True.'
)
if not settings.DEV_MODE:
raise CommandError('You can only run this command in DEV_MODE.')
num = int(kwargs.get('num'))
email = kwargs.get('email')

Expand Down
Loading

0 comments on commit a7565c3

Please sign in to comment.