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: TNL-11060 studio on its own API Gateway API #231

Merged
merged 3 commits into from
Sep 19, 2023
Merged

Conversation

bszabo
Copy link
Contributor

@bszabo bszabo commented Sep 14, 2023

Reference #218 for context.

Relative to that PR, this one sets up an API Gateway to have a dedicated API ID for the Studio content API, and abandons the prior strategy of adding the Studio content API to the omnibus API.

I've tested the ./scripts/aws/invocation_history/prod/0001_deploy_studio_on_new_domain_name.sh script intended for use by the SRE team on the playground account (with name substitutions as needed), and it creates a distinct "Studio content" API, as expected.

I've included OAuth2, heartbeat, and index functionality in the new API, as clients would not normally go to a separate domain name for these.

Documentation formerly added by this PR to the repo has been removed and is now in a confluence document: https://2u-internal.atlassian.net/wiki/spaces/TNL/pages/579502286/TNL-10899+API+Gateway#Deploy,-activate,-and-test-on-prod

The deploy script has been reworked to have the legacy file remain and apply to the legacy omnibus API. There is now a deploy_studio.py script for deploying the Studio content API. Stage variables for omnibus hosts have been removed from the studio version.

Copy link
Member

@jesperhodge jesperhodge left a comment

Choose a reason for hiding this comment

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

I had a few things that confuse me, see my comments. Probably makes sense to go through it together.

- go to AWS Certificate Manager (ACM) (off of top-level serices icon)
- Push orange "Request" button
- Push orange "Next" button with default "Request a public certificate" selected
- Enter the fully qualified domain name "studio.api.stage.edx.org"
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be deployed to prod, right? So I think the domain name needs to be without the stage.


- Select API Gateway "Custom domain names" menu link item
- click on "Create" button
- Enter `studio.api.stage.edx.org` as as the domain name to be created
Copy link
Member

Choose a reason for hiding this comment

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

stage or prod url?

- Should now see the new certificate in a "Pending validation" state
- In the "Domains" pane of this certificate display there is a "Create records in Route 53". You
must take this action for this certificate to exit "Pending validation" state
- Push the "Create records in Route 53" button
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with what "Route 53" means, does this need explanation or is it clear for SRE?

Copy link
Member

Choose a reason for hiding this comment

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

Is it the same route number on stage and prod?

- The newly created custom domain name currently has no APIs or deployments associated with it
- Select the API Gateway "APIs" menu item link
- Click the orange "Create API" button
- Go to the REST API panel and click the oranage "Build" button within it
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Go to the REST API panel and click the oranage "Build" button within it
- Go to the REST API panel and click the orange "Build" button within it

docs/SRE_request_studio_content_api_addition.md Outdated Show resolved Hide resolved
@@ -8,7 +8,7 @@

python3 ./scripts/aws/deploy.py \
--tag None \
--api-base-domain api.edx.org \
--api-base-domain studio.api.edx.org \
--swagger-filename ./swagger/api.yaml \
--landing-page https://stage.edx.org \
--rotation-order $1 \
Copy link
Member

Choose a reason for hiding this comment

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

Why is studio-host courses.stage.edx.org? I'm confused

Copy link
Member

Choose a reason for hiding this comment

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

Why do all these hosts have .stage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a cut-and-paste error. I'll fix.

@@ -9,7 +9,7 @@

python3 ./scripts/aws/deploy.py \
--tag None \
--api-base-domain api.stage.edx.org \
--api-base-domain studio.api.stage.edx.org \
--swagger-filename ./swagger/api.yaml \
--landing-page https://stage.edx.org \
--rotation-order $1 \
Copy link
Member

Choose a reason for hiding this comment

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

studio-host points to courses.stage.edx.org?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The api-base-domain argument selects which API Gateway object API object to use with this request. For working with Studio, the value provided here is correct. See answer to next comment for additional considerations.

@@ -8,7 +8,7 @@

python3 ./scripts/aws/deploy.py \
--tag None \
--api-base-domain api.edx.org \
--api-base-domain studio.api.edx.org \
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's good to change the url here. This repo will then only be usable for the studio api, unless you change the code again, and also maybe someone mistakenly deploys without correcting the url.

Or is this file only for studio api deployment?

It could be better to have this url as an argument - you can specify it in the invocation_history/ scripts - or something like that.

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 right that something is afoul here, but it's even more than what you've called out. Having abandoned use of the omnibus API for studio, we no longer want most of those stage variables set when working with the Studio API. I'll rename the script to make it clear that it's only to be used with the Studio API, and clean up the stage variables.

@bszabo bszabo merged commit 2baa814 into master Sep 19, 2023
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