diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 4beda813a..153fe30cb 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -219,7 +219,7 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr configmap, err := n.store.GetConfigMap(configmapName) if err != nil { - glog.Errorf("Error reading ConfigMap %q: %v", configmapName, err) + glog.Errorf("Error getting ConfigMap %q: %v", configmapName, err) return []ingress.L4Service{} } @@ -236,7 +236,7 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr } reserverdPorts := sets.NewInt(rp...) - // svcRef format: <(str)namespace>/<(str)service>:<(intstr)port>[:<(bool)decode>:<(bool)encode>] + // svcRef format: <(str)namespace>/<(str)service>:<(intstr)port>[:<("PROXY")decode>:<("PROXY")encode>] for port, svcRef := range configmap.Data { externalPort, err := strconv.Atoi(port) if err != nil { @@ -278,7 +278,7 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr svc, err := n.store.GetService(nsName) if err != nil { - glog.Warningf("Error getting Service %q from local store: %v", nsName, err) + glog.Warningf("Error getting Service %q: %v", nsName, err) continue } @@ -339,7 +339,7 @@ func (n *NGINXController) getDefaultUpstream() *ingress.Backend { svcKey := n.cfg.DefaultService svc, err := n.store.GetService(svcKey) if err != nil { - glog.Warningf("Unexpected error getting default backend %q from local store: %v", n.cfg.DefaultService, err) + glog.Warningf("Error getting default backend %q: %v", svcKey, err) upstream.Endpoints = append(upstream.Endpoints, n.DefaultEndpoint()) return upstream } @@ -364,9 +364,11 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] servers := n.createServers(ingresses, upstreams, du) for _, ing := range ingresses { - anns, err := n.store.GetIngressAnnotations(ing) + ingKey := k8s.MetaNamespaceKey(ing) + + anns, err := n.store.GetIngressAnnotations(ingKey) if err != nil { - glog.Errorf("Unexpected error reading annotations for Ingress %q from local store: %v", ing.Name, err) + glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err) } for _, rule := range ing.Spec.Rules { @@ -381,7 +383,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] if rule.HTTP == nil && host != defServerName { - glog.V(3).Infof("Ingress \"%v/%v\" does not contain any HTTP rule, using default backend.", ing.Namespace, ing.Name) + glog.V(3).Infof("Ingress %q does not contain any HTTP rule, using default backend", ingKey) continue } @@ -392,10 +394,12 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] if server.CertificateAuth.CAFileName == "" { server.CertificateAuth = anns.CertificateAuth if server.CertificateAuth.Secret != "" && server.CertificateAuth.CAFileName == "" { - glog.V(3).Infof("Secret %q does not contain 'ca.crt' key, mutual authentication disabled for Ingress \"%v/%v\"", server.CertificateAuth.Secret, ing.Namespace, ing.Name) + glog.V(3).Infof("Secret %q has no 'ca.crt' key, mutual authentication disabled for Ingress %q", + server.CertificateAuth.Secret, ingKey) } } else { - glog.V(3).Infof("Server %v is already configured for mutual authentication (Ingress \"%v/%v\")", server.Hostname, ing.Namespace, ing.Name) + glog.V(3).Infof("Server %q is already configured for mutual authentication (Ingress %q)", + server.Hostname, ingKey) } for _, path := range rule.HTTP.Paths { @@ -417,11 +421,14 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] addLoc = false if !loc.IsDefBackend { - glog.V(3).Infof("Location %q already configured for server %q with upstream %q (Ingress \"%v/%v\")", loc.Path, server.Hostname, loc.Backend, ing.Namespace, ing.Name) + glog.V(3).Infof("Location %q already configured for server %q with upstream %q (Ingress %q)", + loc.Path, server.Hostname, loc.Backend, ingKey) break } - glog.V(3).Infof("Replacing location %q for server %q with upstream %q to use upstream %q (Ingress \"%v/%v\")", loc.Path, server.Hostname, loc.Backend, ups.Name, ing.Namespace, ing.Name) + glog.V(3).Infof("Replacing location %q for server %q with upstream %q to use upstream %q (Ingress %q)", + loc.Path, server.Hostname, loc.Backend, ups.Name, ingKey) + loc.Backend = ups.Name loc.IsDefBackend = false loc.Port = ups.Port @@ -457,7 +464,9 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] // new location if addLoc { - glog.V(3).Infof("Adding location %q for server %q with upstream %q (Ingress \"%v/%v\")", nginxPath, server.Hostname, ups.Name, ing.Namespace, ing.Name) + glog.V(3).Infof("Adding location %q for server %q with upstream %q (Ingress %q)", + nginxPath, server.Hostname, ups.Name, ingKey) + loc := &ingress.Location{ Path: nginxPath, Backend: ups.Name, @@ -520,7 +529,8 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] for _, location := range server.Locations { if upstream.Name == location.Backend { if len(upstream.Endpoints) == 0 { - glog.V(3).Infof("Upstream %q does not have any active endpoints.", upstream.Name) + glog.V(3).Infof("Upstream %q has no active Endpoint", upstream.Name) + location.Backend = "" // for nginx.tmpl checking // check if the location contains endpoints and a custom default backend @@ -530,6 +540,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] if len(endps) > 0 { glog.V(3).Infof("Using custom default backend for location %q in server %q (Service \"%v/%v\")", location.Path, server.Hostname, location.DefaultBackend.Namespace, location.DefaultBackend.Name) + nb := upstream.DeepCopy() name := fmt.Sprintf("custom-default-backend-%v", upstream.Name) nb.Name = name @@ -592,9 +603,11 @@ func (n *NGINXController) createUpstreams(data []*extensions.Ingress, du *ingres upstreams[defUpstreamName] = du for _, ing := range data { - anns, err := n.store.GetIngressAnnotations(ing) + ingKey := k8s.MetaNamespaceKey(ing) + + anns, err := n.store.GetIngressAnnotations(ingKey) if err != nil { - glog.Errorf("Error reading Ingress annotations: %v", err) + glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err) } var defBackend string @@ -736,7 +749,7 @@ func (n *NGINXController) getServiceClusterEndpoint(svcKey string, backend *exte } } if port == -1 { - return endpoint, fmt.Errorf("service %q does not have a port named %q", svc.Name, backend.ServicePort) + return endpoint, fmt.Errorf("Service %q does not have a port named %q", svc.Name, backend.ServicePort) } endpoint.Port = fmt.Sprintf("%d", port) } else { @@ -754,7 +767,7 @@ func (n *NGINXController) serviceEndpoints(svcKey, backendPort string, var upstreams []ingress.Endpoint if err != nil { - return upstreams, fmt.Errorf("error getting Service %q from local store: %v", svcKey, err) + return upstreams, err } glog.V(3).Infof("Obtaining ports information for Service %q", svcKey) @@ -875,9 +888,11 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, // initialize all other servers for _, ing := range data { - anns, err := n.store.GetIngressAnnotations(ing) + ingKey := k8s.MetaNamespaceKey(ing) + + anns, err := n.store.GetIngressAnnotations(ingKey) if err != nil { - glog.Errorf("Error reading Ingress %q annotations from local store: %v", ing.Name, err) + glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err) } // default upstream name @@ -893,7 +908,8 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, // special "catch all" case, Ingress with a backend but no rule defLoc := servers[defServerName].Locations[0] if defLoc.IsDefBackend && len(ing.Spec.Rules) == 0 { - glog.Infof("Ingress \"%v/%v\" defines a backend but no rule. Using it to configure the catch-all server %q", ing.Namespace, ing.Name, defServerName) + glog.Infof("Ingress %q defines a backend but no rule. Using it to configure the catch-all server %q", + ingKey, defServerName) defLoc.IsDefBackend = false defLoc.Backend = backendUpstream.Name @@ -919,7 +935,8 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, defLoc.LuaRestyWAF = anns.LuaRestyWAF defLoc.InfluxDB = anns.InfluxDB } else { - glog.V(3).Infof("Ingress \"%v/%v\" defines both a backend and rules. Using its backend as default upstream for all its rules.", ing.Namespace, ing.Name) + glog.V(3).Infof("Ingress %q defines both a backend and rules. Using its backend as default upstream for all its rules.", + ingKey) } } } @@ -953,9 +970,11 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, // configure default location, alias, and SSL for _, ing := range data { - anns, err := n.store.GetIngressAnnotations(ing) + ingKey := k8s.MetaNamespaceKey(ing) + + anns, err := n.store.GetIngressAnnotations(ingKey) if err != nil { - glog.Errorf("Error reading Ingress %q annotations from local store: %v", ing.Name, err) + glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err) } for _, rule := range ing.Spec.Rules { @@ -971,8 +990,8 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, aliases["Alias"] = host } } else { - glog.Warningf("Aliases already configured for server %q, skipping (Ingress \"%v/%v\")", - host, ing.Namespace, ing.Name) + glog.Warningf("Aliases already configured for server %q, skipping (Ingress %q)", + host, ingKey) } } @@ -980,8 +999,8 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, if servers[host].ServerSnippet == "" { servers[host].ServerSnippet = anns.ServerSnippet } else { - glog.Warningf("Server snippet already configured for server %q, skipping (Ingress \"%v/%v\")", - host, ing.Namespace, ing.Name) + glog.Warningf("Server snippet already configured for server %q, skipping (Ingress %q)", + host, ingKey) } } @@ -996,7 +1015,7 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, } if len(ing.Spec.TLS) == 0 { - glog.V(3).Infof("Ingress \"%v/%v\" does not contains a TLS section.", ing.Namespace, ing.Name) + glog.V(3).Infof("Ingress %q does not contains a TLS section.", ingKey) continue } @@ -1009,22 +1028,23 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, continue } - key := fmt.Sprintf("%v/%v", ing.Namespace, tlsSecretName) - cert, err := n.store.GetLocalSSLCert(key) + secrKey := fmt.Sprintf("%v/%v", ing.Namespace, tlsSecretName) + cert, err := n.store.GetLocalSSLCert(secrKey) if err != nil { - glog.Warningf("SSL certificate %q does not exist in local store.", key) + glog.Warningf("Error getting SSL certificate %q: %v", secrKey, err) continue } err = cert.Certificate.VerifyHostname(host) if err != nil { - glog.Warningf("Unexpected error validating SSL certificate %q for server %q: %v", key, host, err) + glog.Warningf("Unexpected error validating SSL certificate %q for server %q: %v", secrKey, host, err) glog.Warning("Validating certificate against DNS names. This will be deprecated in a future version.") // check the Common Name field // https://github.com/golang/go/issues/22922 err := verifyHostname(host, cert.Certificate) if err != nil { - glog.Warningf("SSL certificate %q does not contain a Common Name or Subject Alternative Name for server %q: %v", key, host, err) + glog.Warningf("SSL certificate %q does not contain a Common Name or Subject Alternative Name for server %q: %v", + secrKey, host, err) continue } } @@ -1065,10 +1085,11 @@ func extractTLSSecretName(host string, ing *extensions.Ingress, // no TLS host matching host name, try each TLS host for matching SAN or CN for _, tls := range ing.Spec.TLS { - key := fmt.Sprintf("%v/%v", ing.Namespace, tls.SecretName) - cert, err := getLocalSSLCert(key) + secrKey := fmt.Sprintf("%v/%v", ing.Namespace, tls.SecretName) + + cert, err := getLocalSSLCert(secrKey) if err != nil { - glog.Warningf("SSL certificate %q does not exist in local store.", key) + glog.Warningf("Error getting SSL certificate %q: %v", secrKey, err) continue } @@ -1080,7 +1101,7 @@ func extractTLSSecretName(host string, ing *extensions.Ingress, if err != nil { continue } - glog.V(3).Infof("Found SSL certificate matching host %q: %q", host, key) + glog.V(3).Infof("Found SSL certificate matching host %q: %q", host, secrKey) return tls.SecretName } diff --git a/internal/ingress/controller/endpoints.go b/internal/ingress/controller/endpoints.go index 69b058fe2..368ba2b1d 100644 --- a/internal/ingress/controller/endpoints.go +++ b/internal/ingress/controller/endpoints.go @@ -27,11 +27,12 @@ import ( "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations/healthcheck" + "k8s.io/ingress-nginx/internal/k8s" ) // getEndpoints returns a list of Endpoint structs for a given service/target port combination. func getEndpoints(s *corev1.Service, port *corev1.ServicePort, proto corev1.Protocol, hz *healthcheck.Config, - getServiceEndpoints func(*corev1.Service) (*corev1.Endpoints, error)) []ingress.Endpoint { + getServiceEndpoints func(string) (*corev1.Endpoints, error)) []ingress.Endpoint { upsServers := []ingress.Endpoint{} @@ -43,13 +44,15 @@ func getEndpoints(s *corev1.Service, port *corev1.ServicePort, proto corev1.Prot // contains multiple port definitions sharing the same targetport processedUpstreamServers := make(map[string]struct{}) + svcKey := k8s.MetaNamespaceKey(s) + // ExternalName services if s.Spec.Type == corev1.ServiceTypeExternalName { - glog.V(3).Infof("Ingress using Service %q of type ExternalName.", s.Name) + glog.V(3).Infof("Ingress using Service %q of type ExternalName.", svcKey) targetPort := port.TargetPort.IntValue() if targetPort <= 0 { - glog.Errorf("ExternalName Service %q has an invalid port (%v)", s.Name, targetPort) + glog.Errorf("ExternalName Service %q has an invalid port (%v)", svcKey, targetPort) return upsServers } @@ -69,10 +72,10 @@ func getEndpoints(s *corev1.Service, port *corev1.ServicePort, proto corev1.Prot }) } - glog.V(3).Infof("Getting Endpoints for Service \"%v/%v\" and port %v", s.Namespace, s.Name, port.String()) - ep, err := getServiceEndpoints(s) + glog.V(3).Infof("Getting Endpoints for Service %q and port %v", svcKey, port.String()) + ep, err := getServiceEndpoints(svcKey) if err != nil { - glog.Warningf("Error obtaining Endpoints for Service \"%v/%v\": %v", s.Namespace, s.Name, err) + glog.Warningf("Error obtaining Endpoints for Service %q: %v", svcKey, err) return upsServers } @@ -114,6 +117,6 @@ func getEndpoints(s *corev1.Service, port *corev1.ServicePort, proto corev1.Prot } } - glog.V(3).Infof("Endpoints found for Service \"%v/%v\": %v", s.Namespace, s.Name, upsServers) + glog.V(3).Infof("Endpoints found for Service %q: %v", svcKey, upsServers) return upsServers } diff --git a/internal/ingress/controller/endpoints_test.go b/internal/ingress/controller/endpoints_test.go index 08e4d6c9f..7d7fbd959 100644 --- a/internal/ingress/controller/endpoints_test.go +++ b/internal/ingress/controller/endpoints_test.go @@ -33,44 +33,44 @@ func TestGetEndpoints(t *testing.T) { port *corev1.ServicePort proto corev1.Protocol hz *healthcheck.Config - fn func(*corev1.Service) (*corev1.Endpoints, error) + fn func(string) (*corev1.Endpoints, error) result []ingress.Endpoint }{ { - "no service should return 0 endpoints", + "no service should return 0 endpoint", nil, nil, corev1.ProtocolTCP, nil, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { return nil, nil }, []ingress.Endpoint{}, }, { - "no service port should return 0 endpoints", + "no service port should return 0 endpoint", &corev1.Service{}, nil, corev1.ProtocolTCP, nil, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { return nil, nil }, []ingress.Endpoint{}, }, { - "a service without endpoints should return 0 endpoints", + "a service without endpoint should return 0 endpoint", &corev1.Service{}, &corev1.ServicePort{Name: "default"}, corev1.ProtocolTCP, nil, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { return &corev1.Endpoints{}, nil }, []ingress.Endpoint{}, }, { - "a service type ServiceTypeExternalName service with an invalid port should return 0 endpoints", + "a service type ServiceTypeExternalName service with an invalid port should return 0 endpoint", &corev1.Service{ Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeExternalName, @@ -79,7 +79,7 @@ func TestGetEndpoints(t *testing.T) { &corev1.ServicePort{Name: "default"}, corev1.ProtocolTCP, nil, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { return &corev1.Endpoints{}, nil }, []ingress.Endpoint{}, @@ -107,7 +107,7 @@ func TestGetEndpoints(t *testing.T) { MaxFails: 0, FailTimeout: 0, }, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { return &corev1.Endpoints{}, nil }, []ingress.Endpoint{ @@ -142,13 +142,13 @@ func TestGetEndpoints(t *testing.T) { MaxFails: 0, FailTimeout: 0, }, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { return &corev1.Endpoints{}, nil }, []ingress.Endpoint{}, }, { - "should return no endpoints when there is an error searching for endpoints", + "should return no endpoint when there is an error searching for endpoints", &corev1.Service{ Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, @@ -170,13 +170,13 @@ func TestGetEndpoints(t *testing.T) { MaxFails: 0, FailTimeout: 0, }, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { return nil, fmt.Errorf("unexpected error") }, []ingress.Endpoint{}, }, { - "should return no endpoints when the protocol does not match", + "should return no endpoint when the protocol does not match", &corev1.Service{ Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, @@ -198,7 +198,7 @@ func TestGetEndpoints(t *testing.T) { MaxFails: 0, FailTimeout: 0, }, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { nodeName := "dummy" return &corev1.Endpoints{ Subsets: []corev1.EndpointSubset{ @@ -221,7 +221,7 @@ func TestGetEndpoints(t *testing.T) { []ingress.Endpoint{}, }, { - "should return no endpoints when there is no ready Addresses", + "should return no endpoint when there is no ready Addresses", &corev1.Service{ Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, @@ -243,7 +243,7 @@ func TestGetEndpoints(t *testing.T) { MaxFails: 0, FailTimeout: 0, }, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { nodeName := "dummy" return &corev1.Endpoints{ Subsets: []corev1.EndpointSubset{ @@ -266,7 +266,7 @@ func TestGetEndpoints(t *testing.T) { []ingress.Endpoint{}, }, { - "should return no endpoints when the name of the port name do not match any port in the endpoint Subsets", + "should return no endpoint when the name of the port name do not match any port in the endpoint Subsets", &corev1.Service{ Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, @@ -288,7 +288,7 @@ func TestGetEndpoints(t *testing.T) { MaxFails: 0, FailTimeout: 0, }, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { nodeName := "dummy" return &corev1.Endpoints{ Subsets: []corev1.EndpointSubset{ @@ -335,7 +335,7 @@ func TestGetEndpoints(t *testing.T) { MaxFails: 0, FailTimeout: 0, }, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { nodeName := "dummy" return &corev1.Endpoints{ Subsets: []corev1.EndpointSubset{ @@ -389,7 +389,7 @@ func TestGetEndpoints(t *testing.T) { MaxFails: 0, FailTimeout: 0, }, - func(*corev1.Service) (*corev1.Endpoints, error) { + func(string) (*corev1.Endpoints, error) { nodeName := "dummy" return &corev1.Endpoints{ Subsets: []corev1.EndpointSubset{ @@ -431,7 +431,7 @@ func TestGetEndpoints(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { result := getEndpoints(testCase.svc, testCase.port, testCase.proto, testCase.hz, testCase.fn) if len(testCase.result) != len(result) { - t.Errorf("expected %v Endpoints but got %v", testCase.result, len(result)) + t.Errorf("Expected %d Endpoints but got %d", len(testCase.result), len(result)) } }) } diff --git a/internal/ingress/controller/store/backend_ssl.go b/internal/ingress/controller/store/backend_ssl.go index d4f150823..b6635d51e 100644 --- a/internal/ingress/controller/store/backend_ssl.go +++ b/internal/ingress/controller/store/backend_ssl.go @@ -33,20 +33,19 @@ import ( "k8s.io/ingress-nginx/internal/net/ssl" ) -// syncSecret keeps in sync Secrets used by Ingress rules with the files on -// disk to allow copy of the content of the secret to disk to be used -// by external processes. +// syncSecret synchronizes the content of a TLS Secret (certificate(s), secret +// key) with the filesystem. The resulting files can be used by NGINX. func (s k8sStore) syncSecret(key string) { s.mu.Lock() defer s.mu.Unlock() - glog.V(3).Infof("starting syncing of secret %v", key) + glog.V(3).Infof("Syncing Secret %q", key) // TODO: getPemCertificate should not write to disk to avoid unnecessary overhead cert, err := s.getPemCertificate(key) if err != nil { if !isErrSecretForAuth(err) { - glog.Warningf("error obtaining PEM from secret %v: %v", key, err) + glog.Warningf("Error obtaining X.509 certificate: %v", err) } return } @@ -58,7 +57,7 @@ func (s k8sStore) syncSecret(key string) { // no need to update return } - glog.Infof("updating secret %v in the local store", key) + glog.Infof("Updating Secret %q in the local store", key) s.sslStore.Update(key, cert) // this update must trigger an update // (like an update event from a change in Ingress) @@ -66,7 +65,7 @@ func (s k8sStore) syncSecret(key string) { return } - glog.Infof("adding secret %v to the local store", key) + glog.Infof("Adding Secret %q to the local store", key) s.sslStore.Add(key, cert) // this update must trigger an update // (like an update event from a change in Ingress) @@ -78,7 +77,7 @@ func (s k8sStore) syncSecret(key string) { func (s k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error) { secret, err := s.listers.Secret.ByKey(secretName) if err != nil { - return nil, fmt.Errorf("error retrieving secret %v: %v", secretName, err) + return nil, err } cert, okcert := secret.Data[apiv1.TLSCertKey] @@ -93,40 +92,42 @@ func (s k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error) var sslCert *ingress.SSLCert if okcert && okkey { if cert == nil { - return nil, fmt.Errorf("secret %v has no 'tls.crt'", secretName) + return nil, fmt.Errorf("key 'tls.crt' missing from Secret %q", secretName) } if key == nil { - return nil, fmt.Errorf("secret %v has no 'tls.key'", secretName) + return nil, fmt.Errorf("key 'tls.key' missing from Secret %q", secretName) } // If 'ca.crt' is also present, it will allow this secret to be used in the // 'nginx.ingress.kubernetes.io/auth-tls-secret' annotation sslCert, err = ssl.AddOrUpdateCertAndKey(nsSecName, cert, key, ca, s.filesystem) if err != nil { - return nil, fmt.Errorf("unexpected error creating pem file: %v", err) + return nil, err } - glog.V(3).Infof("found 'tls.crt' and 'tls.key', configuring %v as a TLS Secret (CN: %v)", secretName, sslCert.CN) + msg := fmt.Sprintf("Configuring Secret %q for TLS encryption (CN: %v)", secretName, sslCert.CN) if ca != nil { - glog.V(3).Infof("found 'ca.crt', secret %v can also be used for Certificate Authentication", secretName) + msg += " and authentication" } + glog.V(3).Info(msg) + } else if ca != nil { sslCert, err = ssl.AddCertAuth(nsSecName, ca, s.filesystem) if err != nil { - return nil, fmt.Errorf("unexpected error creating pem file: %v", err) + return nil, err } // makes this secret in 'syncSecret' to be used for Certificate Authentication // this does not enable Certificate Authentication - glog.V(3).Infof("found only 'ca.crt', configuring %v as an Certificate Authentication Secret", secretName) + glog.V(3).Infof("Configuring Secret %q for TLS authentication", secretName) } else { if auth != nil { return nil, ErrSecretForAuth } - return nil, fmt.Errorf("no keypair or CA cert could be found in %v", secretName) + return nil, fmt.Errorf("Secret %q contains no keypair or CA certificate", secretName) } sslCert.Name = secret.Name @@ -137,8 +138,8 @@ func (s k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error) func (s k8sStore) checkSSLChainIssues() { for _, item := range s.ListLocalSSLCerts() { - secretName := k8s.MetaNamespaceKey(item) - secret, err := s.GetLocalSSLCert(secretName) + secrKey := k8s.MetaNamespaceKey(item) + secret, err := s.GetLocalSSLCert(secrKey) if err != nil { continue } @@ -150,7 +151,7 @@ func (s k8sStore) checkSSLChainIssues() { data, err := ssl.FullChainCert(secret.PemFileName, s.filesystem) if err != nil { - glog.Errorf("unexpected error generating SSL certificate with full intermediate chain CA certs: %v", err) + glog.Errorf("Error generating CA certificate chain for Secret %q: %v", secrKey, err) continue } @@ -158,13 +159,13 @@ func (s k8sStore) checkSSLChainIssues() { file, err := s.filesystem.Create(fullChainPemFileName) if err != nil { - glog.Errorf("unexpected error creating SSL certificate file %v: %v", fullChainPemFileName, err) + glog.Errorf("Error creating SSL certificate file for Secret %q: %v", secrKey, err) continue } _, err = file.Write(data) if err != nil { - glog.Errorf("unexpected error creating SSL certificate: %v", err) + glog.Errorf("Error creating SSL certificate for Secret %q: %v", secrKey, err) continue } @@ -172,14 +173,14 @@ func (s k8sStore) checkSSLChainIssues() { err = mergo.MergeWithOverwrite(dst, secret) if err != nil { - glog.Errorf("unexpected error creating SSL certificate: %v", err) + glog.Errorf("Error creating SSL certificate for Secret %q: %v", secrKey, err) continue } dst.FullChainPemFileName = fullChainPemFileName - glog.Infof("updating local copy of ssl certificate %v with missing intermediate CA certs", secretName) - s.sslStore.Update(secretName, dst) + glog.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() diff --git a/internal/ingress/controller/store/configmap.go b/internal/ingress/controller/store/configmap.go index d679a86c4..a57f7e787 100644 --- a/internal/ingress/controller/store/configmap.go +++ b/internal/ingress/controller/store/configmap.go @@ -17,8 +17,6 @@ limitations under the License. package store import ( - "fmt" - apiv1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" ) @@ -28,14 +26,14 @@ type ConfigMapLister struct { cache.Store } -// ByKey searches for a configmap in the local configmaps Store +// ByKey returns the ConfigMap matching key in the local ConfigMap Store. func (cml *ConfigMapLister) ByKey(key string) (*apiv1.ConfigMap, error) { s, exists, err := cml.GetByKey(key) if err != nil { return nil, err } if !exists { - return nil, fmt.Errorf("configmap %v was not found", key) + return nil, NotExistsError(key) } return s.(*apiv1.ConfigMap), nil } diff --git a/internal/ingress/controller/store/endpoint.go b/internal/ingress/controller/store/endpoint.go index 126b616b6..ff6fbd715 100644 --- a/internal/ingress/controller/store/endpoint.go +++ b/internal/ingress/controller/store/endpoint.go @@ -17,8 +17,6 @@ limitations under the License. package store import ( - "fmt" - apiv1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" ) @@ -28,15 +26,14 @@ type EndpointLister struct { cache.Store } -// GetServiceEndpoints returns the endpoints of a service, matched on service name. -func (s *EndpointLister) GetServiceEndpoints(svc *apiv1.Service) (*apiv1.Endpoints, error) { - key := fmt.Sprintf("%v/%v", svc.Namespace, svc.Name) +// ByKey returns the Endpoints of the Service matching key in the local Endpoint Store. +func (s *EndpointLister) ByKey(key string) (*apiv1.Endpoints, error) { eps, exists, err := s.GetByKey(key) if err != nil { return nil, err } if !exists { - return nil, fmt.Errorf("could not find endpoints for service %v", key) + return nil, NotExistsError(key) } return eps.(*apiv1.Endpoints), nil } diff --git a/internal/ingress/controller/store/ingress.go b/internal/ingress/controller/store/ingress.go index 67a7b6c53..19909cd76 100644 --- a/internal/ingress/controller/store/ingress.go +++ b/internal/ingress/controller/store/ingress.go @@ -17,8 +17,6 @@ limitations under the License. package store import ( - "fmt" - extensions "k8s.io/api/extensions/v1beta1" "k8s.io/client-go/tools/cache" ) @@ -28,14 +26,14 @@ type IngressLister struct { cache.Store } -// ByKey searches for an ingress in the local ingress Store +// ByKey returns the Ingress matching key in the local Ingress Store. func (il IngressLister) ByKey(key string) (*extensions.Ingress, error) { i, exists, err := il.GetByKey(key) if err != nil { return nil, err } if !exists { - return nil, fmt.Errorf("ingress %v was not found", key) + return nil, NotExistsError(key) } return i.(*extensions.Ingress), nil } diff --git a/internal/ingress/controller/store/ingress_annotation.go b/internal/ingress/controller/store/ingress_annotation.go index 676875aca..0a9d4011c 100644 --- a/internal/ingress/controller/store/ingress_annotation.go +++ b/internal/ingress/controller/store/ingress_annotation.go @@ -18,9 +18,22 @@ package store import ( "k8s.io/client-go/tools/cache" + "k8s.io/ingress-nginx/internal/ingress/annotations" ) // IngressAnnotationsLister makes a Store that lists annotations in Ingress rules. type IngressAnnotationsLister struct { cache.Store } + +// ByKey returns the Ingress annotations matching key in the local Ingress annotations Store. +func (il IngressAnnotationsLister) ByKey(key string) (*annotations.Ingress, error) { + i, exists, err := il.GetByKey(key) + if err != nil { + return nil, err + } + if !exists { + return nil, NotExistsError(key) + } + return i.(*annotations.Ingress), nil +} diff --git a/internal/ingress/controller/store/secret.go b/internal/ingress/controller/store/secret.go index 54774e8e9..c749155ed 100644 --- a/internal/ingress/controller/store/secret.go +++ b/internal/ingress/controller/store/secret.go @@ -17,8 +17,6 @@ limitations under the License. package store import ( - "fmt" - apiv1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" ) @@ -28,14 +26,14 @@ type SecretLister struct { cache.Store } -// ByKey searches for a secret in the local secrets Store +// ByKey returns the Secret matching key in the local Secret Store. func (sl *SecretLister) ByKey(key string) (*apiv1.Secret, error) { s, exists, err := sl.GetByKey(key) if err != nil { return nil, err } if !exists { - return nil, fmt.Errorf("secret %v was not found", key) + return nil, NotExistsError(key) } return s.(*apiv1.Secret), nil } diff --git a/internal/ingress/controller/store/service.go b/internal/ingress/controller/store/service.go index 44d235558..7e4731976 100644 --- a/internal/ingress/controller/store/service.go +++ b/internal/ingress/controller/store/service.go @@ -17,8 +17,6 @@ limitations under the License. package store import ( - "fmt" - apiv1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" ) @@ -28,14 +26,14 @@ type ServiceLister struct { cache.Store } -// ByKey searches for a service in the local secrets Store +// ByKey returns the Service matching key in the local Service Store. func (sl *ServiceLister) ByKey(key string) (*apiv1.Service, error) { s, exists, err := sl.GetByKey(key) if err != nil { return nil, err } if !exists { - return nil, fmt.Errorf("service %v was not found", key) + return nil, NotExistsError(key) } return s.(*apiv1.Service), nil } diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index c3f93784d..122b285a2 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -58,25 +58,26 @@ type Storer interface { // GetBackendConfiguration returns the nginx configuration stored in a configmap GetBackendConfiguration() ngx_config.Configuration - // GetConfigMap returns a ConfigmMap using the namespace and name as key + // GetConfigMap returns the ConfigMap matching key. GetConfigMap(key string) (*corev1.ConfigMap, error) - // GetSecret returns a Secret using the namespace and name as key + // GetSecret returns the Secret matching key. GetSecret(key string) (*corev1.Secret, error) - // GetService returns a Service using the namespace and name as key + // GetService returns the Service matching key. GetService(key string) (*corev1.Service, error) - GetServiceEndpoints(svc *corev1.Service) (*corev1.Endpoints, error) + // GetServiceEndpoints returns the Endpoints of a Service matching key. + GetServiceEndpoints(key string) (*corev1.Endpoints, error) - // GetSecret returns an Ingress using the namespace and name as key + // GetIngress returns the Ingress matching key. GetIngress(key string) (*extensions.Ingress, error) - // ListIngresses returns the list of Ingresses + // ListIngresses returns a list of all Ingresses in the store. ListIngresses() []*extensions.Ingress - // GetIngressAnnotations returns the annotations associated to an Ingress - GetIngressAnnotations(ing *extensions.Ingress) (*annotations.Ingress, error) + // GetIngressAnnotations returns the parsed annotations of an Ingress matching key. + GetIngressAnnotations(key string) (*annotations.Ingress, error) // GetLocalSSLCert returns the local copy of a SSLCert GetLocalSSLCert(name string) (*ingress.SSLCert, error) @@ -110,7 +111,7 @@ const ( ConfigurationEvent EventType = "CONFIGURATION" ) -// Event holds the context of an event +// Event holds the context of an event. type Event struct { Type EventType Obj interface{} @@ -125,7 +126,7 @@ type Informer struct { ConfigMap cache.SharedIndexInformer } -// Lister returns the stores for ingresses, services, endpoints, secrets and configmaps. +// Lister contains object listers (stores). type Lister struct { Ingress IngressLister Service ServiceLister @@ -135,6 +136,14 @@ type Lister struct { IngressAnnotation IngressAnnotationsLister } +// NotExistsError is returned when an object does not exist in a local store. +type NotExistsError string + +// Error implements the error interface. +func (e NotExistsError) Error() string { + return fmt.Sprintf("no object matching key %q in local store", string(e)) +} + // Run initiates the synchronization of the informers against the API server. func (i *Informer) Run(stopCh chan struct{}) { go i.Endpoint.Run(stopCh) @@ -601,7 +610,7 @@ func (s k8sStore) syncSecrets(ing *extensions.Ingress) { } } -// GetSecret returns a Secret using the namespace and name as key +// GetSecret returns the Secret matching key. func (s k8sStore) GetSecret(key string) (*corev1.Secret, error) { return s.listers.Secret.ByKey(key) } @@ -618,12 +627,12 @@ func (s k8sStore) ListLocalSSLCerts() []*ingress.SSLCert { return certs } -// GetService returns a Service using the namespace and name as key +// GetService returns the Service matching key. func (s k8sStore) GetService(key string) (*corev1.Service, error) { return s.listers.Service.ByKey(key) } -// GetIngress returns an Ingress using the namespace and name as key +// GetIngress returns the Ingress matching key. func (s k8sStore) GetIngress(key string) (*extensions.Ingress, error) { return s.listers.Ingress.ByKey(key) } @@ -656,17 +665,9 @@ func (s k8sStore) ListIngresses() []*extensions.Ingress { return ingresses } -// GetIngressAnnotations returns the annotations associated to an Ingress -func (s k8sStore) GetIngressAnnotations(ing *extensions.Ingress) (*annotations.Ingress, error) { - key := k8s.MetaNamespaceKey(ing) - item, exists, err := s.listers.IngressAnnotation.GetByKey(key) - if err != nil { - return &annotations.Ingress{}, fmt.Errorf("unexpected error getting ingress annotation %v: %v", key, err) - } - if !exists { - return &annotations.Ingress{}, fmt.Errorf("ingress annotations %v was not found", key) - } - return item.(*annotations.Ingress), nil +// GetIngressAnnotations returns the parsed annotations of an Ingress matching key. +func (s k8sStore) GetIngressAnnotations(key string) (*annotations.Ingress, error) { + return s.listers.IngressAnnotation.ByKey(key) } // GetLocalSSLCert returns the local copy of a SSLCert @@ -674,12 +675,14 @@ func (s k8sStore) GetLocalSSLCert(key string) (*ingress.SSLCert, error) { return s.sslStore.ByKey(key) } +// GetConfigMap returns the ConfigMap matching key. func (s k8sStore) GetConfigMap(key string) (*corev1.ConfigMap, error) { return s.listers.ConfigMap.ByKey(key) } -func (s k8sStore) GetServiceEndpoints(svc *corev1.Service) (*corev1.Endpoints, error) { - return s.listers.Endpoint.GetServiceEndpoints(svc) +// GetServiceEndpoints returns the Endpoints of a Service matching key. +func (s k8sStore) GetServiceEndpoints(key string) (*corev1.Endpoints, error) { + return s.listers.Endpoint.ByKey(key) } // GetAuthCertificate is used by the auth-tls annotations to get a cert from a secret