[Gh 998] Maintenance window #1236

TejasRGitHub · 2024-04-30T17:35:04Z

Feature or Bugfix

Feature

Detail

GH issue with description - Add Maintenance UI View / Window #998

Instructions on using maintenance window

Include yourself in the "DAAdministrators" group.
Goto Admin Section
Switch to Maintenance Tab
You can put data.all in Read-only mode or No-access mode. Select either mode and click on "Start Maintenance" window button
Once the status of the maintenance window is in ACTIVE state. You can be sure that the all ECS tasks have completed runnning and also the scheduled ECS tasks are turned OFF. ( Check this in AWS accounts for testing )
If in Read-only mode, then a red banner will appear whenever any user visits data.all UI . In Read-Only mode, all mutation calls ( are blocked )
If in No-Access mode, then after login a blank page displaying message about data.all maintenance window will appear. In No-Access mode, all graphQL calls are blocked ( Except getGroupForUser and getMaintenanceWindowStatus - which are required for the UI to load minimally).
In order to disable maintenance window, goto the maintenance tab and click stop maintenance window.

Note- In all the mode, data.all admin user is allowed to perform any gql call and access and modify anything on the UI.

Relates

Add Maintenance UI View / Window #998

Security

Please answer the questions below briefly where applicable, or write N/A. Based on
OWASP 10.

Does this PR introduce or modify any input fields or queries - this includes
fetching data from storage outside the application (e.g. a database, an S3 bucket)? N/A
- Is the input sanitized?
- What precautions are you taking before deserializing the data you consume?
- Is injection prevented by parametrizing queries?
- Have you ensured no eval or similar functions are used?
Does this PR introduce any functionality or component that requires authorization? N/A
- How have you ensured it respects the existing AuthN/AuthZ mechanisms?
- Are you logging failed auth attempts?
Are you using or adding any cryptographic features? N/A
- Do you use a standard proven implementations?
- Are the used keys controlled by the customer? Where are they stored?
Are you introducing any new policies/roles/users? Yes
- Have you used the least-privilege principle? How? Least Privelege

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

# Conflicts: # deploy/stacks/lambda_api.py

backend/api_handler.py

backend/dataall/base/utils/api_handler_utils.py

rbernotas · 2024-05-03T21:27:22Z

backend/dataall/modules/maintenance/services/maintenance_service.py

+            # Disable scheduled ECS tasks
+            # Get all the SSM Params related to the scheduled tasks
+            ecs_scheduled_rules = ParameterStoreManager.get_parameters_by_path(
+                region=os.getenv('AWS_REGION', 'eu-west-1'),


Should we really have a default value of eu-west-1 here? Seems like there should be no default and that we check region value and error if it is not a valid region.

I am taking the default value from the other similar boto3 wrappers which are defined in dataall/base/aws/ folder. I am in agreement that we should through error if the AWS_REGION env var is nt set. Maybe we should do it for other boto wrappers.

@dlpzx , what are you views ? Is there any particular reason to keep the eu-west-1

rbernotas · 2024-05-03T21:28:14Z

backend/dataall/modules/maintenance/services/maintenance_service.py

+            )
+            logger.debug(ecs_scheduled_rules)
+            ecs_scheduled_rules_list = [item['Value'] for item in ecs_scheduled_rules]
+            event_bridge_session = EventBridge(region=os.getenv('AWS_REGION', 'eu-west-1'))


Same as above, curious why we would default to eu-west-1 here.

rbernotas · 2024-05-03T21:29:05Z

backend/dataall/modules/maintenance/services/maintenance_service.py

+                )
+            # Enable scheduled ECS tasks
+            ecs_scheduled_rules = ParameterStoreManager.get_parameters_by_path(
+                region=os.getenv('AWS_REGION', 'eu-west-1'),


Same question about default region as above.

rbernotas · 2024-05-03T21:29:13Z

backend/dataall/modules/maintenance/services/maintenance_service.py

+            )
+            logger.debug(ecs_scheduled_rules)
+            ecs_scheduled_rules_list = [item['Value'] for item in ecs_scheduled_rules]
+            event_bridge_session = EventBridge(region=os.getenv('AWS_REGION', 'eu-west-1'))


Same question about default region as above.

rbernotas · 2024-05-03T21:29:24Z

backend/dataall/modules/maintenance/services/maintenance_service.py

