model service network #896

olasaadi99 · 2024-09-30T15:00:12Z

No description provided.

Signed-off-by: Ola Saadi <[email protected]>

olasaadi99 · 2024-10-02T08:34:22Z

you can review before fixing test
service network added

zivnevo

Need dedicated tests:
1. SGs/NACLs block connectivity to all/part-of service network
2. Connectivity to service network is allowed, but not to public internet (and vice versa)
Need to generate a Drawio SquareTreeNode for the service network, like we have for public internet (consult with @haim-kermany )

pkg/ibmvpc/examples/out/analysis_out/actual.txt

pkg/ibmvpc/examples/out/analysis_out/expected.txt

pkg/vpcmodel/externalNetwork.go

pkg/ibmvpc/implicit_routing.go

pkg/ibmvpc/vpc.go

zivnevo · 2024-11-11T10:43:51Z

I ran ./bin/vpcanalyzer report endpoints -c pkg/ibmvpc/examples/input/input_s ervice_network_test.json -o html -f test.html and got the below image.
* Is it correct to have the service network under "Public Internet"?

* Some "Various IP Ranges" nodes are actually service network. It seems like there is some mix there.

* Also, what's "NS" on the icon?
NS stands for network services... I understood it should under the public internet, should I move them?

@haim-kermany
Service network icons should appear under IBM Cloud - you can have them floating outside all regions, or put them into a region of their own, called Service Network.
NS should be SN (Service Network).

Signed-off-by: Ziv Nevo <[email protected]>

pkg/vpcmodel/externalNetwork.go

pkg/ibmvpc/parser.go

haim-kermany · 2024-11-11T15:47:08Z

I ran ./bin/vpcanalyzer report endpoints -c pkg/ibmvpc/examples/input/input_s ervice_network_test.json -o html -f test.html and got the below image.
* Is it correct to have the service network under "Public Internet"?

* Some "Various IP Ranges" nodes are actually service network. It seems like there is some mix there.

* Also, what's "NS" on the icon?
NS stands for network services... I understood it should under the public internet, should I move them?
@haim-kermany Service network icons should appear under IBM Cloud - you can have them floating outside all regions, or put them into a region of their own, called Service Network. NS should be SN (Service Network).

its not trivial, can we consider doing it in a different PR?

pkg/ibmvpc/vpc.go

zivnevo · 2024-11-11T15:31:07Z

pkg/vpcmodel/subnetsConnectivity.go

