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

feat: Improve API key management #57

Merged
merged 5 commits into from
Mar 8, 2024

Conversation

Altonhe
Copy link
Member

@Altonhe Altonhe commented Mar 1, 2024

In response to issue #10, I've modified our approach to API key management to ensure that test keys are only used during the CI test process. This change prevents test keys from being mistakenly used in production. I've configured the test API key to work with the validate.sh script, to test the service's endpoints during the CI process.

@Altonhe Altonhe marked this pull request as ready for review March 1, 2024 04:03
Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

I think this is a smart and sensible improvement. I appreciate that you're injecting it in for the CI process. One question/concern: what happens when I use the validate.sh script while testing changes to the Dockerfile? It won't work anymore right? How do we solve for this? I've actually been making regular use of it as I test out new features. I guess we could re-add it during development, but that feels slightly janky to me.

What if the test key was commented out in the data-sources.xml file, and had a comment included above it that said something to the effect of "Uncomment this to use it during development but be sure to remove it before committing!" And then there was a CI check to make sure we didn't accidentally commit it?

@aaronbrethorst
Copy link
Member

(Also, apologies for taking so long to review this. the last few days have been a bit hectic for me)

@Altonhe
Copy link
Member Author

Altonhe commented Mar 5, 2024

I made some adjustments based on our discussion.

  • The TEST_API_KEY will now be automatically managed by the bootstrap.sh script.
  • By default, The docker-compose file will include the test key. However, this can be customized for production environments.
    Thank you for your feedback.

Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

looks great 👏 thanks for keeping at this!

@aaronbrethorst aaronbrethorst merged commit fd5f6d5 into OneBusAway:main Mar 8, 2024
3 checks passed
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