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

security_groups/dynamic_asgs.go fails due hardcoded ASG private network ranges #1151

Open
renelehmann opened this issue May 31, 2024 · 6 comments

Comments

@renelehmann
Copy link

Issue
While executing the test for enabled dynamic ASG it creates an ASG with fixed private network ranges and checks the connection to cc via https://cloud-controller-ng.service.cf.internal:9024/v2/info.
It does not cover foundations using other IP ranges than these hardcoded private network ranges.

Context
cats version: 16.2.0, 16.3.0, latest 16.4.0
With commit 7f50d0b
the ASG was redefined and the destination 10.0.0.0/0 (which covered our used IPs for cc) has been replaced with 10.0.0./8.

security_groups/dynamic_asgs.go (ASG covers private network ranges only):
https://github.com/cloudfoundry/cf-acceptance-tests/blob/v16.4.0/security_groups/dynamic_asgs.go#L153-L166

Possible Fix
Please revert this ASG definition to the destination 10.0.0.0/0 like it was before or even more open with 0.0.0.0 without any CIDR.
But a better approach would be either:

  1. get the used IPs of all the cc endpoints and define this specific IP destinations on the ASG (e.g. with net.LookupIP and loop trough the range).

or

  1. Introducing a cats-config.json property to define or overwrite the ASG destination range.
@ctlong
Copy link
Member

ctlong commented May 31, 2024

@geofffranks can you please take a look at this issue when you get a chance, thanks.

@geofffranks
Copy link
Contributor

10.0.0.0/0 is equivalent to 0.0.0.0/0. Both allow egress to any IPv4 addr on the internet. There are situations where using 10.0.0.0/0 is an invalid cidr when given to the CNI, which is what triggered this change.

Are you running these tests as part of developing CF, or testing an installation? Generally these tests are not good for exercising the health of a cloudfoundry environment. They're very long and in-depth, to validate code changes to CF before CF components + cf-deployment are released. The preferred method for validating deployments is to run the smoke-tests errand.

@renelehmann
Copy link
Author

Hi @geofffranks , I understood the trigger for the change. The 0.0.0.0 without cidr was only an idea.
The best approach would be really to lookup the used IPs for cc and then define the ASG for this test accordingly.
As further input we use e.g. 100.x.x.x on our management components. Of course this is then nated from external.

We run the CATS test only on development/integration environments and to test against the packaged cloudfoundry component releases before it will be used as a platform release package in production. Beside that we run further test frameworks (rspecs), also on production environments to ensure all works fine and like expected after an update.

@geofffranks
Copy link
Contributor

Oh - are you rolling your own cloudfoundry deployment outside of cf-deployment? Feel free to PR a change to update those ASG blocks to use a variable for a list of network ranges to open on the ASG.

@renelehmann
Copy link
Author

@geofffranks Don't get me wrong. We use and align to cf-deployment as much and as close as possible. But to have the confidence that all combinations of components and also the specific configurations works fine together the CATS run supports a lot in this manner.

I just saw the comma_delmited_asgs.go would need the same list also.

At the moment I feel uncomfortable to PR the needed changes for the approach with the config input variables and to respect the conventions.
Do you see a chance to enhance/add the code with this request?
Otherwise I will try, but it probably takes me some time since I don't use to code on a daily base ;-)

@geofffranks
Copy link
Contributor

Honestly it wouldn't be a high priority for our engineering team at the moment, but it might be something that we'd get to eventually. If you're looking for a speedy fix, a PR is probably the way to go.

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

No branches or pull requests

3 participants