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 8218b4e
Show file tree
Hide file tree
Showing 22 changed files with 92 additions and 64 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
20 changes: 0 additions & 20 deletions docs/topics/development/static-files.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,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/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
17 changes: 17 additions & 0 deletions src/olympia/lib/settings_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,23 @@ def path(*folders):

DEBUG = env('DEBUG', default=False)

# Do NOT provide a default value, this should be explicitly
# set during the docker image build. If it is not set,
# we want to raise an error.
DOCKER_TARGET = env('DOCKER_TARGET')

# "production" is a named docker stage corresponding to the production image.
# when we build the production image, the stage to use is determined
# via the "DOCKER_TARGET" variable which is also passed into the image.
# So if the value is anything other than "production" we are in development mode.
DEV_MODE = DOCKER_TARGET != 'production'

# Used to determine if django should serve static files.
# For local deployments we want nginx to proxy static file requests to the
# uwsgi server and not try to serve them locally.
# In production, nginx serves these files from a CDN.
SERVE_STATIC_FILES = False

DEBUG_TOOLBAR_CONFIG = {
# Deactivate django debug toolbar by default.
'SHOW_TOOLBAR_CALLBACK': lambda request: DEBUG,
Expand Down
11 changes: 9 additions & 2 deletions src/olympia/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
),
]

if settings.DEBUG:
if settings.SERVE_STATIC_FILES:
from django.contrib.staticfiles.views import serve as static_serve

def serve_static_files(request, path, **kwargs):
Expand All @@ -128,7 +128,14 @@ def serve_static_files(request, path, **kwargs):
# fallback for static files that are not available directly over nginx.
# Mostly vendor files from python or npm dependencies that are not available
# in the static files directory.
re_path(r'^static/(?P<path>.*)$', serve_static_files),
re_path(
r'^static/(?P<path>.*)$',
serve_static,
{
'document_root': settings.STATIC_ROOT,
'show_indexes': True
},
),
]
)

Expand Down

0 comments on commit 8218b4e

Please sign in to comment.