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.

Sign up for GitHub

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

Jump to bottom

Improve direct connections to Antrea API in antctl #5135

Merged
merged 2 commits into from
Jun 21, 2023
Merged

Improve direct connections to Antrea API in antctl #5135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
98af1ca
Improve direct connections to Antrea API in antctl
antoninbas Jun 15, 2023
94eba8b
Rename apiCertData to apiCABundle in CRD
antoninbas Jun 20, 2023
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
19 changes: 15 additions & 4 deletions cmd/antrea-agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down
15 changes: 12 additions & 3 deletions pkg/agent/apiserver/apiserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Copy link
Member

Choose a reason for hiding this comment

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

I remember this will include both the generate cert and the CA cert. Should we extract the CA cert and publish it only to reduce the overall size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I think something like this would work:

block, _ := pem.Decode(cert)
certs := x509.ParseCertificates(block)
intermediates := certs[1:] // distribute this only

I am not sure this is the best approach. It feels like it's more common to distribute the cert "file" as is, in the self-signed case. That's also what we do for the Antrea Controller (when we call dynamiccertificates.NewDynamicCAContentFromFile after generating the certificate). That may be best practice for a standalone self-signed certificate (as opposed to the case where the same self-signed CA is used to generate multiple certificates). I feel more comfortable keeping it this way for now. What do you think?

By renaming the field to APICABundle as you suggested, we are free to change the contents of the published data later and remove the leaf certificate.

Copy link
Member

Choose a reason for hiding this comment

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

works for me

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))
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
Expand Down
24 changes: 15 additions & 9 deletions pkg/antctl/raw/featuregates/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
}
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down
42 changes: 3 additions & 39 deletions pkg/antctl/raw/featuregates/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package featuregates

import (
"bytes"
"context"
"io"
"net/http"
"os"
Expand Down Expand Up @@ -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
})
Expand Down Expand Up @@ -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)
}
})
}
}
Loading