Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pudl usage metrics gcp infrastructure #3841

Merged
merged 4 commits into from
Sep 17, 2024
Merged

Add pudl usage metrics gcp infrastructure #3841

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
63f524d
Add pudl usage metrics gcp infrastructure
bendnorman Sep 11, 2024
1aaac08
Give pudl usage metrics etl service account permission to read from a…
bendnorman Sep 11, 2024
fe095b9
Give pudl usage metrics etl service account permission to read from a…
bendnorman Sep 11, 2024
64234ba
Give usage metrics service account permission on s3 logs bucket
bendnorman Sep 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion terraform/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ resource "google_cloud_run_v2_service" "pudl-superset" {
volumes {
name = "cloudsql"
cloud_sql_instance {
instances = ["catalyst-cooperative-pudl:us-central1:superset-database"]
instances = ["catalyst-cooperative-pudl:us-central1:superset-database", "catalyst-cooperative-pudl:us-central1:pudl-usage-metrics-db"]
}
}
}
Expand Down Expand Up @@ -396,3 +396,47 @@ resource "google_service_account_iam_member" "gce-default-account-iam" {
role = "roles/iam.serviceAccountUser"
member = "serviceAccount:[email protected]"
}

resource "google_secret_manager_secret" "pudl_usage_metrics_db_connection_string" {
secret_id = "pudl-usage-metrics-db-connection-string"
replication {
auto {}
}
}
Comment on lines +400 to +405
Copy link
Member Author

Choose a reason for hiding this comment

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

I figured I'd save the connection string in case we need to reconnect the db to superset.


resource "google_storage_bucket" "pudl_usage_metrics_archive_bucket" {
name = "pudl-usage-metrics-archives.catalyst.coop"
location = "US"
storage_class = "STANDARD"

uniform_bucket_level_access = true
}

resource "google_service_account" "usage_metrics_archiver" {
account_id = "usage-metrics-archiver"
display_name = "PUDL usage metrics archiver github action service account"
}
Comment on lines +415 to +418
Copy link
Member Author

Choose a reason for hiding this comment

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

I did create a service account key for the GitHub action in the business repo. @jdangerx would love a WIF tutorial soon!

Copy link
Member

Choose a reason for hiding this comment

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

I've forgotten everything I know about WIF but could re-learn it!


resource "google_storage_bucket_iam_member" "usage_metrics_archiver_gcs_iam" {
for_each = toset(["roles/storage.objectCreator", "roles/storage.objectViewer"])

bucket = google_storage_bucket.pudl_usage_metrics_archive_bucket.name
role = each.key
member = "serviceAccount:${google_service_account.usage_metrics_archiver.email}"
}

resource "google_storage_bucket_iam_member" "usage_metrics_etl_gcs_iam" {
for_each = toset(["roles/storage.legacyBucketReader", "roles/storage.objectViewer"])
Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't find a non legacy role that gives a principle the storage.buckets.get and storage.objects.get permissions. Seems like the GCS python client wants both to access objects in a bucket.

Copy link
Member

Choose a reason for hiding this comment

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

This is probably fine. If we want to switch to non-legacy roles, it looks like we could give roles/storage.objectUser for objects.get and the confusingly named roles/storage.insightsCollectorService gets you buckets.get.


bucket = google_storage_bucket.pudl_usage_metrics_archive_bucket.name
role = each.key
member = "serviceAccount:pudl-usage-metrics-etl@catalyst-cooperative-pudl.iam.gserviceaccount.com"
}

resource "google_storage_bucket_iam_member" "usage_metrics_etl_s3_logs_gcs_iam" {
for_each = toset(["roles/storage.legacyBucketReader", "roles/storage.objectViewer"])

bucket = "pudl-s3-logs.catalyst.coop"
Copy link
Member

Choose a reason for hiding this comment

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

Should we manage this bucket via TF too?

Copy link
Member Author

@bendnorman bendnorman Sep 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Probably! How can we manage a resource in terraform that has already been created in the UI? Also, we should probably move the contents of pudl-s3-logs.catalyst.coop to pudl-usage-metrics-archives.catalyst.coop/s3/ for consistency.

Copy link
Member

Choose a reason for hiding this comment

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

Here's the documentation about handling "resource drift"!

Copy link
Member

Choose a reason for hiding this comment

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

Do we really need 4 different name components on that bucket? Is there a non-archives bucket that we need to differentiate it from? Do we foresee having non-PUDL usage metrics that would need to be stored somewhere else? Or could we put all usage metrics data under usage-metrics.catalyst.coop?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we probably don't need the pudl part because the bucket lives in the pudl-catalyst-cooperative GCP project. It would be nice to keep archive because I could imagine having a bucket to store the outputs of our usage metric ETL to parquet files if we ever move to Big Query. How about usage-metrics-archive.catalyst.coop?

Copy link
Member

Choose a reason for hiding this comment

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

@bendnorman Would love to coordinate a name change with the flight of PRs in pudl-usage-metrics as much as possible, so just let me know what you're thinking.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we'll keep it as is for now but I created an issue to rename it down the line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also created an issue for moving the pudl-s3-logs.catalyst.coop bucket.

role = each.key
member = "serviceAccount:pudl-usage-metrics-etl@catalyst-cooperative-pudl.iam.gserviceaccount.com"
}
Loading