-
Notifications
You must be signed in to change notification settings - Fork 375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add antctl get fqdncache #6868
base: main
Are you sure you want to change the base?
Add antctl get fqdncache #6868
Changes from all commits
17487a4
a0e5592
e3ba43e
5a81c61
e77eead
2ed9291
fcab9bd
796eec1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,8 +15,10 @@ | |
package apis | ||
|
||
import ( | ||
"net" | ||
"strconv" | ||
"strings" | ||
"time" | ||
|
||
corev1 "k8s.io/api/core/v1" | ||
|
||
|
@@ -72,6 +74,28 @@ func (r AntreaAgentInfoResponse) SortRows() bool { | |
return true | ||
} | ||
|
||
type FqdnCacheResponse struct { | ||
fqdnName string | ||
ipAddress net.IP | ||
expirationTime time.Time | ||
} | ||
|
||
func (r FqdnCacheResponse) GetTableHeader() []string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you check if we can add specific test cases to cover this in code cov There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems to be left uncovered conventionally, as it is a simple method that primarily relies on the Transform() function to execute correctly, I don't think unit tests should be added for these funcs |
||
return []string{"FQDN", "ADDRESS", "EXPIRATION TIME"} | ||
} | ||
|
||
func (r FqdnCacheResponse) GetTableRow(maxColumn int) []string { | ||
return []string{ | ||
r.fqdnName, | ||
r.ipAddress.String(), | ||
r.expirationTime.String(), | ||
} | ||
} | ||
|
||
func (r FqdnCacheResponse) SortRows() bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is the default option for sorting false? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the rows of the table won't need to be sorted, as there's no filter or flag for it, so the default is false There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't remember how this function is used exactly, but it would be nice if the output rows were filtered based on the FQDN. |
||
return false | ||
} | ||
|
||
type FeatureGateResponse struct { | ||
Component string `json:"component,omitempty"` | ||
Name string `json:"name,omitempty"` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
// Copyright 2025 Antrea Authors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package fqdncache | ||
|
||
import ( | ||
"encoding/json" | ||
"net/http" | ||
|
||
"k8s.io/klog/v2" | ||
|
||
agentquerier "antrea.io/antrea/pkg/agent/querier" | ||
) | ||
|
||
func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc { | ||
return func(w http.ResponseWriter, r *http.Request) { | ||
dnsEntryCache := aq.GetFqdnCache() | ||
if dnsEntryCache == nil { | ||
return | ||
} | ||
Comment on lines
+29
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note that your There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that was for the older function, when it would return nil instead of empty, but that's fixed now, so I can remove this bit of code. |
||
if err := json.NewEncoder(w).Encode(dnsEntryCache); err != nil { | ||
http.Error(w, "Failed to encode response: "+err.Error(), http.StatusInternalServerError) | ||
klog.ErrorS(err, "Failed to encode response") | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
// Copyright 2025 Antrea Authors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package fqdncache | ||
|
||
import ( | ||
"encoding/json" | ||
"net" | ||
"net/http" | ||
"net/http/httptest" | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
"go.uber.org/mock/gomock" | ||
|
||
queriertest "antrea.io/antrea/pkg/agent/querier/testing" | ||
"antrea.io/antrea/pkg/agent/types" | ||
) | ||
|
||
func TestFqdnCacheQuery(t *testing.T) { | ||
tests := []struct { | ||
name string | ||
expectedStatus int | ||
expectedResponse []types.DnsCacheEntry | ||
}{ | ||
{ | ||
name: "FQDN cache exists", | ||
expectedStatus: http.StatusOK, | ||
expectedResponse: []types.DnsCacheEntry{ | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also add UT cases for 1. multiple addresses for the same FQDN 2. multiple FQDN <-> address mappings |
||
FqdnName: "example.com", | ||
IpAddress: net.ParseIP("10.0.0.1"), | ||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||
}, | ||
}, | ||
}, | ||
{ | ||
name: "FQDN cache exists - multiple addresses same domain", | ||
expectedStatus: http.StatusOK, | ||
expectedResponse: []types.DnsCacheEntry{ | ||
{ | ||
FqdnName: "example.com", | ||
IpAddress: net.ParseIP("10.0.0.1"), | ||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||
}, | ||
{ | ||
FqdnName: "example.com", | ||
IpAddress: net.ParseIP("10.0.0.2"), | ||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||
}, | ||
{ | ||
FqdnName: "example.com", | ||
IpAddress: net.ParseIP("10.0.0.3"), | ||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||
}, | ||
}, | ||
}, | ||
{ | ||
name: "FQDN cache exists - multiple addresses multiple domains", | ||
expectedStatus: http.StatusOK, | ||
expectedResponse: []types.DnsCacheEntry{ | ||
{ | ||
FqdnName: "example.com", | ||
IpAddress: net.ParseIP("10.0.0.1"), | ||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||
}, | ||
{ | ||
FqdnName: "foo.com", | ||
IpAddress: net.ParseIP("10.0.0.4"), | ||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||
}, | ||
{ | ||
FqdnName: "bar.com", | ||
IpAddress: net.ParseIP("10.0.0.5"), | ||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||
}, | ||
}, | ||
}, | ||
{ | ||
name: "FQDN cache does not exist", | ||
expectedStatus: http.StatusOK, | ||
expectedResponse: []types.DnsCacheEntry{}, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
ctrl := gomock.NewController(t) | ||
q := queriertest.NewMockAgentQuerier(ctrl) | ||
q.EXPECT().GetFqdnCache().Return(tt.expectedResponse) | ||
handler := HandleFunc(q) | ||
req, err := http.NewRequest(http.MethodGet, "", nil) | ||
require.NoError(t, err) | ||
recorder := httptest.NewRecorder() | ||
handler.ServeHTTP(recorder, req) | ||
assert.Equal(t, tt.expectedStatus, recorder.Code) | ||
if tt.expectedStatus == http.StatusOK { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would just remove the if here, as the status code is always 200 Note that this unit test actually validates very little (which is expected), so in my opinion we didn't really need multiple test cases, just the second one ( |
||
var receivedResponse []map[string]interface{} | ||
err = json.Unmarshal(recorder.Body.Bytes(), &receivedResponse) | ||
require.NoError(t, err) | ||
for i, rec := range receivedResponse { | ||
parsedTime, err := time.Parse(time.RFC3339, rec["expirationTime"].(string)) | ||
require.NoError(t, err) | ||
assert.Equal(t, tt.expectedResponse[i], types.DnsCacheEntry{ | ||
FqdnName: rec["fqdnName"].(string), | ||
IpAddress: net.ParseIP(rec["ipAddress"].(string)), | ||
ExpirationTime: parsedTime, | ||
}) | ||
} | ||
} | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -903,3 +903,61 @@ func TestValidate(t *testing.T) { | |||||||||
t.Fatalf("groupAddress %s expect %v, but got %v", groupAddress2, v1beta1.RuleActionDrop, item.RuleAction) | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
func TestGetFqdnCache(t *testing.T) { | ||||||||||
controller, _, _ := newTestController() | ||||||||||
expectedEntryList := []agenttypes.DnsCacheEntry{} | ||||||||||
assert.Equal(t, expectedEntryList, controller.GetFqdnCache()) | ||||||||||
|
||||||||||
controller.fqdnController.dnsEntryCache = map[string]dnsMeta{ | ||||||||||
"*.example.com": { | ||||||||||
responseIPs: map[string]ipWithExpiration{ | ||||||||||
"maps.example.com": { | ||||||||||
ip: net.ParseIP("10.0.0.1"), | ||||||||||
expirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||||||||||
}, | ||||||||||
"mail.example.com": { | ||||||||||
ip: net.ParseIP("10.0.0.2"), | ||||||||||
expirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||||||||||
}, | ||||||||||
"photos.example.com": { | ||||||||||
ip: net.ParseIP("10.0.0.3"), | ||||||||||
expirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||||||||||
}, | ||||||||||
}, | ||||||||||
}, | ||||||||||
"antrea.io": { | ||||||||||
responseIPs: map[string]ipWithExpiration{ | ||||||||||
"antrea.io": { | ||||||||||
ip: net.ParseIP("10.0.0.4"), | ||||||||||
expirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||||||||||
}, | ||||||||||
}, | ||||||||||
}, | ||||||||||
Comment on lines
+913
to
+936
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the test input data doesn't match what we actually store in the cache antrea/pkg/agent/controller/networkpolicy/fqdn.go Lines 79 to 82 in 357d38f
So it's actually like this: "antrea.io": {
responseIPs: map[string]ipWithExpiration{
""10.0.0.4"": {
ip: net.ParseIP("10.0.0.4"),
expirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC),
},
},
}, |
||||||||||
} | ||||||||||
|
||||||||||
expectedEntryList = []agenttypes.DnsCacheEntry{ | ||||||||||
{ | ||||||||||
FqdnName: "maps.example.com", | ||||||||||
IpAddress: net.ParseIP("10.0.0.1"), | ||||||||||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||||||||||
}, | ||||||||||
{ | ||||||||||
FqdnName: "mail.example.com", | ||||||||||
IpAddress: net.ParseIP("10.0.0.2"), | ||||||||||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||||||||||
}, | ||||||||||
{ | ||||||||||
FqdnName: "photos.example.com", | ||||||||||
IpAddress: net.ParseIP("10.0.0.3"), | ||||||||||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||||||||||
}, | ||||||||||
{ | ||||||||||
FqdnName: "antrea.io", | ||||||||||
IpAddress: net.ParseIP("10.0.0.4"), | ||||||||||
ExpirationTime: time.Date(2025, 12, 25, 15, 0, 0, 0, time.UTC), | ||||||||||
}, | ||||||||||
} | ||||||||||
returnedList := controller.GetFqdnCache() | ||||||||||
assert.ElementsMatch(t, expectedEntryList, returnedList) | ||||||||||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be
FQDNCacheResponse
per our coding conventionsyou can refer to Quan's document: https://github.com/tnqn/code-review-comments