Expose environment variables from database into build commands (#4894)

* Expose environment variables from database into build commands

Environment variables can be added from the Admin and they will be
used when running build commands. All the variables for that
particular project will be expose to all the commands.

* Rename migration file

* Use project detail endpoint with admin serializer

Add the `environment_variables` field to this serializer that will be
returned only when the user is admin.

* Better work around environment_variables on Project and APIProject

* Test case for UpdateDocsTaskStep.get_env_vars

* Lint ;)

readthedocs/doc_builder/environments.py

@@ -440,7 +440,7 @@ class BuildEnvironment(BaseEnvironment):
         ProjectBuildsSkippedError,
         YAMLParseError,
         BuildTimeoutError,
-        MkDocsYAMLParseError
+        MkDocsYAMLParseError,
     )
 
     def __init__(self, project=None, version=None, build=None, config=None,
@@ -466,7 +466,7 @@ def __exit__(self, exc_type, exc_value, tb):
                 project=self.project.slug,
                 version=self.version.slug,
                 msg='Build finished',
-            )
+            ),
         )
         return ret
 

readthedocs/projects/admin.py

@@ -1,21 +1,35 @@
+# -*- coding: utf-8 -*-
 """Django administration interface for `projects.models`"""
 
-from __future__ import absolute_import
-from django.contrib import admin
-from django.contrib import messages
+from __future__ import (
+    absolute_import,
+    division,
+    print_function,
+    unicode_literals,
+)
+
+from django.contrib import admin, messages
 from django.contrib.admin.actions import delete_selected
 from django.utils.translation import ugettext_lazy as _
 from guardian.admin import GuardedModelAdmin
 
+from readthedocs.builds.models import Version
 from readthedocs.core.models import UserProfile
 from readthedocs.core.utils import broadcast
-from readthedocs.builds.models import Version
-from readthedocs.redirects.models import Redirect
 from readthedocs.notifications.views import SendNotificationView
+from readthedocs.redirects.models import Redirect
 
 from .forms import FeatureForm
-from .models import (Project, ImportedFile, Feature,
-                     ProjectRelationship, EmailHook, WebHook, Domain)
+from .models import (
+    Domain,
+    EmailHook,
+    EnvironmentVariable,
+    Feature,
+    ImportedFile,
+    Project,
+    ProjectRelationship,
+    WebHook,
+)
 from .notifications import ResourceUsageNotification
 from .tasks import remove_dir
 
@@ -202,7 +216,14 @@ def project_count(self, feature):
         return feature.projects.count()
 
 
+class EnvironmentVariableAdmin(admin.ModelAdmin):
+    model = EnvironmentVariable
+    list_display = ('name', 'value', 'project', 'created')
+    search_fields = ('name', 'project__slug')
+
+
 admin.site.register(Project, ProjectAdmin)
+admin.site.register(EnvironmentVariable, EnvironmentVariableAdmin)
 admin.site.register(ImportedFile, ImportedFileAdmin)
 admin.site.register(Domain, DomainAdmin)
 admin.site.register(Feature, FeatureAdmin)

readthedocs/projects/migrations/0033_add_environment_variables.py

@@ -0,0 +1,37 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2018-11-12 13:57
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+import django_extensions.db.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('projects', '0032_increase_webhook_maxsize'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='EnvironmentVariable',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')),
+                ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')),
+                ('name', models.CharField(help_text='Name of the environment variable', max_length=128)),
+                ('value', models.CharField(help_text='Value of the environment variable', max_length=256)),
+            ],
+            options={
+                'ordering': ('-modified', '-created'),
+                'get_latest_by': 'modified',
+                'abstract': False,
+            },
+        ),
+        migrations.AddField(
+            model_name='environmentvariable',
+            name='project',
+            field=models.ForeignKey(help_text='Project where this variable will be used', on_delete=django.db.models.deletion.CASCADE, to='projects.Project'),
+        ),
+    ]

readthedocs/projects/models.py

@@ -15,6 +15,7 @@
 from django.db import models
 from django.utils.encoding import python_2_unicode_compatible
 from django.utils.translation import ugettext_lazy as _
+from django_extensions.db.models import TimeStampedModel
 from future.backports.urllib.parse import urlparse  # noqa
 from guardian.shortcuts import assign
 from taggit.managers import TaggableManager
@@ -869,13 +870,27 @@ def show_advertising(self):
         """
         Whether this project is ad-free
 
-        :return: ``True`` if advertising should be shown and ``False`` otherwise
+        :returns: ``True`` if advertising should be shown and ``False`` otherwise
+        :rtype: bool
         """
         if self.ad_free or self.gold_owners.exists():
             return False
 
         return True
 
