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

Expose environment variables from database into build commands #4894

Merged
merged 6 commits into from
Nov 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions readthedocs/doc_builder/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ class BuildEnvironment(BaseEnvironment):
ProjectBuildsSkippedError,
YAMLParseError,
BuildTimeoutError,
MkDocsYAMLParseError
MkDocsYAMLParseError,
)

def __init__(self, project=None, version=None, build=None, config=None,
Expand All @@ -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

Expand Down
35 changes: 28 additions & 7 deletions readthedocs/projects/admin.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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)
Expand Down
37 changes: 37 additions & 0 deletions readthedocs/projects/migrations/0033_add_environment_variables.py
Original file line number Diff line number Diff line change
@@ -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'),
),
]
39 changes: 38 additions & 1 deletion readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
agjohnson marked this conversation as resolved.
Show resolved Hide resolved
from future.backports.urllib.parse import urlparse # noqa
from guardian.shortcuts import assign
from taggit.managers import TaggableManager
Expand Down Expand Up @@ -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):

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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'),
agjohnson marked this conversation as resolved.
Show resolved Hide resolved
)
11 changes: 7 additions & 4 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -616,7 +616,7 @@ def get_env_vars(self):
self.project.doc_path,
'conda',
self.version.slug,
'bin'
'bin',
),
})
else:
Expand All @@ -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):
Expand Down
6 changes: 1 addition & 5 deletions readthedocs/restapi/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

agjohnson marked this conversation as resolved.
Show resolved Hide resolved
class Meta(ProjectSerializer.Meta):
fields = ProjectSerializer.Meta.fields + (
'enable_epub_build',
Expand All @@ -68,6 +63,7 @@ class Meta(ProjectSerializer.Meta):
'has_valid_clone',
'has_valid_webhook',
'show_advertising',
'environment_variables',
)


Expand Down
27 changes: 26 additions & 1 deletion readthedocs/rtd_tests/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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',
Expand All @@ -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):
Expand Down Expand Up @@ -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'],
Expand Down
52 changes: 51 additions & 1 deletion readthedocs/rtd_tests/tests/test_builds.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):

Expand Down