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

Add OpenAPI spec for dedicated egress IPs #875

Merged

Conversation

ZachEddy
Copy link
Member

This updates the OpenAPI spec for App Platform to include dedicated egress IPs.

Screenshot 2024-05-13 at 10 04 42 AM
Screenshot 2024-05-13 at 10 08 35 AM

@danaelhe
Copy link
Member

danaelhe commented May 13, 2024

Looks good! Looks like there's some linting errors that need to be addressed, you can view locally using make lint

Quick question- it appears the API is different from the UI. In the UI it appears you enable the dedicated egress IPs feature and then two IPs automatically get assigned and enabled. In the API, it appears the user has to input the specific IPs to be used? Is that correct?

@ZachEddy ZachEddy force-pushed the apps/dedicated-egress-openapi-spec branch from 1d36ab9 to aa83d19 Compare May 13, 2024 17:25
@ZachEddy
Copy link
Member Author

ZachEddy commented May 13, 2024

@danaelhe Fixed those linting errors. Thanks for the tip about running make lint locally.

In the API, it appears the user has to input the specific IPs to be used? Is that correct?

Not quite. The public API and UI behavior is the same. The user has to set egress.type to DEDICATED_IP in their app spec to enable the feature via the UI. The same is true of the public API. Those 192.168.1.x IP addresses are included in the HTTP response part of the OpenAPI spec, not the request payload.

Having said that, your comment made me realize my PR was incomplete. I didn't include any OpenAPI spec changes that document how users can enable the feature. I added a change in 4b2fc82 that shows how users can create an app with Dedicated Egress enabled (or update an existing app to enable the feature).

Copy link
Member

@danaelhe danaelhe left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification! Looks great 🚀

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