diff --git a/cmd/nginx/flags.go b/cmd/nginx/flags.go index 0615d94ef..a8f26dafd 100644 --- a/cmd/nginx/flags.go +++ b/cmd/nginx/flags.go @@ -131,8 +131,7 @@ Requires the update-status parameter.`) enableSSLChainCompletion = flags.Bool("enable-ssl-chain-completion", false, `Autocomplete SSL certificate chains with missing intermediate CA certificates. -A valid certificate chain is required to enable OCSP stapling. Certificates -uploaded to Kubernetes must have the "Authority Information Access" X.509 v3 +Certificates uploaded to Kubernetes must have the "Authority Information Access" X.509 v3 extension for this to succeed.`) syncRateLimit = flags.Float32("sync-rate-limit", 0.3, @@ -142,9 +141,8 @@ extension for this to succeed.`) `Customized address to set as the load-balancer status of Ingress objects this controller satisfies. Requires the update-status parameter.`) - dynamicCertificatesEnabled = flags.Bool("enable-dynamic-certificates", true, - `Dynamically update SSL certificates instead of reloading NGINX. -Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not enabled`) + enableDynamicCertificates = flags.Bool("enable-dynamic-certificates", true, + `Dynamically update SSL certificates instead of reloading NGINX. Feature backed by OpenResty Lua libraries.`) enableMetrics = flags.Bool("enable-metrics", true, `Enables the collection of NGINX metrics`) @@ -223,10 +221,6 @@ Takes the form ":port". If not provided, no admission controller is starte klog.Warningf("SSL certificate chain completion is disabled (--enable-ssl-chain-completion=false)") } - if *enableSSLChainCompletion && *dynamicCertificatesEnabled { - return false, nil, fmt.Errorf(`SSL certificate chain completion cannot be enabled when dynamic certificates functionality is enabled. Please check the flags --enable-ssl-chain-completion`) - } - if *publishSvc != "" && *publishStatusAddress != "" { return false, nil, fmt.Errorf("Flags --publish-service and --publish-status-address are mutually exclusive") } @@ -237,29 +231,30 @@ Takes the form ":port". If not provided, no admission controller is starte nginx.HealthCheckTimeout = time.Duration(*defHealthCheckTimeout) * time.Second } + ngx_config.EnableSSLChainCompletion = *enableSSLChainCompletion + ngx_config.EnableDynamicCertificates = *enableDynamicCertificates + config := &controller.Configuration{ - APIServerHost: *apiserverHost, - KubeConfigFile: *kubeConfigFile, - UpdateStatus: *updateStatus, - ElectionID: *electionID, - EnableProfiling: *profiling, - EnableMetrics: *enableMetrics, - MetricsPerHost: *metricsPerHost, - EnableSSLPassthrough: *enableSSLPassthrough, - EnableSSLChainCompletion: *enableSSLChainCompletion, - ResyncPeriod: *resyncPeriod, - DefaultService: *defaultSvc, - Namespace: *watchNamespace, - ConfigMapName: *configMap, - TCPConfigMapName: *tcpConfigMapName, - UDPConfigMapName: *udpConfigMapName, - DefaultSSLCertificate: *defSSLCertificate, - PublishService: *publishSvc, - PublishStatusAddress: *publishStatusAddress, - UpdateStatusOnShutdown: *updateStatusOnShutdown, - UseNodeInternalIP: *useNodeInternalIP, - SyncRateLimit: *syncRateLimit, - DynamicCertificatesEnabled: *dynamicCertificatesEnabled, + APIServerHost: *apiserverHost, + KubeConfigFile: *kubeConfigFile, + UpdateStatus: *updateStatus, + ElectionID: *electionID, + EnableProfiling: *profiling, + EnableMetrics: *enableMetrics, + MetricsPerHost: *metricsPerHost, + EnableSSLPassthrough: *enableSSLPassthrough, + ResyncPeriod: *resyncPeriod, + DefaultService: *defaultSvc, + Namespace: *watchNamespace, + ConfigMapName: *configMap, + TCPConfigMapName: *tcpConfigMapName, + UDPConfigMapName: *udpConfigMapName, + DefaultSSLCertificate: *defSSLCertificate, + PublishService: *publishSvc, + PublishStatusAddress: *publishStatusAddress, + UpdateStatusOnShutdown: *updateStatusOnShutdown, + UseNodeInternalIP: *useNodeInternalIP, + SyncRateLimit: *syncRateLimit, ListenPorts: &ngx_config.ListenPorts{ Default: *defServerPort, Health: *healthzPort, diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index f4f7897a8..abb6216bf 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -30,6 +30,13 @@ import ( "k8s.io/ingress-nginx/internal/runtime" ) +var ( + // EnableSSLChainCompletion Autocomplete SSL certificate chains with missing intermediate CA certificates. + EnableSSLChainCompletion = false + // EnableDynamicCertificates Dynamically update SSL certificates instead of reloading NGINX + EnableDynamicCertificates = true +) + const ( // http://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size // Sets the maximum allowed size of the client request body @@ -755,25 +762,25 @@ func (cfg Configuration) BuildLogFormatUpstream() string { // TemplateConfig contains the nginx configuration to render the file nginx.conf type TemplateConfig struct { - ProxySetHeaders map[string]string - AddHeaders map[string]string - BacklogSize int - Backends []*ingress.Backend - PassthroughBackends []*ingress.SSLPassthroughBackend - Servers []*ingress.Server - TCPBackends []ingress.L4Service - UDPBackends []ingress.L4Service - HealthzURI string - Cfg Configuration - IsIPV6Enabled bool - IsSSLPassthroughEnabled bool - NginxStatusIpv4Whitelist []string - NginxStatusIpv6Whitelist []string - RedirectServers interface{} - ListenPorts *ListenPorts - PublishService *apiv1.Service - DynamicCertificatesEnabled bool - EnableMetrics bool + ProxySetHeaders map[string]string + AddHeaders map[string]string + BacklogSize int + Backends []*ingress.Backend + PassthroughBackends []*ingress.SSLPassthroughBackend + Servers []*ingress.Server + TCPBackends []ingress.L4Service + UDPBackends []ingress.L4Service + HealthzURI string + Cfg Configuration + IsIPV6Enabled bool + IsSSLPassthroughEnabled bool + NginxStatusIpv4Whitelist []string + NginxStatusIpv6Whitelist []string + RedirectServers interface{} + ListenPorts *ListenPorts + PublishService *apiv1.Service + EnableDynamicCertificates bool + EnableMetrics bool PID string StatusSocket string diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 8b14230d2..d9f4b7d6a 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -84,14 +84,10 @@ type Configuration struct { EnableMetrics bool MetricsPerHost bool - EnableSSLChainCompletion bool - FakeCertificate *ingress.SSLCert SyncRateLimit float32 - DynamicCertificatesEnabled bool - DisableCatchAll bool ValidationWebhook string @@ -171,7 +167,7 @@ func (n *NGINXController) syncIngress(interface{}) error { } err := wait.ExponentialBackoff(retry, func() (bool, error) { - err := configureDynamically(pcfg, n.cfg.DynamicCertificatesEnabled) + err := configureDynamically(pcfg) if err == nil { klog.V(2).Infof("Dynamic reconfiguration succeeded.") return true, nil @@ -890,7 +886,7 @@ func (n *NGINXController) serviceEndpoints(svcKey, backendPort string) ([]ingres return upstreams, nil } -// overridePemFileNameAndPemSHA should only be called when DynamicCertificatesEnabled +// overridePemFileNameAndPemSHA should only be called when EnableDynamicCertificates // ideally this function should not exist, the only reason why we use it is that // we rely on PemFileName in nginx.tmpl to configure SSL directives // and PemSHA to force reload @@ -940,7 +936,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress, certificate, err := n.store.GetLocalSSLCert(n.cfg.DefaultSSLCertificate) if err == nil { defaultCertificate = certificate - if n.cfg.DynamicCertificatesEnabled { + if ngx_config.EnableDynamicCertificates { n.overridePemFileNameAndPemSHA(defaultCertificate) } } else { @@ -1123,7 +1119,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress, } } - if n.cfg.DynamicCertificatesEnabled { + if ngx_config.EnableDynamicCertificates { n.overridePemFileNameAndPemSHA(cert) } diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index 232b1f1e5..e42e97704 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -1116,7 +1116,7 @@ func newNGINXController(t *testing.T) *NGINXController { t.Fatalf("error: %v", err) } - storer := store.New(true, + storer := store.New( ns, fmt.Sprintf("%v/config", ns), fmt.Sprintf("%v/tcp", ns), @@ -1126,7 +1126,6 @@ func newNGINXController(t *testing.T) *NGINXController { clientSet, fs, channels.NewRingChannel(10), - false, pod, false) diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 8bb77de0c..149c9264e 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -129,7 +129,6 @@ func NewNGINXController(config *Configuration, mc metric.Collector, fs file.File n.podInfo = pod n.store = store.New( - config.EnableSSLChainCompletion, config.Namespace, config.ConfigMapName, config.TCPConfigMapName, @@ -139,7 +138,6 @@ func NewNGINXController(config *Configuration, mc metric.Collector, fs file.File config.Client, fs, n.updateCh, - config.DynamicCertificatesEnabled, pod, config.DisableCatchAll) @@ -598,24 +596,24 @@ func (n NGINXController) generateTemplate(cfg ngx_config.Configuration, ingressC cfg.SSLDHParam = sslDHParam tc := ngx_config.TemplateConfig{ - ProxySetHeaders: setHeaders, - AddHeaders: addHeaders, - BacklogSize: sysctlSomaxconn(), - Backends: ingressCfg.Backends, - PassthroughBackends: ingressCfg.PassthroughBackends, - Servers: ingressCfg.Servers, - TCPBackends: ingressCfg.TCPEndpoints, - UDPBackends: ingressCfg.UDPEndpoints, - Cfg: cfg, - IsIPV6Enabled: n.isIPV6Enabled && !cfg.DisableIpv6, - NginxStatusIpv4Whitelist: cfg.NginxStatusIpv4Whitelist, - NginxStatusIpv6Whitelist: cfg.NginxStatusIpv6Whitelist, - RedirectServers: buildRedirects(ingressCfg.Servers), - IsSSLPassthroughEnabled: n.cfg.EnableSSLPassthrough, - ListenPorts: n.cfg.ListenPorts, - PublishService: n.GetPublishService(), - DynamicCertificatesEnabled: n.cfg.DynamicCertificatesEnabled, - EnableMetrics: n.cfg.EnableMetrics, + ProxySetHeaders: setHeaders, + AddHeaders: addHeaders, + BacklogSize: sysctlSomaxconn(), + Backends: ingressCfg.Backends, + PassthroughBackends: ingressCfg.PassthroughBackends, + Servers: ingressCfg.Servers, + TCPBackends: ingressCfg.TCPEndpoints, + UDPBackends: ingressCfg.UDPEndpoints, + Cfg: cfg, + IsIPV6Enabled: n.isIPV6Enabled && !cfg.DisableIpv6, + NginxStatusIpv4Whitelist: cfg.NginxStatusIpv4Whitelist, + NginxStatusIpv6Whitelist: cfg.NginxStatusIpv6Whitelist, + RedirectServers: buildRedirects(ingressCfg.Servers), + IsSSLPassthroughEnabled: n.cfg.EnableSSLPassthrough, + ListenPorts: n.cfg.ListenPorts, + PublishService: n.GetPublishService(), + EnableDynamicCertificates: ngx_config.EnableDynamicCertificates, + EnableMetrics: n.cfg.EnableMetrics, HealthzURI: nginx.HealthPath, PID: nginx.PID, @@ -851,7 +849,7 @@ func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configurati copyOfRunningConfig.ControllerPodsCount = 0 copyOfPcfg.ControllerPodsCount = 0 - if n.cfg.DynamicCertificatesEnabled { + if ngx_config.EnableDynamicCertificates { clearCertificates(©OfRunningConfig) clearCertificates(©OfPcfg) } @@ -861,7 +859,7 @@ func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configurati // configureDynamically encodes new Backends in JSON format and POSTs the // payload to an internal HTTP endpoint handled by Lua. -func configureDynamically(pcfg *ingress.Configuration, isDynamicCertificatesEnabled bool) error { +func configureDynamically(pcfg *ingress.Configuration) error { backends := make([]*ingress.Backend, len(pcfg.Backends)) for i, backend := range pcfg.Backends { @@ -949,7 +947,7 @@ func configureDynamically(pcfg *ingress.Configuration, isDynamicCertificatesEnab return fmt.Errorf("unexpected error code: %d", statusCode) } - if isDynamicCertificatesEnabled { + if ngx_config.EnableDynamicCertificates { err = configureCertificates(pcfg) if err != nil { return err diff --git a/internal/ingress/controller/nginx_test.go b/internal/ingress/controller/nginx_test.go index 1de35f97d..1921820e8 100644 --- a/internal/ingress/controller/nginx_test.go +++ b/internal/ingress/controller/nginx_test.go @@ -32,10 +32,14 @@ import ( apiv1 "k8s.io/api/core/v1" "k8s.io/ingress-nginx/internal/ingress" + ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" "k8s.io/ingress-nginx/internal/nginx" ) func TestIsDynamicConfigurationEnough(t *testing.T) { + ngx_config.EnableDynamicCertificates = false + defer func() { ngx_config.EnableDynamicCertificates = true }() + backends := []*ingress.Backend{{ Name: "fakenamespace-myapp-80", Endpoints: []ingress.Endpoint{ @@ -73,9 +77,7 @@ func TestIsDynamicConfigurationEnough(t *testing.T) { Backends: backends, Servers: servers, }, - cfg: &Configuration{ - DynamicCertificatesEnabled: false, - }, + cfg: &Configuration{}, } newConfig := commonConfig @@ -95,12 +97,13 @@ func TestIsDynamicConfigurationEnough(t *testing.T) { Backends: []*ingress.Backend{{Name: "a-backend-8080"}}, Servers: servers, } + + ngx_config.EnableDynamicCertificates = true + if !n.IsDynamicConfigurationEnough(newConfig) { t.Errorf("Expected to be dynamically configurable when only backends change") } - n.cfg.DynamicCertificatesEnabled = true - newServers := []*ingress.Server{{ Hostname: "myapp1.fake", Locations: []*ingress.Location{ @@ -243,7 +246,10 @@ func TestConfigureDynamically(t *testing.T) { ControllerPodsCount: 2, } - err = configureDynamically(commonConfig, false) + ngx_config.EnableDynamicCertificates = false + defer func() { ngx_config.EnableDynamicCertificates = true }() + + err = configureDynamically(commonConfig) if err != nil { t.Errorf("unexpected error posting dynamic configuration: %v", err) } diff --git a/internal/ingress/controller/store/backend_ssl.go b/internal/ingress/controller/store/backend_ssl.go index adb94be68..eeed9bb34 100644 --- a/internal/ingress/controller/store/backend_ssl.go +++ b/internal/ingress/controller/store/backend_ssl.go @@ -20,16 +20,14 @@ import ( "fmt" "strings" - "github.com/imdario/mergo" "k8s.io/klog" apiv1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/k8s" + ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" "k8s.io/ingress-nginx/internal/net/ssl" ) @@ -103,7 +101,7 @@ func (s *k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error return nil, fmt.Errorf("unexpected error creating SSL Cert: %v", err) } - if !s.isDynamicCertificatesEnabled || len(ca) > 0 { + if !ngx_config.EnableDynamicCertificates || len(ca) > 0 { err = ssl.StoreSSLCertOnDisk(s.filesystem, nsSecName, sslCert) if err != nil { return nil, fmt.Errorf("error while storing certificate and key: %v", err) @@ -152,57 +150,6 @@ func (s *k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error return sslCert, nil } -func (s *k8sStore) checkSSLChainIssues() { - for _, item := range s.ListLocalSSLCerts() { - secrKey := k8s.MetaNamespaceKey(item) - secret, err := s.GetLocalSSLCert(secrKey) - if err != nil { - continue - } - - if secret.FullChainPemFileName != "" { - // chain already checked - continue - } - - data, err := ssl.FullChainCert(secret.PemFileName, s.filesystem) - if err != nil { - klog.Errorf("Error generating CA certificate chain for Secret %q: %v", secrKey, err) - continue - } - - fullChainPemFileName := fmt.Sprintf("%v/%v-%v-full-chain.pem", file.DefaultSSLDirectory, secret.Namespace, secret.Name) - - file, err := s.filesystem.Create(fullChainPemFileName) - if err != nil { - klog.Errorf("Error creating SSL certificate file for Secret %q: %v", secrKey, err) - continue - } - - _, err = file.Write(data) - if err != nil { - klog.Errorf("Error creating SSL certificate for Secret %q: %v", secrKey, err) - continue - } - - dst := &ingress.SSLCert{} - - err = mergo.MergeWithOverwrite(dst, secret) - if err != nil { - klog.Errorf("Error creating SSL certificate for Secret %q: %v", secrKey, err) - continue - } - - dst.FullChainPemFileName = fullChainPemFileName - - klog.Infof("Updating local copy of SSL certificate %q with missing intermediate CA certs", secrKey) - s.sslStore.Update(secrKey, dst) - // this update must trigger an update - // (like an update event from a change in Ingress) - s.sendDummyEvent() - } -} - // sendDummyEvent sends a dummy event to trigger an update // This is used in when a secret change func (s *k8sStore) sendDummyEvent() { diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index 299c74e35..a403bfdfa 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -35,7 +35,6 @@ import ( "k8s.io/apimachinery/pkg/labels" k8sruntime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/informers" clientset "k8s.io/client-go/kubernetes" @@ -187,8 +186,6 @@ func (i *Informer) Run(stopCh chan struct{}) { // k8sStore internal Storer implementation using informers and thread safe stores type k8sStore struct { - isOCSPCheckEnabled bool - // backendConfig contains the running configuration from the configmap // this is required because this rarely changes but is a very expensive // operation to execute in each OnUpdate invocation @@ -224,36 +221,31 @@ type k8sStore struct { defaultSSLCertificate string - isDynamicCertificatesEnabled bool - pod *k8s.PodInfo } // New creates a new object store to be used in the ingress controller -func New(checkOCSP bool, +func New( namespace, configmap, tcp, udp, defaultSSLCertificate string, resyncPeriod time.Duration, client clientset.Interface, fs file.Filesystem, updateCh *channels.RingChannel, - isDynamicCertificatesEnabled bool, pod *k8s.PodInfo, disableCatchAll bool) Storer { store := &k8sStore{ - isOCSPCheckEnabled: checkOCSP, - informers: &Informer{}, - listers: &Lister{}, - sslStore: NewSSLCertTracker(), - filesystem: fs, - updateCh: updateCh, - backendConfig: ngx_config.NewDefault(), - syncSecretMu: &sync.Mutex{}, - backendConfigMu: &sync.RWMutex{}, - secretIngressMap: NewObjectRefMap(), - defaultSSLCertificate: defaultSSLCertificate, - isDynamicCertificatesEnabled: isDynamicCertificatesEnabled, - pod: pod, + informers: &Informer{}, + listers: &Lister{}, + sslStore: NewSSLCertTracker(), + filesystem: fs, + updateCh: updateCh, + backendConfig: ngx_config.NewDefault(), + syncSecretMu: &sync.Mutex{}, + backendConfigMu: &sync.RWMutex{}, + secretIngressMap: NewObjectRefMap(), + defaultSSLCertificate: defaultSSLCertificate, + pod: pod, } eventBroadcaster := record.NewBroadcaster() @@ -878,10 +870,6 @@ func (s *k8sStore) setConfig(cmap *corev1.ConfigMap) { func (s *k8sStore) Run(stopCh chan struct{}) { // start informers s.informers.Run(stopCh) - - if s.isOCSPCheckEnabled { - go wait.Until(s.checkSSLChainIssues, 60*time.Second, stopCh) - } } // GetRunningControllerPodsCount returns the number of Running ingress-nginx controller Pods diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index d0adb660c..99bc23bbd 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -41,6 +41,7 @@ import ( "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -87,7 +88,7 @@ func TestStore(t *testing.T) { }(updateCh) fs := newFS(t) - storer := New(true, + storer := New( ns, fmt.Sprintf("%v/config", ns), fmt.Sprintf("%v/tcp", ns), @@ -97,7 +98,6 @@ func TestStore(t *testing.T) { clientSet, fs, updateCh, - false, pod, false) @@ -168,7 +168,7 @@ func TestStore(t *testing.T) { }(updateCh) fs := newFS(t) - storer := New(true, + storer := New( ns, fmt.Sprintf("%v/config", ns), fmt.Sprintf("%v/tcp", ns), @@ -178,7 +178,6 @@ func TestStore(t *testing.T) { clientSet, fs, updateCh, - false, pod, false) @@ -319,7 +318,7 @@ func TestStore(t *testing.T) { }(updateCh) fs := newFS(t) - storer := New(true, + storer := New( ns, fmt.Sprintf("%v/config", ns), fmt.Sprintf("%v/tcp", ns), @@ -329,7 +328,6 @@ func TestStore(t *testing.T) { clientSet, fs, updateCh, - false, pod, false) @@ -426,7 +424,7 @@ func TestStore(t *testing.T) { }(updateCh) fs := newFS(t) - storer := New(true, + storer := New( ns, fmt.Sprintf("%v/config", ns), fmt.Sprintf("%v/tcp", ns), @@ -436,7 +434,6 @@ func TestStore(t *testing.T) { clientSet, fs, updateCh, - false, pod, false) @@ -516,7 +513,7 @@ func TestStore(t *testing.T) { }(updateCh) fs := newFS(t) - storer := New(true, + storer := New( ns, fmt.Sprintf("%v/config", ns), fmt.Sprintf("%v/tcp", ns), @@ -526,7 +523,6 @@ func TestStore(t *testing.T) { clientSet, fs, updateCh, - false, pod, false) @@ -628,7 +624,7 @@ func TestStore(t *testing.T) { }(updateCh) fs := newFS(t) - storer := New(true, + storer := New( ns, fmt.Sprintf("%v/config", ns), fmt.Sprintf("%v/tcp", ns), @@ -638,7 +634,6 @@ func TestStore(t *testing.T) { clientSet, fs, updateCh, - false, pod, false) @@ -708,6 +703,9 @@ func TestStore(t *testing.T) { } t.Run("should exists a secret in the local store and filesystem", func(t *testing.T) { + ngx_config.EnableDynamicCertificates = false + defer func() { ngx_config.EnableDynamicCertificates = true }() + err := framework.WaitForSecretInNamespace(clientSet, ns, name) if err != nil { t.Errorf("error waiting for secret: %v", err) diff --git a/internal/ingress/sslcert.go b/internal/ingress/sslcert.go index 03f139393..2b8d481de 100644 --- a/internal/ingress/sslcert.go +++ b/internal/ingress/sslcert.go @@ -32,9 +32,6 @@ type SSLCert struct { CAFileName string `json:"caFileName"` // PemFileName contains the path to the file with the certificate and key concatenated PemFileName string `json:"pemFileName"` - // FullChainPemFileName contains the path to the file with the certificate and key concatenated - // This certificate contains the full chain (ca + intermediates + cert) - FullChainPemFileName string `json:"fullChainPemFileName"` // PemSHA contains the sha1 of the pem file. // This is used to detect changes in the secret that contains the certificates PemSHA string `json:"pemSha"` diff --git a/internal/ingress/types_equals.go b/internal/ingress/types_equals.go index d3ec2b2ab..63dd2b827 100644 --- a/internal/ingress/types_equals.go +++ b/internal/ingress/types_equals.go @@ -523,9 +523,6 @@ func (s1 *SSLCert) Equal(s2 *SSLCert) bool { if !s1.ExpireTime.Equal(s2.ExpireTime) { return false } - if s1.FullChainPemFileName != s2.FullChainPemFileName { - return false - } if s1.PemCertKey != s2.PemCertKey { return false } diff --git a/internal/net/ssl/ssl.go b/internal/net/ssl/ssl.go index e5b2318ed..aeb8fb953 100644 --- a/internal/net/ssl/ssl.go +++ b/internal/net/ssl/ssl.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" + ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" "k8s.io/ingress-nginx/internal/watch" "k8s.io/klog" ) @@ -74,8 +75,18 @@ func verifyPemCertAgainstRootCA(pemCert *x509.Certificate, ca []byte) error { // CreateSSLCert validates cert and key, extracts common names and returns corresponding SSLCert object func CreateSSLCert(cert, key []byte) (*ingress.SSLCert, error) { var pemCertBuffer bytes.Buffer - pemCertBuffer.Write(cert) + + if ngx_config.EnableSSLChainCompletion { + data, err := fullChainCert(cert) + if err != nil { + klog.Errorf("Error generating certificate chain for Secret: %v", err) + } else { + pemCertBuffer.Reset() + pemCertBuffer.Write(data) + } + } + pemCertBuffer.Write([]byte("\n")) pemCertBuffer.Write(key) @@ -376,7 +387,6 @@ func GetFakeSSLCert(fs file.Filesystem) *ingress.SSLCert { } func getFakeHostSSLCert(host string) ([]byte, []byte) { - var priv interface{} var err error @@ -423,16 +433,11 @@ func getFakeHostSSLCert(host string) ([]byte, []byte) { return cert, key } -// FullChainCert checks if a certificate file contains issues in the intermediate CA chain +// fullChainCert checks if a certificate file contains issues in the intermediate CA chain // Returns a new certificate with the intermediate certificates. // If the certificate does not contains issues with the chain it return an empty byte array -func FullChainCert(in string, fs file.Filesystem) ([]byte, error) { - data, err := fs.ReadFile(in) - if err != nil { - return nil, err - } - - cert, err := certUtil.DecodeCertificate(data) +func fullChainCert(in []byte) ([]byte, error) { + cert, err := certUtil.DecodeCertificate(in) if err != nil { return nil, err } @@ -452,11 +457,6 @@ func FullChainCert(in string, fs file.Filesystem) ([]byte, error) { return nil, err } - certs, err = certUtil.AddRootCA(certs) - if err != nil { - return nil, err - } - return certUtil.EncodeCertificates(certs), nil } diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 575345838..dd10d2608 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -96,7 +96,7 @@ http { end {{ end }} - {{ if $all.DynamicCertificatesEnabled }} + {{ if $all.EnableDynamicCertificates }} ok, res = pcall(require, "certificate") if not ok then error("require failed: " .. tostring(res)) @@ -492,13 +492,8 @@ http { # PEM sha: {{ $redirect.SSLCert.PemSHA }} ssl_certificate {{ $redirect.SSLCert.PemFileName }}; ssl_certificate_key {{ $redirect.SSLCert.PemFileName }}; - {{ if not (empty $redirect.SSLCert.FullChainPemFileName)}} - ssl_trusted_certificate {{ $redirect.SSLCert.FullChainPemFileName }}; - ssl_stapling on; - ssl_stapling_verify on; - {{ end }} - {{ if $all.DynamicCertificatesEnabled}} + {{ if $all.EnableDynamicCertificates}} ssl_certificate_by_lua_block { certificate.call() } @@ -841,13 +836,8 @@ stream { # PEM sha: {{ $server.SSLCert.PemSHA }} ssl_certificate {{ $server.SSLCert.PemFileName }}; ssl_certificate_key {{ $server.SSLCert.PemFileName }}; - {{ if not (empty $server.SSLCert.FullChainPemFileName)}} - ssl_trusted_certificate {{ $server.SSLCert.FullChainPemFileName }}; - ssl_stapling on; - ssl_stapling_verify on; - {{ end }} - {{ if $all.DynamicCertificatesEnabled}} + {{ if $all.EnableDynamicCertificates}} ssl_certificate_by_lua_block { certificate.call() }