+                if maintenance_record.status == MaintenanceStatus.PENDING.value:
+                    # Check if ECS tasks are running
+                    ecs_cluster_name = ParameterStoreManager.get_parameter_value(
+                        region=os.getenv('AWS_REGION', 'eu-west-1'),


Same question about default region as above.

frontend/src/authentication/components/AuthGuard.js

frontend/src/design/components/layout/DefaultNavbar.js

tests/modules/maintenance/test_mainenance_gql.py

backend/dataall/modules/maintenance/api/resolvers.py

tests/modules/maintenance/test_mainenance_gql.py

frontend/src/services/hooks/useGroups.js

frontend/src/services/graphql/MaintenanceWindow/index.js

frontend/src/modules/Administration/views/AdministrationView.js

frontend/src/modules/Administration/components/MaintenanceViewer.js

backend/dataall/modules/maintenance/api/types.py

backend/dataall/modules/maintenance/services/maintenance_service.py

TejasRGitHub · 2024-05-14T22:59:40Z

I think this feature should be covered by the new integration tests

For now, I have added the tests in the tests/modules/maintenance.

For the integration tests, i think they are using the cognito idp to authenticate. As we use our custom auth, I don't think I can add tests easily. I think this could be a part of another PR where-in the same integration tests can be run with a custom IDP user

TejasRGitHub · 2024-05-14T23:14:10Z

Updated code

Tested -
Local dev UI and sanity checks - ✅
AWS deployed data.all - Tested for READ-ONLY and NO-ACCESS modes - ✅

backend/api_handler.py

anushka-singh · 2024-05-15T17:26:53Z

backend/dataall/base/utils/api_handler_utils.py

+from dataall.core.permissions.services.tenant_policy_service import TenantPolicyValidationService
+
+logger = logging.getLogger()
+logger.setLevel(os.environ.get('LOG_LEVEL', 'INFO'))


What is this used for? We are using "log" everywhere else.

Do you mean why are we using the setLevel command ?

the setLevel method is used mostly at the top of logger initialization. I am adding it here to set the log level to whatever is set for each lambda in their environment variable - which is available via the os.environ.get('LOG_LEVEL', 'INFO').

backend/dataall/base/aws/event_bridge.py

backend/dataall/modules/maintenance/api/resolvers.py

backend/dataall/modules/maintenance/services/maintenance_service.py

backend/migrations/versions/b833ad41db68_maintenance_window_schema.py

backend/dataall/base/utils/api_handler_utils.py

TejasRGitHub

updated code

TejasRGitHub · 2024-05-23T18:03:01Z

backend/search_handler.py

+                    'Access-Control-Allow-Methods': '*',
+                },
+                'body': json.dumps(response),
+            }


Created GH issue - #1302

dlpzx

Looks good! I was a bit unsure about how the maintenance mode disabled was going to behave in the frontend. My concern was that the import import { MaintenanceViewer } from '../../Maintenance/components/MaintenanceViewer'; in frontend/src/modules/Administration/views/AdministrationView.js was going to result in errors, but I tested it locally and because it is not used in the tabs it works normal.

If we wanted to be ultra-clean we would check if (isModuleEnabled(ModuleNames.MAINTENANCE)) before importing and using the MaintenanceViewer, but this is not required.

Approving!

@TejasRGitHub

### Feature or Bugfix  - Bugfix ### Detail - Fixes hanging UI render on frontend introduced by AuthGuard useClient hook for Cognito Auth only ### Relates - Continuation of #1236 Thanks @TejasRGitHub for your help here! ### Security Please answer the questions below briefly where applicable, or write `N/A`. Based on [OWASP 10](https://owasp.org/Top10/en/). - Does this PR introduce or modify any input fields or queries - this includes fetching data from storage outside the application (e.g. a database, an S3 bucket)? - Is the input sanitized? - What precautions are you taking before deserializing the data you consume? - Is injection prevented by parametrizing queries? - Have you ensured no `eval` or similar functions are used? - Does this PR introduce any functionality or component that requires authorization? - How have you ensured it respects the existing AuthN/AuthZ mechanisms? - Are you logging failed auth attempts? - Are you using or adding any cryptographic features? - Do you use a standard proven implementations? - Are the used keys controlled by the customer? Where are they stored? - Are you introducing any new policies/roles/users? - Have you used the least-privilege principle? How? By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

TejasRGitHub and others added 23 commits April 9, 2024 23:53

Maintenance Window Frontend Changes - 1

a176949

Frontend views + few backend files comments for code

dc1eb1a

Alembic Script to upgrade db

7febedf

Event-bridge base functions and maintenance modules

c2eedda

Changes for making graphQL Work

482db02

Mutation graphQl corrections

485129e

Resolver changs

89e9944

Struture for event bridge graphql calls

2c400b7

Adding few thigns

3f5ace1

All GraphQL Endpoint working

9dd0890

Complete graphQL endpoint + api_handler logic to block calls

24a2493

Correcting enums for maintenance status

be31c18

Finalizing changes for maintenance window

174ce68

Comments and make lint plus yarn lint fix

1fe784b

Making container id for ssm param to sched task id

627d31c

Merge branch 'main' into data-dot-allGH-998-Maintenance-Window

95c63f5

