feat: add unit tests for CNS endpoint handler APIs#4344
feat: add unit tests for CNS endpoint handler APIs#4344behzad-mir wants to merge 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds unit test coverage for CNS endpoint handler APIs (GetEndpoint/GetEndpointState/Update/Delete flows) and updates CNS REST server structs to include JSON tags so responses/state can be serialized/deserialized consistently.
Changes:
- Added a new
endpoint_handler_test.gowith unit tests covering helper functions and HTTP handler behavior for endpoint state management. - Added JSON tags to
EndpointInfo,IPInfo, andResponseincns/restserver/restserver.go.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
cns/restserver/restserver.go |
Adds JSON tags to endpoint/state response structs to support consistent JSON encoding/decoding. |
cns/restserver/endpoint_handler_test.go |
Introduces new unit tests for endpoint helper methods and the EndpointHandlerAPI HTTP dispatch behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
90f08ab to
7aabc7d
Compare
Adds comprehensive unit tests for: - GetEndpointState API - GetEndpoint API - EndpointStateHandler Also adds JSON tags to EndpointInfo, IPInfo, and Response structs for proper serialization in tests and API responses.
7aabc7d to
401eb65
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| PodName string `json:"PodName,omitempty"` | ||
| PodNamespace string `json:"PodNamespace,omitempty"` | ||
| IfnameToIPMap map[string]*IPInfo `json:"IfnameToIPMap,omitempty"` // key : interface name, value : IPInfo |
There was a problem hiding this comment.
Adding omitempty on PodName, PodNamespace, and IfnameToIPMap changes the JSON output for GetEndpointResponse and the on-disk endpoint state (empty values will now be omitted rather than present as empty string/empty object). If this is intended to be backward-compatible for existing clients/state files, consider removing omitempty (or only applying it where the API contract explicitly allows missing fields).
| PodName string `json:"PodName,omitempty"` | |
| PodNamespace string `json:"PodNamespace,omitempty"` | |
| IfnameToIPMap map[string]*IPInfo `json:"IfnameToIPMap,omitempty"` // key : interface name, value : IPInfo | |
| PodName string `json:"PodName"` | |
| PodNamespace string `json:"PodNamespace"` | |
| IfnameToIPMap map[string]*IPInfo `json:"IfnameToIPMap"` // key : interface name, value : IPInfo |
| httpsvc, err := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, | ||
| &IPtablesProvider{}, &fakes.NMAgentClientFake{}, store.NewMockStore(""), nil, nil, | ||
| fakes.NewMockIMDSClient()) | ||
| require.NoError(t, err, "NewHTTPRestService should not return an error") | ||
| require.NotNil(t, httpsvc, "HTTPRestService should not be nil") | ||
|
|
||
| // Enable endpoint state management (required for stateless CNI) | ||
| httpsvc.Options = make(map[string]interface{}) | ||
| httpsvc.Options[acn.OptManageEndpointState] = true | ||
|
|
||
| // Initialize endpoint state store | ||
| httpsvc.EndpointStateStore = store.NewMockStore("") |
There was a problem hiding this comment.
NewHTTPRestService already initializes Options, and this helper reassigns it (and later creates a new EndpointStateStore). Consider just setting httpsvc.Options[acn.OptManageEndpointState] = true and reusing the store instance to avoid masking default initialization behavior and allocating extra mock stores.
| httpsvc, err := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, | |
| &IPtablesProvider{}, &fakes.NMAgentClientFake{}, store.NewMockStore(""), nil, nil, | |
| fakes.NewMockIMDSClient()) | |
| require.NoError(t, err, "NewHTTPRestService should not return an error") | |
| require.NotNil(t, httpsvc, "HTTPRestService should not be nil") | |
| // Enable endpoint state management (required for stateless CNI) | |
| httpsvc.Options = make(map[string]interface{}) | |
| httpsvc.Options[acn.OptManageEndpointState] = true | |
| // Initialize endpoint state store | |
| httpsvc.EndpointStateStore = store.NewMockStore("") | |
| mockStore := store.NewMockStore("") | |
| httpsvc, err := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, | |
| &IPtablesProvider{}, &fakes.NMAgentClientFake{}, mockStore, nil, nil, | |
| fakes.NewMockIMDSClient()) | |
| require.NoError(t, err, "NewHTTPRestService should not return an error") | |
| require.NotNil(t, httpsvc, "HTTPRestService should not be nil") | |
| // Enable endpoint state management (required for stateless CNI) | |
| httpsvc.Options[acn.OptManageEndpointState] = true | |
| // Initialize endpoint state |
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| err := service.UpdateEndpointHelper(tt.endpointID, tt.updateReq) | ||
| if tt.wantErr { | ||
| require.Error(t, err) |
There was a problem hiding this comment.
This loop runs subtests against a shared service instance, so cases can depend on state mutated by earlier cases. To keep tests independent and easier to maintain, consider creating a fresh service (and persisting initial state) per t.Run, or resetting the store/state between cases.
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| req := httptest.NewRequestWithContext(context.Background(), http.MethodDelete, tt.path, http.NoBody) | ||
| w := httptest.NewRecorder() | ||
|
|
There was a problem hiding this comment.
These subtests reuse a single service instance, which couples the cases (the first DELETE mutates state/store that subsequent cases observe). Consider instantiating a new service per case (or resetting state) so each t.Run is isolated and future parallelization/reordering won’t break expectations.
Adds comprehensive unit tests for:
Also adds JSON tags to EndpointInfo, IPInfo, and Response structs for proper serialization in tests and API responses.
CNS Endpoint Handler Tests
File:
cns/restserver/endpoint_handler_test.go(NEW)TestGetEndpointHelperTestGetEndpointHelper_LegacyFormatTestUpdateEndpointHelperTestDeleteEndpointStateHelperTestEndpointHandlerAPI_GetMethodTestEndpointHandlerAPI_PatchMethodTestEndpointHandlerAPI_DeleteMethodTestEndpointHandlerAPI_OptManageEndpointStateDisabledTestStatelessCNI_EndToEnd_FlowTestStatelessCNI_SwiftV2_MultiNICCNS Util Tests
File:
cns/restserver/util_test.goTestRestoreStateVerification
Requirements:
Notes: