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

Consolidate domain configs #157

Closed
wants to merge 5 commits into from
Closed

Consolidate domain configs #157

wants to merge 5 commits into from

Conversation

lorenyu
Copy link
Collaborator

@lorenyu lorenyu commented Jan 17, 2025

Ticket

n/a

Changes

  • Consolidate hosted_zone, domain_name, and enable_https to a domain_config object separate from the service_config object
  • Remove unneeded variable domain_name from identity_provider/resources

Context for reviewers

While working on another PR I realized the domain configs are intermingled with service configs. This PR cleans that up. Also removed a variable that we don't need since we can just use the configuration set that's associated with the domain identity.

Testing

Running make infra-update-app-service APP_NAME=app ENVIRONMENT=dev shows no changes
image

Also tested notifications to make sure it still works
image

Preview environment

♻️ Environment destroyed ♻️

@lorenyu lorenyu requested a review from coilysiren January 17, 2025 20:56
@lorenyu
Copy link
Collaborator Author

lorenyu commented Jan 17, 2025

Done in navapbc/template-infra#839

@lorenyu lorenyu closed this Jan 17, 2025
@lorenyu lorenyu deleted the lorenyu/domainconfig branch January 17, 2025 22:02
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.

1 participant