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 new spaces for egress when creating a new org #181

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

Conversation

apburnes
Copy link
Contributor

@apburnes apburnes commented Nov 3, 2021

To support newly created orgs with our additional egress controls, all new orgs will have nine spaces based on three envs and each env having three egress types.

Naming suggestions for the egress types welcome!

Changes proposed in this pull request:

  • Add "public", "internal", and "private" to new org spaces to allow for different egress options
    • <env>-public: Apps can send requests to the public internet and our internal service like RDS
    • <env>-internal: Apps can only send requests to the internal services: RDS, ES, Redis
    • <env>-private: Apps are locked down

security considerations

Will improve egress control for tenant apps

@mheadd
Copy link
Contributor

mheadd commented Nov 4, 2021

When we say private, do we mean that an app can only communicate via c2c with another app in the same space (assuming the network policies are in place for this)?

Also, wondering if the use of internal might be confusing if conflated with internal routes. What if we used public, restricted, closed or something similar? Just a thought - happy to kick this around more.

@apburnes
Copy link
Contributor Author

apburnes commented Nov 4, 2021

@mheadd private does mean that an app can only communicate via c2c. I like public, restricted, and closed. Should we also append -egress to be more semantic? Or is that too much?

@mheadd
Copy link
Contributor

mheadd commented Nov 4, 2021

I think using -egress makes sense, @apburnes.

@apburnes
Copy link
Contributor Author

apburnes commented Nov 4, 2021

Updated to public-egress, restricted-egress, and closed-egress

mheadd
mheadd previously approved these changes Nov 4, 2021
Copy link
Contributor

@mheadd mheadd left a comment

Choose a reason for hiding this comment

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

🚀

@apburnes
Copy link
Contributor Author

apburnes commented Nov 4, 2021

I will put this into a holding pattern until we get production tests passing from cloud-gov/deploy-cf#580

@mheadd
Copy link
Contributor

mheadd commented Dec 15, 2021

Before merging this, one thing to consider that I have run into when testing. When new spaces are created they are automatically in the closed-egress ASG. This will prevent someone from pushing an app because of the need for the platform to fetch external dependencies - the staging process will fail. We should consider allowing the staging process to use the public-egress as the app is staged. We can do this by running:

~$ cf bind-security-group public_networks_egress ORGs --lifecycle staging --space SPACE

This way, the app will run in the closed-egress ASG and be staged in the public-egress ASG.

@davemcorwin
Copy link

We could also provide examples of vendoring deps so this wouldn't be necessary and I think this is the recommended approach.

@apburnes
Copy link
Contributor Author

@mheadd the egress rules are only applied at runtime and not during app staging. We left app staging egress open to allow users to install dependencies.

@mheadd
Copy link
Contributor

mheadd commented Dec 15, 2021

@apburnes Apologies for any confusion. When I initially tried that on a closed-egress space it didn't work, but I'm not able to replicate the issue. Disregard my prior comment.

@markdboyd markdboyd changed the base branch from master to main April 14, 2023 20:02
@markdboyd markdboyd dismissed mheadd’s stale review April 14, 2023 20:02

The base branch was changed.

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