@@ -241,7 +241,7 @@ func updateSubnetsConnectivityByTransitGateway(src, dst VPCResourceIntf,
 	c *VPCConfig) (
 	*netset.TransportSet, error) {
 	// assuming a single router representing the tgw for a "MultipleVPCsConfig"
-	if len(c.RoutingResources) != 1 {
+	if len(c.RoutingResources) != 2 { // expecting tgw and sgw (virtual gateway)


Will this hold for AWS as well?

aws does not get here

@adisos , I need your help here. Is the change to the check valid?

I would change this check.. and also the the next line in which it assumes that tgw is at index 0 of this slice. Instead, I think it is better to check here that c.RoutingResources contains exactly one router of kind TGW for MultipleVPCsConfig.

zivnevo · 2024-11-11T15:31:14Z

pkg/vpcmodel/textOutput.go

@@ -15,7 +15,8 @@ type TextOutputFormatter struct {
 }

 func multipleVPCsConfigHeader(c *VPCConfig) (string, error) {
-	if len(c.RoutingResources) != 1 {
+	if len(c.RoutingResources) != 2 { // expecting tgw and sgw (virtual gateway)


aws does not support multi-vpc

We may want to support multi-vpc in AWS in the future.
@adisos , any insight here?

I would change this check here as well, to check here that c.RoutingResources contains exactly one router of kind TGW for MultipleVPCsConfig.
If we support multiple VPCs for AWS as well, this is a more consistent check, and does not depend on whether or not service network gateway is present.

pkg/vpcmodel/explainabilityInput.go

pkg/vpcmodel/explainabilityPrint.go

pkg/vpcmodel/externalNetwork.go

zivnevo · 2024-11-11T15:56:20Z

I ran ./bin/vpcanalyzer report endpoints -c pkg/ibmvpc/examples/input/input_s ervice_network_test.json -o html -f test.html and got the below image.
* Is it correct to have the service network under "Public Internet"?

* Some "Various IP Ranges" nodes are actually service network. It seems like there is some mix there.

* Also, what's "NS" on the icon?
NS stands for network services... I understood it should under the public internet, should I move them?
@haim-kermany Service network icons should appear under IBM Cloud - you can have them floating outside all regions, or put them into a region of their own, called Service Network. NS should be SN (Service Network).
its not trivial, can we consider doing it in a different PR?

Let's just replace NS with SN in this PR. Please open an issue for changing the layout

haim-kermany · 2024-11-11T16:19:21Z

@zivnevo pushed a solution that looks like that:

seems OK if the number of SN are not too big.

pkg/ibmvpc/vpc.go

zivnevo

A few more small things

pkg/vpcmodel/externalNetwork.go

zivnevo

Last two comments

zivnevo · 2024-11-21T10:59:10Z

pkg/vpcmodel/textOutput.go

+	var tgw RoutingResource
+	tgwRouterFound := false
+	for _, router := range c.RoutingResources {
+		if router.Kind() == resourceTypeTGW {
+			if tgwRouterFound {
+				return "", errors.New("unexpected number of RoutingResources for MultipleVPCsConfig, expecting only one TGW")
+			}
+			tgw = router
+			tgwRouterFound = true
+		}
+	}
+	if !tgwRouterFound {
+		return "", errors.New("unexpected number of RoutingResources for MultipleVPCsConfig, expecting TGW")


Since this code is duplicated in pkg/vpcmodel/subnetsConnectivity.go, I think it deserves a function. Can be a method of VPCConfig

zivnevo · 2024-11-21T11:01:02Z

pkg/vpcmodel/textOutput.go

+	var tgw RoutingResource
+	tgwRouterFound := false


No need for the Boolean variable. You can simple check if tgw != nil

olasaadi99 added 2 commits September 29, 2024 14:26

subtract 161.26.0.0/16 and 166.8.0.0/14 from PublicInternetAddressList

31840ff

Signed-off-by: Ola Saadi <[email protected]>

model service network

36dc49d

pull-request-size bot added the size/XXL label Sep 30, 2024

olasaadi99 marked this pull request as draft September 30, 2024 15:00

olasaadi99 linked an issue Sep 30, 2024 that may be closed by this pull request

model service network #401

Closed

2 tasks

olasaadi99 requested review from adisos and zivnevo October 1, 2024 07:14

Merge branch 'main' into 401_service_network

9255b7f

olasaadi99 marked this pull request as ready for review October 6, 2024 09:48

zivnevo requested changes Oct 7, 2024

View reviewed changes

remove files

63aff7a

pull-request-size bot added size/L and removed size/XXL labels Oct 7, 2024

zivnevo requested changes Oct 7, 2024

View reviewed changes

pkg/ibmvpc/vpc.go Show resolved Hide resolved

olasaadi99 added 6 commits October 7, 2024 13:25

fix sgwList, use one pointer

4bf91aa

add doc

19e890b

fiX

1094103

fix

f12feb3

fix

3a182eb

fix

d6ab6ca

olasaadi99 marked this pull request as draft October 14, 2024 11:21

olasaadi99 and others added 4 commits October 14, 2024 14:24

remove

d7db215

Merge branch 'main' into 401_service_network

69a9635

add singleton for network address lists

f344e77

fix ibm analysis tests

b688a8d

pull-request-size bot added size/XXL and removed size/L labels Oct 21, 2024

olasaadi99 added 2 commits October 21, 2024 15:35

fix

ce1ef1e

fix diff

aaf2f5a

olasaadi99 added 3 commits November 7, 2024 18:28

Interpreted destination(s): field

e9d38a4

fix serviceGateway name

1098061

fix Interpreted destination(s)

b36bef3

fix lint

ae72bec

Signed-off-by: Ziv Nevo <[email protected]>

zivnevo requested changes Nov 11, 2024

View reviewed changes

pkg/vpcmodel/externalNetwork.go Outdated Show resolved Hide resolved

zivnevo requested changes Nov 11, 2024

View reviewed changes

pkg/ibmvpc/parser.go Outdated Show resolved Hide resolved

pkg/ibmvpc/parser.go Outdated Show resolved Hide resolved

zivnevo requested changes Nov 11, 2024

View reviewed changes

haim-kermany added 2 commits November 11, 2024 17:57

NS to SN

503c14f

SN layout

3c59d4b

fix sn for groupedExternalNodes

8293a45

zivnevo requested changes Nov 12, 2024

View reviewed changes

pkg/ibmvpc/vpc.go Outdated Show resolved Hide resolved

olasaadi99 added 2 commits November 18, 2024 12:27

fixes

34f75a8

fix

04acb74

olasaadi99 requested a review from zivnevo November 18, 2024 10:39

zivnevo requested changes Nov 20, 2024

View reviewed changes

olasaadi99 added 3 commits November 20, 2024 16:56

fix

4ddb680

fix

8473966

fix

7a71b12

zivnevo requested changes Nov 21, 2024

View reviewed changes

olasaadi99 added 4 commits November 21, 2024 13:28

fix

48f04d1

fix

f64c5de

fix

51ad779

fix

4dac4c6

zivnevo approved these changes Nov 21, 2024

View reviewed changes

olasaadi99 merged commit 20cec14 into main Nov 21, 2024
4 checks passed

olasaadi99 deleted the 401_service_network branch November 21, 2024 11:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

model service network #896

model service network #896

olasaadi99 commented Sep 30, 2024

olasaadi99 commented Oct 2, 2024

zivnevo left a comment •

edited

Loading

zivnevo commented Nov 11, 2024

haim-kermany commented Nov 11, 2024

zivnevo Nov 11, 2024

olasaadi99 Nov 18, 2024

zivnevo Nov 20, 2024

adisos Nov 21, 2024

olasaadi99 Nov 21, 2024

zivnevo Nov 11, 2024

olasaadi99 Nov 18, 2024

zivnevo Nov 20, 2024

adisos Nov 21, 2024

olasaadi99 Nov 21, 2024

zivnevo commented Nov 11, 2024

haim-kermany commented Nov 11, 2024

zivnevo left a comment

zivnevo left a comment

zivnevo Nov 21, 2024

zivnevo Nov 21, 2024

model service network #896

model service network #896

Conversation

olasaadi99 commented Sep 30, 2024

olasaadi99 commented Oct 2, 2024

zivnevo left a comment • edited Loading

Choose a reason for hiding this comment

zivnevo commented Nov 11, 2024

haim-kermany commented Nov 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zivnevo commented Nov 11, 2024

haim-kermany commented Nov 11, 2024

zivnevo left a comment

Choose a reason for hiding this comment

zivnevo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zivnevo left a comment •

edited

Loading