feat: remove unused app client since SSH key is used to secure app au…

…thorization (#1223)

README.md

@@ -129,8 +129,7 @@ Go to GitHub and [create a new app](https://docs.github.com/en/developers/apps/c
      - `Self-hosted runners`: Read & write (to register runner)
 8. Save the new app.
 9. On the General page, make a note of the "App ID" and "Client ID" parameters.
-10. Create a new client secret and also write it down.
-11. Generate a new private key and save the `app.private-key.pem` file.
+10. Generate a new private key and save the `app.private-key.pem` file.
 
 ### Setup terraform module
 
@@ -174,8 +173,6 @@ module "github-runner" {
   github_app = {
     key_base64     = "base64string"
     id             = "1"
-    client_id      = "c-123"
-    client_secret  = "client_secret"
     webhook_secret = "webhook_secret"
   }
 
@@ -377,7 +374,7 @@ No requirements.
 | environment | A name that identifies the environment, used as prefix and for tagging. | `string` | n/a | yes |
 | ghes\_ssl\_verify | GitHub Enterprise SSL verification. Set to 'false' when custom certificate (chains) is used for GitHub Enterprise Server (insecure). | `bool` | `true` | no |
 | ghes\_url | GitHub Enterprise Server URL. Example: https://github.internal.co - DO NOT SET IF USING PUBLIC GITHUB | `string` | `null` | no |
-| github\_app | GitHub app parameters, see your github app. Ensure the key is the base64-encoded `.pem` file (the output of `base64 app.private-key.pem`, not the content of `private-key.pem`). | <pre>object({<br>    key_base64     = string<br>    id             = string<br>    client_id      = string<br>    client_secret  = string<br>    webhook_secret = string<br>  })</pre> | n/a | yes |
+| github\_app | GitHub app parameters, see your github app. Ensure the key is the base64-encoded `.pem` file (the output of `base64 app.private-key.pem`, not the content of `private-key.pem`). | <pre>object({<br>    key_base64     = string<br>    id             = string<br>    webhook_secret = string<br>  })</pre> | n/a | yes |
 | idle\_config | List of time period that can be defined as cron expression to keep a minimum amount of runners active instead of scaling down to 0. By defining this list you can ensure that in time periods that match the cron expression within 5 seconds a runner is kept idle. | <pre>list(object({<br>    cron      = string<br>    timeZone  = string<br>    idleCount = number<br>  }))</pre> | `[]` | no |
 | instance\_profile\_path | The path that will be added to the instance\_profile, if not set the environment name will be used. | `string` | `null` | no |
 | instance\_type | [DEPRECATED] See instance\_types. | `string` | `"m5.large"` | no |

examples/default/main.tf

@@ -27,8 +27,6 @@ module "runners" {
   github_app = {
     key_base64     = var.github_app_key_base64
     id             = var.github_app_id
-    client_id      = var.github_app_client_id
-    client_secret  = var.github_app_client_secret
     webhook_secret = random_password.random.result
   }
 
@@ -48,9 +46,14 @@ module "runners" {
   #   idleCount = 1
   # }]
 
-  # disable KMS and encryption
-  # encrypt_secrets = false
-
   # Let the module manage the service linked role
   # create_service_linked_role_spot = true
+
+  instance_types = ["m5.large", "c5.large"]
+
+  # override delay of events in seconds
+  delay_webhook_event = 5
+
+  # override scaling down
+  scale_down_schedule_expression = "cron(* * * * ? *)"
 }

examples/ubuntu/main.tf

@@ -22,8 +22,6 @@ module "runners" {
   github_app = {
     key_base64     = var.github_app_key_base64
     id             = var.github_app_id
-    client_id      = var.github_app_client_id
-    client_secret  = var.github_app_client_secret
     webhook_secret = random_password.random.result
   }
 

main.tf

@@ -9,10 +9,8 @@ locals {
   ami_filter = length(var.ami_filter) > 0 ? var.ami_filter : local.runner_architecture == "arm64" ? { name = ["amzn2-ami-hvm-2*-arm64-gp2"] } : { name = ["amzn2-ami-hvm-2.*-x86_64-ebs"] }
 
   github_app_parameters = {
-    client_id     = module.ssm.parameters.github_app_client_id
-    client_secret = module.ssm.parameters.github_app_client_secret
-    id            = module.ssm.parameters.github_app_id
-    key_base64    = module.ssm.parameters.github_app_key_base64
+    id         = module.ssm.parameters.github_app_id
+    key_base64 = module.ssm.parameters.github_app_key_base64
   }
 }
 

modules/runners/README.md

@@ -58,6 +58,31 @@ No requirements.
 |------|---------|
 | aws | n/a |
 
+## Modules
+
+No Modules.
+
+## Resources
+
+| Name |
+|------|
+| [aws_ami](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/ami) |
+| [aws_caller_identity](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) |
+| [aws_cloudwatch_event_rule](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_event_rule) |
+| [aws_cloudwatch_event_target](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_event_target) |
+| [aws_cloudwatch_log_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_log_group) |
+| [aws_iam_instance_profile](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_instance_profile) |
+| [aws_iam_policy_document](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) |
+| [aws_iam_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) |
+| [aws_iam_role_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) |
+| [aws_iam_role_policy_attachment](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) |
+| [aws_lambda_event_source_mapping](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_event_source_mapping) |
+| [aws_lambda_function](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_function) |
+| [aws_lambda_permission](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_permission) |
+| [aws_launch_template](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/launch_template) |
+| [aws_security_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) |
+| [aws_ssm_parameter](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ssm_parameter) |
+
 ## Inputs
 
 | Name | Description | Type | Default | Required |
@@ -73,8 +98,9 @@ No requirements.
 | enable\_organization\_runners | n/a | `bool` | n/a | yes |
 | enable\_ssm\_on\_runners | Enable to allow access to the runner instances for debugging purposes via SSM. Note that this adds additional permissions to the runner instances. | `bool` | n/a | yes |
 | environment | A name that identifies the environment, used as prefix and for tagging. | `string` | n/a | yes |
+| ghes\_ssl\_verify | GitHub Enterprise SSL verification. Set to 'false' when custom certificate (chains) is used for GitHub Enterprise Server (insecure). | `bool` | `true` | no |
 | ghes\_url | GitHub Enterprise Server URL. DO NOT SET IF USING PUBLIC GITHUB | `string` | `null` | no |
-| github\_app\_parameters | Parameter Store for GitHub App Parameters. | <pre>object({<br>    key_base64    = map(string)<br>    id            = map(string)<br>    client_id     = map(string)<br>    client_secret = map(string)<br>  })</pre> | n/a | yes |
+| github\_app\_parameters | Parameter Store for GitHub App Parameters. | <pre>object({<br>    key_base64 = map(string)<br>    id         = map(string)<br>  })</pre> | n/a | yes |
 | idle\_config | List of time period that can be defined as cron expression to keep a minimum amount of runners active instead of scaling down to 0. By defining this list you can ensure that in time periods that match the cron expression within 5 seconds a runner is kept idle. | <pre>list(object({<br>    cron      = string<br>    timeZone  = string<br>    idleCount = number<br>  }))</pre> | `[]` | no |
 | instance\_profile\_path | The path that will be added to the instance\_profile, if not set the environment name will be used. | `string` | `null` | no |
 | instance\_type | [DEPRECATED] See instance\_types. | `string` | `"m5.large"` | no |
@@ -126,7 +152,6 @@ No requirements.
 | role\_runner | n/a |
 | role\_scale\_down | n/a |
 | role\_scale\_up | n/a |
-
 <!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
 
 ## Philips Forest

modules/runners/lambdas/runners/src/scale-runners/gh-auth.test.ts

@@ -16,12 +16,8 @@ jest.mock('@octokit/auth-app');
 const cleanEnv = process.env;
 const ENVIRONMENT = 'dev';
 const GITHUB_APP_ID = '1';
-const GITHUB_APP_CLIENT_ID = '1';
-const GITHUB_APP_CLIENT_SECRET = 'client_secret';
 const PARAMETER_GITHUB_APP_ID_NAME = `/actions-runner/${ENVIRONMENT}/github_app_id`;
 const PARAMETER_GITHUB_APP_KEY_BASE64_NAME = `/actions-runner/${ENVIRONMENT}/github_app_key_base64`;
-const PARAMETER_GITHUB_APP_CLIENT_ID_NAME = `/actions-runner/${ENVIRONMENT}/github_app_client_id`;
-const PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME = `/actions-runner/${ENVIRONMENT}/github_app_client_secret`;
 
 const mockedGet = mocked(getParameterValue);
 
@@ -31,8 +27,6 @@ beforeEach(() => {
   process.env = { ...cleanEnv };
   process.env.PARAMETER_GITHUB_APP_ID_NAME = PARAMETER_GITHUB_APP_ID_NAME;
   process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME = PARAMETER_GITHUB_APP_KEY_BASE64_NAME;
-  process.env.PARAMETER_GITHUB_APP_CLIENT_ID_NAME = PARAMETER_GITHUB_APP_CLIENT_ID_NAME;
-  process.env.PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME = PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME;
   nock.disableNetConnect();
 });
 
@@ -83,14 +77,8 @@ describe('Test createGithubAppAuth', () => {
       appId: parseInt(GITHUB_APP_ID),
       privateKey: decryptedValue,
       installationId,
-      clientId: GITHUB_APP_CLIENT_ID,
-      clientSecret: GITHUB_APP_CLIENT_SECRET,
     };
-    mockedGet
-      .mockResolvedValueOnce(GITHUB_APP_ID)
-      .mockResolvedValueOnce(b64)
-      .mockResolvedValueOnce(GITHUB_APP_CLIENT_ID)
-      .mockResolvedValueOnce(GITHUB_APP_CLIENT_SECRET);
+    mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64);
 
     const mockedAuth = jest.fn();
     mockedAuth.mockResolvedValue({ token });
@@ -104,8 +92,6 @@ describe('Test createGithubAppAuth', () => {
     // Assert
     expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_ID_NAME);
     expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME);
-    expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_CLIENT_ID_NAME);
-    expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME);
 
     expect(mockedCreatAppAuth).toBeCalledTimes(1);
     expect(mockedCreatAppAuth).toBeCalledWith(authOptions);
@@ -126,16 +112,10 @@ describe('Test createGithubAppAuth', () => {
       appId: parseInt(GITHUB_APP_ID),
       privateKey: decryptedValue,
       installationId,
-      clientId: GITHUB_APP_CLIENT_ID,
-      clientSecret: GITHUB_APP_CLIENT_SECRET,
       request: mockedRequestInterface.defaults({ baseUrl: githubServerUrl }),
     };
 
-    mockedGet
-      .mockResolvedValueOnce(GITHUB_APP_ID)
-      .mockResolvedValueOnce(b64)
-      .mockResolvedValueOnce(GITHUB_APP_CLIENT_ID)
-      .mockResolvedValueOnce(GITHUB_APP_CLIENT_SECRET);
+    mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64);
     const mockedAuth = jest.fn();
     mockedAuth.mockResolvedValue({ token });
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -149,8 +129,6 @@ describe('Test createGithubAppAuth', () => {
     // Assert
     expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_ID_NAME);
     expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME);
-    expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_CLIENT_ID_NAME);
-    expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME);
 
     expect(mockedCreatAppAuth).toBeCalledTimes(1);
     expect(mockedCreatAppAuth).toBeCalledWith(authOptions);
@@ -172,16 +150,10 @@ describe('Test createGithubAppAuth', () => {
     const authOptions = {
       appId: parseInt(GITHUB_APP_ID),
       privateKey: decryptedValue,
-      clientId: GITHUB_APP_CLIENT_ID,
-      clientSecret: GITHUB_APP_CLIENT_SECRET,
       request: mockedRequestInterface.defaults({ baseUrl: githubServerUrl }),
     };
 
-    mockedGet
-      .mockResolvedValueOnce(GITHUB_APP_ID)
-      .mockResolvedValueOnce(b64)
-      .mockResolvedValueOnce(GITHUB_APP_CLIENT_ID)
-      .mockResolvedValueOnce(GITHUB_APP_CLIENT_SECRET);
+    mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64);
     const mockedAuth = jest.fn();
     mockedAuth.mockResolvedValue({ token });
     mockedCreatAppAuth.mockImplementation(() => {
@@ -194,8 +166,6 @@ describe('Test createGithubAppAuth', () => {
     // Assert
     expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_ID_NAME);
     expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME);
-    expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_CLIENT_ID_NAME);
-    expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME);
 
     expect(mockedCreatAppAuth).toBeCalledTimes(1);
     expect(mockedCreatAppAuth).toBeCalledWith(authOptions);

modules/runners/lambdas/runners/src/scale-runners/gh-auth.ts

@@ -49,8 +49,6 @@ async function createAuth(installationId: number | undefined, ghesApiUrl: string
       await getParameterValue(process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME),
       'base64',
     ).toString(),
-    clientId: await getParameterValue(process.env.PARAMETER_GITHUB_APP_CLIENT_ID_NAME),
-    clientSecret: await getParameterValue(process.env.PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME),
   };
   if (installationId) authOptions = { ...authOptions, installationId };
 

modules/runners/policies/lambda-scale-down.json

@@ -32,8 +32,6 @@
             ],
             "Resource": [
                 "${github_app_key_base64_arn}",
-                "${github_app_client_secret_arn}",
-                "${github_app_client_id_arn}",
                 "${github_app_id_arn}"
             ]
 %{ if kms_key_arn != "" ~}