+    @property
+    def environment_variables(self):
+        """
+        Environment variables to build this particular project.
+
+        :returns: dictionary with all the variables {name: value}
+        :rtype: dict
+        """
+        return {
+            variable.name: variable.value
+            for variable in self.environmentvariable_set.all()
+        }
+
 
 class APIProject(Project):
 
@@ -899,6 +914,7 @@ class Meta:
 
     def __init__(self, *args, **kwargs):
         self.features = kwargs.pop('features', [])
+        environment_variables = kwargs.pop('environment_variables', {})
         ad_free = (not kwargs.pop('show_advertising', True))
         # These fields only exist on the API return, not on the model, so we'll
         # remove them to avoid throwing exceptions due to unexpected fields
@@ -912,6 +928,7 @@ def __init__(self, *args, **kwargs):
 
         # Overwrite the database property with the value from the API
         self.ad_free = ad_free
+        self._environment_variables = environment_variables
 
     def save(self, *args, **kwargs):
         return 0
@@ -924,6 +941,10 @@ def show_advertising(self):
         """Whether this project is ad-free (don't access the database)"""
         return not self.ad_free
 
+    @property
+    def environment_variables(self):
+        return self._environment_variables
+
 
 @python_2_unicode_compatible
 class ImportedFile(models.Model):
