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

Refactoring env stack part4 #1181

Merged
6 changes: 2 additions & 4 deletions backend/dataall/core/environment/api/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ def create_environment(context: Context, source, input={}):
environment_uri=env.environmentUri,
target_type='environment',
target_uri=env.environmentUri,
target_label=env.label,
)
StackService.deploy_stack(targetUri=env.environmentUri)
env.userRoleInEnvironment = EnvironmentPermission.Owner.value
Expand Down Expand Up @@ -365,8 +364,7 @@ def resolve_environment_networks(context: Context, source, **kwargs):


def get_environment(context: Context, source, environmentUri: str = None):
with context.engine.scoped_session() as session:
return EnvironmentService.find_environment_by_uri(session, uri=environmentUri)
return EnvironmentService.find_environment_by_uri(uri=environmentUri)


def resolve_user_role(context: Context, source: Environment):
Expand Down Expand Up @@ -481,7 +479,7 @@ def generate_environment_access_token(context, source, environmentUri: str = Non
def get_environment_stack(context: Context, source: Environment, **kwargs):
return StackService.get_stack_with_cfn_resources(
targetUri=source.environmentUri,
environmentUri=source.environmentUri,
env=source,
)


Expand Down
15 changes: 5 additions & 10 deletions backend/dataall/core/environment/services/environment_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
from dataall.core.environment.db.environment_models import Environment, EnvironmentGroup
from dataall.core.environment.api.enums import EnvironmentPermission, EnvironmentType

from dataall.core.stacks.db.keyvaluetag_repositories import KeyValueTag
from dataall.core.stacks.db.stack_models import Stack
from dataall.core.stacks.db.keyvaluetag_repositories import KeyValueTagRepository
from dataall.core.stacks.api.enums import StackStatus
from dataall.core.environment.services.managed_iam_policies import PolicyManager

Expand Down Expand Up @@ -685,8 +684,9 @@ def get_environment_by_uri(session, uri) -> Environment:

@staticmethod
@ResourcePolicyService.has_resource_permission(environment_permissions.GET_ENVIRONMENT)
def find_environment_by_uri(session, uri) -> Environment:
return EnvironmentService.get_environment_by_uri(session, uri)
def find_environment_by_uri(uri) -> Environment:
with get_context().db_engine.scoped_session() as session:
return EnvironmentService.get_environment_by_uri(session, uri)

@staticmethod
def list_all_active_environments(session) -> [Environment]:
Expand All @@ -699,11 +699,6 @@ def list_all_active_environments(session) -> [Environment]:
log.info(f'Retrieved all active dataall environments {[e.AwsAccountId for e in environments]}')
return environments

@staticmethod
@ResourcePolicyService.has_resource_permission(environment_permissions.GET_ENVIRONMENT)
def get_stack(session, uri, stack_uri) -> Stack:
return session.query(Stack).get(stack_uri)

@staticmethod
@ResourcePolicyService.has_resource_permission(environment_permissions.DELETE_ENVIRONMENT)
def delete_environment(session, uri, environment):
Expand Down Expand Up @@ -733,7 +728,7 @@ def delete_environment(session, uri, environment):
resource_prefix=environment.resourcePrefix,
).delete_all_policies()

KeyValueTag.delete_key_value_tags(session, environment.environmentUri, 'environment')
KeyValueTagRepository.delete_key_value_tags(session, environment.environmentUri, 'environment')
EnvironmentResourceManager.delete_env(session, environment)
EnvironmentParameterRepository(session).delete_params(environment.environmentUri)

Expand Down
4 changes: 2 additions & 2 deletions backend/dataall/core/stacks/api/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
name='getStackLogs',
type=gql.ArrayType(gql.Ref('StackLog')),
args=[
gql.Argument(name='environmentUri', type=gql.NonNullableType(gql.String)),
gql.Argument(name='stackUri', type=gql.NonNullableType(gql.String)),
gql.Argument(name='targetUri', type=gql.NonNullableType(gql.String)),
gql.Argument(name='targetType', type=gql.NonNullableType(gql.String)),
],
resolver=get_stack_logs,
)
Expand Down
74 changes: 19 additions & 55 deletions backend/dataall/core/stacks/api/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,20 @@
import os

from dataall.base.api.context import Context
from dataall.core.environment.db.environment_models import Environment
from dataall.core.environment.services.environment_service import EnvironmentService
from dataall.core.stacks.services.keyvaluetag_service import KeyValueTagService
from dataall.core.stacks.services.stack_service import StackService
from dataall.core.stacks.aws.cloudformation import CloudFormation
from dataall.core.stacks.aws.cloudwatch import CloudWatch
from dataall.core.stacks.db.stack_models import Stack
from dataall.core.stacks.db.keyvaluetag_repositories import KeyValueTag
from dataall.core.stacks.db.stack_repositories import StackRepository
from dataall.base.db import exceptions
from dataall.core.stacks.aws.cloudwatch import CloudWatch
from dataall.base.utils import Parameter