# Conflicts: # deploy/stacks/lambda_api.py

Changes from aws deployed data.all after testing

093fab2

Linting fixes

fed1609

Removing custom maintenance text

11deb7b

Alembic script upgrade fix

f996471

Fixing integration tests

16a7dc1

Few final changes

672ee82

Linting

cdf7eee

TejasRGitHub marked this pull request as ready for review April 30, 2024 18:48

rbernotas reviewed May 3, 2024

View reviewed changes

dlpzx reviewed May 6, 2024

View reviewed changes

backend/dataall/modules/maintenance/api/types.py Outdated Show resolved Hide resolved

dlpzx reviewed May 6, 2024

View reviewed changes

backend/dataall/modules/maintenance/services/maintenance_service.py Show resolved Hide resolved

dlpzx reviewed May 6, 2024

View reviewed changes

backend/dataall/modules/maintenance/services/maintenance_service.py Outdated Show resolved Hide resolved

dlpzx reviewed May 6, 2024

View reviewed changes

backend/dataall/modules/maintenance/services/maintenance_service.py Outdated Show resolved Hide resolved

TejasRGitHub added 4 commits May 13, 2024 17:03

Making enums conform to graphQLMapper

f7fb577

trying few things

e7eba0c

resolving last of code review steps

85803fb

fixing linting

713605f

TejasRGitHub requested review from dlpzx and petrkalos May 14, 2024 22:50

TejasRGitHub added 2 commits May 14, 2024 18:04

Minor Changes

6b383ce

Minor Changes - 1

8180382

anushka-singh reviewed May 15, 2024

View reviewed changes

backend/api_handler.py Outdated Show resolved Hide resolved

anushka-singh reviewed May 15, 2024

View reviewed changes

dlpzx reviewed May 23, 2024

View reviewed changes

backend/dataall/base/aws/event_bridge.py Outdated Show resolved Hide resolved

dlpzx reviewed May 23, 2024

View reviewed changes

backend/dataall/modules/maintenance/api/resolvers.py Outdated Show resolved Hide resolved

dlpzx reviewed May 23, 2024

View reviewed changes

backend/dataall/modules/maintenance/services/maintenance_service.py Outdated Show resolved Hide resolved

dlpzx reviewed May 23, 2024

View reviewed changes

backend/dataall/modules/maintenance/services/maintenance_service.py Show resolved Hide resolved

dlpzx reviewed May 23, 2024

View reviewed changes

backend/migrations/versions/b833ad41db68_maintenance_window_schema.py Show resolved Hide resolved

dlpzx reviewed May 23, 2024

View reviewed changes

backend/dataall/base/utils/api_handler_utils.py Show resolved Hide resolved

trajopadhye and others added 3 commits May 23, 2024 12:14

Merge branch 'main' into data-dot-allGH-998-Maintenance-Window

c027fa1

Moving code and addressing code review comments

c5d6be9

Resolving migration tasks

e5728a5

TejasRGitHub requested a review from dlpzx May 23, 2024 17:54

TejasRGitHub mentioned this pull request May 23, 2024

Investigate whether search lambda (search_handler.py ) can be combined with api_handler.py #1302

Open

TejasRGitHub commented May 23, 2024

View reviewed changes

Removing unneccsary comments

9088ea5

dlpzx approved these changes Jun 3, 2024

View reviewed changes

petrkalos approved these changes Jun 4, 2024

View reviewed changes

dlpzx merged commit 2801125 into data-dot-all:main Jun 4, 2024
10 checks passed

noah-paige mentioned this pull request Jun 7, 2024

Add Maintenance Guard Component separate from AuthGuard #1321

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Gh 998] Maintenance window #1236

[Gh 998] Maintenance window #1236

TejasRGitHub commented Apr 30, 2024

rbernotas May 3, 2024

TejasRGitHub May 6, 2024

rbernotas May 3, 2024

rbernotas May 3, 2024

rbernotas May 3, 2024

rbernotas May 3, 2024

TejasRGitHub commented May 14, 2024 •

edited

Loading

TejasRGitHub commented May 14, 2024

anushka-singh May 15, 2024

TejasRGitHub May 20, 2024

anushka-singh May 20, 2024

TejasRGitHub May 23, 2024

TejasRGitHub left a comment

TejasRGitHub May 23, 2024

dlpzx left a comment

[Gh 998] Maintenance window #1236

[Gh 998] Maintenance window #1236

Conversation

TejasRGitHub commented Apr 30, 2024

Feature or Bugfix

Detail

Instructions on using maintenance window

Relates

Security

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TejasRGitHub commented May 14, 2024 • edited Loading

TejasRGitHub commented May 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TejasRGitHub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlpzx left a comment

Choose a reason for hiding this comment

TejasRGitHub commented May 14, 2024 •

edited

Loading