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

Support reverse proxies + CI integration test #43

Merged
merged 4 commits into from
Jun 6, 2024
Merged

Conversation

joecorall
Copy link
Contributor

@joecorall joecorall commented May 31, 2024

  1. First, adding a CI integration test to make sure we don't introduce any regressions: 53ece2b
  2. Then will see if we can tweak .env and docker-compose.yml to allow this docker compose deployment to be a backend to some other frontend domain that acts as a reverse proxy: 84c3883

Depends on Islandora-Devops/isle-buildkit#335 for new sites to be able to utilize this

@joecorall joecorall marked this pull request as ready for review May 31, 2024 22:18
@joecorall joecorall changed the title Support reverse proxies Support reverse proxies + CI integration test Jun 1, 2024
# if you're not behind a reverse proxy, you probably do not need to edit these IPs
# if you are behind a reverse proxy, most likely you can just replace FRONTEND_IP_1
# with the IP address used on your front end // reverse proxy domain
FRONTEND_IP_1=127.0.0.1/32
Copy link
Contributor

Choose a reason for hiding this comment

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

Shame I didn't see the changes for this go into build kit, we could have made it into a single variable that was delimited by comma. That way we could support any number of IP's.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're welcome to open a new PR to simplify. FWIW I did it this way because the values are used in two places. One comma delimited used in docker compose(traefik), the other is multiple directives in buildkit // nginx directive.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the bits on nginx we can iterate on a list using confd, let's not worry about it for now though. If someone wants to use more than 3 ip's in the future we can sort it then.

type: string
default: 'heads/main'
schedule:
- cron: '15 11 * * *'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why test with cron? Do we expect this to fail without changes to the code base?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah to catch issues with buildkit+starter that are otherwise never discovered

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought being: this is sort of the last mile before someone installs a site, so if some issue does arise between buildkit and starter site this feels like the best place to discover it since it's the place those two repos converge

@nigelgbanks nigelgbanks merged commit 64689c0 into main Jun 6, 2024
3 checks passed
@nigelgbanks nigelgbanks deleted the reverse-proxy branch June 6, 2024 12:44
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