Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 2 additions & 0 deletions birdhouse/components/proxy/conf.d/frontend.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ map $http_x_forwarded_proto $real_scheme {
'' $scheme;
}

include /etc/nginx/conf.extra-directives.d/*/*.conf;

server {
listen 80 ${PROXY_LISTEN_80_PARAMS};
server_name localhost;
Expand Down
2 changes: 2 additions & 0 deletions birdhouse/components/stac-browser/default.env
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export STAC_BROWSER_IMAGE_URI='${STAC_BROWSER_IMAGE}'
# - https://github.com/radiantearth/stac-browser/blob/main/docs/docker.md
export STAC_BROWSER_PATH_PREFIX='/stac-browser/'

export STAC_ADDITIONAL_CORS_ORIGINS='https://geojson.io'
Copy link
Member

Choose a reason for hiding this comment

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

I assume space delimited based on the for loop?
Would need to be indicated since ,/; are common variants when dealing with headers.

I think it might also need the ${BIRDHOUSE_FQDN_PUBLIC} since stac-browser self-pings the stac API.

Should this parameter be in stac/default.env, or this one should update the variable additively?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I'll need to update comments and documentation explaining the format of everything.

I think it might also need the ${BIRDHOUSE_FQDN_PUBLIC} since stac-browser self-pings the stac API.

${BIRDHOUSE_FQDN_PUBLIC} is always allowed because it is the default value for the nginx map. So by default, requests from all domains (including ${BIRDHOUSE_FQDN_PUBLIC}) will allow it.

Should this parameter be in stac/default.env, or this one should update the variable additively?

It doesn't need to be. The default (withouth stac-browser) is that it is empty/unset.


export DELAYED_EVAL="
$DELAYED_EVAL
STAC_BROWSER_IMAGE
Expand Down
1 change: 1 addition & 0 deletions birdhouse/components/stac/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
config/magpie/config.yml
config/proxy/conf.extra-service.d/stac.conf
config/proxy/conf.extra-directives.d/stac.conf
config/canarie-api/canarie_api_monitoring.py
service-config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
map $http_origin $stac_origin_allowed {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remind me what the Nginx map directive do again?

This block means if the http_origin variable exists, create stac_origin_allowed variable with value ${BIRDHOUSE_PROXY_SCHEME}://${BIRDHOUSE_FQDN_PUBLIC}; ${STAC_ADDITIONAL_CORS_ORIGINS} ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's mostly right...

This block creates a new variable stac_origin_allowed whose value is based on the value of the http_origin variable.
If the value of http_origin matches any of the patterns defined in ${STAC_ADDITIONAL_CORS_ORIGINS} then the stac_origin_allowed value will be set to http_origin.
Otherwise stac_origin_allowed will be set to ${BIRDHOUSE_PROXY_SCHEME}://${BIRDHOUSE_FQDN_PUBLIC} (the default).

default ${BIRDHOUSE_PROXY_SCHEME}://${BIRDHOUSE_FQDN_PUBLIC};
${_STAC_ADDITIONAL_CORS_ORIGINS}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Host $host:$server_port;
proxy_buffering off;
include /etc/nginx/conf.d/cors.include;
proxy_hide_header 'Access-Control-Allow-Origin';
add_header Access-Control-Allow-Origin $stac_origin_allowed;
Copy link
Member

@fmigneault fmigneault Oct 21, 2025

Choose a reason for hiding this comment

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

I think the cors.include should still be there since it adds other definitions (allowed headers, allowed methods, etc.) which are also required (and extra ones might be needed as well since it was actually very limited, while STAC API allows PUT, DELETE, etc.).

I think proxy_hide_header 'Access-Control-Allow-Origin'; will remove it.
Does add_header automatically reapply? I was under the impression explicit proxy_pass_header was needed to undo the hide.

On top of this header, an add_header Vary Origin; would be required to advertise (as required by the standard) that it is Origin-dependant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the cors.include should still be there since it adds other definitions
I think proxy_hide_header 'Access-Control-Allow-Origin'; will remove it.

I agree that we should include the rest of cors.include but proxy_hide_header wasn't working for me. I tried a few combinations of things but maybe I did it wrong. I gave up in the end but I can try again with fresh eyes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So the issue is that proxy_hide_header will hide a header that is set from the upstream server (ie. the stac container) but it won't unset the header that is explicitly set by nginx.

To get around this, I modified cors.include slightly to allow for different behaviour depending on whether a variable was set.

}

# Automatically redirect to /stac/stac and exclude redirect when already using /stac
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ services:
volumes:
- ./components/stac/service-config.json:/static/services/stac.json:ro
- ./components/stac/config/proxy/conf.extra-service.d:/etc/nginx/conf.extra-service.d/stac:ro
- ./components/stac/config/proxy/conf.extra-directives.d:/etc/nginx/conf.extra-directives.d/stac:ro
4 changes: 4 additions & 0 deletions birdhouse/components/stac/default.env
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export STAC_POPULATOR_BACKUP_IMAGE='${STAC_POPULATOR_BACKUP_DOCKER}:${STAC_POPUL
# This must match the "stac_version" value in the current STAC catalog.
export PYSTAC_STAC_VERSION_OVERRIDE=1.0.0

export _STAC_ADDITIONAL_CORS_ORIGINS='$(for origin in $STAC_ADDITIONAL_CORS_ORIGINS; do echo "$origin \$http_origin;"; done;)'
Copy link
Member

Choose a reason for hiding this comment

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

I am not 100% sure, but I think the first part ($origin) is some kind of regex to match a given origin. If using https://geojson.io, is it sufficient to match the full request that has other path/query/anchors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It can either be a regex (if it starts with ~) or just a string to match. Theoretically we could use either one.


# add any new variables not already in 'VARS' or 'OPTIONAL_VARS' that must be replaced in templates here
# single quotes are important in below list to keep variable names intact until 'birdhouse-compose' parses them
EXTRA_VARS='
Expand All @@ -71,6 +73,7 @@ export DELAYED_EVAL="
STAC_MIGRATION_DOCKER
STAC_MIGRATION_IMAGE
STAC_POPULATOR_BACKUP_IMAGE
_STAC_ADDITIONAL_CORS_ORIGINS
"

OPTIONAL_VARS="
Expand All @@ -86,4 +89,5 @@ OPTIONAL_VARS="
\$STAC_MIGRATION_TAGGED
\$STAC_MIGRATION_DOCKER
\$STAC_MIGRATION_IMAGE
\$_STAC_ADDITIONAL_CORS_ORIGINS
"
Loading