Skip to content

Commit 3b0f6b5

Browse files
rejain789rejain789
committed
resolving pr comments
1 parent 598a28e commit 3b0f6b5

File tree

4 files changed

+24
-31
lines changed

4 files changed

+24
-31
lines changed

‎cns/middlewares/k8sSwiftV2.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/Azure/azure-container-networking/cns/middlewares/utils"
1111
"github.com/Azure/azure-container-networking/cns/types"
1212
"github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1"
13+
"github.com/Azure/azure-container-networking/network/policy"
1314
"github.com/pkg/errors"
1415
v1 "k8s.io/api/core/v1"
1516
k8stypes "k8s.io/apimachinery/pkg/types"
@@ -40,7 +41,7 @@ var _ cns.IPConfigsHandlerMiddleware = (*K8sSWIFTv2Middleware)(nil)
4041
// and release IP configs handlers.
4142
func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc {
4243
return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) {
43-
podInfo, respCode, message, defaultDenyACLbool := k.GetPodInfoForIPConfigsRequest(ctx, &req)
44+
podInfo, respCode, defaultDenyACLbool, message := k.GetPodInfoForIPConfigsRequest(ctx, &req)
4445

4546
logger.Printf("defaultDenyACLbool value is: %v", defaultDenyACLbool)
4647

@@ -61,12 +62,14 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa
6162
// ipConfigsResp has infra IP configs -> if defaultDenyACLbool is enabled, add the default deny acl's pn the infra IP configs
6263
for i := range ipConfigsResp.PodIPInfo {
6364
ipInfo := &ipConfigsResp.PodIPInfo[i]
65+
var defaultDenyEndpointPolicies []policy.Policy
6466
// there will be no pod connectivity to and from those pods
6567
if defaultDenyACLbool && ipInfo.NICType == cns.InfraNIC {
66-
err = addDefaultDenyACL(ipInfo)
68+
defaultDenyEndpointPolicies, err = addDefaultDenyACL()
6769
if err != nil {
6870
logger.Errorf("failed to add default deny acl's for pod %v with err %v", podInfo.Name(), err)
6971
}
72+
ipInfo.EndpointPolicies = append(ipInfo.EndpointPolicies, defaultDenyEndpointPolicies...)
7073
break
7174
}
7275
}
@@ -118,21 +121,21 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa
118121

119122
// GetPodInfoForIPConfigsRequest validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario.
120123
// nolint
121-
func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string, defaultDenyACL bool) {
124+
func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, defaultDenyACL bool, message string) {
122125
defaultDenyACLbool := false
123126

124127
// Retrieve the pod from the cluster
125128
podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext)
126129
if err != nil {
127130
errBuf := errors.Wrapf(err, "failed to unmarshalling pod info from ipconfigs request %+v", req)
128-
return nil, types.UnexpectedError, errBuf.Error(), defaultDenyACLbool
131+
return nil, types.UnexpectedError, defaultDenyACLbool, errBuf.Error()
129132
}
130133
logger.Printf("[SWIFTv2Middleware] validate ipconfigs request for pod %s", podInfo.Name())
131134
podNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()}
132135
pod := v1.Pod{}
133136
if err := k.Cli.Get(ctx, podNamespacedName, &pod); err != nil {
134137
errBuf := errors.Wrapf(err, "failed to get pod %+v", podNamespacedName)
135-
return nil, types.UnexpectedError, errBuf.Error(), defaultDenyACLbool
138+
return nil, types.UnexpectedError, defaultDenyACLbool, errBuf.Error()
136139
}
137140

138141
// 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
144147
mtpnc := v1alpha1.MultitenantPodNetworkConfig{}
145148
mtpncNamespacedName := k8stypes.NamespacedName{Namespace: podInfo.Namespace(), Name: podInfo.Name()}
146149
if err := k.Cli.Get(ctx, mtpncNamespacedName, &mtpnc); err != nil {
147-
return nil, types.UnexpectedError, fmt.Errorf("failed to get pod's mtpnc from cache : %w", err).Error(), defaultDenyACLbool
150+
return nil, types.UnexpectedError, defaultDenyACLbool, fmt.Errorf("failed to get pod's mtpnc from cache : %w", err).Error()
148151
}
149152
// Check if the MTPNC CRD is ready. If one of the fields is empty, return error
150153
if !mtpnc.IsReady() {
151-
return nil, types.UnexpectedError, errMTPNCNotReady.Error(), defaultDenyACLbool
154+
return nil, types.UnexpectedError, defaultDenyACLbool, errMTPNCNotReady.Error()
152155
}
153156

154157
// copying defaultDenyACL bool from mtpnc
@@ -162,7 +165,7 @@ func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context
162165
for _, interfaceInfo := range interfaceInfos {
163166
if interfaceInfo.DeviceType == v1alpha1.DeviceTypeInfiniBandNIC {
164167
if interfaceInfo.MacAddress == "" || interfaceInfo.NCID == "" {
165-
return nil, types.UnexpectedError, errMTPNCNotReady.Error(), defaultDenyACLbool
168+
return nil, types.UnexpectedError, defaultDenyACLbool, errMTPNCNotReady.Error()
166169
}
167170
req.BackendInterfaceExist = true
168171
req.BackendInterfaceMacAddresses = append(req.BackendInterfaceMacAddresses, interfaceInfo.MacAddress)
@@ -176,7 +179,7 @@ func (k *K8sSWIFTv2Middleware) GetPodInfoForIPConfigsRequest(ctx context.Context
176179
logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist)
177180
logger.Printf("[SWIFTv2Middleware] pod %s has backend interface : %v", podInfo.Name(), req.BackendInterfaceExist)
178181
// retrieve podinfo from orchestrator context
179-
return podInfo, types.Success, "", defaultDenyACLbool
182+
return podInfo, types.Success, defaultDenyACLbool, ""
180183
}
181184

182185
// getIPConfig returns the pod's SWIFT V2 IP configuration.

‎cns/middlewares/k8sSwiftV2_linux.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/Azure/azure-container-networking/cns/logger"
1010
"github.com/Azure/azure-container-networking/cns/middlewares/utils"
1111
"github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1"
12+
"github.com/Azure/azure-container-networking/network/policy"
1213
"github.com/pkg/errors"
1314
)
1415

@@ -104,6 +105,6 @@ func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(_ *cns.PodIpInfo,
104105

105106
func (k *K8sSWIFTv2Middleware) addDefaultRoute(*cns.PodIpInfo, string) {}
106107

107-
func addDefaultDenyACL(_ *cns.PodIpInfo) error {
108-
return nil
108+
func addDefaultDenyACL() ([]policy.Policy, error) {
109+
return []policy.Policy{}, nil
109110
}

‎cns/middlewares/k8sSwiftV2_windows.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ func (k *K8sSWIFTv2Middleware) addDefaultRoute(podIPInfo *cns.PodIpInfo, gwIP st
6464
}
6565

6666
// append the default deny acl's to the list defaultDenyACL field in podIpInfo
67-
func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error {
67+
func addDefaultDenyACL() ([]policy.Policy, error) {
6868
blockEgressACL, err := getDefaultDenyACLPolicy(hcn.DirectionTypeOut)
6969
if err != nil {
70-
return errors.Wrap(err, "Failed to create default deny ACL policy egress")
70+
return []policy.Policy{}, errors.Wrap(err, "failed to create default deny ACL policy egress")
7171
}
7272

7373
blockIngressACL, err := getDefaultDenyACLPolicy(hcn.DirectionTypeIn)
7474
if err != nil {
75-
return errors.Wrap(err, "Failed to create default deny ACL policy ingress")
75+
return []policy.Policy{}, errors.Wrap(err, "Failed to create default deny ACL policy ingress")
7676
}
7777

7878
additionalArgs := []policy.Policy{
@@ -86,9 +86,7 @@ func addDefaultDenyACL(podIPInfo *cns.PodIpInfo) error {
8686
},
8787
}
8888

89-
podIPInfo.EndpointPolicies = append(podIPInfo.EndpointPolicies, additionalArgs...)
90-
91-
return nil
89+
return additionalArgs, nil
9290
}
9391

9492
// create the default deny acl's that need to be added to the list defaultDenyACL field in podIpInfo

‎cns/middlewares/k8sSwiftV2_windows_test.go‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func TestAddDefaultDenyACL(t *testing.T) {
119119
"Priority": 10000
120120
}`)
121121

122-
expectedDefaultDenyACL := []policy.Policy{
122+
expectedDefaultDenyEndpoint := []policy.Policy{
123123
{
124124
Type: policy.EndpointPolicy,
125125
Data: valueOut,
@@ -130,23 +130,14 @@ func TestAddDefaultDenyACL(t *testing.T) {
130130
},
131131
}
132132

133-
podIPInfo := cns.PodIpInfo{
134-
PodIPConfig: cns.IPSubnet{
135-
IPAddress: "20.240.1.242",
136-
PrefixLength: 32,
137-
},
138-
NICType: cns.DelegatedVMNIC,
139-
MacAddress: "12:34:56:78:9a:bc",
140-
}
141-
142-
err := addDefaultDenyACL(&podIPInfo)
133+
defaultDenyEndpoint, err := addDefaultDenyACL()
143134
assert.Equal(t, err, nil)
144135

145136
// Normalize both slices so there is no extra spacing, new lines, etc
146-
normalizedExpected := normalizeKVPairs(t, expectedDefaultDenyACL)
147-
normalizedActual := normalizeKVPairs(t, podIPInfo.EndpointPolicies)
137+
normalizedExpected := normalizeKVPairs(t, expectedDefaultDenyEndpoint)
138+
normalizedActual := normalizeKVPairs(t, defaultDenyEndpoint)
148139
if !reflect.DeepEqual(normalizedExpected, normalizedActual) {
149-
t.Errorf("got '%+v', expected '%+v'", podIPInfo.EndpointPolicies, expectedDefaultDenyACL)
140+
t.Errorf("got '%+v', expected '%+v'", normalizedActual, normalizedExpected)
150141
}
151142
}
152143

0 commit comments

Comments
 (0)