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

FSPT-228: Override frontend hostnames on Dev #229

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

gidsg
Copy link
Contributor

@gidsg gidsg commented Feb 7, 2025

Change description

Override frontend hostnames on Dev to test domain migration

  • Unit tests and other appropriate tests added or updated
  • README and other documentation has been updated / added (if needed)
  • Commit messages are meaningful and follow good commit message guidelines (e.g. "FS-XXXX: Add margin to nav items preventing overlapping of logo")

How to test

Journeys should work as before if they start at:
https://apply.dev.access-funding.communities.gov.uk/ or https://assess.dev.access-funding.communities.gov.uk/

Screenshots of UI changes (if applicable)

Tiny49
Tiny49 previously approved these changes Feb 7, 2025
Copy link
Contributor

@Tiny49 Tiny49 left a comment

Choose a reason for hiding this comment

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

LGTM

@gidsg gidsg marked this pull request as draft February 10, 2025 16:13
@gidsg gidsg force-pushed the fspt-228-dev-config branch from 37bcb72 to a847217 Compare February 10, 2025 16:14
@gidsg gidsg marked this pull request as ready for review February 28, 2025 15:14
- path: /
target_container: nginx
alias:
- apply.access-funding.dev.communities.gov.uk
Copy link
Contributor

@sfount sfount Feb 28, 2025

Choose a reason for hiding this comment

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

Not 100% sure what this additional rules alias is doing but does this communities one need to be here? looks like the others are for aliasing the previous levellingup domains. This is also the only one that isn't relative to the ${COPILOT_ENVIRONMENT_NAME}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sfount Ah, yes, this is v tricksy, it's because we hit the limit of 5 conditions on an ALB listener rule, as we need 6 aliases here, so we need to use the additional_rules to work around this.

This is as per the guidance from the copilot CLI:

You can split the "alias" and "allowed_source_ips" field into separate rules, so that each rule contains up to 5 values: 

http:
  path: /
  alias: [frontend.dev.access-funding.test.levellingup.gov.uk,authenticator.dev.access-funding.test.levellingup.gov.uk,account.access-funding.dev.communities.gov.uk,assess.access-funding.dev.communities.gov.uk]
  allowed_source_ips: []
  additional_rules:
    - path: /
      alias: [apply.access-funding.dev.communities.gov.uk]
      allowed_source_ips: []
✘ validate manifest against environment "dev": validate "http": validate condition values per listener rule: listener rule has more than five conditions 

See also: https://aws.github.io/copilot-cli/docs/manifest/lb-web-service/#http-additional-rules

This is also the only one that isn't relative to the ${COPILOT_ENVIRONMENT_NAME}

Yes, good spot, I can change it to use that variable although it wouldn't make any difference yet as we're only doing it in dev so far.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also good spot @sfount the apply one there is a typo , will fix now!

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.

3 participants