Skip to content

Commit

Permalink
Add command args to configure Isitod with externally managed certific…
Browse files Browse the repository at this point in the history
…ates (#23116)

* Read sidecar-injector certificates default values from env variables if it exists

* rename vars

* Add prefix INJECTION_WEBHOOK_

* Add optional defaults for Istiod servers TLS certs

* Fix lint checks

* Add command args to configure Isitod with externally managed certificates

* Handler for failed CA read
  • Loading branch information
shakti-das authored Apr 29, 2020
1 parent c3909a7 commit 1520f76
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 25 deletions.
8 changes: 8 additions & 0 deletions pilot/cmd/pilot-discovery/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,14 @@ func init() {
discoveryCmd.PersistentFlags().BoolVar(&serverArgs.DiscoveryOptions.EnableProfiling, "profile", true,
"Enable profiling via web interface host:port/debug/pprof")

// Use TLS certificates if provided.
discoveryCmd.PersistentFlags().StringVar(&serverArgs.TLSOptions.CaCertFile, "caCertFile", "",
"File containing the x509 Server CA Certificate")
discoveryCmd.PersistentFlags().StringVar(&serverArgs.TLSOptions.CertFile, "tlsCertFile", "",
"File containing the x509 Server Certificate")
discoveryCmd.PersistentFlags().StringVar(&serverArgs.TLSOptions.KeyFile, "tlsKeyFile", "",
"File containing the x509 private key matching --tlsCertFile")

// Attach the Istio logging options to the command.
loggingOptions.AttachCobraFlags(rootCmd)

Expand Down
9 changes: 9 additions & 0 deletions pilot/pkg/bootstrap/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ type PilotArgs struct {
KeepaliveOptions *istiokeepalive.Options
// ForceStop is set as true when used for testing to make the server stop quickly
ForceStop bool
// Optional TLS configuration
TLSOptions TLSOptions
}

// DiscoveryServiceOptions contains options for create a new discovery
Expand Down Expand Up @@ -118,6 +120,13 @@ type MCPOptions struct {
InitialConnWindowSize int
}

// Optional TLS parameters for the server.
type TLSOptions struct {
CaCertFile string
CertFile string
KeyFile string
}

var PodNamespaceVar = env.RegisterStringVar("POD_NAMESPACE", "istio-system", "")
var podNameVar = env.RegisterStringVar("POD_NAME", "", "")
var serviceAccountVar = env.RegisterStringVar("SERVICE_ACCOUNT", "", "")
Expand Down
43 changes: 31 additions & 12 deletions pilot/pkg/bootstrap/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"crypto/tls"
"crypto/x509"
"fmt"
"io/ioutil"
"net"
"net/http"
"path"
Expand Down Expand Up @@ -219,7 +220,7 @@ func NewServer(args *PilotArgs) (*Server, error) {
}

// CA signing certificate must be created first.
if s.EnableCA() {
if args.TLSOptions.CaCertFile == "" && s.EnableCA() {
var err error
var corev1 v1.CoreV1Interface
if s.kubeClient != nil {
Expand Down Expand Up @@ -274,7 +275,7 @@ func NewServer(args *PilotArgs) (*Server, error) {
return nil, fmt.Errorf("error initializing cluster registries: %v", err)
}
if dns.DNSAddr.Get() != "" {
if err := s.initDNSTLSListener(dns.DNSAddr.Get()); err != nil {
if err := s.initDNSTLSListener(dns.DNSAddr.Get(), args.TLSOptions); err != nil {
log.Warna("error initializing DNS-over-TLS listener ", err)
}

Expand Down Expand Up @@ -540,22 +541,31 @@ func (s *Server) initGrpcServer(options *istiokeepalive.Options) {
}

// initialize DNS server listener - uses the same certs as gRPC
func (s *Server) initDNSTLSListener(dns string) error {
func (s *Server) initDNSTLSListener(dns string, tlsOptions TLSOptions) error {
if dns == "" {
return nil
}
certDir := dnsCertDir

key := path.Join(certDir, constants.KeyFilename)
cert := path.Join(certDir, constants.CertChainFilename)
key := model.GetOrDefault(tlsOptions.KeyFile, path.Join(certDir, constants.KeyFilename))
cert := model.GetOrDefault(tlsOptions.CertFile, path.Join(certDir, constants.CertChainFilename))

certP, err := tls.LoadX509KeyPair(cert, key)
if err != nil {
return err
}

cp := x509.NewCertPool()
rootCertBytes := s.ca.GetCAKeyCertBundle().GetRootCertPem()
var rootCertBytes []byte
if tlsOptions.CaCertFile != "" {
rootCertBytes, err = ioutil.ReadFile(tlsOptions.CaCertFile)
if err != nil {
return err
}
} else {
rootCertBytes = s.ca.GetCAKeyCertBundle().GetRootCertPem()
}

cp.AppendCertsFromPEM(rootCertBytes)

// TODO: check if client certs can be used with coredns or others.
Expand All @@ -579,19 +589,28 @@ func (s *Server) initDNSTLSListener(dns string) error {
}

// initialize secureGRPCServer - using DNS certs
func (s *Server) initSecureGrpcServer(port string, keepalive *istiokeepalive.Options) error {
func (s *Server) initSecureGrpcServer(port string, keepalive *istiokeepalive.Options, tlsOptions TLSOptions) error {
certDir := dnsCertDir

key := path.Join(certDir, constants.KeyFilename)
cert := path.Join(certDir, constants.CertChainFilename)
key := model.GetOrDefault(tlsOptions.KeyFile, path.Join(certDir, constants.KeyFilename))
cert := model.GetOrDefault(tlsOptions.CertFile, path.Join(certDir, constants.CertChainFilename))

certP, err := tls.LoadX509KeyPair(cert, key)
if err != nil {
return err
}

cp := x509.NewCertPool()
rootCertBytes := s.ca.GetCAKeyCertBundle().GetRootCertPem()
var rootCertBytes []byte
if tlsOptions.CaCertFile != "" {
rootCertBytes, err = ioutil.ReadFile(tlsOptions.CaCertFile)
if err != nil {
return err
}
} else {
rootCertBytes = s.ca.GetCAKeyCertBundle().GetRootCertPem()
}

cp.AppendCertsFromPEM(rootCertBytes)

cfg := &tls.Config{
Expand Down Expand Up @@ -795,7 +814,7 @@ func (s *Server) initSecureGrpcListener(args *PilotArgs) error {
// Feature disabled
return nil
}
if s.ca == nil {
if args.TLSOptions.CaCertFile == "" && s.ca == nil {
// Running locally without configured certs - no TLS mode
return nil
}
Expand All @@ -817,7 +836,7 @@ func (s *Server) initSecureGrpcListener(args *PilotArgs) error {
}

// run secure grpc server for Istiod - using DNS-based certs from K8S
err = s.initSecureGrpcServer(port, args.KeepaliveOptions)
err = s.initSecureGrpcServer(port, args.KeepaliveOptions, args.TLSOptions)
if err != nil {
return err
}
Expand Down
6 changes: 4 additions & 2 deletions pilot/pkg/bootstrap/sidecarinjector.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"path/filepath"
"time"

"istio.io/istio/pilot/pkg/model"

"k8s.io/api/admissionregistration/v1beta1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -66,8 +68,8 @@ func (s *Server) initSidecarInjector(args *PilotArgs) error {
ValuesFile: filepath.Join(injectPath, "values"),
MeshFile: args.Mesh.ConfigFile,
Env: s.environment,
CertFile: filepath.Join(dnsCertDir, "cert-chain.pem"),
KeyFile: filepath.Join(dnsCertDir, "key.pem"),
CertFile: model.GetOrDefault(args.TLSOptions.CertFile, filepath.Join(dnsCertDir, "cert-chain.pem")),
KeyFile: model.GetOrDefault(args.TLSOptions.KeyFile, filepath.Join(dnsCertDir, "key.pem")),
// Disable monitoring. The injection metrics will be picked up by Pilots metrics exporter already
MonitoringPort: -1,
Mux: s.httpsMux,
Expand Down
6 changes: 4 additions & 2 deletions pilot/pkg/bootstrap/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"path/filepath"
"strings"

"istio.io/istio/pilot/pkg/model"

"k8s.io/client-go/dynamic"

"istio.io/pkg/env"
Expand Down Expand Up @@ -55,8 +57,8 @@ func (s *Server) initConfigValidation(args *PilotArgs) error {
MixerValidator: validate.NewDefaultValidator(false),
Schemas: collections.Istio,
DomainSuffix: args.Config.ControllerOptions.DomainSuffix,
CertFile: filepath.Join(dnsCertDir, "cert-chain.pem"),
KeyFile: filepath.Join(dnsCertDir, "key.pem"),
CertFile: model.GetOrDefault(args.TLSOptions.CertFile, filepath.Join(dnsCertDir, "cert-chain.pem")),
KeyFile: model.GetOrDefault(args.TLSOptions.KeyFile, filepath.Join(dnsCertDir, "key.pem")),
Mux: s.httpsMux,
}
whServer, err := server.New(params)
Expand Down
23 changes: 14 additions & 9 deletions pilot/pkg/bootstrap/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"net/url"
"time"

"istio.io/istio/pilot/pkg/model"

"istio.io/pkg/filewatcher"
"istio.io/pkg/log"

Expand Down Expand Up @@ -57,14 +59,17 @@ func (s *Server) initHTTPSWebhookServer(args *PilotArgs) error {
},
}

certFile := model.GetOrDefault(args.TLSOptions.CertFile, dnsCertFile)
keyFile := model.GetOrDefault(args.TLSOptions.KeyFile, dnsKeyFile)

// load the cert/key and setup a persistent watch for updates.
cert, err := server.ReloadCertkey(dnsCertFile, dnsKeyFile)
cert, err := server.ReloadCertkey(certFile, keyFile)
if err != nil {
return err
}
s.webhookCert = cert
keyCertWatcher := filewatcher.NewWatcher()
for _, file := range []string{dnsCertFile, dnsKeyFile} {
for _, file := range []string{certFile, keyFile} {
if err := keyCertWatcher.Add(file); err != nil {
return fmt.Errorf("could not watch %v: %v", file, err)
}
Expand All @@ -77,26 +82,26 @@ func (s *Server) initHTTPSWebhookServer(args *PilotArgs) error {
case <-keyCertTimerC:
keyCertTimerC = nil

cert, err := server.ReloadCertkey(dnsCertFile, dnsKeyFile)
cert, err := server.ReloadCertkey(certFile, keyFile)
if err != nil {
return // error logged and metric reported by server.ReloadCertKey
}

s.webhookCertMu.Lock()
s.webhookCert = cert
s.webhookCertMu.Unlock()
case <-keyCertWatcher.Events(dnsCertFile):
case <-keyCertWatcher.Events(certFile):
if keyCertTimerC == nil {
keyCertTimerC = time.After(watchDebounceDelay)
}
case <-keyCertWatcher.Events(dnsKeyFile):
case <-keyCertWatcher.Events(keyFile):
if keyCertTimerC == nil {
keyCertTimerC = time.After(watchDebounceDelay)
}
case <-keyCertWatcher.Errors(dnsCertFile):
log.Errorf("error watching %v: %v", dnsCertFile, err)
case <-keyCertWatcher.Errors(dnsKeyFile):
log.Errorf("error watching %v: %v", dnsKeyFile, err)
case <-keyCertWatcher.Errors(certFile):
log.Errorf("error watching %v: %v", certFile, err)
case <-keyCertWatcher.Errors(keyFile):
log.Errorf("error watching %v: %v", keyFile, err)
case <-stop:
return
}
Expand Down

0 comments on commit 1520f76

Please sign in to comment.