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.

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

Switch from App Engine to Cloud Run for zosia site #4

Open
wants to merge 3 commits into
base: zosia_site_v2024
Choose a base branch
from

Conversation

bartacc
Copy link
Collaborator

@bartacc bartacc commented Jun 28, 2024

These changes were tested with zosia site code from that PR on my personal Google Cloud account. The website works correctly.

This PR contains:

  • Removed App Engine configuration
  • Added private Google Artifact Registry Repository which contains docker images of production zosia site
  • Added migrate job
  • Added collectstatic job with a public Google Cloud Storage bucket which contains all static files for production website
  • Added zosia Cloud Run Service which runs the website using docker image from Google Artifact Registry Repository

TODO later:

  • Add domain mapping to zosia service, so that it can be accessed from zosia.org domain

Copy link
Member

@Bozydarek Bozydarek left a comment

Choose a reason for hiding this comment

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

Looks great 💪 With all this code, it should be much easier for future organizers to set up the Zosia website for future editions 🚀 However, I have a feeling we should improve the README file to make it more readable. Some testing (with other in our association) might be required.

Comment on lines +78 to +92
env {
name = "GOOGLE_CLOUD_PROJECT"
value = local.project_id
}

env {
name = "GCS_BUCKET_NAME"
value = google_storage_bucket.static_files_bucket.name
}

# TODO: Add domain mapping to zosia.org and www.zosia.org
env {
name = "HOSTS"
value = "zosia.org, www.zosia.org"
}
Copy link
Member

Choose a reason for hiding this comment

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

In general, I would consider configuring it as a locally defined map and using dynamic blocks with for_each. However, for three variables, it can remain as is :)

}

resource "google_storage_bucket" "static_files_bucket" {
name = "${random_id.static_files_bucket_prefix.hex}-static-files-bucket"
Copy link
Member

Choose a reason for hiding this comment

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

I understand what you're trying to achieve, but you could have simply created a more readable prefix like "ksiuwr-" or "ksiuwr-infra-" ;)

Comment on lines +115 to +119
cors {
origin = ["*"]
method = ["GET"]
response_header = ["*"]
}
Copy link
Member

Choose a reason for hiding this comment

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

If this bucket is not set to act as a "website", then you don't need to define CORS.

@@ -1,6 +1,7 @@
locals {
project_id = ""
region = "europe-central2"
project_id = ""
Copy link
Member

Choose a reason for hiding this comment

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

You can get this from the project datasource, but it's better to provide it explicitly to ensure you're running it on the correct project. Therefore, I suggest adding a comment like "< update this >".

Comment on lines 14 to +26
project = local.project_id
service = "sql-component.googleapis.com"
}

resource "google_project_service" "artifactregistry_service" {
project = local.project_id
service = "artifactregistry.googleapis.com"
}

resource "google_project_service" "cloudrun_service" {
project = local.project_id
service = "run.googleapis.com"
}
Copy link
Member

Choose a reason for hiding this comment

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

You can change it to a single resource declaration with for_each and provide a list of services to activate.

Comment on lines +1 to +2
resource "google_service_account" "cloudrun_service_account" {
account_id = "cloudrun-service-account"
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest renaming it to indicate that it is a service account for the Zosia website service (even though currently this is the only service you have in this infrastructure).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants