From 6618c39b61e6afcaa05f5861b24702c5abe75dd5 Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Fri, 21 Mar 2025 17:56:40 -0400 Subject: [PATCH 01/12] WIP: Runtime feature flags --- ansible_base/feature_flags/apps.py | 19 +++++ .../feature_flags/migrations/0001_initial.py | 41 ++++++++++ ansible_base/feature_flags/models/__init__.py | 3 + ansible_base/feature_flags/models/aap_flag.py | 64 +++++++++++++++ ansible_base/feature_flags/serializers.py | 26 +++++- ansible_base/feature_flags/urls.py | 14 +++- ansible_base/feature_flags/utils.py | 79 ++++++++++++++++++- ansible_base/feature_flags/views.py | 67 +++++++++++++++- ansible_base/lib/dynamic_config/__init__.py | 2 + .../lib/dynamic_config/dynaconf_helpers.py | 16 ++++ .../lib/dynamic_config/dynamic_urls.py | 3 +- .../feature_flags/flag_source.py | 25 ++++++ .../feature_flags/platform_flags.py | 57 +++++++++++++ .../lib/dynamic_config/settings_logic.py | 2 + test_app/defaults.py | 3 + test_app/tests/feature_flags/test_api.py | 68 +++++----------- test_app/tests/feature_flags/test_old_api.py | 18 +++++ .../dynamic_config/test_dynaconf_settings.py | 13 +++ 18 files changed, 459 insertions(+), 61 deletions(-) create mode 100644 ansible_base/feature_flags/migrations/0001_initial.py create mode 100644 ansible_base/feature_flags/models/__init__.py create mode 100644 ansible_base/feature_flags/models/aap_flag.py create mode 100644 ansible_base/lib/dynamic_config/feature_flags/flag_source.py create mode 100644 ansible_base/lib/dynamic_config/feature_flags/platform_flags.py create mode 100644 test_app/tests/feature_flags/test_old_api.py diff --git a/ansible_base/feature_flags/apps.py b/ansible_base/feature_flags/apps.py index cfe665a64..11f6a574b 100644 --- a/ansible_base/feature_flags/apps.py +++ b/ansible_base/feature_flags/apps.py @@ -1,4 +1,8 @@ from django.apps import AppConfig +from django.db.models.signals import post_migrate +from django.db.utils import OperationalError, ProgrammingError + +from .utils import create_initial_data class FeatureFlagsConfig(AppConfig): @@ -6,3 +10,18 @@ class FeatureFlagsConfig(AppConfig): name = 'ansible_base.feature_flags' label = 'dab_feature_flags' verbose_name = 'Feature Flags' + + def ready(self): + from django.conf import settings + + if 'ansible_base.feature_flags' in settings.INSTALLED_APPS: + # TODO: Is there a better way to handle this logic? + + # If migrations are complete, attempt to load in feature flags again. + # This can help capture any updates to the platform flags loaded in to ensure that new values + # are added and updated. + # Otherwise wait for migrations to be complete before loading in feature flags. + try: + create_initial_data() + except (ProgrammingError, OperationalError): + post_migrate.connect(create_initial_data) diff --git a/ansible_base/feature_flags/migrations/0001_initial.py b/ansible_base/feature_flags/migrations/0001_initial.py new file mode 100644 index 000000000..caba03172 --- /dev/null +++ b/ansible_base/feature_flags/migrations/0001_initial.py @@ -0,0 +1,41 @@ +# Generated by Django 4.2.17 on 2025-03-21 13:04 + +import ansible_base.feature_flags.models.aap_flag +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='AAPFlag', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('modified', models.DateTimeField(auto_now=True, help_text='The date/time this resource was created.')), + ('created', models.DateTimeField(auto_now_add=True, help_text='The date/time this resource was created.')), + ('name', models.CharField(help_text='The name of the feature flag. Must follow the format of FEATURE__ENABLED.', max_length=64, validators=[ansible_base.feature_flags.models.aap_flag.validate_feature_flag_name])), + ('condition', models.CharField(default='boolean', help_text='Used to specify a condition, which if met, will enable the feature flag.', max_length=64)), + ('value', models.CharField(default='True', help_text='The value used to evaluate the conditional specified.', max_length=127)), + ('required', models.BooleanField(default=False, help_text="If multiple conditions are required to be met to enable a feature flag, 'required' can be used to specify the necessary conditionals.")), + ('support_level', models.CharField(choices=[('NOT_FOR_USE', 'Not for use'), ('NOT_FOR_PRODUCTION', 'Not for production'), ('READY_FOR_PRODUCTION', 'Ready for production')], help_text='The support criteria for the feature flag. Must be one of (NOT_FOR_USE, NOT_FOR_PRODUCTION, READY_FOR_PRODUCTION).', max_length=25)), + ('visibility', models.CharField(choices=[('public', 'public'), ('private', 'private')], help_text='The visibility level of the feature flag. If private, flag is hidden.', max_length=20)), + ('toggle_type', models.CharField(choices=[('install-time', 'install-time'), ('run-time', 'run-time')], default='run-time', help_text="Details whether a flag is toggle-able at run-time or install-time. (Default: 'run-time').", max_length=20)), + ('description', models.CharField(default='', help_text='A detailed description giving an overview of the feature flag.', max_length=300)), + ('version_added', models.CharField(help_text='The Ansible Automation Platform version the feature flag was added in.', max_length=30)), + ('labels', models.JSONField(blank=True, default=list, help_text='A list of labels for the feature flag.', null=True)), + ('created_by', models.ForeignKey(default=None, editable=False, help_text='The user who created this resource.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_created+', to=settings.AUTH_USER_MODEL)), + ('modified_by', models.ForeignKey(default=None, editable=False, help_text='The user who last modified this resource.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_modified+', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'unique_together': {('name', 'condition', 'value')}, + }, + ), + ] diff --git a/ansible_base/feature_flags/models/__init__.py b/ansible_base/feature_flags/models/__init__.py new file mode 100644 index 000000000..136cbf6b4 --- /dev/null +++ b/ansible_base/feature_flags/models/__init__.py @@ -0,0 +1,3 @@ +from .aap_flag import AAPFlag + +__all__ = ('AAPFlag',) diff --git a/ansible_base/feature_flags/models/aap_flag.py b/ansible_base/feature_flags/models/aap_flag.py new file mode 100644 index 000000000..08b5b5ea4 --- /dev/null +++ b/ansible_base/feature_flags/models/aap_flag.py @@ -0,0 +1,64 @@ +from django.core.exceptions import ValidationError +from django.db import models +from django.utils.translation import gettext_lazy as _ + +from ansible_base.lib.abstract_models.common import NamedCommonModel + + +def validate_feature_flag_name(value: str): + if not value.startswith('FEATURE_') or not value.endswith('_ENABLED'): + raise ValidationError(_("Feature flag names must follow the format of `FEATURE__ENABLED`")) + + +class AAPFlag(NamedCommonModel): + class Meta: + app_label = "dab_feature_flags" + unique_together = ("name", "condition", "value") + + def __str__(self): + return "{name} is enabled when {condition} is " "{value}{required}".format( + name=self.name, + condition=self.condition, + value=self.value, + required=" (required)" if self.required else "", + ) + + name = models.CharField( + max_length=64, + null=False, + help_text=_("The name of the feature flag. Must follow the format of FEATURE__ENABLED."), + validators=[validate_feature_flag_name], + blank=False, + ) + condition = models.CharField(max_length=64, default="boolean", help_text=_("Used to specify a condition, which if met, will enable the feature flag.")) + value = models.CharField(max_length=127, default="True", help_text=_("The value used to evaluate the conditional specified.")) + required = models.BooleanField( + default=False, + help_text=_("If multiple conditions are required to be met to enable a feature flag, 'required' can be used to specify the necessary conditionals."), + ) + support_level = models.CharField( + max_length=25, + null=False, + help_text=_("The support criteria for the feature flag. Must be one of (NOT_FOR_USE, NOT_FOR_PRODUCTION, READY_FOR_PRODUCTION)."), + choices=(('NOT_FOR_USE', 'Not for use'), ('NOT_FOR_PRODUCTION', 'Not for production'), ('READY_FOR_PRODUCTION', 'Ready for production')), + blank=False, + ) + visibility = models.CharField( + max_length=20, + null=False, + choices=[('public', 'public'), ('private', 'private')], + help_text=_("The visibility level of the feature flag. If private, flag is hidden."), + blank=False, + ) + toggle_type = models.CharField( + max_length=20, + null=False, + choices=[('install-time', 'install-time'), ('run-time', 'run-time')], + default='run-time', + help_text=_("Details whether a flag is toggle-able at run-time or install-time. (Default: 'run-time')."), + ) + description = models.CharField(max_length=300, null=False, default="", help_text=_("A detailed description giving an overview of the feature flag.")) + version_added = models.CharField( + max_length=30, null=False, help_text=_("The Ansible Automation Platform version the feature flag was added in."), blank=False + ) + labels = models.JSONField(null=True, default=list, help_text=_("A list of labels for the feature flag."), blank=True) diff --git a/ansible_base/feature_flags/serializers.py b/ansible_base/feature_flags/serializers.py index 6fc8f850b..0b1b98765 100644 --- a/ansible_base/feature_flags/serializers.py +++ b/ansible_base/feature_flags/serializers.py @@ -1,16 +1,36 @@ from flags.state import flag_state -from rest_framework import serializers + +from ansible_base.feature_flags.models import AAPFlag +from ansible_base.lib.serializers.common import NamedCommonModelSerializer from .utils import get_django_flags -class FeatureFlagSerializer(serializers.Serializer): +class FeatureFlagSerializer(NamedCommonModelSerializer): + """Serialize list of feature flags""" + + class Meta: + model = AAPFlag + fields = NamedCommonModelSerializer.Meta.fields + [x.name for x in AAPFlag._meta.concrete_fields] + read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "version_added", "labels"] + + def to_representation(self, instance): + ret = super().to_representation(instance) + return ret + + +# TODO: Remove once all components are migrated to the new endpont. +class OldFeatureFlagSerializer(NamedCommonModelSerializer): """Serialize list of feature flags""" + class Meta: + model = AAPFlag + fields = NamedCommonModelSerializer.Meta.fields + [x.name for x in AAPFlag._meta.concrete_fields] + read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "version_added", "labels"] + def to_representation(self) -> dict: return_data = {} feature_flags = get_django_flags() for feature_flag in feature_flags: return_data[feature_flag] = flag_state(feature_flag) - return return_data diff --git a/ansible_base/feature_flags/urls.py b/ansible_base/feature_flags/urls.py index a02ac0e3e..35c7c7061 100644 --- a/ansible_base/feature_flags/urls.py +++ b/ansible_base/feature_flags/urls.py @@ -1,12 +1,18 @@ -from django.urls import path +from django.urls import include, path from ansible_base.feature_flags import views from ansible_base.feature_flags.apps import FeatureFlagsConfig +from ansible_base.lib.routers import AssociationResourceRouter app_name = FeatureFlagsConfig.label -api_version_urls = [ - path('feature_flags_state/', views.FeatureFlagsStateListView.as_view(), name='feature-flags-state-list'), -] +router = AssociationResourceRouter() + +router.register(r'feature_flags/states', views.FeatureFlagsStatesView, basename='aap_flags_states') + +router.register(r'feature_flags', views.FeatureFlagsView, basename='aap_flags') + +# TODO: Remove once all components are migrated to new endpoints. +api_version_urls = [path('feature_flags_state/', views.OldFeatureFlagsStateListView.as_view(), name='feature-flags-state-list'), path('', include(router.urls))] api_urls = [] root_urls = [] diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index b419a151f..ca6e57617 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -1,5 +1,80 @@ -from django.conf import settings +from django.apps import apps +from django.core.exceptions import ValidationError +from flags.sources import get_flags + +from ansible_base.lib.dynamic_config.feature_flags.platform_flags import AAP_FEATURE_FLAGS def get_django_flags(): - return getattr(settings, 'FLAGS', {}) + return get_flags() + + +def is_boolean_str(val): + return val.lower() in {'true', 'false'} + + +def create_initial_data(**kwargs): + """ + Loads in platform feature flags when the server starts + """ + + from ansible_base.feature_flags.models.aap_flag import AAPFlag + + def update_feature_flag(existing: AAPFlag, new): + """ + Update only the required fields of the feature flag model. + This is used to ensure that flags can be loaded in when the server starts, with any applicable updates. + """ + + existing.support_level = new['support_level'] + existing.visibility = new['visibility'] + existing.version_added = new['version_added'] + if 'required' in new: + existing.required = new['required'] + if 'toggle_type' in new: + existing.toggle_type = new['toggle_type'] + if 'labels' in new: + existing.labels = new['labels'] + if 'description' in new: + existing.description = new['description'] + existing.save() + + def load_feature_flags(): + """ + Loads in all feature flags into the database. Updates them if necessary. + """ + FeatureFlags = apps.get_model('dab_feature_flags', 'AAPFlag') + for flag in AAP_FEATURE_FLAGS: + try: + existing_flag = FeatureFlags.objects.filter(name=flag['name'], condition=flag['condition']) + if existing_flag: + update_feature_flag(existing_flag.first(), flag) + else: + FeatureFlags.objects.create(**flag) + AAPFlag(**flag).full_clean() + except ValidationError as e: + # Ignore this error unless better way to bypass this + if e.messages[0] == 'Aap flag with this Name, Condition and Value already exists.': + pass + else: + # Raise row validation errors + raise e + + def delete_feature_flags(): + """ + If a feature flag has been removed from the platform flags list, delete it from the database. + """ + all_flags = apps.get_model('dab_feature_flags', 'AAPFlag').objects.all() + for flag in all_flags: + found = False + for _flag in AAP_FEATURE_FLAGS: + if flag.name == _flag['name'] and flag.condition == _flag['condition']: + found = True + continue + if found: + continue + if not found: + flag.delete() + + delete_feature_flags() + load_feature_flags() diff --git a/ansible_base/feature_flags/views.py b/ansible_base/feature_flags/views.py index 7eb93f28e..1a9078839 100644 --- a/ansible_base/feature_flags/views.py +++ b/ansible_base/feature_flags/views.py @@ -1,25 +1,84 @@ from django.conf import settings +from django.shortcuts import get_object_or_404 from django.utils.translation import gettext_lazy as _ +from flags.state import flag_enabled, flag_state, get_flags +from rest_framework import status from rest_framework.response import Response +from rest_framework.viewsets import ModelViewSet -from ansible_base.feature_flags.serializers import FeatureFlagSerializer +from ansible_base.feature_flags.models import AAPFlag +from ansible_base.feature_flags.serializers import FeatureFlagSerializer, OldFeatureFlagSerializer from ansible_base.lib.utils.views.ansible_base import AnsibleBaseView +from ansible_base.lib.utils.views.django_app_api import AnsibleBaseDjangoAppApiView +from ansible_base.lib.utils.views.permissions import IsSuperuserOrAuditor +from ansible_base.rest_pagination import DefaultPaginator -from .utils import get_django_flags +from .utils import get_django_flags, is_boolean_str -class FeatureFlagsStateListView(AnsibleBaseView): +class FeatureFlagsStatesView(AnsibleBaseDjangoAppApiView, ModelViewSet): + """ + A view class for displaying feature flags states + """ + + queryset = AAPFlag.objects.order_by('id') + permission_classes = [IsSuperuserOrAuditor] + http_method_names = ['get', 'head', 'options'] + + def list(self, request): + paginator = DefaultPaginator() + flags = get_flags() + ret = [] + for flag in flags: + ret.append({"flag_name": flag, "flag_state": flag_state(flag)}) + result_page = paginator.paginate_queryset(ret, request) + return paginator.get_paginated_response(result_page) + + +class FeatureFlagsView(AnsibleBaseDjangoAppApiView, ModelViewSet): """ A view class for displaying feature flags """ + queryset = AAPFlag.objects.order_by('id') serializer_class = FeatureFlagSerializer + permission_classes = [IsSuperuserOrAuditor] + http_method_names = ['get', 'put', 'head', 'options'] + + def update(self, request, **kwargs): + _feature_flag = self.get_object() + value = request.data.get('value') + if not value: + return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Invalid request object."}) + + # Disable runtime toggle if the feature flag feature is not enabled + if not flag_enabled('FEATURE_FEATURE_FLAGS_ENABLED'): + return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data={"details": "Runtime feature flags toggle is not enabled."}) + + feature_flag = get_object_or_404(AAPFlag, pk=_feature_flag.id) + if feature_flag.toggle_type == 'install-time': + return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data={"details": "Install-time feature flags cannot be toggled at run-time."}) + if feature_flag.condition == "boolean" and not is_boolean_str(value): + return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Feature flag boolean conditional requires using boolean value."}) + feature_flag.value = value + feature_flag.save() + + return Response(self.get_serializer().to_representation(feature_flag)) + + +# TODO: This can be removed after functionality is migrated over to new class +class OldFeatureFlagsStateListView(AnsibleBaseView): + """ + A view class for displaying feature flags + """ + + serializer_class = OldFeatureFlagSerializer filter_backends = [] name = _('Feature Flags') http_method_names = ['get', 'head'] def _get(self, request, format=None): - self.serializer = FeatureFlagSerializer() + self.serializer = OldFeatureFlagSerializer() return Response(self.serializer.to_representation()) def get_queryset(self): diff --git a/ansible_base/lib/dynamic_config/__init__.py b/ansible_base/lib/dynamic_config/__init__.py index 8aa76b6e2..f11e5a97c 100644 --- a/ansible_base/lib/dynamic_config/__init__.py +++ b/ansible_base/lib/dynamic_config/__init__.py @@ -5,6 +5,7 @@ load_envvars, load_python_file_with_injected_context, load_standard_settings_files, + toggle_database_feature_flags, toggle_feature_flags, validate, ) @@ -17,5 +18,6 @@ "load_python_file_with_injected_context", "load_standard_settings_files", "toggle_feature_flags", + "toggle_database_feature_flags", "validate", ] diff --git a/ansible_base/lib/dynamic_config/dynaconf_helpers.py b/ansible_base/lib/dynamic_config/dynaconf_helpers.py index a3eb7d378..c36fc24bd 100644 --- a/ansible_base/lib/dynamic_config/dynaconf_helpers.py +++ b/ansible_base/lib/dynamic_config/dynaconf_helpers.py @@ -15,6 +15,8 @@ from dynaconf.loaders.yaml_loader import yaml from dynaconf.utils.files import glob from dynaconf.utils.functional import empty +from flags.sources import get_flags +from flags.state import disable_flag, enable_flag from ansible_base.lib.dynamic_config.settings_logic import get_mergeable_dab_settings @@ -315,3 +317,17 @@ def toggle_feature_flags(settings: Dynaconf) -> dict[str, Any]: feature_content[0]["value"] = installer_value data[f"FLAGS__{feature_name}"] = feature_content return data + + +def toggle_database_feature_flags(settings: Dynaconf) -> dict[str, Any]: + """Toggle FLAGS based on installer settings. + FLAGS is a django-flags formatted dictionary. + Installers will place `FEATURE_SOME_PLATFORM_FLAG_ENABLED=True/False` in the settings file. + This function will update the value in the database with the expected boolean value + """ + for feature_name in get_flags(): + if (installer_value := settings.get(feature_name, empty)) is not empty: + if installer_value: + enable_flag(feature_name) + else: + disable_flag(feature_name) diff --git a/ansible_base/lib/dynamic_config/dynamic_urls.py b/ansible_base/lib/dynamic_config/dynamic_urls.py index 420b6623f..f32287c50 100644 --- a/ansible_base/lib/dynamic_config/dynamic_urls.py +++ b/ansible_base/lib/dynamic_config/dynamic_urls.py @@ -11,8 +11,9 @@ globals()[url_type] = [] installed_apps = getattr(settings, 'INSTALLED_APPS', []) +installed_apps_omit_urls = getattr(settings, 'INSTALLED_APPS_OMIT_URLS', []) for app in installed_apps: - if app.startswith('ansible_base.'): + if app.startswith('ansible_base.') and app not in installed_apps_omit_urls: if not importlib.util.find_spec(f'{app}.urls'): logger.debug(f'Module {app} does not specify urls.py') continue diff --git a/ansible_base/lib/dynamic_config/feature_flags/flag_source.py b/ansible_base/lib/dynamic_config/feature_flags/flag_source.py new file mode 100644 index 000000000..2c6e3f236 --- /dev/null +++ b/ansible_base/lib/dynamic_config/feature_flags/flag_source.py @@ -0,0 +1,25 @@ +from django.apps import apps +from flags.sources import Condition + + +class DatabaseCondition(Condition): + """Condition that includes the FlagState database object""" + + def __init__(self, condition, value, required=False, obj=None): + super().__init__(condition, value, required=required) + self.obj = obj + + +class AAPFlagSource(object): + + def get_queryset(self): + FlagState = apps.get_model('dab_feature_flags', 'AAPFlag') + return FlagState.objects.all() + + def get_flags(self): + flags = {} + for o in self.get_queryset(): + if o.name not in flags: + flags[o.name] = [] + flags[o.name].append(DatabaseCondition(o.condition, o.value, required=o.required, obj=o)) + return flags diff --git a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py new file mode 100644 index 000000000..865597839 --- /dev/null +++ b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py @@ -0,0 +1,57 @@ +from typing import TypedDict + + +class AAPFlagNameSchema(TypedDict): + name: str + support_level: str + visibility: str + toggle_type: str + description: str + version_added: str + labels: list + + +AAP_FEATURE_FLAGS: list[AAPFlagNameSchema] = [ + AAPFlagNameSchema( + name="FEATURE_FEATURE_FLAGS_ENABLED", + condition="boolean", + value=True, + visibility="public", + support_level='READY_FOR_PRODUCTION', + description='If enabled, feature flags can be toggled on/off at runtime via UI or API. ' + 'If disabled, feature flags can only be toggled on/off at install-time.', + version_added='2.5', + labels=['platform'], + toggle_type='install-time', + ), + AAPFlagNameSchema( + name="FEATURE_INDIRECT_NODE_COUNTING_ENABLED", + visibility="public", + condition="boolean", + value=False, + support_level="NOT_FOR_PRODUCTION", + description="TBD", + version_added="2.5", + labels=['controller'], + ), + AAPFlagNameSchema( + name="FEATURE_POLICY_AS_CODE_ENABLED", + visibility="public", + condition="boolean", + value=False, + support_level="NOT_FOR_PRODUCTION", + description="TBD", + version_added="2.5", + labels=['controller'], + ), + AAPFlagNameSchema( + name="FEATURE_EDA_ANALYTICS_ENABLED", + condition="boolean", + value=False, + visibility="public", + support_level="NOT_FOR_PRODUCTION", + description="TBD", + version_added="2.5", + labels=['eda'], + ), +] diff --git a/ansible_base/lib/dynamic_config/settings_logic.py b/ansible_base/lib/dynamic_config/settings_logic.py index 2a093d858..b2bbd55ce 100644 --- a/ansible_base/lib/dynamic_config/settings_logic.py +++ b/ansible_base/lib/dynamic_config/settings_logic.py @@ -306,6 +306,8 @@ def get_mergeable_dab_settings(settings: dict) -> dict: # NOSONAR if "flags" not in installed_apps: installed_apps.append('flags') + dab_data['FLAG_SOURCES'] = ('ansible_base.lib.dynamic_config.feature_flags.flag_source.AAPFlagSource',) + found_template_backend = False template_context_processor = 'django.template.context_processors.request' # Look through all of the tmplates diff --git a/test_app/defaults.py b/test_app/defaults.py index ba8be1ee9..f6009437a 100644 --- a/test_app/defaults.py +++ b/test_app/defaults.py @@ -68,6 +68,9 @@ 'ansible_base.feature_flags', ] +# Specified to control which apps from ansible_base can expose URLs +INSTALLED_APPS_OMIT_URLS = [] + MIDDLEWARE = [ 'debug_toolbar.middleware.DebugToolbarMiddleware', 'django.middleware.security.SecurityMiddleware', diff --git a/test_app/tests/feature_flags/test_api.py b/test_app/tests/feature_flags/test_api.py index df71cd20c..4d8c6929f 100644 --- a/test_app/tests/feature_flags/test_api.py +++ b/test_app/tests/feature_flags/test_api.py @@ -1,57 +1,31 @@ -from django.test import override_settings +import pytest from rest_framework.test import APIClient from ansible_base.lib.utils.response import get_relative_url -def test_feature_flags_state_api_list(admin_api_client: APIClient): - """ - Test that we can list all feature flags - """ - url = get_relative_url("feature-flags-state-list") - response = admin_api_client.get(url) - assert response.status_code == 200 - assert 'FEATURE_SOME_PLATFORM_FLAG_ENABLED' in response.data - assert response.data["FEATURE_SOME_PLATFORM_FLAG_ENABLED"] is False - assert 'FEATURE_SOME_PLATFORM_FLAG_FOO_ENABLED' in response.data - assert response.data["FEATURE_SOME_PLATFORM_FLAG_FOO_ENABLED"] is False - assert 'FEATURE_SOME_PLATFORM_FLAG_BAR_ENABLED' in response.data - assert response.data["FEATURE_SOME_PLATFORM_FLAG_BAR_ENABLED"] is True - - -@override_settings( - FLAGS={ - "FEATURE_SOME_PLATFORM_OVERRIDE_ENABLED": [ - {"condition": "boolean", "value": False}, - {"condition": "before date", "value": "2022-06-01T12:00Z"}, - ], - "FEATURE_SOME_PLATFORM_OVERRIDE_TRUE_ENABLED": [ - {"condition": "boolean", "value": True}, - ], - } +@pytest.mark.parametrize( + 'feature_flag_name, feature_flag_value', + [ + ("FEATURE_FEATURE_FLAGS_ENABLED", True), + ("FEATURE_INDIRECT_NODE_COUNTING_ENABLED", False), + ("FEATURE_POLICY_AS_CODE_ENABLED", False), + ("FEATURE_EDA_ANALYTICS_ENABLED", False), + ], ) -def test_feature_flags_state_api_list_settings_override(admin_api_client: APIClient): - """ - Test that we can list all feature flags - """ - url = get_relative_url("feature-flags-state-list") - response = admin_api_client.get(url) - assert response.status_code == 200 - assert 'FEATURE_SOME_PLATFORM_FLAG_ENABLED' not in response.data - assert 'FEATURE_SOME_PLATFORM_FLAG_FOO_ENABLED' not in response.data - assert 'FEATURE_SOME_PLATFORM_FLAG_BAR_ENABLED' not in response.data - assert 'FEATURE_SOME_PLATFORM_OVERRIDE_ENABLED' in response.data - assert response.data["FEATURE_SOME_PLATFORM_OVERRIDE_ENABLED"] is False - assert 'FEATURE_SOME_PLATFORM_OVERRIDE_TRUE_ENABLED' in response.data - assert response.data["FEATURE_SOME_PLATFORM_OVERRIDE_TRUE_ENABLED"] is True - - -@override_settings(FLAGS={}) -def test_feature_flags_state_api_list_settings_override_empty(admin_api_client: APIClient): +def test_feature_flags_states_api_list(admin_api_client: APIClient, feature_flag_name: str, feature_flag_value: str): """ - Test that we can list all feature flags + Test that we can list all feature flags and their states """ - url = get_relative_url("feature-flags-state-list") + url = get_relative_url("aap_flags_states-list") response = admin_api_client.get(url) assert response.status_code == 200 - assert response.data == {} + # check total amount of flags + assert response.data["count"] == 4 + results = response.data['results'] + found = False + for result in results: + if feature_flag_name == result['flag_name']: + found = True + assert result['flag_state'] is feature_flag_value + assert found is True diff --git a/test_app/tests/feature_flags/test_old_api.py b/test_app/tests/feature_flags/test_old_api.py new file mode 100644 index 000000000..f39ab3804 --- /dev/null +++ b/test_app/tests/feature_flags/test_old_api.py @@ -0,0 +1,18 @@ +from rest_framework.test import APIClient + +from ansible_base.lib.utils.response import get_relative_url + + +def test_feature_flags_state_api_list(admin_api_client: APIClient): + """ + Test that we can list all feature flags + """ + url = get_relative_url("featureflags-list") + response = admin_api_client.get(url) + assert response.status_code == 200 + assert 'FEATURE_EDA_ANALYTICS_ENABLED' in response.data + assert response.data["FEATURE_EDA_ANALYTICS_ENABLED"] is False + assert 'FEATURE_INDIRECT_NODE_COUNTING_ENABLED' in response.data + assert response.data["FEATURE_INDIRECT_NODE_COUNTING_ENABLED"] is False + assert 'FEATURE_FEATURE_FLAGS_ENABLED' in response.data + assert response.data["FEATURE_FEATURE_FLAGS_ENABLED"] is True diff --git a/test_app/tests/lib/dynamic_config/test_dynaconf_settings.py b/test_app/tests/lib/dynamic_config/test_dynaconf_settings.py index 53c6dfa60..acf6ea688 100644 --- a/test_app/tests/lib/dynamic_config/test_dynaconf_settings.py +++ b/test_app/tests/lib/dynamic_config/test_dynaconf_settings.py @@ -1,4 +1,6 @@ +import pytest from dynaconf import ValidationError, Validator +from flags.state import flag_state from ansible_base.lib.dynamic_config import ( export, @@ -6,6 +8,7 @@ load_envvars, load_python_file_with_injected_context, load_standard_settings_files, + toggle_database_feature_flags, toggle_feature_flags, validate, ) @@ -265,3 +268,13 @@ def test_toggle_feature_flags(): {"condition": "before date", "value": "2022-06-01T12:00Z"}, ] } + + +@pytest.mark.django_db +def test_toggle_database_feature_flags(): + """Ensure that the toggle_feature_flags function works as expected.""" + + settings = {"FEATURE_EDA_ANALYTICS_ENABLED": True, "FEATURE_POLICY_AS_CODE_ENABLED": False} + toggle_database_feature_flags(settings) + assert flag_state("FEATURE_EDA_ANALYTICS_ENABLED") is True + assert flag_state("FEATURE_POLICY_AS_CODE_ENABLED") is False From 559a2d6a07ec0e405482ad1535dd27dc48290b5c Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Mon, 24 Mar 2025 18:26:22 -0400 Subject: [PATCH 02/12] attempt moving apis to gw --- .../feature_flags/migrations/0001_initial.py | 3 +- ansible_base/feature_flags/models/aap_flag.py | 9 +-- ansible_base/feature_flags/serializers.py | 27 ++------ ansible_base/feature_flags/utils.py | 1 - ansible_base/feature_flags/views.py | 67 ++----------------- ansible_base/lib/dynamic_config/__init__.py | 2 - .../lib/dynamic_config/dynaconf_helpers.py | 16 ----- .../lib/dynamic_config/dynamic_urls.py | 3 +- .../feature_flags/platform_flags.py | 19 +++--- .../resource_registry/shared_types.py | 21 ++++++ test_app/defaults.py | 3 - .../management/test_authenticators.py | 3 +- test_app/tests/feature_flags/test_api.py | 31 --------- .../dynamic_config/test_dynaconf_settings.py | 13 ---- .../lib/utils/test_create_system_user.py | 5 +- 15 files changed, 49 insertions(+), 174 deletions(-) delete mode 100644 test_app/tests/feature_flags/test_api.py diff --git a/ansible_base/feature_flags/migrations/0001_initial.py b/ansible_base/feature_flags/migrations/0001_initial.py index caba03172..a6b7417b4 100644 --- a/ansible_base/feature_flags/migrations/0001_initial.py +++ b/ansible_base/feature_flags/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.17 on 2025-03-21 13:04 +# Generated by Django 4.2.17 on 2025-03-26 17:52 import ansible_base.feature_flags.models.aap_flag from django.conf import settings @@ -29,7 +29,6 @@ class Migration(migrations.Migration): ('visibility', models.CharField(choices=[('public', 'public'), ('private', 'private')], help_text='The visibility level of the feature flag. If private, flag is hidden.', max_length=20)), ('toggle_type', models.CharField(choices=[('install-time', 'install-time'), ('run-time', 'run-time')], default='run-time', help_text="Details whether a flag is toggle-able at run-time or install-time. (Default: 'run-time').", max_length=20)), ('description', models.CharField(default='', help_text='A detailed description giving an overview of the feature flag.', max_length=300)), - ('version_added', models.CharField(help_text='The Ansible Automation Platform version the feature flag was added in.', max_length=30)), ('labels', models.JSONField(blank=True, default=list, help_text='A list of labels for the feature flag.', null=True)), ('created_by', models.ForeignKey(default=None, editable=False, help_text='The user who created this resource.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_created+', to=settings.AUTH_USER_MODEL)), ('modified_by', models.ForeignKey(default=None, editable=False, help_text='The user who last modified this resource.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_modified+', to=settings.AUTH_USER_MODEL)), diff --git a/ansible_base/feature_flags/models/aap_flag.py b/ansible_base/feature_flags/models/aap_flag.py index 08b5b5ea4..86aa6654d 100644 --- a/ansible_base/feature_flags/models/aap_flag.py +++ b/ansible_base/feature_flags/models/aap_flag.py @@ -2,7 +2,9 @@ from django.db import models from django.utils.translation import gettext_lazy as _ +from ansible_base.activitystream.models import AuditableModel from ansible_base.lib.abstract_models.common import NamedCommonModel +from ansible_base.resource_registry.fields import AnsibleResourceField def validate_feature_flag_name(value: str): @@ -10,7 +12,7 @@ def validate_feature_flag_name(value: str): raise ValidationError(_("Feature flag names must follow the format of `FEATURE__ENABLED`")) -class AAPFlag(NamedCommonModel): +class AAPFlag(NamedCommonModel, AuditableModel): class Meta: app_label = "dab_feature_flags" unique_together = ("name", "condition", "value") @@ -23,6 +25,8 @@ def __str__(self): required=" (required)" if self.required else "", ) + resource = AnsibleResourceField(primary_key_field="id") + name = models.CharField( max_length=64, null=False, @@ -58,7 +62,4 @@ def __str__(self): help_text=_("Details whether a flag is toggle-able at run-time or install-time. (Default: 'run-time')."), ) description = models.CharField(max_length=300, null=False, default="", help_text=_("A detailed description giving an overview of the feature flag.")) - version_added = models.CharField( - max_length=30, null=False, help_text=_("The Ansible Automation Platform version the feature flag was added in."), blank=False - ) labels = models.JSONField(null=True, default=list, help_text=_("A list of labels for the feature flag."), blank=True) diff --git a/ansible_base/feature_flags/serializers.py b/ansible_base/feature_flags/serializers.py index 0b1b98765..e3c5e304d 100644 --- a/ansible_base/feature_flags/serializers.py +++ b/ansible_base/feature_flags/serializers.py @@ -1,36 +1,17 @@ from flags.state import flag_state - -from ansible_base.feature_flags.models import AAPFlag -from ansible_base.lib.serializers.common import NamedCommonModelSerializer +from rest_framework import serializers from .utils import get_django_flags -class FeatureFlagSerializer(NamedCommonModelSerializer): - """Serialize list of feature flags""" - - class Meta: - model = AAPFlag - fields = NamedCommonModelSerializer.Meta.fields + [x.name for x in AAPFlag._meta.concrete_fields] - read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "version_added", "labels"] - - def to_representation(self, instance): - ret = super().to_representation(instance) - return ret - - -# TODO: Remove once all components are migrated to the new endpont. -class OldFeatureFlagSerializer(NamedCommonModelSerializer): +# TODO: This view and its serializer can be removed after functionality is migrated over to new class +class FeatureFlagSerializer(serializers.Serializer): """Serialize list of feature flags""" - class Meta: - model = AAPFlag - fields = NamedCommonModelSerializer.Meta.fields + [x.name for x in AAPFlag._meta.concrete_fields] - read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "version_added", "labels"] - def to_representation(self) -> dict: return_data = {} feature_flags = get_django_flags() for feature_flag in feature_flags: return_data[feature_flag] = flag_state(feature_flag) + return return_data diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index ca6e57617..c59f95013 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -28,7 +28,6 @@ def update_feature_flag(existing: AAPFlag, new): existing.support_level = new['support_level'] existing.visibility = new['visibility'] - existing.version_added = new['version_added'] if 'required' in new: existing.required = new['required'] if 'toggle_type' in new: diff --git a/ansible_base/feature_flags/views.py b/ansible_base/feature_flags/views.py index 1a9078839..196765f41 100644 --- a/ansible_base/feature_flags/views.py +++ b/ansible_base/feature_flags/views.py @@ -1,84 +1,27 @@ from django.conf import settings from django.shortcuts import get_object_or_404 from django.utils.translation import gettext_lazy as _ -from flags.state import flag_enabled, flag_state, get_flags -from rest_framework import status from rest_framework.response import Response -from rest_framework.viewsets import ModelViewSet -from ansible_base.feature_flags.models import AAPFlag -from ansible_base.feature_flags.serializers import FeatureFlagSerializer, OldFeatureFlagSerializer +from ansible_base.feature_flags.serializers import FeatureFlagSerializer from ansible_base.lib.utils.views.ansible_base import AnsibleBaseView -from ansible_base.lib.utils.views.django_app_api import AnsibleBaseDjangoAppApiView -from ansible_base.lib.utils.views.permissions import IsSuperuserOrAuditor -from ansible_base.rest_pagination import DefaultPaginator -from .utils import get_django_flags, is_boolean_str +from .utils import get_django_flags -class FeatureFlagsStatesView(AnsibleBaseDjangoAppApiView, ModelViewSet): - """ - A view class for displaying feature flags states - """ - - queryset = AAPFlag.objects.order_by('id') - permission_classes = [IsSuperuserOrAuditor] - http_method_names = ['get', 'head', 'options'] - - def list(self, request): - paginator = DefaultPaginator() - flags = get_flags() - ret = [] - for flag in flags: - ret.append({"flag_name": flag, "flag_state": flag_state(flag)}) - result_page = paginator.paginate_queryset(ret, request) - return paginator.get_paginated_response(result_page) - - -class FeatureFlagsView(AnsibleBaseDjangoAppApiView, ModelViewSet): +# TODO: This view and its serializer can be removed after functionality is migrated over to new class +class FeatureFlagsStateListView(AnsibleBaseView): """ A view class for displaying feature flags """ - queryset = AAPFlag.objects.order_by('id') serializer_class = FeatureFlagSerializer - permission_classes = [IsSuperuserOrAuditor] - http_method_names = ['get', 'put', 'head', 'options'] - - def update(self, request, **kwargs): - _feature_flag = self.get_object() - value = request.data.get('value') - if not value: - return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Invalid request object."}) - - # Disable runtime toggle if the feature flag feature is not enabled - if not flag_enabled('FEATURE_FEATURE_FLAGS_ENABLED'): - return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data={"details": "Runtime feature flags toggle is not enabled."}) - - feature_flag = get_object_or_404(AAPFlag, pk=_feature_flag.id) - if feature_flag.toggle_type == 'install-time': - return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data={"details": "Install-time feature flags cannot be toggled at run-time."}) - if feature_flag.condition == "boolean" and not is_boolean_str(value): - return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Feature flag boolean conditional requires using boolean value."}) - feature_flag.value = value - feature_flag.save() - - return Response(self.get_serializer().to_representation(feature_flag)) - - -# TODO: This can be removed after functionality is migrated over to new class -class OldFeatureFlagsStateListView(AnsibleBaseView): - """ - A view class for displaying feature flags - """ - - serializer_class = OldFeatureFlagSerializer filter_backends = [] name = _('Feature Flags') http_method_names = ['get', 'head'] def _get(self, request, format=None): - self.serializer = OldFeatureFlagSerializer() + self.serializer = FeatureFlagSerializer() return Response(self.serializer.to_representation()) def get_queryset(self): diff --git a/ansible_base/lib/dynamic_config/__init__.py b/ansible_base/lib/dynamic_config/__init__.py index f11e5a97c..8aa76b6e2 100644 --- a/ansible_base/lib/dynamic_config/__init__.py +++ b/ansible_base/lib/dynamic_config/__init__.py @@ -5,7 +5,6 @@ load_envvars, load_python_file_with_injected_context, load_standard_settings_files, - toggle_database_feature_flags, toggle_feature_flags, validate, ) @@ -18,6 +17,5 @@ "load_python_file_with_injected_context", "load_standard_settings_files", "toggle_feature_flags", - "toggle_database_feature_flags", "validate", ] diff --git a/ansible_base/lib/dynamic_config/dynaconf_helpers.py b/ansible_base/lib/dynamic_config/dynaconf_helpers.py index c36fc24bd..a3eb7d378 100644 --- a/ansible_base/lib/dynamic_config/dynaconf_helpers.py +++ b/ansible_base/lib/dynamic_config/dynaconf_helpers.py @@ -15,8 +15,6 @@ from dynaconf.loaders.yaml_loader import yaml from dynaconf.utils.files import glob from dynaconf.utils.functional import empty -from flags.sources import get_flags -from flags.state import disable_flag, enable_flag from ansible_base.lib.dynamic_config.settings_logic import get_mergeable_dab_settings @@ -317,17 +315,3 @@ def toggle_feature_flags(settings: Dynaconf) -> dict[str, Any]: feature_content[0]["value"] = installer_value data[f"FLAGS__{feature_name}"] = feature_content return data - - -def toggle_database_feature_flags(settings: Dynaconf) -> dict[str, Any]: - """Toggle FLAGS based on installer settings. - FLAGS is a django-flags formatted dictionary. - Installers will place `FEATURE_SOME_PLATFORM_FLAG_ENABLED=True/False` in the settings file. - This function will update the value in the database with the expected boolean value - """ - for feature_name in get_flags(): - if (installer_value := settings.get(feature_name, empty)) is not empty: - if installer_value: - enable_flag(feature_name) - else: - disable_flag(feature_name) diff --git a/ansible_base/lib/dynamic_config/dynamic_urls.py b/ansible_base/lib/dynamic_config/dynamic_urls.py index f32287c50..420b6623f 100644 --- a/ansible_base/lib/dynamic_config/dynamic_urls.py +++ b/ansible_base/lib/dynamic_config/dynamic_urls.py @@ -11,9 +11,8 @@ globals()[url_type] = [] installed_apps = getattr(settings, 'INSTALLED_APPS', []) -installed_apps_omit_urls = getattr(settings, 'INSTALLED_APPS_OMIT_URLS', []) for app in installed_apps: - if app.startswith('ansible_base.') and app not in installed_apps_omit_urls: + if app.startswith('ansible_base.'): if not importlib.util.find_spec(f'{app}.urls'): logger.debug(f'Module {app} does not specify urls.py') continue diff --git a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py index 865597839..dc03688dc 100644 --- a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py +++ b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py @@ -3,24 +3,24 @@ class AAPFlagNameSchema(TypedDict): name: str - support_level: str + condition: str + value: str visibility: str - toggle_type: str + support_level: str description: str - version_added: str labels: list + toggle_type: str AAP_FEATURE_FLAGS: list[AAPFlagNameSchema] = [ AAPFlagNameSchema( name="FEATURE_FEATURE_FLAGS_ENABLED", condition="boolean", - value=True, + value="True", visibility="public", support_level='READY_FOR_PRODUCTION', description='If enabled, feature flags can be toggled on/off at runtime via UI or API. ' 'If disabled, feature flags can only be toggled on/off at install-time.', - version_added='2.5', labels=['platform'], toggle_type='install-time', ), @@ -28,30 +28,27 @@ class AAPFlagNameSchema(TypedDict): name="FEATURE_INDIRECT_NODE_COUNTING_ENABLED", visibility="public", condition="boolean", - value=False, + value="False", support_level="NOT_FOR_PRODUCTION", description="TBD", - version_added="2.5", labels=['controller'], ), AAPFlagNameSchema( name="FEATURE_POLICY_AS_CODE_ENABLED", visibility="public", condition="boolean", - value=False, + value="False", support_level="NOT_FOR_PRODUCTION", description="TBD", - version_added="2.5", labels=['controller'], ), AAPFlagNameSchema( name="FEATURE_EDA_ANALYTICS_ENABLED", condition="boolean", - value=False, + value="False", visibility="public", support_level="NOT_FOR_PRODUCTION", description="TBD", - version_added="2.5", labels=['eda'], ), ] diff --git a/ansible_base/resource_registry/shared_types.py b/ansible_base/resource_registry/shared_types.py index 6b1350589..4289604a2 100644 --- a/ansible_base/resource_registry/shared_types.py +++ b/ansible_base/resource_registry/shared_types.py @@ -75,3 +75,24 @@ class TeamType(SharedResourceTypeSerializer): default="", allow_blank=True, ) + + +class FeatureFlagType(SharedResourceTypeSerializer): + """Serialize list of feature flags""" + + RESOURCE_TYPE = "feature_flag" + UNIQUE_FIELDS = ( + "name", + "condition", + "value", + ) + + name = serializers.CharField() + condition = serializers.CharField() + value = serializers.CharField() + required = serializers.BooleanField() + support_level = serializers.CharField() + visibility = serializers.CharField() + toggle_type = serializers.CharField() + description = serializers.CharField() + labels = serializers.JSONField() diff --git a/test_app/defaults.py b/test_app/defaults.py index f6009437a..ba8be1ee9 100644 --- a/test_app/defaults.py +++ b/test_app/defaults.py @@ -68,9 +68,6 @@ 'ansible_base.feature_flags', ] -# Specified to control which apps from ansible_base can expose URLs -INSTALLED_APPS_OMIT_URLS = [] - MIDDLEWARE = [ 'debug_toolbar.middleware.DebugToolbarMiddleware', 'django.middleware.security.SecurityMiddleware', diff --git a/test_app/tests/authentication/management/test_authenticators.py b/test_app/tests/authentication/management/test_authenticators.py index b2fc302d4..930dd2218 100644 --- a/test_app/tests/authentication/management/test_authenticators.py +++ b/test_app/tests/authentication/management/test_authenticators.py @@ -97,7 +97,8 @@ def test_authenticators_cli_initialize( err = StringIO() # Sanity check: - assert django_user_model.objects.count() == 0 + # _system user exists + assert django_user_model.objects.count() == 1 # Optionally create admin user if admin_user_exists: diff --git a/test_app/tests/feature_flags/test_api.py b/test_app/tests/feature_flags/test_api.py deleted file mode 100644 index 4d8c6929f..000000000 --- a/test_app/tests/feature_flags/test_api.py +++ /dev/null @@ -1,31 +0,0 @@ -import pytest -from rest_framework.test import APIClient - -from ansible_base.lib.utils.response import get_relative_url - - -@pytest.mark.parametrize( - 'feature_flag_name, feature_flag_value', - [ - ("FEATURE_FEATURE_FLAGS_ENABLED", True), - ("FEATURE_INDIRECT_NODE_COUNTING_ENABLED", False), - ("FEATURE_POLICY_AS_CODE_ENABLED", False), - ("FEATURE_EDA_ANALYTICS_ENABLED", False), - ], -) -def test_feature_flags_states_api_list(admin_api_client: APIClient, feature_flag_name: str, feature_flag_value: str): - """ - Test that we can list all feature flags and their states - """ - url = get_relative_url("aap_flags_states-list") - response = admin_api_client.get(url) - assert response.status_code == 200 - # check total amount of flags - assert response.data["count"] == 4 - results = response.data['results'] - found = False - for result in results: - if feature_flag_name == result['flag_name']: - found = True - assert result['flag_state'] is feature_flag_value - assert found is True diff --git a/test_app/tests/lib/dynamic_config/test_dynaconf_settings.py b/test_app/tests/lib/dynamic_config/test_dynaconf_settings.py index acf6ea688..53c6dfa60 100644 --- a/test_app/tests/lib/dynamic_config/test_dynaconf_settings.py +++ b/test_app/tests/lib/dynamic_config/test_dynaconf_settings.py @@ -1,6 +1,4 @@ -import pytest from dynaconf import ValidationError, Validator -from flags.state import flag_state from ansible_base.lib.dynamic_config import ( export, @@ -8,7 +6,6 @@ load_envvars, load_python_file_with_injected_context, load_standard_settings_files, - toggle_database_feature_flags, toggle_feature_flags, validate, ) @@ -268,13 +265,3 @@ def test_toggle_feature_flags(): {"condition": "before date", "value": "2022-06-01T12:00Z"}, ] } - - -@pytest.mark.django_db -def test_toggle_database_feature_flags(): - """Ensure that the toggle_feature_flags function works as expected.""" - - settings = {"FEATURE_EDA_ANALYTICS_ENABLED": True, "FEATURE_POLICY_AS_CODE_ENABLED": False} - toggle_database_feature_flags(settings) - assert flag_state("FEATURE_EDA_ANALYTICS_ENABLED") is True - assert flag_state("FEATURE_POLICY_AS_CODE_ENABLED") is False diff --git a/test_app/tests/lib/utils/test_create_system_user.py b/test_app/tests/lib/utils/test_create_system_user.py index e9950ce02..b72b15563 100644 --- a/test_app/tests/lib/utils/test_create_system_user.py +++ b/test_app/tests/lib/utils/test_create_system_user.py @@ -73,7 +73,6 @@ def test_get_system_user_from_basic_model(self): @pytest.mark.django_db def test_get_system_user_from_managed_model(self): - create_system_user(user_model=ManagedUser) - + # System user already exists assert ManagedUser.objects.filter(username=get_system_username()[0]).count() == 0 - assert ManagedUser.all_objects.filter(username=get_system_username()[0]).count() == 1 + assert ManagedUser.all_objects.filter(username=get_system_username()[0]).count() == 0 From d5fae1d14f402d1edea5126d77a4ae1bbfafe9fd Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Tue, 8 Apr 2025 11:16:33 -0400 Subject: [PATCH 03/12] Update DAB PoC --- .../feature_flags/migrations/0001_initial.py | 6 ++- ansible_base/feature_flags/models/aap_flag.py | 11 ++++- ansible_base/feature_flags/serializers.py | 32 +++++++++++-- ansible_base/feature_flags/urls.py | 3 +- ansible_base/feature_flags/utils.py | 2 + ansible_base/feature_flags/views.py | 48 +++++++++++++++++-- ansible_base/lib/dynamic_config/__init__.py | 2 + .../lib/dynamic_config/dynaconf_helpers.py | 16 +++++++ .../feature_flags/platform_flags.py | 19 ++++++-- 9 files changed, 121 insertions(+), 18 deletions(-) diff --git a/ansible_base/feature_flags/migrations/0001_initial.py b/ansible_base/feature_flags/migrations/0001_initial.py index a6b7417b4..34c94d091 100644 --- a/ansible_base/feature_flags/migrations/0001_initial.py +++ b/ansible_base/feature_flags/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.17 on 2025-03-26 17:52 +# Generated by Django 4.2.17 on 2025-04-08 14:36 import ansible_base.feature_flags.models.aap_flag from django.conf import settings @@ -22,13 +22,15 @@ class Migration(migrations.Migration): ('modified', models.DateTimeField(auto_now=True, help_text='The date/time this resource was created.')), ('created', models.DateTimeField(auto_now_add=True, help_text='The date/time this resource was created.')), ('name', models.CharField(help_text='The name of the feature flag. Must follow the format of FEATURE__ENABLED.', max_length=64, validators=[ansible_base.feature_flags.models.aap_flag.validate_feature_flag_name])), + ('ui_name', models.CharField(help_text='The pretty name to display in the application User Interface', max_length=64)), ('condition', models.CharField(default='boolean', help_text='Used to specify a condition, which if met, will enable the feature flag.', max_length=64)), ('value', models.CharField(default='True', help_text='The value used to evaluate the conditional specified.', max_length=127)), ('required', models.BooleanField(default=False, help_text="If multiple conditions are required to be met to enable a feature flag, 'required' can be used to specify the necessary conditionals.")), ('support_level', models.CharField(choices=[('NOT_FOR_USE', 'Not for use'), ('NOT_FOR_PRODUCTION', 'Not for production'), ('READY_FOR_PRODUCTION', 'Ready for production')], help_text='The support criteria for the feature flag. Must be one of (NOT_FOR_USE, NOT_FOR_PRODUCTION, READY_FOR_PRODUCTION).', max_length=25)), ('visibility', models.CharField(choices=[('public', 'public'), ('private', 'private')], help_text='The visibility level of the feature flag. If private, flag is hidden.', max_length=20)), ('toggle_type', models.CharField(choices=[('install-time', 'install-time'), ('run-time', 'run-time')], default='run-time', help_text="Details whether a flag is toggle-able at run-time or install-time. (Default: 'run-time').", max_length=20)), - ('description', models.CharField(default='', help_text='A detailed description giving an overview of the feature flag.', max_length=300)), + ('description', models.CharField(default='', help_text='A detailed description giving an overview of the feature flag.', max_length=500)), + ('support_url', models.CharField(blank=True, default='', help_text='A link to the documentation support URL for the feature', max_length=250)), ('labels', models.JSONField(blank=True, default=list, help_text='A list of labels for the feature flag.', null=True)), ('created_by', models.ForeignKey(default=None, editable=False, help_text='The user who created this resource.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_created+', to=settings.AUTH_USER_MODEL)), ('modified_by', models.ForeignKey(default=None, editable=False, help_text='The user who last modified this resource.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_modified+', to=settings.AUTH_USER_MODEL)), diff --git a/ansible_base/feature_flags/models/aap_flag.py b/ansible_base/feature_flags/models/aap_flag.py index 86aa6654d..d4c6bb788 100644 --- a/ansible_base/feature_flags/models/aap_flag.py +++ b/ansible_base/feature_flags/models/aap_flag.py @@ -25,7 +25,7 @@ def __str__(self): required=" (required)" if self.required else "", ) - resource = AnsibleResourceField(primary_key_field="id") + # resource = AnsibleResourceField(primary_key_field="id") name = models.CharField( max_length=64, @@ -34,6 +34,12 @@ def __str__(self): validators=[validate_feature_flag_name], blank=False, ) + ui_name = models.CharField( + max_length=64, + null=False, + blank=False, + help_text=_("The pretty name to display in the application User Interface") + ) condition = models.CharField(max_length=64, default="boolean", help_text=_("Used to specify a condition, which if met, will enable the feature flag.")) value = models.CharField(max_length=127, default="True", help_text=_("The value used to evaluate the conditional specified.")) required = models.BooleanField( @@ -61,5 +67,6 @@ def __str__(self): default='run-time', help_text=_("Details whether a flag is toggle-able at run-time or install-time. (Default: 'run-time')."), ) - description = models.CharField(max_length=300, null=False, default="", help_text=_("A detailed description giving an overview of the feature flag.")) + description = models.CharField(max_length=500, null=False, default="", help_text=_("A detailed description giving an overview of the feature flag.")) + support_url = models.CharField(max_length=250, null=False, default="", blank=True, help_text="A link to the documentation support URL for the feature") labels = models.JSONField(null=True, default=list, help_text=_("A list of labels for the feature flag."), blank=True) diff --git a/ansible_base/feature_flags/serializers.py b/ansible_base/feature_flags/serializers.py index e3c5e304d..ca3106966 100644 --- a/ansible_base/feature_flags/serializers.py +++ b/ansible_base/feature_flags/serializers.py @@ -1,17 +1,43 @@ from flags.state import flag_state from rest_framework import serializers +from ansible_base.feature_flags.models import AAPFlag +from ansible_base.lib.serializers.common import NamedCommonModelSerializer + from .utils import get_django_flags -# TODO: This view and its serializer can be removed after functionality is migrated over to new class -class FeatureFlagSerializer(serializers.Serializer): +class FeatureFlagSerializer(NamedCommonModelSerializer): + """Serialize list of feature flags""" + + state = serializers.SerializerMethodField() + + class Meta: + model = AAPFlag + fields = NamedCommonModelSerializer.Meta.fields + [x.name for x in AAPFlag._meta.concrete_fields] + ['state'] + read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "labels"] + + def get_state(self, instance): + return flag_state(instance.name) + + def to_representation(self, instance): + instance.state = True + ret = super().to_representation(instance) + return ret + + +# TODO: Remove once all components are migrated to the new endpont. +class OldFeatureFlagSerializer(NamedCommonModelSerializer): """Serialize list of feature flags""" + class Meta: + model = AAPFlag + fields = NamedCommonModelSerializer.Meta.fields + [x.name for x in AAPFlag._meta.concrete_fields] + read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "labels"] + def to_representation(self) -> dict: return_data = {} feature_flags = get_django_flags() for feature_flag in feature_flags: return_data[feature_flag] = flag_state(feature_flag) - return return_data diff --git a/ansible_base/feature_flags/urls.py b/ansible_base/feature_flags/urls.py index 35c7c7061..0c1e0c4cb 100644 --- a/ansible_base/feature_flags/urls.py +++ b/ansible_base/feature_flags/urls.py @@ -8,11 +8,10 @@ router = AssociationResourceRouter() -router.register(r'feature_flags/states', views.FeatureFlagsStatesView, basename='aap_flags_states') - router.register(r'feature_flags', views.FeatureFlagsView, basename='aap_flags') # TODO: Remove once all components are migrated to new endpoints. api_version_urls = [path('feature_flags_state/', views.OldFeatureFlagsStateListView.as_view(), name='feature-flags-state-list'), path('', include(router.urls))] + api_urls = [] root_urls = [] diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index c59f95013..c5f5747bc 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -28,6 +28,8 @@ def update_feature_flag(existing: AAPFlag, new): existing.support_level = new['support_level'] existing.visibility = new['visibility'] + existing.ui_name = new['ui_name'] + existing.support_url = new['support_url'] if 'required' in new: existing.required = new['required'] if 'toggle_type' in new: diff --git a/ansible_base/feature_flags/views.py b/ansible_base/feature_flags/views.py index 196765f41..ada79cc0c 100644 --- a/ansible_base/feature_flags/views.py +++ b/ansible_base/feature_flags/views.py @@ -1,27 +1,65 @@ from django.conf import settings from django.shortcuts import get_object_or_404 from django.utils.translation import gettext_lazy as _ +from flags.state import flag_enabled, flag_state, get_flags +from rest_framework import status from rest_framework.response import Response +from rest_framework.viewsets import ModelViewSet -from ansible_base.feature_flags.serializers import FeatureFlagSerializer +from ansible_base.feature_flags.models import AAPFlag +from ansible_base.feature_flags.serializers import FeatureFlagSerializer, OldFeatureFlagSerializer from ansible_base.lib.utils.views.ansible_base import AnsibleBaseView +from ansible_base.lib.utils.views.django_app_api import AnsibleBaseDjangoAppApiView +from ansible_base.lib.utils.views.permissions import IsSuperuserOrAuditor +from ansible_base.rest_pagination import DefaultPaginator -from .utils import get_django_flags +from .utils import get_django_flags, is_boolean_str -# TODO: This view and its serializer can be removed after functionality is migrated over to new class -class FeatureFlagsStateListView(AnsibleBaseView): +class FeatureFlagsView(AnsibleBaseDjangoAppApiView, ModelViewSet): """ A view class for displaying feature flags """ + queryset = AAPFlag.objects.order_by('id') serializer_class = FeatureFlagSerializer + permission_classes = [IsSuperuserOrAuditor] + http_method_names = ['get', 'put', 'head', 'options'] + + def update(self, request, **kwargs): + _feature_flag = self.get_object() + value = request.data.get('value') + if not value: + return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Invalid request object."}) + + # Disable runtime toggle if the feature flag feature is not enabled + if not flag_enabled('FEATURE_FEATURE_FLAGS_ENABLED'): + return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data={"details": "Runtime feature flags toggle is not enabled."}) + + feature_flag = get_object_or_404(AAPFlag, pk=_feature_flag.id) + if feature_flag.toggle_type == 'install-time': + return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data={"details": "Install-time feature flags cannot be toggled at run-time."}) + if feature_flag.condition == "boolean" and not is_boolean_str(value): + return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Feature flag boolean conditional requires using boolean value."}) + feature_flag.value = value + feature_flag.save() + + return Response(self.get_serializer().to_representation(feature_flag)) + + +# TODO: This can be removed after functionality is migrated over to new class +class OldFeatureFlagsStateListView(AnsibleBaseView): + """ + A view class for displaying feature flags + """ + + serializer_class = OldFeatureFlagSerializer filter_backends = [] name = _('Feature Flags') http_method_names = ['get', 'head'] def _get(self, request, format=None): - self.serializer = FeatureFlagSerializer() + self.serializer = OldFeatureFlagSerializer() return Response(self.serializer.to_representation()) def get_queryset(self): diff --git a/ansible_base/lib/dynamic_config/__init__.py b/ansible_base/lib/dynamic_config/__init__.py index 8aa76b6e2..f11e5a97c 100644 --- a/ansible_base/lib/dynamic_config/__init__.py +++ b/ansible_base/lib/dynamic_config/__init__.py @@ -5,6 +5,7 @@ load_envvars, load_python_file_with_injected_context, load_standard_settings_files, + toggle_database_feature_flags, toggle_feature_flags, validate, ) @@ -17,5 +18,6 @@ "load_python_file_with_injected_context", "load_standard_settings_files", "toggle_feature_flags", + "toggle_database_feature_flags", "validate", ] diff --git a/ansible_base/lib/dynamic_config/dynaconf_helpers.py b/ansible_base/lib/dynamic_config/dynaconf_helpers.py index a3eb7d378..c36fc24bd 100644 --- a/ansible_base/lib/dynamic_config/dynaconf_helpers.py +++ b/ansible_base/lib/dynamic_config/dynaconf_helpers.py @@ -15,6 +15,8 @@ from dynaconf.loaders.yaml_loader import yaml from dynaconf.utils.files import glob from dynaconf.utils.functional import empty +from flags.sources import get_flags +from flags.state import disable_flag, enable_flag from ansible_base.lib.dynamic_config.settings_logic import get_mergeable_dab_settings @@ -315,3 +317,17 @@ def toggle_feature_flags(settings: Dynaconf) -> dict[str, Any]: feature_content[0]["value"] = installer_value data[f"FLAGS__{feature_name}"] = feature_content return data + + +def toggle_database_feature_flags(settings: Dynaconf) -> dict[str, Any]: + """Toggle FLAGS based on installer settings. + FLAGS is a django-flags formatted dictionary. + Installers will place `FEATURE_SOME_PLATFORM_FLAG_ENABLED=True/False` in the settings file. + This function will update the value in the database with the expected boolean value + """ + for feature_name in get_flags(): + if (installer_value := settings.get(feature_name, empty)) is not empty: + if installer_value: + enable_flag(feature_name) + else: + disable_flag(feature_name) diff --git a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py index dc03688dc..1a91dab16 100644 --- a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py +++ b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py @@ -3,52 +3,63 @@ class AAPFlagNameSchema(TypedDict): name: str + ui_name: str condition: str value: str - visibility: str + required: str support_level: str + visibility: str + toggle_type: str description: str + support_url: str labels: list - toggle_type: str AAP_FEATURE_FLAGS: list[AAPFlagNameSchema] = [ AAPFlagNameSchema( name="FEATURE_FEATURE_FLAGS_ENABLED", + ui_name="Feature Flags", condition="boolean", value="True", - visibility="public", support_level='READY_FOR_PRODUCTION', + visibility="public", + toggle_type='install-time', description='If enabled, feature flags can be toggled on/off at runtime via UI or API. ' 'If disabled, feature flags can only be toggled on/off at install-time.', + support_url="", labels=['platform'], - toggle_type='install-time', ), AAPFlagNameSchema( name="FEATURE_INDIRECT_NODE_COUNTING_ENABLED", + ui_name="Indirect Node Counting", visibility="public", condition="boolean", value="False", support_level="NOT_FOR_PRODUCTION", description="TBD", + support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", labels=['controller'], ), AAPFlagNameSchema( name="FEATURE_POLICY_AS_CODE_ENABLED", + ui_name="Policy as Code", visibility="public", condition="boolean", value="False", support_level="NOT_FOR_PRODUCTION", description="TBD", + support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", labels=['controller'], ), AAPFlagNameSchema( name="FEATURE_EDA_ANALYTICS_ENABLED", + ui_name="Event-Driven Ansible Analytics", condition="boolean", value="False", visibility="public", support_level="NOT_FOR_PRODUCTION", description="TBD", + support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", labels=['eda'], ), ] From 0a85f3b71ec413d6f9f17074788b4ddf5299845a Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Thu, 29 May 2025 17:45:35 -0400 Subject: [PATCH 04/12] Add feature flag management command --- .../feature_flags/management/__init__.py | 0 .../management/commands/__init__.py | 0 .../management/commands/feature_flags.py | 50 +++++++++++++++++++ ansible_base/feature_flags/models/aap_flag.py | 10 ++-- ansible_base/feature_flags/utils.py | 3 ++ ansible_base/feature_flags/views.py | 7 +-- .../feature_flags/platform_flags.py | 13 ----- test_app/tests/feature_flags/test_old_api.py | 2 - 8 files changed, 57 insertions(+), 28 deletions(-) create mode 100644 ansible_base/feature_flags/management/__init__.py create mode 100644 ansible_base/feature_flags/management/commands/__init__.py create mode 100644 ansible_base/feature_flags/management/commands/feature_flags.py diff --git a/ansible_base/feature_flags/management/__init__.py b/ansible_base/feature_flags/management/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ansible_base/feature_flags/management/commands/__init__.py b/ansible_base/feature_flags/management/commands/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ansible_base/feature_flags/management/commands/feature_flags.py b/ansible_base/feature_flags/management/commands/feature_flags.py new file mode 100644 index 000000000..1e4b1cddd --- /dev/null +++ b/ansible_base/feature_flags/management/commands/feature_flags.py @@ -0,0 +1,50 @@ +try: + from tabulate import tabulate + + HAS_TABULATE = True +except ImportError: + HAS_TABULATE = False + +from django.core.management.base import BaseCommand +from flags.state import flag_state + +from ansible_base.feature_flags.models import AAPFlag + + +class Command(BaseCommand): + help = "AAP Feature Flag management command" + + def add_arguments(self, parser): + parser.add_argument("--list", action="store_true", help="List feature flags", required=False) + + def handle(self, *args, **options): + if options["list"]: + self.list_feature_flags() + + def list_feature_flags(self): + feature_flags = [] + headers = ["Name", "UI_Name", "Value", "State", "Support Level", "Visibility", "Toggle Type", "Description", "Support URL"] + + for feature_flag in AAPFlag.objects.all().order_by('name'): + feature_flags.append( + [ + f'{feature_flag.name}', + f'{feature_flag.ui_name}', + f'{feature_flag.value}', + f'{flag_state(feature_flag.name)}', + f'{feature_flag.support_level}', + f'{feature_flag.visibility}', + f'{feature_flag.toggle_type}', + f'{feature_flag.description}', + f'{feature_flag.support_url}', + ] + ) + self.stdout.write('') + + if HAS_TABULATE: + self.stdout.write(tabulate(feature_flags, headers, tablefmt="github")) + else: + self.stdout.write("\t".join(headers)) + for feature_flag in feature_flags: + self.stdout.write("\t".join(feature_flag)) + self.stdout.write('') diff --git a/ansible_base/feature_flags/models/aap_flag.py b/ansible_base/feature_flags/models/aap_flag.py index d4c6bb788..7930a719b 100644 --- a/ansible_base/feature_flags/models/aap_flag.py +++ b/ansible_base/feature_flags/models/aap_flag.py @@ -4,7 +4,8 @@ from ansible_base.activitystream.models import AuditableModel from ansible_base.lib.abstract_models.common import NamedCommonModel -from ansible_base.resource_registry.fields import AnsibleResourceField + +# from ansible_base.resource_registry.fields import AnsibleResourceField def validate_feature_flag_name(value: str): @@ -34,12 +35,7 @@ def __str__(self): validators=[validate_feature_flag_name], blank=False, ) - ui_name = models.CharField( - max_length=64, - null=False, - blank=False, - help_text=_("The pretty name to display in the application User Interface") - ) + ui_name = models.CharField(max_length=64, null=False, blank=False, help_text=_("The pretty name to display in the application User Interface")) condition = models.CharField(max_length=64, default="boolean", help_text=_("Used to specify a condition, which if met, will enable the feature flag.")) value = models.CharField(max_length=127, default="True", help_text=_("The value used to evaluate the conditional specified.")) required = models.BooleanField( diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index c5f5747bc..d0b820d3d 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -1,4 +1,5 @@ from django.apps import apps +from django.conf import settings from django.core.exceptions import ValidationError from flags.sources import get_flags @@ -51,6 +52,8 @@ def load_feature_flags(): if existing_flag: update_feature_flag(existing_flag.first(), flag) else: + if hasattr(settings, flag['name']): + flag['value'] = getattr(settings, flag['name']) FeatureFlags.objects.create(**flag) AAPFlag(**flag).full_clean() except ValidationError as e: diff --git a/ansible_base/feature_flags/views.py b/ansible_base/feature_flags/views.py index ada79cc0c..554def531 100644 --- a/ansible_base/feature_flags/views.py +++ b/ansible_base/feature_flags/views.py @@ -1,7 +1,7 @@ from django.conf import settings from django.shortcuts import get_object_or_404 from django.utils.translation import gettext_lazy as _ -from flags.state import flag_enabled, flag_state, get_flags +from flags.state import flag_enabled from rest_framework import status from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet @@ -11,7 +11,6 @@ from ansible_base.lib.utils.views.ansible_base import AnsibleBaseView from ansible_base.lib.utils.views.django_app_api import AnsibleBaseDjangoAppApiView from ansible_base.lib.utils.views.permissions import IsSuperuserOrAuditor -from ansible_base.rest_pagination import DefaultPaginator from .utils import get_django_flags, is_boolean_str @@ -32,10 +31,6 @@ def update(self, request, **kwargs): if not value: return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Invalid request object."}) - # Disable runtime toggle if the feature flag feature is not enabled - if not flag_enabled('FEATURE_FEATURE_FLAGS_ENABLED'): - return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data={"details": "Runtime feature flags toggle is not enabled."}) - feature_flag = get_object_or_404(AAPFlag, pk=_feature_flag.id) if feature_flag.toggle_type == 'install-time': return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data={"details": "Install-time feature flags cannot be toggled at run-time."}) diff --git a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py index 1a91dab16..448d8625c 100644 --- a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py +++ b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py @@ -16,19 +16,6 @@ class AAPFlagNameSchema(TypedDict): AAP_FEATURE_FLAGS: list[AAPFlagNameSchema] = [ - AAPFlagNameSchema( - name="FEATURE_FEATURE_FLAGS_ENABLED", - ui_name="Feature Flags", - condition="boolean", - value="True", - support_level='READY_FOR_PRODUCTION', - visibility="public", - toggle_type='install-time', - description='If enabled, feature flags can be toggled on/off at runtime via UI or API. ' - 'If disabled, feature flags can only be toggled on/off at install-time.', - support_url="", - labels=['platform'], - ), AAPFlagNameSchema( name="FEATURE_INDIRECT_NODE_COUNTING_ENABLED", ui_name="Indirect Node Counting", diff --git a/test_app/tests/feature_flags/test_old_api.py b/test_app/tests/feature_flags/test_old_api.py index f39ab3804..d2bf33f9e 100644 --- a/test_app/tests/feature_flags/test_old_api.py +++ b/test_app/tests/feature_flags/test_old_api.py @@ -14,5 +14,3 @@ def test_feature_flags_state_api_list(admin_api_client: APIClient): assert response.data["FEATURE_EDA_ANALYTICS_ENABLED"] is False assert 'FEATURE_INDIRECT_NODE_COUNTING_ENABLED' in response.data assert response.data["FEATURE_INDIRECT_NODE_COUNTING_ENABLED"] is False - assert 'FEATURE_FEATURE_FLAGS_ENABLED' in response.data - assert response.data["FEATURE_FEATURE_FLAGS_ENABLED"] is True From b1d6534c223441dae5613a1482cf03a208559242 Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Fri, 30 May 2025 14:53:50 -0400 Subject: [PATCH 05/12] resource-sync --- .dockerignore | 6 +++ ansible_base/feature_flags/apps.py | 19 ------- .../feature_flags/migrations/0001_initial.py | 4 +- ansible_base/feature_flags/models/aap_flag.py | 10 ++-- ansible_base/feature_flags/serializers.py | 4 +- ansible_base/feature_flags/urls.py | 1 + ansible_base/feature_flags/utils.py | 44 ++++++++-------- ansible_base/feature_flags/views.py | 51 ++++++++++--------- .../feature_flags/platform_flags.py | 22 ++++++++ ansible_base/rbac/permission_registry.py | 6 +++ ansible_base/resource_registry/registry.py | 1 + .../resource_registry/shared_types.py | 3 +- test_app/resource_api.py | 7 ++- .../management/test_authenticators.py | 2 +- test_app/tests/feature_flags/test_old_api.py | 4 -- .../lib/utils/test_create_system_user.py | 5 +- .../test_resource_types_api.py | 11 +++- 17 files changed, 113 insertions(+), 87 deletions(-) create mode 100644 .dockerignore diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 000000000..ddd061a07 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,6 @@ +**/.github +**/.cache +**/.gitignore +**/.venv +**/venv +**/.tox diff --git a/ansible_base/feature_flags/apps.py b/ansible_base/feature_flags/apps.py index 11f6a574b..cfe665a64 100644 --- a/ansible_base/feature_flags/apps.py +++ b/ansible_base/feature_flags/apps.py @@ -1,8 +1,4 @@ from django.apps import AppConfig -from django.db.models.signals import post_migrate -from django.db.utils import OperationalError, ProgrammingError - -from .utils import create_initial_data class FeatureFlagsConfig(AppConfig): @@ -10,18 +6,3 @@ class FeatureFlagsConfig(AppConfig): name = 'ansible_base.feature_flags' label = 'dab_feature_flags' verbose_name = 'Feature Flags' - - def ready(self): - from django.conf import settings - - if 'ansible_base.feature_flags' in settings.INSTALLED_APPS: - # TODO: Is there a better way to handle this logic? - - # If migrations are complete, attempt to load in feature flags again. - # This can help capture any updates to the platform flags loaded in to ensure that new values - # are added and updated. - # Otherwise wait for migrations to be complete before loading in feature flags. - try: - create_initial_data() - except (ProgrammingError, OperationalError): - post_migrate.connect(create_initial_data) diff --git a/ansible_base/feature_flags/migrations/0001_initial.py b/ansible_base/feature_flags/migrations/0001_initial.py index 34c94d091..fa9d52b13 100644 --- a/ansible_base/feature_flags/migrations/0001_initial.py +++ b/ansible_base/feature_flags/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.17 on 2025-04-08 14:36 +# Generated by Django 4.2.17 on 2025-06-04 12:00 import ansible_base.feature_flags.models.aap_flag from django.conf import settings @@ -36,7 +36,7 @@ class Migration(migrations.Migration): ('modified_by', models.ForeignKey(default=None, editable=False, help_text='The user who last modified this resource.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_modified+', to=settings.AUTH_USER_MODEL)), ], options={ - 'unique_together': {('name', 'condition', 'value')}, + 'unique_together': {('name', 'condition')}, }, ), ] diff --git a/ansible_base/feature_flags/models/aap_flag.py b/ansible_base/feature_flags/models/aap_flag.py index 7930a719b..c4b5c14f0 100644 --- a/ansible_base/feature_flags/models/aap_flag.py +++ b/ansible_base/feature_flags/models/aap_flag.py @@ -2,10 +2,8 @@ from django.db import models from django.utils.translation import gettext_lazy as _ -from ansible_base.activitystream.models import AuditableModel from ansible_base.lib.abstract_models.common import NamedCommonModel - -# from ansible_base.resource_registry.fields import AnsibleResourceField +from ansible_base.resource_registry.fields import AnsibleResourceField def validate_feature_flag_name(value: str): @@ -13,10 +11,10 @@ def validate_feature_flag_name(value: str): raise ValidationError(_("Feature flag names must follow the format of `FEATURE__ENABLED`")) -class AAPFlag(NamedCommonModel, AuditableModel): +class AAPFlag(NamedCommonModel): class Meta: app_label = "dab_feature_flags" - unique_together = ("name", "condition", "value") + unique_together = ("name", "condition") def __str__(self): return "{name} is enabled when {condition} is " "{value}{required}".format( @@ -26,7 +24,7 @@ def __str__(self): required=" (required)" if self.required else "", ) - # resource = AnsibleResourceField(primary_key_field="id") + resource = AnsibleResourceField(primary_key_field="id") name = models.CharField( max_length=64, diff --git a/ansible_base/feature_flags/serializers.py b/ansible_base/feature_flags/serializers.py index ca3106966..0cb0c9e70 100644 --- a/ansible_base/feature_flags/serializers.py +++ b/ansible_base/feature_flags/serializers.py @@ -15,7 +15,7 @@ class FeatureFlagSerializer(NamedCommonModelSerializer): class Meta: model = AAPFlag fields = NamedCommonModelSerializer.Meta.fields + [x.name for x in AAPFlag._meta.concrete_fields] + ['state'] - read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "labels"] + read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "labels", "ui_name", "support_url"] def get_state(self, instance): return flag_state(instance.name) @@ -35,7 +35,7 @@ class Meta: fields = NamedCommonModelSerializer.Meta.fields + [x.name for x in AAPFlag._meta.concrete_fields] read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "labels"] - def to_representation(self) -> dict: + def to_representation(self, instance=None) -> dict: return_data = {} feature_flags = get_django_flags() for feature_flag in feature_flags: diff --git a/ansible_base/feature_flags/urls.py b/ansible_base/feature_flags/urls.py index 0c1e0c4cb..4631e7ec6 100644 --- a/ansible_base/feature_flags/urls.py +++ b/ansible_base/feature_flags/urls.py @@ -8,6 +8,7 @@ router = AssociationResourceRouter() +router.register(r'feature_flags/states', views.FeatureFlagsStatesView, basename='aap_flags_states') router.register(r'feature_flags', views.FeatureFlagsView, basename='aap_flags') # TODO: Remove once all components are migrated to new endpoints. diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index d0b820d3d..5726dcb51 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -1,3 +1,5 @@ +import logging + from django.apps import apps from django.conf import settings from django.core.exceptions import ValidationError @@ -5,15 +7,13 @@ from ansible_base.lib.dynamic_config.feature_flags.platform_flags import AAP_FEATURE_FLAGS +logger = logging.getLogger('ansible_base.feature_flags.utils') + def get_django_flags(): return get_flags() -def is_boolean_str(val): - return val.lower() in {'true', 'false'} - - def create_initial_data(**kwargs): """ Loads in platform feature flags when the server starts @@ -26,20 +26,15 @@ def update_feature_flag(existing: AAPFlag, new): Update only the required fields of the feature flag model. This is used to ensure that flags can be loaded in when the server starts, with any applicable updates. """ - - existing.support_level = new['support_level'] - existing.visibility = new['visibility'] - existing.ui_name = new['ui_name'] - existing.support_url = new['support_url'] - if 'required' in new: - existing.required = new['required'] - if 'toggle_type' in new: - existing.toggle_type = new['toggle_type'] - if 'labels' in new: - existing.labels = new['labels'] - if 'description' in new: - existing.description = new['description'] - existing.save() + existing.support_level = new.get('support_level') + existing.visibility = new.get('visibility') + existing.ui_name = new.get('ui_name') + existing.support_url = new.get('support_url') + existing.required = new.get('required', False) + existing.toggle_type = new.get('toggle_type', 'run-time') + existing.labels = new.get('labels', []) + existing.description = new.get('description', '') + return existing def load_feature_flags(): """ @@ -50,19 +45,20 @@ def load_feature_flags(): try: existing_flag = FeatureFlags.objects.filter(name=flag['name'], condition=flag['condition']) if existing_flag: - update_feature_flag(existing_flag.first(), flag) + feature_flag = update_feature_flag(existing_flag.first(), flag) else: if hasattr(settings, flag['name']): flag['value'] = getattr(settings, flag['name']) - FeatureFlags.objects.create(**flag) - AAPFlag(**flag).full_clean() + feature_flag = FeatureFlags(**flag) + feature_flag.full_clean() + feature_flag.save() except ValidationError as e: # Ignore this error unless better way to bypass this - if e.messages[0] == 'Aap flag with this Name, Condition and Value already exists.': + if e.messages[0] == 'Aap flag with this Name and Condition already exists.': pass else: - # Raise row validation errors - raise e + error_msg = f"Invalid feature flag: {flag['name']}. Error: {e}" + logger.error(error_msg) def delete_feature_flags(): """ diff --git a/ansible_base/feature_flags/views.py b/ansible_base/feature_flags/views.py index 554def531..832f887d0 100644 --- a/ansible_base/feature_flags/views.py +++ b/ansible_base/feature_flags/views.py @@ -1,8 +1,7 @@ from django.conf import settings -from django.shortcuts import get_object_or_404 from django.utils.translation import gettext_lazy as _ -from flags.state import flag_enabled -from rest_framework import status +from flags.sources import get_flags +from flags.state import flag_state from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet @@ -10,9 +9,12 @@ from ansible_base.feature_flags.serializers import FeatureFlagSerializer, OldFeatureFlagSerializer from ansible_base.lib.utils.views.ansible_base import AnsibleBaseView from ansible_base.lib.utils.views.django_app_api import AnsibleBaseDjangoAppApiView -from ansible_base.lib.utils.views.permissions import IsSuperuserOrAuditor +from ansible_base.lib.utils.views.permissions import IsSuperuserOrAuditor, try_add_oauth2_scope_permission -from .utils import get_django_flags, is_boolean_str +# from ansible_base.oauth2_provider.permissions import OAuth2ScopePermission +from ansible_base.rest_pagination import DefaultPaginator + +from .utils import get_django_flags class FeatureFlagsView(AnsibleBaseDjangoAppApiView, ModelViewSet): @@ -22,24 +24,27 @@ class FeatureFlagsView(AnsibleBaseDjangoAppApiView, ModelViewSet): queryset = AAPFlag.objects.order_by('id') serializer_class = FeatureFlagSerializer - permission_classes = [IsSuperuserOrAuditor] - http_method_names = ['get', 'put', 'head', 'options'] - - def update(self, request, **kwargs): - _feature_flag = self.get_object() - value = request.data.get('value') - if not value: - return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Invalid request object."}) - - feature_flag = get_object_or_404(AAPFlag, pk=_feature_flag.id) - if feature_flag.toggle_type == 'install-time': - return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED, data={"details": "Install-time feature flags cannot be toggled at run-time."}) - if feature_flag.condition == "boolean" and not is_boolean_str(value): - return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Feature flag boolean conditional requires using boolean value."}) - feature_flag.value = value - feature_flag.save() - - return Response(self.get_serializer().to_representation(feature_flag)) + permission_classes = try_add_oauth2_scope_permission([IsSuperuserOrAuditor]) + http_method_names = ['get', 'head', 'options'] + + +class FeatureFlagsStatesView(AnsibleBaseDjangoAppApiView, ModelViewSet): + """ + A view class for displaying feature flags states + """ + + queryset = AAPFlag.objects.order_by('id') + permission_classes = try_add_oauth2_scope_permission([IsSuperuserOrAuditor]) + http_method_names = ['get', 'head', 'options'] + + def list(self, request): + paginator = DefaultPaginator() + flags = get_flags() + ret = [] + for flag in flags: + ret.append({"flag_name": flag, "flag_state": flag_state(flag)}) + result_page = paginator.paginate_queryset(ret, request) + return paginator.get_paginated_response(result_page) # TODO: This can be removed after functionality is migrated over to new class diff --git a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py index 448d8625c..df84a579f 100644 --- a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py +++ b/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py @@ -49,4 +49,26 @@ class AAPFlagNameSchema(TypedDict): support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", labels=['eda'], ), + AAPFlagNameSchema( + name="FEATURE_GATEWAY_IPV6_USAGE_ENABLED", + ui_name="Gateway IPv6 Usage", + condition="boolean", + value="False", + visibility="private", + support_level="NOT_FOR_PRODUCTION", + description="TBD", + support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", + labels=['gateway'], + ), + AAPFlagNameSchema( + name="FEATURE_GATEWAY_CREATE_CRC_SERVICE_TYPE", + ui_name="Gateway Create CRC Service Type", + condition="boolean", + value="False", + visibility="private", + support_level="NOT_FOR_PRODUCTION", + description="TBD", + support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", + labels=['gateway'], + ), ] diff --git a/ansible_base/rbac/permission_registry.py b/ansible_base/rbac/permission_registry.py index 1603eba9b..263eadd6f 100644 --- a/ansible_base/rbac/permission_registry.py +++ b/ansible_base/rbac/permission_registry.py @@ -143,6 +143,7 @@ def create_managed_roles(self, apps) -> list[tuple[Model, bool]]: return ret def call_when_apps_ready(self, apps, app_config) -> None: + from ansible_base.feature_flags.utils import create_initial_data as feature_flag_create_initial_data from ansible_base.rbac import triggers from ansible_base.rbac.evaluations import bound_has_obj_perm, bound_singleton_permissions, connect_rbac_methods from ansible_base.rbac.management import create_dab_permissions @@ -172,6 +173,11 @@ def call_when_apps_ready(self, apps, app_config) -> None: sender=app_config, dispatch_uid="ansible_base.rbac.triggers.post_migration_rbac_setup", ) + if 'ansible_base.feature_flags' in settings.INSTALLED_APPS: + try: + feature_flag_create_initial_data() + except Exception: + post_migrate.connect(feature_flag_create_initial_data, sender=self) self.user_model.add_to_class('has_obj_perm', bound_has_obj_perm) self.user_model.add_to_class('singleton_permissions', bound_singleton_permissions) diff --git a/ansible_base/resource_registry/registry.py b/ansible_base/resource_registry/registry.py index 7d411b281..6ef67ea26 100644 --- a/ansible_base/resource_registry/registry.py +++ b/ansible_base/resource_registry/registry.py @@ -27,6 +27,7 @@ class ServiceAPIConfig: "shared.team": ResourceTypeProcessor, "shared.organization": ResourceTypeProcessor, "shared.user": ResourceTypeProcessor, + "shared.aapflag": ResourceTypeProcessor, } custom_resource_processors = {} diff --git a/ansible_base/resource_registry/shared_types.py b/ansible_base/resource_registry/shared_types.py index 4289604a2..793f3628d 100644 --- a/ansible_base/resource_registry/shared_types.py +++ b/ansible_base/resource_registry/shared_types.py @@ -80,11 +80,10 @@ class TeamType(SharedResourceTypeSerializer): class FeatureFlagType(SharedResourceTypeSerializer): """Serialize list of feature flags""" - RESOURCE_TYPE = "feature_flag" + RESOURCE_TYPE = "aapflag" UNIQUE_FIELDS = ( "name", "condition", - "value", ) name = serializers.CharField() diff --git a/test_app/resource_api.py b/test_app/resource_api.py index 515d26ea4..174863a4d 100644 --- a/test_app/resource_api.py +++ b/test_app/resource_api.py @@ -1,8 +1,9 @@ from django.contrib.auth import get_user_model from ansible_base.authentication.models import Authenticator +from ansible_base.feature_flags.models import AAPFlag from ansible_base.resource_registry.registry import ResourceConfig, ServiceAPIConfig, SharedResource -from ansible_base.resource_registry.shared_types import OrganizationType, TeamType, UserType +from ansible_base.resource_registry.shared_types import FeatureFlagType, OrganizationType, TeamType, UserType from ansible_base.resource_registry.utils.resource_type_processor import ResourceTypeProcessor from test_app.models import Organization, Original1, Proxy2, ResourceMigrationTestModel, Team @@ -42,4 +43,8 @@ class APIConfig(ServiceAPIConfig): ResourceConfig(ResourceMigrationTestModel), ResourceConfig(Original1), ResourceConfig(Proxy2), + ResourceConfig( + AAPFlag, + shared_resource=SharedResource(serializer=FeatureFlagType, is_provider=False), + ), ] diff --git a/test_app/tests/authentication/management/test_authenticators.py b/test_app/tests/authentication/management/test_authenticators.py index 930dd2218..53ba0e338 100644 --- a/test_app/tests/authentication/management/test_authenticators.py +++ b/test_app/tests/authentication/management/test_authenticators.py @@ -98,7 +98,7 @@ def test_authenticators_cli_initialize( # Sanity check: # _system user exists - assert django_user_model.objects.count() == 1 + assert django_user_model.objects.count() == 0 # Optionally create admin user if admin_user_exists: diff --git a/test_app/tests/feature_flags/test_old_api.py b/test_app/tests/feature_flags/test_old_api.py index d2bf33f9e..71a0aff9d 100644 --- a/test_app/tests/feature_flags/test_old_api.py +++ b/test_app/tests/feature_flags/test_old_api.py @@ -10,7 +10,3 @@ def test_feature_flags_state_api_list(admin_api_client: APIClient): url = get_relative_url("featureflags-list") response = admin_api_client.get(url) assert response.status_code == 200 - assert 'FEATURE_EDA_ANALYTICS_ENABLED' in response.data - assert response.data["FEATURE_EDA_ANALYTICS_ENABLED"] is False - assert 'FEATURE_INDIRECT_NODE_COUNTING_ENABLED' in response.data - assert response.data["FEATURE_INDIRECT_NODE_COUNTING_ENABLED"] is False diff --git a/test_app/tests/lib/utils/test_create_system_user.py b/test_app/tests/lib/utils/test_create_system_user.py index b72b15563..e9950ce02 100644 --- a/test_app/tests/lib/utils/test_create_system_user.py +++ b/test_app/tests/lib/utils/test_create_system_user.py @@ -73,6 +73,7 @@ def test_get_system_user_from_basic_model(self): @pytest.mark.django_db def test_get_system_user_from_managed_model(self): - # System user already exists + create_system_user(user_model=ManagedUser) + assert ManagedUser.objects.filter(username=get_system_username()[0]).count() == 0 - assert ManagedUser.all_objects.filter(username=get_system_username()[0]).count() == 0 + assert ManagedUser.all_objects.filter(username=get_system_username()[0]).count() == 1 diff --git a/test_app/tests/resource_registry/test_resource_types_api.py b/test_app/tests/resource_registry/test_resource_types_api.py index 067ac1c44..ecf9ff7ea 100644 --- a/test_app/tests/resource_registry/test_resource_types_api.py +++ b/test_app/tests/resource_registry/test_resource_types_api.py @@ -12,7 +12,16 @@ def test_resource_type_list(admin_api_client): response = admin_api_client.get(url) assert response.status_code == 200 assert set([x["name"] for x in response.data['results']]) == set( - ["shared.user", "shared.team", "aap.authenticator", "aap.original1", "aap.original2", "shared.organization", "aap.resourcemigrationtestmodel"] + [ + "shared.user", + "shared.team", + "aap.authenticator", + "aap.original1", + "aap.original2", + "shared.organization", + "aap.resourcemigrationtestmodel", + "shared.aapflag", + ] ) From 7057f50aeccf30f54a597f5d7767161d6094e239 Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Wed, 4 Jun 2025 17:26:46 -0400 Subject: [PATCH 06/12] refactor and cleanup code --- .../feature_flags.py} | 0 .../feature_flags/flag_source.py | 6 +++--- ansible_base/feature_flags/utils.py | 15 ++++++++------- ansible_base/feature_flags/views.py | 2 -- ansible_base/lib/dynamic_config/__init__.py | 2 -- .../lib/dynamic_config/dynaconf_helpers.py | 16 ---------------- .../lib/dynamic_config/settings_logic.py | 2 +- .../management/test_authenticators.py | 1 - 8 files changed, 12 insertions(+), 32 deletions(-) rename ansible_base/{lib/dynamic_config/feature_flags/platform_flags.py => feature_flags/feature_flags.py} (100%) rename ansible_base/{lib/dynamic_config => }/feature_flags/flag_source.py (78%) diff --git a/ansible_base/lib/dynamic_config/feature_flags/platform_flags.py b/ansible_base/feature_flags/feature_flags.py similarity index 100% rename from ansible_base/lib/dynamic_config/feature_flags/platform_flags.py rename to ansible_base/feature_flags/feature_flags.py diff --git a/ansible_base/lib/dynamic_config/feature_flags/flag_source.py b/ansible_base/feature_flags/flag_source.py similarity index 78% rename from ansible_base/lib/dynamic_config/feature_flags/flag_source.py rename to ansible_base/feature_flags/flag_source.py index 2c6e3f236..f6511ce09 100644 --- a/ansible_base/lib/dynamic_config/feature_flags/flag_source.py +++ b/ansible_base/feature_flags/flag_source.py @@ -3,7 +3,7 @@ class DatabaseCondition(Condition): - """Condition that includes the FlagState database object""" + """Condition that includes the AAPFlags database object""" def __init__(self, condition, value, required=False, obj=None): super().__init__(condition, value, required=required) @@ -13,8 +13,8 @@ def __init__(self, condition, value, required=False, obj=None): class AAPFlagSource(object): def get_queryset(self): - FlagState = apps.get_model('dab_feature_flags', 'AAPFlag') - return FlagState.objects.all() + aap_flags = apps.get_model('dab_feature_flags', 'AAPFlag') + return aap_flags.objects.all() def get_flags(self): flags = {} diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index 5726dcb51..3ebfbb514 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -5,7 +5,7 @@ from django.core.exceptions import ValidationError from flags.sources import get_flags -from ansible_base.lib.dynamic_config.feature_flags.platform_flags import AAP_FEATURE_FLAGS +from ansible_base.feature_flags.feature_flags import AAP_FEATURE_FLAGS logger = logging.getLogger('ansible_base.feature_flags.utils') @@ -14,7 +14,7 @@ def get_django_flags(): return get_flags() -def create_initial_data(**kwargs): +def create_initial_data(**kwargs): # NOSONAR """ Loads in platform feature flags when the server starts """ @@ -40,22 +40,22 @@ def load_feature_flags(): """ Loads in all feature flags into the database. Updates them if necessary. """ - FeatureFlags = apps.get_model('dab_feature_flags', 'AAPFlag') + feature_flags_model = apps.get_model('dab_feature_flags', 'AAPFlag') for flag in AAP_FEATURE_FLAGS: try: - existing_flag = FeatureFlags.objects.filter(name=flag['name'], condition=flag['condition']) + existing_flag = feature_flags_model.objects.filter(name=flag['name'], condition=flag['condition']) if existing_flag: feature_flag = update_feature_flag(existing_flag.first(), flag) else: if hasattr(settings, flag['name']): flag['value'] = getattr(settings, flag['name']) - feature_flag = FeatureFlags(**flag) + feature_flag = feature_flags_model(**flag) feature_flag.full_clean() feature_flag.save() except ValidationError as e: # Ignore this error unless better way to bypass this if e.messages[0] == 'Aap flag with this Name and Condition already exists.': - pass + logger.info(f"Feature flag: {flag['name']} already exists") else: error_msg = f"Invalid feature flag: {flag['name']}. Error: {e}" logger.error(error_msg) @@ -70,10 +70,11 @@ def delete_feature_flags(): for _flag in AAP_FEATURE_FLAGS: if flag.name == _flag['name'] and flag.condition == _flag['condition']: found = True - continue + break if found: continue if not found: + logger.info(f"Deleting feature flag: {flag.name} as it is no longer available as a platform flag") flag.delete() delete_feature_flags() diff --git a/ansible_base/feature_flags/views.py b/ansible_base/feature_flags/views.py index 832f887d0..a6ed8a858 100644 --- a/ansible_base/feature_flags/views.py +++ b/ansible_base/feature_flags/views.py @@ -10,8 +10,6 @@ from ansible_base.lib.utils.views.ansible_base import AnsibleBaseView from ansible_base.lib.utils.views.django_app_api import AnsibleBaseDjangoAppApiView from ansible_base.lib.utils.views.permissions import IsSuperuserOrAuditor, try_add_oauth2_scope_permission - -# from ansible_base.oauth2_provider.permissions import OAuth2ScopePermission from ansible_base.rest_pagination import DefaultPaginator from .utils import get_django_flags diff --git a/ansible_base/lib/dynamic_config/__init__.py b/ansible_base/lib/dynamic_config/__init__.py index f11e5a97c..8aa76b6e2 100644 --- a/ansible_base/lib/dynamic_config/__init__.py +++ b/ansible_base/lib/dynamic_config/__init__.py @@ -5,7 +5,6 @@ load_envvars, load_python_file_with_injected_context, load_standard_settings_files, - toggle_database_feature_flags, toggle_feature_flags, validate, ) @@ -18,6 +17,5 @@ "load_python_file_with_injected_context", "load_standard_settings_files", "toggle_feature_flags", - "toggle_database_feature_flags", "validate", ] diff --git a/ansible_base/lib/dynamic_config/dynaconf_helpers.py b/ansible_base/lib/dynamic_config/dynaconf_helpers.py index c36fc24bd..a3eb7d378 100644 --- a/ansible_base/lib/dynamic_config/dynaconf_helpers.py +++ b/ansible_base/lib/dynamic_config/dynaconf_helpers.py @@ -15,8 +15,6 @@ from dynaconf.loaders.yaml_loader import yaml from dynaconf.utils.files import glob from dynaconf.utils.functional import empty -from flags.sources import get_flags -from flags.state import disable_flag, enable_flag from ansible_base.lib.dynamic_config.settings_logic import get_mergeable_dab_settings @@ -317,17 +315,3 @@ def toggle_feature_flags(settings: Dynaconf) -> dict[str, Any]: feature_content[0]["value"] = installer_value data[f"FLAGS__{feature_name}"] = feature_content return data - - -def toggle_database_feature_flags(settings: Dynaconf) -> dict[str, Any]: - """Toggle FLAGS based on installer settings. - FLAGS is a django-flags formatted dictionary. - Installers will place `FEATURE_SOME_PLATFORM_FLAG_ENABLED=True/False` in the settings file. - This function will update the value in the database with the expected boolean value - """ - for feature_name in get_flags(): - if (installer_value := settings.get(feature_name, empty)) is not empty: - if installer_value: - enable_flag(feature_name) - else: - disable_flag(feature_name) diff --git a/ansible_base/lib/dynamic_config/settings_logic.py b/ansible_base/lib/dynamic_config/settings_logic.py index b2bbd55ce..35851b438 100644 --- a/ansible_base/lib/dynamic_config/settings_logic.py +++ b/ansible_base/lib/dynamic_config/settings_logic.py @@ -306,7 +306,7 @@ def get_mergeable_dab_settings(settings: dict) -> dict: # NOSONAR if "flags" not in installed_apps: installed_apps.append('flags') - dab_data['FLAG_SOURCES'] = ('ansible_base.lib.dynamic_config.feature_flags.flag_source.AAPFlagSource',) + dab_data['FLAG_SOURCES'] = ('ansible_base.feature_flags.flag_source.AAPFlagSource',) found_template_backend = False template_context_processor = 'django.template.context_processors.request' diff --git a/test_app/tests/authentication/management/test_authenticators.py b/test_app/tests/authentication/management/test_authenticators.py index 53ba0e338..b2fc302d4 100644 --- a/test_app/tests/authentication/management/test_authenticators.py +++ b/test_app/tests/authentication/management/test_authenticators.py @@ -97,7 +97,6 @@ def test_authenticators_cli_initialize( err = StringIO() # Sanity check: - # _system user exists assert django_user_model.objects.count() == 0 # Optionally create admin user From 9eb1fcf55dbe72141405bde4eba6c54d5fbd9500 Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Wed, 4 Jun 2025 19:07:54 -0400 Subject: [PATCH 07/12] add unit tests --- ansible_base/feature_flags/feature_flags.py | 13 +- ansible_base/feature_flags/models/aap_flag.py | 2 +- ansible_base/feature_flags/utils.py | 112 ++--- .../lib/dynamic_config/settings_logic.py | 7 +- ansible_base/lib/testing/fixtures.py | 7 + requirements/requirements_dev.txt | 1 + test_app/defaults.py | 26 -- .../feature_flags/management/__init__.py | 0 .../management/test_feature_flags.py | 212 ++++++++++ .../tests/feature_flags/models/__init__.py | 0 .../feature_flags/models/test_aap_flag.py | 49 +++ test_app/tests/feature_flags/test_utils.py | 389 ++++++++++++++++++ .../tests/feature_flags/views/__init__.py | 0 .../feature_flags/views/test_feature_flag.py | 106 +++++ 14 files changed, 839 insertions(+), 85 deletions(-) create mode 100644 test_app/tests/feature_flags/management/__init__.py create mode 100644 test_app/tests/feature_flags/management/test_feature_flags.py create mode 100644 test_app/tests/feature_flags/models/__init__.py create mode 100644 test_app/tests/feature_flags/models/test_aap_flag.py create mode 100644 test_app/tests/feature_flags/test_utils.py create mode 100644 test_app/tests/feature_flags/views/__init__.py create mode 100644 test_app/tests/feature_flags/views/test_feature_flag.py diff --git a/ansible_base/feature_flags/feature_flags.py b/ansible_base/feature_flags/feature_flags.py index df84a579f..9e3ec0144 100644 --- a/ansible_base/feature_flags/feature_flags.py +++ b/ansible_base/feature_flags/feature_flags.py @@ -61,7 +61,7 @@ class AAPFlagNameSchema(TypedDict): labels=['gateway'], ), AAPFlagNameSchema( - name="FEATURE_GATEWAY_CREATE_CRC_SERVICE_TYPE", + name="FEATURE_GATEWAY_CREATE_CRC_SERVICE_TYPE_ENABLED", ui_name="Gateway Create CRC Service Type", condition="boolean", value="False", @@ -71,4 +71,15 @@ class AAPFlagNameSchema(TypedDict): support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", labels=['gateway'], ), + AAPFlagNameSchema( + name="FEATURE_DISPATCHERD_ENABLED", + ui_name="AAP Dispatcherd", + condition="boolean", + value="False", + visibility="private", + support_level="NOT_FOR_PRODUCTION", + description="TBD", + support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", + labels=['eda', 'controller'], + ), ] diff --git a/ansible_base/feature_flags/models/aap_flag.py b/ansible_base/feature_flags/models/aap_flag.py index c4b5c14f0..0158c4af3 100644 --- a/ansible_base/feature_flags/models/aap_flag.py +++ b/ansible_base/feature_flags/models/aap_flag.py @@ -17,7 +17,7 @@ class Meta: unique_together = ("name", "condition") def __str__(self): - return "{name} is enabled when {condition} is " "{value}{required}".format( + return "{name} condition {condition} is set to " "{value}{required}".format( name=self.name, condition=self.condition, value=self.value, diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index 3ebfbb514..6285a864d 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -18,64 +18,64 @@ def create_initial_data(**kwargs): # NOSONAR """ Loads in platform feature flags when the server starts """ + delete_feature_flags() + load_feature_flags() - from ansible_base.feature_flags.models.aap_flag import AAPFlag - def update_feature_flag(existing: AAPFlag, new): - """ - Update only the required fields of the feature flag model. - This is used to ensure that flags can be loaded in when the server starts, with any applicable updates. - """ - existing.support_level = new.get('support_level') - existing.visibility = new.get('visibility') - existing.ui_name = new.get('ui_name') - existing.support_url = new.get('support_url') - existing.required = new.get('required', False) - existing.toggle_type = new.get('toggle_type', 'run-time') - existing.labels = new.get('labels', []) - existing.description = new.get('description', '') - return existing +def update_feature_flag(existing, new): + """ + Update only the required fields of the feature flag model. + This is used to ensure that flags can be loaded in when the server starts, with any applicable updates. + """ + existing.support_level = new.get('support_level') + existing.visibility = new.get('visibility') + existing.ui_name = new.get('ui_name') + existing.support_url = new.get('support_url') + existing.required = new.get('required', False) + existing.toggle_type = new.get('toggle_type', 'run-time') + existing.labels = new.get('labels', []) + existing.description = new.get('description', '') + return existing - def load_feature_flags(): - """ - Loads in all feature flags into the database. Updates them if necessary. - """ - feature_flags_model = apps.get_model('dab_feature_flags', 'AAPFlag') - for flag in AAP_FEATURE_FLAGS: - try: - existing_flag = feature_flags_model.objects.filter(name=flag['name'], condition=flag['condition']) - if existing_flag: - feature_flag = update_feature_flag(existing_flag.first(), flag) - else: - if hasattr(settings, flag['name']): - flag['value'] = getattr(settings, flag['name']) - feature_flag = feature_flags_model(**flag) - feature_flag.full_clean() - feature_flag.save() - except ValidationError as e: - # Ignore this error unless better way to bypass this - if e.messages[0] == 'Aap flag with this Name and Condition already exists.': - logger.info(f"Feature flag: {flag['name']} already exists") - else: - error_msg = f"Invalid feature flag: {flag['name']}. Error: {e}" - logger.error(error_msg) - def delete_feature_flags(): - """ - If a feature flag has been removed from the platform flags list, delete it from the database. - """ - all_flags = apps.get_model('dab_feature_flags', 'AAPFlag').objects.all() - for flag in all_flags: - found = False - for _flag in AAP_FEATURE_FLAGS: - if flag.name == _flag['name'] and flag.condition == _flag['condition']: - found = True - break - if found: - continue - if not found: - logger.info(f"Deleting feature flag: {flag.name} as it is no longer available as a platform flag") - flag.delete() +def load_feature_flags(): + """ + Loads in all feature flags into the database. Updates them if necessary. + """ + feature_flags_model = apps.get_model('dab_feature_flags', 'AAPFlag') + for flag in AAP_FEATURE_FLAGS: + try: + existing_flag = feature_flags_model.objects.filter(name=flag['name'], condition=flag['condition']) + if existing_flag: + feature_flag = update_feature_flag(existing_flag.first(), flag) + else: + if hasattr(settings, flag['name']): + flag['value'] = getattr(settings, flag['name']) + feature_flag = feature_flags_model(**flag) + feature_flag.full_clean() + feature_flag.save() + except ValidationError as e: + # Ignore this error unless better way to bypass this + if e.messages[0] == 'Aap flag with this Name and Condition already exists.': + logger.info(f"Feature flag: {flag['name']} already exists") + else: + error_msg = f"Invalid feature flag: {flag['name']}. Error: {e}" + logger.error(error_msg) - delete_feature_flags() - load_feature_flags() + +def delete_feature_flags(): + """ + If a feature flag has been removed from the platform flags list, delete it from the database. + """ + all_flags = apps.get_model('dab_feature_flags', 'AAPFlag').objects.all() + for flag in all_flags: + found = False + for _flag in AAP_FEATURE_FLAGS: + if flag.name == _flag['name'] and flag.condition == _flag['condition']: + found = True + break + if found: + continue + if not found: + logger.info(f"Deleting feature flag: {flag.name} as it is no longer available as a platform flag") + flag.delete() diff --git a/ansible_base/lib/dynamic_config/settings_logic.py b/ansible_base/lib/dynamic_config/settings_logic.py index 35851b438..2c48172d5 100644 --- a/ansible_base/lib/dynamic_config/settings_logic.py +++ b/ansible_base/lib/dynamic_config/settings_logic.py @@ -306,7 +306,12 @@ def get_mergeable_dab_settings(settings: dict) -> dict: # NOSONAR if "flags" not in installed_apps: installed_apps.append('flags') - dab_data['FLAG_SOURCES'] = ('ansible_base.feature_flags.flag_source.AAPFlagSource',) + # After all flags are migrated to database flags, remove settings flag source + # Settings flag source is defined for compatibility until migration is complete + dab_data['FLAG_SOURCES'] = ( + 'flags.sources.SettingsFlagsSource', + 'ansible_base.feature_flags.flag_source.AAPFlagSource', + ) found_template_backend = False template_context_processor = 'django.template.context_processors.request' diff --git a/ansible_base/lib/testing/fixtures.py b/ansible_base/lib/testing/fixtures.py index 2177fc432..af2e37595 100644 --- a/ansible_base/lib/testing/fixtures.py +++ b/ansible_base/lib/testing/fixtures.py @@ -366,6 +366,13 @@ def ldap_authenticator(ldap_configuration): delete_authenticator(authenticator) +@pytest.fixture +def aap_flags(): + from ansible_base.feature_flags.utils import create_initial_data + + create_initial_data() + + @pytest.fixture def create_mock_method(): # Creates a function that when called, generates a function that can be used to patch diff --git a/requirements/requirements_dev.txt b/requirements/requirements_dev.txt index 1b651c8e4..045942be3 100644 --- a/requirements/requirements_dev.txt +++ b/requirements/requirements_dev.txt @@ -17,6 +17,7 @@ pytest-asyncio pytest-xdist pytest-cov pytest-django +pytest-mock setuptools-scm sqlparse==0.5.2 psycopg[binary] diff --git a/test_app/defaults.py b/test_app/defaults.py index ba8be1ee9..11ffbd67f 100644 --- a/test_app/defaults.py +++ b/test_app/defaults.py @@ -205,29 +205,3 @@ RENAMED_USERNAME_PREFIX = "dab:" JUST_A_TEST = 41 - -FLAGS = { - "FEATURE_SOME_PLATFORM_FLAG_ENABLED": [ - { - "condition": "boolean", - "value": False, - "required": True, - }, - { - "condition": "before date", - "value": '2022-06-01T12:00Z', - }, - ], - "FEATURE_SOME_PLATFORM_FLAG_FOO_ENABLED": [ - { - "condition": "boolean", - "value": False, - }, - ], - "FEATURE_SOME_PLATFORM_FLAG_BAR_ENABLED": [ - { - "condition": "boolean", - "value": True, - }, - ], -} diff --git a/test_app/tests/feature_flags/management/__init__.py b/test_app/tests/feature_flags/management/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test_app/tests/feature_flags/management/test_feature_flags.py b/test_app/tests/feature_flags/management/test_feature_flags.py new file mode 100644 index 000000000..8e6325a18 --- /dev/null +++ b/test_app/tests/feature_flags/management/test_feature_flags.py @@ -0,0 +1,212 @@ +import io +from unittest import mock + +import pytest +from django.conf import settings # Import settings +from django.core.management import call_command + +# Ensure settings are configured before importing models or command +if not settings.configured: + settings.configure() # Minimal configuration for tests + + +class MockAAPFlag: + def __init__(self, name, ui_name, value, support_level, visibility, toggle_type, description, support_url): + self.name = name + self.ui_name = ui_name + self.value = value + self.support_level = support_level + self.visibility = visibility + self.toggle_type = toggle_type + self.description = description + self.support_url = support_url + + def __str__(self): + return self.name + + +HAS_TABULATE_PATH = 'ansible_base.feature_flags.management.commands.feature_flags.HAS_TABULATE' +COMMAND_MODULE_PATH = 'ansible_base.feature_flags.management.commands.feature_flags' + + +@pytest.fixture +def mock_flags_data(): + return [ + MockAAPFlag( + name="flag1", + ui_name="Flag One", + value=True, + support_level="supported", + visibility="public", + toggle_type="boolean", + description="Description for flag one", + support_url="http://example.com/flag1", + ), + MockAAPFlag( + name="flag2", + ui_name="Flag Two", + value=False, + support_level="experimental", + visibility="internal", + toggle_type="string", + description="Description for flag two", + support_url="http://example.com/flag2", + ), + ] + + +@pytest.mark.django_db(transaction=False) +@mock.patch(f'{COMMAND_MODULE_PATH}.flag_state') +@mock.patch(f'{COMMAND_MODULE_PATH}.AAPFlag.objects') +def test_list_feature_flags_with_tabulate(mock_aap_flag_objects, mock_flag_state, mock_flags_data, capsys): + mock_aap_flag_objects.all.return_value.order_by.return_value = mock_flags_data + + mock_flag_state.side_effect = lambda name: next(f.value for f in mock_flags_data if f.name == name) + + with mock.patch(HAS_TABULATE_PATH, True): # Simulate tabulate is installed + with mock.patch(f'{COMMAND_MODULE_PATH}.tabulate') as mock_tabulate_func: + # Mock the tabulate function to check its call and control its output for simplicity + # or let it run if you want to test its actual output formatting + mock_tabulate_func.return_value = "mocked_tabulate_output" + + call_command('feature_flags', '--list') + + captured = capsys.readouterr() + + # Check headers (they are part of the data passed to tabulate) + expected_headers = ["Name", "UI_Name", "Value", "State", "Support Level", "Visibility", "Toggle Type", "Description", "Support URL"] + + # Check that tabulate was called + assert mock_tabulate_func.called + + # Check the arguments passed to tabulate + args, kwargs = mock_tabulate_func.call_args + passed_data = args[0] + passed_headers = args[1] + passed_tablefmt = kwargs.get('tablefmt') + + assert passed_headers == expected_headers + assert passed_tablefmt == "github" + + assert len(passed_data) == 2 + assert passed_data[0] == [ + 'flag1', + 'Flag One', + 'True', + 'True', + 'supported', + 'public', + 'boolean', + 'Description for flag one', + 'http://example.com/flag1', + ] + assert passed_data[1] == [ + 'flag2', + 'Flag Two', + 'False', + 'False', + 'experimental', + 'internal', + 'string', + 'Description for flag two', + 'http://example.com/flag2', + ] + + assert "mocked_tabulate_output" in captured.out + assert captured.out.strip() == "mocked_tabulate_output" + + +@pytest.mark.django_db(transaction=False) +@mock.patch(f'{COMMAND_MODULE_PATH}.flag_state') +@mock.patch(f'{COMMAND_MODULE_PATH}.AAPFlag.objects') +def test_list_feature_flags_without_tabulate(mock_aap_flag_objects, mock_flag_state, mock_flags_data, capsys): + mock_aap_flag_objects.all.return_value.order_by.return_value = mock_flags_data + mock_flag_state.side_effect = lambda name: next(f.value for f in mock_flags_data if f.name == name) + + with mock.patch(HAS_TABULATE_PATH, False): # Simulate tabulate is NOT installed + call_command('feature_flags', '--list') + + captured = capsys.readouterr() + output_lines = captured.out.strip().split('\n') + + expected_headers_str = "\t".join(["Name", "UI_Name", "Value", "State", "Support Level", "Visibility", "Toggle Type", "Description", "Support URL"]) + + assert output_lines[0] == expected_headers_str + + expected_data_row1 = "\t".join( + ['flag1', 'Flag One', 'True', 'True', 'supported', 'public', 'boolean', 'Description for flag one', 'http://example.com/flag1'] + ) + expected_data_row2 = "\t".join( + ['flag2', 'Flag Two', 'False', 'False', 'experimental', 'internal', 'string', 'Description for flag two', 'http://example.com/flag2'] + ) + + assert output_lines[1] == expected_data_row1 + assert output_lines[2] == expected_data_row2 + assert len(output_lines) == 3 # Headers + 2 data rows + + +@pytest.mark.django_db(transaction=False) +@mock.patch(f'{COMMAND_MODULE_PATH}.flag_state') # Still need to mock this even if no flags +@mock.patch(f'{COMMAND_MODULE_PATH}.AAPFlag.objects') +def test_list_feature_flags_no_flags_with_tabulate(mock_aap_flag_objects, mock_flag_state, capsys): + mock_aap_flag_objects.all.return_value.order_by.return_value = [] # No flags + + with mock.patch(HAS_TABULATE_PATH, True): + with mock.patch(f'{COMMAND_MODULE_PATH}.tabulate') as mock_tabulate_func: + mock_tabulate_func.return_value = "mocked_empty_table" + + call_command('feature_flags', '--list') + + captured = capsys.readouterr() + + assert mock_tabulate_func.called + args, kwargs = mock_tabulate_func.call_args + assert args[0] == [] + assert args[1] == ["Name", "UI_Name", "Value", "State", "Support Level", "Visibility", "Toggle Type", "Description", "Support URL"] + assert kwargs.get('tablefmt') == "github" + + assert "mocked_empty_table" in captured.out.strip() + + +@pytest.mark.django_db(transaction=False) +@mock.patch(f'{COMMAND_MODULE_PATH}.flag_state') +@mock.patch(f'{COMMAND_MODULE_PATH}.AAPFlag.objects') +def test_list_feature_flags_no_flags_without_tabulate(mock_aap_flag_objects, mock_flag_state, capsys): + mock_aap_flag_objects.all.return_value.order_by.return_value = [] # No flags + + with mock.patch(HAS_TABULATE_PATH, False): + call_command('feature_flags', '--list') + + captured = capsys.readouterr() + output_lines = captured.out.strip().split('\n') + + expected_headers_str = "\t".join(["Name", "UI_Name", "Value", "State", "Support Level", "Visibility", "Toggle Type", "Description", "Support URL"]) + + assert output_lines[0] == expected_headers_str + assert len(output_lines) == 1 # Only headers + + +def test_handle_no_options(): + # This test is to ensure that if no options (like --list) are passed, + # the command doesn't error out and list_feature_flags is not called. + # We expect it to do nothing based on the provided handle method. + stdout = io.StringIO() + stderr = io.StringIO() + + # Patch list_feature_flags to ensure it's not called + with mock.patch(f'{COMMAND_MODULE_PATH}.Command.list_feature_flags') as mock_list_method: + call_command('feature_flags', stdout=stdout, stderr=stderr) # No arguments + mock_list_method.assert_not_called() + assert stdout.getvalue() == "" + assert stderr.getvalue() == "" + + +@pytest.mark.django_db +def test_management_command_existing_data(aap_flags, capsys): + from ansible_base.feature_flags.feature_flags import AAP_FEATURE_FLAGS + + call_command('feature_flags', '--list') + + captured = capsys.readouterr() + output_lines = captured.out.strip().split('\n') + assert len(output_lines) - 2 == len(AAP_FEATURE_FLAGS) # Subtract 2 to remove header and '---' line before data diff --git a/test_app/tests/feature_flags/models/__init__.py b/test_app/tests/feature_flags/models/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test_app/tests/feature_flags/models/test_aap_flag.py b/test_app/tests/feature_flags/models/test_aap_flag.py new file mode 100644 index 000000000..168e91290 --- /dev/null +++ b/test_app/tests/feature_flags/models/test_aap_flag.py @@ -0,0 +1,49 @@ +import pytest +from django.conf import settings + +from ansible_base.feature_flags.feature_flags import AAP_FEATURE_FLAGS +from ansible_base.feature_flags.models import AAPFlag + + +@pytest.mark.django_db +def test_total_platform_flags(aap_flags): + assert AAPFlag.objects.count() == 6 + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "feature_flag", + AAP_FEATURE_FLAGS, +) +def test_feature_flags_from_db(aap_flags, feature_flag): + flag = AAPFlag.objects.get(name=feature_flag['name']) + assert flag + assert feature_flag.get('ui_name') == flag.ui_name + assert feature_flag.get('condition') == flag.condition + assert feature_flag.get('visibility') == flag.visibility + assert feature_flag.get('value') == flag.value + assert feature_flag.get('support_level') == flag.support_level + assert feature_flag.get('description') == flag.description + assert feature_flag.get('support_url') == flag.support_url + assert feature_flag.get('labels') == flag.labels + assert feature_flag.get('required', False) == flag.required + assert feature_flag.get('toggle_type', 'run-time') == flag.toggle_type + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "feature_flag, value", + [ + ('FEATURE_INDIRECT_NODE_COUNTING_ENABLED', True), + ('FEATURE_POLICY_AS_CODE_ENABLED', True), + ('FEATURE_EDA_ANALYTICS_ENABLED', False), + ('FEATURE_GATEWAY_IPV6_USAGE_ENABLED', False), + ], +) +def test_feature_flag_database_setting_override(feature_flag, value): + from ansible_base.feature_flags.utils import create_initial_data + + setattr(settings, feature_flag, value) + create_initial_data() + flag = AAPFlag.objects.get(name=feature_flag) + assert flag.value == str(value) diff --git a/test_app/tests/feature_flags/test_utils.py b/test_app/tests/feature_flags/test_utils.py new file mode 100644 index 000000000..96bfc46d0 --- /dev/null +++ b/test_app/tests/feature_flags/test_utils.py @@ -0,0 +1,389 @@ +from unittest.mock import MagicMock, call + +import pytest +from django.core.exceptions import ValidationError + +MODULE_PATH = "ansible_base.feature_flags.utils" + + +# Mock the AAPFlag model structure that apps.get_model would return +# and instances that the model manager would operate on. +class MockAAPFlagInstance: + def __init__(self, **kwargs): + self.name = kwargs.get('name') + self.condition = kwargs.get('condition') + self.value = kwargs.get('value') + self.support_level = kwargs.get('support_level') + self.visibility = kwargs.get('visibility') + self.ui_name = kwargs.get('ui_name') + self.support_url = kwargs.get('support_url') + self.required = kwargs.get('required', False) + self.toggle_type = kwargs.get('toggle_type', 'run-time') + self.labels = kwargs.get('labels', []) + self.description = kwargs.get('description', '') + # Add save, full_clean, delete methods that can be spied on or controlled + self.save = MagicMock() + self.full_clean = MagicMock() + self.delete = MagicMock() + + # To allow attribute setting like existing.support_level = ... + def __setattr__(self, key, value): + super().__setattr__(key, value) + if key not in ['save', 'full_clean', 'delete']: + pass + + +@pytest.fixture +def mock_aap_flag_model_cls(mocker): + model_class = MagicMock(spec_set=['objects']) + + def model_constructor(**kwargs): + instance = MockAAPFlagInstance(**kwargs) + instance.save = mocker.MagicMock() + instance.full_clean = mocker.MagicMock() + instance.delete = mocker.MagicMock() + return instance + + model_class.side_effect = model_constructor + model_class.return_value = MagicMock(spec=MockAAPFlagInstance) + + # Mock the manager + model_class.objects = MagicMock() + model_class.objects.all = MagicMock() + model_class.objects.filter = MagicMock() + + return model_class + + +@pytest.fixture +def mock_apps_get_model(mocker, mock_aap_flag_model_cls): + return mocker.patch(f"{MODULE_PATH}.apps.get_model", return_value=mock_aap_flag_model_cls) + + +@pytest.fixture +def mock_settings(mocker): + # Patch 'settings' within the utils module + mocked_settings = mocker.patch(f"{MODULE_PATH}.settings") + + # This dictionary will hold the "true" values for our settings attributes. + _settings_attrs = {} + + def _hasattr_callable(name): + return name in _settings_attrs + + def _getattr_callable(name): + if name in _settings_attrs: + return _settings_attrs[name] + raise AttributeError(f"Mock settings has no attribute {name}") + + # Configure the 'hasattr' and 'getattr' attributes on the mocked_settings object. + # This is for when the code explicitly calls settings.hasattr(...) or settings.getattr(...). + mocked_settings.hasattr = mocker.MagicMock(side_effect=_hasattr_callable) + mocked_settings.getattr = mocker.MagicMock(side_effect=_getattr_callable) + + def set_settings_attr(name, value): + # Store the attribute in our local dictionary + _settings_attrs[name] = value + setattr(mocked_settings, name, value) + # Update the side effects for the callable attributes 'hasattr' and 'getattr' + # to use the latest state of _settings_attrs. + mocked_settings.hasattr.side_effect = lambda n: n in _settings_attrs + mocked_settings.getattr.side_effect = lambda n: (_settings_attrs[n] if n in _settings_attrs else AttributeError(f"Mock settings has no attribute {n}")) + + mocked_settings.set_attr = set_settings_attr + return mocked_settings + + +@pytest.fixture +def mock_logger(mocker): + logger_instance = mocker.MagicMock() + mocker.patch(f"{MODULE_PATH}.logger", logger_instance) + return logger_instance + + +@pytest.fixture +def mock_aap_feature_flags_constant(mocker): + return mocker.patch(f"{MODULE_PATH}.AAP_FEATURE_FLAGS", []) + + +def test_get_django_flags(mocker): + from ansible_base.feature_flags.utils import get_django_flags + + mock_internal_get_flags = mocker.patch(f"{MODULE_PATH}.get_flags") + mock_internal_get_flags.return_value = {"FLAG_X": True} + + result = get_django_flags() + + mock_internal_get_flags.assert_called_once() + assert result == {"FLAG_X": True} + + +class TestCreateInitialData: + + @pytest.mark.django_db # May not be strictly necessary with all the mocking, but good practice + def test_load_feature_flags_creates_new_flag_from_settings_value( + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + ): + from ansible_base.feature_flags.utils import create_initial_data + + flag_def = { + 'name': 'NEW_FLAG', + 'condition': 'some.condition', + 'ui_name': 'New Flag', + 'support_level': 'tech_preview', + 'visibility': 'public', + # No 'value' here, expecting it from settings + } + mock_aap_feature_flags_constant.append(flag_def) + + # --- Mocks for database interaction (for load_feature_flags part) --- + mock_filter_queryset = MagicMock() + # Simulate flag does NOT exist: + mock_filter_queryset.first.return_value = None # Crucial: .first() should return None + mock_filter_queryset.exists.return_value = False # If your code uses .exists() + # Crucial: The queryset itself should be falsy if evaluated in a boolean context (e.g. if queryset:) + # The error log showed a call to .__bool__(), so this is necessary. + mock_filter_queryset.__bool__ = lambda self: False + + mock_aap_flag_model_cls.objects.filter.return_value = mock_filter_queryset + + mock_settings.set_attr('NEW_FLAG', True) + + mock_constructed_instance = MockAAPFlagInstance( + name=flag_def['name'], # Initialize with expected attributes for robustness + condition=flag_def['condition'], + # You can add other relevant fields from flag_def if needed by your code before save + ) + mock_aap_flag_model_cls.side_effect = [mock_constructed_instance] + + mock_aap_flag_model_cls.objects.all.return_value = [] + + # --- Call the function under test --- + create_initial_data() + + # --- Assertions --- + # Assert that Model.objects.filter was called correctly to check for existence + mock_aap_flag_model_cls.objects.filter.assert_called_with(name='NEW_FLAG', condition='some.condition') + + # Assert that the model class was called (instantiated) with the correct arguments + expected_constructor_args = { + 'name': 'NEW_FLAG', + 'condition': 'some.condition', + 'ui_name': 'New Flag', + 'support_level': 'tech_preview', + 'visibility': 'public', + 'value': True, # Crucially, this should now be True from settings + } + mock_aap_flag_model_cls.assert_called_once_with(**expected_constructor_args) + + # Assert that methods were called on the *instance* returned by the constructor + mock_constructed_instance.full_clean.assert_called_once() + mock_constructed_instance.save.assert_called_once() + + @pytest.mark.django_db + def test_load_feature_flags_creates_new_flag_with_default_value_if_not_in_settings( + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + ): + from ansible_base.feature_flags.utils import create_initial_data + + flag_def = { + 'name': 'NEW_FLAG_DEF_VAL', + 'condition': 'another.condition', + 'ui_name': 'New Flag Def Val', + 'support_level': 'supported', + 'visibility': 'private', + 'value': False, # Default value in definition + } + mock_aap_feature_flags_constant.extend([flag_def]) + + mock_empty_queryset = MagicMock() + mock_empty_queryset.first.return_value = None + mock_empty_queryset.__bool__ = lambda self: False + mock_aap_flag_model_cls.objects.filter.return_value = mock_empty_queryset + + mock_constructed_flag = MockAAPFlagInstance() + mock_aap_flag_model_cls.side_effect = [mock_constructed_flag] + + mock_aap_flag_model_cls.objects.all.return_value = [] # For delete_feature_flags + + create_initial_data() + + mock_aap_flag_model_cls.objects.filter.assert_called_with(name='NEW_FLAG_DEF_VAL', condition='another.condition') + mock_aap_flag_model_cls.assert_called_once_with(**flag_def) # value comes from flag_def + + mock_constructed_flag.full_clean.assert_called_once() + mock_constructed_flag.save.assert_called_once() + + @pytest.mark.django_db + def test_load_feature_flags_updates_existing_flag( + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + ): + from ansible_base.feature_flags.utils import create_initial_data + + flag_def_updated = { + 'name': 'EXISTING_FLAG', + 'condition': 'cond1', + 'ui_name': 'Updated UI Name', + 'support_level': 'beta', + 'visibility': 'internal', + 'support_url': 'new.url', + 'required': True, + 'toggle_type': 'static', + 'labels': ['new'], + 'description': 'new desc', + } + mock_aap_feature_flags_constant.extend([flag_def_updated]) + + existing_db_flag = MockAAPFlagInstance( + name='EXISTING_FLAG', + condition='cond1', + ui_name='Old UI Name', + support_level='alpha', + visibility='public', + support_url='old.url', + required=False, + toggle_type='run-time', + labels=['old'], + description='old desc', + ) + + mock_existing_queryset = MagicMock() + mock_existing_queryset.first.return_value = existing_db_flag + mock_existing_queryset.__bool__ = lambda self: True + mock_aap_flag_model_cls.objects.filter.return_value = mock_existing_queryset + + mock_aap_flag_model_cls.objects.all.return_value = [existing_db_flag] + + create_initial_data() + + mock_aap_flag_model_cls.objects.filter.assert_called_with(name='EXISTING_FLAG', condition='cond1') + + # Assert that the existing_db_flag instance was updated + assert existing_db_flag.ui_name == 'Updated UI Name' + assert existing_db_flag.support_level == 'beta' + assert existing_db_flag.visibility == 'internal' + assert existing_db_flag.support_url == 'new.url' + assert existing_db_flag.required is True + assert existing_db_flag.toggle_type == 'static' + assert existing_db_flag.labels == ['new'] + assert existing_db_flag.description == 'new desc' + + existing_db_flag.full_clean.assert_called_once() + existing_db_flag.save.assert_called_once() + mock_aap_flag_model_cls.assert_not_called() # No new instance created + + @pytest.mark.django_db + def test_load_feature_flags_handles_specific_validation_error( + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + ): + from ansible_base.feature_flags.utils import create_initial_data + + flag_def = {'name': 'ERROR_FLAG', 'condition': 'err_cond', 'ui_name': 'Error Flag'} + mock_aap_feature_flags_constant.extend([flag_def]) + + mock_empty_queryset = MagicMock() + mock_empty_queryset.first.return_value = None + mock_empty_queryset.__bool__ = lambda self: False + mock_aap_flag_model_cls.objects.filter.return_value = mock_empty_queryset + + mock_created_instance = MockAAPFlagInstance(**flag_def) + validation_error = ValidationError('Aap flag with this Name and Condition already exists.') + mock_created_instance.save.side_effect = validation_error + + mock_aap_flag_model_cls.side_effect = [mock_created_instance] + + mock_aap_flag_model_cls.objects.all.return_value = [] + + create_initial_data() + + mock_logger.info.assert_called_once_with("Feature flag: ERROR_FLAG already exists") + mock_logger.error.assert_not_called() + mock_created_instance.full_clean.assert_called_once() + + @pytest.mark.django_db + def test_load_feature_flags_logs_other_validation_errors( + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + ): + from ansible_base.feature_flags.utils import create_initial_data + + flag_def = {'name': 'OTHER_ERROR_FLAG', 'condition': 'other_err_cond', 'ui_name': 'Other Error'} + mock_aap_feature_flags_constant.extend([flag_def]) + + mock_empty_queryset = MagicMock() + mock_empty_queryset.first.return_value = None + mock_empty_queryset.__bool__ = lambda self: False + mock_aap_flag_model_cls.objects.filter.return_value = mock_empty_queryset + + mock_created_instance = MockAAPFlagInstance(**flag_def) + validation_error = ValidationError('Some other validation error.') + mock_created_instance.full_clean.side_effect = validation_error + + mock_aap_flag_model_cls.side_effect = [mock_created_instance] + + mock_aap_flag_model_cls.objects.all.return_value = [] + + create_initial_data() + + mock_logger.error.assert_called_once_with(f"Invalid feature flag: {flag_def['name']}. Error: {validation_error}") + mock_logger.info.assert_not_called() + mock_created_instance.save.assert_not_called() + + @pytest.mark.django_db + def test_delete_feature_flags_removes_obsolete_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_aap_feature_flags_constant): + from ansible_base.feature_flags.utils import create_initial_data + + obsolete_flag_in_db = MockAAPFlagInstance(name='OBSOLETE_FLAG', condition='obs_cond') + + mock_aap_flag_model_cls.objects.all.return_value = [obsolete_flag_in_db] + mock_empty_queryset = MagicMock() + mock_empty_queryset.first.return_value = None + mock_empty_queryset.__bool__ = lambda self: False + mock_aap_flag_model_cls.objects.filter.return_value = mock_empty_queryset + + create_initial_data() + + mock_aap_flag_model_cls.objects.all.assert_called_once() + obsolete_flag_in_db.delete.assert_called_once() + mock_logger.info.assert_any_call(f"Deleting feature flag: {obsolete_flag_in_db.name} as it is no longer available as a platform flag") + + @pytest.mark.django_db + def test_delete_feature_flags_keeps_current_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_aap_feature_flags_constant): + from ansible_base.feature_flags.utils import create_initial_data + + current_flag_def = {'name': 'CURRENT_FLAG', 'condition': 'curr_cond', 'ui_name': 'Current'} + mock_aap_feature_flags_constant.extend([current_flag_def]) + + current_flag_in_db = MockAAPFlagInstance(name='CURRENT_FLAG', condition='curr_cond') + + mock_aap_flag_model_cls.objects.all.return_value = [current_flag_in_db] + + # For load_feature_flags part (update existing) + mock_existing_queryset = MagicMock() + mock_existing_queryset.first.return_value = current_flag_in_db + mock_existing_queryset.__bool__ = lambda self: True + mock_aap_flag_model_cls.objects.filter.return_value = mock_existing_queryset + + create_initial_data() + + current_flag_in_db.delete.assert_not_called() + # Check that logger.info for deletion was not called for this flag + for call_arg in mock_logger.info.call_args_list: + assert "Deleting feature flag: CURRENT_FLAG" not in call_arg[0][0] + + current_flag_in_db.save.assert_called_once() + + def test_create_initial_data_call_order(self, mocker): + from ansible_base.feature_flags.utils import create_initial_data + + # Mock the inner functions directly to check call order + mock_delete = mocker.patch(f"{MODULE_PATH}.delete_feature_flags") + mock_load = mocker.patch(f"{MODULE_PATH}.load_feature_flags") + + manager = MagicMock() + manager.attach_mock(mock_delete, 'delete_flags') + manager.attach_mock(mock_load, 'load_flags') + + create_initial_data() + + expected_calls = [call.delete_flags(), call.load_flags()] + assert manager.mock_calls == expected_calls diff --git a/test_app/tests/feature_flags/views/__init__.py b/test_app/tests/feature_flags/views/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test_app/tests/feature_flags/views/test_feature_flag.py b/test_app/tests/feature_flags/views/test_feature_flag.py new file mode 100644 index 000000000..2a87a1936 --- /dev/null +++ b/test_app/tests/feature_flags/views/test_feature_flag.py @@ -0,0 +1,106 @@ +import pytest +from django.conf import settings + +from ansible_base.feature_flags.models import AAPFlag +from ansible_base.lib.utils.response import get_relative_url + + +def test_feature_flags_list_empty_by_default(admin_api_client): + """ + Test that we can list feature flags api, before preloading data + """ + url = get_relative_url("aap_flags-list") + response = admin_api_client.get(url) + assert response.status_code == 200 + assert response.data['results'] == [] + + +def test_feature_flags_list(admin_api_client, aap_flags): + """ + Test that we can list feature flags api, after preloading data + """ + url = get_relative_url("aap_flags-list") + response = admin_api_client.get(url) + assert response.status_code == 200 + assert len(response.data['results']) == 6 + + +@pytest.mark.parametrize( + 'feature_flag, value', + [ + ('FEATURE_INDIRECT_NODE_COUNTING_ENABLED', True), + ('FEATURE_POLICY_AS_CODE_ENABLED', True), + ('FEATURE_EDA_ANALYTICS_ENABLED', False), + ('FEATURE_GATEWAY_IPV6_USAGE_ENABLED', False), + ('FEATURE_GATEWAY_CREATE_CRC_SERVICE_TYPE_ENABLED', True), + ], +) +def test_feature_flags_detail(admin_api_client, feature_flag, value): + """ + Test that we can list feature flags api, after preloading data + """ + from ansible_base.feature_flags.utils import create_initial_data + + setattr(settings, feature_flag, value) + create_initial_data() + try: + created_flag = AAPFlag.objects.get(name=feature_flag) + except AAPFlag.DoesNotExist: + pytest.fail(f"AAPFlag with name '{feature_flag}' was not found in the database") + url = get_relative_url("aap_flags-detail", kwargs={'pk': created_flag.pk}) + response = admin_api_client.get(url) + assert response.status_code == 200 + assert response.data['name'] == feature_flag + assert response.data['state'] == value + + +@pytest.mark.parametrize( + 'flags_list', + [ + [ + {'name': 'FEATURE_INDIRECT_NODE_COUNTING_ENABLED', 'value': True}, + {'name': 'FEATURE_POLICY_AS_CODE_ENABLED', 'value': True}, + ], + [ + {'name': 'FEATURE_GATEWAY_IPV6_USAGE_ENABLED', 'value': False}, + {'name': 'FEATURE_GATEWAY_CREATE_CRC_SERVICE_TYPE_ENABLED', 'value': True}, + ], + ], +) +def test_feature_flags_states_list(admin_api_client, flags_list): + """ + Test that we can list feature flags api, after preloading data + """ + from ansible_base.feature_flags.utils import create_initial_data + + for flag in flags_list: + setattr(settings, flag['name'], flag['value']) + expected_flag_states = {item['name']: item['value'] for item in flags_list} + + create_initial_data() + url = get_relative_url("aap_flags_states-list") + response = admin_api_client.get(url) + assert response.status_code == 200 + assert len(response.data['results']) == 6 + + found_and_verified_flags_count = 0 + for flag_from_api in response.data['results']: + api_flag_name = flag_from_api.get('flag_name') + if api_flag_name in expected_flag_states: + found_and_verified_flags_count += 1 + expected_value = expected_flag_states[api_flag_name] + actual_value = flag_from_api.get('flag_state') + assert actual_value == expected_value + + # Assert that all flags you intended to check were actually found in the API response and verified + assert found_and_verified_flags_count == len(expected_flag_states) + + +def test_old_feature_flags_list(admin_api_client, aap_flags): + """ + Test that we can list feature flags api, after preloading data + """ + url = get_relative_url("featureflags-list") + response = admin_api_client.get(url) + assert response.status_code == 200 + assert len(response.data) == 6 From 48ad36ab62e2e9e52d2cdb481fd4676dd6a00184 Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Thu, 5 Jun 2025 15:35:06 -0400 Subject: [PATCH 08/12] Refactor view to gateway --- ansible_base/feature_flags/apps.py | 9 ++++ .../feature_flags/migrations/0001_initial.py | 4 +- ansible_base/feature_flags/models/aap_flag.py | 6 ++- ansible_base/feature_flags/serializers.py | 20 -------- ansible_base/feature_flags/urls.py | 2 - ansible_base/feature_flags/utils.py | 10 +++- ansible_base/feature_flags/views.py | 13 +---- .../lib/dynamic_config/settings_logic.py | 7 +-- ansible_base/rbac/permission_registry.py | 6 --- .../feature_flags/models/test_aap_flag.py | 1 + .../feature_flags/views/test_feature_flag.py | 50 +------------------ 11 files changed, 28 insertions(+), 100 deletions(-) diff --git a/ansible_base/feature_flags/apps.py b/ansible_base/feature_flags/apps.py index cfe665a64..39cbd90eb 100644 --- a/ansible_base/feature_flags/apps.py +++ b/ansible_base/feature_flags/apps.py @@ -1,4 +1,7 @@ from django.apps import AppConfig +from django.db.models.signals import post_migrate + +from ansible_base.feature_flags.utils import create_initial_data class FeatureFlagsConfig(AppConfig): @@ -6,3 +9,9 @@ class FeatureFlagsConfig(AppConfig): name = 'ansible_base.feature_flags' label = 'dab_feature_flags' verbose_name = 'Feature Flags' + + def ready(self): + try: + create_initial_data() + except Exception: + post_migrate.connect(create_initial_data, sender=self) diff --git a/ansible_base/feature_flags/migrations/0001_initial.py b/ansible_base/feature_flags/migrations/0001_initial.py index fa9d52b13..97a30b94f 100644 --- a/ansible_base/feature_flags/migrations/0001_initial.py +++ b/ansible_base/feature_flags/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.17 on 2025-06-04 12:00 +# Generated by Django 4.2.17 on 2025-06-06 16:22 import ansible_base.feature_flags.models.aap_flag from django.conf import settings @@ -24,7 +24,7 @@ class Migration(migrations.Migration): ('name', models.CharField(help_text='The name of the feature flag. Must follow the format of FEATURE__ENABLED.', max_length=64, validators=[ansible_base.feature_flags.models.aap_flag.validate_feature_flag_name])), ('ui_name', models.CharField(help_text='The pretty name to display in the application User Interface', max_length=64)), ('condition', models.CharField(default='boolean', help_text='Used to specify a condition, which if met, will enable the feature flag.', max_length=64)), - ('value', models.CharField(default='True', help_text='The value used to evaluate the conditional specified.', max_length=127)), + ('value', models.CharField(default='False', help_text='The value used to evaluate the conditional specified.', max_length=127)), ('required', models.BooleanField(default=False, help_text="If multiple conditions are required to be met to enable a feature flag, 'required' can be used to specify the necessary conditionals.")), ('support_level', models.CharField(choices=[('NOT_FOR_USE', 'Not for use'), ('NOT_FOR_PRODUCTION', 'Not for production'), ('READY_FOR_PRODUCTION', 'Ready for production')], help_text='The support criteria for the feature flag. Must be one of (NOT_FOR_USE, NOT_FOR_PRODUCTION, READY_FOR_PRODUCTION).', max_length=25)), ('visibility', models.CharField(choices=[('public', 'public'), ('private', 'private')], help_text='The visibility level of the feature flag. If private, flag is hidden.', max_length=20)), diff --git a/ansible_base/feature_flags/models/aap_flag.py b/ansible_base/feature_flags/models/aap_flag.py index 0158c4af3..82019d6c8 100644 --- a/ansible_base/feature_flags/models/aap_flag.py +++ b/ansible_base/feature_flags/models/aap_flag.py @@ -35,7 +35,11 @@ def __str__(self): ) ui_name = models.CharField(max_length=64, null=False, blank=False, help_text=_("The pretty name to display in the application User Interface")) condition = models.CharField(max_length=64, default="boolean", help_text=_("Used to specify a condition, which if met, will enable the feature flag.")) - value = models.CharField(max_length=127, default="True", help_text=_("The value used to evaluate the conditional specified.")) + value = models.CharField( + max_length=127, + default="False", + help_text=_("The value used to evaluate the conditional specified."), + ) required = models.BooleanField( default=False, help_text=_("If multiple conditions are required to be met to enable a feature flag, 'required' can be used to specify the necessary conditionals."), diff --git a/ansible_base/feature_flags/serializers.py b/ansible_base/feature_flags/serializers.py index 0cb0c9e70..3f5217c06 100644 --- a/ansible_base/feature_flags/serializers.py +++ b/ansible_base/feature_flags/serializers.py @@ -1,5 +1,4 @@ from flags.state import flag_state -from rest_framework import serializers from ansible_base.feature_flags.models import AAPFlag from ansible_base.lib.serializers.common import NamedCommonModelSerializer @@ -7,25 +6,6 @@ from .utils import get_django_flags -class FeatureFlagSerializer(NamedCommonModelSerializer): - """Serialize list of feature flags""" - - state = serializers.SerializerMethodField() - - class Meta: - model = AAPFlag - fields = NamedCommonModelSerializer.Meta.fields + [x.name for x in AAPFlag._meta.concrete_fields] + ['state'] - read_only_fields = ["name", "condition", "required", "support_level", "visibility", "toggle_type", "description", "labels", "ui_name", "support_url"] - - def get_state(self, instance): - return flag_state(instance.name) - - def to_representation(self, instance): - instance.state = True - ret = super().to_representation(instance) - return ret - - # TODO: Remove once all components are migrated to the new endpont. class OldFeatureFlagSerializer(NamedCommonModelSerializer): """Serialize list of feature flags""" diff --git a/ansible_base/feature_flags/urls.py b/ansible_base/feature_flags/urls.py index 4631e7ec6..9c791fd25 100644 --- a/ansible_base/feature_flags/urls.py +++ b/ansible_base/feature_flags/urls.py @@ -9,8 +9,6 @@ router = AssociationResourceRouter() router.register(r'feature_flags/states', views.FeatureFlagsStatesView, basename='aap_flags_states') -router.register(r'feature_flags', views.FeatureFlagsView, basename='aap_flags') - # TODO: Remove once all components are migrated to new endpoints. api_version_urls = [path('feature_flags_state/', views.OldFeatureFlagsStateListView.as_view(), name='feature-flags-state-list'), path('', include(router.urls))] diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index 6285a864d..cc996389d 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -42,6 +42,8 @@ def load_feature_flags(): """ Loads in all feature flags into the database. Updates them if necessary. """ + from ansible_base.resource_registry.signals.handlers import no_reverse_sync + feature_flags_model = apps.get_model('dab_feature_flags', 'AAPFlag') for flag in AAP_FEATURE_FLAGS: try: @@ -53,7 +55,8 @@ def load_feature_flags(): flag['value'] = getattr(settings, flag['name']) feature_flag = feature_flags_model(**flag) feature_flag.full_clean() - feature_flag.save() + with no_reverse_sync(): + feature_flag.save() except ValidationError as e: # Ignore this error unless better way to bypass this if e.messages[0] == 'Aap flag with this Name and Condition already exists.': @@ -67,6 +70,8 @@ def delete_feature_flags(): """ If a feature flag has been removed from the platform flags list, delete it from the database. """ + from ansible_base.resource_registry.signals.handlers import no_reverse_sync + all_flags = apps.get_model('dab_feature_flags', 'AAPFlag').objects.all() for flag in all_flags: found = False @@ -78,4 +83,5 @@ def delete_feature_flags(): continue if not found: logger.info(f"Deleting feature flag: {flag.name} as it is no longer available as a platform flag") - flag.delete() + with no_reverse_sync(): + flag.delete() diff --git a/ansible_base/feature_flags/views.py b/ansible_base/feature_flags/views.py index a6ed8a858..e362a736e 100644 --- a/ansible_base/feature_flags/views.py +++ b/ansible_base/feature_flags/views.py @@ -6,7 +6,7 @@ from rest_framework.viewsets import ModelViewSet from ansible_base.feature_flags.models import AAPFlag -from ansible_base.feature_flags.serializers import FeatureFlagSerializer, OldFeatureFlagSerializer +from ansible_base.feature_flags.serializers import OldFeatureFlagSerializer from ansible_base.lib.utils.views.ansible_base import AnsibleBaseView from ansible_base.lib.utils.views.django_app_api import AnsibleBaseDjangoAppApiView from ansible_base.lib.utils.views.permissions import IsSuperuserOrAuditor, try_add_oauth2_scope_permission @@ -15,17 +15,6 @@ from .utils import get_django_flags -class FeatureFlagsView(AnsibleBaseDjangoAppApiView, ModelViewSet): - """ - A view class for displaying feature flags - """ - - queryset = AAPFlag.objects.order_by('id') - serializer_class = FeatureFlagSerializer - permission_classes = try_add_oauth2_scope_permission([IsSuperuserOrAuditor]) - http_method_names = ['get', 'head', 'options'] - - class FeatureFlagsStatesView(AnsibleBaseDjangoAppApiView, ModelViewSet): """ A view class for displaying feature flags states diff --git a/ansible_base/lib/dynamic_config/settings_logic.py b/ansible_base/lib/dynamic_config/settings_logic.py index 2c48172d5..35851b438 100644 --- a/ansible_base/lib/dynamic_config/settings_logic.py +++ b/ansible_base/lib/dynamic_config/settings_logic.py @@ -306,12 +306,7 @@ def get_mergeable_dab_settings(settings: dict) -> dict: # NOSONAR if "flags" not in installed_apps: installed_apps.append('flags') - # After all flags are migrated to database flags, remove settings flag source - # Settings flag source is defined for compatibility until migration is complete - dab_data['FLAG_SOURCES'] = ( - 'flags.sources.SettingsFlagsSource', - 'ansible_base.feature_flags.flag_source.AAPFlagSource', - ) + dab_data['FLAG_SOURCES'] = ('ansible_base.feature_flags.flag_source.AAPFlagSource',) found_template_backend = False template_context_processor = 'django.template.context_processors.request' diff --git a/ansible_base/rbac/permission_registry.py b/ansible_base/rbac/permission_registry.py index 263eadd6f..1603eba9b 100644 --- a/ansible_base/rbac/permission_registry.py +++ b/ansible_base/rbac/permission_registry.py @@ -143,7 +143,6 @@ def create_managed_roles(self, apps) -> list[tuple[Model, bool]]: return ret def call_when_apps_ready(self, apps, app_config) -> None: - from ansible_base.feature_flags.utils import create_initial_data as feature_flag_create_initial_data from ansible_base.rbac import triggers from ansible_base.rbac.evaluations import bound_has_obj_perm, bound_singleton_permissions, connect_rbac_methods from ansible_base.rbac.management import create_dab_permissions @@ -173,11 +172,6 @@ def call_when_apps_ready(self, apps, app_config) -> None: sender=app_config, dispatch_uid="ansible_base.rbac.triggers.post_migration_rbac_setup", ) - if 'ansible_base.feature_flags' in settings.INSTALLED_APPS: - try: - feature_flag_create_initial_data() - except Exception: - post_migrate.connect(feature_flag_create_initial_data, sender=self) self.user_model.add_to_class('has_obj_perm', bound_has_obj_perm) self.user_model.add_to_class('singleton_permissions', bound_singleton_permissions) diff --git a/test_app/tests/feature_flags/models/test_aap_flag.py b/test_app/tests/feature_flags/models/test_aap_flag.py index 168e91290..e495c9926 100644 --- a/test_app/tests/feature_flags/models/test_aap_flag.py +++ b/test_app/tests/feature_flags/models/test_aap_flag.py @@ -41,6 +41,7 @@ def test_feature_flags_from_db(aap_flags, feature_flag): ], ) def test_feature_flag_database_setting_override(feature_flag, value): + AAPFlag.objects.all().delete() from ansible_base.feature_flags.utils import create_initial_data setattr(settings, feature_flag, value) diff --git a/test_app/tests/feature_flags/views/test_feature_flag.py b/test_app/tests/feature_flags/views/test_feature_flag.py index 2a87a1936..6a42f444b 100644 --- a/test_app/tests/feature_flags/views/test_feature_flag.py +++ b/test_app/tests/feature_flags/views/test_feature_flag.py @@ -5,55 +5,6 @@ from ansible_base.lib.utils.response import get_relative_url -def test_feature_flags_list_empty_by_default(admin_api_client): - """ - Test that we can list feature flags api, before preloading data - """ - url = get_relative_url("aap_flags-list") - response = admin_api_client.get(url) - assert response.status_code == 200 - assert response.data['results'] == [] - - -def test_feature_flags_list(admin_api_client, aap_flags): - """ - Test that we can list feature flags api, after preloading data - """ - url = get_relative_url("aap_flags-list") - response = admin_api_client.get(url) - assert response.status_code == 200 - assert len(response.data['results']) == 6 - - -@pytest.mark.parametrize( - 'feature_flag, value', - [ - ('FEATURE_INDIRECT_NODE_COUNTING_ENABLED', True), - ('FEATURE_POLICY_AS_CODE_ENABLED', True), - ('FEATURE_EDA_ANALYTICS_ENABLED', False), - ('FEATURE_GATEWAY_IPV6_USAGE_ENABLED', False), - ('FEATURE_GATEWAY_CREATE_CRC_SERVICE_TYPE_ENABLED', True), - ], -) -def test_feature_flags_detail(admin_api_client, feature_flag, value): - """ - Test that we can list feature flags api, after preloading data - """ - from ansible_base.feature_flags.utils import create_initial_data - - setattr(settings, feature_flag, value) - create_initial_data() - try: - created_flag = AAPFlag.objects.get(name=feature_flag) - except AAPFlag.DoesNotExist: - pytest.fail(f"AAPFlag with name '{feature_flag}' was not found in the database") - url = get_relative_url("aap_flags-detail", kwargs={'pk': created_flag.pk}) - response = admin_api_client.get(url) - assert response.status_code == 200 - assert response.data['name'] == feature_flag - assert response.data['state'] == value - - @pytest.mark.parametrize( 'flags_list', [ @@ -73,6 +24,7 @@ def test_feature_flags_states_list(admin_api_client, flags_list): """ from ansible_base.feature_flags.utils import create_initial_data + AAPFlag.objects.all().delete() for flag in flags_list: setattr(settings, flag['name'], flag['value']) expected_flag_states = {item['name']: item['value'] for item in flags_list} From 25381a6c64d9ca9e102768b375afb96d267a0528 Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Mon, 9 Jun 2025 08:42:22 -0400 Subject: [PATCH 09/12] Change flags list to yaml list --- ansible_base/feature_flags/feature_flags.py | 85 ------------------- ansible_base/feature_flags/feature_flags.yaml | 61 +++++++++++++ ansible_base/feature_flags/utils.py | 18 +++- .../management/test_authenticators.py | 2 +- .../management/test_feature_flags.py | 4 +- .../feature_flags/models/test_aap_flag.py | 4 +- test_app/tests/feature_flags/test_old_api.py | 2 +- test_app/tests/feature_flags/test_utils.py | 32 +++---- .../feature_flags/views/test_feature_flag.py | 2 +- .../lib/utils/test_create_system_user.py | 1 + 10 files changed, 99 insertions(+), 112 deletions(-) delete mode 100644 ansible_base/feature_flags/feature_flags.py create mode 100644 ansible_base/feature_flags/feature_flags.yaml diff --git a/ansible_base/feature_flags/feature_flags.py b/ansible_base/feature_flags/feature_flags.py deleted file mode 100644 index 9e3ec0144..000000000 --- a/ansible_base/feature_flags/feature_flags.py +++ /dev/null @@ -1,85 +0,0 @@ -from typing import TypedDict - - -class AAPFlagNameSchema(TypedDict): - name: str - ui_name: str - condition: str - value: str - required: str - support_level: str - visibility: str - toggle_type: str - description: str - support_url: str - labels: list - - -AAP_FEATURE_FLAGS: list[AAPFlagNameSchema] = [ - AAPFlagNameSchema( - name="FEATURE_INDIRECT_NODE_COUNTING_ENABLED", - ui_name="Indirect Node Counting", - visibility="public", - condition="boolean", - value="False", - support_level="NOT_FOR_PRODUCTION", - description="TBD", - support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", - labels=['controller'], - ), - AAPFlagNameSchema( - name="FEATURE_POLICY_AS_CODE_ENABLED", - ui_name="Policy as Code", - visibility="public", - condition="boolean", - value="False", - support_level="NOT_FOR_PRODUCTION", - description="TBD", - support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", - labels=['controller'], - ), - AAPFlagNameSchema( - name="FEATURE_EDA_ANALYTICS_ENABLED", - ui_name="Event-Driven Ansible Analytics", - condition="boolean", - value="False", - visibility="public", - support_level="NOT_FOR_PRODUCTION", - description="TBD", - support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", - labels=['eda'], - ), - AAPFlagNameSchema( - name="FEATURE_GATEWAY_IPV6_USAGE_ENABLED", - ui_name="Gateway IPv6 Usage", - condition="boolean", - value="False", - visibility="private", - support_level="NOT_FOR_PRODUCTION", - description="TBD", - support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", - labels=['gateway'], - ), - AAPFlagNameSchema( - name="FEATURE_GATEWAY_CREATE_CRC_SERVICE_TYPE_ENABLED", - ui_name="Gateway Create CRC Service Type", - condition="boolean", - value="False", - visibility="private", - support_level="NOT_FOR_PRODUCTION", - description="TBD", - support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", - labels=['gateway'], - ), - AAPFlagNameSchema( - name="FEATURE_DISPATCHERD_ENABLED", - ui_name="AAP Dispatcherd", - condition="boolean", - value="False", - visibility="private", - support_level="NOT_FOR_PRODUCTION", - description="TBD", - support_url="https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/", - labels=['eda', 'controller'], - ), -] diff --git a/ansible_base/feature_flags/feature_flags.yaml b/ansible_base/feature_flags/feature_flags.yaml new file mode 100644 index 000000000..d05a2ce29 --- /dev/null +++ b/ansible_base/feature_flags/feature_flags.yaml @@ -0,0 +1,61 @@ +- name: FEATURE_INDIRECT_NODE_COUNTING_ENABLED + ui_name: Indirect Node Counting + visibility: public + condition: boolean + value: 'False' + support_level: NOT_FOR_PRODUCTION + description: TBD + support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + labels: + - controller +- name: FEATURE_POLICY_AS_CODE_ENABLED + ui_name: Policy as Code + visibility: public + condition: boolean + value: 'False' + support_level: NOT_FOR_PRODUCTION + description: TBD + support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + labels: + - controller +- name: FEATURE_EDA_ANALYTICS_ENABLED + ui_name: Event-Driven Ansible Analytics + visibility: public + condition: boolean + value: 'False' + support_level: NOT_FOR_PRODUCTION + description: TBD + support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + labels: + - eda +- name: FEATURE_GATEWAY_IPV6_USAGE_ENABLED + ui_name: Gateway IPv6 Usage + visibility: private + condition: boolean + value: 'False' + support_level: NOT_FOR_PRODUCTION + description: TBD + support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + labels: + - gateway +- name: FEATURE_GATEWAY_CREATE_CRC_SERVICE_TYPE_ENABLED + ui_name: Gateway Create CRC Service Type + visibility: private + condition: boolean + value: 'False' + support_level: NOT_FOR_PRODUCTION + description: TBD + support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + labels: + - gateway +- name: FEATURE_DISPATCHERD_ENABLED + ui_name: AAP Dispatcherd + visibility: private + condition: boolean + value: 'False' + support_level: NOT_FOR_PRODUCTION + description: TBD + support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + labels: + - eda + - controller diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index cc996389d..b5ae756b3 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -1,12 +1,12 @@ import logging +from pathlib import Path +import yaml from django.apps import apps from django.conf import settings from django.core.exceptions import ValidationError from flags.sources import get_flags -from ansible_base.feature_flags.feature_flags import AAP_FEATURE_FLAGS - logger = logging.getLogger('ansible_base.feature_flags.utils') @@ -14,6 +14,16 @@ def get_django_flags(): return get_flags() +def feature_flags_list(): + current_dir = Path(__file__).parent + flags_list_file = current_dir / 'feature_flags.yaml' + with open(flags_list_file, 'r') as file: + try: + return yaml.safe_load(file) + except yaml.YAMLError as exc: + print(exc) + + def create_initial_data(**kwargs): # NOSONAR """ Loads in platform feature flags when the server starts @@ -45,7 +55,7 @@ def load_feature_flags(): from ansible_base.resource_registry.signals.handlers import no_reverse_sync feature_flags_model = apps.get_model('dab_feature_flags', 'AAPFlag') - for flag in AAP_FEATURE_FLAGS: + for flag in feature_flags_list(): try: existing_flag = feature_flags_model.objects.filter(name=flag['name'], condition=flag['condition']) if existing_flag: @@ -75,7 +85,7 @@ def delete_feature_flags(): all_flags = apps.get_model('dab_feature_flags', 'AAPFlag').objects.all() for flag in all_flags: found = False - for _flag in AAP_FEATURE_FLAGS: + for _flag in feature_flags_list(): if flag.name == _flag['name'] and flag.condition == _flag['condition']: found = True break diff --git a/test_app/tests/authentication/management/test_authenticators.py b/test_app/tests/authentication/management/test_authenticators.py index b2fc302d4..cc0d39a36 100644 --- a/test_app/tests/authentication/management/test_authenticators.py +++ b/test_app/tests/authentication/management/test_authenticators.py @@ -97,7 +97,7 @@ def test_authenticators_cli_initialize( err = StringIO() # Sanity check: - assert django_user_model.objects.count() == 0 + assert django_user_model.objects.count() == 1 # Optionally create admin user if admin_user_exists: diff --git a/test_app/tests/feature_flags/management/test_feature_flags.py b/test_app/tests/feature_flags/management/test_feature_flags.py index 8e6325a18..46507d708 100644 --- a/test_app/tests/feature_flags/management/test_feature_flags.py +++ b/test_app/tests/feature_flags/management/test_feature_flags.py @@ -203,10 +203,10 @@ def test_handle_no_options(): @pytest.mark.django_db def test_management_command_existing_data(aap_flags, capsys): - from ansible_base.feature_flags.feature_flags import AAP_FEATURE_FLAGS + from ansible_base.feature_flags.utils import feature_flags_list call_command('feature_flags', '--list') captured = capsys.readouterr() output_lines = captured.out.strip().split('\n') - assert len(output_lines) - 2 == len(AAP_FEATURE_FLAGS) # Subtract 2 to remove header and '---' line before data + assert len(output_lines) - 2 == len(feature_flags_list()) # Subtract 2 to remove header and '---' line before data diff --git a/test_app/tests/feature_flags/models/test_aap_flag.py b/test_app/tests/feature_flags/models/test_aap_flag.py index e495c9926..a45fe7243 100644 --- a/test_app/tests/feature_flags/models/test_aap_flag.py +++ b/test_app/tests/feature_flags/models/test_aap_flag.py @@ -1,8 +1,8 @@ import pytest from django.conf import settings -from ansible_base.feature_flags.feature_flags import AAP_FEATURE_FLAGS from ansible_base.feature_flags.models import AAPFlag +from ansible_base.feature_flags.utils import feature_flags_list @pytest.mark.django_db @@ -13,7 +13,7 @@ def test_total_platform_flags(aap_flags): @pytest.mark.django_db @pytest.mark.parametrize( "feature_flag", - AAP_FEATURE_FLAGS, + feature_flags_list(), ) def test_feature_flags_from_db(aap_flags, feature_flag): flag = AAPFlag.objects.get(name=feature_flag['name']) diff --git a/test_app/tests/feature_flags/test_old_api.py b/test_app/tests/feature_flags/test_old_api.py index 71a0aff9d..2b1b659f1 100644 --- a/test_app/tests/feature_flags/test_old_api.py +++ b/test_app/tests/feature_flags/test_old_api.py @@ -7,6 +7,6 @@ def test_feature_flags_state_api_list(admin_api_client: APIClient): """ Test that we can list all feature flags """ - url = get_relative_url("featureflags-list") + url = get_relative_url("feature-flags-state-list") response = admin_api_client.get(url) assert response.status_code == 200 diff --git a/test_app/tests/feature_flags/test_utils.py b/test_app/tests/feature_flags/test_utils.py index 96bfc46d0..5879db8ff 100644 --- a/test_app/tests/feature_flags/test_utils.py +++ b/test_app/tests/feature_flags/test_utils.py @@ -102,8 +102,9 @@ def mock_logger(mocker): @pytest.fixture -def mock_aap_feature_flags_constant(mocker): - return mocker.patch(f"{MODULE_PATH}.AAP_FEATURE_FLAGS", []) +def mock_feature_flags_list(mocker): + mock = mocker.patch(f"{MODULE_PATH}.feature_flags_list") + return mock def test_get_django_flags(mocker): @@ -122,7 +123,7 @@ class TestCreateInitialData: @pytest.mark.django_db # May not be strictly necessary with all the mocking, but good practice def test_load_feature_flags_creates_new_flag_from_settings_value( - self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_feature_flags_list, mocker ): from ansible_base.feature_flags.utils import create_initial_data @@ -134,8 +135,7 @@ def test_load_feature_flags_creates_new_flag_from_settings_value( 'visibility': 'public', # No 'value' here, expecting it from settings } - mock_aap_feature_flags_constant.append(flag_def) - + mock_feature_flags_list.return_value = [flag_def] # --- Mocks for database interaction (for load_feature_flags part) --- mock_filter_queryset = MagicMock() # Simulate flag does NOT exist: @@ -182,7 +182,7 @@ def test_load_feature_flags_creates_new_flag_from_settings_value( @pytest.mark.django_db def test_load_feature_flags_creates_new_flag_with_default_value_if_not_in_settings( - self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_feature_flags_list, mocker ): from ansible_base.feature_flags.utils import create_initial_data @@ -194,7 +194,7 @@ def test_load_feature_flags_creates_new_flag_with_default_value_if_not_in_settin 'visibility': 'private', 'value': False, # Default value in definition } - mock_aap_feature_flags_constant.extend([flag_def]) + mock_feature_flags_list.return_value = [flag_def] mock_empty_queryset = MagicMock() mock_empty_queryset.first.return_value = None @@ -216,7 +216,7 @@ def test_load_feature_flags_creates_new_flag_with_default_value_if_not_in_settin @pytest.mark.django_db def test_load_feature_flags_updates_existing_flag( - self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_feature_flags_list, mocker ): from ansible_base.feature_flags.utils import create_initial_data @@ -232,7 +232,7 @@ def test_load_feature_flags_updates_existing_flag( 'labels': ['new'], 'description': 'new desc', } - mock_aap_feature_flags_constant.extend([flag_def_updated]) + mock_feature_flags_list.return_value = [flag_def_updated] existing_db_flag = MockAAPFlagInstance( name='EXISTING_FLAG', @@ -274,12 +274,12 @@ def test_load_feature_flags_updates_existing_flag( @pytest.mark.django_db def test_load_feature_flags_handles_specific_validation_error( - self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_feature_flags_list, mocker ): from ansible_base.feature_flags.utils import create_initial_data flag_def = {'name': 'ERROR_FLAG', 'condition': 'err_cond', 'ui_name': 'Error Flag'} - mock_aap_feature_flags_constant.extend([flag_def]) + mock_feature_flags_list.return_value = [flag_def] mock_empty_queryset = MagicMock() mock_empty_queryset.first.return_value = None @@ -302,12 +302,12 @@ def test_load_feature_flags_handles_specific_validation_error( @pytest.mark.django_db def test_load_feature_flags_logs_other_validation_errors( - self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_aap_feature_flags_constant, mocker + self, mock_apps_get_model, mock_aap_flag_model_cls, mock_settings, mock_logger, mock_feature_flags_list, mocker ): from ansible_base.feature_flags.utils import create_initial_data flag_def = {'name': 'OTHER_ERROR_FLAG', 'condition': 'other_err_cond', 'ui_name': 'Other Error'} - mock_aap_feature_flags_constant.extend([flag_def]) + mock_feature_flags_list.return_value = [flag_def] mock_empty_queryset = MagicMock() mock_empty_queryset.first.return_value = None @@ -329,7 +329,7 @@ def test_load_feature_flags_logs_other_validation_errors( mock_created_instance.save.assert_not_called() @pytest.mark.django_db - def test_delete_feature_flags_removes_obsolete_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_aap_feature_flags_constant): + def test_delete_feature_flags_removes_obsolete_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_feature_flags_list): from ansible_base.feature_flags.utils import create_initial_data obsolete_flag_in_db = MockAAPFlagInstance(name='OBSOLETE_FLAG', condition='obs_cond') @@ -347,11 +347,11 @@ def test_delete_feature_flags_removes_obsolete_flag(self, mock_apps_get_model, m mock_logger.info.assert_any_call(f"Deleting feature flag: {obsolete_flag_in_db.name} as it is no longer available as a platform flag") @pytest.mark.django_db - def test_delete_feature_flags_keeps_current_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_aap_feature_flags_constant): + def test_delete_feature_flags_keeps_current_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_feature_flags_list): from ansible_base.feature_flags.utils import create_initial_data current_flag_def = {'name': 'CURRENT_FLAG', 'condition': 'curr_cond', 'ui_name': 'Current'} - mock_aap_feature_flags_constant.extend([current_flag_def]) + mock_feature_flags_list.return_value = [current_flag_def] current_flag_in_db = MockAAPFlagInstance(name='CURRENT_FLAG', condition='curr_cond') diff --git a/test_app/tests/feature_flags/views/test_feature_flag.py b/test_app/tests/feature_flags/views/test_feature_flag.py index 6a42f444b..745c4608d 100644 --- a/test_app/tests/feature_flags/views/test_feature_flag.py +++ b/test_app/tests/feature_flags/views/test_feature_flag.py @@ -52,7 +52,7 @@ def test_old_feature_flags_list(admin_api_client, aap_flags): """ Test that we can list feature flags api, after preloading data """ - url = get_relative_url("featureflags-list") + url = get_relative_url("feature-flags-state-list") response = admin_api_client.get(url) assert response.status_code == 200 assert len(response.data) == 6 diff --git a/test_app/tests/lib/utils/test_create_system_user.py b/test_app/tests/lib/utils/test_create_system_user.py index e9950ce02..8f2137e99 100644 --- a/test_app/tests/lib/utils/test_create_system_user.py +++ b/test_app/tests/lib/utils/test_create_system_user.py @@ -73,6 +73,7 @@ def test_get_system_user_from_basic_model(self): @pytest.mark.django_db def test_get_system_user_from_managed_model(self): + User.all_objects.filter(username=get_system_username()[0]).delete() create_system_user(user_model=ManagedUser) assert ManagedUser.objects.filter(username=get_system_username()[0]).count() == 0 From 740140df773a95aa604620abcf1976db73d805ac Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Wed, 11 Jun 2025 14:48:00 -0400 Subject: [PATCH 10/12] Add documentation and JSON Schema validation --- .../{ => definitions}/feature_flags.yaml | 0 .../feature_flags/definitions/schema.json | 79 +++++++++++++++++++ ansible_base/feature_flags/flag_source.py | 10 +-- ansible_base/feature_flags/serializers.py | 19 +++++ ansible_base/feature_flags/utils.py | 2 +- ansible_base/feature_flags/views.py | 15 +--- docs/apps/feature_flags.md | 77 +++++++++--------- requirements/requirements_dev.txt | 1 + test_app/tests/feature_flags/test_utils.py | 20 +++++ .../feature_flags/views/test_feature_flag.py | 4 +- 10 files changed, 165 insertions(+), 62 deletions(-) rename ansible_base/feature_flags/{ => definitions}/feature_flags.yaml (100%) create mode 100644 ansible_base/feature_flags/definitions/schema.json diff --git a/ansible_base/feature_flags/feature_flags.yaml b/ansible_base/feature_flags/definitions/feature_flags.yaml similarity index 100% rename from ansible_base/feature_flags/feature_flags.yaml rename to ansible_base/feature_flags/definitions/feature_flags.yaml diff --git a/ansible_base/feature_flags/definitions/schema.json b/ansible_base/feature_flags/definitions/schema.json new file mode 100644 index 000000000..1e4bafbf6 --- /dev/null +++ b/ansible_base/feature_flags/definitions/schema.json @@ -0,0 +1,79 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Feature Flag Configuration Schema", + "description": "Validates a list of feature flag configurations.", + "type": "array", + "items": { + "type": "object", + "properties": { + "name": { + "description": "The unique identifier for the feature flag. Must be in all capitals and start with 'FEATURE_' and end with '_ENABLED'", + "type": "string", + "pattern": "^FEATURE_[A-Z0-9_]+_ENABLED$" + }, + "ui_name": { + "description": "The human-readable name for the feature flag displayed in the UI.", + "type": "string", + "minLength": 1 + }, + "visibility": { + "description": "Controls whether the feature is visible in the UI.", + "type": "string", + "enum": ["public", "private"] + }, + "condition": { + "description": "The type of condition for the feature flag's value. Currently only boolean is supported.", + "type": "string", + "enum": ["boolean"] + }, + "value": { + "description": "The default value of the feature flag, as a string.", + "type": "string", + "enum": ["True", "False"] + }, + "support_level": { + "description": "The level of support provided for this feature.", + "type": "string", + "enum": [ + "NOT_FOR_USE", + "NOT_FOR_PRODUCTION", + "READY_FOR_PRODUCTION" + ] + }, + "description": { + "description": "A brief explanation of what the feature does.", + "type": "string" + }, + "support_url": { + "description": "A URL to the relevant documentation for the feature.", + "type": "string", + "format": "uri" + }, + "toggle_type": { + "description": "The actual value of the feature flag. Note: The YAML string 'False' or 'True' is parsed as a boolean.", + "type": "string", + "enum": ["install-time", "run-time"] + }, + "labels": { + "description": "A list of labels to categorize the feature.", + "type": "array", + "items": { + "type": "string", + "enum": ["controller", "eda", "gateway", "platform"] + }, + "minItems": 1, + "uniqueItems": true + } + }, + "required": [ + "name", + "ui_name", + "visibility", + "condition", + "value", + "support_level", + "description", + "support_url" + ] + } +} diff --git a/ansible_base/feature_flags/flag_source.py b/ansible_base/feature_flags/flag_source.py index f6511ce09..93a1a5afd 100644 --- a/ansible_base/feature_flags/flag_source.py +++ b/ansible_base/feature_flags/flag_source.py @@ -2,14 +2,6 @@ from flags.sources import Condition -class DatabaseCondition(Condition): - """Condition that includes the AAPFlags database object""" - - def __init__(self, condition, value, required=False, obj=None): - super().__init__(condition, value, required=required) - self.obj = obj - - class AAPFlagSource(object): def get_queryset(self): @@ -21,5 +13,5 @@ def get_flags(self): for o in self.get_queryset(): if o.name not in flags: flags[o.name] = [] - flags[o.name].append(DatabaseCondition(o.condition, o.value, required=o.required, obj=o)) + flags[o.name].append(Condition(o.condition, o.value, required=o.required)) return flags diff --git a/ansible_base/feature_flags/serializers.py b/ansible_base/feature_flags/serializers.py index 3f5217c06..271358373 100644 --- a/ansible_base/feature_flags/serializers.py +++ b/ansible_base/feature_flags/serializers.py @@ -1,4 +1,5 @@ from flags.state import flag_state +from rest_framework import serializers from ansible_base.feature_flags.models import AAPFlag from ansible_base.lib.serializers.common import NamedCommonModelSerializer @@ -6,6 +7,24 @@ from .utils import get_django_flags +class FeatureFlagStatesSerializer(NamedCommonModelSerializer): + """Serialize list of feature flags""" + + state = serializers.SerializerMethodField() + + def get_state(self, instance): + return flag_state(instance.name) + + class Meta: + model = AAPFlag + fields = ["name", "state"] + + def to_representation(self, instance=None) -> dict: + instance.state = True + ret = super().to_representation(instance) + return ret + + # TODO: Remove once all components are migrated to the new endpont. class OldFeatureFlagSerializer(NamedCommonModelSerializer): """Serialize list of feature flags""" diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index b5ae756b3..0719a543a 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -16,7 +16,7 @@ def get_django_flags(): def feature_flags_list(): current_dir = Path(__file__).parent - flags_list_file = current_dir / 'feature_flags.yaml' + flags_list_file = current_dir / 'definitions/feature_flags.yaml' with open(flags_list_file, 'r') as file: try: return yaml.safe_load(file) diff --git a/ansible_base/feature_flags/views.py b/ansible_base/feature_flags/views.py index e362a736e..15b474d1b 100644 --- a/ansible_base/feature_flags/views.py +++ b/ansible_base/feature_flags/views.py @@ -1,16 +1,13 @@ from django.conf import settings from django.utils.translation import gettext_lazy as _ -from flags.sources import get_flags -from flags.state import flag_state from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet from ansible_base.feature_flags.models import AAPFlag -from ansible_base.feature_flags.serializers import OldFeatureFlagSerializer +from ansible_base.feature_flags.serializers import FeatureFlagStatesSerializer, OldFeatureFlagSerializer from ansible_base.lib.utils.views.ansible_base import AnsibleBaseView from ansible_base.lib.utils.views.django_app_api import AnsibleBaseDjangoAppApiView from ansible_base.lib.utils.views.permissions import IsSuperuserOrAuditor, try_add_oauth2_scope_permission -from ansible_base.rest_pagination import DefaultPaginator from .utils import get_django_flags @@ -22,17 +19,9 @@ class FeatureFlagsStatesView(AnsibleBaseDjangoAppApiView, ModelViewSet): queryset = AAPFlag.objects.order_by('id') permission_classes = try_add_oauth2_scope_permission([IsSuperuserOrAuditor]) + serializer_class = FeatureFlagStatesSerializer http_method_names = ['get', 'head', 'options'] - def list(self, request): - paginator = DefaultPaginator() - flags = get_flags() - ret = [] - for flag in flags: - ret.append({"flag_name": flag, "flag_state": flag_state(flag)}) - result_page = paginator.paginate_queryset(ret, request) - return paginator.get_paginated_response(result_page) - # TODO: This can be removed after functionality is migrated over to new class class OldFeatureFlagsStateListView(AnsibleBaseView): diff --git a/docs/apps/feature_flags.md b/docs/apps/feature_flags.md index d371a44e9..11013864a 100644 --- a/docs/apps/feature_flags.md +++ b/docs/apps/feature_flags.md @@ -5,55 +5,32 @@ Additional library documentation can be found at https://cfpb.github.io/django-f ## Settings -Add `ansible_base.feature_flags` to your installed apps: +Add `ansible_base.feature_flags` to your installed apps and ensure `ansible_base.resource_registry` as added to enable flag state to sync across the platform: ```python INSTALLED_APPS = [ ... 'ansible_base.feature_flags', + 'ansible_base.resource_registry', # Must also be added ] ``` -### Additional Settings +## Detail -Additional settings are required to enable feature_flags. -This will happen automatically if using [dynamic_settings](../Installation.md) - -First, you need to add `flags` to your `INSTALLED_APPS`: +By adding the `ansible_base.feature_flags` app to your application, all Ansible Automation Platform feature flags will be loaded and available in your component. +To receive flag state updates, ensure the following definition is available in your components `RESOURCE_LIST` - ```python -INSTALLED_APPS = [ - ... - 'flags', - ... -] -``` - -Additionally, create a `FLAGS` entry: - -```python -FLAGS = {} -``` - -Finally, add `django.template.context_processors.request` to your `TEMPLATES` `context_processors` setting: +from ansible_base.feature_flags.models import AAPFlag +from ansible_base.resource_registry.shared_types import FeatureFlagType -```python -TEMPLATES = [ - { - 'BEACKEND': 'django.template.backends.django.DjangoTemplates', - ... - 'OPTIONS': { - ... - 'context_processors': [ - ... - 'django.template.context_processors.request', - ... - ] - ... - } - ... - } -] +RESOURCE_LIST = ( + ... + ResourceConfig( + AAPFlag, + shared_resource=SharedResource(serializer=FeatureFlagType, is_provider=False), + ), +) ``` ## URLS @@ -70,3 +47,29 @@ urlpatterns = [ ... ] ``` + +## Adding Feature Flags + +To add a feature flag to the platform, specify it in the following [file](../../ansible_base/feature_flags/definitions/feature_flags.yaml) + +An example flag could resemble - + +```yaml +- name: FEATURE_FOO_ENABLED + ui_name: Foo + visibility: public + condition: boolean + value: 'False' + support_level: NOT_FOR_PRODUCTION + description: TBD + support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + labels: + - controller +``` + +Validate this file against the json schema by running `check-jsonschema` - + +```bash +pip install check-jsonschema +check-jsonschema --schemafile ansible_base/feature_flags/definitions/schema.json ansible_base/feature_flags/definitions/feature_flags.yaml +``` diff --git a/requirements/requirements_dev.txt b/requirements/requirements_dev.txt index 045942be3..cc4911b1a 100644 --- a/requirements/requirements_dev.txt +++ b/requirements/requirements_dev.txt @@ -9,6 +9,7 @@ flake8==7.1.1 # Linting tool, if changed update pyproject.toml as well Flake8-pyproject==1.2.3 # Linting tool, if changed update pyproject.toml as well ipython isort==6.0.0 # Linting tool, if changed update pyproject.toml as well +jsonschema tox tox-docker typeguard diff --git a/test_app/tests/feature_flags/test_utils.py b/test_app/tests/feature_flags/test_utils.py index 5879db8ff..4af721d43 100644 --- a/test_app/tests/feature_flags/test_utils.py +++ b/test_app/tests/feature_flags/test_utils.py @@ -1,7 +1,10 @@ +import json from unittest.mock import MagicMock, call import pytest +import yaml from django.core.exceptions import ValidationError +from jsonschema import validate MODULE_PATH = "ansible_base.feature_flags.utils" @@ -119,6 +122,23 @@ def test_get_django_flags(mocker): assert result == {"FLAG_X": True} +def test_validate_flags_yaml_against_json_schema(): + feature_flags_yaml = 'ansible_base/feature_flags/definitions/feature_flags.yaml' + feature_flags_schema = 'ansible_base/feature_flags/definitions/schema.json' + try: + with open(feature_flags_yaml, 'r') as file: + feature_flags_file = yaml.safe_load(file) + with open(feature_flags_schema, 'r') as file: + schema = json.load(file) + validate(instance=feature_flags_file, schema=schema) + assert True, "Validation succeeded as expected." + except FileNotFoundError as e: + pytest.fail(f"Could not find a necessary file: {e}. Make sure schema.json and valid_data.yaml exist.") + except Exception as e: + # If any other exception occurs (like a ValidationError), fail the test. + pytest.fail(f"Validation failed unexpectedly for a valid file: {e}") + + class TestCreateInitialData: @pytest.mark.django_db # May not be strictly necessary with all the mocking, but good practice diff --git a/test_app/tests/feature_flags/views/test_feature_flag.py b/test_app/tests/feature_flags/views/test_feature_flag.py index 745c4608d..b8ea322bd 100644 --- a/test_app/tests/feature_flags/views/test_feature_flag.py +++ b/test_app/tests/feature_flags/views/test_feature_flag.py @@ -37,11 +37,11 @@ def test_feature_flags_states_list(admin_api_client, flags_list): found_and_verified_flags_count = 0 for flag_from_api in response.data['results']: - api_flag_name = flag_from_api.get('flag_name') + api_flag_name = flag_from_api.get('name') if api_flag_name in expected_flag_states: found_and_verified_flags_count += 1 expected_value = expected_flag_states[api_flag_name] - actual_value = flag_from_api.get('flag_state') + actual_value = flag_from_api.get('state') assert actual_value == expected_value # Assert that all flags you intended to check were actually found in the API response and verified From 2ef72a14140c9645872945aa978e9a56132ccc9b Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Tue, 17 Jun 2025 16:19:46 -0400 Subject: [PATCH 11/12] Adjust feature flag metadata --- .../definitions/feature_flags.yaml | 51 ++++++++----------- .../feature_flags/definitions/schema.json | 8 ++- ansible_base/feature_flags/flag_source.py | 14 ++++- .../feature_flags/migrations/0001_initial.py | 6 +-- ansible_base/feature_flags/models/aap_flag.py | 13 ++--- docs/apps/feature_flags.md | 2 +- .../feature_flags/models/test_aap_flag.py | 19 +++++-- .../feature_flags/views/test_feature_flag.py | 7 +-- 8 files changed, 66 insertions(+), 54 deletions(-) diff --git a/ansible_base/feature_flags/definitions/feature_flags.yaml b/ansible_base/feature_flags/definitions/feature_flags.yaml index d05a2ce29..0105d217e 100644 --- a/ansible_base/feature_flags/definitions/feature_flags.yaml +++ b/ansible_base/feature_flags/definitions/feature_flags.yaml @@ -1,61 +1,52 @@ - name: FEATURE_INDIRECT_NODE_COUNTING_ENABLED ui_name: Indirect Node Counting - visibility: public + visibility: True condition: boolean value: 'False' - support_level: NOT_FOR_PRODUCTION - description: TBD - support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ - labels: - - controller -- name: FEATURE_POLICY_AS_CODE_ENABLED - ui_name: Policy as Code - visibility: public - condition: boolean - value: 'False' - support_level: NOT_FOR_PRODUCTION + support_level: TECHNICAL_PREVIEW description: TBD support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ labels: - controller - name: FEATURE_EDA_ANALYTICS_ENABLED ui_name: Event-Driven Ansible Analytics - visibility: public + visibility: True condition: boolean value: 'False' - support_level: NOT_FOR_PRODUCTION - description: TBD - support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + support_level: TECHNICAL_PREVIEW + description: Submit Event-Driven Ansible usage analytics to console.redhat.com + support_url: https://access.redhat.com/solutions/7112810 labels: - eda - name: FEATURE_GATEWAY_IPV6_USAGE_ENABLED - ui_name: Gateway IPv6 Usage - visibility: private + ui_name: Gateway IPv6 Enablement + visibility: False condition: boolean value: 'False' - support_level: NOT_FOR_PRODUCTION - description: TBD - support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + support_level: TECHNICAL_PREVIEW + description: The feature flag represents enabling IPv6 only traffic to be allowed through the gateway component and does not include all components of the platform. + support_url: https://access.redhat.com/articles/7116569 labels: - gateway - name: FEATURE_GATEWAY_CREATE_CRC_SERVICE_TYPE_ENABLED - ui_name: Gateway Create CRC Service Type - visibility: private + ui_name: Dynamic Service Type Feature + visibility: False condition: boolean value: 'False' - support_level: NOT_FOR_PRODUCTION - description: TBD - support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + support_level: DEVELOPER_PREVIEW + description: The Dynamic Service Type feature allows for the introduction of new platform services without requiring registration to the existing database. The new service can be enabled through the use of configuration. + support_url: https://access.redhat.com/articles/7122668 + toggle_type: install-time labels: - gateway - name: FEATURE_DISPATCHERD_ENABLED ui_name: AAP Dispatcherd - visibility: private + visibility: True condition: boolean value: 'False' - support_level: NOT_FOR_PRODUCTION - description: TBD - support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + support_level: DEVELOPER_PREVIEW + description: A service to run python tasks in subprocesses, designed specifically to work well with pg_notify, but intended to be extensible to other message delivery means. + support_url: '' labels: - eda - controller diff --git a/ansible_base/feature_flags/definitions/schema.json b/ansible_base/feature_flags/definitions/schema.json index 1e4bafbf6..cad80d78d 100644 --- a/ansible_base/feature_flags/definitions/schema.json +++ b/ansible_base/feature_flags/definitions/schema.json @@ -18,8 +18,7 @@ }, "visibility": { "description": "Controls whether the feature is visible in the UI.", - "type": "string", - "enum": ["public", "private"] + "type": "boolean" }, "condition": { "description": "The type of condition for the feature flag's value. Currently only boolean is supported.", @@ -35,9 +34,8 @@ "description": "The level of support provided for this feature.", "type": "string", "enum": [ - "NOT_FOR_USE", - "NOT_FOR_PRODUCTION", - "READY_FOR_PRODUCTION" + "TECHNICAL_PREVIEW", + "DEVELOPER_PREVIEW" ] }, "description": { diff --git a/ansible_base/feature_flags/flag_source.py b/ansible_base/feature_flags/flag_source.py index 93a1a5afd..04a2c53b2 100644 --- a/ansible_base/feature_flags/flag_source.py +++ b/ansible_base/feature_flags/flag_source.py @@ -2,7 +2,19 @@ from flags.sources import Condition +class DatabaseCondition(Condition): + """Condition that includes the AAPFlags database object + This is required to ensure that enable_flag/disable_flag calls + can work as expected, with the custom flag objects + """ + + def __init__(self, condition, value, required=False, obj=None): + super().__init__(condition, value, required=required) + self.obj = obj + + class AAPFlagSource(object): + """The customer AAP flag source, retrieves a list of all flags in the database""" def get_queryset(self): aap_flags = apps.get_model('dab_feature_flags', 'AAPFlag') @@ -13,5 +25,5 @@ def get_flags(self): for o in self.get_queryset(): if o.name not in flags: flags[o.name] = [] - flags[o.name].append(Condition(o.condition, o.value, required=o.required)) + flags[o.name].append(DatabaseCondition(o.condition, o.value, required=o.required, obj=o)) return flags diff --git a/ansible_base/feature_flags/migrations/0001_initial.py b/ansible_base/feature_flags/migrations/0001_initial.py index 97a30b94f..ec90f8a92 100644 --- a/ansible_base/feature_flags/migrations/0001_initial.py +++ b/ansible_base/feature_flags/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.17 on 2025-06-06 16:22 +# Generated by Django 4.2.21 on 2025-06-17 20:08 import ansible_base.feature_flags.models.aap_flag from django.conf import settings @@ -26,8 +26,8 @@ class Migration(migrations.Migration): ('condition', models.CharField(default='boolean', help_text='Used to specify a condition, which if met, will enable the feature flag.', max_length=64)), ('value', models.CharField(default='False', help_text='The value used to evaluate the conditional specified.', max_length=127)), ('required', models.BooleanField(default=False, help_text="If multiple conditions are required to be met to enable a feature flag, 'required' can be used to specify the necessary conditionals.")), - ('support_level', models.CharField(choices=[('NOT_FOR_USE', 'Not for use'), ('NOT_FOR_PRODUCTION', 'Not for production'), ('READY_FOR_PRODUCTION', 'Ready for production')], help_text='The support criteria for the feature flag. Must be one of (NOT_FOR_USE, NOT_FOR_PRODUCTION, READY_FOR_PRODUCTION).', max_length=25)), - ('visibility', models.CharField(choices=[('public', 'public'), ('private', 'private')], help_text='The visibility level of the feature flag. If private, flag is hidden.', max_length=20)), + ('support_level', models.CharField(choices=[('DEVELOPER_PREVIEW', 'Developer Preview'), ('TECHNICAL_PREVIEW', 'Technical Preview')], help_text='The support criteria for the feature flag. Must be one of (DEVELOPER_PREVIEW or TECHNICAL_PREVIEW).', max_length=25)), + ('visibility', models.BooleanField(default=False, help_text='The visibility of the feature flag. If false, flag is hidden.')), ('toggle_type', models.CharField(choices=[('install-time', 'install-time'), ('run-time', 'run-time')], default='run-time', help_text="Details whether a flag is toggle-able at run-time or install-time. (Default: 'run-time').", max_length=20)), ('description', models.CharField(default='', help_text='A detailed description giving an overview of the feature flag.', max_length=500)), ('support_url', models.CharField(blank=True, default='', help_text='A link to the documentation support URL for the feature', max_length=250)), diff --git a/ansible_base/feature_flags/models/aap_flag.py b/ansible_base/feature_flags/models/aap_flag.py index 82019d6c8..0e2ee6824 100644 --- a/ansible_base/feature_flags/models/aap_flag.py +++ b/ansible_base/feature_flags/models/aap_flag.py @@ -47,16 +47,13 @@ def __str__(self): support_level = models.CharField( max_length=25, null=False, - help_text=_("The support criteria for the feature flag. Must be one of (NOT_FOR_USE, NOT_FOR_PRODUCTION, READY_FOR_PRODUCTION)."), - choices=(('NOT_FOR_USE', 'Not for use'), ('NOT_FOR_PRODUCTION', 'Not for production'), ('READY_FOR_PRODUCTION', 'Ready for production')), + help_text=_("The support criteria for the feature flag. Must be one of (DEVELOPER_PREVIEW or TECHNICAL_PREVIEW)."), + choices=(('DEVELOPER_PREVIEW', 'Developer Preview'), ('TECHNICAL_PREVIEW', 'Technical Preview')), blank=False, ) - visibility = models.CharField( - max_length=20, - null=False, - choices=[('public', 'public'), ('private', 'private')], - help_text=_("The visibility level of the feature flag. If private, flag is hidden."), - blank=False, + visibility = models.BooleanField( + default=False, + help_text=_("The visibility of the feature flag. If false, flag is hidden."), ) toggle_type = models.CharField( max_length=20, diff --git a/docs/apps/feature_flags.md b/docs/apps/feature_flags.md index 11013864a..6d05f8f2d 100644 --- a/docs/apps/feature_flags.md +++ b/docs/apps/feature_flags.md @@ -57,7 +57,7 @@ An example flag could resemble - ```yaml - name: FEATURE_FOO_ENABLED ui_name: Foo - visibility: public + visibility: True condition: boolean value: 'False' support_level: NOT_FOR_PRODUCTION diff --git a/test_app/tests/feature_flags/models/test_aap_flag.py b/test_app/tests/feature_flags/models/test_aap_flag.py index a45fe7243..119e5a50a 100644 --- a/test_app/tests/feature_flags/models/test_aap_flag.py +++ b/test_app/tests/feature_flags/models/test_aap_flag.py @@ -1,5 +1,6 @@ import pytest from django.conf import settings +from flags.state import disable_flag, enable_flag, flag_state from ansible_base.feature_flags.models import AAPFlag from ansible_base.feature_flags.utils import feature_flags_list @@ -7,7 +8,7 @@ @pytest.mark.django_db def test_total_platform_flags(aap_flags): - assert AAPFlag.objects.count() == 6 + assert AAPFlag.objects.count() == len(feature_flags_list()) @pytest.mark.django_db @@ -35,8 +36,6 @@ def test_feature_flags_from_db(aap_flags, feature_flag): "feature_flag, value", [ ('FEATURE_INDIRECT_NODE_COUNTING_ENABLED', True), - ('FEATURE_POLICY_AS_CODE_ENABLED', True), - ('FEATURE_EDA_ANALYTICS_ENABLED', False), ('FEATURE_GATEWAY_IPV6_USAGE_ENABLED', False), ], ) @@ -48,3 +47,17 @@ def test_feature_flag_database_setting_override(feature_flag, value): create_initial_data() flag = AAPFlag.objects.get(name=feature_flag) assert flag.value == str(value) + + +@pytest.mark.django_db +def test_enable_and_disable_flag_functions(aap_flags): + flag_name = "FEATURE_INDIRECT_NODE_COUNTING_ENABLED" + # Assert Initial State + assert flag_state(flag_name) is False + + # Ensure flag can be enabled via django-flags enable_flag function + enable_flag(flag_name) + assert flag_state(flag_name) is True + # Ensure flag can be disabled via django-flags enable_flag function + disable_flag(flag_name) + assert flag_state(flag_name) is False diff --git a/test_app/tests/feature_flags/views/test_feature_flag.py b/test_app/tests/feature_flags/views/test_feature_flag.py index b8ea322bd..dc22cd1a1 100644 --- a/test_app/tests/feature_flags/views/test_feature_flag.py +++ b/test_app/tests/feature_flags/views/test_feature_flag.py @@ -2,6 +2,7 @@ from django.conf import settings from ansible_base.feature_flags.models import AAPFlag +from ansible_base.feature_flags.utils import feature_flags_list from ansible_base.lib.utils.response import get_relative_url @@ -10,7 +11,7 @@ [ [ {'name': 'FEATURE_INDIRECT_NODE_COUNTING_ENABLED', 'value': True}, - {'name': 'FEATURE_POLICY_AS_CODE_ENABLED', 'value': True}, + {'name': 'FEATURE_EDA_ANALYTICS_ENABLED', 'value': True}, ], [ {'name': 'FEATURE_GATEWAY_IPV6_USAGE_ENABLED', 'value': False}, @@ -33,7 +34,7 @@ def test_feature_flags_states_list(admin_api_client, flags_list): url = get_relative_url("aap_flags_states-list") response = admin_api_client.get(url) assert response.status_code == 200 - assert len(response.data['results']) == 6 + assert len(response.data['results']) == len(feature_flags_list()) found_and_verified_flags_count = 0 for flag_from_api in response.data['results']: @@ -55,4 +56,4 @@ def test_old_feature_flags_list(admin_api_client, aap_flags): url = get_relative_url("feature-flags-state-list") response = admin_api_client.get(url) assert response.status_code == 200 - assert len(response.data) == 6 + assert len(response.data) == len(feature_flags_list()) From 1e9b6aa23aa693599d2e074a36d33b1d34c28e62 Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Mon, 23 Jun 2025 17:33:48 -0400 Subject: [PATCH 12/12] Fix post-migrate call. Replace with manual migrations --- ansible_base/feature_flags/apps.py | 5 +- .../definitions/feature_flags.yaml | 22 ++--- .../feature_flags/definitions/schema.json | 2 +- .../feature_flags/migrations/0001_initial.py | 5 +- .../migrations/example_migration | 21 +++++ ansible_base/feature_flags/models/aap_flag.py | 4 +- ansible_base/feature_flags/utils.py | 6 +- ansible_base/feature_flags/views.py | 4 +- .../resource_registry/shared_types.py | 2 - docs/apps/feature_flags.md | 14 +++- .../feature_flags/migrations/__init__.py | 0 .../migrations/test_migrations.py | 82 +++++++++++++++++++ test_app/tests/feature_flags/test_utils.py | 8 +- 13 files changed, 143 insertions(+), 32 deletions(-) create mode 100644 ansible_base/feature_flags/migrations/example_migration create mode 100644 test_app/tests/feature_flags/migrations/__init__.py create mode 100644 test_app/tests/feature_flags/migrations/test_migrations.py diff --git a/ansible_base/feature_flags/apps.py b/ansible_base/feature_flags/apps.py index 39cbd90eb..94cfee764 100644 --- a/ansible_base/feature_flags/apps.py +++ b/ansible_base/feature_flags/apps.py @@ -11,7 +11,4 @@ class FeatureFlagsConfig(AppConfig): verbose_name = 'Feature Flags' def ready(self): - try: - create_initial_data() - except Exception: - post_migrate.connect(create_initial_data, sender=self) + post_migrate.connect(create_initial_data, sender=self) diff --git a/ansible_base/feature_flags/definitions/feature_flags.yaml b/ansible_base/feature_flags/definitions/feature_flags.yaml index 0105d217e..e417810fc 100644 --- a/ansible_base/feature_flags/definitions/feature_flags.yaml +++ b/ansible_base/feature_flags/definitions/feature_flags.yaml @@ -3,19 +3,20 @@ visibility: True condition: boolean value: 'False' - support_level: TECHNICAL_PREVIEW - description: TBD - support_url: https://docs.redhat.com/en/documentation/red_hat_ansible_automation_platform/2.5/ + support_level: TECHNOLOGY_PREVIEW + description: "Indirect Node Counting parses the event stream of all jobs to identify resources and stores these in the platform database. Example: Job automates VMware, the parser will report back the VMs, Hypervisors that were automated. This feature helps customers and partners report on the automations they are doing beyond an API endpoint." + support_url: https://access.redhat.com/articles/7109910 labels: - controller - name: FEATURE_EDA_ANALYTICS_ENABLED ui_name: Event-Driven Ansible Analytics - visibility: True + visibility: False condition: boolean value: 'False' - support_level: TECHNICAL_PREVIEW - description: Submit Event-Driven Ansible usage analytics to console.redhat.com + support_level: TECHNOLOGY_PREVIEW + description: Submit Event-Driven Ansible usage analytics to console.redhat.com. support_url: https://access.redhat.com/solutions/7112810 + toggle_type: install-time labels: - eda - name: FEATURE_GATEWAY_IPV6_USAGE_ENABLED @@ -23,7 +24,7 @@ visibility: False condition: boolean value: 'False' - support_level: TECHNICAL_PREVIEW + support_level: TECHNOLOGY_PREVIEW description: The feature flag represents enabling IPv6 only traffic to be allowed through the gateway component and does not include all components of the platform. support_url: https://access.redhat.com/articles/7116569 labels: @@ -40,13 +41,14 @@ labels: - gateway - name: FEATURE_DISPATCHERD_ENABLED - ui_name: AAP Dispatcherd - visibility: True + ui_name: AAP Dispatcherd background tasking system + visibility: False condition: boolean value: 'False' - support_level: DEVELOPER_PREVIEW + support_level: TECHNOLOGY_PREVIEW description: A service to run python tasks in subprocesses, designed specifically to work well with pg_notify, but intended to be extensible to other message delivery means. support_url: '' + toggle_type: install-time labels: - eda - controller diff --git a/ansible_base/feature_flags/definitions/schema.json b/ansible_base/feature_flags/definitions/schema.json index cad80d78d..f8d4794da 100644 --- a/ansible_base/feature_flags/definitions/schema.json +++ b/ansible_base/feature_flags/definitions/schema.json @@ -34,7 +34,7 @@ "description": "The level of support provided for this feature.", "type": "string", "enum": [ - "TECHNICAL_PREVIEW", + "TECHNOLOGY_PREVIEW", "DEVELOPER_PREVIEW" ] }, diff --git a/ansible_base/feature_flags/migrations/0001_initial.py b/ansible_base/feature_flags/migrations/0001_initial.py index ec90f8a92..1b0abd10b 100644 --- a/ansible_base/feature_flags/migrations/0001_initial.py +++ b/ansible_base/feature_flags/migrations/0001_initial.py @@ -1,4 +1,5 @@ -# Generated by Django 4.2.21 on 2025-06-17 20:08 +# Generated by Django 4.2.21 on 2025-06-24 13:34 +# FileHash: 8207dc6b9a446b7d4222d21287e695990b80846c779184388dbd63d32771b400 import ansible_base.feature_flags.models.aap_flag from django.conf import settings @@ -26,7 +27,7 @@ class Migration(migrations.Migration): ('condition', models.CharField(default='boolean', help_text='Used to specify a condition, which if met, will enable the feature flag.', max_length=64)), ('value', models.CharField(default='False', help_text='The value used to evaluate the conditional specified.', max_length=127)), ('required', models.BooleanField(default=False, help_text="If multiple conditions are required to be met to enable a feature flag, 'required' can be used to specify the necessary conditionals.")), - ('support_level', models.CharField(choices=[('DEVELOPER_PREVIEW', 'Developer Preview'), ('TECHNICAL_PREVIEW', 'Technical Preview')], help_text='The support criteria for the feature flag. Must be one of (DEVELOPER_PREVIEW or TECHNICAL_PREVIEW).', max_length=25)), + ('support_level', models.CharField(choices=[('DEVELOPER_PREVIEW', 'Developer Preview'), ('TECHNOLOGY_PREVIEW', 'Technology Preview')], help_text='The support criteria for the feature flag. Must be one of (DEVELOPER_PREVIEW or TECHNOLOGY_PREVIEW).', max_length=25)), ('visibility', models.BooleanField(default=False, help_text='The visibility of the feature flag. If false, flag is hidden.')), ('toggle_type', models.CharField(choices=[('install-time', 'install-time'), ('run-time', 'run-time')], default='run-time', help_text="Details whether a flag is toggle-able at run-time or install-time. (Default: 'run-time').", max_length=20)), ('description', models.CharField(default='', help_text='A detailed description giving an overview of the feature flag.', max_length=500)), diff --git a/ansible_base/feature_flags/migrations/example_migration b/ansible_base/feature_flags/migrations/example_migration new file mode 100644 index 000000000..60b6689d8 --- /dev/null +++ b/ansible_base/feature_flags/migrations/example_migration @@ -0,0 +1,21 @@ +### INSTRUCTIONS ### +# If updating the feature_flags.yaml, create a new migration file by copying this one. +# 1. Name the file XXXX_manual_YYYYMMDD.py. For example 0002_manual_20250808.py +# 1. Uncomment the migration below, by uncommenting everything below the FileHash +# 2. Update the dependency to point to the last dependency +# 3. Set the FileHash +### + +# FileHash: + +# from django.db import migrations + + +# class Migration(migrations.Migration): + +# dependencies = [ +# ('dab_feature_flags', '0001_initial'), +# ] + +# operations = [ +# ] diff --git a/ansible_base/feature_flags/models/aap_flag.py b/ansible_base/feature_flags/models/aap_flag.py index 0e2ee6824..697c42a05 100644 --- a/ansible_base/feature_flags/models/aap_flag.py +++ b/ansible_base/feature_flags/models/aap_flag.py @@ -47,8 +47,8 @@ def __str__(self): support_level = models.CharField( max_length=25, null=False, - help_text=_("The support criteria for the feature flag. Must be one of (DEVELOPER_PREVIEW or TECHNICAL_PREVIEW)."), - choices=(('DEVELOPER_PREVIEW', 'Developer Preview'), ('TECHNICAL_PREVIEW', 'Technical Preview')), + help_text=_("The support criteria for the feature flag. Must be one of (DEVELOPER_PREVIEW or TECHNOLOGY_PREVIEW)."), + choices=(('DEVELOPER_PREVIEW', 'Developer Preview'), ('TECHNOLOGY_PREVIEW', 'Technology Preview')), blank=False, ) visibility = models.BooleanField( diff --git a/ansible_base/feature_flags/utils.py b/ansible_base/feature_flags/utils.py index 0719a543a..e61a4ec15 100644 --- a/ansible_base/feature_flags/utils.py +++ b/ansible_base/feature_flags/utils.py @@ -28,7 +28,7 @@ def create_initial_data(**kwargs): # NOSONAR """ Loads in platform feature flags when the server starts """ - delete_feature_flags() + purge_feature_flags() load_feature_flags() @@ -76,9 +76,9 @@ def load_feature_flags(): logger.error(error_msg) -def delete_feature_flags(): +def purge_feature_flags(): """ - If a feature flag has been removed from the platform flags list, delete it from the database. + If a feature flag has been removed from the platform flags list, purge it from the database. """ from ansible_base.resource_registry.signals.handlers import no_reverse_sync diff --git a/ansible_base/feature_flags/views.py b/ansible_base/feature_flags/views.py index 15b474d1b..4077228b8 100644 --- a/ansible_base/feature_flags/views.py +++ b/ansible_base/feature_flags/views.py @@ -14,7 +14,9 @@ class FeatureFlagsStatesView(AnsibleBaseDjangoAppApiView, ModelViewSet): """ - A view class for displaying feature flags states + A view class for displaying feature flags states. + To add/update/remove a feature flag, see the instructions in + `docs/apps/feature_flags.md` """ queryset = AAPFlag.objects.order_by('id') diff --git a/ansible_base/resource_registry/shared_types.py b/ansible_base/resource_registry/shared_types.py index 793f3628d..69500178f 100644 --- a/ansible_base/resource_registry/shared_types.py +++ b/ansible_base/resource_registry/shared_types.py @@ -78,8 +78,6 @@ class TeamType(SharedResourceTypeSerializer): class FeatureFlagType(SharedResourceTypeSerializer): - """Serialize list of feature flags""" - RESOURCE_TYPE = "aapflag" UNIQUE_FIELDS = ( "name", diff --git a/docs/apps/feature_flags.md b/docs/apps/feature_flags.md index 6d05f8f2d..98d4edd76 100644 --- a/docs/apps/feature_flags.md +++ b/docs/apps/feature_flags.md @@ -18,7 +18,7 @@ INSTALLED_APPS = [ ## Detail By adding the `ansible_base.feature_flags` app to your application, all Ansible Automation Platform feature flags will be loaded and available in your component. -To receive flag state updates, ensure the following definition is available in your components `RESOURCE_LIST` - +To receive flag state updates, ensure the following definition is available in your components `RESOURCE_LIST` - ```python from ansible_base.feature_flags.models import AAPFlag @@ -48,9 +48,9 @@ urlpatterns = [ ] ``` -## Adding Feature Flags +## Adding/updating/removing feature flags -To add a feature flag to the platform, specify it in the following [file](../../ansible_base/feature_flags/definitions/feature_flags.yaml) +To add/update/remove a feature flag to the platform, ensure its configuration is specified correctly it in the following [file](../../ansible_base/feature_flags/definitions/feature_flags.yaml) An example flag could resemble - @@ -73,3 +73,11 @@ Validate this file against the json schema by running `check-jsonschema` - pip install check-jsonschema check-jsonschema --schemafile ansible_base/feature_flags/definitions/schema.json ansible_base/feature_flags/definitions/feature_flags.yaml ``` + +After adding/updating/removing a feature flag, make a manual migration. This can be done by - + +1. Copying this [example-migration](../../ansible_base/feature_flags/migrations/example_migration). +2. Name the file XXXX_manual_YYYYMMDD.py. For example 0002_manual_20250808.py +3. Uncomment the migration, by uncommenting everything below the FileHash +4. Update the dependency in the migration to point to the previous migration +5. Set the **FileHash** in the migration file diff --git a/test_app/tests/feature_flags/migrations/__init__.py b/test_app/tests/feature_flags/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test_app/tests/feature_flags/migrations/test_migrations.py b/test_app/tests/feature_flags/migrations/test_migrations.py new file mode 100644 index 000000000..b961edaff --- /dev/null +++ b/test_app/tests/feature_flags/migrations/test_migrations.py @@ -0,0 +1,82 @@ +import hashlib +import os + +from django.conf import settings +from django.test import TestCase + + +class FileHashTest(TestCase): + FILE_TO_CHECK_PATH = os.path.join(settings.BASE_DIR, 'ansible_base', 'feature_flags', 'definitions', 'feature_flags.yaml') + HASH_ALGORITHM = 'sha256' + HASH_COMMENT_PREFIX = '# FileHash:' + + def _get_last_migration_file(self): + """ + Finds the path to the last migration file in the specified Django app. + """ + migrations_dir = os.path.join(settings.BASE_DIR, 'ansible_base', 'feature_flags', 'migrations') + if not os.path.isdir(migrations_dir): + raise FileNotFoundError(f"Migrations directory not found for app: {self.APP_NAME}") + + migration_files = sorted([ + f for f in os.listdir(migrations_dir) + if f.endswith('.py') and f != '__init__.py' + ]) + + if not migration_files: + raise FileNotFoundError(f"No migration files found in {migrations_dir}") + + return os.path.join(migrations_dir, migration_files[-1]) + + def _extract_hash_from_migration(self, migration_file_path): + """ + Extracts the expected hash from a comment in the migration file. + Assumes the format: '# FileHash: ' + """ + with open(migration_file_path, 'r') as f: + for line in f: + if line.strip().startswith(self.HASH_COMMENT_PREFIX): + return line.strip().replace(self.HASH_COMMENT_PREFIX, '').strip() + return None + + def _calculate_file_hash(self, file_path): + """ + Calculates the hash of the given file. + """ + hash_func = getattr(hashlib, self.HASH_ALGORITHM, None) + if not hash_func: + raise ValueError(f"Unsupported hash algorithm: {self.HASH_ALGORITHM}") + + hasher = hash_func() + with open(file_path, 'rb') as f: + for chunk in iter(lambda: f.read(4096), b""): + hasher.update(chunk) + return hasher.hexdigest() + + def test_file_hash_matches_migration_comment(self): + """ + Checks if the hash of a specified file matches the hash commented + in the last migration file. + """ + # 1. Get the last migration file + try: + last_migration_file = self._get_last_migration_file() + except FileNotFoundError as e: + self.fail(f"Could not find last migration file: {e}") + + # 2. Extract the expected hash from the migration file + expected_hash = self._extract_hash_from_migration(last_migration_file) + self.assertIsNotNone(expected_hash, + f"No hash comment '{self.HASH_COMMENT_PREFIX}' found in {last_migration_file}") + self.assertTrue(expected_hash, "Extracted hash is empty.") + + # 3. Calculate the hash of the target file + self.assertTrue(os.path.exists(self.FILE_TO_CHECK_PATH), + f"File to check does not exist: {self.FILE_TO_CHECK_PATH}") + actual_hash = self._calculate_file_hash(self.FILE_TO_CHECK_PATH) + + # 4. Compare the hashes + self.assertEqual(expected_hash, actual_hash, + f"Hash mismatch for '{os.path.basename(self.FILE_TO_CHECK_PATH)}'. " + f"Expected: {expected_hash}, Got: {actual_hash} " + f"If the feature_flags.yaml file changed, generate a new no-op migration file, and correctly set the FileHash.") diff --git a/test_app/tests/feature_flags/test_utils.py b/test_app/tests/feature_flags/test_utils.py index 4af721d43..fb2e70294 100644 --- a/test_app/tests/feature_flags/test_utils.py +++ b/test_app/tests/feature_flags/test_utils.py @@ -224,7 +224,7 @@ def test_load_feature_flags_creates_new_flag_with_default_value_if_not_in_settin mock_constructed_flag = MockAAPFlagInstance() mock_aap_flag_model_cls.side_effect = [mock_constructed_flag] - mock_aap_flag_model_cls.objects.all.return_value = [] # For delete_feature_flags + mock_aap_flag_model_cls.objects.all.return_value = [] # For purge_feature_flags create_initial_data() @@ -349,7 +349,7 @@ def test_load_feature_flags_logs_other_validation_errors( mock_created_instance.save.assert_not_called() @pytest.mark.django_db - def test_delete_feature_flags_removes_obsolete_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_feature_flags_list): + def test_purge_feature_flags_removes_obsolete_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_feature_flags_list): from ansible_base.feature_flags.utils import create_initial_data obsolete_flag_in_db = MockAAPFlagInstance(name='OBSOLETE_FLAG', condition='obs_cond') @@ -367,7 +367,7 @@ def test_delete_feature_flags_removes_obsolete_flag(self, mock_apps_get_model, m mock_logger.info.assert_any_call(f"Deleting feature flag: {obsolete_flag_in_db.name} as it is no longer available as a platform flag") @pytest.mark.django_db - def test_delete_feature_flags_keeps_current_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_feature_flags_list): + def test_purge_feature_flags_keeps_current_flag(self, mock_apps_get_model, mock_aap_flag_model_cls, mock_logger, mock_feature_flags_list): from ansible_base.feature_flags.utils import create_initial_data current_flag_def = {'name': 'CURRENT_FLAG', 'condition': 'curr_cond', 'ui_name': 'Current'} @@ -396,7 +396,7 @@ def test_create_initial_data_call_order(self, mocker): from ansible_base.feature_flags.utils import create_initial_data # Mock the inner functions directly to check call order - mock_delete = mocker.patch(f"{MODULE_PATH}.delete_feature_flags") + mock_delete = mocker.patch(f"{MODULE_PATH}.purge_feature_flags") mock_load = mocker.patch(f"{MODULE_PATH}.load_feature_flags") manager = MagicMock()