Skip to content

AWS OIDC: DeployDatabaseService per VPC#35899

Merged
marcoandredinis merged 3 commits intomasterfrom
marco/deployservice_vpc
Jan 8, 2024
Merged

AWS OIDC: DeployDatabaseService per VPC#35899
marcoandredinis merged 3 commits intomasterfrom
marco/deployservice_vpc

Conversation

@marcoandredinis
Copy link
Copy Markdown
Contributor

@marcoandredinis marcoandredinis commented Dec 19, 2023

This PR adds a new method for deploying services using the AWS OIDC Integration.
It will deploy multiple, one per element in deployments array.

Each one will only proxy a subset of existing Databases, using the account-id, region and vpc-id labels to select them.

Demo

POST .../deploydatabaseservice
{
    "region": "us-east-1",
    "taskRoleARN": "TeleportMarcoTestRDSSingleDB",
    "deployments": [
        {"vpcId": "vpc-<redacted1>", "subnetIds":["subnet-1", "subnet-2"]},
        {"vpcId": "vpc-<redacted2>", "subnetIds":["subnet-1", "subnet-2"]}
    ]
}

<--
{
  "clusterArn": "arn:aws:ecs:us-east-1:278576220453:cluster/lenix-teleport",
  "clusterDashboardUrl": "https://us-east-1.console.aws.amazon.com/ecs/v2/clusters/lenix-teleport/services"
}
2023-12-20T18:52:24Z DEBU             Upsert ECS Cluster account-id:278576220453 deployservice:database-service ecs-cluster:lenix-teleport integration:teleportdev region:us-east-1 awsoidc/deploydatabaseservice.go:214
2023-12-20T18:52:25Z DEBU             Upsert ECS TaskDefinition. account-id:278576220453 deployservice:database-service ecs-cluster:lenix-teleport ecs-service-name:database-service-vpc-<redacted1> ecs-task-name:lenix-teleport-database-service-vpc-<redacted1> integration:teleportdev region:us-east-1 vpc-id:vpc-<redacted1> awsoidc/deploydatabaseservice.go:254
2023-12-20T18:52:26Z DEBU             Upsert ECS Service. account-id:278576220453 deployservice:database-service ecs-cluster:lenix-teleport ecs-service-name:database-service-vpc-<redacted1> ecs-task-name:lenix-teleport-database-service-vpc-<redacted1> integration:teleportdev region:us-east-1 vpc-id:vpc-<redacted1> awsoidc/deploydatabaseservice.go:268
2023-12-20T18:52:27Z DEBU             Upsert ECS TaskDefinition. account-id:278576220453 deployservice:database-service ecs-cluster:lenix-teleport ecs-service-name:database-service-vpc-<redacted2> ecs-task-name:lenix-teleport-database-service-vpc-<redacted2> integration:teleportdev region:us-east-1 vpc-id:vpc-<redacted2> awsoidc/deploydatabaseservice.go:254
2023-12-20T18:52:27Z DEBU             Upsert ECS Service. account-id:278576220453 deployservice:database-service ecs-cluster:lenix-teleport ecs-service-name:database-service-vpc-<redacted2> ecs-task-name:lenix-teleport-database-service-vpc-<redacted2> integration:teleportdev region:us-east-1 vpc-id:vpc-<redacted2> awsoidc/deploydatabaseservice.go:268

Services running in ECS Cluster:
image

DB Services heartbeat back their labels tctl get db_services:

kind: db_service
metadata:
  expires: "2023-12-20T19:07:15.537837586Z"
  id: 1703098635576820826
  name: 3999f6c0-6612-44b2-89f9-c837dc1bf039
  revision: f86f72c6-6181-40cb-9745-eed311782f9d
spec:
  resources:
  - aws: {}
    labels:
      account-id: "278576220453"
      region: us-east-1
      vpc-id: vpc-02xyz
version: v1
---
kind: db_service
metadata:
  expires: "2023-12-20T19:07:20.354861775Z"
  id: 1703098640393153666
  name: ff88fe75-0022-4963-8a80-c28571de2785
  revision: 1df4172c-f016-4875-9087-b1f4b1a144c1
