From 8a2783a33c84283498894dd867da30cf20e932fd Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 18 Dec 2024 17:25:42 -0800 Subject: [PATCH 01/48] updated CNS for adding default deny acl's --- cns/NetworkContainerContract.go | 3 ++ cns/middlewares/k8sSwiftV2.go | 37 +++++++++++--- cns/middlewares/k8sSwiftV2_linux.go | 4 ++ cns/middlewares/k8sSwiftV2_linux_test.go | 14 +++--- cns/middlewares/k8sSwiftV2_windows.go | 64 ++++++++++++++++++++++++ 5 files changed, 107 insertions(+), 15 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 394f871f09..18829c3f8b 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -7,6 +7,7 @@ import ( "strconv" "strings" + "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" @@ -503,6 +504,8 @@ type PodIpInfo struct { Routes []Route // PnpId is set for backend interfaces, Pnp Id identifies VF. Plug and play id(pnp) is also called as PCI ID PnPID string + // Defauly Deny ACL's to configure on HNS endpoints for Swiftv2 window nodes + DefaultDenyACL []cni.KVPair } type HostIPInfo struct { diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index a11290c205..2cb9937d88 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -40,7 +40,9 @@ var _ cns.IPConfigsHandlerMiddleware = (*K8sSWIFTv2Middleware)(nil) // and release IP configs handlers. func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, respCode, message := k.validateIPConfigsRequest(ctx, &req) + podInfo, respCode, message, defaultDenyACLbool := k.validateIPConfigsRequest(ctx, &req) + + logger.Printf("defaultDenyACLbool value is: %v", defaultDenyACLbool) if respCode != types.Success { return &cns.IPConfigsResponse{ @@ -55,6 +57,19 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa if !req.SecondaryInterfacesExist { return ipConfigsResp, err } + + // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny acl's pn the infra IP configs + for i := range ipConfigsResp.PodIPInfo { + ipInfo := &ipConfigsResp.PodIPInfo[i] + // there will be no pod connectivity to and from those pods + if defaultDenyACLbool { + err = addDefaultDenyACL(ipInfo) + if err != nil { + logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) + } + } + } + // If the pod is v2, get the infra IP configs from the handler first and then add the SWIFTv2 IP config defer func() { // Release the default IP config if there is an error @@ -102,19 +117,21 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa // validateIPConfigsRequest validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. // nolint -func (k *K8sSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string) { +func (k *K8sSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string, defaultDenyACL bool) { + defaultDenyACLbool := false + // Retrieve the pod from the cluster podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) if err != nil { errBuf := errors.Wrapf(err, "failed to unmarshalling pod info from ipconfigs request %+v", req) - return nil, types.UnexpectedError, errBuf.Error() + return nil, types.UnexpectedError, errBuf.Error(), defaultDenyACLbool } logger.Printf("[SWIFTv2Middleware] validate ipconfigs request for pod %s", podInfo.Name()) podNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()} pod := v1.Pod{} if err := k.Cli.Get(ctx, podNamespacedName, &pod); err != nil { errBuf := errors.Wrapf(err, "failed to get pod %+v", podNamespacedName) - return nil, types.UnexpectedError, errBuf.Error() + return nil, types.UnexpectedError, errBuf.Error(), defaultDenyACLbool } // check the pod labels for Swift V2, set the request's SecondaryInterfaceSet flag to true and check if its MTPNC CRD is ready @@ -126,12 +143,16 @@ func (k *K8sSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req mtpnc := v1alpha1.MultitenantPodNetworkConfig{} mtpncNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()} if err := k.Cli.Get(ctx, mtpncNamespacedName, &mtpnc); err != nil { - return nil, types.UnexpectedError, fmt.Errorf("failed to get pod's mtpnc from cache : %w", err).Error() + return nil, types.UnexpectedError, fmt.Errorf("failed to get pod's mtpnc from cache : %w", err).Error(), defaultDenyACLbool } // Check if the MTPNC CRD is ready. If one of the fields is empty, return error if !mtpnc.IsReady() { - return nil, types.UnexpectedError, errMTPNCNotReady.Error() + return nil, types.UnexpectedError, errMTPNCNotReady.Error(), defaultDenyACLbool } + + // copying defaultDenyACL bool from mtpnc + defaultDenyACLbool = mtpnc.Status.DefaultDenyACL + // If primary Ip is set in status field, it indicates the presence of secondary interfaces if mtpnc.Status.PrimaryIP != "" { req.SecondaryInterfacesExist = true @@ -140,7 +161,7 @@ func (k *K8sSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req for _, interfaceInfo := range interfaceInfos { if interfaceInfo.DeviceType == v1alpha1.DeviceTypeInfiniBandNIC { if interfaceInfo.MacAddress == "" || interfaceInfo.NCID == "" { - return nil, types.UnexpectedError, errMTPNCNotReady.Error() + return nil, types.UnexpectedError, errMTPNCNotReady.Error(), defaultDenyACLbool } req.BackendInterfaceExist = true req.BackendInterfaceMacAddresses = append(req.BackendInterfaceMacAddresses, interfaceInfo.MacAddress) @@ -154,7 +175,7 @@ func (k *K8sSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist) logger.Printf("[SWIFTv2Middleware] pod %s has backend interface : %v", podInfo.Name(), req.BackendInterfaceExist) // retrieve podinfo from orchestrator context - return podInfo, types.Success, "" + return podInfo, types.Success, "", defaultDenyACLbool } // getIPConfig returns the pod's SWIFT V2 IP configuration. diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index e9a93de0e2..99b8ae7846 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -103,3 +103,7 @@ func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(_ *cns.PodIpInfo, } func (k *K8sSWIFTv2Middleware) addDefaultRoute(*cns.PodIpInfo, string) {} + +func addDefaultDenyACL(_ *cns.PodIpInfo) error { + return nil +} diff --git a/cns/middlewares/k8sSwiftV2_linux_test.go b/cns/middlewares/k8sSwiftV2_linux_test.go index 76be6b2149..8d42f8cfe1 100644 --- a/cns/middlewares/k8sSwiftV2_linux_test.go +++ b/cns/middlewares/k8sSwiftV2_linux_test.go @@ -144,7 +144,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq.OrchestratorContext = b happyReq.SecondaryInterfacesExist = false - _, respCode, err := middleware.validateIPConfigsRequest(context.TODO(), happyReq) + _, respCode, err, _ := middleware.validateIPConfigsRequest(context.TODO(), happyReq) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq.SecondaryInterfacesExist, true) @@ -158,7 +158,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq2.OrchestratorContext = b happyReq2.SecondaryInterfacesExist = false - _, respCode, err = middleware.validateIPConfigsRequest(context.TODO(), happyReq2) + _, respCode, err, _ = middleware.validateIPConfigsRequest(context.TODO(), happyReq2) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq.SecondaryInterfacesExist, true) @@ -172,7 +172,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq3.OrchestratorContext = b happyReq3.SecondaryInterfacesExist = false - _, respCode, err = middleware.validateIPConfigsRequest(context.TODO(), happyReq3) + _, respCode, err, _ = middleware.validateIPConfigsRequest(context.TODO(), happyReq3) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq3.SecondaryInterfacesExist, false) @@ -188,7 +188,7 @@ func TestValidateMultitenantIPConfigsRequestFailure(t *testing.T) { InfraContainerID: testPod1Info.InfraContainerID(), } failReq.OrchestratorContext = []byte("invalid") - _, respCode, _ := middleware.validateIPConfigsRequest(context.TODO(), failReq) + _, respCode, _, _ := middleware.validateIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) // Pod doesn't exist in cache test @@ -198,19 +198,19 @@ func TestValidateMultitenantIPConfigsRequestFailure(t *testing.T) { } b, _ := testPod2Info.OrchestratorContext() failReq.OrchestratorContext = b - _, respCode, _ = middleware.validateIPConfigsRequest(context.TODO(), failReq) + _, respCode, _, _ = middleware.validateIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) // Failed to get MTPNC b, _ = testPod3Info.OrchestratorContext() failReq.OrchestratorContext = b - _, respCode, _ = middleware.validateIPConfigsRequest(context.TODO(), failReq) + _, respCode, _, _ = middleware.validateIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) // MTPNC not ready b, _ = testPod4Info.OrchestratorContext() failReq.OrchestratorContext = b - _, respCode, _ = middleware.validateIPConfigsRequest(context.TODO(), failReq) + _, respCode, _, _ = middleware.validateIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) } diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 2be2fbd1df..64c9bd02c9 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -1,9 +1,14 @@ package middlewares import ( + "encoding/json" + + "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/middlewares/utils" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" + "github.com/Microsoft/hcsshim/hcn" "github.com/pkg/errors" ) @@ -58,3 +63,62 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st } podIPInfo.Routes = append(podIPInfo.Routes, route) } + +// append the default deny acl's to the list defaultDenyACL field in podIpInfo +func addDefaultDenyACL(podIpInfo *cns.PodIpInfo) error { + blockEgressACL, err := getDefaultDenyACLPolicy(hcn.DirectionTypeOut) + if err != nil { + return errors.Wrap(err, "Failed to create default deny ACL policy egress") + } + + blockIngressACL, err := getDefaultDenyACLPolicy(hcn.DirectionTypeIn) + if err != nil { + return errors.Wrap(err, "Failed to create default deny ACL policy ingress") + } + + additionalArgs := []cni.KVPair{ + { + Name: "EndpointPolicy", + Value: blockEgressACL, + }, + { + Name: "EndpointPolicy", + Value: blockIngressACL, + }, + } + + podIpInfo.DefaultDenyACL = append(podIpInfo.DefaultDenyACL, additionalArgs...) + + logger.Printf("The length of podIpInfo.DefaultDenyACL is: %v", len(podIpInfo.DefaultDenyACL)) + + return nil +} + +// create the default deny acl's that need to be added to the list defaultDenyACL field in podIpInfo +func getDefaultDenyACLPolicy(direction hcn.DirectionType) ([]byte, error) { + const DefaultDenyPriority = 10000 + const policyType = "ACL" + type DefaultDenyACL struct { + Type string `json:"Type"` + Action hcn.ActionType `json:"Action"` + Direction hcn.DirectionType `json:"Direction"` + Priority int `json:"Priority"` + } + + denyACL := DefaultDenyACL{ + Type: policyType, + Action: hcn.ActionTypeBlock, + Direction: direction, + Priority: DefaultDenyPriority, + } + + denyACLJSON, err := json.Marshal(denyACL) + + logger.Printf("ACL Created for direction %s is : %s", direction, denyACLJSON) + + if err != nil { + return nil, errors.Wrap(err, "error marshalling default deny policy to json") + } + + return denyACLJSON, nil +} From 0ad9230acc964040136b8e0061f2e1dc3bf9a653 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 20 Dec 2024 01:02:08 -0800 Subject: [PATCH 02/48] added infra nic change --- cns/middlewares/k8sSwiftV2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 2cb9937d88..881c6f8ec5 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -62,7 +62,7 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa for i := range ipConfigsResp.PodIPInfo { ipInfo := &ipConfigsResp.PodIPInfo[i] // there will be no pod connectivity to and from those pods - if defaultDenyACLbool { + if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { err = addDefaultDenyACL(ipInfo) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) From b4534d4dd9e26186dd2ae1acb0f03d25235e0b90 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 20 Dec 2024 16:45:08 -0800 Subject: [PATCH 03/48] added unit tests --- cns/middlewares/k8sSwiftV2_windows.go | 5 -- cns/middlewares/k8sSwiftV2_windows_test.go | 73 ++++++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 64c9bd02c9..545b1ffd8f 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -5,7 +5,6 @@ import ( "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/middlewares/utils" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" "github.com/Microsoft/hcsshim/hcn" @@ -89,8 +88,6 @@ func addDefaultDenyACL(podIpInfo *cns.PodIpInfo) error { podIpInfo.DefaultDenyACL = append(podIpInfo.DefaultDenyACL, additionalArgs...) - logger.Printf("The length of podIpInfo.DefaultDenyACL is: %v", len(podIpInfo.DefaultDenyACL)) - return nil } @@ -114,8 +111,6 @@ func getDefaultDenyACLPolicy(direction hcn.DirectionType) ([]byte, error) { denyACLJSON, err := json.Marshal(denyACL) - logger.Printf("ACL Created for direction %s is : %s", direction, denyACLJSON) - if err != nil { return nil, errors.Wrap(err, "error marshalling default deny policy to json") } diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index dab24685f9..2eb9ccd61f 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -1,12 +1,15 @@ package middlewares import ( + "encoding/json" "reflect" "testing" + "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/middlewares/mock" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" + "github.com/stretchr/testify/require" "gotest.tools/v3/assert" ) @@ -100,3 +103,73 @@ func TestAddDefaultRoute(t *testing.T) { t.Errorf("got '%+v', expected '%+v'", ipInfo.Routes, expectedRoutes) } } + +func TestAddDefaultDenyACL(t *testing.T) { + valueOut := []byte(`{ + "Type": "ACL", + "Action": "Block", + "Direction": "Out", + "Priority": 10000 + }`) + + valueIn := []byte(`{ + "Type": "ACL", + "Action": "Block", + "Direction": "In", + "Priority": 10000 + }`) + + expectedDefaultDenyACL := []cni.KVPair{ + { + Name: "EndpointPolicy", + Value: valueOut, + }, + { + Name: "EndpointPolicy", + Value: valueIn, + }, + } + + podIPInfo := cns.PodIpInfo{ + PodIPConfig: cns.IPSubnet{ + IPAddress: "20.240.1.242", + PrefixLength: 32, + }, + NICType: cns.DelegatedVMNIC, + MacAddress: "12:34:56:78:9a:bc", + } + + err := addDefaultDenyACL(&podIPInfo) + assert.Equal(t, err, nil) + + // Normalize both slices so there is no extra spacing, new lines, etc + normalizedExpected := normalizeKVPairs(t, expectedDefaultDenyACL) + normalizedActual := normalizeKVPairs(t, podIPInfo.DefaultDenyACL) + if !reflect.DeepEqual(normalizedExpected, normalizedActual) { + t.Errorf("got '%+v', expected '%+v'", podIPInfo.DefaultDenyACL, expectedDefaultDenyACL) + } +} + +// normalizeKVPairs normalizes the JSON values in the KV pairs by unmarshaling them into a map, then marshaling them back to compact JSON to remove any extra space, new lines, etc +func normalizeKVPairs(t *testing.T, kvPairs []cni.KVPair) []cni.KVPair { + normalized := make([]cni.KVPair, len(kvPairs)) + + for i, kv := range kvPairs { + var unmarshaledValue map[string]interface{} + // Unmarshal the Value into a map + err := json.Unmarshal(kv.Value, &unmarshaledValue) + require.NoError(t, err, "Failed to unmarshal JSON value") + + // Marshal it back to compact JSON + normalizedValue, err := json.Marshal(unmarshaledValue) + require.NoError(t, err, "Failed to re-marshal JSON value") + + // Replace Value with the normalized compact JSON + normalized[i] = cni.KVPair{ + Name: kv.Name, + Value: normalizedValue, + } + } + + return normalized +} From ed09360fc722d8afe90e7bedc6d356e02e0e121b Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 23 Dec 2024 13:34:14 -0800 Subject: [PATCH 04/48] resolved pr comments --- cns/middlewares/k8sSwiftV2.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 881c6f8ec5..e97f683c63 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -59,14 +59,14 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny acl's pn the infra IP configs - for i := range ipConfigsResp.PodIPInfo { - ipInfo := &ipConfigsResp.PodIPInfo[i] + for _, ipInfo := range ipConfigsResp.PodIPInfo { // there will be no pod connectivity to and from those pods if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - err = addDefaultDenyACL(ipInfo) + err = addDefaultDenyACL(&ipInfo) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } + break } } From baf31a3dec6a52fde5f577fe3072962f63b772d7 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 23 Dec 2024 14:29:03 -0800 Subject: [PATCH 05/48] updating to fix github checks --- cns/middlewares/k8sSwiftV2.go | 5 +++-- cns/middlewares/k8sSwiftV2_windows.go | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index e97f683c63..e82755152e 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -59,10 +59,11 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny acl's pn the infra IP configs - for _, ipInfo := range ipConfigsResp.PodIPInfo { + for i := range ipConfigsResp.PodIPInfo { + ipInfo := &ipConfigsResp.PodIPInfo[i] // there will be no pod connectivity to and from those pods if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - err = addDefaultDenyACL(&ipInfo) + err = addDefaultDenyACL(ipInfo) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 545b1ffd8f..cf670fa3c0 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -64,7 +64,7 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st } // append the default deny acl's to the list defaultDenyACL field in podIpInfo -func addDefaultDenyACL(podIpInfo *cns.PodIpInfo) error { +func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error { blockEgressACL, err := getDefaultDenyACLPolicy(hcn.DirectionTypeOut) if err != nil { return errors.Wrap(err, "Failed to create default deny ACL policy egress") @@ -86,7 +86,7 @@ func addDefaultDenyACL(podIpInfo *cns.PodIpInfo) error { }, } - podIpInfo.DefaultDenyACL = append(podIpInfo.DefaultDenyACL, additionalArgs...) + podIPInfo.DefaultDenyACL = append(podIPInfo.DefaultDenyACL, additionalArgs...) return nil } From d531de8d5fe822ee0e9869301d4bba76d71f44da Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 23 Dec 2024 14:44:08 -0800 Subject: [PATCH 06/48] added logging lines --- cns/middlewares/k8sSwiftV2.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index e82755152e..8122fecc97 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -62,7 +62,9 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa for i := range ipConfigsResp.PodIPInfo { ipInfo := &ipConfigsResp.PodIPInfo[i] // there will be no pod connectivity to and from those pods + logger.Printf("type of nic is %s", string(ipInfo.NICType)) if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { + logger.Printf("adding default deny acl's") err = addDefaultDenyACL(ipInfo) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) From b234290b3615517040b64f924e05d881299c9e3f Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 23 Dec 2024 15:15:55 -0800 Subject: [PATCH 07/48] removing unnecessary logging lines --- cns/middlewares/k8sSwiftV2.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 8122fecc97..e82755152e 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -62,9 +62,7 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa for i := range ipConfigsResp.PodIPInfo { ipInfo := &ipConfigsResp.PodIPInfo[i] // there will be no pod connectivity to and from those pods - logger.Printf("type of nic is %s", string(ipInfo.NICType)) if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - logger.Printf("adding default deny acl's") err = addDefaultDenyACL(ipInfo) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) From 5187545884f11dbd26237b1ea0a77f5d278353ac Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 6 Jan 2025 14:40:36 -0800 Subject: [PATCH 08/48] removed cni circular dependency --- cni/netconfig.go | 11 +++-------- cni/network/network_windows_test.go | 3 ++- cns/NetworkContainerContract.go | 4 ++-- cns/middlewares/k8sSwiftV2_windows.go | 4 ++-- cns/middlewares/k8sSwiftV2_windows_test.go | 10 +++++----- common/config.go | 10 ++++++++++ 6 files changed, 24 insertions(+), 18 deletions(-) diff --git a/cni/netconfig.go b/cni/netconfig.go index c7e0c0ca7e..68d5cd775e 100644 --- a/cni/netconfig.go +++ b/cni/netconfig.go @@ -7,6 +7,7 @@ import ( "encoding/json" "strings" + acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/network/policy" cniTypes "github.com/containernetworking/cni/pkg/types" ) @@ -15,12 +16,6 @@ const ( PolicyStr string = "Policy" ) -// KVPair represents a K-V pair of a json object. -type KVPair struct { - Name string `json:"name"` - Value json.RawMessage `json:"value"` -} - type PortMapping struct { HostPort int `json:"hostPort"` ContainerPort int `json:"containerPort"` @@ -78,7 +73,7 @@ type NetworkConfig struct { DNS cniTypes.DNS `json:"dns,omitempty"` RuntimeConfig RuntimeConfig `json:"runtimeConfig,omitempty"` WindowsSettings WindowsSettings `json:"windowsSettings,omitempty"` - AdditionalArgs []KVPair `json:"AdditionalArgs,omitempty"` + AdditionalArgs []acn.KVPair `json:"AdditionalArgs,omitempty"` } type WindowsSettings struct { @@ -121,7 +116,7 @@ func ParseNetworkConfig(b []byte) (*NetworkConfig, error) { } // GetPoliciesFromNwCfg returns network policies from network config. -func GetPoliciesFromNwCfg(kvp []KVPair) []policy.Policy { +func GetPoliciesFromNwCfg(kvp []acn.KVPair) []policy.Policy { var policies []policy.Policy for _, pair := range kvp { if strings.Contains(pair.Name, PolicyStr) { diff --git a/cni/network/network_windows_test.go b/cni/network/network_windows_test.go index 9da54a4ca4..39da4bd414 100644 --- a/cni/network/network_windows_test.go +++ b/cni/network/network_windows_test.go @@ -12,6 +12,7 @@ import ( "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" + acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/network" "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/network/policy" @@ -941,7 +942,7 @@ func TestPluginWindowsAdd(t *testing.T) { EnableExactMatchForPodName: true, Master: "eth0", // these are added to test that policies propagate to endpoint info - AdditionalArgs: []cni.KVPair{ + AdditionalArgs: []acn.KVPair{ { Name: "EndpointPolicy", Value: GetRawOutBoundNATPolicy(), diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 18829c3f8b..72ab3f0b2e 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -7,8 +7,8 @@ import ( "strconv" "strings" - "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns/types" + acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" "github.com/pkg/errors" @@ -505,7 +505,7 @@ type PodIpInfo struct { // PnpId is set for backend interfaces, Pnp Id identifies VF. Plug and play id(pnp) is also called as PCI ID PnPID string // Defauly Deny ACL's to configure on HNS endpoints for Swiftv2 window nodes - DefaultDenyACL []cni.KVPair + DefaultDenyACL []acn.KVPair } type HostIPInfo struct { diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index cf670fa3c0..01f21250f8 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -3,9 +3,9 @@ package middlewares import ( "encoding/json" - "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/middlewares/utils" + acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" "github.com/Microsoft/hcsshim/hcn" "github.com/pkg/errors" @@ -75,7 +75,7 @@ func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error { return errors.Wrap(err, "Failed to create default deny ACL policy ingress") } - additionalArgs := []cni.KVPair{ + additionalArgs := []acn.KVPair{ { Name: "EndpointPolicy", Value: blockEgressACL, diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index 2eb9ccd61f..324b46f279 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -5,9 +5,9 @@ import ( "reflect" "testing" - "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/middlewares/mock" + acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" "github.com/stretchr/testify/require" "gotest.tools/v3/assert" @@ -119,7 +119,7 @@ func TestAddDefaultDenyACL(t *testing.T) { "Priority": 10000 }`) - expectedDefaultDenyACL := []cni.KVPair{ + expectedDefaultDenyACL := []acn.KVPair{ { Name: "EndpointPolicy", Value: valueOut, @@ -151,8 +151,8 @@ func TestAddDefaultDenyACL(t *testing.T) { } // normalizeKVPairs normalizes the JSON values in the KV pairs by unmarshaling them into a map, then marshaling them back to compact JSON to remove any extra space, new lines, etc -func normalizeKVPairs(t *testing.T, kvPairs []cni.KVPair) []cni.KVPair { - normalized := make([]cni.KVPair, len(kvPairs)) +func normalizeKVPairs(t *testing.T, kvPairs []acn.KVPair) []acn.KVPair { + normalized := make([]acn.KVPair, len(kvPairs)) for i, kv := range kvPairs { var unmarshaledValue map[string]interface{} @@ -165,7 +165,7 @@ func normalizeKVPairs(t *testing.T, kvPairs []cni.KVPair) []cni.KVPair { require.NoError(t, err, "Failed to re-marshal JSON value") // Replace Value with the normalized compact JSON - normalized[i] = cni.KVPair{ + normalized[i] = acn.KVPair{ Name: kv.Name, Value: normalizedValue, } diff --git a/common/config.go b/common/config.go index 3434c2e2e1..96f7eb870f 100644 --- a/common/config.go +++ b/common/config.go @@ -3,6 +3,10 @@ package common +import ( + "encoding/json" +) + // Command line options. const ( // Operating environment. @@ -146,3 +150,9 @@ const ( // OptCNIConflistScenarioAlias "shorthand" for the cni conflist scenairo, see above OptCNIConflistScenarioAlias = "cniconflistscenario" ) + +// KVPair represents a K-V pair of a json object. +type KVPair struct { + Name string `json:"name"` + Value json.RawMessage `json:"value"` +} From e89f70f052eee64968e92f6654aaa19c8a2862ab Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 6 Jan 2025 15:02:49 -0800 Subject: [PATCH 09/48] switch from having consts to making them inline --- cns/middlewares/k8sSwiftV2_windows.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 01f21250f8..6aff92ef6e 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -93,8 +93,6 @@ func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error { // create the default deny acl's that need to be added to the list defaultDenyACL field in podIpInfo func getDefaultDenyACLPolicy(direction hcn.DirectionType) ([]byte, error) { - const DefaultDenyPriority = 10000 - const policyType = "ACL" type DefaultDenyACL struct { Type string `json:"Type"` Action hcn.ActionType `json:"Action"` @@ -103,10 +101,10 @@ func getDefaultDenyACLPolicy(direction hcn.DirectionType) ([]byte, error) { } denyACL := DefaultDenyACL{ - Type: policyType, + Type: "ACL", // policy type is ACL Action: hcn.ActionTypeBlock, Direction: direction, - Priority: DefaultDenyPriority, + Priority: 10_000, // default deny priority will be 10_000 } denyACLJSON, err := json.Marshal(denyACL) From a56b665597185ca06462416b79890ca731d98874 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 8 Jan 2025 18:56:42 -0800 Subject: [PATCH 10/48] cns changes based on update to network container contrac --- cni/netconfig.go | 10 +++++++--- cni/network/network_windows_test.go | 3 +-- cns/NetworkContainerContract.go | 6 +++--- cns/middlewares/k8sSwiftV2_windows.go | 14 +++++++------- cns/middlewares/k8sSwiftV2_windows_test.go | 7 ++++--- common/config.go | 10 ---------- 6 files changed, 22 insertions(+), 28 deletions(-) diff --git a/cni/netconfig.go b/cni/netconfig.go index 68d5cd775e..5e26e63f0c 100644 --- a/cni/netconfig.go +++ b/cni/netconfig.go @@ -7,7 +7,6 @@ import ( "encoding/json" "strings" - acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/network/policy" cniTypes "github.com/containernetworking/cni/pkg/types" ) @@ -16,6 +15,11 @@ const ( PolicyStr string = "Policy" ) +type KVPair struct { + Name string `json:"name"` + Value json.RawMessage `json:"value"` +} + type PortMapping struct { HostPort int `json:"hostPort"` ContainerPort int `json:"containerPort"` @@ -73,7 +77,7 @@ type NetworkConfig struct { DNS cniTypes.DNS `json:"dns,omitempty"` RuntimeConfig RuntimeConfig `json:"runtimeConfig,omitempty"` WindowsSettings WindowsSettings `json:"windowsSettings,omitempty"` - AdditionalArgs []acn.KVPair `json:"AdditionalArgs,omitempty"` + AdditionalArgs []KVPair `json:"AdditionalArgs,omitempty"` } type WindowsSettings struct { @@ -116,7 +120,7 @@ func ParseNetworkConfig(b []byte) (*NetworkConfig, error) { } // GetPoliciesFromNwCfg returns network policies from network config. -func GetPoliciesFromNwCfg(kvp []acn.KVPair) []policy.Policy { +func GetPoliciesFromNwCfg(kvp []KVPair) []policy.Policy { var policies []policy.Policy for _, pair := range kvp { if strings.Contains(pair.Name, PolicyStr) { diff --git a/cni/network/network_windows_test.go b/cni/network/network_windows_test.go index 39da4bd414..9da54a4ca4 100644 --- a/cni/network/network_windows_test.go +++ b/cni/network/network_windows_test.go @@ -12,7 +12,6 @@ import ( "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" - acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/network" "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/network/policy" @@ -942,7 +941,7 @@ func TestPluginWindowsAdd(t *testing.T) { EnableExactMatchForPodName: true, Master: "eth0", // these are added to test that policies propagate to endpoint info - AdditionalArgs: []acn.KVPair{ + AdditionalArgs: []cni.KVPair{ { Name: "EndpointPolicy", Value: GetRawOutBoundNATPolicy(), diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 72ab3f0b2e..ef2d887db8 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -8,8 +8,8 @@ import ( "strings" "github.com/Azure/azure-container-networking/cns/types" - acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" + "github.com/Azure/azure-container-networking/network/policy" "github.com/google/uuid" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -504,8 +504,8 @@ type PodIpInfo struct { Routes []Route // PnpId is set for backend interfaces, Pnp Id identifies VF. Plug and play id(pnp) is also called as PCI ID PnPID string - // Defauly Deny ACL's to configure on HNS endpoints for Swiftv2 window nodes - DefaultDenyACL []acn.KVPair + // Default Deny ACL's to configure on HNS endpoints for Swiftv2 window nodes + EdpointPolicies []policy.Policy } type HostIPInfo struct { diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 6aff92ef6e..d0eec71ba3 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -5,8 +5,8 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/middlewares/utils" - acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" + "github.com/Azure/azure-container-networking/network/policy" "github.com/Microsoft/hcsshim/hcn" "github.com/pkg/errors" ) @@ -75,18 +75,18 @@ func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error { return errors.Wrap(err, "Failed to create default deny ACL policy ingress") } - additionalArgs := []acn.KVPair{ + additionalArgs := []policy.Policy{ { - Name: "EndpointPolicy", - Value: blockEgressACL, + Type: policy.ACLPolicy, + Data: blockEgressACL, }, { - Name: "EndpointPolicy", - Value: blockIngressACL, + Type: policy.ACLPolicy, + Data: blockIngressACL, }, } - podIPInfo.DefaultDenyACL = append(podIPInfo.DefaultDenyACL, additionalArgs...) + podIPInfo.EdpointPolicies = append(podIPInfo.EdpointPolicies, additionalArgs...) return nil } diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index 324b46f279..3d549319b6 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -5,6 +5,7 @@ import ( "reflect" "testing" + "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/middlewares/mock" acn "github.com/Azure/azure-container-networking/common" @@ -119,7 +120,7 @@ func TestAddDefaultDenyACL(t *testing.T) { "Priority": 10000 }`) - expectedDefaultDenyACL := []acn.KVPair{ + expectedDefaultDenyACL := []cni.KVPair{ { Name: "EndpointPolicy", Value: valueOut, @@ -151,8 +152,8 @@ func TestAddDefaultDenyACL(t *testing.T) { } // normalizeKVPairs normalizes the JSON values in the KV pairs by unmarshaling them into a map, then marshaling them back to compact JSON to remove any extra space, new lines, etc -func normalizeKVPairs(t *testing.T, kvPairs []acn.KVPair) []acn.KVPair { - normalized := make([]acn.KVPair, len(kvPairs)) +func normalizeKVPairs(t *testing.T, kvPairs []acn.KVPair) []cni.KVPair { + normalized := make([]cni.KVPair, len(kvPairs)) for i, kv := range kvPairs { var unmarshaledValue map[string]interface{} diff --git a/common/config.go b/common/config.go index 96f7eb870f..3434c2e2e1 100644 --- a/common/config.go +++ b/common/config.go @@ -3,10 +3,6 @@ package common -import ( - "encoding/json" -) - // Command line options. const ( // Operating environment. @@ -150,9 +146,3 @@ const ( // OptCNIConflistScenarioAlias "shorthand" for the cni conflist scenairo, see above OptCNIConflistScenarioAlias = "cniconflistscenario" ) - -// KVPair represents a K-V pair of a json object. -type KVPair struct { - Name string `json:"name"` - Value json.RawMessage `json:"value"` -} From 725d6cabc3b935f225b3db2ecda3a638162daa1c Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 8 Jan 2025 19:18:20 -0800 Subject: [PATCH 11/48] fixed spelling --- cns/NetworkContainerContract.go | 2 +- cns/middlewares/k8sSwiftV2_windows.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index ef2d887db8..c93187a0e2 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -505,7 +505,7 @@ type PodIpInfo struct { // PnpId is set for backend interfaces, Pnp Id identifies VF. Plug and play id(pnp) is also called as PCI ID PnPID string // Default Deny ACL's to configure on HNS endpoints for Swiftv2 window nodes - EdpointPolicies []policy.Policy + EndpointPolicies []policy.Policy } type HostIPInfo struct { diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index d0eec71ba3..1f200ac259 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -86,7 +86,7 @@ func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error { }, } - podIPInfo.EdpointPolicies = append(podIPInfo.EdpointPolicies, additionalArgs...) + podIPInfo.EndpointPolicies = append(podIPInfo.EndpointPolicies, additionalArgs...) return nil } From 9d2ef054e485b7b43cde8da4b30e365d21b125e7 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 8 Jan 2025 19:24:38 -0800 Subject: [PATCH 12/48] updated unit test --- cns/middlewares/k8sSwiftV2_windows_test.go | 29 +++++++++++----------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index 3d549319b6..5480815dce 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -5,11 +5,10 @@ import ( "reflect" "testing" - "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/middlewares/mock" - acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" + "github.com/Azure/azure-container-networking/network/policy" "github.com/stretchr/testify/require" "gotest.tools/v3/assert" ) @@ -120,14 +119,14 @@ func TestAddDefaultDenyACL(t *testing.T) { "Priority": 10000 }`) - expectedDefaultDenyACL := []cni.KVPair{ + expectedDefaultDenyACL := []policy.Policy{ { - Name: "EndpointPolicy", - Value: valueOut, + Type: policy.ACLPolicy, + Data: valueOut, }, { - Name: "EndpointPolicy", - Value: valueIn, + Type: policy.ACLPolicy, + Data: valueIn, }, } @@ -145,20 +144,20 @@ func TestAddDefaultDenyACL(t *testing.T) { // Normalize both slices so there is no extra spacing, new lines, etc normalizedExpected := normalizeKVPairs(t, expectedDefaultDenyACL) - normalizedActual := normalizeKVPairs(t, podIPInfo.DefaultDenyACL) + normalizedActual := normalizeKVPairs(t, podIPInfo.EndpointPolicies) if !reflect.DeepEqual(normalizedExpected, normalizedActual) { - t.Errorf("got '%+v', expected '%+v'", podIPInfo.DefaultDenyACL, expectedDefaultDenyACL) + t.Errorf("got '%+v', expected '%+v'", podIPInfo.EndpointPolicies, expectedDefaultDenyACL) } } // normalizeKVPairs normalizes the JSON values in the KV pairs by unmarshaling them into a map, then marshaling them back to compact JSON to remove any extra space, new lines, etc -func normalizeKVPairs(t *testing.T, kvPairs []acn.KVPair) []cni.KVPair { - normalized := make([]cni.KVPair, len(kvPairs)) +func normalizeKVPairs(t *testing.T, kvPairs []policy.Policy) []policy.Policy { + normalized := make([]policy.Policy, len(kvPairs)) for i, kv := range kvPairs { var unmarshaledValue map[string]interface{} // Unmarshal the Value into a map - err := json.Unmarshal(kv.Value, &unmarshaledValue) + err := json.Unmarshal(kv.Data, &unmarshaledValue) require.NoError(t, err, "Failed to unmarshal JSON value") // Marshal it back to compact JSON @@ -166,9 +165,9 @@ func normalizeKVPairs(t *testing.T, kvPairs []acn.KVPair) []cni.KVPair { require.NoError(t, err, "Failed to re-marshal JSON value") // Replace Value with the normalized compact JSON - normalized[i] = acn.KVPair{ - Name: kv.Name, - Value: normalizedValue, + normalized[i] = policy.Policy{ + Type: policy.ACLPolicy, + Data: normalizedValue, } } From 753852c83a8a36814430c4b31bdb34d87f697ef3 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 8 Jan 2025 19:28:40 -0800 Subject: [PATCH 13/48] updated test --- cns/middlewares/k8sSwiftV2_windows_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index 5480815dce..fb6ed62978 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -151,10 +151,10 @@ func TestAddDefaultDenyACL(t *testing.T) { } // normalizeKVPairs normalizes the JSON values in the KV pairs by unmarshaling them into a map, then marshaling them back to compact JSON to remove any extra space, new lines, etc -func normalizeKVPairs(t *testing.T, kvPairs []policy.Policy) []policy.Policy { - normalized := make([]policy.Policy, len(kvPairs)) +func normalizeKVPairs(t *testing.T, policies []policy.Policy) []policy.Policy { + normalized := make([]policy.Policy, len(policies)) - for i, kv := range kvPairs { + for i, kv := range policies { var unmarshaledValue map[string]interface{} // Unmarshal the Value into a map err := json.Unmarshal(kv.Data, &unmarshaledValue) From aa59a399683f0f7bca94d3a324297fdb19a02660 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 8 Jan 2025 19:38:34 -0800 Subject: [PATCH 14/48] reverted a comment --- cni/netconfig.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cni/netconfig.go b/cni/netconfig.go index 5e26e63f0c..c7e0c0ca7e 100644 --- a/cni/netconfig.go +++ b/cni/netconfig.go @@ -15,6 +15,7 @@ const ( PolicyStr string = "Policy" ) +// KVPair represents a K-V pair of a json object. type KVPair struct { Name string `json:"name"` Value json.RawMessage `json:"value"` From 82cfe55bd4e24b5923750e99ffcbf4e8693e6ea0 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 8 Jan 2025 19:45:34 -0800 Subject: [PATCH 15/48] updated name of function --- cns/middlewares/k8sSwiftV2.go | 6 +++--- cns/middlewares/k8sSwiftV2_linux_test.go | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index e82755152e..20e514c13a 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -40,7 +40,7 @@ var _ cns.IPConfigsHandlerMiddleware = (*K8sSWIFTv2Middleware)(nil) // and release IP configs handlers. func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, respCode, message, defaultDenyACLbool := k.validateIPConfigsRequest(ctx, &req) + podInfo, respCode, message, defaultDenyACLbool := k.GetPodInfoForIPConfigsRequest(ctx, &req) logger.Printf("defaultDenyACLbool value is: %v", defaultDenyACLbool) @@ -116,9 +116,9 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } } -// validateIPConfigsRequest validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. +// GetPodInfoForIPConfigsRequest validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. // nolint -func (k *K8sSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string, defaultDenyACL bool) { +func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string, defaultDenyACL bool) { defaultDenyACLbool := false // Retrieve the pod from the cluster diff --git a/cns/middlewares/k8sSwiftV2_linux_test.go b/cns/middlewares/k8sSwiftV2_linux_test.go index 8d42f8cfe1..8bd0990728 100644 --- a/cns/middlewares/k8sSwiftV2_linux_test.go +++ b/cns/middlewares/k8sSwiftV2_linux_test.go @@ -144,7 +144,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq.OrchestratorContext = b happyReq.SecondaryInterfacesExist = false - _, respCode, err, _ := middleware.validateIPConfigsRequest(context.TODO(), happyReq) + _, respCode, err, _ := middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq.SecondaryInterfacesExist, true) @@ -158,7 +158,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq2.OrchestratorContext = b happyReq2.SecondaryInterfacesExist = false - _, respCode, err, _ = middleware.validateIPConfigsRequest(context.TODO(), happyReq2) + _, respCode, err, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq2) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq.SecondaryInterfacesExist, true) @@ -172,7 +172,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq3.OrchestratorContext = b happyReq3.SecondaryInterfacesExist = false - _, respCode, err, _ = middleware.validateIPConfigsRequest(context.TODO(), happyReq3) + _, respCode, err, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq3) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq3.SecondaryInterfacesExist, false) @@ -188,7 +188,7 @@ func TestValidateMultitenantIPConfigsRequestFailure(t *testing.T) { InfraContainerID: testPod1Info.InfraContainerID(), } failReq.OrchestratorContext = []byte("invalid") - _, respCode, _, _ := middleware.validateIPConfigsRequest(context.TODO(), failReq) + _, respCode, _, _ := middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) // Pod doesn't exist in cache test @@ -198,19 +198,19 @@ func TestValidateMultitenantIPConfigsRequestFailure(t *testing.T) { } b, _ := testPod2Info.OrchestratorContext() failReq.OrchestratorContext = b - _, respCode, _, _ = middleware.validateIPConfigsRequest(context.TODO(), failReq) + _, respCode, _, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) // Failed to get MTPNC b, _ = testPod3Info.OrchestratorContext() failReq.OrchestratorContext = b - _, respCode, _, _ = middleware.validateIPConfigsRequest(context.TODO(), failReq) + _, respCode, _, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) // MTPNC not ready b, _ = testPod4Info.OrchestratorContext() failReq.OrchestratorContext = b - _, respCode, _, _ = middleware.validateIPConfigsRequest(context.TODO(), failReq) + _, respCode, _, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) } From 4192c27556f74229d84f17e7a19e7282644420c1 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 9 Jan 2025 13:28:00 -0800 Subject: [PATCH 16/48] changed policy type --- cns/middlewares/k8sSwiftV2_windows.go | 4 ++-- cns/middlewares/k8sSwiftV2_windows_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 1f200ac259..2ee79f6484 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -77,11 +77,11 @@ func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error { additionalArgs := []policy.Policy{ { - Type: policy.ACLPolicy, + Type: policy.EndpointPolicy, Data: blockEgressACL, }, { - Type: policy.ACLPolicy, + Type: policy.EndpointPolicy, Data: blockIngressACL, }, } diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index fb6ed62978..2fa465424d 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -121,11 +121,11 @@ func TestAddDefaultDenyACL(t *testing.T) { expectedDefaultDenyACL := []policy.Policy{ { - Type: policy.ACLPolicy, + Type: policy.EndpointPolicy, Data: valueOut, }, { - Type: policy.ACLPolicy, + Type: policy.EndpointPolicy, Data: valueIn, }, } @@ -166,7 +166,7 @@ func normalizeKVPairs(t *testing.T, policies []policy.Policy) []policy.Policy { // Replace Value with the normalized compact JSON normalized[i] = policy.Policy{ - Type: policy.ACLPolicy, + Type: policy.EndpointPolicy, Data: normalizedValue, } } From 23fba7e607427795b78d29aea45e4d59206ec21a Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 9 Jan 2025 16:24:43 -0800 Subject: [PATCH 17/48] added a new line --- cns/middlewares/k8sSwiftV2_windows.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 2ee79f6484..5a32f6857a 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -108,7 +108,6 @@ func getDefaultDenyACLPolicy(direction hcn.DirectionType) ([]byte, error) { } denyACLJSON, err := json.Marshal(denyACL) - if err != nil { return nil, errors.Wrap(err, "error marshalling default deny policy to json") } From 3b0f6b5a6b1a957ef369995275a438ccdd69727c Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 14 Jan 2025 12:48:17 -0800 Subject: [PATCH 18/48] resolving pr comments --- cns/middlewares/k8sSwiftV2.go | 21 ++++++++++++--------- cns/middlewares/k8sSwiftV2_linux.go | 5 +++-- cns/middlewares/k8sSwiftV2_windows.go | 10 ++++------ cns/middlewares/k8sSwiftV2_windows_test.go | 19 +++++-------------- 4 files changed, 24 insertions(+), 31 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 20e514c13a..fa3b175cae 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-container-networking/cns/middlewares/utils" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" + "github.com/Azure/azure-container-networking/network/policy" "github.com/pkg/errors" v1 "k8s.io/api/core/v1" k8stypes "k8s.io/apimachinery/pkg/types" @@ -40,7 +41,7 @@ var _ cns.IPConfigsHandlerMiddleware = (*K8sSWIFTv2Middleware)(nil) // and release IP configs handlers. func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, respCode, message, defaultDenyACLbool := k.GetPodInfoForIPConfigsRequest(ctx, &req) + podInfo, respCode, defaultDenyACLbool, message := k.GetPodInfoForIPConfigsRequest(ctx, &req) logger.Printf("defaultDenyACLbool value is: %v", defaultDenyACLbool) @@ -61,12 +62,14 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny acl's pn the infra IP configs for i := range ipConfigsResp.PodIPInfo { ipInfo := &ipConfigsResp.PodIPInfo[i] + var defaultDenyEndpointPolicies []policy.Policy // there will be no pod connectivity to and from those pods if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - err = addDefaultDenyACL(ipInfo) + defaultDenyEndpointPolicies, err = addDefaultDenyACL() if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } + ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEndpointPolicies...) break } } @@ -118,21 +121,21 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa // GetPodInfoForIPConfigsRequest validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. // nolint -func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string, defaultDenyACL bool) { +func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, defaultDenyACL bool, message string) { defaultDenyACLbool := false // Retrieve the pod from the cluster podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) if err != nil { errBuf := errors.Wrapf(err, "failed to unmarshalling pod info from ipconfigs request %+v", req) - return nil, types.UnexpectedError, errBuf.Error(), defaultDenyACLbool + return nil, types.UnexpectedError, defaultDenyACLbool, errBuf.Error() } logger.Printf("[SWIFTv2Middleware] validate ipconfigs request for pod %s", podInfo.Name()) podNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()} pod := v1.Pod{} if err := k.Cli.Get(ctx, podNamespacedName, &pod); err != nil { errBuf := errors.Wrapf(err, "failed to get pod %+v", podNamespacedName) - return nil, types.UnexpectedError, errBuf.Error(), defaultDenyACLbool + return nil, types.UnexpectedError, defaultDenyACLbool, errBuf.Error() } // check the pod labels for Swift V2, set the request's SecondaryInterfaceSet flag to true and check if its MTPNC CRD is ready @@ -144,11 +147,11 @@ func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context mtpnc := v1alpha1.MultitenantPodNetworkConfig{} mtpncNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()} if err := k.Cli.Get(ctx, mtpncNamespacedName, &mtpnc); err != nil { - return nil, types.UnexpectedError, fmt.Errorf("failed to get pod's mtpnc from cache : %w", err).Error(), defaultDenyACLbool + return nil, types.UnexpectedError, defaultDenyACLbool, fmt.Errorf("failed to get pod's mtpnc from cache : %w", err).Error() } // Check if the MTPNC CRD is ready. If one of the fields is empty, return error if !mtpnc.IsReady() { - return nil, types.UnexpectedError, errMTPNCNotReady.Error(), defaultDenyACLbool + return nil, types.UnexpectedError, defaultDenyACLbool, errMTPNCNotReady.Error() } // copying defaultDenyACL bool from mtpnc @@ -162,7 +165,7 @@ func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context for _, interfaceInfo := range interfaceInfos { if interfaceInfo.DeviceType == v1alpha1.DeviceTypeInfiniBandNIC { if interfaceInfo.MacAddress == "" || interfaceInfo.NCID == "" { - return nil, types.UnexpectedError, errMTPNCNotReady.Error(), defaultDenyACLbool + return nil, types.UnexpectedError, defaultDenyACLbool, errMTPNCNotReady.Error() } req.BackendInterfaceExist = true req.BackendInterfaceMacAddresses = append(req.BackendInterfaceMacAddresses, interfaceInfo.MacAddress) @@ -176,7 +179,7 @@ func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist) logger.Printf("[SWIFTv2Middleware] pod %s has backend interface : %v", podInfo.Name(), req.BackendInterfaceExist) // retrieve podinfo from orchestrator context - return podInfo, types.Success, "", defaultDenyACLbool + return podInfo, types.Success, defaultDenyACLbool, "" } // getIPConfig returns the pod's SWIFT V2 IP configuration. diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 99b8ae7846..709d2d5aff 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/middlewares/utils" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" + "github.com/Azure/azure-container-networking/network/policy" "github.com/pkg/errors" ) @@ -104,6 +105,6 @@ func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(_ *cns.PodIpInfo, func (k *K8sSWIFTv2Middleware) addDefaultRoute(*cns.PodIpInfo, string) {} -func addDefaultDenyACL(_ *cns.PodIpInfo) error { - return nil +func addDefaultDenyACL() ([]policy.Policy, error) { + return []policy.Policy{}, nil } diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 5a32f6857a..fded3d194c 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -64,15 +64,15 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st } // append the default deny acl's to the list defaultDenyACL field in podIpInfo -func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error { +func addDefaultDenyACL() ([]policy.Policy, error) { blockEgressACL, err := getDefaultDenyACLPolicy(hcn.DirectionTypeOut) if err != nil { - return errors.Wrap(err, "Failed to create default deny ACL policy egress") + return []policy.Policy{}, errors.Wrap(err, "failed to create default deny ACL policy egress") } blockIngressACL, err := getDefaultDenyACLPolicy(hcn.DirectionTypeIn) if err != nil { - return errors.Wrap(err, "Failed to create default deny ACL policy ingress") + return []policy.Policy{}, errors.Wrap(err, "Failed to create default deny ACL policy ingress") } additionalArgs := []policy.Policy{ @@ -86,9 +86,7 @@ func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error { }, } - podIPInfo.EndpointPolicies = append(podIPInfo.EndpointPolicies, additionalArgs...) - - return nil + return additionalArgs, nil } // create the default deny acl's that need to be added to the list defaultDenyACL field in podIpInfo diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index 2fa465424d..bbf7bd4528 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -119,7 +119,7 @@ func TestAddDefaultDenyACL(t *testing.T) { "Priority": 10000 }`) - expectedDefaultDenyACL := []policy.Policy{ + expectedDefaultDenyEndpoint := []policy.Policy{ { Type: policy.EndpointPolicy, Data: valueOut, @@ -130,23 +130,14 @@ func TestAddDefaultDenyACL(t *testing.T) { }, } - podIPInfo := cns.PodIpInfo{ - PodIPConfig: cns.IPSubnet{ - IPAddress: "20.240.1.242", - PrefixLength: 32, - }, - NICType: cns.DelegatedVMNIC, - MacAddress: "12:34:56:78:9a:bc", - } - - err := addDefaultDenyACL(&podIPInfo) + defaultDenyEndpoint, err := addDefaultDenyACL() assert.Equal(t, err, nil) // Normalize both slices so there is no extra spacing, new lines, etc - normalizedExpected := normalizeKVPairs(t, expectedDefaultDenyACL) - normalizedActual := normalizeKVPairs(t, podIPInfo.EndpointPolicies) + normalizedExpected := normalizeKVPairs(t, expectedDefaultDenyEndpoint) + normalizedActual := normalizeKVPairs(t, defaultDenyEndpoint) if !reflect.DeepEqual(normalizedExpected, normalizedActual) { - t.Errorf("got '%+v', expected '%+v'", podIPInfo.EndpointPolicies, expectedDefaultDenyACL) + t.Errorf("got '%+v', expected '%+v'", normalizedActual, normalizedExpected) } } From 63ae2188a79b8da6da8890495f02cf521a074f96 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 14 Jan 2025 14:56:38 -0800 Subject: [PATCH 19/48] resolving pr comments --- cns/endpointmanager/endpointmanager.go | 21 ---------- cns/endpointmanager/endpointmanager_linux.go | 13 ------ .../endpointmanager_windows.go | 42 ------------------- 3 files changed, 76 deletions(-) delete mode 100644 cns/endpointmanager/endpointmanager.go delete mode 100644 cns/endpointmanager/endpointmanager_linux.go delete mode 100644 cns/endpointmanager/endpointmanager_windows.go diff --git a/cns/endpointmanager/endpointmanager.go b/cns/endpointmanager/endpointmanager.go deleted file mode 100644 index e71f0a6cbc..0000000000 --- a/cns/endpointmanager/endpointmanager.go +++ /dev/null @@ -1,21 +0,0 @@ -package endpointmanager - -import ( - "context" - - "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/restserver" -) - -type EndpointManager struct { - cli releaseIPsClient // nolint -} - -type releaseIPsClient interface { - ReleaseIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) error - GetEndpoint(ctx context.Context, endpointID string) (*restserver.GetEndpointResponse, error) -} - -func WithPlatformReleaseIPsManager(cli releaseIPsClient) *EndpointManager { - return &EndpointManager{cli: cli} -} diff --git a/cns/endpointmanager/endpointmanager_linux.go b/cns/endpointmanager/endpointmanager_linux.go deleted file mode 100644 index 4441d84110..0000000000 --- a/cns/endpointmanager/endpointmanager_linux.go +++ /dev/null @@ -1,13 +0,0 @@ -package endpointmanager - -import ( - "context" - - "github.com/Azure/azure-container-networking/cns" - "github.com/pkg/errors" -) - -// ReleaseIPs implements an Interface in fsnotify for async delete of the HNS endpoint and IP addresses -func (em *EndpointManager) ReleaseIPs(ctx context.Context, ipconfigreq cns.IPConfigsRequest) error { - return errors.Wrap(em.cli.ReleaseIPs(ctx, ipconfigreq), "failed to release IP from CNS") -} diff --git a/cns/endpointmanager/endpointmanager_windows.go b/cns/endpointmanager/endpointmanager_windows.go deleted file mode 100644 index dd928b721d..0000000000 --- a/cns/endpointmanager/endpointmanager_windows.go +++ /dev/null @@ -1,42 +0,0 @@ -package endpointmanager - -import ( - "context" - - "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/hnsclient" - "github.com/Azure/azure-container-networking/cns/logger" - "github.com/pkg/errors" -) - -// ReleaseIPs implements an Interface in fsnotify for async delete of the HNS endpoint and IP addresses -func (em *EndpointManager) ReleaseIPs(ctx context.Context, ipconfigreq cns.IPConfigsRequest) error { - logger.Printf("deleting HNS Endpoint asynchronously") - // remove HNS endpoint - if err := em.deleteEndpoint(ctx, ipconfigreq.InfraContainerID); err != nil { - logger.Errorf("failed to remove HNS endpoint %s", err.Error()) - } - return errors.Wrap(em.cli.ReleaseIPs(ctx, ipconfigreq), "failed to release IP from CNS") -} - -// deleteEndpoint API to get the state and then remove assiciated HNS -func (em *EndpointManager) deleteEndpoint(ctx context.Context, containerid string) error { - endpointResponse, err := em.cli.GetEndpoint(ctx, containerid) - if err != nil { - return errors.Wrap(err, "failed to read the endpoint from CNS state") - } - for _, ipInfo := range endpointResponse.EndpointInfo.IfnameToIPMap { - hnsEndpointID := ipInfo.HnsEndpointID - // we need to get the HNSENdpoint via the IP address if the HNSEndpointID is not present in the statefile - if ipInfo.HnsEndpointID == "" { - if hnsEndpointID, err = hnsclient.GetHNSEndpointbyIP(ipInfo.IPv4, ipInfo.IPv6); err != nil { - return errors.Wrap(err, "failed to find HNS endpoint with id") - } - } - logger.Printf("deleting HNS Endpoint with id %v", hnsEndpointID) - if err := hnsclient.DeleteHNSEndpointbyID(hnsEndpointID); err != nil { - return errors.Wrap(err, "failed to delete HNS endpoint with id "+ipInfo.HnsEndpointID) - } - } - return nil -} From 41938742747ce23d5c40e74e75389789c1a36697 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 14 Jan 2025 15:08:48 -0800 Subject: [PATCH 20/48] re-added back --- cns/endpointmanager/endpointmanager.go | 21 ++++++++++ cns/endpointmanager/endpointmanager_linux.go | 13 ++++++ .../endpointmanager_windows.go | 42 +++++++++++++++++++ 3 files changed, 76 insertions(+) create mode 100644 cns/endpointmanager/endpointmanager.go create mode 100644 cns/endpointmanager/endpointmanager_linux.go create mode 100644 cns/endpointmanager/endpointmanager_windows.go diff --git a/cns/endpointmanager/endpointmanager.go b/cns/endpointmanager/endpointmanager.go new file mode 100644 index 0000000000..e71f0a6cbc --- /dev/null +++ b/cns/endpointmanager/endpointmanager.go @@ -0,0 +1,21 @@ +package endpointmanager + +import ( + "context" + + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/restserver" +) + +type EndpointManager struct { + cli releaseIPsClient // nolint +} + +type releaseIPsClient interface { + ReleaseIPs(ctx context.Context, ipconfig cns.IPConfigsRequest) error + GetEndpoint(ctx context.Context, endpointID string) (*restserver.GetEndpointResponse, error) +} + +func WithPlatformReleaseIPsManager(cli releaseIPsClient) *EndpointManager { + return &EndpointManager{cli: cli} +} diff --git a/cns/endpointmanager/endpointmanager_linux.go b/cns/endpointmanager/endpointmanager_linux.go new file mode 100644 index 0000000000..4441d84110 --- /dev/null +++ b/cns/endpointmanager/endpointmanager_linux.go @@ -0,0 +1,13 @@ +package endpointmanager + +import ( + "context" + + "github.com/Azure/azure-container-networking/cns" + "github.com/pkg/errors" +) + +// ReleaseIPs implements an Interface in fsnotify for async delete of the HNS endpoint and IP addresses +func (em *EndpointManager) ReleaseIPs(ctx context.Context, ipconfigreq cns.IPConfigsRequest) error { + return errors.Wrap(em.cli.ReleaseIPs(ctx, ipconfigreq), "failed to release IP from CNS") +} diff --git a/cns/endpointmanager/endpointmanager_windows.go b/cns/endpointmanager/endpointmanager_windows.go new file mode 100644 index 0000000000..dd928b721d --- /dev/null +++ b/cns/endpointmanager/endpointmanager_windows.go @@ -0,0 +1,42 @@ +package endpointmanager + +import ( + "context" + + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/hnsclient" + "github.com/Azure/azure-container-networking/cns/logger" + "github.com/pkg/errors" +) + +// ReleaseIPs implements an Interface in fsnotify for async delete of the HNS endpoint and IP addresses +func (em *EndpointManager) ReleaseIPs(ctx context.Context, ipconfigreq cns.IPConfigsRequest) error { + logger.Printf("deleting HNS Endpoint asynchronously") + // remove HNS endpoint + if err := em.deleteEndpoint(ctx, ipconfigreq.InfraContainerID); err != nil { + logger.Errorf("failed to remove HNS endpoint %s", err.Error()) + } + return errors.Wrap(em.cli.ReleaseIPs(ctx, ipconfigreq), "failed to release IP from CNS") +} + +// deleteEndpoint API to get the state and then remove assiciated HNS +func (em *EndpointManager) deleteEndpoint(ctx context.Context, containerid string) error { + endpointResponse, err := em.cli.GetEndpoint(ctx, containerid) + if err != nil { + return errors.Wrap(err, "failed to read the endpoint from CNS state") + } + for _, ipInfo := range endpointResponse.EndpointInfo.IfnameToIPMap { + hnsEndpointID := ipInfo.HnsEndpointID + // we need to get the HNSENdpoint via the IP address if the HNSEndpointID is not present in the statefile + if ipInfo.HnsEndpointID == "" { + if hnsEndpointID, err = hnsclient.GetHNSEndpointbyIP(ipInfo.IPv4, ipInfo.IPv6); err != nil { + return errors.Wrap(err, "failed to find HNS endpoint with id") + } + } + logger.Printf("deleting HNS Endpoint with id %v", hnsEndpointID) + if err := hnsclient.DeleteHNSEndpointbyID(hnsEndpointID); err != nil { + return errors.Wrap(err, "failed to delete HNS endpoint with id "+ipInfo.HnsEndpointID) + } + } + return nil +} From 06eb949f79fd14681699e54963d581ee23c5550c Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 14 Jan 2025 21:41:11 -0800 Subject: [PATCH 21/48] updated creating acl code to make it more modularized --- cns/middlewares/k8sSwiftV2.go | 17 +++++-- cns/middlewares/k8sSwiftV2_linux.go | 4 +- cns/middlewares/k8sSwiftV2_windows.go | 54 +++++++++------------- cns/middlewares/k8sSwiftV2_windows_test.go | 51 ++++++++++++++------ 4 files changed, 72 insertions(+), 54 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index fa3b175cae..398e09ab00 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -59,17 +59,26 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa return ipConfigsResp, err } - // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny acl's pn the infra IP configs + // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny endpoint policies as a property in PodIpInfo for i := range ipConfigsResp.PodIPInfo { ipInfo := &ipConfigsResp.PodIPInfo[i] - var defaultDenyEndpointPolicies []policy.Policy // there will be no pod connectivity to and from those pods + var defaultDenyEngressPolicy, defaultDenyIngressPolicy policy.Policy + if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - defaultDenyEndpointPolicies, err = addDefaultDenyACL() + defaultDenyEngressPolicy, err = getEndpointPolicyL(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } - ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEndpointPolicies...) + + defaultDenyIngressPolicy, err = getEndpointPolicyL(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) + if err != nil { + logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) + } + + ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEngressPolicy, defaultDenyIngressPolicy) + logger.Printf("Created endpoint policies for defaultDenyEngressPolicy and defaultDenyIngressPolicy") + break } } diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 709d2d5aff..993e1e57e6 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -105,6 +105,6 @@ func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(_ *cns.PodIpInfo, func (k *K8sSWIFTv2Middleware) addDefaultRoute(*cns.PodIpInfo, string) {} -func addDefaultDenyACL() ([]policy.Policy, error) { - return []policy.Policy{}, nil +func getEndpointPolicyL(_, _, _ string, _ int) (policy.Policy, error) { + return policy.Policy{}, nil } diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index fded3d194c..1d81c312ed 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -7,7 +7,6 @@ import ( "github.com/Azure/azure-container-networking/cns/middlewares/utils" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" "github.com/Azure/azure-container-networking/network/policy" - "github.com/Microsoft/hcsshim/hcn" "github.com/pkg/errors" ) @@ -63,52 +62,41 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st podIPInfo.Routes = append(podIPInfo.Routes, route) } -// append the default deny acl's to the list defaultDenyACL field in podIpInfo -func addDefaultDenyACL() ([]policy.Policy, error) { - blockEgressACL, err := getDefaultDenyACLPolicy(hcn.DirectionTypeOut) +// get policy of type endpoint policy given the params +func getEndpointPolicyL(policyType string, action string, direction string, priority int) (policy.Policy, error) { + endpointPolicy, err := createEndpointPolicy(policyType, action, direction, priority) if err != nil { - return []policy.Policy{}, errors.Wrap(err, "failed to create default deny ACL policy egress") + return policy.Policy{}, errors.Wrap(err, "failed to create endpoint policy") } - blockIngressACL, err := getDefaultDenyACLPolicy(hcn.DirectionTypeIn) - if err != nil { - return []policy.Policy{}, errors.Wrap(err, "Failed to create default deny ACL policy ingress") - } - - additionalArgs := []policy.Policy{ - { - Type: policy.EndpointPolicy, - Data: blockEgressACL, - }, - { - Type: policy.EndpointPolicy, - Data: blockIngressACL, - }, + additionalArgs := policy.Policy{ + Type: policy.EndpointPolicy, + Data: endpointPolicy, } return additionalArgs, nil } -// create the default deny acl's that need to be added to the list defaultDenyACL field in podIpInfo -func getDefaultDenyACLPolicy(direction hcn.DirectionType) ([]byte, error) { - type DefaultDenyACL struct { - Type string `json:"Type"` - Action hcn.ActionType `json:"Action"` - Direction hcn.DirectionType `json:"Direction"` - Priority int `json:"Priority"` +// create policy given the params +func createEndpointPolicy(policyType string, action string, direction string, priority int) ([]byte, error) { + type EndpointPolicy struct { + Type string `json:"Type"` + Action string `json:"Action"` + Direction string `json:"Direction"` + Priority int `json:"Priority"` } - denyACL := DefaultDenyACL{ - Type: "ACL", // policy type is ACL - Action: hcn.ActionTypeBlock, + policy := EndpointPolicy{ + Type: policyType, + Action: action, Direction: direction, - Priority: 10_000, // default deny priority will be 10_000 + Priority: priority, } - denyACLJSON, err := json.Marshal(denyACL) + rawPolicy, err := json.Marshal(policy) if err != nil { - return nil, errors.Wrap(err, "error marshalling default deny policy to json") + return nil, errors.Wrap(err, "error marshalling policy to json") } - return denyACLJSON, nil + return rawPolicy, nil } diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index bbf7bd4528..a1e898d199 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -2,6 +2,7 @@ package middlewares import ( "encoding/json" + "fmt" "reflect" "testing" @@ -105,19 +106,35 @@ func TestAddDefaultRoute(t *testing.T) { } func TestAddDefaultDenyACL(t *testing.T) { - valueOut := []byte(`{ - "Type": "ACL", - "Action": "Block", - "Direction": "Out", - "Priority": 10000 - }`) - - valueIn := []byte(`{ - "Type": "ACL", - "Action": "Block", - "Direction": "In", - "Priority": 10000 - }`) + const policyType = "ACL" + const action = "Block" + const ingressDir = "In" + const egressDir = "Out" + const priority = 10000 + + valueIn := []byte(fmt.Sprintf(`{ + "Type": "%s", + "Action": "%s", + "Direction": "%s", + "Priority": %d + }`, + policyType, + action, + ingressDir, + priority, + )) + + valueOut := []byte(fmt.Sprintf(`{ + "Type": "%s", + "Action": "%s", + "Direction": "%s", + "Priority": %d + }`, + policyType, + action, + egressDir, + priority, + )) expectedDefaultDenyEndpoint := []policy.Policy{ { @@ -129,13 +146,17 @@ func TestAddDefaultDenyACL(t *testing.T) { Data: valueIn, }, } + var allEndpoints []policy.Policy - defaultDenyEndpoint, err := addDefaultDenyACL() + defaultDenyEngressPolicy, err := getEndpointPolicyL("ACL", "Block", "Out", 10000) + defaultDenyIngressPolicy, err := getEndpointPolicyL("ACL", "Block", "In", 10000) + + allEndpoints = append(allEndpoints, defaultDenyEngressPolicy, defaultDenyIngressPolicy) assert.Equal(t, err, nil) // Normalize both slices so there is no extra spacing, new lines, etc normalizedExpected := normalizeKVPairs(t, expectedDefaultDenyEndpoint) - normalizedActual := normalizeKVPairs(t, defaultDenyEndpoint) + normalizedActual := normalizeKVPairs(t, allEndpoints) if !reflect.DeepEqual(normalizedExpected, normalizedActual) { t.Errorf("got '%+v', expected '%+v'", normalizedActual, normalizedExpected) } From f88932c6d1323517cbae9e710a4eede85fd50d77 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 14 Jan 2025 21:51:19 -0800 Subject: [PATCH 22/48] fixed golint errors --- cns/middlewares/k8sSwiftV2_windows.go | 6 +++--- cns/middlewares/k8sSwiftV2_windows_test.go | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 1d81c312ed..87b9592675 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -63,7 +63,7 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st } // get policy of type endpoint policy given the params -func getEndpointPolicyL(policyType string, action string, direction string, priority int) (policy.Policy, error) { +func getEndpointPolicyL(policyType, action, direction string, priority int) (policy.Policy, error) { endpointPolicy, err := createEndpointPolicy(policyType, action, direction, priority) if err != nil { return policy.Policy{}, errors.Wrap(err, "failed to create endpoint policy") @@ -86,14 +86,14 @@ func createEndpointPolicy(policyType string, action string, direction string, pr Priority int `json:"Priority"` } - policy := EndpointPolicy{ + endpointPolicy := EndpointPolicy{ Type: policyType, Action: action, Direction: direction, Priority: priority, } - rawPolicy, err := json.Marshal(policy) + rawPolicy, err := json.Marshal(endpointPolicy) if err != nil { return nil, errors.Wrap(err, "error marshalling policy to json") } diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index a1e898d199..cdc05a2d51 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -147,9 +147,10 @@ func TestAddDefaultDenyACL(t *testing.T) { }, } var allEndpoints []policy.Policy - - defaultDenyEngressPolicy, err := getEndpointPolicyL("ACL", "Block", "Out", 10000) - defaultDenyIngressPolicy, err := getEndpointPolicyL("ACL", "Block", "In", 10000) + var defaultDenyEngressPolicy, defaultDenyIngressPolicy policy.Policy + var err error + defaultDenyEngressPolicy, err = getEndpointPolicyL("ACL", "Block", "Out", 10000) + defaultDenyIngressPolicy, err = getEndpointPolicyL("ACL", "Block", "In", 10000) allEndpoints = append(allEndpoints, defaultDenyEngressPolicy, defaultDenyIngressPolicy) assert.Equal(t, err, nil) From 5b19657385aa6c0a70a7458fbc6778150c23aecf Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 14 Jan 2025 22:08:45 -0800 Subject: [PATCH 23/48] fixed golint --- cns/middlewares/k8sSwiftV2.go | 4 ++-- cns/middlewares/k8sSwiftV2_linux.go | 2 +- cns/middlewares/k8sSwiftV2_windows.go | 4 ++-- cns/middlewares/k8sSwiftV2_windows_test.go | 10 ++++++++-- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 398e09ab00..60c802a610 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -66,12 +66,12 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa var defaultDenyEngressPolicy, defaultDenyIngressPolicy policy.Policy if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - defaultDenyEngressPolicy, err = getEndpointPolicyL(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) + defaultDenyEngressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } - defaultDenyIngressPolicy, err = getEndpointPolicyL(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) + defaultDenyIngressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 993e1e57e6..646c3bbd93 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -105,6 +105,6 @@ func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(_ *cns.PodIpInfo, func (k *K8sSWIFTv2Middleware) addDefaultRoute(*cns.PodIpInfo, string) {} -func getEndpointPolicyL(_, _, _ string, _ int) (policy.Policy, error) { +func getEndpointPolicy(_, _, _ string, _ int) (policy.Policy, error) { return policy.Policy{}, nil } diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 87b9592675..3d56987083 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -63,7 +63,7 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st } // get policy of type endpoint policy given the params -func getEndpointPolicyL(policyType, action, direction string, priority int) (policy.Policy, error) { +func getEndpointPolicy(policyType, action, direction string, priority int) (policy.Policy, error) { endpointPolicy, err := createEndpointPolicy(policyType, action, direction, priority) if err != nil { return policy.Policy{}, errors.Wrap(err, "failed to create endpoint policy") @@ -78,7 +78,7 @@ func getEndpointPolicyL(policyType, action, direction string, priority int) (pol } // create policy given the params -func createEndpointPolicy(policyType string, action string, direction string, priority int) ([]byte, error) { +func createEndpointPolicy(policyType, action, direction string, priority int) ([]byte, error) { type EndpointPolicy struct { Type string `json:"Type"` Action string `json:"Action"` diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index cdc05a2d51..c9e571698f 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -149,8 +149,14 @@ func TestAddDefaultDenyACL(t *testing.T) { var allEndpoints []policy.Policy var defaultDenyEngressPolicy, defaultDenyIngressPolicy policy.Policy var err error - defaultDenyEngressPolicy, err = getEndpointPolicyL("ACL", "Block", "Out", 10000) - defaultDenyIngressPolicy, err = getEndpointPolicyL("ACL", "Block", "In", 10000) + defaultDenyEngressPolicy, err = getEndpointPolicy("ACL", "Block", "Out", 10_000) + if err != nil { + fmt.Printf("failed to create endpoint policy") + } + defaultDenyIngressPolicy, err = getEndpointPolicy("ACL", "Block", "In", 10_000) + if err != nil { + fmt.Printf("failed to create endpoint policy") + } allEndpoints = append(allEndpoints, defaultDenyEngressPolicy, defaultDenyIngressPolicy) assert.Equal(t, err, nil) From f347d49f1d76f92459cfe227f11cd4ba352c2418 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 14 Jan 2025 22:35:36 -0800 Subject: [PATCH 24/48] added tests --- cns/middlewares/k8sSwiftV2_linux_test.go | 6 +++--- cns/middlewares/k8sSwiftV2_windows_test.go | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_linux_test.go b/cns/middlewares/k8sSwiftV2_linux_test.go index 8bd0990728..2c7f498ff2 100644 --- a/cns/middlewares/k8sSwiftV2_linux_test.go +++ b/cns/middlewares/k8sSwiftV2_linux_test.go @@ -144,7 +144,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq.OrchestratorContext = b happyReq.SecondaryInterfacesExist = false - _, respCode, err, _ := middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq) + _, respCode, _, err := middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq.SecondaryInterfacesExist, true) @@ -158,7 +158,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq2.OrchestratorContext = b happyReq2.SecondaryInterfacesExist = false - _, respCode, err, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq2) + _, respCode, _, err = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq2) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq.SecondaryInterfacesExist, true) @@ -172,7 +172,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq3.OrchestratorContext = b happyReq3.SecondaryInterfacesExist = false - _, respCode, err, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq3) + _, respCode, _, err = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq3) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq3.SecondaryInterfacesExist, false) diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index c9e571698f..59c612994f 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -149,6 +149,7 @@ func TestAddDefaultDenyACL(t *testing.T) { var allEndpoints []policy.Policy var defaultDenyEngressPolicy, defaultDenyIngressPolicy policy.Policy var err error + defaultDenyEngressPolicy, err = getEndpointPolicy("ACL", "Block", "Out", 10_000) if err != nil { fmt.Printf("failed to create endpoint policy") From 6543abee9cd1fdee0265a238d9423bd7fc5c5f75 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 14 Jan 2025 23:43:05 -0800 Subject: [PATCH 25/48] fixed spelling --- cns/middlewares/k8sSwiftV2.go | 8 ++++---- cns/middlewares/k8sSwiftV2_windows_test.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 60c802a610..5c7134fe4b 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -63,10 +63,10 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa for i := range ipConfigsResp.PodIPInfo { ipInfo := &ipConfigsResp.PodIPInfo[i] // there will be no pod connectivity to and from those pods - var defaultDenyEngressPolicy, defaultDenyIngressPolicy policy.Policy + var defaultDenyEgressPolicy, defaultDenyIngressPolicy policy.Policy if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - defaultDenyEngressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) + defaultDenyEgressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } @@ -76,8 +76,8 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } - ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEngressPolicy, defaultDenyIngressPolicy) - logger.Printf("Created endpoint policies for defaultDenyEngressPolicy and defaultDenyIngressPolicy") + ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEgressPolicy, defaultDenyIngressPolicy) + logger.Printf("Created endpoint policies for defaultDenyEgressPolicy and defaultDenyIngressPolicy") break } diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index 59c612994f..d31566c126 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -147,10 +147,10 @@ func TestAddDefaultDenyACL(t *testing.T) { }, } var allEndpoints []policy.Policy - var defaultDenyEngressPolicy, defaultDenyIngressPolicy policy.Policy + var defaultDenyEgressPolicy, defaultDenyIngressPolicy policy.Policy var err error - defaultDenyEngressPolicy, err = getEndpointPolicy("ACL", "Block", "Out", 10_000) + defaultDenyEgressPolicy, err = getEndpointPolicy("ACL", "Block", "Out", 10_000) if err != nil { fmt.Printf("failed to create endpoint policy") } @@ -159,7 +159,7 @@ func TestAddDefaultDenyACL(t *testing.T) { fmt.Printf("failed to create endpoint policy") } - allEndpoints = append(allEndpoints, defaultDenyEngressPolicy, defaultDenyIngressPolicy) + allEndpoints = append(allEndpoints, defaultDenyEgressPolicy, defaultDenyIngressPolicy) assert.Equal(t, err, nil) // Normalize both slices so there is no extra spacing, new lines, etc From 1b969d508905a1b59b2379cf4bcd6042ff1de7dc Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 15 Jan 2025 09:55:58 -0800 Subject: [PATCH 26/48] moved an assertion line --- cns/middlewares/k8sSwiftV2_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index d31566c126..5d03734007 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -160,7 +160,6 @@ func TestAddDefaultDenyACL(t *testing.T) { } allEndpoints = append(allEndpoints, defaultDenyEgressPolicy, defaultDenyIngressPolicy) - assert.Equal(t, err, nil) // Normalize both slices so there is no extra spacing, new lines, etc normalizedExpected := normalizeKVPairs(t, expectedDefaultDenyEndpoint) @@ -168,6 +167,7 @@ func TestAddDefaultDenyACL(t *testing.T) { if !reflect.DeepEqual(normalizedExpected, normalizedActual) { t.Errorf("got '%+v', expected '%+v'", normalizedActual, normalizedExpected) } + assert.Equal(t, err, nil) } // normalizeKVPairs normalizes the JSON values in the KV pairs by unmarshaling them into a map, then marshaling them back to compact JSON to remove any extra space, new lines, etc From 512b25847b81870ac07d4a55a069d830c8a7ed63 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Wed, 15 Jan 2025 14:22:18 -0800 Subject: [PATCH 27/48] reformated creating acl's --- cns/middlewares/k8sSwiftV2_windows.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 3d56987083..e9562e8c61 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -79,14 +79,12 @@ func getEndpointPolicy(policyType, action, direction string, priority int) (poli // create policy given the params func createEndpointPolicy(policyType, action, direction string, priority int) ([]byte, error) { - type EndpointPolicy struct { + endpointPolicy := struct { Type string `json:"Type"` Action string `json:"Action"` Direction string `json:"Direction"` Priority int `json:"Priority"` - } - - endpointPolicy := EndpointPolicy{ + }{ Type: policyType, Action: action, Direction: direction, From b29454d26b86497531e25a6067a2505c022e5de2 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 16 Jan 2025 16:00:00 -0800 Subject: [PATCH 28/48] refactored code per pr comments --- cns/middlewares/k8sSwiftV2.go | 234 +++++++++-------------- cns/middlewares/k8sSwiftV2_linux.go | 67 ++++++- cns/middlewares/k8sSwiftV2_linux_test.go | 14 +- cns/middlewares/k8sSwiftV2_windows.go | 119 ++++++++++++ 4 files changed, 279 insertions(+), 155 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 5c7134fe4b..a064ff53ee 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -10,7 +10,6 @@ import ( "github.com/Azure/azure-container-networking/cns/middlewares/utils" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" - "github.com/Azure/azure-container-networking/network/policy" "github.com/pkg/errors" v1 "k8s.io/api/core/v1" k8stypes "k8s.io/apimachinery/pkg/types" @@ -37,158 +36,32 @@ type K8sSWIFTv2Middleware struct { // Verify interface compliance at compile time var _ cns.IPConfigsHandlerMiddleware = (*K8sSWIFTv2Middleware)(nil) -// IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request -// and release IP configs handlers. -func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { - return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, respCode, defaultDenyACLbool, message := k.GetPodInfoForIPConfigsRequest(ctx, &req) +func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string) { + // gets pod info for the specified request + podInfo, pod, respCode, message := k.GetPodInfo(ctx, req) + if respCode != types.Success { + return nil, respCode, message + } - logger.Printf("defaultDenyACLbool value is: %v", defaultDenyACLbool) + // validates if pod is swiftv2 + isSwiftv2 := ValidateSwiftv2Pod(pod) + var mtpnc v1alpha1.MultitenantPodNetworkConfig + // if swiftv2 is enabled, check if mtpnc is ready + if isSwiftv2 { + mtpnc, respCode, message = k.getMTPNC(ctx, podInfo) if respCode != types.Success { - return &cns.IPConfigsResponse{ - Response: cns.Response{ - ReturnCode: respCode, - Message: message, - }, - }, errors.New("failed to validate IP configs request") - } - ipConfigsResp, err := defaultHandler(ctx, req) - // If the pod is not v2, return the response from the handler - if !req.SecondaryInterfacesExist { - return ipConfigsResp, err - } - - // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny endpoint policies as a property in PodIpInfo - for i := range ipConfigsResp.PodIPInfo { - ipInfo := &ipConfigsResp.PodIPInfo[i] - // there will be no pod connectivity to and from those pods - var defaultDenyEgressPolicy, defaultDenyIngressPolicy policy.Policy - - if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - defaultDenyEgressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) - if err != nil { - logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) - } - - defaultDenyIngressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) - if err != nil { - logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) - } - - ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEgressPolicy, defaultDenyIngressPolicy) - logger.Printf("Created endpoint policies for defaultDenyEgressPolicy and defaultDenyIngressPolicy") - - break - } + return nil, respCode, message } - // If the pod is v2, get the infra IP configs from the handler first and then add the SWIFTv2 IP config - defer func() { - // Release the default IP config if there is an error - if err != nil { - _, err = failureHandler(ctx, req) - if err != nil { - logger.Errorf("failed to release default IP config : %v", err) - } - } - }() - if err != nil { - return ipConfigsResp, err - } - SWIFTv2PodIPInfos, err := k.getIPConfig(ctx, podInfo) - if err != nil { - return &cns.IPConfigsResponse{ - Response: cns.Response{ - ReturnCode: types.FailedToAllocateIPConfig, - Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, req), - }, - PodIPInfo: []cns.PodIpInfo{}, - }, errors.Wrapf(err, "failed to get SWIFTv2 IP config : %v", req) - } - ipConfigsResp.PodIPInfo = append(ipConfigsResp.PodIPInfo, SWIFTv2PodIPInfos...) - // Set routes for the pod - for i := range ipConfigsResp.PodIPInfo { - ipInfo := &ipConfigsResp.PodIPInfo[i] - // Backend nics doesn't need routes to be set - if ipInfo.NICType != cns.BackendNIC { - err = k.setRoutes(ipInfo) - if err != nil { - return &cns.IPConfigsResponse{ - Response: cns.Response{ - ReturnCode: types.FailedToAllocateIPConfig, - Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, req), - }, - PodIPInfo: []cns.PodIpInfo{}, - }, errors.Wrapf(err, "failed to set routes for pod %s", podInfo.Name()) - } - } + // update ipConfigRequest + respCode, message = k.UpdateIPConfigRequest(mtpnc, podInfo, req) + if respCode != types.Success { + return nil, respCode, message } - return ipConfigsResp, nil - } -} - -// GetPodInfoForIPConfigsRequest validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. -// nolint -func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, defaultDenyACL bool, message string) { - defaultDenyACLbool := false - - // Retrieve the pod from the cluster - podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) - if err != nil { - errBuf := errors.Wrapf(err, "failed to unmarshalling pod info from ipconfigs request %+v", req) - return nil, types.UnexpectedError, defaultDenyACLbool, errBuf.Error() - } - logger.Printf("[SWIFTv2Middleware] validate ipconfigs request for pod %s", podInfo.Name()) - podNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()} - pod := v1.Pod{} - if err := k.Cli.Get(ctx, podNamespacedName, &pod); err != nil { - errBuf := errors.Wrapf(err, "failed to get pod %+v", podNamespacedName) - return nil, types.UnexpectedError, defaultDenyACLbool, errBuf.Error() } - // check the pod labels for Swift V2, set the request's SecondaryInterfaceSet flag to true and check if its MTPNC CRD is ready - _, swiftV2PodNetworkLabel := pod.Labels[configuration.LabelPodSwiftV2] - _, swiftV2PodNetworkInstanceLabel := pod.Labels[configuration.LabelPodNetworkInstanceSwiftV2] - if swiftV2PodNetworkLabel || swiftV2PodNetworkInstanceLabel { - - // Check if the MTPNC CRD exists for the pod, if not, return error - mtpnc := v1alpha1.MultitenantPodNetworkConfig{} - mtpncNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()} - if err := k.Cli.Get(ctx, mtpncNamespacedName, &mtpnc); err != nil { - return nil, types.UnexpectedError, defaultDenyACLbool, fmt.Errorf("failed to get pod's mtpnc from cache : %w", err).Error() - } - // Check if the MTPNC CRD is ready. If one of the fields is empty, return error - if !mtpnc.IsReady() { - return nil, types.UnexpectedError, defaultDenyACLbool, errMTPNCNotReady.Error() - } - - // copying defaultDenyACL bool from mtpnc - defaultDenyACLbool = mtpnc.Status.DefaultDenyACL - - // If primary Ip is set in status field, it indicates the presence of secondary interfaces - if mtpnc.Status.PrimaryIP != "" { - req.SecondaryInterfacesExist = true - } - interfaceInfos := mtpnc.Status.InterfaceInfos - for _, interfaceInfo := range interfaceInfos { - if interfaceInfo.DeviceType == v1alpha1.DeviceTypeInfiniBandNIC { - if interfaceInfo.MacAddress == "" || interfaceInfo.NCID == "" { - return nil, types.UnexpectedError, defaultDenyACLbool, errMTPNCNotReady.Error() - } - req.BackendInterfaceExist = true - req.BackendInterfaceMacAddresses = append(req.BackendInterfaceMacAddresses, interfaceInfo.MacAddress) - - } - if interfaceInfo.DeviceType == v1alpha1.DeviceTypeVnetNIC { - req.SecondaryInterfacesExist = true - } - } - } - logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist) - logger.Printf("[SWIFTv2Middleware] pod %s has backend interface : %v", podInfo.Name(), req.BackendInterfaceExist) - // retrieve podinfo from orchestrator context - return podInfo, types.Success, defaultDenyACLbool, "" + return podInfo, types.Success, "" } // getIPConfig returns the pod's SWIFT V2 IP configuration. @@ -283,3 +156,74 @@ func (k *K8sSWIFTv2Middleware) getIPConfig(ctx context.Context, podInfo cns.PodI func (k *K8sSWIFTv2Middleware) Type() cns.SWIFTV2Mode { return cns.K8sSWIFTV2 } + +// gets Pod Data +func (k *K8sSWIFTv2Middleware) GetPodInfo(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, k8sPod v1.Pod, respCode types.ResponseCode, message string) { + // Retrieve the pod from the cluster + podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) + if err != nil { + errBuf := errors.Wrapf(err, "failed to unmarshalling pod info from ipconfigs request %+v", req) + return nil, v1.Pod{}, types.UnexpectedError, errBuf.Error() + } + logger.Printf("[SWIFTv2Middleware] validate ipconfigs request for pod %s", podInfo.Name()) + podNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()} + pod := v1.Pod{} + if err := k.Cli.Get(ctx, podNamespacedName, &pod); err != nil { + errBuf := errors.Wrapf(err, "failed to get pod %+v", podNamespacedName) + return nil, v1.Pod{}, types.UnexpectedError, errBuf.Error() + } + return podInfo, pod, types.Success, "" +} + +// validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. +func ValidateSwiftv2Pod(pod v1.Pod) bool { + // check the pod labels for Swift V2, set the request's SecondaryInterfaceSet flag to true and check if its MTPNC CRD is ready + _, swiftV2PodNetworkLabel := pod.Labels[configuration.LabelPodSwiftV2] + _, swiftV2PodNetworkInstanceLabel := pod.Labels[configuration.LabelPodNetworkInstanceSwiftV2] + // check if mtpnc is nil here, if not nil then proceed + if swiftV2PodNetworkLabel || swiftV2PodNetworkInstanceLabel { + return true + } + return false +} + +// checks if MTPNC is ready +func (k *K8sSWIFTv2Middleware) getMTPNC(ctx context.Context, podInfo cns.PodInfo) (mtpncResource v1alpha1.MultitenantPodNetworkConfig, respCode types.ResponseCode, message string) { + // Check if the MTPNC CRD exists for the pod, if not, return error + mtpnc := v1alpha1.MultitenantPodNetworkConfig{} + mtpncNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()} + if err := k.Cli.Get(ctx, mtpncNamespacedName, &mtpnc); err != nil { + return v1alpha1.MultitenantPodNetworkConfig{}, types.UnexpectedError, fmt.Errorf("failed to get pod's mtpnc from cache : %w", err).Error() + } + // Check if the MTPNC CRD is ready. If one of the fields is empty, return error + if !mtpnc.IsReady() { + return v1alpha1.MultitenantPodNetworkConfig{}, types.UnexpectedError, errMTPNCNotReady.Error() + } + return mtpnc, types.Success, "" +} + +// Updates Ip Config Request +func (k *K8sSWIFTv2Middleware) UpdateIPConfigRequest(mtpnc v1alpha1.MultitenantPodNetworkConfig, podInfo cns.PodInfo, req *cns.IPConfigsRequest) ( + respCode types.ResponseCode, + message string, +) { + interfaceInfos := mtpnc.Status.InterfaceInfos + for _, interfaceInfo := range interfaceInfos { + if interfaceInfo.DeviceType == v1alpha1.DeviceTypeInfiniBandNIC { + if interfaceInfo.MacAddress == "" || interfaceInfo.NCID == "" { + return types.UnexpectedError, errMTPNCNotReady.Error() + } + req.BackendInterfaceExist = true + req.BackendInterfaceMacAddresses = append(req.BackendInterfaceMacAddresses, interfaceInfo.MacAddress) + + } + if interfaceInfo.DeviceType == v1alpha1.DeviceTypeVnetNIC { + req.SecondaryInterfacesExist = true + } + } + + logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist) + logger.Printf("[SWIFTv2Middleware] pod %s has backend interface : %v", podInfo.Name(), req.BackendInterfaceExist) + + return types.Success, "" +} diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 646c3bbd93..b693925f0f 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -1,6 +1,7 @@ package middlewares import ( + "context" "fmt" "net/netip" @@ -8,8 +9,8 @@ import ( "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/middlewares/utils" + "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" - "github.com/Azure/azure-container-networking/network/policy" "github.com/pkg/errors" ) @@ -105,6 +106,66 @@ func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(_ *cns.PodIpInfo, func (k *K8sSWIFTv2Middleware) addDefaultRoute(*cns.PodIpInfo, string) {} -func getEndpointPolicy(_, _, _ string, _ int) (policy.Policy, error) { - return policy.Policy{}, nil +// IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request +// and release IP configs handlers. +func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { + return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + podInfo, respCode, message := k.GetPodInfoForIPConfigsRequest(ctx, &req) + + if respCode != types.Success { + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: respCode, + Message: message, + }, + }, errors.New("failed to validate IP configs request") + } + ipConfigsResp, err := defaultHandler(ctx, req) + // If the pod is not v2, return the response from the handler + if !req.SecondaryInterfacesExist { + return ipConfigsResp, err + } + // If the pod is v2, get the infra IP configs from the handler first and then add the SWIFTv2 IP config + defer func() { + // Release the default IP config if there is an error + if err != nil { + _, err = failureHandler(ctx, req) + if err != nil { + logger.Errorf("failed to release default IP config : %v", err) + } + } + }() + if err != nil { + return ipConfigsResp, err + } + SWIFTv2PodIPInfos, err := k.getIPConfig(ctx, podInfo) + if err != nil { + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.FailedToAllocateIPConfig, + Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, req), + }, + PodIPInfo: []cns.PodIpInfo{}, + }, errors.Wrapf(err, "failed to get SWIFTv2 IP config : %v", req) + } + ipConfigsResp.PodIPInfo = append(ipConfigsResp.PodIPInfo, SWIFTv2PodIPInfos...) + // Set routes for the pod + for i := range ipConfigsResp.PodIPInfo { + ipInfo := &ipConfigsResp.PodIPInfo[i] + // Backend nics doesn't need routes to be set + if ipInfo.NICType != cns.BackendNIC { + err = k.setRoutes(ipInfo) + if err != nil { + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.FailedToAllocateIPConfig, + Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, req), + }, + PodIPInfo: []cns.PodIpInfo{}, + }, errors.Wrapf(err, "failed to set routes for pod %s", podInfo.Name()) + } + } + } + return ipConfigsResp, nil + } } diff --git a/cns/middlewares/k8sSwiftV2_linux_test.go b/cns/middlewares/k8sSwiftV2_linux_test.go index 2c7f498ff2..0ff125fa1f 100644 --- a/cns/middlewares/k8sSwiftV2_linux_test.go +++ b/cns/middlewares/k8sSwiftV2_linux_test.go @@ -144,7 +144,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq.OrchestratorContext = b happyReq.SecondaryInterfacesExist = false - _, respCode, _, err := middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq) + _, respCode, err := middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq.SecondaryInterfacesExist, true) @@ -158,7 +158,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq2.OrchestratorContext = b happyReq2.SecondaryInterfacesExist = false - _, respCode, _, err = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq2) + _, respCode, err = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq2) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq.SecondaryInterfacesExist, true) @@ -172,7 +172,7 @@ func TestValidateMultitenantIPConfigsRequestSuccess(t *testing.T) { happyReq3.OrchestratorContext = b happyReq3.SecondaryInterfacesExist = false - _, respCode, _, err = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq3) + _, respCode, err = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), happyReq3) assert.Equal(t, err, "") assert.Equal(t, respCode, types.Success) assert.Equal(t, happyReq3.SecondaryInterfacesExist, false) @@ -188,7 +188,7 @@ func TestValidateMultitenantIPConfigsRequestFailure(t *testing.T) { InfraContainerID: testPod1Info.InfraContainerID(), } failReq.OrchestratorContext = []byte("invalid") - _, respCode, _, _ := middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) + _, respCode, _ := middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) // Pod doesn't exist in cache test @@ -198,19 +198,19 @@ func TestValidateMultitenantIPConfigsRequestFailure(t *testing.T) { } b, _ := testPod2Info.OrchestratorContext() failReq.OrchestratorContext = b - _, respCode, _, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) + _, respCode, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) // Failed to get MTPNC b, _ = testPod3Info.OrchestratorContext() failReq.OrchestratorContext = b - _, respCode, _, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) + _, respCode, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) // MTPNC not ready b, _ = testPod4Info.OrchestratorContext() failReq.OrchestratorContext = b - _, respCode, _, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) + _, respCode, _ = middleware.GetPodInfoForIPConfigsRequest(context.TODO(), failReq) assert.Equal(t, respCode, types.UnexpectedError) } diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index e9562e8c61..dd27a3fcc6 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -1,10 +1,14 @@ package middlewares import ( + "context" "encoding/json" + "fmt" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/middlewares/utils" + "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" "github.com/Azure/azure-container-networking/network/policy" "github.com/pkg/errors" @@ -98,3 +102,118 @@ func createEndpointPolicy(policyType, action, direction string, priority int) ([ return rawPolicy, nil } + +// IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request +// and release IP configs handlers. +func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { + return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + + podInfo, respCode, message := k.GetPodInfoForIPConfigsRequest(ctx, &req) + + if respCode != types.Success { + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: respCode, + Message: message, + }, + }, errors.New("failed to validate IP configs request") + } + ipConfigsResp, err := defaultHandler(ctx, req) + // If the pod is not v2, return the response from the handler + if !req.SecondaryInterfacesExist { + return ipConfigsResp, err + } + + // Get MTPNC + mtpnc, respCode, message := k.getMTPNC(ctx, podInfo) + if respCode != types.Success { + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: respCode, + Message: message, + }, + }, errors.New("failed to validate IP configs request") + } + + // checks for default deny bool variable + defaultDenyACLbool, err := GetDefaultDenyBool(mtpnc) + + // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny endpoint policies as a property in PodIpInfo + for i := range ipConfigsResp.PodIPInfo { + ipInfo := &ipConfigsResp.PodIPInfo[i] + // there will be no pod connectivity to and from those pods + var defaultDenyEgressPolicy, defaultDenyIngressPolicy policy.Policy + + if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { + defaultDenyEgressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) + if err != nil { + logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) + } + + defaultDenyIngressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) + if err != nil { + logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) + } + + ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEgressPolicy, defaultDenyIngressPolicy) + logger.Printf("Created endpoint policies for defaultDenyEgressPolicy and defaultDenyIngressPolicy") + + break + } + } + + // If the pod is v2, get the infra IP configs from the handler first and then add the SWIFTv2 IP config + defer func() { + // Release the default IP config if there is an error + if err != nil { + _, err = failureHandler(ctx, req) + if err != nil { + logger.Errorf("failed to release default IP config : %v", err) + } + } + }() + if err != nil { + return ipConfigsResp, err + } + SWIFTv2PodIPInfos, err := k.getIPConfig(ctx, podInfo) + if err != nil { + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.FailedToAllocateIPConfig, + Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, req), + }, + PodIPInfo: []cns.PodIpInfo{}, + }, errors.Wrapf(err, "failed to get SWIFTv2 IP config : %v", req) + } + ipConfigsResp.PodIPInfo = append(ipConfigsResp.PodIPInfo, SWIFTv2PodIPInfos...) + // Set routes for the pod + for i := range ipConfigsResp.PodIPInfo { + ipInfo := &ipConfigsResp.PodIPInfo[i] + // Backend nics doesn't need routes to be set + if ipInfo.NICType != cns.BackendNIC { + err = k.setRoutes(ipInfo) + if err != nil { + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.FailedToAllocateIPConfig, + Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, req), + }, + PodIPInfo: []cns.PodIpInfo{}, + }, errors.Wrapf(err, "failed to set routes for pod %s", podInfo.Name()) + } + } + } + return ipConfigsResp, nil + } +} + +func GetDefaultDenyBool(mtpnc v1alpha1.MultitenantPodNetworkConfig) (bool, error) { + + // Check if the MTPNC CRD is ready. If one of the fields is empty, return error + if !mtpnc.IsReady() { + return false, errMTPNCNotReady + } + + // returns the value of DefaultDenyACL from mtpnc + return mtpnc.Status.DefaultDenyACL, nil +} From fbc02b362a60350dc2597610f608d436f8696b13 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 17 Jan 2025 10:10:41 -0800 Subject: [PATCH 29/48] fixed lint --- cns/middlewares/k8sSwiftV2_windows.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index dd27a3fcc6..fe328a02fa 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -67,8 +67,8 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st } // get policy of type endpoint policy given the params -func getEndpointPolicy(policyType, action, direction string, priority int) (policy.Policy, error) { - endpointPolicy, err := createEndpointPolicy(policyType, action, direction, priority) +func getEndpointPolicy(policyType policy.CNIPolicyType, action, direction string, priority int) (policy.Policy, error) { + endpointPolicy, err := createEndpointPolicy(string(policyType), action, direction, priority) if err != nil { return policy.Policy{}, errors.Wrap(err, "failed to create endpoint policy") } @@ -145,12 +145,12 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa var defaultDenyEgressPolicy, defaultDenyIngressPolicy policy.Policy if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - defaultDenyEgressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) + defaultDenyEgressPolicy, err = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } - defaultDenyIngressPolicy, err = getEndpointPolicy(string(policy.ACLPolicy), cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) + defaultDenyIngressPolicy, err = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) if err != nil { logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) } From 4eeea48b2367d79ecb338d5456676360c91200c6 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 17 Jan 2025 12:12:47 -0800 Subject: [PATCH 30/48] moved GetEndpointPolicy so that it is only run on init --- cns/middlewares/k8sSwiftV2_windows.go | 33 ++++++++++++--------------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index fe328a02fa..e6e178cc50 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -14,6 +14,21 @@ import ( "github.com/pkg/errors" ) +var defaultDenyEgressPolicy policy.Policy +var defaultDenyIngressPolicy policy.Policy +var err error + +func init() { + defaultDenyEgressPolicy, err = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) + if err != nil { + logger.Errorf("failed to add default deny egress acl's for pod with err %v", err) + } + defaultDenyIngressPolicy, err = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) + if err != nil { + logger.Errorf("failed to add default deny ingress acl's for pod with err %v", err) + } +} + // for AKS L1VH, do not set default route on infraNIC to avoid customer pod reaching all infra vnet services // default route is set for secondary interface NIC(i.e,delegatedNIC) func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { @@ -142,19 +157,7 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa for i := range ipConfigsResp.PodIPInfo { ipInfo := &ipConfigsResp.PodIPInfo[i] // there will be no pod connectivity to and from those pods - var defaultDenyEgressPolicy, defaultDenyIngressPolicy policy.Policy - if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { - defaultDenyEgressPolicy, err = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) - if err != nil { - logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) - } - - defaultDenyIngressPolicy, err = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) - if err != nil { - logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err) - } - ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEgressPolicy, defaultDenyIngressPolicy) logger.Printf("Created endpoint policies for defaultDenyEgressPolicy and defaultDenyIngressPolicy") @@ -208,12 +211,6 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } func GetDefaultDenyBool(mtpnc v1alpha1.MultitenantPodNetworkConfig) (bool, error) { - - // Check if the MTPNC CRD is ready. If one of the fields is empty, return error - if !mtpnc.IsReady() { - return false, errMTPNCNotReady - } - // returns the value of DefaultDenyACL from mtpnc return mtpnc.Status.DefaultDenyACL, nil } From 870f7097929a9d046a2b66d06c64af6cec024650 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 17 Jan 2025 12:20:57 -0800 Subject: [PATCH 31/48] updated code --- cns/middlewares/k8sSwiftV2_windows.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index e6e178cc50..67c5f42117 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -16,7 +16,8 @@ import ( var defaultDenyEgressPolicy policy.Policy var defaultDenyIngressPolicy policy.Policy -var err error +var errIngress error +var errEgress error func init() { defaultDenyEgressPolicy, err = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) @@ -159,7 +160,11 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa // there will be no pod connectivity to and from those pods if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEgressPolicy, defaultDenyIngressPolicy) - logger.Printf("Created endpoint policies for defaultDenyEgressPolicy and defaultDenyIngressPolicy") + if errEgress != nil || errIngress != nil { + logger.Printf("There was an error creating endpoint policies for defaultDeny policies") + } else { + logger.Printf("Successfully created endpoint policies for defaultDenyEgressPolicy and defaultDenyIngressPolicy") + } break } From f26cdd4fa2f731cf87b3e575dc19397c31fedbff Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 17 Jan 2025 12:23:32 -0800 Subject: [PATCH 32/48] updated error message --- cns/middlewares/k8sSwiftV2_windows.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 67c5f42117..5cfefc07de 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -20,13 +20,13 @@ var errIngress error var errEgress error func init() { - defaultDenyEgressPolicy, err = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) - if err != nil { - logger.Errorf("failed to add default deny egress acl's for pod with err %v", err) + defaultDenyEgressPolicy, errIngress = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) + if errIngress != nil { + logger.Errorf("failed to add default deny egress acl's for pod with err %v", errIngress) } - defaultDenyIngressPolicy, err = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) - if err != nil { - logger.Errorf("failed to add default deny ingress acl's for pod with err %v", err) + defaultDenyIngressPolicy, errEgress = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) + if errEgress != nil { + logger.Errorf("failed to add default deny ingress acl's for pod with err %v", errEgress) } } From 61c48625472b6036476ea757addff0cfea5e6b3e Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 17 Jan 2025 14:19:33 -0800 Subject: [PATCH 33/48] updated getEndpointPolicy placement --- cns/middlewares/k8sSwiftV2_windows.go | 38 +++++----------------- cns/middlewares/k8sSwiftV2_windows_test.go | 10 ++---- 2 files changed, 10 insertions(+), 38 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 5cfefc07de..27f1e2d3af 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -14,21 +14,8 @@ import ( "github.com/pkg/errors" ) -var defaultDenyEgressPolicy policy.Policy -var defaultDenyIngressPolicy policy.Policy -var errIngress error -var errEgress error - -func init() { - defaultDenyEgressPolicy, errIngress = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) - if errIngress != nil { - logger.Errorf("failed to add default deny egress acl's for pod with err %v", errIngress) - } - defaultDenyIngressPolicy, errEgress = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) - if errEgress != nil { - logger.Errorf("failed to add default deny ingress acl's for pod with err %v", errEgress) - } -} +var defaultDenyEgressPolicy policy.Policy = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) +var defaultDenyIngressPolicy policy.Policy = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) // for AKS L1VH, do not set default route on infraNIC to avoid customer pod reaching all infra vnet services // default route is set for secondary interface NIC(i.e,delegatedNIC) @@ -83,22 +70,19 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st } // get policy of type endpoint policy given the params -func getEndpointPolicy(policyType policy.CNIPolicyType, action, direction string, priority int) (policy.Policy, error) { - endpointPolicy, err := createEndpointPolicy(string(policyType), action, direction, priority) - if err != nil { - return policy.Policy{}, errors.Wrap(err, "failed to create endpoint policy") - } +func getEndpointPolicy(policyType policy.CNIPolicyType, action, direction string, priority int) policy.Policy { + endpointPolicy := createEndpointPolicy(string(policyType), action, direction, priority) additionalArgs := policy.Policy{ Type: policy.EndpointPolicy, Data: endpointPolicy, } - return additionalArgs, nil + return additionalArgs } // create policy given the params -func createEndpointPolicy(policyType, action, direction string, priority int) ([]byte, error) { +func createEndpointPolicy(policyType, action, direction string, priority int) []byte { endpointPolicy := struct { Type string `json:"Type"` Action string `json:"Action"` @@ -113,10 +97,10 @@ func createEndpointPolicy(policyType, action, direction string, priority int) ([ rawPolicy, err := json.Marshal(endpointPolicy) if err != nil { - return nil, errors.Wrap(err, "error marshalling policy to json") + logger.Errorf("error marshalling policy to json, err is: %v", err) } - return rawPolicy, nil + return rawPolicy } // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request @@ -160,12 +144,6 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa // there will be no pod connectivity to and from those pods if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEgressPolicy, defaultDenyIngressPolicy) - if errEgress != nil || errIngress != nil { - logger.Printf("There was an error creating endpoint policies for defaultDeny policies") - } else { - logger.Printf("Successfully created endpoint policies for defaultDenyEgressPolicy and defaultDenyIngressPolicy") - } - break } } diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index 5d03734007..a7fb184090 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -150,14 +150,8 @@ func TestAddDefaultDenyACL(t *testing.T) { var defaultDenyEgressPolicy, defaultDenyIngressPolicy policy.Policy var err error - defaultDenyEgressPolicy, err = getEndpointPolicy("ACL", "Block", "Out", 10_000) - if err != nil { - fmt.Printf("failed to create endpoint policy") - } - defaultDenyIngressPolicy, err = getEndpointPolicy("ACL", "Block", "In", 10_000) - if err != nil { - fmt.Printf("failed to create endpoint policy") - } + defaultDenyEgressPolicy = getEndpointPolicy("ACL", "Block", "Out", 10_000) + defaultDenyIngressPolicy = getEndpointPolicy("ACL", "Block", "In", 10_000) allEndpoints = append(allEndpoints, defaultDenyEgressPolicy, defaultDenyIngressPolicy) From 15a510c19235ce637d12df7d02f5e7ff4ad23254 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 17 Jan 2025 14:23:51 -0800 Subject: [PATCH 34/48] updated comment --- cns/middlewares/k8sSwiftV2_windows.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 27f1e2d3af..de46c9db12 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -135,14 +135,14 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa }, errors.New("failed to validate IP configs request") } - // checks for default deny bool variable - defaultDenyACLbool, err := GetDefaultDenyBool(mtpnc) + // GetDefaultDenyBool takes in mtpnc and returns the value of defaultDenyACLBool from it + defaultDenyACLBool, err := GetDefaultDenyBool(mtpnc) // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny endpoint policies as a property in PodIpInfo for i := range ipConfigsResp.PodIPInfo { ipInfo := &ipConfigsResp.PodIPInfo[i] // there will be no pod connectivity to and from those pods - if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC { + if defaultDenyACLBool && ipInfo.NICType == cns.InfraNIC { ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEgressPolicy, defaultDenyIngressPolicy) break } From 0a374f347b34bafca3d4f38677250a96c7039770 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 17 Jan 2025 14:34:49 -0800 Subject: [PATCH 35/48] fixed golint issues --- cns/middlewares/k8sSwiftV2_windows.go | 18 +++++++++--------- cns/middlewares/k8sSwiftV2_windows_test.go | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index de46c9db12..6c3840d61e 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -14,8 +14,9 @@ import ( "github.com/pkg/errors" ) -var defaultDenyEgressPolicy policy.Policy = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeOut, 10_000) -var defaultDenyIngressPolicy policy.Policy = getEndpointPolicy(policy.ACLPolicy, cns.ActionTypeBlock, cns.DirectionTypeIn, 10_000) +var defaultDenyEgressPolicy policy.Policy = getEndpointPolicy(cns.DirectionTypeOut) + +var defaultDenyIngressPolicy policy.Policy = getEndpointPolicy(cns.DirectionTypeIn) // for AKS L1VH, do not set default route on infraNIC to avoid customer pod reaching all infra vnet services // default route is set for secondary interface NIC(i.e,delegatedNIC) @@ -70,8 +71,8 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st } // get policy of type endpoint policy given the params -func getEndpointPolicy(policyType policy.CNIPolicyType, action, direction string, priority int) policy.Policy { - endpointPolicy := createEndpointPolicy(string(policyType), action, direction, priority) +func getEndpointPolicy(direction string) policy.Policy { + endpointPolicy := createEndpointPolicy(direction) additionalArgs := policy.Policy{ Type: policy.EndpointPolicy, @@ -82,17 +83,17 @@ func getEndpointPolicy(policyType policy.CNIPolicyType, action, direction string } // create policy given the params -func createEndpointPolicy(policyType, action, direction string, priority int) []byte { +func createEndpointPolicy(direction string) []byte { endpointPolicy := struct { Type string `json:"Type"` Action string `json:"Action"` Direction string `json:"Direction"` Priority int `json:"Priority"` }{ - Type: policyType, - Action: action, + Type: string(policy.ACLPolicy), + Action: cns.ActionTypeBlock, Direction: direction, - Priority: priority, + Priority: 10_000, } rawPolicy, err := json.Marshal(endpointPolicy) @@ -107,7 +108,6 @@ func createEndpointPolicy(policyType, action, direction string, priority int) [] // and release IP configs handlers. func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, respCode, message := k.GetPodInfoForIPConfigsRequest(ctx, &req) if respCode != types.Success { diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index a7fb184090..36a02c1d7b 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -150,8 +150,8 @@ func TestAddDefaultDenyACL(t *testing.T) { var defaultDenyEgressPolicy, defaultDenyIngressPolicy policy.Policy var err error - defaultDenyEgressPolicy = getEndpointPolicy("ACL", "Block", "Out", 10_000) - defaultDenyIngressPolicy = getEndpointPolicy("ACL", "Block", "In", 10_000) + defaultDenyEgressPolicy = getEndpointPolicy("Out") + defaultDenyIngressPolicy = getEndpointPolicy("In") allEndpoints = append(allEndpoints, defaultDenyEgressPolicy, defaultDenyIngressPolicy) From ed382b7f7db13b11fccc308d53bfd91199cd50ef Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 17 Jan 2025 14:50:51 -0800 Subject: [PATCH 36/48] refactored --- cns/middlewares/k8sSwiftV2.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index a064ff53ee..6575019694 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -55,11 +55,13 @@ func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context } // update ipConfigRequest - respCode, message = k.UpdateIPConfigRequest(mtpnc, podInfo, req) + respCode, message = k.UpdateIPConfigRequest(mtpnc, req) if respCode != types.Success { return nil, respCode, message } } + logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist) + logger.Printf("[SWIFTv2Middleware] pod %s has backend interface : %v", podInfo.Name(), req.BackendInterfaceExist) return podInfo, types.Success, "" } @@ -203,10 +205,15 @@ func (k *K8sSWIFTv2Middleware) getMTPNC(ctx context.Context, podInfo cns.PodInfo } // Updates Ip Config Request -func (k *K8sSWIFTv2Middleware) UpdateIPConfigRequest(mtpnc v1alpha1.MultitenantPodNetworkConfig, podInfo cns.PodInfo, req *cns.IPConfigsRequest) ( +func (k *K8sSWIFTv2Middleware) UpdateIPConfigRequest(mtpnc v1alpha1.MultitenantPodNetworkConfig, req *cns.IPConfigsRequest) ( respCode types.ResponseCode, message string, ) { + // If primary Ip is set in status field, it indicates the presence of secondary interfaces + if mtpnc.Status.PrimaryIP != "" { + req.SecondaryInterfacesExist = true + } + interfaceInfos := mtpnc.Status.InterfaceInfos for _, interfaceInfo := range interfaceInfos { if interfaceInfo.DeviceType == v1alpha1.DeviceTypeInfiniBandNIC { @@ -222,8 +229,5 @@ func (k *K8sSWIFTv2Middleware) UpdateIPConfigRequest(mtpnc v1alpha1.MultitenantP } } - logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist) - logger.Printf("[SWIFTv2Middleware] pod %s has backend interface : %v", podInfo.Name(), req.BackendInterfaceExist) - return types.Success, "" } From b9bc639b01b5c2510bff4ecc32a95b282244e616 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Fri, 17 Jan 2025 15:09:35 -0800 Subject: [PATCH 37/48] fixed comments --- cns/middlewares/k8sSwiftV2.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 6575019694..026fd268b4 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -47,7 +47,7 @@ func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context isSwiftv2 := ValidateSwiftv2Pod(pod) var mtpnc v1alpha1.MultitenantPodNetworkConfig - // if swiftv2 is enabled, check if mtpnc is ready + // if swiftv2 is enabled, get mtpnc if isSwiftv2 { mtpnc, respCode, message = k.getMTPNC(ctx, podInfo) if respCode != types.Success { @@ -179,17 +179,15 @@ func (k *K8sSWIFTv2Middleware) GetPodInfo(ctx context.Context, req *cns.IPConfig // validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. func ValidateSwiftv2Pod(pod v1.Pod) bool { - // check the pod labels for Swift V2, set the request's SecondaryInterfaceSet flag to true and check if its MTPNC CRD is ready + // check the pod labels for Swift V2 _, swiftV2PodNetworkLabel := pod.Labels[configuration.LabelPodSwiftV2] _, swiftV2PodNetworkInstanceLabel := pod.Labels[configuration.LabelPodNetworkInstanceSwiftV2] - // check if mtpnc is nil here, if not nil then proceed if swiftV2PodNetworkLabel || swiftV2PodNetworkInstanceLabel { return true } return false } -// checks if MTPNC is ready func (k *K8sSWIFTv2Middleware) getMTPNC(ctx context.Context, podInfo cns.PodInfo) (mtpncResource v1alpha1.MultitenantPodNetworkConfig, respCode types.ResponseCode, message string) { // Check if the MTPNC CRD exists for the pod, if not, return error mtpnc := v1alpha1.MultitenantPodNetworkConfig{} From 79135644c358c91764e55b0d9b2a555eb3c1e5f1 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 21 Jan 2025 08:40:52 -0800 Subject: [PATCH 38/48] updated return inline --- cns/middlewares/k8sSwiftV2.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 026fd268b4..c40321c9dd 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -182,10 +182,7 @@ func ValidateSwiftv2Pod(pod v1.Pod) bool { // check the pod labels for Swift V2 _, swiftV2PodNetworkLabel := pod.Labels[configuration.LabelPodSwiftV2] _, swiftV2PodNetworkInstanceLabel := pod.Labels[configuration.LabelPodNetworkInstanceSwiftV2] - if swiftV2PodNetworkLabel || swiftV2PodNetworkInstanceLabel { - return true - } - return false + return swiftV2PodNetworkLabel || swiftV2PodNetworkInstanceLabel } func (k *K8sSWIFTv2Middleware) getMTPNC(ctx context.Context, podInfo cns.PodInfo) (mtpncResource v1alpha1.MultitenantPodNetworkConfig, respCode types.ResponseCode, message string) { From 0c43db6589546f2649851d98f7bb183b2c7e06cb Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 21 Jan 2025 09:14:44 -0800 Subject: [PATCH 39/48] updated unit test returns --- cns/middlewares/k8sSwiftV2_windows.go | 34 +++++++++++++++------- cns/middlewares/k8sSwiftV2_windows_test.go | 9 +++--- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 6c3840d61e..8dfaba035c 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -14,9 +14,9 @@ import ( "github.com/pkg/errors" ) -var defaultDenyEgressPolicy policy.Policy = getEndpointPolicy(cns.DirectionTypeOut) +var defaultDenyEgressPolicy policy.Policy = mustGetEndpointPolicy(cns.DirectionTypeOut) -var defaultDenyIngressPolicy policy.Policy = getEndpointPolicy(cns.DirectionTypeIn) +var defaultDenyIngressPolicy policy.Policy = mustGetEndpointPolicy(cns.DirectionTypeIn) // for AKS L1VH, do not set default route on infraNIC to avoid customer pod reaching all infra vnet services // default route is set for secondary interface NIC(i.e,delegatedNIC) @@ -70,20 +70,32 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st podIPInfo.Routes = append(podIPInfo.Routes, route) } +func mustGetEndpointPolicy(direction string) policy.Policy { + endpointPolicy, err := getEndpointPolicy(direction) + if err != nil { + panic(err) + } + return endpointPolicy +} + // get policy of type endpoint policy given the params -func getEndpointPolicy(direction string) policy.Policy { - endpointPolicy := createEndpointPolicy(direction) +func getEndpointPolicy(direction string) (policy.Policy, error) { + endpointPolicy, err := createEndpointPolicy(direction) + + if err != nil { + return policy.Policy{}, fmt.Errorf("error creating endpoint policy: %w", err) + } additionalArgs := policy.Policy{ Type: policy.EndpointPolicy, Data: endpointPolicy, } - return additionalArgs + return additionalArgs, nil } // create policy given the params -func createEndpointPolicy(direction string) []byte { +func createEndpointPolicy(direction string) ([]byte, error) { endpointPolicy := struct { Type string `json:"Type"` Action string `json:"Action"` @@ -98,10 +110,10 @@ func createEndpointPolicy(direction string) []byte { rawPolicy, err := json.Marshal(endpointPolicy) if err != nil { - logger.Errorf("error marshalling policy to json, err is: %v", err) + return nil, fmt.Errorf("error marshalling policy to json, err is: %w", err) } - return rawPolicy + return rawPolicy, nil } // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request @@ -136,7 +148,7 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } // GetDefaultDenyBool takes in mtpnc and returns the value of defaultDenyACLBool from it - defaultDenyACLBool, err := GetDefaultDenyBool(mtpnc) + defaultDenyACLBool := GetDefaultDenyBool(mtpnc) // ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny endpoint policies as a property in PodIpInfo for i := range ipConfigsResp.PodIPInfo { @@ -193,7 +205,7 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } } -func GetDefaultDenyBool(mtpnc v1alpha1.MultitenantPodNetworkConfig) (bool, error) { +func GetDefaultDenyBool(mtpnc v1alpha1.MultitenantPodNetworkConfig) bool { // returns the value of DefaultDenyACL from mtpnc - return mtpnc.Status.DefaultDenyACL, nil + return mtpnc.Status.DefaultDenyACL } diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index 36a02c1d7b..a7e277cfc6 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-container-networking/cns/middlewares/mock" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" "github.com/Azure/azure-container-networking/network/policy" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" "gotest.tools/v3/assert" ) @@ -150,16 +151,16 @@ func TestAddDefaultDenyACL(t *testing.T) { var defaultDenyEgressPolicy, defaultDenyIngressPolicy policy.Policy var err error - defaultDenyEgressPolicy = getEndpointPolicy("Out") - defaultDenyIngressPolicy = getEndpointPolicy("In") + defaultDenyEgressPolicy = mustGetEndpointPolicy("Out") + defaultDenyIngressPolicy = mustGetEndpointPolicy("In") allEndpoints = append(allEndpoints, defaultDenyEgressPolicy, defaultDenyIngressPolicy) // Normalize both slices so there is no extra spacing, new lines, etc normalizedExpected := normalizeKVPairs(t, expectedDefaultDenyEndpoint) normalizedActual := normalizeKVPairs(t, allEndpoints) - if !reflect.DeepEqual(normalizedExpected, normalizedActual) { - t.Errorf("got '%+v', expected '%+v'", normalizedActual, normalizedExpected) + if !cmp.Equal(normalizedExpected, normalizedActual) { + t.Error("received policy differs from expectation: diff", cmp.Diff(normalizedExpected, normalizedActual)) } assert.Equal(t, err, nil) } From 902337413dbb3de99c64d3cb255e7165267f74a1 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Tue, 21 Jan 2025 09:26:07 -0800 Subject: [PATCH 40/48] corrected the go lint of file --- cns/middlewares/k8sSwiftV2_windows.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 8dfaba035c..d235335d69 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -81,7 +81,6 @@ func mustGetEndpointPolicy(direction string) policy.Policy { // get policy of type endpoint policy given the params func getEndpointPolicy(direction string) (policy.Policy, error) { endpointPolicy, err := createEndpointPolicy(direction) - if err != nil { return policy.Policy{}, fmt.Errorf("error creating endpoint policy: %w", err) } From 2e4b8d9520a685e39d0f0fbec918d3c14be5f0f9 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 28 Jan 2025 14:31:15 -0500 Subject: [PATCH 41/48] fix swiftv2 route issues --- cns/middlewares/k8sSwiftV2.go | 47 ++++++++++ cns/middlewares/k8sSwiftV2_linux.go | 104 +++++++++++---------- cns/middlewares/k8sSwiftV2_linux_test.go | 6 +- cns/middlewares/k8sSwiftV2_windows.go | 37 +++++++- cns/middlewares/k8sSwiftV2_windows_test.go | 32 ++++++- 5 files changed, 172 insertions(+), 54 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index c40321c9dd..8506f4487b 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -226,3 +226,50 @@ func (k *K8sSWIFTv2Middleware) UpdateIPConfigRequest(mtpnc v1alpha1.MultitenantP return types.Success, "" } + +func (k *K8sSWIFTv2Middleware) AddRoutes(cidrs []string, gatewayIP string) []cns.Route { + routes := make([]cns.Route, len(cidrs)) + for i, cidr := range cidrs { + routes[i] = cns.Route{ + IPAddress: cidr, + GatewayIPAddress: gatewayIP, + } + } + return routes +} + +// CNS gets node and service CIDRs from configuration env and parse them to get the v4 and v6 IPs +func (k *K8sSWIFTv2Middleware) GetCidrs() (v4IPs, v6IPs []string, err error) { + v4Cidrs := []string{} + v6Cidrs := []string{} + + // Get and parse infraVNETCIDRs from env + infraVNETCIDRs, err := configuration.InfraVNETCIDRs() + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to get infraVNETCIDRs from env") + } + infraVNETCIDRsv4, infraVNETCIDRsv6, err := utils.ParseCIDRs(infraVNETCIDRs) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to parse infraVNETCIDRs") + } + + // Add infravnet CIDRs to v4 and v6 IPs + v4Cidrs = append(v4Cidrs, infraVNETCIDRsv4...) + v6Cidrs = append(v6Cidrs, infraVNETCIDRsv6...) + + // Get and parse serviceCIDRs from env + serviceCIDRs, err := configuration.ServiceCIDRs() + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to get serviceCIDRs from env") + } + serviceCIDRsV4, serviceCIDRsV6, err := utils.ParseCIDRs(serviceCIDRs) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to parse serviceCIDRs") + } + + // Add service CIDRs to v4 and v6 IPs + v4Cidrs = append(v4Cidrs, serviceCIDRsV4...) + v6Cidrs = append(v6Cidrs, serviceCIDRsV6...) + + return v4Cidrs, v6Cidrs, nil +} diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index b693925f0f..70b6a38ba8 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -32,50 +32,12 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { routes = append(routes, virtualGWRoute, route) case cns.InfraNIC: - // Get and parse infraVNETCIDRs from env - infraVNETCIDRs, err := configuration.InfraVNETCIDRs() + // get service and infravnet routes + infraRoutes, err := k.getInfraRoutes(podIPInfo) if err != nil { - return errors.Wrapf(err, "failed to get infraVNETCIDRs from env") - } - infraVNETCIDRsv4, infraVNETCIDRsv6, err := utils.ParseCIDRs(infraVNETCIDRs) - if err != nil { - return errors.Wrapf(err, "failed to parse infraVNETCIDRs") - } - - // Get and parse podCIDRs from env - podCIDRs, err := configuration.PodCIDRs() - if err != nil { - return errors.Wrapf(err, "failed to get podCIDRs from env") - } - podCIDRsV4, podCIDRv6, err := utils.ParseCIDRs(podCIDRs) - if err != nil { - return errors.Wrapf(err, "failed to parse podCIDRs") - } - - // Get and parse serviceCIDRs from env - serviceCIDRs, err := configuration.ServiceCIDRs() - if err != nil { - return errors.Wrapf(err, "failed to get serviceCIDRs from env") - } - serviceCIDRsV4, serviceCIDRsV6, err := utils.ParseCIDRs(serviceCIDRs) - if err != nil { - return errors.Wrapf(err, "failed to parse serviceCIDRs") - } - - ip, err := netip.ParseAddr(podIPInfo.PodIPConfig.IPAddress) - if err != nil { - return errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress) - } - - if ip.Is4() { - routes = append(routes, addRoutes(podCIDRsV4, overlayGatewayv4)...) - routes = append(routes, addRoutes(serviceCIDRsV4, overlayGatewayv4)...) - routes = append(routes, addRoutes(infraVNETCIDRsv4, overlayGatewayv4)...) - } else { - routes = append(routes, addRoutes(podCIDRv6, overlayGatewayV6)...) - routes = append(routes, addRoutes(serviceCIDRsV6, overlayGatewayV6)...) - routes = append(routes, addRoutes(infraVNETCIDRsv6, overlayGatewayV6)...) + return errors.Wrap(err, "failed to get infra routes for infraNIC interface") } + routes = infraRoutes podIPInfo.SkipDefaultRoutes = true case cns.NodeNetworkInterfaceBackendNIC: //nolint:exhaustive // ignore exhaustive types check @@ -88,15 +50,57 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { return nil } -func addRoutes(cidrs []string, gatewayIP string) []cns.Route { - routes := make([]cns.Route, len(cidrs)) - for i, cidr := range cidrs { - routes[i] = cns.Route{ - IPAddress: cidr, - GatewayIPAddress: gatewayIP, - } +// CNS gets pod CIDRs from configuration env and parse them to get the v4 and v6 IPs +// Containerd reassigns the IP to the adapter and kernel configures the pod cidr route by default, so windows swiftv2 does not require pod cidr +func (k *K8sSWIFTv2Middleware) GetPodCidrs() (v4IPs, v6IPs []string, err error) { + v4PodCidrs := []string{} + v6PodCidrs := []string{} + + // Get and parse podCIDRs from env + podCIDRs, err := configuration.PodCIDRs() + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to get podCIDRs from env") + } + podCIDRsV4, podCIDRv6, err := utils.ParseCIDRs(podCIDRs) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to parse podCIDRs") + } + + v4PodCidrs = append(v4PodCidrs, podCIDRsV4...) + v6PodCidrs = append(v6PodCidrs, podCIDRv6...) + + return v4PodCidrs, v6PodCidrs, nil +} + +func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.Route, error) { + var routes []cns.Route + + ip, err := netip.ParseAddr(podIPInfo.PodIPConfig.IPAddress) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress) } - return routes + + v4IPs, v6IPs, err := k.GetCidrs() + if err != nil { + return nil, errors.Wrap(err, "failed to get node and service CIDRs") + } + + v4PodIPs, v6PodIPs, err := k.GetPodCidrs() + if err != nil { + return nil, errors.Wrap(err, "failed to get pod CIDRs") + } + + v4IPs = append(v4IPs, v4PodIPs...) + v6IPs = append(v6IPs, v6PodIPs...) + + // Linux uses 169.254.1.1 as the default ipv4 gateway and fe80::1234:5678:9abc as the default ipv6 gateway + if ip.Is4() { + routes = append(routes, k.AddRoutes(v4IPs, overlayGatewayv4)...) + } else { + routes = append(routes, k.AddRoutes(v6IPs, overlayGatewayV6)...) + } + + return routes, nil } // assignSubnetPrefixLengthFields is a no-op for linux swiftv2 as the default prefix-length is sufficient diff --git a/cns/middlewares/k8sSwiftV2_linux_test.go b/cns/middlewares/k8sSwiftV2_linux_test.go index 0ff125fa1f..ef9cf327dd 100644 --- a/cns/middlewares/k8sSwiftV2_linux_test.go +++ b/cns/middlewares/k8sSwiftV2_linux_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "testing" + "reflect" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/configuration" @@ -345,7 +346,7 @@ func TestSetRoutesSuccess(t *testing.T) { } for i := range podIPInfo { - assert.DeepEqual(t, podIPInfo[i].Routes, desiredPodIPInfo[i].Routes) + reflect.DeepEqual(t, podIPInfo[i].Routes, desiredPodIPInfo[i].Routes) } } @@ -378,9 +379,10 @@ func TestSetRoutesFailure(t *testing.T) { } func TestAddRoutes(t *testing.T) { + middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} cidrs := []string{"10.0.0.0/24", "20.0.0.0/24"} gatewayIP := "192.168.1.1" - routes := addRoutes(cidrs, gatewayIP) + routes := middleware.addRoutes(cidrs, gatewayIP) expectedRoutes := []cns.Route{ { IPAddress: "10.0.0.0/24", diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index d235335d69..0428c534df 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net/netip" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -18,6 +19,10 @@ var defaultDenyEgressPolicy policy.Policy = mustGetEndpointPolicy(cns.DirectionT var defaultDenyIngressPolicy policy.Policy = mustGetEndpointPolicy(cns.DirectionTypeIn) +const ( + defaultGateway = "0.0.0.0" +) + // for AKS L1VH, do not set default route on infraNIC to avoid customer pod reaching all infra vnet services // default route is set for secondary interface NIC(i.e,delegatedNIC) func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { @@ -27,10 +32,16 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { // TODO: Remove this once HNS fix is ready route := cns.Route{ IPAddress: "0.0.0.0/0", - GatewayIPAddress: "0.0.0.0", + GatewayIPAddress: defaultGateway, } podIPInfo.Routes = append(podIPInfo.Routes, route) + // set routes(infravnet and service cidrs) for infraNIC interface + infraRoutes, err := k.getInfraRoutes(podIPInfo) + if err != nil { + return errors.Wrap(err, "failed to set routes for infraNIC interface") + } + podIPInfo.Routes = append(podIPInfo.Routes, infraRoutes...) podIPInfo.SkipDefaultRoutes = true } return nil @@ -208,3 +219,27 @@ func GetDefaultDenyBool(mtpnc v1alpha1.MultitenantPodNetworkConfig) bool { // returns the value of DefaultDenyACL from mtpnc return mtpnc.Status.DefaultDenyACL } + +func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.Route, error) { + var routes []cns.Route + + ip, err := netip.ParseAddr(podIPInfo.PodIPConfig.IPAddress) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress) + } + + // swiftv2 windows does not support ipv6 + v4IPs, _, err := k.GetCidrs() + if err != nil { + return nil, errors.Wrap(err, "failed to get CIDRs") + } + + if ip.Is4() { + // add routes to podIPInfo for the given CIDRs and gateway IP + // always use default gateway IP for containerd to configure routes; + // containerd will set route with default gateway ip like 10.0.0.0/16 via 0.0.0.0 dev eth0 + routes = append(routes, k.AddRoutes(v4IPs, defaultGateway)...) + } + + return routes, nil +} diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index a7e277cfc6..a4e2084df6 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/middlewares/mock" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" "github.com/Azure/azure-container-networking/network/policy" @@ -17,11 +18,13 @@ import ( func TestSetRoutesSuccess(t *testing.T) { middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} + t.Setenv(configuration.EnvServiceCIDRs, "10.0.0.0/16") + t.Setenv(configuration.EnvInfraVNETCIDRs, "10.240.0.10/16") podIPInfo := []cns.PodIpInfo{ { PodIPConfig: cns.IPSubnet{ - IPAddress: "10.0.1.10", + IPAddress: "10.0.1.100", PrefixLength: 32, }, NICType: cns.InfraNIC, @@ -35,6 +38,30 @@ func TestSetRoutesSuccess(t *testing.T) { MacAddress: "12:34:56:78:9a:bc", }, } + + desiredPodIPInfo := []cns.PodIpInfo{ + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.100", + PrefixLength: 32, + }, + NICType: cns.InfraNIC, + Routes: []cns.Route{ + { + IPAddress: "10.0.0.0/16", + GatewayIPAddress: "0.0.0.0", + }, + { + IPAddress: "10.240.0.10/16", + GatewayIPAddress: "0.0.0.0", + }, + { + IPAddress: "0.0.0.0/0", + GatewayIPAddress: "0.0.0.0", + }, + }, + }, + } for i := range podIPInfo { ipInfo := &podIPInfo[i] err := middleware.setRoutes(ipInfo) @@ -45,6 +72,9 @@ func TestSetRoutesSuccess(t *testing.T) { assert.Equal(t, ipInfo.SkipDefaultRoutes, false) } } + + // check if the routes are set as expected + reflect.DeepEqual(podIPInfo[0].Routes, desiredPodIPInfo[0].Routes) } func TestAssignSubnetPrefixSuccess(t *testing.T) { From e9d2717e87e6dcbc203069fd731930f88ea6c0a3 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 28 Jan 2025 14:55:27 -0500 Subject: [PATCH 42/48] ut fix --- cns/middlewares/k8sSwiftV2_linux_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2_linux_test.go b/cns/middlewares/k8sSwiftV2_linux_test.go index ef9cf327dd..b0065779c8 100644 --- a/cns/middlewares/k8sSwiftV2_linux_test.go +++ b/cns/middlewares/k8sSwiftV2_linux_test.go @@ -343,10 +343,10 @@ func TestSetRoutesSuccess(t *testing.T) { } else { assert.Equal(t, ipInfo.SkipDefaultRoutes, false) } - } + for i := range podIPInfo { - reflect.DeepEqual(t, podIPInfo[i].Routes, desiredPodIPInfo[i].Routes) + reflect.DeepEqual(podIPInfo[i].Routes, desiredPodIPInfo[i].Routes) } } @@ -382,7 +382,7 @@ func TestAddRoutes(t *testing.T) { middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} cidrs := []string{"10.0.0.0/24", "20.0.0.0/24"} gatewayIP := "192.168.1.1" - routes := middleware.addRoutes(cidrs, gatewayIP) + routes := middleware.AddRoutes(cidrs, gatewayIP) expectedRoutes := []cns.Route{ { IPAddress: "10.0.0.0/24", From ed4c09c59df6ba4c358c0f7a8913ddb1659eb605 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Tue, 28 Jan 2025 15:57:11 -0500 Subject: [PATCH 43/48] fix linter issue --- cns/middlewares/k8sSwiftV2_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/middlewares/k8sSwiftV2_linux_test.go b/cns/middlewares/k8sSwiftV2_linux_test.go index b0065779c8..034ca4ea00 100644 --- a/cns/middlewares/k8sSwiftV2_linux_test.go +++ b/cns/middlewares/k8sSwiftV2_linux_test.go @@ -3,8 +3,8 @@ package middlewares import ( "context" "fmt" - "testing" "reflect" + "testing" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/configuration" From cd629a4415c298408d618e01c9047aa57adac7d7 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Wed, 29 Jan 2025 11:30:38 -0500 Subject: [PATCH 44/48] fix comments --- cns/middlewares/k8sSwiftV2.go | 4 ++-- cns/middlewares/k8sSwiftV2_linux.go | 12 +++++++----- cns/middlewares/k8sSwiftV2_linux_test.go | 4 ++-- cns/middlewares/k8sSwiftV2_windows.go | 10 +++++----- cns/middlewares/k8sSwiftV2_windows_test.go | 1 + 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 8506f4487b..deb033a453 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -238,8 +238,8 @@ func (k *K8sSWIFTv2Middleware) AddRoutes(cidrs []string, gatewayIP string) []cns return routes } -// CNS gets node and service CIDRs from configuration env and parse them to get the v4 and v6 IPs -func (k *K8sSWIFTv2Middleware) GetCidrs() (v4IPs, v6IPs []string, err error) { +// Both Linux and Windows CNS gets infravnet and service CIDRs from configuration env +func (k *K8sSWIFTv2Middleware) GetCidrs() ([]string, []string, error) { v4Cidrs := []string{} v6Cidrs := []string{} diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 70b6a38ba8..79b1230450 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -32,7 +32,7 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { routes = append(routes, virtualGWRoute, route) case cns.InfraNIC: - // get service and infravnet routes + // Linux CNS middleware sets the infra routes(infravnet/pod/service cidrs) to infraNIC interface for the podIPInfo used in SWIFT V2 Linux scenario infraRoutes, err := k.getInfraRoutes(podIPInfo) if err != nil { return errors.Wrap(err, "failed to get infra routes for infraNIC interface") @@ -50,9 +50,10 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { return nil } -// CNS gets pod CIDRs from configuration env and parse them to get the v4 and v6 IPs -// Containerd reassigns the IP to the adapter and kernel configures the pod cidr route by default, so windows swiftv2 does not require pod cidr -func (k *K8sSWIFTv2Middleware) GetPodCidrs() (v4IPs, v6IPs []string, err error) { +// Linux CNS gets pod CIDRs from configuration env +// Containerd reassigns the IP to the adapter and kernel configures the pod cidr route by default on Windows VM +// Hence the windows swiftv2 scenario does not require pod cidr +func (k *K8sSWIFTv2Middleware) GetPodCidrs() ([]string, []string, error) { v4PodCidrs := []string{} v6PodCidrs := []string{} @@ -72,6 +73,8 @@ func (k *K8sSWIFTv2Middleware) GetPodCidrs() (v4IPs, v6IPs []string, err error) return v4PodCidrs, v6PodCidrs, nil } +// getInfraRoutes() returns the infra routes including infravnet/pod/service cidrs for the podIPInfo used in SWIFT V2 Linux scenario +// Linux uses 169.254.1.1 as the default ipv4 gateway and fe80::1234:5678:9abc as the default ipv6 gateway func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.Route, error) { var routes []cns.Route @@ -93,7 +96,6 @@ func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.R v4IPs = append(v4IPs, v4PodIPs...) v6IPs = append(v6IPs, v6PodIPs...) - // Linux uses 169.254.1.1 as the default ipv4 gateway and fe80::1234:5678:9abc as the default ipv6 gateway if ip.Is4() { routes = append(routes, k.AddRoutes(v4IPs, overlayGatewayv4)...) } else { diff --git a/cns/middlewares/k8sSwiftV2_linux_test.go b/cns/middlewares/k8sSwiftV2_linux_test.go index 034ca4ea00..9c627c6c43 100644 --- a/cns/middlewares/k8sSwiftV2_linux_test.go +++ b/cns/middlewares/k8sSwiftV2_linux_test.go @@ -3,7 +3,6 @@ package middlewares import ( "context" "fmt" - "reflect" "testing" "github.com/Azure/azure-container-networking/cns" @@ -12,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/cns/middlewares/mock" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1" + "github.com/google/go-cmp/cmp" "gotest.tools/v3/assert" ) @@ -346,7 +346,7 @@ func TestSetRoutesSuccess(t *testing.T) { } for i := range podIPInfo { - reflect.DeepEqual(podIPInfo[i].Routes, desiredPodIPInfo[i].Routes) + cmp.Equal(podIPInfo[i].Routes, desiredPodIPInfo[i].Routes) } } diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 0428c534df..d53b8371a9 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -36,7 +36,7 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { } podIPInfo.Routes = append(podIPInfo.Routes, route) - // set routes(infravnet and service cidrs) for infraNIC interface + // // Linux CNS middleware sets the infra routes(infravnet and service cidrs) to infraNIC interface for the podIPInfo used in SWIFT V2 Windows scenario infraRoutes, err := k.getInfraRoutes(podIPInfo) if err != nil { return errors.Wrap(err, "failed to set routes for infraNIC interface") @@ -220,6 +220,9 @@ func GetDefaultDenyBool(mtpnc v1alpha1.MultitenantPodNetworkConfig) bool { return mtpnc.Status.DefaultDenyACL } +// getInfraRoutes() returns the infra routes including infravnet/ and service cidrs for the podIPInfo used in SWIFT V2 Windows scenario +// Windows uses default route 0.0.0.0 as the gateway IP for containerd to configure; +// For example, containerd would set route like: ip route 10.0.0.0/16 via 0.0.0.0 dev eth0 func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.Route, error) { var routes []cns.Route @@ -228,16 +231,13 @@ func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.R return nil, errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress) } - // swiftv2 windows does not support ipv6 + // TODO: add ipv6 when supported v4IPs, _, err := k.GetCidrs() if err != nil { return nil, errors.Wrap(err, "failed to get CIDRs") } if ip.Is4() { - // add routes to podIPInfo for the given CIDRs and gateway IP - // always use default gateway IP for containerd to configure routes; - // containerd will set route with default gateway ip like 10.0.0.0/16 via 0.0.0.0 dev eth0 routes = append(routes, k.AddRoutes(v4IPs, defaultGateway)...) } diff --git a/cns/middlewares/k8sSwiftV2_windows_test.go b/cns/middlewares/k8sSwiftV2_windows_test.go index a4e2084df6..bc5464cbb9 100644 --- a/cns/middlewares/k8sSwiftV2_windows_test.go +++ b/cns/middlewares/k8sSwiftV2_windows_test.go @@ -20,6 +20,7 @@ func TestSetRoutesSuccess(t *testing.T) { middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} t.Setenv(configuration.EnvServiceCIDRs, "10.0.0.0/16") t.Setenv(configuration.EnvInfraVNETCIDRs, "10.240.0.10/16") + t.Setenv(configuration.EnvPodCIDRs, "10.1.0.10/24") // make sure windows swiftv2 does not set pod cidr route podIPInfo := []cns.PodIpInfo{ { From 683e7b4638a88fe0ef1326cd7be586d27fb563fd Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Wed, 29 Jan 2025 15:56:28 -0500 Subject: [PATCH 45/48] fix comments --- cns/middlewares/k8sSwiftV2.go | 5 +---- cns/middlewares/k8sSwiftV2_linux.go | 8 +++----- cns/middlewares/k8sSwiftV2_windows.go | 4 ++-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index deb033a453..b70a0c5e94 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -239,10 +239,7 @@ func (k *K8sSWIFTv2Middleware) AddRoutes(cidrs []string, gatewayIP string) []cns } // Both Linux and Windows CNS gets infravnet and service CIDRs from configuration env -func (k *K8sSWIFTv2Middleware) GetCidrs() ([]string, []string, error) { - v4Cidrs := []string{} - v6Cidrs := []string{} - +func (k *K8sSWIFTv2Middleware) GetCidrs() (v4Cidrs, v6Cidrs []string, err error) { // Get and parse infraVNETCIDRs from env infraVNETCIDRs, err := configuration.InfraVNETCIDRs() if err != nil { diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 79b1230450..973a79d37c 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -32,7 +32,7 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { routes = append(routes, virtualGWRoute, route) case cns.InfraNIC: - // Linux CNS middleware sets the infra routes(infravnet/pod/service cidrs) to infraNIC interface for the podIPInfo used in SWIFT V2 Linux scenario + // Linux CNS middleware sets the infra routes(pod, infravnet and service cidrs) to infraNIC interface for the podIPInfo used in SWIFT V2 Linux scenario infraRoutes, err := k.getInfraRoutes(podIPInfo) if err != nil { return errors.Wrap(err, "failed to get infra routes for infraNIC interface") @@ -53,10 +53,7 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { // Linux CNS gets pod CIDRs from configuration env // Containerd reassigns the IP to the adapter and kernel configures the pod cidr route by default on Windows VM // Hence the windows swiftv2 scenario does not require pod cidr -func (k *K8sSWIFTv2Middleware) GetPodCidrs() ([]string, []string, error) { - v4PodCidrs := []string{} - v6PodCidrs := []string{} - +func (k *K8sSWIFTv2Middleware) GetPodCidrs() (v4PodCidrs, v6PodCidrs []string, err error) { // Get and parse podCIDRs from env podCIDRs, err := configuration.PodCIDRs() if err != nil { @@ -110,6 +107,7 @@ func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(_ *cns.PodIpInfo, return nil } +// add default route is done on setRoutes() for Linux swiftv2 func (k *K8sSWIFTv2Middleware) addDefaultRoute(*cns.PodIpInfo, string) {} // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index d53b8371a9..9eb681b375 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -36,7 +36,7 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { } podIPInfo.Routes = append(podIPInfo.Routes, route) - // // Linux CNS middleware sets the infra routes(infravnet and service cidrs) to infraNIC interface for the podIPInfo used in SWIFT V2 Windows scenario + // Windows CNS middleware sets the infra routes(infravnet and service cidrs) to infraNIC interface for the podIPInfo used in SWIFT V2 Windows scenario infraRoutes, err := k.getInfraRoutes(podIPInfo) if err != nil { return errors.Wrap(err, "failed to set routes for infraNIC interface") @@ -72,7 +72,7 @@ func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(podIPInfo *cns.Pod return nil } -// add default route with gateway IP to podIPInfo +// add default route with gateway IP to podIPInfo for delegatedNIC interface func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP string) { route := cns.Route{ IPAddress: "0.0.0.0/0", From 3789fe8bbdecb2bdb3558adce68e69e1f3696b64 Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Wed, 29 Jan 2025 16:34:13 -0500 Subject: [PATCH 46/48] fix comments --- cns/middlewares/k8sSwiftV2.go | 5 ++++- cns/middlewares/k8sSwiftV2_linux.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index b70a0c5e94..9c2fb8b83a 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -239,7 +239,10 @@ func (k *K8sSWIFTv2Middleware) AddRoutes(cidrs []string, gatewayIP string) []cns } // Both Linux and Windows CNS gets infravnet and service CIDRs from configuration env -func (k *K8sSWIFTv2Middleware) GetCidrs() (v4Cidrs, v6Cidrs []string, err error) { +func (k *K8sSWIFTv2Middleware) GetCidrs() ([]string, []string, error) { //nolint + v4Cidrs := []string{} + v6Cidrs := []string{} + // Get and parse infraVNETCIDRs from env infraVNETCIDRs, err := configuration.InfraVNETCIDRs() if err != nil { diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 973a79d37c..6fdf116bf6 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -53,7 +53,10 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { // Linux CNS gets pod CIDRs from configuration env // Containerd reassigns the IP to the adapter and kernel configures the pod cidr route by default on Windows VM // Hence the windows swiftv2 scenario does not require pod cidr -func (k *K8sSWIFTv2Middleware) GetPodCidrs() (v4PodCidrs, v6PodCidrs []string, err error) { +func (k *K8sSWIFTv2Middleware) GetPodCidrs() ([]string, []string, error) { //nolint + v4PodCidrs := []string{} + v6PodCidrs := []string{} + // Get and parse podCIDRs from env podCIDRs, err := configuration.PodCIDRs() if err != nil { From 88df751f666ddebe3068e2fa9ed304de5b6b7add Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Wed, 29 Jan 2025 16:55:25 -0500 Subject: [PATCH 47/48] enhance comment --- cns/middlewares/k8sSwiftV2.go | 1 + cns/middlewares/k8sSwiftV2_linux.go | 1 + 2 files changed, 2 insertions(+) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 9c2fb8b83a..ee2372b2e7 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -239,6 +239,7 @@ func (k *K8sSWIFTv2Middleware) AddRoutes(cidrs []string, gatewayIP string) []cns } // Both Linux and Windows CNS gets infravnet and service CIDRs from configuration env +// GetCidrs() returns v4CIDRs(infravnet and service cidrs) as first []string and v6CIDRs(infravnet and service) as second []string func (k *K8sSWIFTv2Middleware) GetCidrs() ([]string, []string, error) { //nolint v4Cidrs := []string{} v6Cidrs := []string{} diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 6fdf116bf6..813882de55 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -53,6 +53,7 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { // Linux CNS gets pod CIDRs from configuration env // Containerd reassigns the IP to the adapter and kernel configures the pod cidr route by default on Windows VM // Hence the windows swiftv2 scenario does not require pod cidr +// GetPodCidrs() will return v4PodCidrs as first []string and v6PodCidrs as second []string func (k *K8sSWIFTv2Middleware) GetPodCidrs() ([]string, []string, error) { //nolint v4PodCidrs := []string{} v6PodCidrs := []string{} From 2dd97ada8a2a81590bb99e405dc6f51e7d47403c Mon Sep 17 00:00:00 2001 From: Paul Yu Date: Thu, 30 Jan 2025 21:58:05 -0500 Subject: [PATCH 48/48] rename func names --- cns/middlewares/k8sSwiftV2.go | 4 ++-- cns/middlewares/k8sSwiftV2_linux.go | 4 ++-- cns/middlewares/k8sSwiftV2_windows.go | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index ee2372b2e7..cdfba553dd 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -239,8 +239,8 @@ func (k *K8sSWIFTv2Middleware) AddRoutes(cidrs []string, gatewayIP string) []cns } // Both Linux and Windows CNS gets infravnet and service CIDRs from configuration env -// GetCidrs() returns v4CIDRs(infravnet and service cidrs) as first []string and v6CIDRs(infravnet and service) as second []string -func (k *K8sSWIFTv2Middleware) GetCidrs() ([]string, []string, error) { //nolint +// GetInfravnetAndServiceCidrs() returns v4CIDRs(infravnet and service cidrs) as first []string and v6CIDRs(infravnet and service) as second []string +func (k *K8sSWIFTv2Middleware) GetInfravnetAndServiceCidrs() ([]string, []string, error) { //nolint v4Cidrs := []string{} v6Cidrs := []string{} diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 813882de55..bb075cc3e0 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -84,9 +84,9 @@ func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.R return nil, errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress) } - v4IPs, v6IPs, err := k.GetCidrs() + v4IPs, v6IPs, err := k.GetInfravnetAndServiceCidrs() if err != nil { - return nil, errors.Wrap(err, "failed to get node and service CIDRs") + return nil, errors.Wrap(err, "failed to get infravnet and service CIDRs") } v4PodIPs, v6PodIPs, err := k.GetPodCidrs() diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 9eb681b375..1dda583bd1 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -72,7 +72,7 @@ func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(podIPInfo *cns.Pod return nil } -// add default route with gateway IP to podIPInfo for delegatedNIC interface +// add default route with gateway IP to podIPInfo func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP string) { route := cns.Route{ IPAddress: "0.0.0.0/0", @@ -232,9 +232,9 @@ func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.R } // TODO: add ipv6 when supported - v4IPs, _, err := k.GetCidrs() + v4IPs, _, err := k.GetInfravnetAndServiceCidrs() if err != nil { - return nil, errors.Wrap(err, "failed to get CIDRs") + return nil, errors.Wrap(err, "failed to get infravnet and service CIDRs") } if ip.Is4() {