Skip to content

Commit

Permalink
Merge branch 'main' into feature/vscode-tool-add
Browse files Browse the repository at this point in the history
  • Loading branch information
jamesstottmoj committed Feb 21, 2024
2 parents 8f43765 + cb57411 commit e9f2120
Show file tree
Hide file tree
Showing 16 changed files with 158 additions and 23 deletions.
4 changes: 3 additions & 1 deletion controlpanel/api/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ def oidc_provider_statement(self):
"identity_provider_arn": iam_arn(
f"oidc-provider/{settings.OIDC_APP_EKS_PROVIDER}"
),
"app_name": self.app.slug,
"app_namespace": self.app.namespace,
}
)
return json.loads(statement)
Expand All @@ -508,6 +508,8 @@ def create_iam_role(self):
assume_role_policy = deepcopy(BASE_ASSUME_ROLE_POLICY)
assume_role_policy["Statement"].append(self.oidc_provider_statement)
self.aws_role_service.create_role(self.iam_role_name, assume_role_policy)
for env in self.get_deployment_envs():
self._create_secrets(env_name=env)

def grant_bucket_access(self, bucket_arn, access_level, path_arns):
self.aws_role_service.grant_bucket_access(
Expand Down
17 changes: 17 additions & 0 deletions controlpanel/api/migrations/0032_app_namespace.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 4.2.7 on 2024-02-20 16:00

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("api", "0031_add_soft_delete_fields"),
]

operations = [
migrations.AddField(
model_name="app",
name="namespace",
field=models.CharField(blank=True, max_length=63, null=True, unique=True),
),
]
20 changes: 20 additions & 0 deletions controlpanel/api/migrations/0033_add_namespaces_values_to_apps.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Generated by Django 4.2.7 on 2024-02-20 16:01

from django.db import migrations


def add_namespaces(apps, schema_editor):
App = apps.get_model("api", "App")
for app in App.objects.all():
app.namespace = f"data-platform-app-{app.slug}"
app.save()


class Migration(migrations.Migration):
dependencies = [
("api", "0032_app_namespace"),
]

operations = [
migrations.RunPython(code=add_namespaces, reverse_code=migrations.RunPython.noop)
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 4.2.7 on 2024-02-20 16:11

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("api", "0033_add_namespaces_values_to_apps"),
]

operations = [
migrations.AlterField(
model_name="app",
name="namespace",
field=models.CharField(max_length=63, unique=True),
),
]
8 changes: 6 additions & 2 deletions controlpanel/api/models/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ class App(TimeStampedModel):
# are not within the fields which will be searched frequently
app_conf = models.JSONField(null=True)

# Stores the Cloud Platform namespace name
namespace = models.CharField(unique=True, max_length=63)

# Non database field just for passing extra parameters
disable_authentication = False
connections = {}
Expand Down Expand Up @@ -251,12 +254,13 @@ def auth0_client_name(self, env_name=None):

def app_url_name(self, env_name):
format_pattern = settings.APP_URL_NAME_PATTERN.get(env_name.upper())
namespace = self.namespace.removeprefix("data-platform-app-")
if not format_pattern:
format_pattern = settings.APP_URL_NAME_PATTERN.get(self.DEFAULT_SETTING_KEY_WORD)
if format_pattern:
return format_pattern.format(app_name=self.slug, env=env_name)
return format_pattern.format(app_name=namespace, env=env_name)
else:
return self.slug
return namespace

def get_auth_client(self, env_name):
env_name = env_name or self.DEFAULT_AUTH_CATEGORY
Expand Down
3 changes: 2 additions & 1 deletion controlpanel/api/tasks/handlers/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ class CreateAppAWSRole(BaseModelTaskHandler):
name = "create_app_aws_role"

