BYOB: Bootstrap Athena Infrastructure by logand22 · Pull Request #33272 · gravitational/teleport

logand22 · 2023-10-10T22:28:27Z

This PR is a part of https://github.com/gravitational/cloud/blob/master/rfd/0077-Bring-your-own-bucket.md. This PR adds the ability to deploy the infrastructure necessary to run the new scalable audit logs on the desired infrastructure instead of the cloud.

We expect the customer to run the following AWS services themselves:

Glue Database & Table
Athena Workspace
Long Term S3 Bucket & Transient S3 Bucket

In addition to the above resources, we also are slightly opinionated about things like:

Bucket Lifecycle Configuration
Object lock on long-term bucket

This is stay in line with cloud in addition to encourage some best practices.

ℹ️ It is not required for the customer to deploy the infrastructure using our script. This script should mainly serve to help bootstrap customers who don't wish to use something like Terraform instead.

nklaassen

Looking pretty good, a few comments

nklaassen · 2023-10-18T18:56:59Z

+	integrationBootstrapCmd := integrationCmd.Command("bootstrap", "Bootstrap an integration")
+	integrationBootstrapCreateExternalCloudAuditCmd := integrationBootstrapCmd.Command("externalcloudaudit", "Bootstraps external cloud audit infrastructure.")
+	integrationBootstrapCreateExternalCloudAuditCmd.Flag("aws-region", "The region to use. Overrides config/env settings").StringVar(&ccf.
+		IntegrationBootstrapCreateExternalCloudAuditArguments.Region)
+	integrationBootstrapCreateExternalCloudAuditCmd.Flag("long-term-storage-bucket", "S3 Bucket for long term storage of audit events and session recordings.").
+		Required().StringVar(&ccf.IntegrationBootstrapCreateExternalCloudAuditArguments.LongTermStorageBucket)
+	integrationBootstrapCreateExternalCloudAuditCmd.Flag("transient-bucket", "S3 bucket for transient storage of athena query results and large payloads.").
+		Required().StringVar(&ccf.IntegrationBootstrapCreateExternalCloudAuditArguments.TransientBucket)
+
+	integrationBootstrapCreateExternalCloudAuditCmd.Flag("athena-workgroup", "Name of athena workgroup").Default("teleport").StringVar(&ccf.
+		IntegrationBootstrapCreateExternalCloudAuditArguments.AthenaWorkgroup)
+	integrationBootstrapCreateExternalCloudAuditCmd.Flag("glue-database-name", "Name of the glue database to create").Default("teleport").StringVar(&ccf.
+		IntegrationBootstrapCreateExternalCloudAuditArguments.DatabaseName)
+	integrationBootstrapCreateExternalCloudAuditCmd.Flag("glue-table-name", "Name of the glue table to create").Default("auditevents").StringVar(&ccf.
+		IntegrationBootstrapCreateExternalCloudAuditArguments.TableName)


I met with the design team yesterday and we agreed it would be preferable to have a single one-off command to handle both the resource bootstrap and creating the IAM permissions. Instead of a whole bunch of arguments I think I'd like to pass the entire ExternalCloudAudit config as a JSON blob. Maybe leave the CLI command out of this PR, I will handle combining it with the IAM permissions command I already created, if you can just write the function that handles creating all the resources, taking this ExternalCloudAudit type and the region as input. Eventually I think I will actually add the region to that type, we should store it somewhere.

orca-security-us

Orca Security Scan Summary

Status	Check	Issues by priority
Passed	Infrastructure as Code	0 0 0 0	View in Orca
Passed	Vulnerabilities	0 0 0 0	View in Orca
Passed	Secrets	0 0 0 0	View in Orca

nklaassen

This is looking really clean, I like it

nklaassen · 2023-10-20T23:33:15Z

+		return "", "", trace.BadParameter("input is nil")
+	}
+
+	auditEventsBucket, err := url.Parse(input.AuditEventsLongTermURI)


I wrote something to validate s3 URIs here, maybe you can reuse it (and/or improve it, I just realized I forgot to check for the s3 scheme 🤦 )

teleport/api/types/externalcloudaudit/externalcloudaudit.go

Line 133 in 9f8530e

