Bug fixes after 2-7-0-rc #1785

TejasRGitHub · 2025-03-12T19:39:09Z

Feature or Bugfix

Bugfix

Detail

Fixes for maintenance window - [BUG] Maintenance window doesn't start correctly #1778
Importing a glue db with "-" in it - [BUG] Glue db validation incorrect #1776

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? N/A
- 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 · 2025-03-12T19:39:42Z

backend/dataall/base/utils/naming_convention.py

@@ -14,10 +14,10 @@ class NamingConventionPattern(Enum):
    IAM = {'regex': '[^a-zA-Z0-9-_]', 'separator': '-', 'max_length': 63}  # Role names up to 64 chars
    IAM_POLICY = {'regex': '[^a-zA-Z0-9-_]', 'separator': '-', 'max_length': 128}  # Policy names up to 128 chars
    GLUE = {
-        'regex': '[^a-zA-Z0-9_]',
+        'regex': '[^a-zA-Z0-9_-]',


Adding "-" into the regex

TejasRGitHub · 2025-03-12T19:39:52Z

backend/dataall/modules/s3_datasets/cdk/assets/glueprofilingjob/glue_script.py

@@ -100,7 +100,7 @@ def run_table_profiling(
        logger.debug('Profiling table for %s %s ', database, table)
        logger.debug('using %s', database)
        spark.sql('use `{}`'.format(database))
-        df = spark.sql('select * from {}'.format(table))
+        df = spark.sql('select * from `{}`'.format(table))


Minor correction

TejasRGitHub · 2025-03-12T19:40:26Z

deploy/stacks/pipeline.py

@@ -426,7 +426,6 @@ def validate_deployment_params(self, source, repo_connection_arn, git_branch, re
                    or 'client_id' not in custom_auth_configs
                    or 'response_types' not in custom_auth_configs
                    or 'scopes' not in custom_auth_configs
-                    or 'jwks_url' not in custom_auth_configs
                    or 'claims_mapping' not in custom_auth_configs


Removing jwks_url from pipeline validation as it is no longer used

TejasRGitHub · 2025-03-12T19:42:24Z

frontend/src/authentication/components/MaintenanceGuard.js

    // Check if the maintenance window is enabled and has NO-ACCESS Status
    // If yes then display a blank screen with a message that data.all is in maintenance mode ( Check use of isNoAccessMaintenance state )
    if (isModuleEnabled(ModuleNames.MAINTENANCE) === true) {
-      if (client) {
+      if (client && groups) {
        checkMaintenanceMode().catch((e) => dispatch({ type: SET_ERROR, e }));


Adding groups check, so that the UI is correctly displayed. Without this, when data.all is in No-Access maintenance mode, No-access maintenance window is shown for a fraction of second and then goes away and proper UI is rendered

TejasRGitHub · 2025-03-12T19:43:05Z

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

@@ -491,6 +487,14 @@ export const MaintenanceViewer = () => {
    }
  }, [client]);

+  useEffect(() => {


See #1778 (comment) for more details

anushka-singh · 2025-03-12T20:58:07Z

backend/dataall/base/utils/naming_convention.py

@@ -14,10 +14,10 @@ class NamingConventionPattern(Enum):
    IAM = {'regex': '[^a-zA-Z0-9-_]', 'separator': '-', 'max_length': 63}  # Role names up to 64 chars
    IAM_POLICY = {'regex': '[^a-zA-Z0-9-_]', 'separator': '-', 'max_length': 128}  # Policy names up to 128 chars
    GLUE = {
-        'regex': '[^a-zA-Z0-9_]',
+        'regex': '[^a-zA-Z0-9_-]',
        'separator': '_',


But we are specifying separator as '_'.
What will happen if we have dashes in the name? It wont detect the separator as '-'.

The separator field is used to create a data.all generated name ( like dataall_glue_db, etc ) when we create a dataset and data.all creates the S3 bucket, gluedb, etc.

The regex field is used when making a name that is compliant ( with that regex ). In such cases, when data.all creates a name, and uses "_" as separator then that would be a valid name since it is part of the character set defined by regex

anushka-singh · 2025-03-12T21:11:39Z

frontend/package-lock.json

+      "version": "1.8.3",
+      "resolved": "https://registry.npmjs.org/axios/-/axios-1.8.3.tgz",
+      "integrity": "sha512-iP4DebzoNlP/YN2dpwCgb8zoCmhtkajzS48JvwmkSkXvPI3DHc7m+XYL5tGnSlJtR6nImXZmdCuN5aP8dh1d8A==",


Where are these changes coming from and why do we need them?

These changes are because I bumped versions of axios to 1.8.2 and as a consequence has to update the package-lock.json and yarn.lock with npm install.
The version of axios which was present was flagged by npm-audit gh action check

rbernotas · 2025-03-12T21:24:19Z

backend/dataall/base/aws/parameter_store.py

-            )['Parameters']
+            paginator = ParameterStoreManager.client(AwsAccountId, region).get_paginator('get_parameters_by_path')
+            for page in paginator.paginate(Path=parameter_path):
+                parameter_value_list.extend(page['Parameters'])


Could be the following instead to avoid KeyError on 'Parameters' if it does not exist, unless you are really wanting that exception if it doesn't exist:

parameter_value_list.extend(page.get('Parameters', []))

Umm. Taking a look at the API response ,https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/ssm/client/get_parameters_by_path.html, Parameters is always returned ( return empty list [] if nothing is found). Since this contract is declared, I am expecting Parameters to always be present. Also, if Parameters is not present, then I would want this function to raise an exception which bubbles up to the user enabling maintenance window.

rbernotas · 2025-03-12T21:27:16Z

backend/dataall/modules/s3_datasets/cdk/assets/glueprofilingjob/glue_script.py

@@ -100,7 +100,7 @@ def run_table_profiling(
        logger.debug('Profiling table for %s %s ', database, table)
        logger.debug('using %s', database)
        spark.sql('use `{}`'.format(database))
-        df = spark.sql('select * from {}'.format(table))
+        df = spark.sql('select * from `{}`'.format(table))


could also just be:

df = spark.sql(f"SELECT * FROM {table}")

Yeah I could do this as well. I just wanted add the `` -

df = spark.sql(f"SELECT * FROM `{table}`")

TejasRGitHub commented Mar 12, 2025

View reviewed changes

TejasRGitHub requested review from anushka-singh, rbernotas, dlpzx and SofiaSazonova March 12, 2025 19:43

TejasRGitHub added 4 commits March 12, 2025 15:25

Fixes for bugs

894a725

Adding maintenance window fixes

1682a0e

bumping version of axios

c0d19b8

Adding package-lock.json and yarn.lock

a766e87

TejasRGitHub force-pushed the gh-fixes-after-2-7-0-rc branch from 793bb81 to a766e87 Compare March 12, 2025 20:26

anushka-singh reviewed Mar 12, 2025

View reviewed changes

rbernotas reviewed Mar 12, 2025

View reviewed changes

petrkalos added this to v2.8.0 Apr 24, 2025

petrkalos moved this to Nominated in v2.8.0 Apr 24, 2025

petrkalos moved this from Nominated to Done in v2.8.0 Apr 24, 2025

petrkalos moved this from Done to Review in progress in v2.8.0 Apr 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bug fixes after 2-7-0-rc #1785

Bug fixes after 2-7-0-rc #1785

Uh oh!

TejasRGitHub commented Mar 12, 2025

Uh oh!

TejasRGitHub Mar 12, 2025

Uh oh!

TejasRGitHub Mar 12, 2025

Uh oh!

TejasRGitHub Mar 12, 2025

Uh oh!

TejasRGitHub Mar 12, 2025

Uh oh!

TejasRGitHub Mar 12, 2025

Uh oh!

anushka-singh Mar 12, 2025

Uh oh!

TejasRGitHub Mar 12, 2025

Uh oh!

anushka-singh Mar 12, 2025

Uh oh!

TejasRGitHub Mar 12, 2025

Uh oh!

rbernotas Mar 12, 2025

Uh oh!

TejasRGitHub Mar 12, 2025

Uh oh!

rbernotas Mar 12, 2025

Uh oh!

TejasRGitHub Mar 12, 2025

Uh oh!

Uh oh!

Bug fixes after 2-7-0-rc #1785

Are you sure you want to change the base?

Bug fixes after 2-7-0-rc #1785

Uh oh!

Conversation

TejasRGitHub commented Mar 12, 2025

Feature or Bugfix

Detail

Security

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!