Skip to content
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 query for policy conjunction ID #6470

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkg/agent/apis/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,13 @@ func (r PodInterfaceResponse) SortRows() bool {
return true
}

// PolicyRuleConjunctionIDsResponse describes the response struct of a policy-rule-conjunction-ids command.
type PolicyRuleConjunctionIDsResponse struct {
RuleName string `json:"name,omitempty"`
Direction string `json:"direction,omitempty"`
ConjunctionIDs []uint32 `json:"conjunctionIDs,omitempty"`
}

// ServiceExternalIPInfo contains the essential information for Services with type of Loadbalancer managed by Antrea.
type ServiceExternalIPInfo struct {
ServiceName string `json:"serviceName,omitempty" antctl:"name,Name of the Service"`
Expand Down
2 changes: 2 additions & 0 deletions pkg/agent/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"antrea.io/antrea/pkg/agent/apiserver/handlers/ovsflows"
"antrea.io/antrea/pkg/agent/apiserver/handlers/ovstracing"
"antrea.io/antrea/pkg/agent/apiserver/handlers/podinterface"
"antrea.io/antrea/pkg/agent/apiserver/handlers/policyconjunctions"
"antrea.io/antrea/pkg/agent/apiserver/handlers/serviceexternalip"
agentquerier "antrea.io/antrea/pkg/agent/querier"
systeminstall "antrea.io/antrea/pkg/apis/system/install"
Expand Down Expand Up @@ -93,6 +94,7 @@ func installHandlers(aq agentquerier.AgentQuerier, npq querier.AgentNetworkPolic
s.Handler.NonGoRestfulMux.HandleFunc("/networkpolicies", networkpolicy.HandleFunc(aq))
s.Handler.NonGoRestfulMux.HandleFunc("/appliedtogroups", appliedtogroup.HandleFunc(npq))
s.Handler.NonGoRestfulMux.HandleFunc("/addressgroups", addressgroup.HandleFunc(npq))
s.Handler.NonGoRestfulMux.HandleFunc("/policyconjunctions", policyconjunctions.HandleFunc(aq))
s.Handler.NonGoRestfulMux.HandleFunc("/ovsflows", ovsflows.HandleFunc(aq))
s.Handler.NonGoRestfulMux.HandleFunc("/ovstracing", ovstracing.HandleFunc(aq))
s.Handler.NonGoRestfulMux.HandleFunc("/serviceexternalip", serviceexternalip.HandleFunc(seipq))
Expand Down
5 changes: 2 additions & 3 deletions pkg/agent/apiserver/handlers/networkpolicy/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,10 @@ func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc {

// From user shorthand input to cpv1beta1.NetworkPolicyType
var mapToNetworkPolicyType = map[string]cpv1beta.NetworkPolicyType{
"NP": cpv1beta.K8sNetworkPolicy,
"K8SNP": cpv1beta.K8sNetworkPolicy,
"ACNP": cpv1beta.AntreaClusterNetworkPolicy,
"ANNP": cpv1beta.AntreaNetworkPolicy,
"ANP": cpv1beta.AntreaNetworkPolicy,
"ANP": cpv1beta.AdminNetworkPolicy,
Dyanngg marked this conversation as resolved.
Show resolved Hide resolved
}

// Create a Network Policy Filter from URL Query
Expand All @@ -79,7 +78,7 @@ func newFilterFromURLQuery(query url.Values) (*querier.NetworkPolicyQueryFilter,
strSourceType := strings.ToUpper(query.Get("type"))
npSourceType, ok := mapToNetworkPolicyType[strSourceType]
if strSourceType != "" && !ok {
return nil, "", fmt.Errorf("invalid policy source type. Valid values are K8sNP, ACNP, ANNP and ANP (deprecated)")
return nil, "", fmt.Errorf("invalid policy source type. Valid values are K8sNP, ACNP, ANNP and ANP")
}
source := query.Get("source")
name := query.Get("name")
Expand Down
79 changes: 79 additions & 0 deletions pkg/agent/apiserver/handlers/policyconjunctions/handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright 2024 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 policyconjunctions

import (
"encoding/json"
"net/http"
"strings"

"k8s.io/apimachinery/pkg/util/sets"

agentquerier "antrea.io/antrea/pkg/agent/querier"
cpv1beta "antrea.io/antrea/pkg/apis/controlplane/v1beta2"
"antrea.io/antrea/pkg/querier"
)

// From user shorthand input to cpv1beta1.NetworkPolicyType
var mapToNetworkPolicyType = map[string]cpv1beta.NetworkPolicyType{
"K8SNP": cpv1beta.K8sNetworkPolicy,
"ACNP": cpv1beta.AntreaClusterNetworkPolicy,
"ANNP": cpv1beta.AntreaNetworkPolicy,
"ANP": cpv1beta.AdminNetworkPolicy,
}

var clusterScopedResources = sets.New[string]("ACNP", "ANP")

// HandleFunc creates a http.HandlerFunc which uses an AgentNetworkPolicyInfoQuerier
// to query network policy rules in current agent.
func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it make sense to add this information (list of conj IDs) to the existing networkpolicy handler instead of defining a new HTTP endpoint? We can always enrich it if we need to support a few additional query parameters? Likewise, maybe we could use the existing antctl command and have a flag to display conjunction IDs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of now the response type of the antctl get networkpolicy command is basically controlplane.NetworkPolicy since it's a shared endpoint between controller and agent. As a result it might be a bit tricky to add a conjunction ID field to the Response type, as for the controller it is this antctl command is a resourceEndpoint command https://github.com/antrea-io/antrea/blob/main/pkg/antctl/antctl.go#L227

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some differences already in the Agent command compared to the Controller command, and the handlers are different, but I think I see your point: we still use the same response type for both handlers, and it may not be a good idea to change that.

Is there any information that would be useful for debugging on the Agent-side besides conjunction IDs? I am wondering if we should have a more generic (but still Agent-specific) endpoint / command, not one named specifically "policyconjunctions". For example, would it make sense to also include the ruleID in the response, or anything else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ummm if you are talking about the ID of the rule stored in agent, usually it does not carry information that we can use during debugging. It is a hash that's computed based on all fields of a rule and not really relatable to any controlplane rule info.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #6487 which fixed some old antctl ovs-dump command that would essentially show conjunction ID information (buried in the flows). However, it feels to me that it's still quite different from the command provided in this PR: the ovs-dump returns the datapath state, which, in times of troubleshooting, might return empty flows based on the match keys stored. Wondering if this PR is still warranted @tnqn

return func(w http.ResponseWriter, r *http.Request) {
query := r.URL.Query()
uid := query.Get("uid")
npFilter := &querier.NetworkPolicyQueryFilter{Name: uid}
if uid == "" {
if query.Get("source") == "" {
http.Error(w, "policy uid or name must be provided", http.StatusBadRequest)
return
}
policyType := strings.ToUpper(query.Get("type"))
cpType, ok := mapToNetworkPolicyType[policyType]
if !ok {
http.Error(w, "valid policy type must be provided with policy name", http.StatusBadRequest)
return
}
if !clusterScopedResources.Has(policyType) && query.Get("namespace") == "" {
http.Error(w, "policy Namespace must be provided for policy type "+policyType, http.StatusBadRequest)
return
}
npFilter = &querier.NetworkPolicyQueryFilter{
SourceName: query.Get("source"),
Namespace: query.Get("namespace"),
SourceType: cpType,
}
}
npq := aq.GetNetworkPolicyInfoQuerier()
policies := npq.GetNetworkPolicies(npFilter)
if len(policies) == 0 {
w.WriteHeader(http.StatusNotFound)
return
}
uid = string(policies[0].SourceRef.UID)
realizedRules := npq.GetRealizedRulesByPolicy(uid)
if err := json.NewEncoder(w).Encode(realizedRules); err != nil {
http.Error(w, "Failed to encode response: "+err.Error(), http.StatusInternalServerError)
}
}
}
99 changes: 99 additions & 0 deletions pkg/agent/apiserver/handlers/policyconjunctions/handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Copyright 2024 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 policyconjunctions

import (
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

aqtest "antrea.io/antrea/pkg/agent/querier/testing"
cpv1beta "antrea.io/antrea/pkg/apis/controlplane/v1beta2"
qtest "antrea.io/antrea/pkg/querier/testing"
)

func TestBadRequests(t *testing.T) {
badRequests := map[string]string{
"Policy name only": "?source=allow-http",
"No policy type": "?source=allow-http&namespace=ns1",
"No namespace for ANNP": "?source=allow-http&type=ANNP",
"No namespace for K8s NP": "?source=allow-http&type=K8sNP",
}
handler := HandleFunc(nil)
for k, r := range badRequests {
req, err := http.NewRequest(http.MethodGet, r, nil)
assert.Nil(t, err)
recorder := httptest.NewRecorder()
handler.ServeHTTP(recorder, req)
assert.Equal(t, http.StatusBadRequest, recorder.Code, k)
}
}

func TestPolicyConjunctionsQuery(t *testing.T) {
c := gomock.NewController(t)
tests := []struct {
name string
query string
policiesReturned []cpv1beta.NetworkPolicy
expectedStatus int
}{
{
name: "policy found",
query: "?uid=uid1",
policiesReturned: []cpv1beta.NetworkPolicy{
{
ObjectMeta: metav1.ObjectMeta{
Name: "uid1",
},
SourceRef: &cpv1beta.NetworkPolicyReference{
Type: cpv1beta.AntreaClusterNetworkPolicy,
Name: "test-acnp",
UID: "uid1",
},
},
},
expectedStatus: http.StatusOK,
},
{
name: "policy not found",
query: "?uid=uid2",
policiesReturned: []cpv1beta.NetworkPolicy{},
expectedStatus: http.StatusNotFound,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
npQuerier := qtest.NewMockAgentNetworkPolicyInfoQuerier(c)
npQuerier.EXPECT().GetNetworkPolicies(gomock.Any()).Return(tt.policiesReturned).Times(1)
if len(tt.policiesReturned) == 1 {
npQuerier.EXPECT().GetRealizedRulesByPolicy(string(tt.policiesReturned[0].SourceRef.UID)).Times(1)
}
aq := aqtest.NewMockAgentQuerier(c)
aq.EXPECT().GetNetworkPolicyInfoQuerier().Return(npQuerier).Times(1)

handler := HandleFunc(aq)
req, err := http.NewRequest(http.MethodGet, tt.query, nil)
assert.Nil(t, err)

recorder := httptest.NewRecorder()
handler.ServeHTTP(recorder, req)
assert.Equal(t, tt.expectedStatus, recorder.Code)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"k8s.io/client-go/util/workqueue"
"k8s.io/klog/v2"

"antrea.io/antrea/pkg/agent/apis"
"antrea.io/antrea/pkg/agent/client"
"antrea.io/antrea/pkg/agent/config"
"antrea.io/antrea/pkg/agent/controller/networkpolicy/l7engine"
Expand Down Expand Up @@ -583,6 +584,10 @@ func (c *Controller) GetRuleByFlowID(ruleFlowID uint32) *types.PolicyRule {
return rule
}

func (c *Controller) GetRealizedRulesByPolicy(uid string) []apis.PolicyRuleConjunctionIDsResponse {
return c.podReconciler.GetRealizedRulesByPolicy(uid)
}

func (c *Controller) GetControllerConnectionStatus() bool {
// When the watchers are connected, controller connection status is true. Otherwise, it is false.
return c.addressGroupWatcher.isConnected() && c.appliedToGroupWatcher.isConnected() && c.networkPolicyWatcher.isConnected()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
k8stesting "k8s.io/client-go/testing"
"k8s.io/component-base/metrics/legacyregistry"

"antrea.io/antrea/pkg/agent/apis"
"antrea.io/antrea/pkg/agent/config"
"antrea.io/antrea/pkg/agent/controller/networkpolicy/l7engine"
"antrea.io/antrea/pkg/agent/metrics"
Expand Down Expand Up @@ -169,6 +170,10 @@ func (r *mockReconciler) GetRuleByFlowID(_ uint32) (*agenttypes.PolicyRule, bool
return nil, false, nil
}

func (r *mockReconciler) GetRealizedRulesByPolicy(_ string) []apis.PolicyRuleConjunctionIDsResponse {
return nil
}

func (r *mockReconciler) getLastRealized(ruleID string) (*CompletedRule, bool) {
r.Lock()
defer r.Unlock()
Expand Down
5 changes: 5 additions & 0 deletions pkg/agent/controller/networkpolicy/node_reconciler_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"

"antrea.io/antrea/pkg/agent/apis"
"antrea.io/antrea/pkg/agent/config"
"antrea.io/antrea/pkg/agent/route"
"antrea.io/antrea/pkg/agent/types"
Expand Down Expand Up @@ -320,6 +321,10 @@ func (r *nodeReconciler) GetRuleByFlowID(ruleFlowID uint32) (*types.PolicyRule,
return nil, false, nil
}

func (r *nodeReconciler) GetRealizedRulesByPolicy(uid string) []apis.PolicyRuleConjunctionIDsResponse {
return nil
}

func (r *nodeReconciler) computeIPTRules(rule *CompletedRule) (map[iptables.Protocol]*types.NodePolicyRule, *nodePolicyLastRealized) {
ruleID := rule.ID
lastRealized := newNodePolicyLastRealized()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package networkpolicy

import (
"antrea.io/antrea/pkg/agent/apis"
"antrea.io/antrea/pkg/agent/route"
"antrea.io/antrea/pkg/agent/types"
)
Expand All @@ -44,6 +45,10 @@ func (r *nodeReconciler) GetRuleByFlowID(ruleID uint32) (*types.PolicyRule, bool
return nil, false, nil
}

func (r *nodeReconciler) GetRealizedRulesByPolicy(uid string) []apis.PolicyRuleConjunctionIDsResponse {
return nil
}

func (r *nodeReconciler) RunIDAllocatorWorker(stopCh <-chan struct{}) {

}
27 changes: 26 additions & 1 deletion pkg/agent/controller/networkpolicy/pod_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog/v2"

"antrea.io/antrea/pkg/agent/apis"
"antrea.io/antrea/pkg/agent/interfacestore"
"antrea.io/antrea/pkg/agent/openflow"
proxytypes "antrea.io/antrea/pkg/agent/proxy/types"
Expand Down Expand Up @@ -69,6 +70,9 @@ type Reconciler interface {
// GetRuleByFlowID returns the rule from the async rule cache in idAllocator cache.
GetRuleByFlowID(ruleID uint32) (*types.PolicyRule, bool, error)

// GetRealizedRulesByPolicy returns the conjunction info of the queried policy from the lastRealized cache.
GetRealizedRulesByPolicy(uid string) []apis.PolicyRuleConjunctionIDsResponse

// RunIDAllocatorWorker runs the worker that deletes the rules from the cache
// in idAllocator.
RunIDAllocatorWorker(stopCh <-chan struct{})
Expand Down Expand Up @@ -162,7 +166,7 @@ type podPolicyLastRealized struct {
// It's same in all Openflow rules, because named port is only for
// destination Pods.
podIPs sets.Set[string]
// fqdnIPaddresses tracks the last realized set of IP addresses resolved for
// fqdnIPAddresses tracks the last realized set of IP addresses resolved for
// the fqdn selector of this policy rule. It must be empty for policy rule
// that is not egress and does not have toFQDN field.
fqdnIPAddresses sets.Set[string]
Expand Down Expand Up @@ -1046,6 +1050,27 @@ func (r *podReconciler) GetRuleByFlowID(ruleFlowID uint32) (*types.PolicyRule, b
return r.idAllocator.getRuleFromAsyncCache(ruleFlowID)
}

func (r *podReconciler) GetRealizedRulesByPolicy(uid string) []apis.PolicyRuleConjunctionIDsResponse {
var responses []apis.PolicyRuleConjunctionIDsResponse
r.lastRealizeds.Range(func(k, v interface{}) bool {
r := v.(*podPolicyLastRealized)
if string(r.SourceRef.UID) == uid {
resp := apis.PolicyRuleConjunctionIDsResponse{
RuleName: r.Name,
Direction: string(r.Direction),
}
ofIDs := make([]uint32, 0, len(r.ofIDs))
for _, ofID := range r.ofIDs {
ofIDs = append(ofIDs, ofID)
}
resp.ConjunctionIDs = ofIDs
responses = append(responses, resp)
}
return true
})
return responses
}

func (r *podReconciler) getOFPorts(members v1beta2.GroupMemberSet) sets.Set[int32] {
ofPorts := sets.New[int32]()
for _, m := range members {
Expand Down
Loading
Loading