log = logging.getLogger(__name__)


def get_stack(context: Context, source, environmentUri: str = None, stackUri: str = None):
with context.engine.scoped_session() as session:
env: Environment = EnvironmentService.get_environment_by_uri(session, environmentUri)
stack: Stack = StackRepository.get_stack_by_uri(session, stackUri)
cfn_task = StackService.save_describe_stack_task(session, env, stack, None)
CloudFormation.describe_stack_resources(engine=context.engine, task=cfn_task)
return EnvironmentService.get_stack(
session=session,
uri=environmentUri,
stack_uri=stackUri,
)
env = EnvironmentService.find_environment_by_uri(uri=environmentUri)
return StackService.get_and_describe_stack_in_env(env, stackUri)


def resolve_link(context, source, **kwargs):
Expand Down Expand Up @@ -67,51 +56,26 @@ def resolve_task_id(context, source: Stack, **kwargs):
return source.EcsTaskArn.split('/')[-1]


def get_stack_logs(context: Context, source, environmentUri: str = None, stackUri: str = None):
with context.engine.scoped_session() as session:
stack = EnvironmentService.get_stack(session=session, uri=environmentUri, stack_uri=stackUri)
if not stack.EcsTaskArn:
raise exceptions.AWSResourceNotFound(
action='GET_STACK_LOGS',
message='Logs could not be found for this stack',
)

query = f"""fields @timestamp, @message, @logStream, @log as @logGroup
| sort @timestamp asc
| filter @logStream like "{stack.EcsTaskArn.split('/')[-1]}"
"""
envname = os.getenv('envname', 'local')
results = CloudWatch.run_query(
query=query,
log_group_name=f"/{Parameter().get_parameter(env=envname, path='resourcePrefix')}/{envname}/ecs/cdkproxy",
days=1,
)
log.info(f'Running Logs query {query}')
return results
def get_stack_logs(context: Context, source, targetUri: str = None, targetType: str = None):
query = StackService.get_stack_logs(target_uri=targetUri, target_type=targetType)
envname = os.getenv('envname', 'local')
log_group_name = f"/{Parameter().get_parameter(env=envname, path='resourcePrefix')}/{envname}/ecs/cdkproxy"
results = CloudWatch.run_query(
query=query,
log_group_name=log_group_name,
days=1,
)
log.info(f'Running Logs query {query} for log_group_name={log_group_name}')
return results


def update_stack(context: Context, source, targetUri: str = None, targetType: str = None):
with context.engine.scoped_session() as session:
stack = StackRepository.update_stack(session=session, uri=targetUri, target_type=targetType)
StackService.deploy_stack(stack.targetUri)
return stack
return StackService.update_stack_by_target_uri(targetUri, targetType)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General comment about the refactoring...

  • doesn't the existence of the resolvers.py become redundant? They are now just proxy calls so can't we just point the resolver (where the GraphQL query is defined) directly to the service?
  • by creating the session within the business logic it's going to be more difficult to unit-test those methods because it will by harder to mock the results from the db. Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's open question yet. At least it gives us asyncronous calls (like in get_logs)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The principle was that in resolvers we check the api input and in service we just execute business logic. In the PRs for refactoring we are adding validation in the service layer but not in the resolvers. I did not add any comment in these PRs, because we already talked about implementing a better validation mechanism. I opened an issue so that we do not forget about it



def list_key_value_tags(context: Context, source, targetUri: str = None, targetType: str = None):
with context.engine.scoped_session() as session:
return KeyValueTag.list_key_value_tags(
session=session,
uri=targetUri,
target_type=targetType,
)
return KeyValueTagService.list_key_value_tags(targetUri, targetType)


def update_key_value_tags(context: Context, source, input=None):
with context.engine.scoped_session() as session:
kv_tags = KeyValueTag.update_key_value_tags(
session=session,
uri=input['targetUri'],
data=input,
)
StackService.deploy_stack(targetUri=input['targetUri'])
return kv_tags
return StackService.update_stack_tags(input)
84 changes: 15 additions & 69 deletions backend/dataall/core/stacks/db/keyvaluetag_repositories.py
Original file line number Diff line number Diff line change
@@ -1,96 +1,42 @@
import logging

from dataall.base.context import get_context
from dataall.core.permissions.services.resource_policy_service import ResourcePolicyService
from dataall.core.stacks.db import stack_models as models
from dataall.core.stacks.db.target_type_repositories import TargetType
from dataall.base.db import exceptions
from dataall.core.stacks.db.stack_models import KeyValueTag
from typing import List

logger = logging.getLogger(__name__)


class KeyValueTag:
class KeyValueTagRepository:
@staticmethod
def update_key_value_tags(session, uri: str, data: dict = None) -> [models.KeyValueTag]:
if not uri:
raise exceptions.RequiredParameter('targetUri')
if not data:
raise exceptions.RequiredParameter('data')
if not data.get('targetType'):
raise exceptions.RequiredParameter('targetType')

context = get_context()
ResourcePolicyService.check_user_resource_permission(
session=session,
username=context.username,
groups=context.groups,
resource_uri=uri,
permission_name=TargetType.get_resource_update_permission_name(data['targetType']),
)

tag_keys = [tag['key'].lower() for tag in data.get('tags', [])]
if tag_keys and len(tag_keys) != len(set(tag_keys)):
raise exceptions.UnauthorizedOperation(
action='SAVE_KEY_VALUE_TAGS',
message='Duplicate tag keys found. Please note that Tag keys are case insensitive',
)

tags = []
session.query(models.KeyValueTag).filter(
models.KeyValueTag.targetUri == uri,
models.KeyValueTag.targetType == data['targetType'],
).delete()
for tag in data.get('tags'):
kv_tag: models.KeyValueTag = models.KeyValueTag(
targetUri=uri, targetType=data['targetType'], key=tag['key'], value=tag['value'], cascade=tag['cascade']
)
tags.append(kv_tag)
session.add(kv_tag)

return tags

@staticmethod
def list_key_value_tags(session, uri, target_type) -> dict:
context = get_context()
ResourcePolicyService.check_user_resource_permission(
session=session,
username=context.username,
groups=context.groups,
resource_uri=uri,
permission_name=TargetType.get_resource_read_permission_name(target_type),
)
return KeyValueTag.find_key_value_tags(session, uri, target_type)

@staticmethod
def find_key_value_tags(session, target_uri, target_type) -> [models.KeyValueTag]:
def find_key_value_tags(session, target_uri, target_type) -> List[KeyValueTag]:
return (
session.query(models.KeyValueTag)
session.query(KeyValueTag)
.filter(
models.KeyValueTag.targetUri == target_uri,
models.KeyValueTag.targetType == target_type,
KeyValueTag.targetUri == target_uri,
KeyValueTag.targetType == target_type,
)
.all()
)

@staticmethod
def find_environment_cascade_key_value_tags(session, target_uri) -> [models.KeyValueTag]:
def find_environment_cascade_key_value_tags(session, target_uri) -> List[KeyValueTag]:
return (
session.query(models.KeyValueTag)
session.query(KeyValueTag)
.filter(
models.KeyValueTag.targetUri == target_uri,
models.KeyValueTag.targetType == 'environment',
models.KeyValueTag.cascade.is_(True),
KeyValueTag.targetUri == target_uri,
KeyValueTag.targetType == 'environment',
KeyValueTag.cascade.is_(True),
)
.all()
)

@staticmethod
def delete_key_value_tags(session, target_uri, target_type):
return (
session.query(models.KeyValueTag)
session.query(KeyValueTag)
.filter(
models.KeyValueTag.targetUri == target_uri,
models.KeyValueTag.targetType == target_type,
KeyValueTag.targetUri == target_uri,
KeyValueTag.targetType == target_type,
)
.delete()
)
20 changes: 1 addition & 19 deletions backend/dataall/core/stacks/db/stack_repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def find_stack_by_uri(session, stack_uri):
return stack

@staticmethod
def create_stack(session, environment_uri, target_label, target_uri, target_type, payload=None) -> models.Stack:
def create_stack(session, environment_uri, target_uri, target_type, payload=None) -> models.Stack:
environment: Environment = session.query(Environment).get(environment_uri)
if not environment:
raise exceptions.ObjectNotFound('Environment', environment_uri)
Expand All @@ -63,21 +63,3 @@ def create_stack(session, environment_uri, target_label, target_uri, target_type
session.add(stack)
session.commit()
return stack

@staticmethod
def update_stack(session, uri: str, target_type: str) -> [models.Stack]:
if not uri:
raise exceptions.RequiredParameter('targetUri')
if not target_type:
raise exceptions.RequiredParameter('targetType')

context = get_context()
ResourcePolicyService.check_user_resource_permission(
session=session,
username=context.username,
groups=context.groups,
resource_uri=uri,
permission_name=TargetType.get_resource_update_permission_name(target_type),
)
stack = StackRepository.get_stack_by_target_uri(session, target_uri=uri)
return stack
Loading
Loading