Propose updates to ExternalEntity CRD named port usage

[abhiraut]

Lets discuss the proposal based on discussion #1084 (comment) on this issue.

In a nutshell, it is proposed that we update the ExternalEntity CRD spec to remove named ports per interface and instead associate named ports with the entity itself. This would require a change in the GroupMember struct used in internal communications.
Following are the proposed changes:

ExternalEntity CRD

type ExternalEntitySpec struct {
        Endpoints []Endpoint
+       Ports []NamedPort
        ExternalNode string
}
type Endpoint struct {
        IP string
        Name string
-       Ports []NamedPort
}

Change proposed to the GroupMember struct:

type GroupMember struct {
	Pod *PodReference
	ExternalEntity *ExternalEntityReference
	IPs []IPAddress
	Ports []NamedPort
}

[abhiraut]

/cc @jianjuns @tnqn @suwang48404 @Dyanngg

[Dyanngg]

Thanks @abhiraut for creating the issue. CC @wenyingd as the proposed GroupMember struct change affects dual stack pod processing.

[Dyanngg]

@suwang48404 Are we good with the proposed change in EE crd? i.e.

--- a/build/yamls/base/crds.yml
+++ b/build/yamls/base/crds.yml
@@ -547,16 +547,16 @@ spec:
                     format: ipv4
                   name:
                     type: string
-                  ports:
-                    type: array
-                    items:
-                      type: object
-                      properties:
-                        protocol:
-                          type: string
-                        port:
-                          x-kubernetes-int-or-string: true
-                        name:
-                          type: string
+            ports:
+              type: array
+              items:
+                type: object
+                properties:
+                  protocol:
+                    type: string
+                  port:
+                    x-kubernetes-int-or-string: true
+                  name:
+                    type: string
             externalNode:
               type: string

[suwang48404]

I am sorry, what is difference before and after this change? can u summarize, yaml is bit harder to read.

If we are going to change EE format, it will have impact on projects already depends on its older format. So versioning may be a good way to go on this?

Thx, Su

[Dyanngg]

As summarized in the issue base, the proposed change is to move the ports array from being a property of endpoint, to a property that belongs to the base spec of externalEntity. This way we do not duplicate this information for each endpoint. @abhiraut to comment on versioning

[jianjuns]

type GroupMember struct {
	Pod *PodReference
	ExternalEntity *ExternalEntityReference
	IPs []IPAddress
	Ports []NamedPort
}

Questions

1. IPs are used in AddressGroup only right?
2. For AppliedToGroup, do we have the requirements to apply to part of interfaces/endpoints in the VM?

[abhiraut]

yes for 1. i was thinking we will do what we have been doing with GroupMemberPod conversions, i.e. includeIP only if generating GroupMember for AddressGroups

2. @suwang48404 could answer that one

[jianjuns]

Thought over. Anyway we might extend GroupMember to have an Endpoints field then. So, the current definition looks good.

[github-actions]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment, or this will be closed in 180 days

[Dyanngg]

fixed by #1335