feat: remove unused SSH keys (#652)

## Description

Whenever an executor is created it needs an SSH key so the agent is able to contact it. In case the executor is killed this SSH key is not removed. Over time they accumulate and reach the AWS limit. From this point no further SSH keys can be created and thus the module is no longer able to create executors. You won't be able to run any pipeline.

Some users have created their own housekeeping of SSH keys. But this should be done within the module as the keys are created here.

Challenge: Make sure that keys are deleted only which belong to our module. Unfortunately the GitLab Runner does not tag the keys. We use the following logic to determine the keys to delete:

- they are unused
- their name starts with `runner`
- they have the `var.environment`/`var.overrides['name_docker_machine']` somewhere in the name

Solves part of #623 
Closes #592

Author: kayman-mk

.gitignore

@@ -31,4 +31,7 @@ builds/
 
 # exceptions for semantic release
 !.release/package*
-!.release/*.lock
+!.release/*.lock
+
+# Python
+venv/

locals.tf

@@ -25,6 +25,7 @@ locals {
   name_runner_agent_instance = var.overrides["name_runner_agent_instance"] == "" ? local.tags["Name"] : var.overrides["name_runner_agent_instance"]
   name_sg                    = var.overrides["name_sg"] == "" ? local.tags["Name"] : var.overrides["name_sg"]
   name_iam_objects           = lookup(var.overrides, "name_iam_objects", "") == "" ? local.tags["Name"] : var.overrides["name_iam_objects"]
+
   runners_additional_volumes = <<-EOT
   %{~if var.runners_add_dind_volumes~},"/certs/client", "/builds", "/var/run/docker.sock:/var/run/docker.sock"%{endif~}%{~for volume in var.runners_additional_volumes~},"${volume}"%{endfor~}
   EOT

main.tf

@@ -545,6 +545,7 @@ module "terminate_agent_hook" {
   asg_name                             = aws_autoscaling_group.gitlab_runner_instance.name
   cloudwatch_logging_retention_in_days = var.cloudwatch_logging_retention_in_days
   name_iam_objects                     = local.name_iam_objects
+  name_docker_machine_runners          = local.runner_tags_merged["Name"]
   role_permissions_boundary            = var.permissions_boundary == "" ? null : "arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:policy/${var.permissions_boundary}"
   kms_key_id                           = local.kms_key
   arn_format                           = var.arn_format

modules/terminate-agent-hook/iam.tf

@@ -40,7 +40,7 @@ data "aws_iam_policy_document" "lambda" {
       "ec2:DescribeInstances",
       "ec2:DescribeTags",
       "ec2:DescribeRegions",
-      "ec2:DescribeInstanceStatus"
+      "ec2:DescribeInstanceStatus",
     ]
     resources = ["*"]
     effect    = "Allow"
@@ -71,6 +71,7 @@ data "aws_iam_policy_document" "lambda" {
     ]
     resources = [var.asg_arn]
   }
+
   statement {
     sid = "GitLabRunnerLifecycleTerminateLogs"
     actions = [
@@ -85,13 +86,40 @@ data "aws_iam_policy_document" "lambda" {
       "${aws_cloudwatch_log_group.lambda.arn}:log-stream:*"
     ]
   }
+
+  statement {
+    sid = "SSHKeyHousekeepingList"
+
+    effect = "Allow"
+    actions = [
+      "ec2:DescribeKeyPairs"
+    ]
+    resources = ["*"]
+  }
+
+  # separate statement due to the condition
+  statement {
+    sid = "SSHKeyHousekeepingDelete"
+
+    effect = "Allow"
+    actions = [
+      "ec2:DeleteKeyPair"
+    ]
+    resources = ["*"]
+    condition {
+      test     = "StringLike"
+      variable = "ec2:KeyPairName"
+      values   = ["runner-*"]
+    }
+  }
 }
 
 resource "aws_iam_policy" "lambda" {
-  name   = "${var.name_iam_objects}-${var.name}"
+  name   = "${var.name_iam_objects}-${var.name}-lambda"
   path   = "/"
   policy = data.aws_iam_policy_document.lambda.json
-  tags   = var.tags
+
+  tags = var.tags
 }
 
 resource "aws_iam_role_policy_attachment" "lambda" {

modules/terminate-agent-hook/lambda/lambda_function.py

@@ -1,34 +1,36 @@
 """
-AWS Lambda function to terminate orphaned GitLab runners.
+AWS Lambda function to terminate orphaned GitLab runners and remove unused resources.
 
-This checks for running GitLab runner instances and terminates them,
-intended to be triggered by an ASG life cycle hook at instance termination.
+- This checks for running GitLab runner instances and terminates them, intended to be triggered by an ASG life cycle hook at
+  instance termination.
+- Removes all unused SSH keys
 
-https://github.com/npalm/terraform-aws-gitlab-runner/issues/317 has some
-discussion about this scenario.
+https://github.com/npalm/terraform-aws-gitlab-runner/issues/317 has some discussion about this scenario.
 
 This is rudimentary and doesn't check if a build runner has a current job.
 """
 import boto3
+import botocore
 import json
+import os
 
-def ec2_list(client, **args):
 
+def ec2_list(client, **args):
     print(json.dumps({
         "Level": "info",
         "Message": f"Searching for children of GitLab runner instance {args['parent']}"
     }))
 
     ec2_instances = client.describe_instances(Filters=[
-            {
-                "Name": "instance-state-name",
-                "Values": ['running', 'pending'],
-            },
-            {
-                "Name": "tag:gitlab-runner-parent-id",
-                "Values": ["*"]
-            }
-        ]).get("Reservations")
+        {
+            "Name": "instance-state-name",
+            "Values": ['running', 'pending'],
+        },
+        {
+            "Name": "tag:gitlab-runner-parent-id",
+            "Values": ["*"]
+        }
+    ]).get("Reservations")
 
     _terminate_list = []
     for _instances in ec2_instances:
@@ -87,34 +89,92 @@ def ec2_list(client, **args):
 
     return _terminate_list
 
+
+def remove_unused_ssh_key_pairs(client, executor_name_part):
+    print(json.dumps({
+        "Level": "info",
+        "Message": f"Removing unused SSH key pairs for agent {executor_name_part}"
+    }))
+
+    # build list of SSH keys to keep
+    paginator = client.get_paginator('describe_instances')
+    reservations = paginator.paginate(Filters=[
+        {
+            "Name": "key-name",
+            "Values": ['runner-*'],
+        },
+        {
+            "Name": "instance-state-name",
+            "Values": ['pending', 'running'],
+        },
+    ]).build_full_result().get("Reservations")
+
+    used_key_pairs = []
+
+    for reservation in reservations:
+        for instance in reservation["Instances"]:
+            used_key_pairs.append(instance['KeyName'])
+
+    all_key_pairs = client.describe_key_pairs(Filters=[
+        {
+            "Name": "key-name",
+            "Values": ['runner-*'],
+        },
+    ])
+
+    for key_pair in all_key_pairs['KeyPairs']:
+        key_name = key_pair['KeyName']
+
+        if key_name not in used_key_pairs:
+            # make sure to delete only those keys which belongs to our module
+            # unfortunately there are no tags set on the keys and GitLab runner is not able to do that
+            if executor_name_part in key_name:
+                try:
+                    client.delete_key_pair(KeyName=key_name)
+
+                    print(json.dumps({
+                        "Level": "info",
+                        "Message": f"Key pair deleted: {key_name}"
+                    }))
+                except botocore.exceptions.ClientError as error:
+                    print(json.dumps({
+                        "Level": "error",
+                        "Message": f"Unable to delete key pair: {key_name}",
+                        "Exception": str(error)
+                    }))
+
+
 def handler(event, context):
     response = []
     event_detail = event['detail']
     client = boto3.client("ec2", region_name=event['region'])
     if event_detail['LifecycleTransition'] != "autoscaling:EC2_INSTANCE_TERMINATING":
         exit()
 
-    _terminate_list = ec2_list(client=client,parent=event_detail['EC2InstanceId'])
+    # find the executors connected to this agent and terminate them as well
+    _terminate_list = ec2_list(client=client, parent=event_detail['EC2InstanceId'])
     if len(_terminate_list) > 0:
         print(json.dumps({
             "Level": "info",
             "Message": f"Terminating instances {', '.join(_terminate_list)}"
         }))
         try:
             client.terminate_instances(InstanceIds=_terminate_list, DryRun=False)
-            return f"Terminated instances {', '.join(_terminate_list)}"
         except Exception as e:
             print(json.dumps({
                 "Level": "exception",
                 "Exception": str(e)
             }))
-            raise Exception(f"Encountered exception when terminating instances: {str(e)}")
     else:
         print(json.dumps({
             "Level": "info",
             "Message": "No instances to terminate."
         }))
-        return "No instances to terminate."
+
+    remove_unused_ssh_key_pairs(client=client, executor_name_part=os.environ['NAME_EXECUTOR_INSTANCE'])
+
+    return f"Housekeeping done"
+
 
 if __name__ == "__main__":
-    handler(None, None)
+    handler(None, None)

modules/terminate-agent-hook/main.tf

@@ -31,6 +31,12 @@ resource "aws_lambda_function" "terminate_runner_instances" {
   timeout          = local.lambda_timeout
   tags             = var.tags
 
+  environment {
+    variables = {
+      NAME_EXECUTOR_INSTANCE = var.name_docker_machine_runners
+    }
+  }
+
   dynamic "tracing_config" {
     for_each = var.enable_xray_tracing ? [1] : []
 

modules/terminate-agent-hook/variables.tf

@@ -45,6 +45,11 @@ variable "name_iam_objects" {
   default     = ""
 }
 
+variable "name_docker_machine_runners" {
+  description = "The `Name` tag of EC2 instances created by the runner agent."
+  type        = string
+}
+
 variable "kms_key_id" {
   description = "KMS key id to encrypted the CloudWatch logs. Ensure CloudWatch has access to the provided KMS key."
   type        = string