spec:
  resources:
  - aws: {}
    labels:
      account-id: "278576220453"
      region: us-east-1
      vpc-id: vpc-09xyz
version: v1

We are able to use the DatabaseService and access the Database

$ tsh db connect marcodemodb01-rds-us-east-1-278576220453
psql (15.5, server 14.7)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_128_GCM_SHA256, compression: off)
Type "help" for help.

postgres=> select 1+1;
 ?column? 
----------
        2
(1 row)

postgres=> 

Logs from the DatabaseService:
image

@marcoandredinis marcoandredinis added discover Issues related to Teleport Discover backport/branch/v14 no-changelog Indicates that a PR does not require a changelog entry labels Dec 20, 2023
Comment thread lib/cloud/aws/policy_statements.go Outdated
Copy link
Copy Markdown
Contributor Author

@marcoandredinis marcoandredinis Dec 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the Auto-Upgrade service expects a single ECS Service on the cluster, and with a specific name.
This PR changes that assumption: we now have multiple services and their name depends on the VPCs.
So, Auto-Upgrade is broken for this new version of the DeployService.

To get multiple ECS Services from a ECS Cluster without their name, we must use the ListServices.
(DescribeServices requires the Services name as input).

Given that the Auto-Upgrader runs unattended, it has no way to ask the user to add permissions in case it lacks any.
Adding this new permission now ensures we have the required permissions and are ready to implement the Auto-Upgrader for multiple ECS Services.

A followup PR will fix the Auto-Upgrader for this new version of the DeployService.

@marcoandredinis marcoandredinis marked this pull request as ready for review December 20, 2023 19:35
@github-actions github-actions Bot requested review from gzdunek and kimlisa December 20, 2023 19:35
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but i couldn't test it yet due to build issues. i'm going to attempt again in the morning tested 👍

Comment thread lib/web/apiserver.go Outdated
Comment thread lib/web/ui/integration.go Outdated
Comment thread lib/web/ui/integration.go
@marcoandredinis marcoandredinis force-pushed the marco/deployservice_vpc branch 4 times, most recently from f9e9e53 to 9c692ba Compare December 21, 2023 14:18
@marcoandredinis marcoandredinis requested review from r0mant and removed request for gzdunek December 21, 2023 15:24
@marcoandredinis marcoandredinis force-pushed the marco/deployservice_vpc branch 2 times, most recently from 608f3ec to 95cd53c Compare December 22, 2023 12:46
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good but I've a question about API design.

Comment thread lib/integrations/awsoidc/deployservice.go Outdated
Comment thread lib/web/apiserver.go Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we're introducing a new API specifically for database services, rather than either updating existing "deployservice" to allow deploying multiple, or at least having a separate generic "deployservices"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deployservice API tries to be generic for every service, but I don't think that was going to work.
When I was trying to re-use it, it didn't felt very flexible. And that's a considerable small change: deploy multiple database services instead of one.
If we changed the API, we would also need to change the UI.

This new API is more specific, and is able to handle the single RDS enrollment anyway, we just need to change the UI.

After changing the UI, the previous endpoint can be removed.

marcoandredinis and others added 3 commits January 4, 2024 15:32
Co-authored-by: Lisa Kim <lisa@goteleport.com>
Co-authored-by: Lisa Kim <lisa@goteleport.com>
@marcoandredinis marcoandredinis added this pull request to the merge queue Jan 8, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 8, 2024
@marcoandredinis marcoandredinis added this pull request to the merge queue Jan 8, 2024
Merged via the queue into master with commit 2cc0d62 Jan 8, 2024
@marcoandredinis marcoandredinis deleted the marco/deployservice_vpc branch January 8, 2024 17:06
@public-teleport-github-review-bot
Copy link
Copy Markdown

@marcoandredinis See the table below for backport results.

Branch Result
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discover Issues related to Teleport Discover no-changelog Indicates that a PR does not require a changelog entry size/lg

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants