Update the Type field name to PolicyType for SetPolicy #1194
Conversation
Signed-off-by: netal <netalgupta17@gmail.com>
|
@netal You'll need to run |
|
@dcantah Azure NPM is in the process of adding support for this setpolicy settings, you can see preliminary changes in here https://github.com/Azure/azure-container-networking/blob/1bcf6d80b54965515780e2d3f90d41598147f46e/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go#L153 |
|
This would be a breaking change if anyone is depending on the code as it exists today. Can you expand on why this change is needed? |
|
+1 with Kevin. You could add a json struct tag to have it be marshalled to the PolicyType name to avoid the actual field rename I suppose if this is really needed. type SetPolicySetting struct {
Id string
Name string
Type SetPolicyType `json:"PolicyType"`
Values string
} |
|
As of now, there are no customers using Setpolicy. Azure NPM is the first customer using this policy. |
Signed-off-by: netal <netalgupta17@gmail.com>
Do we know that no one in the community has taken a dependency on this code? I'd still like to understand better what motivates this change. Was a new field added to this struct internally? |
|
No. The "Type" field is an internal field which gets inherited from the base class of SetPolicy. All our automated testing calls HNS APIs directly and the field name is "PolicyType" in our internal mars file. Hence this bug was not caught through that. Once Azure NPM started using SetPolicy though hcsshim, they caught this bug. |
Signed-off-by: netal <netalgupta17@gmail.com>
|
@kevpar Do the new changes look alright? |
Related work items: microsoft#1067, microsoft#1097, microsoft#1119, microsoft#1170, microsoft#1176, microsoft#1180, microsoft#1181, microsoft#1182, microsoft#1183, microsoft#1184, microsoft#1185, microsoft#1186, microsoft#1187, microsoft#1188, microsoft#1189, microsoft#1191, microsoft#1193, microsoft#1194, microsoft#1195, microsoft#1196, microsoft#1197, microsoft#1200, microsoft#1201, microsoft#1202, microsoft#1203, microsoft#1204, microsoft#1205, microsoft#1206, microsoft#1207, microsoft#1209, microsoft#1210, microsoft#1211, microsoft#1218, microsoft#1219, microsoft#1220, microsoft#1223
|
@dcantah when can we have a release with this version ? |
Add json struct tag to SetPolicyType's Type field The "Type" fieldname was interfering with an internal "Type" field. Added a struct tag to marshal it as "PolicyType" instead. Signed-off-by: netal <netalgupta17@gmail.com>
The "Type" fieldname was interfering with a internal "Type" field. Changing it to "PolicyType"
Signed-off-by: netal netalgupta17@gmail.com