add dev cloudcasting api module and vars #895

braddf · 2025-08-20T16:11:10Z

Pull Request

Description

Adding terraform module for new Cloudcasting API deploy.

N.B. this may be subject to change, and/or we might simplify this module and incorporate some functionality into existing services, e.g. the API elements into the Quartz API.

Checklist:

My code follows OCF's coding style guidelines
I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
I have checked my code and corrected any misspellings

peterdudfield · 2025-08-20T16:55:04Z

terraform/nowcasting/development/variables.tf

+  type        = string
+  default     = "not-set"
+}
+variable "s3_secret_access_key" {


you wont need these things, it will use what IAM role and permission from that

Yeah I was just trying to get deployed with minimal changes to the code for now, this will be better going forward, but is obv different to how Suvan has had to set it up

Yea i see. I would probably still remove them here, and adjust Suvans code a tiny bit. In general the less credientials and secruty things we can do, the better.

Ill leave it up to you on that.

if you do keep this in, please make sure the access key and id are tightly locked down, and only have access to relevant things

If the code uses boto3 it should fallback to the default connection strategy. I think EB might even set these variables in the environment by default so you might not even need to make any code changes at all - not that I've looked at the code.

peterdudfield · 2025-08-20T16:55:34Z

terraform/nowcasting/development/main.tf

+    { "name" : "ENVIRONMENT", "value" : local.environment },
+    { "name" : "S3_BUCKET_NAME", "value" : var.s3_cloudcasting_bucket_name },
+    { "name" : "S3_REGION_NAME", "value" : var.s3_cloudcasting_region_name },
+    { "name" : "S3_ACCESS_KEY_ID", "value" : var.s3_access_key_id },


as below, you can remove S3_ACCESS_KEY_ID and S3_SECRET_ACCESS_KEY.
There's a small chance we will have to change the code accordindly

Might need to add the

s3_bucket = [{ bucket_read_policy_arn = module.s3.iam-policy-s3-sat-read.arn }]

when replacing these too.

peterdudfield

Looks great, just some small changes, but then it should be all good

devsjc · 2025-08-21T09:54:33Z

terraform/nowcasting/development/main.tf

+    { "name" : "AUTH0_DOMAIN", "value" : var.auth_domain },
+    { "name" : "AUTH0_API_AUDIENCE", "value" : var.auth_api_audience },
+    { "name" : "AUTH0_CLIENT_ID", "value" : var.auth_dashboard_client_id },
+    { "name" : "ENVIRONMENT", "value" : local.environment },


Does the app use any of these variables? If not, I'd remove them from here altogether.

devsjc · 2025-08-21T09:57:59Z

terraform/nowcasting/development/main.tf

+  ]
+  container-name = "cloudcasting_api"
+  container-tag  = var.api_version
+  container-registry = "openclimatefix"


Just worth checking, this will only work if the container is deployed to docker hub. I suspect it's only being deployed to GHCR, so this will in fact want to be ghcr.io/openclimatefix. In fact, most of these probably want to be changed to that, but that's another task for another day!

Although from what you're saying it sounds like there isn't a build CI, so maybe it is being manually pushed to docker hub - but in that case I wouldn't expect it to be under the openclimatefix org...

devsjc · 2025-08-21T10:04:07Z

terraform/nowcasting/development/main.tf

+    { "name" : "S3_BUCKET_NAME", "value" : var.s3_cloudcasting_bucket_name },
+    { "name" : "S3_REGION_NAME", "value" : var.s3_cloudcasting_region_name },


[1] These don't need to be variables - since they're accessing the sat bucket, we can use the output from the s3 module that creates the bucket:

{ "name" : "S3_BUCKET_NAME", "value" : module.s3.s3-sat-bucket.id }
{ "name" : "S3_REGION_NAME", "value" : var.region }

devsjc · 2025-08-21T10:04:29Z

terraform/nowcasting/development/variables.tf

+variable "s3_cloudcasting_bucket_name" {
+  description = "The name of the S3 bucket to use for storing data"
+  type        = string
+  default     = "not-set"
+}
+
+variable "s3_cloudcasting_region_name" {
+  description = "The region of the S3 bucket to use for storing data"
+  type        = string
+  default     = "not-set"
+}


These can be removed as per [1]

devsjc · 2025-08-21T10:05:26Z

terraform/nowcasting/development/variables.tf

+  type        = string
+  default     = "not-set"
+}
+variable "s3_secret_access_key" {


If the code uses boto3 it should fallback to the default connection strategy. I think EB might even set these variables in the environment by default so you might not even need to make any code changes at all - not that I've looked at the code.

devsjc · 2025-08-21T10:09:04Z

terraform/nowcasting/development/main.tf

+    { "name" : "ENVIRONMENT", "value" : local.environment },
+    { "name" : "S3_BUCKET_NAME", "value" : var.s3_cloudcasting_bucket_name },
+    { "name" : "S3_REGION_NAME", "value" : var.s3_cloudcasting_region_name },
+    { "name" : "S3_ACCESS_KEY_ID", "value" : var.s3_access_key_id },


Might need to add the

s3_bucket = [{ bucket_read_policy_arn = module.s3.iam-policy-s3-sat-read.arn }]

when replacing these too.

devsjc · 2025-08-21T10:10:26Z

terraform/nowcasting/development/main.tf

+    { "name" : "S3_DOWNLOAD_INTERVAL", "value" : var.s3_download_interval },
+    { "name" : "RELOAD", "value" : var.cloudcasting_reload},


[2] Do you see these changing often? If not, they might be alright hard coded in the module declaration here, instead of being a variable. Using a var isn't necessarily wrong, but it's just one fewer transient to keep track of. If we do keep them as vars, I'd prefix the s3_download_interval with cloudcasting as well, just so it's clear what it pertains to in the terraform app - it's a bit vague as-is.

devsjc · 2025-08-21T10:10:50Z

terraform/nowcasting/development/variables.tf

+variable "s3_download_interval" {
+  description = "The interval in seconds at which to download data from the S3 bucket"
+  type        = number
+  default     = 30
+}


Maybe can be removed as per [2]

devsjc · 2025-08-21T10:11:04Z

terraform/nowcasting/development/variables.tf

+variable "cloudcasting_reload" {
+  type = bool
+  default = true
+}


Maybe can be removed as per [2]

devsjc · 2025-08-21T10:12:22Z

terraform/nowcasting/development/main.tf

+    { "name" : "RELOAD", "value" : var.cloudcasting_reload},
+  ]
+  container-name = "cloudcasting_api"
+  container-tag  = var.api_version


This means it will use the same tag as the (Quartz)API - which I'm assuming isn't what you want? Unless the packaging version of this app is synced to the (Quartz)API?

peterdudfield · 2025-08-22T14:45:11Z

I hope this is ok @braddf , i changed this to a draft, as I understood you have currently deploy this a different way

add dev cloudcasting api module and vars

912ddbd

braddf requested a review from devsjc August 20, 2025 16:11

add cloudcasting app to readme; fix log links

7eacdea

braddf requested a review from peterdudfield August 20, 2025 16:33

peterdudfield reviewed Aug 20, 2025

View reviewed changes

peterdudfield requested changes Aug 20, 2025

View reviewed changes

peterdudfield approved these changes Aug 20, 2025

View reviewed changes

devsjc requested changes Aug 21, 2025

View reviewed changes

peterdudfield marked this pull request as draft August 22, 2025 14:44

		{ "name" : "S3_BUCKET_NAME", "value" : var.s3_cloudcasting_bucket_name },
		{ "name" : "S3_REGION_NAME", "value" : var.s3_cloudcasting_region_name },

		{ "name" : "S3_DOWNLOAD_INTERVAL", "value" : var.s3_download_interval },
		{ "name" : "RELOAD", "value" : var.cloudcasting_reload},

Uh oh!

add dev cloudcasting api module and vars #895

Are you sure you want to change the base?

add dev cloudcasting api module and vars #895

Uh oh!

Conversation

braddf commented Aug 20, 2025

Pull Request

Description

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterdudfield left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devsjc Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterdudfield commented Aug 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

devsjc Aug 21, 2025 •

edited

Loading