def handle(self):
cluster.App(self.object).create_iam_role()
task_user = User.objects.filter(pk=self.task_user_pk).first()
cluster.App(self.object, task_user.github_api_token).create_iam_role()
self.complete()
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
"StringEquals": {
"{{ identity_provider }}:aud": "sts.amazonaws.com",
"{{ identity_provider }}:sub": [
"system:serviceaccount:data-platform-app-{{ app_name }}-dev:data-platform-app-{{ app_name }}-dev-sa",
"system:serviceaccount:data-platform-app-{{ app_name }}-prod:data-platform-app-{{ app_name }}-prod-sa"
"system:serviceaccount:{{ app_namespace }}-dev:{{ app_namespace }}-dev-sa",
"system:serviceaccount:{{ app_namespace }}-prod:{{ app_namespace }}-prod-sa"
]
}
}
Expand Down
16 changes: 15 additions & 1 deletion controlpanel/frontend/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def _check_inputs_for_custom_connection(self, cleaned_data):
return auth0_conn_data


class CreateAppForm(AppAuth0Form):
class CreateAppForm(forms.Form):

repo_url = forms.CharField(
max_length=512,
Expand Down Expand Up @@ -148,8 +148,10 @@ class CreateAppForm(AppAuth0Form):
empty_label="Select",
required=False,
)
namespace = forms.CharField(required=True, max_length=63)

def __init__(self, *args, **kwargs):
self.request = kwargs.pop("request", None)
super().__init__(*args, **kwargs)
self.fields["existing_datasource_id"].queryset = self.get_datasource_queryset()

Expand Down Expand Up @@ -204,6 +206,18 @@ def clean_repo_url(self):

return repo_url

def clean_namespace(self):
"""
Removes the env suffix if the user included it
"""
namespace = self.cleaned_data["namespace"]
for suffix in ["-dev", "-prod"]:
if suffix in namespace:
namespace = namespace.removesuffix(suffix)
break

return namespace


class UpdateAppAuth0ConnectionsForm(AppAuth0Form):
env_name = forms.CharField(widget=forms.HiddenInput)
Expand Down
26 changes: 22 additions & 4 deletions controlpanel/frontend/jinja2/webapp-create.html
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,28 @@ <h1 class="govuk-heading-xl">{{ page_title }}</h1>
}) }}
{% endif %}

<input type="text" class="govuk-input" id="display_result_repo" name="repo_url" />
<input type="text" class="govuk-input" id="display_result_repo" name="repo_url" required />

<br/>
<br/>
</div>
<div class="govuk-form-group" id="container-element">

<label class="govuk-label govuk-label--m" for="display_result_repo">Cloud Platform namespace
</label>

{% set error_repo_msg = form.errors.get("namespace") %}
{% if error_repo_msg %}
{% set errorId = 'namespace-error' %}
{{ govukErrorMessage({
"id": errorId,
"html": error_repo_msg|join(". "),
}) }}
{% endif %}
<span class="govuk-hint">Enter namespace with the -env suffix removed</span>
<input type="text" class="govuk-input" id="id_namespace" name="namespace" required />

</div>


{{ govukRadios({
"name": "connect_bucket",
"fieldset": {
Expand All @@ -89,7 +104,7 @@ <h1 class="govuk-heading-xl">{{ page_title }}</h1>
},
},
"hint": {
"text": "Connect an existing app data source to your app, or create a new one.",
"text": "Connect an existing app data source to your app, or create a new one. If you don't need to connect to an S3 bucket, select 'Do this later'",
},
"items": [
{
Expand All @@ -100,6 +115,9 @@ <h1 class="govuk-heading-xl">{{ page_title }}</h1>
{
"value": "existing",
"html": existing_datasource_html|safe,
"hint": {
"text": "Only buckets that you have admin access to are displayed",
},
"checked": form.connect_bucket.value() == "existing"
},
{
Expand Down
2 changes: 0 additions & 2 deletions controlpanel/frontend/views/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,6 @@ def get_form_kwargs(self):
kwargs = FormMixin.get_form_kwargs(self)
kwargs.update(
request=self.request,
all_connections_names=auth0.ExtendedAuth0().connections.get_all_connection_names(), # noqa: E501
custom_connections=auth0.ExtendedConnections.custom_connections(),
)
return kwargs

Expand Down
1 change: 1 addition & 0 deletions controlpanel/frontend/views/apps_mng.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def register_app(self, user, app_data):
name=name,
repo_url=repo_url,
current_user=user,
namespace=app_data["namespace"],
)
self._add_app_to_users(new_app, user)
self._create_or_link_datasource(new_app, user, app_data)
Expand Down
24 changes: 18 additions & 6 deletions tests/api/cluster/test_app.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
# Standard library
from copy import deepcopy
from unittest.mock import MagicMock, patch
from unittest.mock import MagicMock, patch, call

# Third-party
import pytest
from django.conf import settings

# First-party/Local
from controlpanel.api import cluster, models
Expand All @@ -13,7 +12,11 @@

@pytest.fixture
def app():
return models.App(slug="test-app", repo_url="https://gitpub.example.com/test-repo")
return models.App(
slug="test-app",
repo_url="https://gitpub.example.com/test-repo",
namespace="test-namespace",
)


@pytest.fixture
Expand Down Expand Up @@ -65,8 +68,8 @@ def oidc_provider_statement(app, settings):
"StringEquals": {
f"{settings.OIDC_APP_EKS_PROVIDER}:aud": "sts.amazonaws.com",
f"{settings.OIDC_APP_EKS_PROVIDER}:sub": [
f"system:serviceaccount:data-platform-app-{app.slug}-dev:data-platform-app-{app.slug}-dev-sa", # noqa
f"system:serviceaccount:data-platform-app-{app.slug}-prod:data-platform-app-{app.slug}-prod-sa" # noqa
f"system:serviceaccount:{app.namespace}-dev:{app.namespace}-dev-sa", # noqa
f"system:serviceaccount:{app.namespace}-prod:{app.namespace}-prod-sa" # noqa
]
}
}
Expand All @@ -77,13 +80,22 @@ def test_oidc_provider_statement(app, oidc_provider_statement):
assert cluster.App(app).oidc_provider_statement == oidc_provider_statement


def test_app_create_iam_role(aws_create_role, app, oidc_provider_statement):
@patch("controlpanel.api.cluster.App.get_deployment_envs")
@patch("controlpanel.api.cluster.App._create_secrets")
def test_app_create_iam_role(
_create_secrets, get_deployment_envs, aws_create_role, app, oidc_provider_statement
):
expected_assume_role = deepcopy(BASE_ASSUME_ROLE_POLICY)
expected_assume_role["Statement"].append(oidc_provider_statement)

get_deployment_envs.return_value = ["dev", "prod"]
cluster.App(app).create_iam_role()

aws_create_role.assert_called_with(app.iam_role_name, expected_assume_role)
_create_secrets.assert_has_calls([
call(env_name="dev"),
call(env_name="prod"),
])


@pytest.fixture # noqa: F405
Expand Down
17 changes: 15 additions & 2 deletions tests/api/models/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ def test_slug_characters_replaced():

@pytest.mark.django_db
def test_slug_collisions_increments():
app = App.objects.create(repo_url="[email protected]:org/foo-bar.git")
app = App.objects.create(repo_url="[email protected]:org/foo-bar.git", namespace="foo-bar")
assert "foo-bar" == app.slug

app2 = App.objects.create(repo_url="https://www.example.com/org/foo-bar")
app2 = App.objects.create(repo_url="https://www.example.com/org/foo-bar", namespace="foo-bar-2")
assert "foo-bar-2" == app2.slug


Expand Down Expand Up @@ -209,3 +209,16 @@ def test_app_allowed_ip_ranges():
def test_iam_role_arn():
app = App(slug="example-app")
assert app.iam_role_arn == f"arn:aws:iam::{settings.AWS_DATA_ACCOUNT_ID}:role/test_app_example-app"


@pytest.mark.parametrize("namespace, env, expected", [
("data-platform-app-example", "dev", "example-dev"),
("example", "dev", "example-dev"),
("data-platform-example", "dev", "data-platform-example-dev"),
("data-platform-app-example", "prod", "example"),
("example", "prod", "example"),
("data-platform-example", "prod", "data-platform-example"),
])
def test_app_url_name(namespace, env, expected):
app = App(namespace=namespace)
assert app.app_url_name(env_name=env) == expected
5 changes: 3 additions & 2 deletions tests/api/tasks/test_create_app_aws_role.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Standard library
from unittest.mock import patch
from unittest.mock import patch, MagicMock

# Third-party
import pytest
Expand All @@ -25,13 +25,14 @@ def test_cluster_not_called_without_valid_app(cluster, complete, users):


@pytest.mark.django_db
@patch("controlpanel.api.auth0.ExtendedAuth0", new=MagicMock())
@patch("controlpanel.api.tasks.handlers.base.BaseModelTaskHandler.complete")
@patch("controlpanel.api.tasks.handlers.app.cluster")
def test_valid_app_and_user(cluster, complete, users):
app = mommy.make("api.App")

create_app_aws_role(app.pk, users["superuser"].pk)

cluster.App.assert_called_once_with(app)
cluster.App.assert_called_once_with(app, users["superuser"].github_api_token)
cluster.App.return_value.create_iam_role.assert_called_once()
complete.assert_called_once()
16 changes: 16 additions & 0 deletions tests/frontend/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import pytest
from django.core.exceptions import ValidationError
from django.urls import reverse
from mock import MagicMock

# First-party/Local
from controlpanel.api import aws
Expand Down Expand Up @@ -155,6 +156,7 @@ def test_create_app_form_clean_new_datasource(create_app_request_superuser):
"repo_url": "https://github.com/ministryofjustice/my_repo",
"connect_bucket": "new",
"new_datasource_name": "test-bucketname",
"namespace": "my-repo"
},
request=create_app_request_superuser,
)
Expand All @@ -170,6 +172,7 @@ def test_create_app_form_clean_new_datasource(create_app_request_superuser):
"deployment_envs": ["test"],
"repo_url": "https://github.com/ministryofjustice/my_repo",
"connect_bucket": "new",
"namespace": "my-repo"
},
request=create_app_request_superuser,
)
Expand Down Expand Up @@ -287,6 +290,7 @@ def test_create_app_form_clean_repo_url(create_app_request_superuser):
"repo_url": "https://github.com/ministryofjustice/my_repo",
"connect_bucket": "new",
"new_datasource_name": "test-bucketname",
"namespace": "my-repo"
},
request=create_app_request_superuser,
)
Expand Down Expand Up @@ -460,3 +464,15 @@ def test_ip_allowlist_form_missing_name():
}
f = forms.IPAllowlistForm(data)
assert f.errors["name"] == ["This field is required."]


@pytest.mark.parametrize("env", ["dev", "prod"])
@mock.patch(
"controlpanel.frontend.forms.CreateAppForm.get_datasource_queryset",
new=MagicMock,
)
def test_clean_namespace(env):
form = forms.CreateAppForm()
form.cleaned_data = {"namespace": f"my-namespace-{env}"}

assert form.clean_namespace() == "my-namespace"
1 change: 1 addition & 0 deletions tests/frontend/views/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ def test_register_app_with_creating_datasource(client, users):
repo_url=f"https://github.com/ministryofjustice/{test_app_name}",
connect_bucket="new",
new_datasource_name=test_bucket_name,
namespace="test-app-namespace",
)
response = client.post(reverse("create-app"), data)

Expand Down

0 comments on commit e9f2120

Please sign in to comment.