From 4af79f1b1c59047f4eacc4b8db30834fe947bda6 Mon Sep 17 00:00:00 2001 From: Shashank Ram Date: Thu, 1 Jul 2021 13:43:49 -0700 Subject: [PATCH] catalog/traffic-policies: add error codes Adds error codes for inbound and outbound traffic policy related errors. Part of #2866 Signed-off-by: Shashank Ram --- pkg/catalog/inbound_traffic_policies.go | 19 +++++-- pkg/catalog/outbound_traffic_policies.go | 41 +++++++++----- pkg/errcode/errcode.go | 71 ++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 20 deletions(-) diff --git a/pkg/catalog/inbound_traffic_policies.go b/pkg/catalog/inbound_traffic_policies.go index 7b8d1eb3d5..8f87e93ec0 100644 --- a/pkg/catalog/inbound_traffic_policies.go +++ b/pkg/catalog/inbound_traffic_policies.go @@ -6,6 +6,7 @@ import ( access "github.com/servicemeshinterface/smi-sdk-go/pkg/apis/access/v1alpha3" "github.com/openservicemesh/osm/pkg/constants" + "github.com/openservicemesh/osm/pkg/errcode" "github.com/openservicemesh/osm/pkg/identity" "github.com/openservicemesh/osm/pkg/service" "github.com/openservicemesh/osm/pkg/trafficpolicy" @@ -85,7 +86,8 @@ func (mc *MeshCatalog) listInboundPoliciesForTrafficSplits(upstreamIdentity iden // fetch all routes referenced in traffic target routeMatches, err := mc.routesFromRules(t.Spec.Rules, t.Namespace) if err != nil { - log.Error().Err(err).Msgf("Error finding route matches from TrafficTarget %s in namespace %s", t.Name, t.Namespace) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrFetchingSMIHTTPRouteGroupForTrafficTarget.String()). + Msgf("Error finding route matches from TrafficTarget %s in namespace %s", t.Name, t.Namespace) continue } @@ -104,7 +106,8 @@ func (mc *MeshCatalog) listInboundPoliciesForTrafficSplits(upstreamIdentity iden } hostnames, err := mc.GetServiceHostnames(apexService, locality) if err != nil { - log.Error().Err(err).Msgf("Error getting service hostnames for apex service %v", apexService) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrServiceHostnames.String()). + Msgf("Error getting service hostnames for apex service %v", apexService) continue } servicePolicy := trafficpolicy.NewInboundTrafficPolicy(apexService.FQDN(), hostnames) @@ -137,13 +140,15 @@ func (mc *MeshCatalog) buildInboundPolicies(t *access.TrafficTarget, svc service // fetch all routes referenced in traffic target routeMatches, err := mc.routesFromRules(t.Spec.Rules, t.Namespace) if err != nil { - log.Error().Err(err).Msgf("Error finding route matches from TrafficTarget %s in namespace %s", t.Name, t.Namespace) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrFetchingSMIHTTPRouteGroupForTrafficTarget.String()). + Msgf("Error finding route matches from TrafficTarget %s in namespace %s", t.Name, t.Namespace) return inboundPolicies } hostnames, err := mc.GetServiceHostnames(svc, service.LocalNS) if err != nil { - log.Error().Err(err).Msgf("Error getting service hostnames for service %s", svc) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrServiceHostnames.String()). + Msgf("Error getting service hostnames for service %s", svc) return inboundPolicies } @@ -175,7 +180,8 @@ func (mc *MeshCatalog) buildInboundPermissiveModePolicies(svc service.MeshServic hostnames, err := mc.GetServiceHostnames(svc, service.LocalNS) if err != nil { - log.Error().Err(err).Msgf("Error getting service hostnames for service %s", svc) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrServiceHostnames.String()). + Msgf("Error getting service hostnames for service %s", svc) return inboundPolicies } @@ -225,7 +231,8 @@ func (mc *MeshCatalog) getHTTPPathsPerRoute() (map[trafficpolicy.TrafficSpecName for _, trafficSpecs := range mc.meshSpec.ListHTTPTrafficSpecs() { log.Debug().Msgf("Discovered TrafficSpec resource: %s/%s", trafficSpecs.Namespace, trafficSpecs.Name) if trafficSpecs.Spec.Matches == nil { - log.Error().Msgf("TrafficSpec %s/%s has no matches in route; Skipping...", trafficSpecs.Namespace, trafficSpecs.Name) + log.Error().Str(errcode.Kind, errcode.ErrSMIHTTPRouteGroupNoMatch.String()). + Msgf("TrafficSpec %s/%s has no matches in route; Skipping...", trafficSpecs.Namespace, trafficSpecs.Name) continue } diff --git a/pkg/catalog/outbound_traffic_policies.go b/pkg/catalog/outbound_traffic_policies.go index d2bded1ea2..8644cd13c3 100644 --- a/pkg/catalog/outbound_traffic_policies.go +++ b/pkg/catalog/outbound_traffic_policies.go @@ -6,6 +6,7 @@ import ( access "github.com/servicemeshinterface/smi-sdk-go/pkg/apis/access/v1alpha3" "github.com/openservicemesh/osm/pkg/constants" + "github.com/openservicemesh/osm/pkg/errcode" "github.com/openservicemesh/osm/pkg/identity" "github.com/openservicemesh/osm/pkg/k8s" "github.com/openservicemesh/osm/pkg/service" @@ -74,7 +75,8 @@ func (mc *MeshCatalog) listOutboundTrafficPoliciesForTrafficSplits(sourceNamespa } hostnames, err := mc.GetServiceHostnames(svc, locality) if err != nil { - log.Error().Err(err).Msgf("Error getting service hostnames for apex service %v", svc) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrServiceHostnames.String()). + Msgf("Error getting service hostnames for apex service %v", svc) continue } policy := trafficpolicy.NewOutboundTrafficPolicy(svc.FQDN(), hostnames) @@ -97,7 +99,9 @@ func (mc *MeshCatalog) listOutboundTrafficPoliciesForTrafficSplits(sourceNamespa policy.Routes = []*trafficpolicy.RouteWeightedClusters{rwc} if apexServices.Contains(svc) { - log.Error().Msgf("Skipping Traffic Split policy %s in namespaces %s as there is already a traffic split policy for apex service %v", split.Name, split.Namespace, svc) + // TODO: enhancement(#2759) + log.Error().Str(errcode.Kind, errcode.ErrMultipleSMISplitPerServiceUnsupported.String()). + Msgf("Skipping Traffic Split policy %s in namespaces %s as there is already a traffic split policy for apex service %v", split.Name, split.Namespace, svc) } else { outboundPoliciesFromSplits = append(outboundPoliciesFromSplits, policy) apexServices.Add(svc) @@ -133,7 +137,8 @@ func (mc *MeshCatalog) ListAllowedOutboundServicesForIdentity(serviceIdentity id Namespace: t.Spec.Destination.Namespace, }) if err != nil { - log.Error().Err(err).Msgf("No Services found matching Service Account %s in Namespace %s", t.Spec.Destination.Name, t.Namespace) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrNoMatchingServiceForServiceAccount.String()). + Msgf("No Services found matching Service Account %s in Namespace %s", t.Spec.Destination.Name, t.Namespace) break } for _, destService := range destServices { @@ -167,14 +172,16 @@ func (mc *MeshCatalog) buildOutboundPermissiveModePolicies(sourceNamespace strin } hostnames, err := mc.GetServiceHostnames(destService, locality) if err != nil { - log.Error().Err(err).Msgf("Error getting service hostnames for service %s", destService) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrServiceHostnames.String()). + Msgf("Error getting service hostnames for service %s", destService) continue } weightedCluster := getDefaultWeightedClusterForService(destService) policy := trafficpolicy.NewOutboundTrafficPolicy(destService.FQDN(), hostnames) if err := policy.AddRoute(trafficpolicy.WildCardRouteMatch, weightedCluster); err != nil { - log.Error().Err(err).Msgf("Error adding route to outbound policy in permissive mode for destination %s(%s)", destService.Name, destService.Namespace) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrAddingRouteToOutboundTrafficPolicy.String()). + Msgf("Error adding route to outbound policy in permissive mode for destination %s", destService) continue } outPolicies = append(outPolicies, policy) @@ -190,15 +197,17 @@ func (mc *MeshCatalog) buildOutboundPolicies(sourceServiceIdentity identity.Serv // fetch services running workloads with destination service account destServices, err := mc.getDestinationServicesFromTrafficTarget(t) if err != nil { - log.Error().Err(err).Msgf("Error resolving destination from traffic target %s (%s)", t.Name, t.Namespace) - return outboundPolicies + log.Error().Err(err).Str(errcode.Kind, errcode.ErrFetchingServiceForTrafficTargetDestination.String()). + Msgf("Error resolving destination services from TraficTarget %s/%s", t.Namespace, t.Name) + return nil } - // fetch all routes referenced in traffic target + // fetch all routes referenced in the TrafficTarget routeMatches, err := mc.routesFromRules(t.Spec.Rules, t.Namespace) if err != nil { - log.Error().Err(err).Msgf("Error finding route matches from TrafficTarget %s in namespace %s", t.Name, t.Namespace) - return outboundPolicies + log.Error().Err(err).Str(errcode.Kind, errcode.ErrFetchingSMIHTTPRouteGroupForTrafficTarget.String()). + Msgf("Error finding route matches from TrafficTarget %s/%s", t.Namespace, t.Name) + return nil } // build an outbound traffic policy for each destination service @@ -212,7 +221,8 @@ func (mc *MeshCatalog) buildOutboundPolicies(sourceServiceIdentity identity.Serv } hostnames, err := mc.GetServiceHostnames(destService, locality) if err != nil { - log.Error().Err(err).Msgf("Error getting service hostnames for service %s", destService) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrServiceHostnames.String()). + Msgf("Error getting service hostnames for service %s", destService) continue } weightedCluster := getDefaultWeightedClusterForService(destService) @@ -226,7 +236,8 @@ func (mc *MeshCatalog) buildOutboundPolicies(sourceServiceIdentity identity.Serv if _, ok := routeMatch.Headers[hostHeaderKey]; ok { policyWithHostHeader := trafficpolicy.NewOutboundTrafficPolicy(routeMatch.Headers[hostHeaderKey], []string{routeMatch.Headers[hostHeaderKey]}) if err := policyWithHostHeader.AddRoute(trafficpolicy.WildCardRouteMatch, weightedCluster); err != nil { - log.Error().Err(err).Msgf("Error adding Route to outbound policy for source %s(%s) and destination %s (%s) with host header %s", source.Name, source.Namespace, destService.Name, destService.Namespace, routeMatch.Headers[hostHeaderKey]) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrAddingRouteToOutboundTrafficPolicy.String()). + Msgf("Error adding Route to outbound policy for source %s/%s and destination %s/%s with host header %s", source.Namespace, source.Name, destService.Namespace, destService.Name, routeMatch.Headers[hostHeaderKey]) continue } outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, outboundPolicies, policyWithHostHeader) @@ -236,7 +247,8 @@ func (mc *MeshCatalog) buildOutboundPolicies(sourceServiceIdentity identity.Serv } if needWildCardRoute { if err := policy.AddRoute(trafficpolicy.WildCardRouteMatch, weightedCluster); err != nil { - log.Error().Err(err).Msgf("Error adding Route to outbound policy for source %s(%s) and destination %s (%s)", source.Name, source.Namespace, destService.Name, destService.Namespace) + log.Error().Err(err).Str(errcode.Kind, errcode.ErrAddingRouteToOutboundTrafficPolicy.String()). + Msgf("Error adding Route to outbound policy for source %s/%s and destination %s/%s", source.Namespace, source.Name, destService.Namespace, destService.Name) continue } } @@ -277,7 +289,8 @@ func (mc *MeshCatalog) GetWeightedClustersForUpstream(upstream service.MeshServi } if apexServices.Contains(split.Spec.Service) { - log.Error().Msgf("Skipping traffic split policy %s/%s as there is already a corresponding policy for apex service %s", split.Namespace, split.Name, split.Spec.Service) + log.Error().Str(errcode.Kind, errcode.ErrMultipleSMISplitPerServiceUnsupported.String()). + Msgf("Skipping traffic split policy %s/%s as there is already a corresponding policy for apex service %s", split.Namespace, split.Name, split.Spec.Service) continue } diff --git a/pkg/errcode/errcode.go b/pkg/errcode/errcode.go index 39491a393d..3895c0dab9 100644 --- a/pkg/errcode/errcode.go +++ b/pkg/errcode/errcode.go @@ -50,6 +50,31 @@ const ( // ErrEgressSMIHTTPRouteGroupNotFound indicates the SMI HTTPRouteGroup specified in the egress policy was not found ErrEgressSMIHTTPRouteGroupNotFound + + // ErrFetchingSMIHTTPRouteGroupForTrafficTarget indicates the SMI HTTPRouteGroup specified as a match in an SMI + // TrafficTarget resource was not able to be retrieved + ErrFetchingSMIHTTPRouteGroupForTrafficTarget + + // ErrSMIHTTPRouteGroupNoMatch indicates the SMI HTTPRouteGroup resource has no matches specified + ErrSMIHTTPRouteGroupNoMatch + + // ErrMultipleSMISplitPerServiceUnsupported indicates multiple SMI split policies per service exists and is not supported + ErrMultipleSMISplitPerServiceUnsupported + + // ErrAddingRouteToOutboundTrafficPolicy indicates there was an error adding a route to an outbound traffic policy + ErrAddingRouteToOutboundTrafficPolicy + + // ErrFetchingServiceForTrafficTargetDestination indicates an error retrieving services associated with a TrafficTarget destination + ErrFetchingServiceForTrafficTargetDestination +) + +// Range 3000-3500 is reserved for errors related to k8s constructs (service accounts, namespaces, etc.) +const ( + // ErrServiceHostnames indicates the hostnames associated with a service could not be computed + ErrServiceHostnames errCode = iota + 3000 + + // ErrNoMatchingServiceForServiceAccount indicates there are no services associated with the service account + ErrNoMatchingServiceForServiceAccount ) // String returns the error code as a string, ex. E1000 @@ -123,5 +148,51 @@ The specified match was ignored by the system while applying the egress policy. The SMI HTTPRouteGroup resource specified as a match in an egress policy was not found. Please verify that the specified SMI HTTPRouteGroup resource exists in the same namespace as the egress policy referencing it as a match. +`, + + ErrFetchingSMIHTTPRouteGroupForTrafficTarget: ` +The SMI HTTPRouteGroup resources specified as a match in an SMI TrafficTarget policy was +unable to be retrieved by the system. +The associated SMI TrafficTarget policy was ignored by the system. Please verify that the +matches specified for the Traffictarget resource exist in the same namespace as the +TrafficTarget policy referencing the match. +`, + + ErrSMIHTTPRouteGroupNoMatch: ` +The SMI HTTPRouteGroup resource is invalid as it does not have any matches specified. +The SMI HTTPRouteGroup policy was ignored by the system. +`, + + ErrMultipleSMISplitPerServiceUnsupported: ` +There are multiple SMI traffic split policies associated with the same apex(root) +service specified in the policies. The system does not support this scenario so +onlt the first encountered policy is processed by the system, subsequent policies +referring the same apex service are ignored. +`, + + ErrAddingRouteToOutboundTrafficPolicy: ` +There was an error adding a route match to an outbound traffic policy representation +within the system. +The associated route was ignored by the system. +`, + + ErrFetchingServiceForTrafficTargetDestination: ` +The system was unable to lookup the services associated with the destination specified +in the SMI TrafficTarget policy. +The associated SMI TrafficTarget policy was ignored by the system. +`, + + // + // Range 3000-3500 + // + ErrServiceHostnames: ` +The hostnames (FQDN) used to access the k8s service could not be retrieved by the system. +Any HTTP traffic policy associated with this service was ignored by the system. +`, + + ErrNoMatchingServiceForServiceAccount: ` +The system expected k8s services to be associated with a service account, but no such +service was found. A service matches a service account if the pods backing the service +belong to the service account. `, }