modules/runners/policies/lambda-scale-up.json

@@ -45,8 +45,6 @@
             ],
             "Resource": [
                 "${github_app_key_base64_arn}",
-                "${github_app_client_secret_arn}",
-                "${github_app_client_id_arn}",
                 "${github_app_id_arn}"
             ]
         },

modules/runners/scale-down.tf

@@ -13,16 +13,14 @@ resource "aws_lambda_function" "scale_down" {
 
   environment {
     variables = {
-      ENVIRONMENT                             = var.environment
-      MINIMUM_RUNNING_TIME_IN_MINUTES         = var.minimum_running_time_in_minutes
-      RUNNER_BOOT_TIME_IN_MINUTES             = var.runner_boot_time_in_minutes
-      SCALE_DOWN_CONFIG                       = jsonencode(var.idle_config)
-      GHES_URL                                = var.ghes_url
-      NODE_TLS_REJECT_UNAUTHORIZED            = var.ghes_url != null && ! var.ghes_ssl_verify ? 0 : 1
-      PARAMETER_GITHUB_APP_CLIENT_ID_NAME     = var.github_app_parameters.client_id.name
-      PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME = var.github_app_parameters.client_secret.name
-      PARAMETER_GITHUB_APP_ID_NAME            = var.github_app_parameters.id.name
-      PARAMETER_GITHUB_APP_KEY_BASE64_NAME    = var.github_app_parameters.key_base64.name
+      ENVIRONMENT                          = var.environment
+      MINIMUM_RUNNING_TIME_IN_MINUTES      = var.minimum_running_time_in_minutes
+      RUNNER_BOOT_TIME_IN_MINUTES          = var.runner_boot_time_in_minutes
+      SCALE_DOWN_CONFIG                    = jsonencode(var.idle_config)
+      GHES_URL                             = var.ghes_url
+      NODE_TLS_REJECT_UNAUTHORIZED         = var.ghes_url != null && ! var.ghes_ssl_verify ? 0 : 1
+      PARAMETER_GITHUB_APP_ID_NAME         = var.github_app_parameters.id.name
+      PARAMETER_GITHUB_APP_KEY_BASE64_NAME = var.github_app_parameters.key_base64.name
     }
   }
 
@@ -72,11 +70,9 @@ resource "aws_iam_role_policy" "scale_down" {
   name = "${var.environment}-lambda-scale-down-policy"
   role = aws_iam_role.scale_down.name
   policy = templatefile("${path.module}/policies/lambda-scale-down.json", {
-    github_app_client_id_arn     = var.github_app_parameters.client_id.arn
-    github_app_client_secret_arn = var.github_app_parameters.client_secret.arn
-    github_app_id_arn            = var.github_app_parameters.id.arn
-    github_app_key_base64_arn    = var.github_app_parameters.key_base64.arn
-    kms_key_arn                  = local.kms_key_arn
+    github_app_id_arn         = var.github_app_parameters.id.arn
+    github_app_key_base64_arn = var.github_app_parameters.key_base64.arn
+    kms_key_arn               = local.kms_key_arn
   })
 }
 

modules/runners/scale-up.tf

@@ -14,19 +14,17 @@ resource "aws_lambda_function" "scale_up" {
 
   environment {
     variables = {
-      ENABLE_ORGANIZATION_RUNNERS             = var.enable_organization_runners
-      ENVIRONMENT                             = var.environment
-      GHES_URL                                = var.ghes_url
-      NODE_TLS_REJECT_UNAUTHORIZED            = var.ghes_url != null && ! var.ghes_ssl_verify ? 0 : 1
-      RUNNER_EXTRA_LABELS                     = var.runner_extra_labels
-      RUNNER_GROUP_NAME                       = var.runner_group_name
-      RUNNERS_MAXIMUM_COUNT                   = var.runners_maximum_count
-      LAUNCH_TEMPLATE_NAME                    = join(",", [for template in aws_launch_template.runner : template.name])
-      SUBNET_IDS                              = join(",", var.subnet_ids)
-      PARAMETER_GITHUB_APP_CLIENT_ID_NAME     = var.github_app_parameters.client_id.name
-      PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME = var.github_app_parameters.client_secret.name
-      PARAMETER_GITHUB_APP_ID_NAME            = var.github_app_parameters.id.name
-      PARAMETER_GITHUB_APP_KEY_BASE64_NAME    = var.github_app_parameters.key_base64.name
+      ENABLE_ORGANIZATION_RUNNERS          = var.enable_organization_runners
+      ENVIRONMENT                          = var.environment
+      GHES_URL                             = var.ghes_url
+      NODE_TLS_REJECT_UNAUTHORIZED         = var.ghes_url != null && ! var.ghes_ssl_verify ? 0 : 1
+      RUNNER_EXTRA_LABELS                  = var.runner_extra_labels
+      RUNNER_GROUP_NAME                    = var.runner_group_name
+      RUNNERS_MAXIMUM_COUNT                = var.runners_maximum_count
+      LAUNCH_TEMPLATE_NAME                 = join(",", [for template in aws_launch_template.runner : template.name])
+      SUBNET_IDS                           = join(",", var.subnet_ids)
+      PARAMETER_GITHUB_APP_ID_NAME         = var.github_app_parameters.id.name
+      PARAMETER_GITHUB_APP_KEY_BASE64_NAME = var.github_app_parameters.key_base64.name
     }
   }
 
@@ -70,13 +68,11 @@ resource "aws_iam_role_policy" "scale_up" {
   name = "${var.environment}-lambda-scale-up-policy"
   role = aws_iam_role.scale_up.name
   policy = templatefile("${path.module}/policies/lambda-scale-up.json", {
-    arn_runner_instance_role     = aws_iam_role.runner.arn
-    sqs_arn                      = var.sqs_build_queue.arn
-    github_app_client_id_arn     = var.github_app_parameters.client_id.arn
-    github_app_client_secret_arn = var.github_app_parameters.client_secret.arn
-    github_app_id_arn            = var.github_app_parameters.id.arn
-    github_app_key_base64_arn    = var.github_app_parameters.key_base64.arn
-    kms_key_arn                  = local.kms_key_arn
+    arn_runner_instance_role  = aws_iam_role.runner.arn
+    sqs_arn                   = var.sqs_build_queue.arn
+    github_app_id_arn         = var.github_app_parameters.id.arn
+    github_app_key_base64_arn = var.github_app_parameters.key_base64.arn
+    kms_key_arn               = local.kms_key_arn
   })
 }
 

modules/runners/variables.tf

@@ -116,10 +116,8 @@ variable "enable_organization_runners" {
 variable "github_app_parameters" {
   description = "Parameter Store for GitHub App Parameters."
   type = object({
-    key_base64    = map(string)
-    id            = map(string)
-    client_id     = map(string)
-    client_secret = map(string)
+    key_base64 = map(string)
+    id         = map(string)
   })
 }