From 94b4dd013867710b799da1bbed565b64688c7cdc Mon Sep 17 00:00:00 2001 From: "kai [they]" Date: Wed, 27 Nov 2024 15:04:02 -0800 Subject: [PATCH] Removes feature flags (#790) ## Ticket Relates to https://github.com/navapbc/template-infra/issues/781 Closes https://github.com/navapbc/template-infra/issues/791 ## Changes - removes evidently from the infrastructure layer - removes evidently from the application layer ## Testing https://github.com/navapbc/platform-test/pull/142 --- app/Dockerfile | 8 ++-- app/app.py | 9 ---- app/feature_flags.py | 20 --------- infra/app/app-config/feature_flags.tf | 3 -- infra/app/app-config/outputs.tf | 4 -- infra/app/service/main.tf | 12 +---- infra/modules/feature-flags/access_policy.tf | 21 --------- infra/modules/feature-flags/logs.tf | 21 --------- infra/modules/feature-flags/main.tf | 46 -------------------- infra/modules/feature-flags/outputs.tf | 9 ---- infra/modules/feature-flags/variables.tf | 9 ---- infra/project-config/aws_services.tf | 1 - template-only-test/template_infra_test.go | 6 --- 13 files changed, 5 insertions(+), 164 deletions(-) delete mode 100644 app/feature_flags.py delete mode 100644 infra/app/app-config/feature_flags.tf delete mode 100644 infra/modules/feature-flags/access_policy.tf delete mode 100644 infra/modules/feature-flags/logs.tf delete mode 100644 infra/modules/feature-flags/main.tf delete mode 100644 infra/modules/feature-flags/outputs.tf delete mode 100644 infra/modules/feature-flags/variables.tf diff --git a/app/Dockerfile b/app/Dockerfile index 52e4ce60..d11579bf 100644 --- a/app/Dockerfile +++ b/app/Dockerfile @@ -1,13 +1,11 @@ -# Pin to Alpine 3.19 since aws-cli was removed in Alpine 3.20 -# see https://github.com/alpinelinux/docker-alpine/issues/396 -# https://wiki.alpinelinux.org/wiki/Release_Notes_for_Alpine_3.20.0#aws-cli -FROM python:3-alpine3.19 as release +FROM python:3-alpine as release RUN adduser --system --disabled-password --no-create-home app WORKDIR /app -RUN apk --no-cache add \ +RUN apk update && \ + apk --no-cache add \ aws-cli~=2 \ postgresql14-client~=14 diff --git a/app/app.py b/app/app.py index d206d700..683386b4 100644 --- a/app/app.py +++ b/app/app.py @@ -7,7 +7,6 @@ import storage from db import get_db_connection -from feature_flags import is_feature_enabled logging.basicConfig() logger = logging.getLogger() @@ -50,14 +49,6 @@ def migrations(): return f"Last migration on {last_migration_date}" -@app.route("/feature-flags") -def feature_flags(): - foo_status = "enabled" if is_feature_enabled("foo") else "disabled" - bar_status = "enabled" if is_feature_enabled("bar") else "disabled" - - return f"

Feature foo is {foo_status}

Feature bar is {bar_status}

" - - @app.route("/document-upload") def document_upload(): path = f"uploads/{datetime.now().date()}/${{filename}}" diff --git a/app/feature_flags.py b/app/feature_flags.py deleted file mode 100644 index 51d1888b..00000000 --- a/app/feature_flags.py +++ /dev/null @@ -1,20 +0,0 @@ -import logging -import os - -import boto3 - -logger = logging.getLogger() - - -def is_feature_enabled(feature_name: str) -> bool: - feature_flags_project_name = os.environ.get("FEATURE_FLAGS_PROJECT") - - logger.info("Getting Evidently client") - client = boto3.client("evidently") - - response = client.evaluate_feature( - entityId="anonymous", - feature=feature_name, - project=feature_flags_project_name, - ) - return response["value"]["boolValue"] diff --git a/infra/app/app-config/feature_flags.tf b/infra/app/app-config/feature_flags.tf deleted file mode 100644 index 6224b253..00000000 --- a/infra/app/app-config/feature_flags.tf +++ /dev/null @@ -1,3 +0,0 @@ -locals { - feature_flags = ["foo", "bar"] -} diff --git a/infra/app/app-config/outputs.tf b/infra/app/app-config/outputs.tf index 16a35792..86407d31 100644 --- a/infra/app/app-config/outputs.tf +++ b/infra/app/app-config/outputs.tf @@ -14,10 +14,6 @@ output "environments" { value = local.environments } -output "feature_flags" { - value = local.feature_flags -} - output "has_database" { value = local.has_database } diff --git a/infra/app/service/main.tf b/infra/app/service/main.tf index aa0b5d0c..50f9e137 100644 --- a/infra/app/service/main.tf +++ b/infra/app/service/main.tf @@ -179,8 +179,7 @@ module "service" { extra_environment_variables = merge( { - FEATURE_FLAGS_PROJECT = module.feature_flags.evidently_project_name - BUCKET_NAME = local.storage_config.bucket_name + BUCKET_NAME = local.storage_config.bucket_name }, local.identity_provider_environment_variables, local.service_config.extra_environment_variables @@ -199,8 +198,7 @@ module "service" { extra_policies = merge( { - feature_flags_access = module.feature_flags.access_policy_arn, - storage_access = module.storage.access_policy_arn + storage_access = module.storage.access_policy_arn }, module.app_config.enable_identity_provider ? { identity_provider_access = module.identity_provider_client[0].access_policy_arn, @@ -221,12 +219,6 @@ module "monitoring" { incident_management_service_integration_url = module.app_config.has_incident_management_service && !local.is_temporary ? data.aws_ssm_parameter.incident_management_service_integration_url[0].value : null } -module "feature_flags" { - source = "../../modules/feature-flags" - service_name = local.service_config.service_name - feature_flags = module.app_config.feature_flags -} - module "storage" { source = "../../modules/storage" name = local.storage_config.bucket_name diff --git a/infra/modules/feature-flags/access_policy.tf b/infra/modules/feature-flags/access_policy.tf deleted file mode 100644 index 67b6c7da..00000000 --- a/infra/modules/feature-flags/access_policy.tf +++ /dev/null @@ -1,21 +0,0 @@ -resource "aws_iam_policy" "access_policy" { - name = "${local.evidently_project_name}-access" - path = "/" - description = "Allows calls to EvaluateFeature on the Evidently project ${local.evidently_project_name}" - - policy = data.aws_iam_policy_document.access_policy.json -} - -#tfsec:ignore:aws-iam-no-policy-wildcards -data "aws_iam_policy_document" "access_policy" { - statement { - sid = "AllowEvaluateFeature" - effect = "Allow" - actions = [ - "evidently:EvaluateFeature", - ] - resources = [ - "${aws_evidently_project.feature_flags.arn}/feature/*", - ] - } -} diff --git a/infra/modules/feature-flags/logs.tf b/infra/modules/feature-flags/logs.tf deleted file mode 100644 index 8d7dc7ea..00000000 --- a/infra/modules/feature-flags/logs.tf +++ /dev/null @@ -1,21 +0,0 @@ -data "aws_caller_identity" "current" {} -data "aws_region" "current" {} - -resource "aws_cloudwatch_log_group" "logs" { - # Prefix log group name with /aws/vendedlogs/ to handle situations where the resource policy - # that AWS automatically creates to allow Evidently to send logs to CloudWatch exceeds the - # 5120 character limit. - # see https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/AWS-logs-and-resource-policy.html#AWS-vended-logs-permissions - # see https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_iam-quotas.html#reference_iam-quotas-entity-length - # - # Note that manually creating resource policies is also not ideal, as there is a quote of - # up to 10 CloudWatch Logs resource policies per Region per account, which can't be changed. - # see https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/cloudwatch_limits_cwl.html - name = "/aws/vendedlogs/feature-flags/${local.evidently_project_name}" - - # checkov:skip=CKV_AWS_158:Feature flag evaluation logs are not sensitive - - # Conservatively retain logs for 5 years. - # Looser requirements may allow shorter retention periods - retention_in_days = 1827 -} diff --git a/infra/modules/feature-flags/main.tf b/infra/modules/feature-flags/main.tf deleted file mode 100644 index d450c34f..00000000 --- a/infra/modules/feature-flags/main.tf +++ /dev/null @@ -1,46 +0,0 @@ -locals { - evidently_project_name = "${var.service_name}-flags" -} - -resource "aws_evidently_project" "feature_flags" { - name = local.evidently_project_name - description = "Feature flags for ${var.service_name}" - data_delivery { - cloudwatch_logs { - log_group = aws_cloudwatch_log_group.logs.name - } - } -} - -resource "aws_evidently_feature" "feature_flag" { - for_each = var.feature_flags - - name = each.key - project = aws_evidently_project.feature_flags.name - description = "Enables the ${each.key} feature" - variations { - name = "FeatureOff" - value { - bool_value = false - } - } - variations { - name = "FeatureOn" - value { - bool_value = true - } - } - - # default_variation specifies the variation to use as the default variation. - # Ignore this in terraform to allow business users to enable a feature for all users. - # - # entity_overrides specifies users that should always be served a specific variation of a feature. - # Ignore this in terraform to allow business users and developers to select feature variations - # for testing or pilot purposes. - lifecycle { - ignore_changes = [ - default_variation, - entity_overrides, - ] - } -} diff --git a/infra/modules/feature-flags/outputs.tf b/infra/modules/feature-flags/outputs.tf deleted file mode 100644 index 5269dd6c..00000000 --- a/infra/modules/feature-flags/outputs.tf +++ /dev/null @@ -1,9 +0,0 @@ -output "access_policy_arn" { - description = "Policy that allows access to query feature flag values" - value = aws_iam_policy.access_policy.arn -} - -output "evidently_project_name" { - description = "Name of AWS Evidently feature flags project" - value = local.evidently_project_name -} diff --git a/infra/modules/feature-flags/variables.tf b/infra/modules/feature-flags/variables.tf deleted file mode 100644 index 0c395a1f..00000000 --- a/infra/modules/feature-flags/variables.tf +++ /dev/null @@ -1,9 +0,0 @@ -variable "feature_flags" { - type = set(string) - description = "A set of feature flag names" -} - -variable "service_name" { - type = string - description = "The name of the service that the feature flagging system will be associated with" -} diff --git a/infra/project-config/aws_services.tf b/infra/project-config/aws_services.tf index 3350babf..a3c7cdeb 100644 --- a/infra/project-config/aws_services.tf +++ b/infra/project-config/aws_services.tf @@ -14,7 +14,6 @@ locals { "elasticbeanstalk", "elasticloadbalancing", "events", - "evidently", "iam", "kms", "lambda", diff --git a/template-only-test/template_infra_test.go b/template-only-test/template_infra_test.go index 33049c11..ebacf63c 100644 --- a/template-only-test/template_infra_test.go +++ b/template-only-test/template_infra_test.go @@ -193,12 +193,6 @@ func ValidateDevEnvironment(t *testing.T) { return responseStatus == 200 }) - // Hit feature flags endpoint to make sure Evidently integration is working - featureFlagsEndpoint := fmt.Sprintf("%s/feature-flags", serviceEndpoint) - http_helper.HttpGetWithRetryWithCustomValidation(t, featureFlagsEndpoint, nil, 10, 3*time.Second, func(responseStatus int, responseBody string) bool { - return responseStatus == 200 - }) - fmt.Println("::endgroup::") }