Improve direct connections to Antrea API in antctl (#5135)

For some commands (get featuregates, supportbundle, proxy), antctl
connects directly to the Agent / Controller API when it is run from
outside of the cluster.

We try to address some shortcomings in the implementation:

1) Antctl was giving priority to the Node's InternalIP to determine how
   to connect to the API. This doesn't work when the machine on which
   antctl runs doesn't have connectivity to the InternalIP (e.g., if I
   am running antctl on my laptop and Antrea is installed in an EKS
   cluster). To fix this issue, we instead give priority to the Node's
   ExternalIP.
2) The connections were always "insecure" (no TLS verification). To fix
   this we need to retrieve the correct CA certificate and use it in the
   client TLS config. For the Controller, the CA certificate is
   available in the kube-ssytem/antrea-ca ConfigMap, which is easy to
   retrieve. For the Agent, the self-signed certificate is now published
   as part of the AntreaAgentInfo CRD (field name APICABundle), and
   hence is available to antctl. We use `[]byte` as the field type as it
   feels more common, but `string` would also have been acceptable for
   that type of data.

An `--insecure` flag is available for these commands, if users want to
fallback to the previous behavior.

Signed-off-by: Antonin Bas <[email protected]>

cmd/antrea-agent/agent.go

@@ -798,6 +798,7 @@ func run(o *Options) error {
 		statsCollector := stats.NewCollector(antreaClientProvider, ofClient, networkPolicyController, mcastController)
 		go statsCollector.Run(stopCh)
 	}
