diff --git a/cmd/check-gke-ingress/app/command/root.go b/cmd/check-gke-ingress/app/command/root.go index c2d599e22a..12f7dcba7b 100644 --- a/cmd/check-gke-ingress/app/command/root.go +++ b/cmd/check-gke-ingress/app/command/root.go @@ -21,6 +21,9 @@ import ( "os" "github.com/spf13/cobra" + "k8s.io/ingress-gce/cmd/check-gke-ingress/app/ingress" + "k8s.io/ingress-gce/cmd/check-gke-ingress/app/kube" + "k8s.io/ingress-gce/cmd/check-gke-ingress/app/report" ) var ( @@ -38,7 +41,20 @@ var rootCmd = &cobra.Command{ fmt.Fprintf(os.Stderr, "Error parsing flags: %v", err) os.Exit(1) } - fmt.Println("Starting check-gke-ingress") + client, err := kube.NewClientSet(kubecontext, kubeconfig) + beconfigClient, err := kube.NewBackendConfigClientSet(kubecontext, kubeconfig) + feConfigClient, err := kube.NewFrontendConfigClientSet(kubecontext, kubeconfig) + if err != nil { + fmt.Fprintf(os.Stderr, "Error connecting to Kubernetes: %v", err) + os.Exit(1) + } + output := ingress.CheckAllIngresses(namespace, client, beconfigClient, feConfigClient) + res, err := report.JsonReport(&output) + if err != nil { + fmt.Fprintf(os.Stderr, "Error processing results: %v", err) + os.Exit(1) + } + fmt.Print(res) }, } diff --git a/cmd/check-gke-ingress/app/ingress/ingress.go b/cmd/check-gke-ingress/app/ingress/ingress.go new file mode 100644 index 0000000000..dd86cd117b --- /dev/null +++ b/cmd/check-gke-ingress/app/ingress/ingress.go @@ -0,0 +1,167 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ingress + +import ( + "context" + "fmt" + "os" + "reflect" + "runtime" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/ingress-gce/cmd/check-gke-ingress/app/report" + beconfigclient "k8s.io/ingress-gce/pkg/backendconfig/client/clientset/versioned" + feconfigclient "k8s.io/ingress-gce/pkg/frontendconfig/client/clientset/versioned" +) + +func CheckAllIngresses(namespace string, client kubernetes.Interface, beconfigClient beconfigclient.Interface, feConfigClient feconfigclient.Interface) report.Report { + output := report.Report{ + Resources: []*report.Resource{}, + } + + ingressList, err := client.NetworkingV1().Ingresses(namespace).List(context.TODO(), metav1.ListOptions{}) + if err != nil { + fmt.Fprintf(os.Stderr, "Error listing ingresses: %v", err) + os.Exit(1) + } + + ingressChecks := []ingressCheckFunc{ + CheckIngressRule, + CheckL7ILBFrontendConfig, + CheckRuleHostOverwrite, + } + + serviceChecks := []serviceCheckFunc{ + CheckServiceExistence, + CheckBackendConfigAnnotation, + CheckAppProtocolAnnotation, + CheckL7ILBNegAnnotation, + } + + feconfigChecks := []frontendConfigCheckFunc{CheckFrontendConfigExistence} + + beconfigChecks := []backendConfigCheckFunc{ + CheckBackendConfigExistence, + CheckHealthCheckTimeout, + } + + for _, ingress := range ingressList.Items { + + // Ingress related checks + ingressRes := &report.Resource{ + Kind: "Ingress", + Namespace: ingress.Namespace, + Name: ingress.Name, + Checks: []*report.Check{}, + } + ingressChecker := &IngressChecker{ + client: client, + ingress: &ingress, + } + + for _, check := range ingressChecks { + checkName, res, msg := check(ingressChecker) + addCheckResult(ingressRes, checkName, msg, res) + } + + // FrontendConfig related checks + feconfigChecker := &FrontendConfigChecker{ + client: feConfigClient, + namespace: ingress.Namespace, + name: ingress.Name, + } + + for _, check := range feconfigChecks { + checkName, res, msg := check(feconfigChecker) + addCheckResult(ingressRes, checkName, msg, res) + } + + // Get the names of the services referenced by the ingress. + svcNames := []string{} + if ingress.Spec.DefaultBackend != nil { + svcNames = append(svcNames, ingress.Spec.DefaultBackend.Service.Name) + } + if ingress.Spec.Rules != nil { + for _, rule := range ingress.Spec.Rules { + if rule.HTTP != nil { + for _, path := range rule.HTTP.Paths { + if path.Backend.Service != nil { + svcNames = append(svcNames, path.Backend.Service.Name) + } + } + } + } + } + + // Service related checks + for _, svcName := range svcNames { + serviceChecker := &ServiceChecker{ + namespace: ingress.Namespace, + name: svcName, + isL7ILB: isL7ILB(&ingress), + client: client, + } + + for _, check := range serviceChecks { + checkName, res, msg := check(serviceChecker) + addCheckResult(ingressRes, checkName, msg, res) + } + + // Get all the BackendConfigs referenced by the service. + beconfigNames := []string{} + if serviceChecker.beConfigs != nil { + if serviceChecker.beConfigs.Default != "" { + beconfigNames = append(beconfigNames, serviceChecker.beConfigs.Default) + } + for _, beconfig := range serviceChecker.beConfigs.Ports { + beconfigNames = append(beconfigNames, beconfig) + } + } + // BackendConfig related rules + for _, beconfigName := range beconfigNames { + beconfigChecker := &BackendConfigChecker{ + namespace: ingress.Namespace, + name: beconfigName, + client: beconfigClient, + serviceName: svcName, + } + + for _, check := range beconfigChecks { + checkName, res, msg := check(beconfigChecker) + addCheckResult(ingressRes, checkName, msg, res) + } + } + } + output.Resources = append(output.Resources, ingressRes) + } + + return output +} + +func addCheckResult(ingressRes *report.Resource, checkName, msg, res string) { + ingressRes.Checks = append(ingressRes.Checks, &report.Check{ + Name: checkName, + Message: msg, + Result: res, + }) +} + +func getCheckName(check func()) string { + return runtime.FuncForPC(reflect.ValueOf(check).Pointer()).Name() +} diff --git a/cmd/check-gke-ingress/app/ingress/rule.go b/cmd/check-gke-ingress/app/ingress/rule.go index 5b238b8819..01e6e2fb01 100644 --- a/cmd/check-gke-ingress/app/ingress/rule.go +++ b/cmd/check-gke-ingress/app/ingress/rule.go @@ -34,143 +34,228 @@ import ( feconfigclient "k8s.io/ingress-gce/pkg/frontendconfig/client/clientset/versioned" ) -// CheckServiceExistence checks whether a service exists. -func CheckServiceExistence(namespace, name string, client clientset.Interface) (*corev1.Service, string, string) { - svc, err := client.CoreV1().Services(namespace).Get(context.TODO(), name, metav1.GetOptions{}) - if err != nil { - if apierrors.IsNotFound(err) { - return nil, report.Failed, fmt.Sprintf("Service %s/%s does not exist", namespace, name) - } - return nil, report.Failed, fmt.Sprintf("Failed to get service %s/%s: %v", namespace, name, err) - } - return svc, report.Passed, fmt.Sprintf("Service %s/%s found", namespace, name) +const ( + ServiceExistenceCheck = "ServiceExistenceCheck" + BackendConfigAnnotationCheck = "BackendConfigAnnotationCheck" + BackendConfigExistenceCheck = "BackendConfigExistenceCheck" + HealthCheckTimeoutCheck = "HealthCheckTimeoutCheck" + IngressRuleCheck = "IngressRuleCheck" + FrontendConfigExistenceCheck = "FrontendConfigExistenceCheck" + RuleHostOverwriteCheck = "RuleHostOverwriteCheck" + AppProtocolAnnotationCheck = "AppProtocolAnnotationCheck" + L7ILBFrontendConfigCheck = "L7ILBFrontendConfigCheck" + L7ILBNegAnnotationCheck = "L7ILBNegAnnotationCheck" +) + +type IngressChecker struct { + // Kubernetes client + client clientset.Interface + // Ingress object to be checked + ingress *networkingv1.Ingress } -// CheckBackendConfigAnnotation checks the BackendConfig annotation of a -// service for: -// -// whether the annotation is a valid BackendConfig json object. -// whether the annotation has `default` or `ports` field. -func CheckBackendConfigAnnotation(svc *corev1.Service) (*annotations.BackendConfigs, string, string) { - val, ok := getBackendConfigAnnotation(svc) - if !ok { - return nil, report.Skipped, fmt.Sprintf("Service %s/%s does not have backendconfig annotation", svc.Namespace, svc.Name) - } - beConfigs := &annotations.BackendConfigs{} - if err := json.Unmarshal([]byte(val), beConfigs); err != nil { - return nil, report.Failed, fmt.Sprintf("BackendConfig annotation is invalid in service %s/%s", svc.Namespace, svc.Name) - } - if beConfigs.Default == "" && beConfigs.Ports == nil { - return nil, report.Failed, fmt.Sprintf("BackendConfig annotation is missing both `default` and `ports` field in service %s/%s", svc.Namespace, svc.Name) - } - return beConfigs, report.Passed, fmt.Sprintf("BackendConfig annotation is valid in service %s/%s", svc.Namespace, svc.Name) +type ServiceChecker struct { + // Kubernetes client + client clientset.Interface + // Service object to be checked + service *corev1.Service + // Backendconfig annotatation in the service + beConfigs *annotations.BackendConfigs + // Namespace of the service + namespace string + // Name of the service + name string + // Whether the service is referenced by an internal ingress + isL7ILB bool } -// CheckBackendConfigExistence checks whether a BackendConfig exists. -func CheckBackendConfigExistence(ns string, beConfigName string, svcName string, client beconfigclient.Interface) (*beconfigv1.BackendConfig, string, string) { - beConfig, err := client.CloudV1().BackendConfigs(ns).Get(context.TODO(), beConfigName, metav1.GetOptions{}) - if err != nil { - if apierrors.IsNotFound(err) { - return nil, report.Failed, fmt.Sprintf("BackendConfig %s/%s in service %s/%s does not exist", ns, beConfigName, ns, svcName) +type BackendConfigChecker struct { + // BackendConfig client + client beconfigclient.Interface + // Namespace of the backendConfig + namespace string + // Name of the backendConfig + name string + // Backendconfig object to be checked + beConfig *beconfigv1.BackendConfig + // Name of the service by which the backendConfig is referenced + serviceName string +} + +type FrontendConfigChecker struct { + // FrontendConfig client + client feconfigclient.Interface + // Namespace of the frontendConfig + namespace string + // Name of the frontendConfig + name string + // FrontendConfig object to be checked + feConfig *feconfigv1beta1.FrontendConfig +} + +type ingressCheckFunc func(c *IngressChecker) (string, string, string) + +type serviceCheckFunc func(c *ServiceChecker) (string, string, string) + +type backendConfigCheckFunc func(c *BackendConfigChecker) (string, string, string) + +type frontendConfigCheckFunc func(c *FrontendConfigChecker) (string, string, string) + +// CheckIngressRule checks whether an ingress rule has the http field. +func CheckIngressRule(c *IngressChecker) (string, string, string) { + for _, rule := range c.ingress.Spec.Rules { + if rule.HTTP == nil { + return IngressRuleCheck, report.Failed, "IngressRule has no field `http`" } - return nil, report.Failed, fmt.Sprintf("Failed to get backendConfig %s/%s in service %s/%s", ns, beConfigName, ns, svcName) } - return beConfig, report.Passed, fmt.Sprintf("BackendConfig %s/%s in service %s/%s found", ns, beConfigName, ns, svcName) + return IngressRuleCheck, report.Passed, "IngressRule has field `http`" } -// CheckHealthCheckTimeout checks whether timeout time is smaller than check -// interval in backendconfig health check configuration. -func CheckHealthCheckTimeout(beConfig *beconfigv1.BackendConfig, svcName string) (string, string) { - if beConfig.Spec.HealthCheck == nil { - return report.Skipped, fmt.Sprintf("BackendConfig %s/%s in service %s/%s does not have healthcheck specified", beConfig.Namespace, beConfig.Name, beConfig.Namespace, svcName) - } - if beConfig.Spec.HealthCheck.TimeoutSec == nil || beConfig.Spec.HealthCheck.CheckIntervalSec == nil { - return report.Skipped, fmt.Sprintf("BackendConfig %s/%s in service %s/%s does not have timeoutSec or checkIntervalSec specified", beConfig.Namespace, beConfig.Name, beConfig.Namespace, svcName) +// CheckL7ILBFrontendConfig checks whether an internal ingress has a +// frontendConfig. It will fail if an internal ingress has a frontendConfig. +func CheckL7ILBFrontendConfig(c *IngressChecker) (string, string, string) { + if !isL7ILB(c.ingress) { + return L7ILBFrontendConfigCheck, report.Skipped, fmt.Sprintf("Ingress %s/%s is not for L7 internal load balancing", c.ingress.Namespace, c.ingress.Name) } - if *beConfig.Spec.HealthCheck.TimeoutSec > *beConfig.Spec.HealthCheck.CheckIntervalSec { - return report.Failed, fmt.Sprintf("BackendConfig %s/%s in service %s/%s has healthcheck timeoutSec greater than checkIntervalSec", beConfig.Namespace, beConfig.Name, beConfig.Namespace, svcName) + if _, ok := getFrontendConfigAnnotation(c.ingress); ok { + return L7ILBFrontendConfigCheck, report.Failed, fmt.Sprintf("Ingress %s/%s for L7 internal load balancing has a frontendConfig annotation", c.ingress.Namespace, c.ingress.Name) } - return report.Passed, fmt.Sprintf("BackendConfig %s/%s in service %s/%s healthcheck configuration is valid", beConfig.Namespace, beConfig.Name, beConfig.Namespace, svcName) + return L7ILBFrontendConfigCheck, report.Passed, fmt.Sprintf("Ingress %s/%s for L7 internal load balancing does not have a frontendConfig annotation", c.ingress.Namespace, c.ingress.Name) } -// CheckIngressRule checks whether an ingress rule has the http field. -func CheckIngressRule(ingressRule *networkingv1.IngressRule) (*networkingv1.HTTPIngressRuleValue, string, string) { - if ingressRule.HTTP == nil { - return nil, report.Failed, "IngressRule has no field `http`" +// CheckRuleHostOverwrite checks whether hosts of ingress rules are unique. +func CheckRuleHostOverwrite(c *IngressChecker) (string, string, string) { + hostSet := make(map[string]struct{}) + for _, rule := range c.ingress.Spec.Rules { + if _, ok := hostSet[rule.Host]; ok { + return RuleHostOverwriteCheck, report.Failed, fmt.Sprintf("Ingress rules have identical host: %s", rule.Host) + } + hostSet[rule.Host] = struct{}{} } - return ingressRule.HTTP, report.Passed, "IngressRule has field `http`" + return RuleHostOverwriteCheck, report.Passed, fmt.Sprintf("Ingress rule hosts are unique") } -// CheckFrontendConfigExistence checks whether a FrontendConfig exists. -func CheckFrontendConfigExistence(namespace, name string, client feconfigclient.Interface) (*feconfigv1beta1.FrontendConfig, string, string) { - feConfig, err := client.NetworkingV1beta1().FrontendConfigs(namespace).Get(context.TODO(), name, metav1.GetOptions{}) +// CheckServiceExistence checks whether a service exists. +func CheckServiceExistence(c *ServiceChecker) (string, string, string) { + service, err := c.client.CoreV1().Services(c.namespace).Get(context.TODO(), c.name, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { - return nil, report.Failed, fmt.Sprintf("FrontendConfig %s/%s does not exist", namespace, name) + return ServiceExistenceCheck, report.Failed, fmt.Sprintf("Service %s/%s does not exist", c.namespace, c.name) } - return nil, report.Failed, fmt.Sprintf("Failed to get frontendConfig %s/%s", namespace, name) + return ServiceExistenceCheck, report.Failed, fmt.Sprintf("Failed to get service %s/%s: %v", c.namespace, c.name, err) } - return feConfig, report.Passed, fmt.Sprintf("FrontendConfig %s/%s found", namespace, name) + c.service = service + return ServiceExistenceCheck, report.Passed, fmt.Sprintf("Service %s/%s found", c.namespace, c.name) } -// CheckRuleHostOverwrite checks whether hosts of ingress rules are unique. -func CheckRuleHostOverwrite(rules []networkingv1.IngressRule) (string, string) { - hostSet := make(map[string]struct{}) - for _, rule := range rules { - if _, ok := hostSet[rule.Host]; ok { - return report.Failed, fmt.Sprintf("Ingress rules have identical host: %s", rule.Host) - } - hostSet[rule.Host] = struct{}{} +// CheckBackendConfigAnnotation checks the BackendConfig annotation of a +// service for: +// +// whether the annotation is a valid BackendConfig json object. +// whether the annotation has `default` or `ports` field. +func CheckBackendConfigAnnotation(c *ServiceChecker) (string, string, string) { + if c.service == nil { + return BackendConfigAnnotationCheck, report.Skipped, fmt.Sprintf("Service %s/%s does not exist", c.namespace, c.name) + } + val, ok := getBackendConfigAnnotation(c.service) + if !ok { + return BackendConfigAnnotationCheck, report.Skipped, fmt.Sprintf("Service %s/%s does not have backendconfig annotation", c.service.Namespace, c.service.Name) } - return report.Passed, fmt.Sprintf("Ingress rule hosts are unique") + beConfigs := &annotations.BackendConfigs{} + if err := json.Unmarshal([]byte(val), beConfigs); err != nil { + return BackendConfigAnnotationCheck, report.Failed, fmt.Sprintf("BackendConfig annotation is invalid in service %s/%s", c.service.Namespace, c.service.Name) + } + if beConfigs.Default == "" && beConfigs.Ports == nil { + return BackendConfigAnnotationCheck, report.Failed, fmt.Sprintf("BackendConfig annotation is missing both `default` and `ports` field in service %s/%s", c.service.Namespace, c.service.Name) + } + c.beConfigs = beConfigs + return BackendConfigAnnotationCheck, report.Passed, fmt.Sprintf("BackendConfig annotation is valid in service %s/%s", c.service.Namespace, c.service.Name) } // CheckAppProtocolAnnotation check whether the protocal annotation specified // in a service is in valid format and with valid protocols. -func CheckAppProtocolAnnotation(svc *corev1.Service) (string, string) { - val, ok := getAppProtocolsAnnotation(svc) +func CheckAppProtocolAnnotation(c *ServiceChecker) (string, string, string) { + if c.service == nil { + return AppProtocolAnnotationCheck, report.Skipped, fmt.Sprintf("Service %s/%s does not exist", c.namespace, c.name) + } + val, ok := getAppProtocolsAnnotation(c.service) if !ok { - return report.Skipped, fmt.Sprintf("Service %s/%s does not have AppProtocolAnnotation", svc.Namespace, svc.Name) + return AppProtocolAnnotationCheck, report.Skipped, fmt.Sprintf("Service %s/%s does not have AppProtocolAnnotation", c.service.Namespace, c.service.Name) } var portToProtocols map[string]annotations.AppProtocol if err := json.Unmarshal([]byte(val), &portToProtocols); err != nil { - return report.Failed, fmt.Sprintf("AppProtocol annotation is in invalid format in service %s/%s", svc.Namespace, svc.Name) + return AppProtocolAnnotationCheck, report.Failed, fmt.Sprintf("AppProtocol annotation is in invalid format in service %s/%s", c.service.Namespace, c.service.Name) } for _, protocol := range portToProtocols { if protocol != annotations.ProtocolHTTP && protocol != annotations.ProtocolHTTPS && protocol != annotations.ProtocolHTTP2 { - return report.Failed, fmt.Sprintf("Invalid port application protocol in service %s/%s: %v, must be one of [`HTTP`,`HTTPS`,`HTTP2`]", svc.Namespace, svc.Name, protocol) + return AppProtocolAnnotationCheck, report.Failed, fmt.Sprintf("Invalid port application protocol in service %s/%s: %v, must be one of [`HTTP`,`HTTPS`,`HTTP2`]", c.service.Namespace, c.service.Name, protocol) } } - return report.Passed, fmt.Sprintf("AppProtocol annotation is valid in service %s/%s", svc.Namespace, svc.Name) -} - -// CheckL7ILBFrontendConfig checks whether an internal ingress has a -// frontendConfig. It will fail if an internal ingress has a frontendConfig. -func CheckL7ILBFrontendConfig(ing *networkingv1.Ingress) (string, string) { - if !isL7ILB(ing) { - return report.Skipped, fmt.Sprintf("Ingress %s/%s is not for L7 internal load balancing", ing.Namespace, ing.Name) - } - if _, ok := getFrontendConfigAnnotation(ing); ok { - return report.Failed, fmt.Sprintf("Ingress %s/%s for L7 internal load balancing has a frontendConfig annotation", ing.Namespace, ing.Name) - } - return report.Passed, fmt.Sprintf("Ingress %s/%s for L7 internal load balancing does not have a frontendConfig annotation", ing.Namespace, ing.Name) + return AppProtocolAnnotationCheck, report.Passed, fmt.Sprintf("AppProtocol annotation is valid in service %s/%s", c.service.Namespace, c.service.Name) } // CheckL7ILBNegAnnotation check whether a service which belongs to an internal // ingress has a correct NEG annotation. -func CheckL7ILBNegAnnotation(svc *corev1.Service) (string, string) { - val, ok := getNegAnnotation(svc) +func CheckL7ILBNegAnnotation(c *ServiceChecker) (string, string, string) { + if !c.isL7ILB { + return L7ILBNegAnnotationCheck, report.Skipped, fmt.Sprintf("Service %s/%s is not referenced by an internal ingress", c.service.Namespace, c.service.Name) + } + val, ok := getNegAnnotation(c.service) if !ok { - return report.Failed, fmt.Sprintf("No Neg annotation found in service %s/%s for internal HTTP(S) load balancing", svc.Namespace, svc.Name) + return L7ILBNegAnnotationCheck, report.Failed, fmt.Sprintf("No Neg annotation found in service %s/%s for internal HTTP(S) load balancing", c.service.Namespace, c.service.Name) } var res annotations.NegAnnotation if err := json.Unmarshal([]byte(val), &res); err != nil { - return report.Failed, fmt.Sprintf("Invalid Neg annotation found in service %s/%s for internal HTTP(S) load balancing", svc.Namespace, svc.Name) + return L7ILBNegAnnotationCheck, report.Failed, fmt.Sprintf("Invalid Neg annotation found in service %s/%s for internal HTTP(S) load balancing", c.service.Namespace, c.service.Name) } if !res.Ingress { - return report.Failed, fmt.Sprintf("Neg annotation ingress field is not true in service %s/%s for internal HTTP(S) load balancing", svc.Namespace, svc.Name) + return L7ILBNegAnnotationCheck, report.Failed, fmt.Sprintf("Neg annotation ingress field is not true in service %s/%s for internal HTTP(S) load balancing", c.service.Namespace, c.service.Name) + } + return L7ILBNegAnnotationCheck, report.Passed, fmt.Sprintf("Neg annotation is set correctly in service %s/%s for internal HTTP(S) load balancing", c.service.Namespace, c.service.Name) +} + +// CheckBackendConfigExistence checks whether a BackendConfig exists. +func CheckBackendConfigExistence(c *BackendConfigChecker) (string, string, string) { + beConfig, err := c.client.CloudV1().BackendConfigs(c.namespace).Get(context.TODO(), c.name, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + return BackendConfigExistenceCheck, report.Failed, fmt.Sprintf("BackendConfig %s/%s referenced by service %s/%s does not exist", c.namespace, c.name, c.namespace, c.serviceName) + } + return BackendConfigExistenceCheck, report.Failed, fmt.Sprintf("Failed to get backendConfig %s/%s referenced by service %s/%s", c.namespace, c.name, c.namespace, c.serviceName) + } + c.beConfig = beConfig + return BackendConfigExistenceCheck, report.Passed, fmt.Sprintf("Found backendConfig %s/%s referenced by service %s/%s", c.namespace, c.name, c.namespace, c.serviceName) +} + +// CheckHealthCheckTimeout checks whether timeout time is smaller than check +// interval in backendconfig health check configuration. +func CheckHealthCheckTimeout(c *BackendConfigChecker) (string, string, string) { + if c.beConfig == nil { + return HealthCheckTimeoutCheck, report.Skipped, fmt.Sprintf("BackendConfig %s/%s does not exist", c.namespace, c.name) + } + if c.beConfig.Spec.HealthCheck == nil { + return HealthCheckTimeoutCheck, report.Skipped, fmt.Sprintf("BackendConfig %s/%s does not have healthcheck specified", c.namespace, c.name) + } + if c.beConfig.Spec.HealthCheck.TimeoutSec == nil || c.beConfig.Spec.HealthCheck.CheckIntervalSec == nil { + return HealthCheckTimeoutCheck, report.Skipped, fmt.Sprintf("BackendConfig %s/%s does not have timeoutSec or checkIntervalSec specified", c.namespace, c.name) + } + if *c.beConfig.Spec.HealthCheck.TimeoutSec > *c.beConfig.Spec.HealthCheck.CheckIntervalSec { + return HealthCheckTimeoutCheck, report.Failed, fmt.Sprintf("BackendConfig %s/%s has healthcheck timeoutSec greater than checkIntervalSec", c.namespace, c.name) + } + return HealthCheckTimeoutCheck, report.Passed, fmt.Sprintf("BackendConfig %s/%s healthcheck configuration is valid", c.namespace, c.name) +} + +// CheckFrontendConfigExistence checks whether a FrontendConfig exists. +func CheckFrontendConfigExistence(c *FrontendConfigChecker) (string, string, string) { + feConfig, err := c.client.NetworkingV1beta1().FrontendConfigs(c.namespace).Get(context.TODO(), c.name, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + return FrontendConfigExistenceCheck, report.Failed, fmt.Sprintf("FrontendConfig %s/%s does not exist", c.namespace, c.name) + } + return FrontendConfigExistenceCheck, report.Failed, fmt.Sprintf("Failed to get frontendConfig %s/%s", c.namespace, c.name) } - return report.Passed, fmt.Sprintf("Neg annotation is set correctly in service %s/%s for internal HTTP(S) load balancing", svc.Namespace, svc.Name) + c.feConfig = feConfig + return FrontendConfigExistenceCheck, report.Passed, fmt.Sprintf("FrontendConfig %s/%s found", c.namespace, c.name) } // getBackendConfigAnnotation gets the BackendConfig annotation from a service. diff --git a/cmd/check-gke-ingress/app/ingress/rule_test.go b/cmd/check-gke-ingress/app/ingress/rule_test.go index c936a4f25f..d02e0ad70d 100644 --- a/cmd/check-gke-ingress/app/ingress/rule_test.go +++ b/cmd/check-gke-ingress/app/ingress/rule_test.go @@ -46,6 +46,7 @@ func TestCheckServiceExistence(t *testing.T) { for _, tc := range []struct { desc string + checker ServiceChecker namespace string name string expect string @@ -73,7 +74,12 @@ func TestCheckServiceExistence(t *testing.T) { expect: report.Failed, }, } { - _, res, _ := CheckServiceExistence(tc.namespace, tc.name, client) + checker := &ServiceChecker{ + client: client, + namespace: tc.namespace, + name: tc.name, + } + _, res, _ := CheckServiceExistence(checker) if res != tc.expect { t.Errorf("For test case %q, expect check result = %s, but got %s", tc.desc, tc.expect, res) } @@ -140,7 +146,10 @@ func TestCheckBackendConfigAnnotation(t *testing.T) { expect: report.Failed, }, } { - _, res, _ := CheckBackendConfigAnnotation(&tc.svc) + checker := &ServiceChecker{ + service: &tc.svc, + } + _, res, _ := CheckBackendConfigAnnotation(checker) if res != tc.expect { t.Errorf("For test case %q, expect check result = %s, but got %s", tc.desc, tc.expect, res) } @@ -185,7 +194,12 @@ func TestCheckBackendConfigExistence(t *testing.T) { expect: report.Failed, }, } { - _, res, _ := CheckBackendConfigExistence(tc.namespace, tc.name, "svc-1", client) + checker := &BackendConfigChecker{ + namespace: tc.namespace, + name: tc.name, + client: client, + } + _, res, _ := CheckBackendConfigExistence(checker) if res != tc.expect { t.Errorf("For test case %q, expect check result = %s, but got %s", tc.desc, tc.expect, res) } @@ -193,7 +207,6 @@ func TestCheckBackendConfigExistence(t *testing.T) { } func TestCheckHealthCheckConfig(t *testing.T) { - thirtyVar := int64(30) twentyVar := int64(20) for _, tc := range []struct { @@ -262,7 +275,10 @@ func TestCheckHealthCheckConfig(t *testing.T) { }, Spec: tc.spec, } - res, _ := CheckHealthCheckTimeout(&beconfig, "") + checker := &BackendConfigChecker{ + beConfig: &beconfig, + } + _, res, _ := CheckHealthCheckTimeout(checker) if diff := cmp.Diff(tc.expect, res); diff != "" { t.Errorf("For test case %s, (-want +got):\n%s", tc.desc, diff) } @@ -307,7 +323,12 @@ func TestCheckFrontendConfigExistence(t *testing.T) { expect: report.Failed, }, } { - _, res, _ := CheckFrontendConfigExistence(tc.namespace, tc.name, client) + checker := &FrontendConfigChecker{ + namespace: tc.namespace, + name: tc.name, + client: client, + } + _, res, _ := CheckFrontendConfigExistence(checker) if res != tc.expect { t.Errorf("For test case %q, expect check result = %s, but got %s", tc.desc, tc.expect, res) } @@ -344,7 +365,16 @@ func TestCheckIngressRule(t *testing.T) { expect: report.Failed, }, } { - _, res, _ := CheckIngressRule(&tc.ingressRule) + checker := &IngressChecker{ + ingress: &networkingv1.Ingress{ + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + tc.ingressRule, + }, + }, + }, + } + _, res, _ := CheckIngressRule(checker) if diff := cmp.Diff(tc.expect, res); diff != "" { t.Errorf("For test case %s, (-want +got):\n%s", tc.desc, diff) } @@ -387,7 +417,14 @@ func TestCheckRuleHostOverwrite(t *testing.T) { expect: report.Passed, }, } { - res, _ := CheckRuleHostOverwrite(tc.rules) + checker := &IngressChecker{ + ingress: &networkingv1.Ingress{ + Spec: networkingv1.IngressSpec{ + Rules: tc.rules, + }, + }, + } + _, res, _ := CheckRuleHostOverwrite(checker) if res != tc.expect { t.Errorf("For test case %q, expect check result = %s, but got %s", tc.desc, tc.expect, res) } @@ -467,7 +504,10 @@ func TestCheckAppProtocolAnnotation(t *testing.T) { expect: report.Failed, }, } { - res, _ := CheckAppProtocolAnnotation(&tc.svc) + checker := &ServiceChecker{ + service: &tc.svc, + } + _, res, _ := CheckAppProtocolAnnotation(checker) if res != tc.expect { t.Errorf("For test case %q, expect check result = %s, but got %s", tc.desc, tc.expect, res) } @@ -521,7 +561,10 @@ func TestCheckL7ILBFrontendConfig(t *testing.T) { expect: report.Passed, }, } { - res, _ := CheckL7ILBFrontendConfig(&tc.ingress) + checker := &IngressChecker{ + ingress: &tc.ingress, + } + _, res, _ := CheckL7ILBFrontendConfig(checker) if res != tc.expect { t.Errorf("For test case %q, expect check result = %s, but got %s", tc.desc, tc.expect, res) } @@ -584,9 +627,116 @@ func TestCheckL7ILBNegAnnotation(t *testing.T) { expect: report.Passed, }, } { - res, _ := CheckL7ILBNegAnnotation(&tc.svc) + checker := &ServiceChecker{ + service: &tc.svc, + isL7ILB: true, + } + _, res, _ := CheckL7ILBNegAnnotation(checker) if res != tc.expect { t.Errorf("For test case %q, expect check result = %s, but got %s", tc.desc, tc.expect, res) } } } + +// TestCheckAllIngresses tests whether all the checks are triggered. +func TestCheckAllIngresses(t *testing.T) { + + client := fake.NewSimpleClientset() + beClient := fakebeconfig.NewSimpleClientset() + feClient := fakefeconfig.NewSimpleClientset() + + ingress1 := networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "test", + Annotations: map[string]string{ + annotations.IngressClassKey: annotations.GceL7ILBIngressClass, + annotations.FrontendConfigKey: "feConfig-1", + }, + }, + Spec: networkingv1.IngressSpec{ + DefaultBackend: &networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "svc-1", + }, + }, + }, + } + ingress2 := networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress2", + Namespace: "test", + Annotations: map[string]string{ + annotations.IngressClassKey: annotations.GceL7ILBIngressClass, + }, + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: "abc.xyz", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/*", + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "svc-1", + }, + }, + }, + }, + }, + }, + }, + { + Host: "abc.xyz", + }, + }, + }, + } + client.NetworkingV1().Ingresses("test").Create(context.TODO(), &ingress1, metav1.CreateOptions{}) + client.NetworkingV1().Ingresses("test").Create(context.TODO(), &ingress2, metav1.CreateOptions{}) + + serivce1 := corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + Annotations: map[string]string{ + annotations.BackendConfigKey: `{"default": "beconfig1"}`, + }, + }, + } + client.CoreV1().Services("test").Create(context.TODO(), &serivce1, metav1.CreateOptions{}) + + beClient.CloudV1().BackendConfigs("test").Create(context.TODO(), &beconfigv1.BackendConfig{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "beconfig1", + }, + }, metav1.CreateOptions{}) + report := CheckAllIngresses("test", client, beClient, feClient) + checkSet := make(map[string]struct{}) + for _, resource := range report.Resources { + for _, check := range resource.Checks { + checkSet[check.Name] = struct{}{} + } + } + + for _, check := range []string{ + ServiceExistenceCheck, + BackendConfigAnnotationCheck, + BackendConfigExistenceCheck, + HealthCheckTimeoutCheck, + IngressRuleCheck, + FrontendConfigExistenceCheck, + RuleHostOverwriteCheck, + AppProtocolAnnotationCheck, + L7ILBFrontendConfigCheck, + L7ILBNegAnnotationCheck, + } { + if _, ok := checkSet[check]; !ok { + t.Errorf("Missing check %s in CheckAllIngresses", check) + } + } +}