From 7f50679e68eb90ef2886fd02af02454d3fc62951 Mon Sep 17 00:00:00 2001
From: Alan Rominger <arominge@redhat.com>
Date: Wed, 15 Feb 2023 14:54:46 -0500
Subject: [PATCH] Do not create setting with invalid value in data migration
 (#13576)

* Do not create setting with invalid value in data migration

* Add test for conf app data migration
---
 awx/conf/migrations/_ldap_group_type.py      |  9 +++++--
 awx/conf/tests/functional/test_migrations.py | 25 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 awx/conf/tests/functional/test_migrations.py

diff --git a/awx/conf/migrations/_ldap_group_type.py b/awx/conf/migrations/_ldap_group_type.py
index b6580f8cae..378f934342 100644
--- a/awx/conf/migrations/_ldap_group_type.py
+++ b/awx/conf/migrations/_ldap_group_type.py
@@ -1,7 +1,11 @@
 import inspect
 
 from django.conf import settings
-from django.utils.timezone import now
+
+import logging
+
+
+logger = logging.getLogger('awx.conf.migrations')
 
 
 def fill_ldap_group_type_params(apps, schema_editor):
@@ -15,7 +19,7 @@ def fill_ldap_group_type_params(apps, schema_editor):
         entry = qs[0]
         group_type_params = entry.value
     else:
-        entry = Setting(key='AUTH_LDAP_GROUP_TYPE_PARAMS', value=group_type_params, created=now(), modified=now())
+        return  # for new installs we prefer to use the default value
 
     init_attrs = set(inspect.getfullargspec(group_type.__init__).args[1:])
     for k in list(group_type_params.keys()):
@@ -23,4 +27,5 @@ def fill_ldap_group_type_params(apps, schema_editor):
             del group_type_params[k]
 
     entry.value = group_type_params
+    logger.warning(f'Migration updating AUTH_LDAP_GROUP_TYPE_PARAMS with value {entry.value}')
     entry.save()
diff --git a/awx/conf/tests/functional/test_migrations.py b/awx/conf/tests/functional/test_migrations.py
new file mode 100644
index 0000000000..d3fddb292b
--- /dev/null
+++ b/awx/conf/tests/functional/test_migrations.py
@@ -0,0 +1,25 @@
+import pytest
+
+from awx.conf.migrations._ldap_group_type import fill_ldap_group_type_params
+from awx.conf.models import Setting
+
+from django.apps import apps
+
+
+@pytest.mark.django_db
+def test_fill_group_type_params_no_op():
+    fill_ldap_group_type_params(apps, 'dont-use-me')
+    assert Setting.objects.count() == 0
+
+
+@pytest.mark.django_db
+def test_keep_old_setting_with_default_value():
+    Setting.objects.create(key='AUTH_LDAP_GROUP_TYPE', value={'name_attr': 'cn', 'member_attr': 'member'})
+    fill_ldap_group_type_params(apps, 'dont-use-me')
+    assert Setting.objects.count() == 1
+    s = Setting.objects.first()
+    assert s.value == {'name_attr': 'cn', 'member_attr': 'member'}
+
+
+# NOTE: would be good to test the removal of attributes by migration
+# but this requires fighting with the validator and is not done here