@@ -1109,3 +1130,19 @@ def get_feature_display(self):
         implement this behavior.
         """
         return dict(self.FEATURES).get(self.feature_id, self.feature_id)
+
+
+class EnvironmentVariable(TimeStampedModel, models.Model):
+    name = models.CharField(
+        max_length=128,
+        help_text=_('Name of the environment variable'),
+    )
+    value = models.CharField(
+        max_length=256,
+        help_text=_('Value of the environment variable'),
+    )
+    project = models.ForeignKey(
+        Project,
+        on_delete=models.CASCADE,
+        help_text=_('Project where this variable will be used'),
+    )

readthedocs/projects/tasks.py

@@ -548,7 +548,7 @@ def get_build(build_pk):
         if build_pk:
             build = api_v2.build(build_pk).get()
         private_keys = [
-            'project', 'version', 'resource_uri', 'absolute_uri'
+            'project', 'version', 'resource_uri', 'absolute_uri',
         ]
         return {
             key: val
@@ -605,7 +605,7 @@ def get_env_vars(self):
         env = {
             'READTHEDOCS': True,
             'READTHEDOCS_VERSION': self.version.slug,
-            'READTHEDOCS_PROJECT': self.project.slug
+            'READTHEDOCS_PROJECT': self.project.slug,
         }
 
         if self.config.conda is not None:
@@ -616,7 +616,7 @@ def get_env_vars(self):
                     self.project.doc_path,
                     'conda',
                     self.version.slug,
-                    'bin'
+                    'bin',
                 ),
             })
         else:
@@ -625,10 +625,13 @@ def get_env_vars(self):
                     self.project.doc_path,
                     'envs',
                     self.version.slug,
-                    'bin'
+                    'bin',
                 ),
             })
 
+        # Update environment from Project's specific environment variables
+        env.update(self.project.environment_variables)
+
         return env
 
     def set_valid_clone(self):

readthedocs/restapi/serializers.py

@@ -43,11 +43,6 @@ class ProjectAdminSerializer(ProjectSerializer):
         slug_field='feature_id',
     )
 
-    show_advertising = serializers.SerializerMethodField()
-
-    def get_show_advertising(self, obj):
-        return obj.show_advertising
-
     class Meta(ProjectSerializer.Meta):
         fields = ProjectSerializer.Meta.fields + (
             'enable_epub_build',
@@ -68,6 +63,7 @@ class Meta(ProjectSerializer.Meta):
             'has_valid_clone',
             'has_valid_webhook',
             'show_advertising',
+            'environment_variables',
         )
 
 

readthedocs/rtd_tests/tests/test_api.py

@@ -26,7 +26,7 @@
 from readthedocs.builds.models import Build, BuildCommandResult, Version
 from readthedocs.integrations.models import Integration
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
-from readthedocs.projects.models import APIProject, Feature, Project
+from readthedocs.projects.models import APIProject, Feature, Project, EnvironmentVariable
 from readthedocs.restapi.views.integrations import GitHubWebhookView
 from readthedocs.restapi.views.task_views import get_status_data
 
@@ -704,6 +704,27 @@ def test_remote_organization_pagination(self):
         self.assertEqual(len(resp.data['results']), 25)  # page_size
         self.assertIn('?page=2', resp.data['next'])
 
+    def test_project_environment_variables(self):
+        user = get(User, is_staff=True)
+        project = get(Project, main_language_project=None)
+        get(
+            EnvironmentVariable,
+            name='TOKEN',
+            value='a1b2c3',
+            project=project,
+        )
+
+        client = APIClient()
+        client.force_authenticate(user=user)
+
+        resp = client.get('/api/v2/project/%s/' % (project.pk))
+        self.assertEqual(resp.status_code, 200)
+        self.assertIn('environment_variables', resp.data)
+        self.assertEqual(
+            resp.data['environment_variables'],
+            {'TOKEN': 'a1b2c3'},
+        )
+
     def test_init_api_project(self):
         project_data = {
             'name': 'Test Project',
@@ -716,13 +737,16 @@ def test_init_api_project(self):
         self.assertEqual(api_project.features, [])
         self.assertFalse(api_project.ad_free)
         self.assertTrue(api_project.show_advertising)
+        self.assertEqual(api_project.environment_variables, {})
 
         project_data['features'] = ['test-feature']
         project_data['show_advertising'] = False
+        project_data['environment_variables'] = {'TOKEN': 'a1b2c3'}
         api_project = APIProject(**project_data)
         self.assertEqual(api_project.features, ['test-feature'])
         self.assertTrue(api_project.ad_free)
         self.assertFalse(api_project.show_advertising)
+        self.assertEqual(api_project.environment_variables, {'TOKEN': 'a1b2c3'})
 
 
 class APIImportTests(TestCase):
@@ -1186,6 +1210,7 @@ def test_get_version_by_id(self):
                 'default_version': 'latest',
                 'description': '',
                 'documentation_type': 'sphinx',
+                'environment_variables': {},
                 'enable_epub_build': True,
                 'enable_pdf_build': True,
                 'features': ['allow_deprecated_webhooks'],

readthedocs/rtd_tests/tests/test_builds.py

@@ -6,14 +6,15 @@
 )
 
 import mock
+import os
 from django.test import TestCase
 from django_dynamic_fixture import fixture, get
 
 from readthedocs.builds.models import Build, Version
 from readthedocs.doc_builder.config import load_yaml_config
 from readthedocs.doc_builder.environments import LocalBuildEnvironment
 from readthedocs.doc_builder.python_environments import Virtualenv
-from readthedocs.projects.models import Project
+from readthedocs.projects.models import Project, EnvironmentVariable
 from readthedocs.projects.tasks import UpdateDocsTaskStep
 from readthedocs.rtd_tests.tests.test_config_integration import create_load
 
@@ -250,6 +251,55 @@ def test_save_config_in_build_model(self, load_config, api_v2):
         assert 'sphinx' in build_config
         assert build_config['doctype'] == 'sphinx'
 
+    def test_get_env_vars(self):
+        project = get(
+            Project,
+            slug='project',
+            documentation_type='sphinx',
+        )
+        get(
+            EnvironmentVariable,
+            name='TOKEN',
+            value='a1b2c3',
+            project=project,
+        )
+        build = get(Build)
+        version = get(Version, slug='1.8', project=project)
+        task = UpdateDocsTaskStep(
+            project=project, version=version, build={'id': build.pk},
+        )
+
+        # mock this object to make sure that we are NOT in a conda env
+        task.config = mock.Mock(conda=None)
+
+        env = {
+            'READTHEDOCS': True,
+            'READTHEDOCS_VERSION': version.slug,
+            'READTHEDOCS_PROJECT': project.slug,
+            'BIN_PATH': os.path.join(
+                project.doc_path,
+                'envs',
+                version.slug,
+                'bin',
+            ),
+            'TOKEN': 'a1b2c3',
+        }
+        self.assertEqual(task.get_env_vars(), env)
+
+        # mock this object to make sure that we are in a conda env
+        task.config = mock.Mock(conda=True)
+        env.update({
+            'CONDA_ENVS_PATH': os.path.join(project.doc_path, 'conda'),
+            'CONDA_DEFAULT_ENV': version.slug,
+            'BIN_PATH': os.path.join(
+                project.doc_path,
+                'conda',
+                version.slug,
+                'bin',
+            ),
+        })
+        self.assertEqual(task.get_env_vars(), env)
+
 
 class BuildModelTests(TestCase):