[COST-4515] Upgrade to Django 4.1 (#4853)

* upgrade to Django 4.1 * Update deprecated ‘warn’ method * Remove deprecated test case method * Remove deprecated Series.bool method * Correct docstring and class attribute name * Cleanup file handle * Fix flaky test This assert statement will always fail since the template schema does not yet exists. This test will only pass if another test runs before this test creates the template schema. * Update clone_schema.sql Do not define a custom sequence object since that is no longer required with the move to identity columns. Co-authored-by: Michael Skarbek <[email protected]> Co-authored-by: David Nakabaale <[email protected]> * Update clone_schema SQL function when necessary We may not end up wanting this to be so automatic, but this is a good way to handle it if needed. The only problem is getting the hash is a bit clunky since the contents of clone_schema.sql do not exactly match the function definition once it is stored in the database. I added the hash output to logs in order to make updating the hash easier. * Avoid unnecessary globals * update comments in clone_schema func * Update clone_schema hash * add management cmd to migrate serial cols * Update clone_schema.sql Do not define a custom sequence object since that is no longer required with the move to identity columns. Co-authored-by: Michael Skarbek <[email protected]> Co-authored-by: David Nakabaale <[email protected]> * Update license information Add correct copyright and license information to management command. Create project level licenses to reflect the use of multiple licenses within the project. * Improvements to serial to identity management command - Use SQL composition rather than format strings to protect against SQL injection attacks. The risk is quite low, but it’s best to just do the right thing. - Reorganize the code to avoid a nested function definiton and an unnecessary global variable that only needed to be local. * Do not update clone_schema function automatically * Add migration to update clone_schema function * fixup! Improvements to serial to identity management command --------- Co-authored-by: Sam Doran <[email protected]> Co-authored-by: Michael Skarbek <[email protected]>
project-koku · Feb 13, 2024 · 4f62011 · 4f62011
1 parent 8595845
commit 4f62011
Show file tree

Hide file tree

Showing 22 changed files with 327 additions and 163 deletions.
diff --git a/Pipfile b/Pipfile
@@ -20,7 +20,7 @@ celery = ">=5.2.2"
 ciso8601 = ">=2.1"
 confluent-kafka = ">=2.1.0"
 croniter = "*"
-Django = "~=4.0.0"
+Django = "~=4.1.0"
 django-cors-headers = ">=3.1"
 django-environ = ">=0.4"
 django-extensions = ">=2.2"

diff --git a/Pipfile.lock b/Pipfile.lock
diff --git a/db_functions/clone_schema.sql b/db_functions/clone_schema.sql
@@ -388,48 +388,6 @@ BEGIN
     END IF;
     EXECUTE 'CREATE SCHEMA ' || dst_schema || ' ;';
 
-    /*
-     * Create sequences
-     */
-    IF cardinality(sequence_objects) > 0
-    THEN
-        IF _verbose
-        THEN
-            RAISE INFO 'Creating sequences for %', dst_schema;
-        END IF;
-        FOREACH jobject IN ARRAY sequence_objects
-        LOOP
-            IF _verbose
-            THEN
-                RAISE INFO '    %.%', dst_schema, quote_ident(jobject->>'sequence_name'::text);
-            END IF;
-            EXECUTE FORMAT('CREATE SEQUENCE IF NOT EXISTS %s.%I AS %s START WITH %s INCREMENT BY %s MINVALUE %s MAXVALUE %s CACHE %s %s ;',
-                        dst_schema,
-                        jobject->>'sequence_name'::text,
-                        jobject->>'sequence_type'::text,
-                        jobject->>'sequence_start'::text,
-                        jobject->>'sequence_inc'::text,
-                        jobject->>'sequence_min'::text,
-                        jobject->>'sequence_max'::text,
-                        jobject->>'sequence_cache'::text,
-                        jobject->>'sequence_cycle');
-
-            IF copy_data OR
-            (jobject->>'sequence_name' ~ 'partitioned_tables'::text) OR
-            (jobject->>'sequence_name' ~ 'django_migrations'::text)
-            THEN
-                EXECUTE 'SELECT setval(''' || dst_schema || '.' || quote_ident(jobject->>'sequence_name') || '''::regclass, '::text ||
-                        '(SELECT last_value + 1 FROM ' ||
-                        src_schema || '.' || quote_ident(jobject->>'sequence_name') || ') );'::text;
-            END IF;
-        END LOOP;
-    ELSE
-        IF _verbose
-        THEN
-            RAISE INFO 'No sequences for %', dst_schema;
-        END IF;
-    END IF;
-
     /*
      * Create tables
      */
@@ -509,37 +467,27 @@ BEGIN
     END IF;
 
     /*
-     * Create sequence owner links
+     * Set sequence value
      */
     IF cardinality(sequence_owner_info) > 0
     THEN
         IF _verbose
         THEN
-            RAISE INFO 'Setting sequence ownership for objects in %', dst_schema;
+            RAISE INFO 'Set current value for sequence objects in %', dst_schema;
         END IF;
         FOREACH jobject IN ARRAY sequence_owner_info
         LOOP
             IF _verbose
             THEN
                 RAISE INFO '    Update primary key default for %.%', dst_schema, quote_ident(jobject->>'owner_object'::text);
             END IF;
-            EXECUTE FORMAT('ALTER TABLE %s.%I ALTER COLUMN %I SET DEFAULT nextval( ''%s.%I''::regclass );',
-                        dst_schema,
-                        jobject->>'owner_object'::text,
-                        jobject->>'owner_column'::text,
-                        dst_schema,
-                        jobject->>'sequence_name'::text);
 
-            IF _verbose
-            THEN
-                RAISE INFO '    Update sequence owned-by table column to %."%"', dest_obj, jobject->>'owner_column'::text;
-            END IF;
-            EXECUTE FORMAT('ALTER SEQUENCE %s.%I OWNED BY %s.%I.%I ;',
+            EXECUTE FORMAT('SELECT setval(''%s.%I'', (SELECT max(%I) FROM %s.%I));',
                         dst_schema,
                         jobject->>'sequence_name'::text,
-                        dest_schema,
-                        jobject->>'owner_object'::text,
-                        jobject->>'owner_column'::text);
+                        jobject->>'owner_column'::text,
+                        dst_schema,
+                        jobject->>'owner_object'::text);
         END LOOP;
     ELSE
         IF _verbose

diff --git a/koku/api/iam/models.py b/koku/api/iam/models.py
@@ -20,7 +20,6 @@
 from koku.migration_sql_helpers import apply_sql_file
 from koku.migration_sql_helpers import find_db_functions_dir
 
-
 LOG = logging.getLogger(__name__)
 
 
@@ -85,9 +84,9 @@ class Tenant(TenantMixin):
     _TEMPLATE_SCHEMA = os.environ.get("TEMPLATE_SCHEMA", "template0")
     _CLONE_SCHEMA_FUNC_FILENAME = os.path.join(find_db_functions_dir(), "clone_schema.sql")
     _CLONE_SCHEMA_FUNC_SCHEMA = "public"
-    _CLONE_SHEMA_FUNC_NAME = "clone_schema"
+    _CLONE_SCHEMA_FUNC_NAME = "clone_schema"
     _CLONE_SCHEMA_FUNC_SIG = (
-        f"{_CLONE_SCHEMA_FUNC_SCHEMA}.{_CLONE_SHEMA_FUNC_NAME}("
+        f"{_CLONE_SCHEMA_FUNC_SCHEMA}.{_CLONE_SCHEMA_FUNC_NAME}("
         "source_schema text, dest_schema text, "
         "copy_data boolean DEFAULT false, "
         "_verbose boolean DEFAULT false"
@@ -102,25 +101,25 @@ class Tenant(TenantMixin):
     auto_create_schema = False
 
     def _check_clone_func(self):
-        LOG.info(f'Verify that clone function "{self._CLONE_SCHEMA_FUNC_SIG}" exists')
+        LOG.info(f'Verify that clone function "{self._CLONE_SCHEMA_FUNC_NAME}" exists')
         res = dbfunc_exists(
-            conn, self._CLONE_SCHEMA_FUNC_SCHEMA, self._CLONE_SHEMA_FUNC_NAME, self._CLONE_SCHEMA_FUNC_SIG
+            conn, self._CLONE_SCHEMA_FUNC_SCHEMA, self._CLONE_SCHEMA_FUNC_NAME, self._CLONE_SCHEMA_FUNC_SIG
         )
         if not res:
-            LOG.warning(f'Clone function "{self._CLONE_SCHEMA_FUNC_SIG}" does not exist')
-            LOG.info(f'Creating clone function "{self._CLONE_SCHEMA_FUNC_SIG}"')
+            LOG.warning(f'Clone function "{self._CLONE_SCHEMA_FUNC_NAME}" does not exist')
+            LOG.info(f'Creating clone function "{self._CLONE_SCHEMA_FUNC_NAME}"')
             apply_sql_file(conn.schema_editor(), self._CLONE_SCHEMA_FUNC_FILENAME, literal_placeholder=True)
             res = dbfunc_exists(
-                conn, self._CLONE_SCHEMA_FUNC_SCHEMA, self._CLONE_SHEMA_FUNC_NAME, self._CLONE_SCHEMA_FUNC_SIG
+                conn, self._CLONE_SCHEMA_FUNC_SCHEMA, self._CLONE_SCHEMA_FUNC_NAME, self._CLONE_SCHEMA_FUNC_SIG
             )
         else:
-            LOG.info("Clone function exists")
+            LOG.info(f'Clone function "{self._CLONE_SCHEMA_FUNC_NAME}" exists. Not creating.')
 
         return res
 
     def _verify_template(self, verbosity=1):
         LOG.info(f'Verify that template schema "{self._TEMPLATE_SCHEMA}" exists')
-        # This is using the teanant table data as the source of truth which can be dangerous.
+        # This is using the tenant table data as the source of truth which can be dangerous.
         # If this becomes unreliable, then the database itself should be the source of truth
         # and extra code must be written to handle the sync of the table data to the state of
         # the database.

diff --git a/koku/api/iam/test/test_clone_schema.py b/koku/api/iam/test/test_clone_schema.py
@@ -16,18 +16,15 @@
 from koku.database import dbfunc_exists
 
 
-_CLONE_FUNC_SCHEMA = Tenant._CLONE_SCHEMA_FUNC_SCHEMA
-_CLONE_FUNC_SIG = Tenant._CLONE_SCHEMA_FUNC_SIG
-_CLONE_FUNC_NAME = Tenant._CLONE_SHEMA_FUNC_NAME
-
-
 def _verify_clone_func():
-    return dbfunc_exists(conn, _CLONE_FUNC_SCHEMA, _CLONE_FUNC_NAME, _CLONE_FUNC_SIG)
+    return dbfunc_exists(
+        conn, Tenant._CLONE_SCHEMA_FUNC_SCHEMA, Tenant._CLONE_SCHEMA_FUNC_NAME, Tenant._CLONE_SCHEMA_FUNC_SIG
+    )
 
 
 def _drop_clone_func():
     sql = f"""
-drop function if exists {_CLONE_FUNC_SIG.replace(' DEFAULT false', '')} ;
+drop function if exists {Tenant._CLONE_SCHEMA_FUNC_SIG.replace(' DEFAULT false', '')} ;
 """
     with conn.cursor() as cur:
         cur.execute(sql, None)
@@ -131,7 +128,6 @@ def test_tenant_object_delete_leaves_template(self):
         """
         cust_tenant = "acct90909093"
         self.assertFalse(schema_exists(cust_tenant))
-        self.assertTrue(schema_exists(Tenant._TEMPLATE_SCHEMA))
 
         t = Tenant(schema_name=cust_tenant)
         t.save()

diff --git a/koku/api/migrations/0062_update_clone_schema_function.py b/koku/api/migrations/0062_update_clone_schema_function.py
@@ -0,0 +1,24 @@
+# Generated by Django 4.1.13 on 2024-02-02 16:08
+import pathlib
+
+from django.db import migrations
+
+from koku.migration_sql_helpers import apply_sql_file
+from koku.migration_sql_helpers import find_db_functions_dir
+
+
+class Migration(migrations.Migration):
+    def update_clone_schema_function(apps, schema_editor):
+        path = pathlib.Path(find_db_functions_dir()) / "clone_schema.sql"
+        apply_sql_file(schema_editor, path, literal_placeholder=True)
+
+    dependencies = [
+        ("api", "0061_alter_providerinfrastructuremap_unique_together"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            code=update_clone_schema_function,
+            reverse_code=migrations.RunPython.noop,
+        )
+    ]
diff --git a/koku/api/provider/serializers.py b/koku/api/provider/serializers.py
@@ -360,7 +360,7 @@ def create(self, validated_data):
                 "Cost management does not allow duplicate accounts. "
                 "A source already exists with these details. Edit source settings to configure a new source."
             )
-            LOG.warn(message)
+            LOG.warning(message)
             raise serializers.ValidationError(error_obj(ProviderErrors.DUPLICATE_AUTH, message))
 
         provider = Provider.objects.create(**validated_data)
@@ -422,7 +422,7 @@ def update(self, instance, validated_data):
                         "Cost management does not allow duplicate accounts. "
                         "A source already exists with these details. Edit source settings to configure a new source."
                     )
-                    LOG.warn(message)
+                    LOG.warning(message)
                     raise serializers.ValidationError(error_obj(ProviderErrors.DUPLICATE_AUTH, message))
 
             for key in validated_data.keys():

diff --git a/koku/koku/migration_sql_helpers.py b/koku/koku/migration_sql_helpers.py
@@ -42,9 +42,12 @@ def apply_sql_file(conn, path, literal_placeholder=False):
     Returns:
         True
     """
-    sqlbuff = open(path).read()
+    with open(path) as file:
+        sqlbuff = file.read()
+
     if literal_placeholder:
         sqlbuff = sqlbuff.replace("%", "%%")
+
     if isinstance(conn, BaseDatabaseSchemaEditor):
         try:
             conn.execute(sqlbuff)

diff --git a/koku/masu/external/accounts/hierarchy/aws/aws_org_unit_crawler.py b/koku/masu/external/accounts/hierarchy/aws/aws_org_unit_crawler.py
@@ -62,9 +62,9 @@ def crawl_account_hierarchy(self):
                 if not self.errors_raised:
                     self._mark_nodes_deleted()
         except ParamValidationError as param_error:
-            LOG.warn(msg=error_message, exc_info=param_error)
+            LOG.warning(msg=error_message, exc_info=param_error)
         except ClientError as boto_error:
-            LOG.warn(msg=error_message, exc_info=boto_error)
+            LOG.warning(msg=error_message, exc_info=boto_error)
         except Exception as unknown_error:
             LOG.exception(msg=error_message, exc_info=unknown_error)