func ValidateS3URI(uri string) error {

But you'll still have to re-parse them here so maybe not much benefit

I'm wondering if it it's better to store the URIs in there raw form or already parsed. That way, we validate and parse before storing it and then don't need to parse later. Idk what's more common. In this case I'd have to parse again to extract the values as I don't need prefix. In addition to my stricter rules I think it's best to keep them separate for now.

nklaassen · 2023-10-20T23:41:01Z

+
+	for _, tc := range tt {
+		t.Run(tc.desc, func(t *testing.T) {
+			clt := &fakeBootstrapInfraClient{


if you want to go one step further with this test, the fake client could actually store some representation of everything that got created, and you could then check that it's right

You could use this to write a test for idempotency by making the client actually return AlreadyExists errors if the thing was already created. I guess that's kind of what you're doing with the *Exists booleans but this way you could even call BootstrapInfra twice with semi-realistic results, and optionally make the first call fail halfway through with some other error

Yea I definitely am looking for improvements on the testing side of things. I'll try to incorporate what you mentioned above and see how it turns out thanks.

So I refactored slightly given your recommendations in 48cda44. Let me know what you think and how I could potentially improve it further

github-actions · 2023-10-25T00:18:26Z

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

github-actions · 2023-10-25T18:50:03Z

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

logand22 · 2023-10-27T19:41:40Z

Basing the no-changelog label off the previous externalcloudaudit-iam command not including it. I assume we'll have a changelog entry for the entire feature in a later PR.

codingllama

Reviewing mostly for Go style, as I'm not familiar with the AWS RPCs themselves.

codingllama

LGTM.

public-teleport-github-review-bot · 2023-11-01T22:39:26Z

@logand22 See the table below for backport results.

Branch	Result
branch/v13	Failed
branch/v14	Failed

* [v14] external cloud audit proto Backport #33022 to branch/v14 * [v14] external_cloud_audit: add resource layer Backport #32833 to branch/v14 * [v14] feat: IAM permissions for BYOBucket Backport #33416 to branch/v14 This commit adds a one-off teleport command that configures the necessary IAM permissions for the upcoming External Cloud Audit (BYOBucket) feature. An example command invocation looks like: ``` $ teleport integration configure externalcloudaudit-iam \ --aws-region us-west-2 --role nic-byob-test --policy nic-byob \ --session-recordings s3://nic-byob/sess-rec-v2 \ --audit-events s3://nic-byob/events --athena-results s3://nic-byob/results \ --athena-workgroup primary --glue-database nic_byob --glue-table nic_byob_table ``` In normal usage this command will be generated for the user so that they can just copy a command from the Web UI and run it in AWS CloudShell. The permissions generated here are based on https://github.com/gravitational/cloud/blob/rfd/77-bring-your-own-bucket/rfd/0077-Bring-your-own-bucket.md, but only include the permissions necessary for using the feature at runtime and not any permissions necessary to bootstrap/create the resources. * [v14] feat: generate randomized ExternalCloudAudit config Backport #33555 to branch/v14 * [v14] BYOB: Bootstrap Athena Infrastructure Backport #33272 to branch/v14 * [v14] feat: cached auto-refreshing AWS credentials for BYOBucket Backport #34380 to branch/v14 This commit implements a "Configurator" for the BYOBucket feature that provides AWS credentials that can be used by the v1 or v2 AWS SDKs for Go. These credentials are generated via an AWS OIDC integration: auth signs a JWT and we swap that with AWS STS for AWS credentials. It also reports whether or not the BYOB feature `IsUsed()` currently, and provides access to the current cluster ExternalCloudAudit spec. This looks a bit weird because of a chicken-egg problem where the audit log must be set up before the auth server can be created, but the auth server must be created to provide the OIDC signing facilities. This will be more clear in following PRs. * [v14] fix: correct IAM policies for BYOB Backport #34484 to branch/v14 This commit fixes the IAM policies generated by the oneoff externalcloudaudit bootstrap command based on manual testing, and brings them more in line with the original RFD https://github.com/gravitational/cloud/blob/master/rfd/0077-Bring-your-own-bucket.md * [v14] feat: enable External Cloud Audit backend Backport #34606 to branch/v14 This commit enables the External Cloud Audit (BYOBucket) feature with a fully functional backend by setting up the Athena and S3 audit components with the right AWS configurations and resource locations. * [v14] Add ExternalCloudAudit permissions to user context ACL Backport #34289 to branch/v14 --------- Co-authored-by: Tobiasz Heller <14020794+tobiaszheller@users.noreply.github.com> Co-authored-by: Logan Davis <38335829+logand22@users.noreply.github.com> Co-authored-by: matheus <matheus.battirola@goteleport.com>

logand22 force-pushed the logan/bootstrap-athena-infra branch from 62e8a86 to b01cac2 Compare October 10, 2023 22:30

nklaassen reviewed Oct 10, 2023

View reviewed changes

nklaassen reviewed Oct 12, 2023

View reviewed changes

Comment thread lib/integrations/externalcloudaudit/long_term_storage.go Outdated

nklaassen mentioned this pull request Oct 17, 2023

feat: generate randomized ExternalCloudAudit config #33555

Merged

nklaassen reviewed Oct 18, 2023

View reviewed changes

logand22 force-pushed the logan/bootstrap-athena-infra branch from d9c7317 to d48be80 Compare October 18, 2023 20:52

orca-security-us Bot reviewed Oct 19, 2023

View reviewed changes

nklaassen reviewed Oct 20, 2023

View reviewed changes

logand22 added 12 commits October 23, 2023 11:27

Add downloads to checkpoint file

3001168

Initial externalcloudaudit bootstrap

60d2558

Update changes

119de79

Undo changes to migration

421f42b

Rework

ecb24a1

Refactor

4d31138

Add simple tests

a587f36

Fix gomod?

ef44a7b

Remove large payload reference

9f41dc2

Fix athena workgroup comment

d375b4a

Simplify some error handling and returns

a3f2de5

Improve error mesaging

a560205

logand22 force-pushed the logan/bootstrap-athena-infra branch from fba93f3 to a560205 Compare October 23, 2023 16:30

logand22 and others added 9 commits October 23, 2023 12:06

Update generate draft

258a3b8

Added prefixes to tests

0478855

Add downloads to checkpoint file

c3b072c

Merge branch 'master' into logan/bootstrap-athena-infra

e88f451

Refactor tests

48cda44

Merge branch 'master' into logan/bootstrap-athena-infra

4461347

Fix extra bootstrap err

d5beeaf

Fix linter issues

ed7f2f8

Add license file

26310d2

logand22 added 2 commits October 24, 2023 18:47

Update test

d05fb12

Undo unintended migratin chages

ce0de38

logand22 marked this pull request as ready for review October 25, 2023 00:17

logand22 requested a review from nklaassen October 25, 2023 00:18

github-actions Bot added the size/lg label Oct 25, 2023

github-actions Bot requested review from Joerger and codingllama October 25, 2023 00:18

Merge branch 'master' into logan/bootstrap-athena-infra

02f90f8

logand22 added the no-changelog Indicates that a PR does not require a changelog entry label Oct 27, 2023

logand22 added 2 commits October 27, 2023 14:41

Merge branch 'master' into logan/bootstrap-athena-infra

d32c3e8

Merge branch 'master' into logan/bootstrap-athena-infra

6830c94

nklaassen approved these changes Oct 31, 2023

View reviewed changes

Comment thread tool/teleport/common/teleport.go Outdated

codingllama reviewed Oct 31, 2023

View reviewed changes

logand22 added 4 commits October 31, 2023 13:34

Remove testing return

141fbba

Address PR review comments

ffc9570

Update testing package name

ece8a17

Use errWanted and ErrorContains

a56c088

codingllama approved these changes Nov 1, 2023

View reviewed changes

public-teleport-github-review-bot Bot removed the request for review from Joerger November 1, 2023 16:27

logand22 added backport/branch/v13 labels Nov 1, 2023

logand22 added this pull request to the merge queue Nov 1, 2023

Merged via the queue into master with commit b084c7e Nov 1, 2023

logand22 deleted the logan/bootstrap-athena-infra branch November 1, 2023 22:37

Conversation

logand22 commented Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nklaassen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nklaassen Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

orca-security-us Bot left a comment

Choose a reason for hiding this comment

Orca Security Scan Summary

Uh oh!

nklaassen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nklaassen Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

logand22 Oct 23, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nklaassen Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

logand22 Oct 23, 2023

Choose a reason for hiding this comment

Uh oh!

logand22 Oct 24, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 25, 2023

Uh oh!

github-actions Bot commented Oct 25, 2023

Uh oh!

logand22 commented Oct 27, 2023

Uh oh!

Uh oh!

codingllama left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codingllama left a comment

Choose a reason for hiding this comment

Uh oh!

public-teleport-github-review-bot Bot commented Nov 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

logand22 commented Oct 10, 2023 •

edited

Loading