developmentseed / eoapi-cdk

AWS CDK constructs for deploying eoAPI
https://developmentseed.org/eoapi-cdk
11 stars 4 forks source link

feat: update runtimes and add pgstac customization options #100

Closed vincentsarago closed 7 months ago

vincentsarago commented 8 months ago

This PR closes #88 and add customizations option for PgSTAC bootstrap to:

:warning: Checklist if your PR is changing anything else than documentation

Merge request description

emileten commented 8 months ago

deployment workflow https://github.com/developmentseed/eoapi-cdk/actions/runs/8045501396

emileten commented 8 months ago

Questions for my understanding

enable/disable the context extension enable/disable the mosaic table index

What were we doing before, was it disabled by default?

vincentsarago commented 8 months ago

@emileten we were adding the mosaic index by default but not the context extension, maybe we can switch back to false 👍

emileten commented 8 months ago

Maybe we can switch back to false

I'll defer to you on this decision, I don't know much about these options. Doesn't sound like it hurts to have the context on by default ?

emileten commented 8 months ago

A live example of the bootstrapper logs

The workflow failed again, there's something we need to fix in the bootstrapper changes https://github.com/developmentseed/eoapi-cdk/actions/runs/8045602231/job/21971192880#step:9:2478

The lambda logs https://us-east-1.console.aws.amazon.com/cloudwatch/home?region=us-east-1#logsV2:log-groups/log-group/$252Faws$252Flambda$252Fpgstac60fe14-pgstacdblambda91737152-ELDP6T9rb8jI/log-events/2024$252F02$252F26$252F$255B$2524LATEST$255Df9a0fff7fa2042bb84e34c9220cb7071

Unable to bootstrap database with exception=must be member of role "pgstac_ingest" 

Also httpx failed responding with the error

send(..) failed executing httpx.put(..): can only concatenate str (not "int") to str 
emileten commented 8 months ago

@vincentsarago just checking, were you expecting the last commit to fix the bug ? or is there more to do

vincentsarago commented 8 months ago

I was hopping yes 😅 but couldn't see where I had to click to run the deployment workflow

emileten commented 8 months ago

ugh sorry, just started one. Will show you later

emileten commented 8 months ago

@vincentsarago looks like we still have the problem

[ERROR] InsufficientPrivilege: must be member of role "pgstac_ingest"
Traceback (most recent call last):
  File "/var/task/handler.py", line 280, in handler
    raise e
  File "/var/task/handler.py", line 243, in handler
    Migrate(pgdb).run_migration(params["pgstac_version"])
  File "/var/task/pypgstac/migrate.py", line 149, in run_migration
    cur.execute(migration_sql)
  File "/var/task/psycopg/cursor.py", line 732, in execute
    raise ex.with_traceback(None)

I can take a look tomorrow as well.

FYI to run a deployment:

vincentsarago commented 7 months ago

@emileten let's fall back to pgstac 0.7.10 to validate the changes and then we can debug why it doesn't work for pgstac 0.8.4

emileten commented 7 months ago

@vincentsarago like this https://github.com/developmentseed/eoapi-cdk/pull/100/commits/0d675e21d782cdefa7697d5310d6e6aece21a866?

vincentsarago commented 7 months ago

@emileten just pgstac, we can keep titiler-pgstac and tipg to the latest version

emileten commented 7 months ago

Ok seems to work https://github.com/developmentseed/eoapi-cdk/actions/runs/8204255860/job/22438560336#step:10:798.

Now we need to

debug why it doesn't work for pgstac 0.8.4

😄

emileten commented 7 months ago

there seems to be a 0.8.3 change that sounds like it could have triggered this https://github.com/stac-utils/pgstac/blob/e0b7de4c9c8130c55de9fe6eb0a87324cd705135/CHANGELOG.md?plain=1#L37 we can try with the previous version

vincentsarago commented 7 months ago

👁️ https://github.com/stac-utils/pgstac/issues/239

vincentsarago commented 7 months ago

@emileten if I understand well

looking at https://github.com/developmentseed/eoapi-cdk/pull/100#issuecomment-1983778135 it seems the migration step fails with

[ERROR] InsufficientPrivilege: must be member of role "pgstac_ingest"

in https://github.com/developmentseed/eoapi-cdk/blob/44252534b55ce3266add771ec1f4de7c6848e53e/lib/database/bootstrapper_runtime/handler.py#L243

when we use

https://github.com/developmentseed/eoapi-cdk/blob/44252534b55ce3266add771ec1f4de7c6848e53e/lib/database/bootstrapper_runtime/handler.py#L231-L232

username/password (must be postgres by default)

After the migration we then grant the username/password (pgstac_user by default)

https://github.com/developmentseed/eoapi-cdk/blob/44252534b55ce3266add771ec1f4de7c6848e53e/lib/database/bootstrapper_runtime/handler.py#L133-L135

so to me it seems that we should GRANT pgstac_ingest TO postgres; but to me it seems weird because pgstac_ingest role doesn't exist before the migration starts...

cc @bitner

bitner commented 7 months ago

@emileten @vincentsarago The pypgstac migrate should be run as superuser (or a role with sufficient root-like privileges in the case of systems like RDS that do not allow true superusers -- this is typically the postgres role). pypgstac migrate on a fresh database will create the pgstac_read, pgstac_ingest, and pgstac_admin roles.

Once pgstac has been installed, you can assign these roles to other database login roles. Ideally, you would use least-access necessary roles for different tasks. Best practice would be to have three roles to interact with pgstac.

emileten commented 7 months ago

https://github.com/developmentseed/eoapi-cdk/actions/runs/8266556264/job/22614791059