+
 	agentQuerier := querier.NewAgentQuerier(
 		nodeConfig,
 		networkConfig,
@@ -813,10 +814,6 @@ func run(o *Options) error {
 		nodeInformer.Lister(),
 	)
 
-	agentMonitor := monitor.NewAgentMonitor(crdClient, agentQuerier)
-
-	go agentMonitor.Run(stopCh)
-
 	if features.DefaultFeatureGate.Enabled(features.SupportBundleCollection) {
 		nodeNamespace := ""
 		nodeType := controlplane.SupportBundleCollectionNodeTypeNode
@@ -855,8 +852,22 @@ func run(o *Options) error {
 	if err != nil {
 		return fmt.Errorf("error when creating agent API server: %v", err)
 	}
+
+	// The certificate is static and will not be rotated; it will be re-generated if the Agent restarts.
+	agentAPICertData := apiServer.GetCertData()
+	if agentAPICertData == nil {
+		return fmt.Errorf("error when getting generated cert for agent API server")
+	}
+
 	go apiServer.Run(stopCh)
 
+	// The API certificate is passed on directly to the monitor, instead of being provided by
+	// the agentQuerier. This is to avoid a circular dependency between apiServer and
+	// agentQuerier. The apiServer already depends on the agentQuerier to implement some API
+	// handlers. The certificate data is only available after initializing the apiServer.
+	agentMonitor := monitor.NewAgentMonitor(crdClient, agentQuerier, agentAPICertData)
+	go agentMonitor.Run(stopCh)
+
 	// Start PacketIn
 	go ofClient.StartPacketInHandler(stopCh)
 

pkg/agent/apiserver/apiserver.go

@@ -52,7 +52,7 @@ import (
 	antreaversion "antrea.io/antrea/pkg/version"
 )
 
-const Name = "antrea-agent-api"
+const CertPairName = "antrea-agent-api"
 
 var (
 	scheme = runtime.NewScheme()
@@ -74,6 +74,15 @@ func (s *agentAPIServer) Run(stopCh <-chan struct{}) error {
 	return s.GenericAPIServer.PrepareRun().Run(stopCh)
 }
 
+func (s *agentAPIServer) GetCertData() []byte {
+	secureServingInfo := s.GenericAPIServer.SecureServingInfo
+	if secureServingInfo == nil {
+		return nil
+	}
+	cert, _ := secureServingInfo.Cert.CurrentCertKeyContent()
+	return cert
+}
+
 func installHandlers(aq agentquerier.AgentQuerier, npq querier.AgentNetworkPolicyInfoQuerier, mq querier.AgentMulticastInfoQuerier, seipq querier.ServiceExternalIPStatusQuerier, s *genericapiserver.GenericAPIServer) {
 	s.Handler.NonGoRestfulMux.HandleFunc("/loglevel", loglevel.HandleFunc())
 	s.Handler.NonGoRestfulMux.HandleFunc("/podmulticaststats", multicast.HandleFunc(mq))
@@ -116,7 +125,7 @@ func New(aq agentquerier.AgentQuerier,
 	if err != nil {
 		return nil, err
 	}
-	s, err := cfg.New(Name, genericapiserver.NewEmptyDelegate())
+	s, err := cfg.New(CertPairName, genericapiserver.NewEmptyDelegate())
 	if err != nil {
 		return nil, err
 	}
@@ -143,7 +152,7 @@ func newConfig(aq agentquerier.AgentQuerier,
 
 	// Set the PairName but leave certificate directory blank to generate in-memory by default.
 	secureServing.ServerCert.CertDirectory = ""
-	secureServing.ServerCert.PairName = Name
+	secureServing.ServerCert.PairName = CertPairName
 
 	if err := secureServing.MaybeDefaultWithSelfSignedCerts("localhost", nil, []net.IP{net.ParseIP("127.0.0.1"), net.IPv6loopback}); err != nil {
 		return nil, fmt.Errorf("error creating self-signed certificates: %v", err)

pkg/antctl/raw/featuregates/command.go

@@ -36,6 +36,10 @@ var Command *cobra.Command
 var getClients = getConfigAndClients
 var getRestClient = getRestClientByMode
 
+var option = &struct {
+	insecure bool
+}{}
+
 func init() {
 	Command = &cobra.Command{
 		Use:   "featuregates",
@@ -49,6 +53,7 @@ func init() {
 		Command.Long = "Print Antrea feature gates info including Controller and Agent"
 	} else if runtime.Mode == runtime.ModeController && !runtime.InPod {
 		Command.Long = "Print Antrea feature gates info including Controller and Agent"
+		Command.Flags().BoolVar(&option.insecure, "insecure", false, "Skip TLS verification when connecting to Antrea API.")
 		Command.RunE = controllerRemoteRunE
 	}
 }
@@ -66,12 +71,13 @@ func controllerRemoteRunE(cmd *cobra.Command, _ []string) error {
 }
 
 func featureGateRequest(cmd *cobra.Command, mode string) error {
+	ctx := cmd.Context()
 	kubeconfig, k8sClientset, antreaClientset, err := getClients(cmd)
 	if err != nil {
 		return err
 	}
 
-	client, err := getRestClient(kubeconfig, k8sClientset, antreaClientset, mode)
+	client, err := getRestClient(ctx, kubeconfig, k8sClientset, antreaClientset, mode)
 	if err != nil {
 		return err
 	}
@@ -114,26 +120,26 @@ func getConfigAndClients(cmd *cobra.Command) (*rest.Config, kubernetes.Interface
 	return kubeconfig, k8sClientset, antreaClientset, nil
 }
 
-func getRestClientByMode(kubeconfig *rest.Config, k8sClientset kubernetes.Interface, antreaClientset antrea.Interface, mode string) (*rest.RESTClient, error) {
-	kubeconfig.GroupVersion = &schema.GroupVersion{Group: "", Version: ""}
-	restconfigTmpl := rest.CopyConfig(kubeconfig)
-	raw.SetupKubeconfig(restconfigTmpl)
+func getRestClientByMode(ctx context.Context, kubeconfig *rest.Config, k8sClientset kubernetes.Interface, antreaClientset antrea.Interface, mode string) (*rest.RESTClient, error) {
+	cfg := rest.CopyConfig(kubeconfig)
+	cfg.GroupVersion = &schema.GroupVersion{Group: "", Version: ""}
 	var err error
 	var client *rest.RESTClient
 	switch mode {
 	case runtime.ModeAgent, runtime.ModeController:
-		client, err = rest.RESTClientFor(restconfigTmpl)
+		raw.SetupLocalKubeconfig(cfg)
+		client, err = rest.RESTClientFor(cfg)
 	case "remote":
-		client, err = getControllerClient(k8sClientset, antreaClientset, restconfigTmpl)
+		client, err = getControllerClient(ctx, k8sClientset, antreaClientset, cfg, option.insecure)
 	}
 	if err != nil {
 		return nil, fmt.Errorf("failed to create rest client: %w", err)
 	}
 	return client, nil
 }
 
-func getControllerClient(k8sClientset kubernetes.Interface, antreaClientset antrea.Interface, cfgTmpl *rest.Config) (*rest.RESTClient, error) {
-	controllerClientCfg, err := raw.CreateControllerClientCfg(k8sClientset, antreaClientset, cfgTmpl)
+func getControllerClient(ctx context.Context, k8sClientset kubernetes.Interface, antreaClientset antrea.Interface, kubeconfig *rest.Config, insecure bool) (*rest.RESTClient, error) {
+	controllerClientCfg, err := raw.CreateControllerClientCfg(ctx, k8sClientset, antreaClientset, kubeconfig, insecure)
 	if err != nil {
 		return nil, fmt.Errorf("error when creating controller client config: %w", err)
 	}

pkg/antctl/raw/featuregates/command_test.go

@@ -16,6 +16,7 @@ package featuregates
 
 import (
 	"bytes"
+	"context"
 	"io"
 	"net/http"
 	"os"
@@ -305,9 +306,9 @@ func TestGetFeatureGates(t *testing.T) {
 	}
 }
 
-func getFakeFunc(response []byte) func(kubeconfig *rest.Config, k8sClientset kubernetes.Interface, antreaClientset antrea.Interface, mode string) (*rest.RESTClient, error) {
+func getFakeFunc(response []byte) func(ctx context.Context, kubeconfig *rest.Config, k8sClientset kubernetes.Interface, antreaClientset antrea.Interface, mode string) (*rest.RESTClient, error) {
 	restClient, _ := rest.RESTClientFor(clientConfig)
-	return func(kubeconfig *rest.Config, k8sClientset kubernetes.Interface, antreaClientset antrea.Interface, mode string) (*rest.RESTClient, error) {
+	return func(ctx context.Context, kubeconfig *rest.Config, k8sClientset kubernetes.Interface, antreaClientset antrea.Interface, mode string) (*rest.RESTClient, error) {
 		fakeHttpClient := fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
 			return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewBuffer(response))}, nil
 		})
@@ -350,40 +351,3 @@ kind: Config`)
 	require.NoError(t, err)
 	assert.Equal(t, "http://192.168.1.10", newconfig.Host)
 }
-
-func TestGetRestClientByMode(t *testing.T) {
-	k8sClient := k8sfake.NewSimpleClientset(node1.DeepCopyObject())
-	tests := []struct {
-		name            string
-		expectedErr     string
-		antreaClientset antrea.Interface
-		mode            string
-	}{
-		{
-			name: "get rest client by agent mode successfully",
-			mode: "agent",
-		},
-		{
-			name:            "get rest client by remote mode successfully",
-			antreaClientset: antreafakeclient.NewSimpleClientset(controllerInfo.DeepCopyObject()),
-			mode:            "remote",
-		},
-		{
-			name:            "failed to get rest client by remote mode",
-			expectedErr:     "failed to create rest client: error when creating controller client config: antreacontrollerinfos.crd.antrea.io \"antrea-controller\" not found",
-			antreaClientset: antreafakeclient.NewSimpleClientset(),
-			mode:            "remote",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			_, err := getRestClientByMode(clientConfig, k8sClient, tt.antreaClientset, tt.mode)
-			if tt.expectedErr == "" {
-				assert.NoError(t, err)
-			} else {
-				assert.EqualError(t, err, tt.expectedErr)
-			}
-		})
-	}
-}