fix: improve cert cleanup for certs that are not in iam

broker/duplicate_certs.py

@@ -5,6 +5,7 @@
 from broker.aws import alb, iam_govcloud
 from broker.extensions import config, db
 from broker.models import ALBServiceInstance, DedicatedALBServiceInstance, Certificate
+from broker.tasks.iam import _delete_server_certificate
 
 logger = logging.getLogger(__name__)
 
@@ -94,16 +95,6 @@ def delete_duplicate_cert_db_record(duplicate_cert):
     db.session.delete(certificate)
 
 
-def delete_iam_server_certificate(certificate_name):
-    try:
-        iam_govcloud.delete_server_certificate(ServerCertificateName=certificate_name)
-        logger.info(f"Deleted IAM certificate {certificate_name}")
-    except iam_govcloud.exceptions.NoSuchEntityException as e:
-        logger.info(
-            f"IAM certificate {certificate_name} does not exist: {e}, continuing"
-        )
-
-
 def get_listener_cert_arns(listener_arn, alb=alb):
     response = alb.describe_listener_certificates(
         ListenerArn=listener_arn,
@@ -147,7 +138,7 @@ def delete_cert_record_and_resource(
             )
 
         if certificate.iam_server_certificate_name:
-            delete_iam_server_certificate(certificate.iam_server_certificate_name)
+            _delete_server_certificate(iam_govcloud, certificate)
 
         # only commit deletion if previous operations were successful
         db.session.commit()

tests/integration/alb/test_alb_remove_duplicate_certs.py

@@ -16,7 +16,6 @@
     get_matching_alb_listener_arns_for_cert_arns,
     delete_duplicate_cert_db_record,
     delete_cert_record_and_resource,
-    delete_iam_server_certificate,
     remove_certificate_from_listener_and_verify_removal,
 )
 from broker.models import Certificate
@@ -169,27 +168,6 @@ def test_delete_duplicate_cert_record_commit(no_context_app, no_context_clean_db
         assert len(Certificate.query.all()) == 0
 
 
-def test_delete_iam_server_certificate_success(iam_govcloud):
-    iam_govcloud.expects_delete_server_certificate("name1")
-
-    delete_iam_server_certificate("name1")
-
-
-def test_delete_iam_server_certificate_no_certificate(iam_govcloud):
-    iam_govcloud.expects_delete_server_certificate_returning_no_such_entity("name1")
-
-    delete_iam_server_certificate("name1")
-
-
-def test_delete_iam_server_certificate_unexpected_error(iam_govcloud):
-    with pytest.raises(ClientError):
-        iam_govcloud.expects_delete_server_certificate_returning_unexpected_error(
-            "name1"
-        )
-
-        delete_iam_server_certificate("name1")
-
-
 def test_delete_cert_record_and_resource_handle_exception(
     no_context_clean_db, no_context_app
 ):
@@ -238,6 +216,7 @@ def test_delete_cert_record_and_resource_success(
 
         alb.expect_remove_certificate_from_listener(service_instance.id, "arn1")
         alb.expect_get_certificates_for_listener(service_instance.id)
+        iam_govcloud.expect_get_server_certificate("name1")
         iam_govcloud.expects_delete_server_certificate("name1")
         delete_cert_record_and_resource(certificate, "1234")
 
@@ -279,7 +258,7 @@ def test_delete_cert_record_and_resource_no_certificate(
 
         alb.expect_remove_certificate_from_listener(service_instance.id, "arn1")
         alb.expect_get_certificates_for_listener(service_instance.id)
-        iam_govcloud.expects_delete_server_certificate_returning_no_such_entity("name1")
+        iam_govcloud.expects_get_server_certificate_returning_no_such_entity("name1")
         delete_cert_record_and_resource(certificate, "1234")
 
         assert len(Certificate.query.all()) == 0

tests/integration/dedicated_alb/test_dedicated_alb_remove_duplicate_certs.py

@@ -16,7 +16,6 @@
     get_matching_alb_listener_arns_for_cert_arns,
     delete_duplicate_cert_db_record,
     delete_cert_record_and_resource,
-    delete_iam_server_certificate,
     remove_certificate_from_listener_and_verify_removal,
 )
 from broker.models import Certificate
@@ -169,27 +168,6 @@ def test_delete_duplicate_cert_record_commit(no_context_app, no_context_clean_db
         assert len(Certificate.query.all()) == 0
 
 
-def test_delete_iam_server_certificate_success(iam_govcloud):
-    iam_govcloud.expects_delete_server_certificate("name1")
-
-    delete_iam_server_certificate("name1")
-
-
-def test_delete_iam_server_certificate_no_certificate(iam_govcloud):
-    iam_govcloud.expects_delete_server_certificate_returning_no_such_entity("name1")
-
-    delete_iam_server_certificate("name1")
-
-
-def test_delete_iam_server_certificate_unexpected_error(iam_govcloud):
-    with pytest.raises(ClientError):
-        iam_govcloud.expects_delete_server_certificate_returning_unexpected_error(
-            "name1"
-        )
-
-        delete_iam_server_certificate("name1")
-
-
 def test_delete_cert_record_and_resource_handle_exception(
     no_context_clean_db, no_context_app
 ):
@@ -238,6 +216,7 @@ def test_delete_cert_record_and_resource_success(
 
         alb.expect_remove_certificate_from_listener(service_instance.id, "arn1")
         alb.expect_get_certificates_for_listener(service_instance.id)
+        iam_govcloud.expect_get_server_certificate("name1")
         iam_govcloud.expects_delete_server_certificate("name1")
         delete_cert_record_and_resource(certificate, "1234")
 
@@ -279,7 +258,7 @@ def test_delete_cert_record_and_resource_no_certificate(
 
         alb.expect_remove_certificate_from_listener(service_instance.id, "arn1")
         alb.expect_get_certificates_for_listener(service_instance.id)
-        iam_govcloud.expects_delete_server_certificate_returning_no_such_entity("name1")
+        iam_govcloud.expects_get_server_certificate_returning_no_such_entity("name1")
         delete_cert_record_and_resource(certificate, "1234")
 
         assert len(Certificate.query.all()) == 0