From 66a28534eb2eacc4a7bf3a6c818821f24a4b44a7 Mon Sep 17 00:00:00 2001 From: Alice Wasko Date: Wed, 1 May 2024 17:46:09 -0700 Subject: [PATCH] give grouped mappings their own settings Signed-off-by: Alice Wasko --- python/ambassador/diagnostics/diagnostics.py | 51 +-- python/ambassador/envoy/v3/v3listener.py | 31 +- python/ambassador/envoy/v3/v3route.py | 193 +++++---- python/ambassador/ir/ir.py | 136 +++++-- python/ambassador/ir/irambassador.py | 36 +- python/ambassador/ir/irbasemappinggroup.py | 23 +- python/ambassador/ir/ircors.py | 4 +- python/ambassador/ir/irhost.py | 6 +- python/ambassador/ir/irhttpmapping.py | 7 +- python/ambassador/ir/irhttpmappinggroup.py | 377 +++++++----------- python/ambassador/ir/irlistener.py | 5 +- python/ambassador/ir/irmappingfactory.py | 27 +- python/ambassador/ir/irtcpmapping.py | 7 +- python/ambassador/ir/irtcpmappinggroup.py | 62 +-- python/ambassador/ir/irutils.py | 4 + python/ambassador_diag/diagd.py | 11 +- python/tests/unit/test_cache.py | 2 +- python/tests/unit/test_irmapping.py | 4 +- python/tests/unit/test_mapping.py | 18 +- .../tests/unit/test_mapping_canary_group.py | 2 +- 20 files changed, 433 insertions(+), 573 deletions(-) diff --git a/python/ambassador/diagnostics/diagnostics.py b/python/ambassador/diagnostics/diagnostics.py index de1f4f8924e..4dc1d71ff8f 100644 --- a/python/ambassador/diagnostics/diagnostics.py +++ b/python/ambassador/diagnostics/diagnostics.py @@ -414,40 +414,6 @@ def __init__(self, ir: IR, econf: EnvoyConfig) -> None: f'A future Ambassador version will change the GRPC protocol version for {" and ".join(things_to_warn)}. See the CHANGELOG for details.' ) - # # Warn people about the default port change. - # if self.ir.ambassador_module.service_port < 1024: - # # Does it look like they explicitly asked for this? - # amod = self.ir.aconf.get_module('ambassador') - # - # if not (amod and amod.get('service_port')): - # # They did not explictly set the port. Warn them about the - # # port change. - # new_defaults = [ "port 8080 for HTTP" ] - # - # if self.ir.tls_contexts: - # new_defaults.append("port 8443 for HTTPS") - # - # default_ports = " and ".join(new_defaults) - # - # listen_ports = [ str(l.service_port) for l in self.ir.listeners ] - # self.ir.logger.info("listen_ports %s" % listen_ports) - # - # port_or_ports = "port" if (len(listen_ports) == 1) else "ports" - # - # last_port = listen_ports.pop() - # - # els = [ last_port ] - # - # if len(listen_ports) > 0: - # els.insert(0, ", ".join(listen_ports)) - # - # port_nums = " and ".join(els) - # - # m1 = f'Ambassador 0.60 will default to listening on {default_ports}.' - # m2 = f'You will need to change your configuration to continue using {port_or_ports} {port_nums}.' - # - # self.ir.aconf.post_notice(f'{m1} {m2}') - # Copy in the toplevel 'error' and 'notice' sets. self.errors = self.ir.aconf.errors self.notices = self.ir.aconf.notices @@ -499,7 +465,7 @@ def __init__(self, ir: IR, econf: EnvoyConfig) -> None: # Always generate the full group set so that we can look up groups. self.groups = { "grp-%s" % group.group_id: group - for group in self.ir.groups.values() + for group in self.ir.get_base_mapping_groups() if group.location != "--diagnostics--" } @@ -608,12 +574,14 @@ def as_dict(self) -> dict: "envoy_elements": self.envoy_elements, "errors": self.errors, "notices": self.notices, - "groups": {key: self.flattened(value) for key, value in self.groups.items()}, + "groups": { + key: self.flatten_mapping_group(value) for key, value in self.groups.items() + }, # 'clusters': { key: value.as_dict() for key, value in self.clusters.items() }, "tlscontexts": [x.as_dict() for x in self.ir.tls_contexts.values()], } - def flattened(self, group: IRBaseMappingGroup) -> dict: + def flatten_mapping_group(self, group: IRBaseMappingGroup) -> dict: flattened = {k: v for k, v in group.as_dict().items() if k != "mappings"} flattened_mappings = [] @@ -632,12 +600,10 @@ def flattened(self, group: IRBaseMappingGroup) -> dict: fm["prefix"] = m.get("prefix") rewrite = m.get("rewrite", None) - if rewrite: fm["rewrite"] = rewrite host = m.get("host", None) - if host: fm["host"] = host @@ -696,10 +662,9 @@ def overview(self, request, estat: EnvoyStats) -> Dict[str, Any]: result = DiagResult(self, estat, request) - for group in self.ir.ordered_groups(): - # TCPMappings are currently handled elsewhere. - if isinstance(group, IRHTTPMappingGroup): - result.include_httpgroup(group) + # TCPMappings are currently handled elsewhere. + for mapping_group in self.ir.ordered_http_mapping_groups(): + result.include_httpgroup(mapping_group) return result.as_dict() diff --git a/python/ambassador/envoy/v3/v3listener.py b/python/ambassador/envoy/v3/v3listener.py index b2eda1f279e..35c53d6f011 100644 --- a/python/ambassador/envoy/v3/v3listener.py +++ b/python/ambassador/envoy/v3/v3listener.py @@ -169,9 +169,9 @@ def __str__(self) -> str: def tlscontext_for_tcpmapping( - irgroup: IRTCPMappingGroup, config: "V3Config" + tcp_mapping_group: IRTCPMappingGroup, config: "V3Config" ) -> Optional["IRTLSContext"]: - group_host = irgroup.get("host") + group_host = tcp_mapping_group.get("host") if not group_host: return None @@ -685,14 +685,14 @@ def finalize_tcp(self) -> None: if self._log_debug: self.config.ir.logger.debug(f" build chain[{repr(chain_key)}]={chain}") - for irgroup in chain.hosts.values(): - if not isinstance(irgroup, IRTCPMappingGroup): + for tcp_mapping_group in chain.hosts.values(): + if not isinstance(tcp_mapping_group, IRTCPMappingGroup): continue # First up, which clusters do we need to talk to? clusters = [ {"name": mapping.cluster.envoy_name, "weight": mapping._weight} - for mapping in irgroup.mappings + for mapping in tcp_mapping_group.mappings ] # From that, we can sort out a basic tcp_proxy filter config. @@ -707,7 +707,7 @@ def finalize_tcp(self) -> None: # OK. Basic filter chain entry next. filter_chain: Dict[str, Any] = { - "name": f"tcphost-{irgroup.name}", + "name": f"tcphost-{tcp_mapping_group.name}", "filters": [tcp_filter], } @@ -743,14 +743,11 @@ def finalize_tcp(self) -> None: def compute_tcpchains(self) -> None: self.config.ir.logger.debug(" compute_tcpchains") - for irgroup in self.config.ir.ordered_groups(): - if not isinstance(irgroup, IRTCPMappingGroup): - continue - + for tcp_mapping_group in self.config.ir.ordered_tcp_mapping_groups(): if self._log_debug: - self.config.ir.logger.debug(f" consider {irgroup}") + self.config.ir.logger.debug(f" consider {tcp_mapping_group}") - if irgroup.bind_to() != self.bind_to: + if tcp_mapping_group.bind_to() != self.bind_to: self.config.ir.logger.debug(" reject") continue @@ -760,21 +757,21 @@ def compute_tcpchains(self) -> None: # than for a 'Host'. Same deal applies with TLS: you can't do host-based matching # without it. - group_host = irgroup.get("host", None) + group_host = tcp_mapping_group.get("host", None) if not group_host: # cleartext # Special case. No host (aka hostname) in a TCPMapping means an unconditional forward, # so just add this immediately as a "*" chain. - self.add_chain("tcp", None, "*", "*").add_tcphost(irgroup) + self.add_chain("tcp", None, "*", "*").add_tcphost(tcp_mapping_group) else: # TLS/SNI - context = tlscontext_for_tcpmapping(irgroup, self.config) + context = tlscontext_for_tcpmapping(tcp_mapping_group, self.config) if not context: - irgroup.post_error("No matching TLSContext found, disabling!") + tcp_mapping_group.post_error("No matching TLSContext found, disabling!") continue # group_host comes from `TCPMapping.host` which is expected to be a valid dns hostname # without a port so no need to parse out a port sni = group_host - self.add_chain("tcp", context, group_host, sni).add_tcphost(irgroup) + self.add_chain("tcp", context, group_host, sni).add_tcphost(tcp_mapping_group) def compute_httpchains(self) -> None: # Compute the set of chains we need, HTTP version. The core here is matching diff --git a/python/ambassador/envoy/v3/v3route.py b/python/ambassador/envoy/v3/v3route.py index 55dfb4edbe0..8a398bd8b94 100644 --- a/python/ambassador/envoy/v3/v3route.py +++ b/python/ambassador/envoy/v3/v3route.py @@ -12,13 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License +import re from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union from typing import cast as typecast from ...cache import Cacheable -from ...ir.irbasemapping import IRBaseMapping +from ...ir.irhttpmapping import IRHTTPMapping from ...ir.irhttpmappinggroup import IRHTTPMappingGroup -from ...ir.irutils import hostglob_matches +from ...ir.irutils import are_mapping_group_fixes_disabled, hostglob_matches from ..common import EnvoyRoute from .v3ratelimitaction import V3RateLimitAction @@ -262,8 +263,10 @@ def action_redirect(self, variant) -> None: class V3Route(Cacheable): + _group: IRHTTPMappingGroup + def __init__( - self, config: "V3Config", group: IRHTTPMappingGroup, mapping: IRBaseMapping + self, config: "V3Config", group: IRHTTPMappingGroup, mapping: IRHTTPMapping ) -> None: super().__init__() @@ -271,6 +274,9 @@ def __init__( self.logger = group.logger self._group = group + # To start, get all the route matching settings from the group since every Mapping within it shares the same + # matching settings + # Passing a list to set is _very important_ here, lest you get a set of # the individual characters in group.host! self["_host_constraints"] = set([group.get("host") or "*"]) @@ -280,6 +286,10 @@ def __init__( envoy_route = EnvoyRoute(group).envoy_route + seed_mapping = mapping + if are_mapping_group_fixes_disabled(): + seed_mapping = group.seed_mapping + mapping_prefix = mapping.get("prefix", None) route_prefix = mapping_prefix if mapping_prefix is not None else group.get("prefix") @@ -294,8 +304,15 @@ def __init__( "default_value": {"numerator": mapping.get("_weight", 100), "denominator": "HUNDRED"} } + match = {"case_sensitive": case_sensitive, "runtime_fraction": runtime_fraction} + if len(mapping) > 0: - if not "cluster" in mapping: + if "cluster" in mapping: + runtime_fraction[ + "runtime_key" + ] = f"routing.traffic_shift.{mapping.cluster.envoy_name}" + # If it's a redirect mapping we don't have a cluster, otherwise this is an error + elif mapping.get("host_redirect", None) == None: config.ir.logger.error( "%s: Mapping %s has no cluster? %s", mapping.rkey, @@ -303,10 +320,6 @@ def __init__( mapping.as_json(), ) self["_failed"] = True - else: - runtime_fraction[ - "runtime_key" - ] = f"routing.traffic_shift.{mapping.cluster.envoy_name}" match = {"case_sensitive": case_sensitive, "runtime_fraction": runtime_fraction} @@ -331,16 +344,19 @@ def __init__( else: match.update(regex_matcher(config, route_prefix)) - headers = self.generate_headers(config, group) - if len(headers) > 0: - match["headers"] = headers + headers_match = self.generate_headers_match(config, group) + if len(headers_match) > 0: + match["headers"] = headers_match - query_parameters = self.generate_query_parameters(config, group) - if len(query_parameters) > 0: - match["query_parameters"] = query_parameters + query_params_match = self.generate_query_params_match(config, group) + if len(query_params_match) > 0: + match["query_parameters"] = query_params_match self["match"] = match + # Ok, now we move on to getting all of the traffic transformations and routing settings that can be different for + # each Mapping within the group + # `typed_per_filter_config` is used to pass typed configuration to Envoy filters typed_per_filter_config = {} @@ -378,7 +394,7 @@ def __init__( "response_map": {"mappers": filter_config["mappers"]}, } - if mapping.get("bypass_auth", False) or (group.get("host_redirect", None) != None): + if mapping.get("bypass_auth", False) or (mapping.get("host_redirect", None) != None): typed_per_filter_config["envoy.filters.http.ext_authz"] = { "@type": "type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthzPerRoute", "disabled": True, @@ -395,36 +411,36 @@ def __init__( if len(typed_per_filter_config) > 0: self["typed_per_filter_config"] = typed_per_filter_config - request_headers_to_add = group.get("add_request_headers", None) + request_headers_to_add = seed_mapping.get("add_request_headers", None) if request_headers_to_add: self["request_headers_to_add"] = self.generate_headers_to_add(request_headers_to_add) - response_headers_to_add = group.get("add_response_headers", None) + response_headers_to_add = seed_mapping.get("add_response_headers", None) if response_headers_to_add: self["response_headers_to_add"] = self.generate_headers_to_add(response_headers_to_add) - request_headers_to_remove = group.get("remove_request_headers", None) + request_headers_to_remove = seed_mapping.get("remove_request_headers", None) if request_headers_to_remove: if type(request_headers_to_remove) != list: request_headers_to_remove = [request_headers_to_remove] self["request_headers_to_remove"] = request_headers_to_remove - response_headers_to_remove = group.get("remove_response_headers", None) + response_headers_to_remove = seed_mapping.get("remove_response_headers", None) if response_headers_to_remove: if type(response_headers_to_remove) != list: response_headers_to_remove = [response_headers_to_remove] self["response_headers_to_remove"] = response_headers_to_remove - host_redirect = group.get("host_redirect", None) + host_redirect = seed_mapping.get("host_redirect", None) if host_redirect: # We have a host_redirect. Deal with it. - self["redirect"] = {"host_redirect": host_redirect.service} + self["redirect"] = {"host_redirect": mapping.service} - path_redirect = host_redirect.get("path_redirect", None) - prefix_redirect = host_redirect.get("prefix_redirect", None) - regex_redirect = host_redirect.get("regex_redirect", None) - response_code = host_redirect.get("redirect_response_code", None) + path_redirect = mapping.get("path_redirect", None) + prefix_redirect = mapping.get("prefix_redirect", None) + regex_redirect = mapping.get("regex_redirect", None) + response_code = mapping.get("redirect_response_code", None) # We enforce that only one of path_redirect or prefix_redirect is set in the IR. # But here, we just prefer path_redirect if that's set. @@ -477,7 +493,7 @@ def __init__( if idle_timeout_ms is not None: route["idle_timeout"] = "%0.3fs" % (idle_timeout_ms / 1000.0) - regex_rewrite = self.generate_regex_rewrite(config, group) + regex_rewrite = self.generate_regex_rewrite(config, mapping) if len(regex_rewrite) > 0: route["regex_rewrite"] = regex_rewrite elif mapping.get("rewrite", None): @@ -489,50 +505,51 @@ def __init__( if "auto_host_rewrite" in mapping: route["auto_host_rewrite"] = mapping["auto_host_rewrite"] - hash_policy = self.generate_hash_policy(group) + hash_policy = self.generate_hash_policy(seed_mapping) if len(hash_policy) > 0: route["hash_policy"] = [hash_policy] cors = None - if "cors" in group: - cors = group.cors + if "cors" in seed_mapping: + cors = seed_mapping.cors elif "cors" in config.ir.ambassador_module: cors = config.ir.ambassador_module.cors if cors: - # Duplicate this IRCORS, then set its group ID correctly. cors = cors.dup() - cors.set_id(group.group_id) + cors.set_id(mapping.cache_key) route["cors"] = cors.as_dict() retry_policy = None - if "retry_policy" in group: - retry_policy = group.retry_policy.as_dict() + if "retry_policy" in seed_mapping: + retry_policy = seed_mapping.retry_policy.as_dict() elif "retry_policy" in config.ir.ambassador_module: retry_policy = config.ir.ambassador_module.retry_policy.as_dict() if retry_policy: route["retry_policy"] = retry_policy - # Is shadowing enabled? - shadow = group.get("shadows", None) - - if shadow: - shadow = shadow[0] - - weight = shadow.get("weight", 100) - - route["request_mirror_policies"] = [ - { - "cluster": shadow.cluster.envoy_name, - "runtime_fraction": { - "default_value": {"numerator": weight, "denominator": "HUNDRED"} - }, - } - ] + # Are we doing any traffic shadowing? + # Shadow mappings get added to all generated routes for a mapping group since it is an addition to the routing action, and not a replacement for the + # routing action like a redirect would be. This is why we keep them separate from the regular Mappings. + if len(group.shadow_mappings) > 0: + mirror_policies = [] + for shadow_mapping in group.shadow_mappings: + mirror_policies.append( + { + "cluster": shadow_mapping.cluster.envoy_name, + "runtime_fraction": { + "default_value": { + "numerator": shadow_mapping.get("weight", 100), + "denominator": "HUNDRED", + } + }, + } + ) + route["request_mirror_policies"] = mirror_policies # Is RateLimit a thing? rlsvc = config.ir.ratelimit @@ -542,13 +559,13 @@ def __init__( # labels have already been handled, as has translating from v0 'rate_limits' to # v1 'labels'). - if "labels" in group: + if "labels" in mapping: # The Envoy RateLimit filter only supports one domain, so grab the configured domain # from the RateLimitService and use that to look up the labels we should use. rate_limits = [] - for rl in group.labels.get(rlsvc.domain, []): + for rl in mapping.labels.get(rlsvc.domain, []): action = V3RateLimitAction(config, rl) if action.valid: @@ -558,9 +575,9 @@ def __init__( route["rate_limits"] = rate_limits # Save upgrade configs. - if group.get("allow_upgrade"): + if seed_mapping.get("allow_upgrade"): route["upgrade_configs"] = [ - {"upgrade_type": proto} for proto in group.get("allow_upgrade", []) + {"upgrade_type": proto} for proto in seed_mapping.get("allow_upgrade", []) ] self["route"] = route @@ -593,7 +610,11 @@ def matches_domains(self, domains: List[str]) -> bool: @classmethod def get_route( - cls, config: "V3Config", cache_key: str, irgroup: IRHTTPMappingGroup, mapping: IRBaseMapping + cls, + config: "V3Config", + cache_key: str, + mapping_group: IRHTTPMappingGroup, + mapping: IRHTTPMapping, ) -> "V3Route": route: "V3Route" @@ -601,17 +622,13 @@ def get_route( if cached_route is None: # Cache miss. - # config.ir.logger.info(f"V3Route: cache miss for {cache_key}, synthesizing route") - - route = V3Route(config, irgroup, mapping) + route = V3Route(config, mapping_group, mapping) # Cheat a bit and force the route's cache_key. route.cache_key = cache_key - - # config.ir.logger.info("V3Route: synthesized %s" % v3prettyroute(route)) - config.cache.add(route) - config.cache.link(irgroup, route) + # config.cache.link(mapping, route) + config.cache.link(mapping_group, route) config.cache.dump("V3Route synth %s: %s", cache_key, v3prettyroute(route)) else: # Cache hit. We know a priori that it's a V3Route, but let's assert that @@ -619,8 +636,6 @@ def get_route( assert isinstance(cached_route, V3Route) route = cached_route - # config.ir.logger.info(f"V3Route: cache hit for {cache_key}") - # One way or another, we have a route now. return route @@ -628,47 +643,25 @@ def get_route( def generate(cls, config: "V3Config") -> None: config.routes = [] - for irgroup in config.ir.ordered_groups(): - if not isinstance(irgroup, IRHTTPMappingGroup): - # We only want HTTP mapping groups here. - continue - - if irgroup.get("host_redirect") is not None and len(irgroup.get("mappings", [])) == 0: - # This is a host-redirect-only group, which is weird, but can happen. Do we - # have a cached route for it? - key = f"Route-{irgroup.group_id}-hostredirect" - - # Casting an empty dict to an IRBaseMapping may look weird, but in fact IRBaseMapping - # is (ultimately) a subclass of dict, so it's the cleanest way to pass in a completely - # empty IRBaseMapping to V3Route(). - # - # (We could also have written V3Route to allow the mapping to be Optional, but that - # makes a lot of its constructor much uglier.) - route = config.save_element( - "route", - irgroup, - cls.get_route(config, key, irgroup, typecast(IRBaseMapping, {})), - ) - config.routes.append(route) - - # Repeat for our real mappings. - for mapping in irgroup.mappings: - key = f"Route-{irgroup.group_id}-{mapping.cache_key}" - - route = cls.get_route(config, key, irgroup, mapping) - + # Build a route for each Mapping we have + for mapping_group in config.ir.ordered_http_mapping_groups(): + for mapping in mapping_group.mappings: + assert isinstance(mapping, IRHTTPMapping) + key = f"Route-{mapping_group.group_id}-{mapping.cache_key}" + route = cls.get_route(config, key, mapping_group, mapping) if not route.get("_failed", False): - config.routes.append(config.save_element("route", irgroup, route)) + config.routes.append(config.save_element("route", mapping_group, route)) + else: + config.ir.post_error(f'Route with key "{key}" failed to build') # Once that's done, go build the variants on each route. config.route_variants = [] - for route in config.routes: # Set up a currently-empty set of variants for this route. config.route_variants.append(V3RouteVariants(route)) @staticmethod - def generate_headers(config: "V3Config", mapping_group: IRHTTPMappingGroup) -> List[dict]: + def generate_headers_match(config: "V3Config", mapping_group: IRHTTPMappingGroup) -> List[dict]: headers = [] group_headers = mapping_group.get("headers", []) @@ -708,7 +701,7 @@ def generate_headers(config: "V3Config", mapping_group: IRHTTPMappingGroup) -> L return headers @staticmethod - def generate_query_parameters( + def generate_query_params_match( config: "V3Config", mapping_group: IRHTTPMappingGroup ) -> List[dict]: query_parameters = [] @@ -740,9 +733,9 @@ def generate_query_parameters( return query_parameters @staticmethod - def generate_hash_policy(mapping_group: IRHTTPMappingGroup) -> dict: + def generate_hash_policy(mapping: IRHTTPMapping) -> dict: hash_policy = {} - load_balancer = mapping_group.get("load_balancer", None) + load_balancer = mapping.get("load_balancer", None) if load_balancer is not None: lb_policy = load_balancer.get("policy") if lb_policy in ["ring_hash", "maglev"]: @@ -782,9 +775,9 @@ def generate_headers_to_add(header_dict: dict) -> List[dict]: return headers @staticmethod - def generate_regex_rewrite(config: "V3Config", mapping_group: IRHTTPMappingGroup) -> dict: + def generate_regex_rewrite(config: "V3Config", mapping: IRHTTPMapping) -> dict: regex_rewrite = {} - group_regex_rewrite = mapping_group.get("regex_rewrite", None) + group_regex_rewrite = mapping.get("regex_rewrite", None) if group_regex_rewrite is not None: pattern = group_regex_rewrite.get("pattern", None) if pattern is not None: diff --git a/python/ambassador/ir/ir.py b/python/ambassador/ir/ir.py index 5676e56be99..7767ba7c846 100644 --- a/python/ambassador/ir/ir.py +++ b/python/ambassador/ir/ir.py @@ -34,12 +34,15 @@ from .irgofilter import IRGOFilter from .irhost import HostFactory, IRHost from .irhttpmapping import IRHTTPMapping +from .irhttpmappinggroup import IRHTTPMappingGroup from .irlistener import IRListener, ListenerFactory from .irlogservice import IRLogService, IRLogServiceFactory from .irmappingfactory import MappingFactory from .irratelimit import IRRateLimit from .irresource import IRResource from .irserviceresolver import IRServiceResolver, IRServiceResolverFactory, SvcEndpointSet +from .irtcpmapping import IRTCPMapping +from .irtcpmappinggroup import IRTCPMappingGroup from .irtls import IRAmbassadorTLS, TLSModuleFactory from .irtlscontext import IRTLSContext, TLSContextFactory from .irtracing import IRTracing @@ -83,7 +86,8 @@ class IR: edge_stack_allowed: bool file_checker: IRFileChecker filters: List[IRFilter] - groups: Dict[str, IRBaseMappingGroup] + http_mapping_groups: Dict[str, IRHTTPMappingGroup] + tcp_mapping_groups: Dict[str, IRTCPMappingGroup] grpc_services: Dict[str, IRCluster] hosts: Dict[str, IRHost] invalid: List[Dict] @@ -272,7 +276,8 @@ def __init__( self.breakers = {} self.clusters = {} self.filters = [] - self.groups = {} + self.tcp_mapping_groups = {} + self.http_mapping_groups = {} self.grpc_services = {} self.hosts = {} # self.invalidate_groups_for is handled above. @@ -660,8 +665,6 @@ def agent_finalize(self, aconf) -> None: self.logger.debug(f"Intercept agent not active, skipping finalization") return - # self.logger.info(f"Intercept agent active for {self.agent_service}, finalizing") - # We don't want to listen on the default AES ports (8080, 8443) as that is likely to # conflict with the user's application running in the same Pod. agent_listen_port_str = os.environ.get("AGENT_LISTEN_PORT", None) @@ -694,8 +697,6 @@ def agent_finalize(self, aconf) -> None: self.agent_active = False return - # self.logger.info(f"Intercept agent active for {self.agent_service}:{agent_port}, adding fallback mapping") - # XXX OMG this is a crock. Don't use precedence -1000000 for this, because otherwise Edge # Stack might decide it's the Edge Policy Console fallback mapping and force it to be # routed insecure. !*@&#*!@&#* We need per-mapping security settings. @@ -726,7 +727,7 @@ def agent_finalize(self, aconf) -> None: ) # No, really. See comment above. mapping.referenced_by(self.ambassador_module) - self.add_mapping(aconf, mapping) + self.add_http_mapping(aconf, mapping) def cache_fetch(self, key: str) -> Optional[IRResource]: """ @@ -978,20 +979,21 @@ def save_listener(self, listener: IRListener) -> None: if is_valid: self.listeners[listener_key] = listener - def add_mapping(self, aconf: Config, mapping: IRBaseMapping) -> Optional[IRBaseMappingGroup]: + def add_http_mapping(self, aconf: Config, mapping: IRHTTPMapping) -> None: mapping.check_status() - if mapping.is_active(): - if mapping.group_id not in self.groups: + if mapping.group_id not in self.http_mapping_groups: # Is this group in our external cache? group_key = mapping.group_class().key_for_id(mapping.group_id) group = self.cache_fetch(group_key) if group is not None: - self.logger.debug(f"IR: got group from cache for {mapping.name}") + self.logger.debug(f"IR: got http mapping group from cache for {mapping.name}") else: - self.logger.debug(f"IR: synthesizing group for {mapping.name}") - group_name = "GROUP: %s" % mapping.name + self.logger.debug( + f"IR: synthesizing new http mapping group for Mapping: {mapping.name}" + ) + group_name = "HTTP_MAPPING_GROUP: %s" % mapping.name group_class = mapping.group_class() group = group_class( ir=self, @@ -1000,26 +1002,57 @@ def add_mapping(self, aconf: Config, mapping: IRBaseMapping) -> Optional[IRBaseM name=group_name, mapping=mapping, ) - - # There's no way group can be anything but a non-None IRBaseMappingGroup - # here. assert() that so that mypy understands it. - assert isinstance(group, IRBaseMappingGroup) # for mypy - self.groups[group.group_id] = group + assert isinstance(group, IRHTTPMappingGroup) + self.http_mapping_groups[group.group_id] = group else: - self.logger.debug(f"IR: already have group for {mapping.name}") - group = self.groups[mapping.group_id] + self.logger.debug(f"IR: already have http mapping group for {mapping.name}") + group = self.http_mapping_groups[mapping.group_id] group.add_mapping(aconf, mapping) + self.cache_add(mapping) + self.cache_add(group) + self.cache_link(mapping, group) + def add_tcp_mapping(self, aconf: Config, mapping: IRTCPMapping) -> None: + mapping.check_status() + if mapping.is_active(): + if mapping.group_id not in self.tcp_mapping_groups: + # Is this group in our external cache? + group_key = mapping.group_class().key_for_id(mapping.group_id) + group = self.cache_fetch(group_key) + + if group is not None: + self.logger.debug(f"IR: got tcp mapping group from cache for {mapping.name}") + else: + self.logger.debug( + f"IR: synthesizing new tcp mapping group for TCPMapping: {mapping.name}" + ) + group_name = "TCP_MAPPING_GROUP: %s" % mapping.name + group_class = mapping.group_class() + group = group_class( + ir=self, + aconf=aconf, + location=mapping.location, + name=group_name, + mapping=mapping, + ) + assert isinstance(group, IRTCPMappingGroup) + self.tcp_mapping_groups[group.group_id] = group + else: + self.logger.debug(f"IR: already have tcp mapping group for {mapping.name}") + group = self.tcp_mapping_groups[mapping.group_id] + group.add_mapping(aconf, mapping) self.cache_add(mapping) self.cache_add(group) self.cache_link(mapping, group) - return group - else: - return None + def get_base_mapping_groups(self) -> List[IRBaseMappingGroup]: + return list(self.http_mapping_groups.values()) + list(self.tcp_mapping_groups.values()) + + def ordered_http_mapping_groups(self) -> Iterable[IRHTTPMappingGroup]: + return reversed(sorted(self.http_mapping_groups.values(), key=lambda x: x["group_weight"])) - def ordered_groups(self) -> Iterable[IRBaseMappingGroup]: - return reversed(sorted(self.groups.values(), key=lambda x: x["group_weight"])) + def ordered_tcp_mapping_groups(self) -> Iterable[IRTCPMappingGroup]: + return reversed(sorted(self.tcp_mapping_groups.values(), key=lambda x: x["group_weight"])) def has_cluster(self, name: str) -> bool: return name in self.clusters @@ -1081,7 +1114,10 @@ def as_dict(self) -> Dict[str, Any]: "hosts": [host.as_dict() for host in self.hosts.values()], "listeners": [self.listeners[x].as_dict() for x in sorted(self.listeners.keys())], "filters": [filt.as_dict() for filt in self.filters], - "groups": [group.as_dict() for group in self.ordered_groups()], + "http_mapping_groups": [ + group.as_dict() for group in self.ordered_http_mapping_groups() + ], + "tcp_mapping_groups": [group.as_dict() for group in self.ordered_tcp_mapping_groups()], "tls_contexts": [context.as_dict() for context in self.tls_contexts.values()], "services": self.services, "k8s_status_updates": self.k8s_status_updates, @@ -1345,21 +1381,17 @@ def features(self) -> Dict[str, Any]: group_resolver_consul = 0 # groups using the ConsulResolver mapping_count = 0 # total mappings - for group in self.ordered_groups(): + for http_group in self.ordered_http_mapping_groups(): group_count += 1 + group_http_count += 1 - if group.get("kind", "IRHTTPMappingGroup") == "IRTCPMappingGroup": - group_tcp_count += 1 - else: - group_http_count += 1 - - if group.get("precedence", 0) != 0: + if http_group.get("precedence", 0) != 0: group_precedence_count += 1 using_headers = False using_regex_headers = False - for header in group.get("headers", []): + for header in http_group.get("headers", []): using_headers = True if header["regex"]: @@ -1372,24 +1404,46 @@ def features(self) -> Dict[str, Any]: if using_regex_headers: group_regex_header_count += 1 - if len(group.mappings) > 1: + if len(http_group.mappings) > 1: group_canary_count += 1 - mapping_count += len(group.mappings) + mapping_count += len(http_group.mappings) - if group.get("shadows", []): + if http_group.get("shadows", []): group_shadow_count += 1 - if group.get("weight", 100) != 100: + if http_group.get("weight", 100) != 100: group_shadow_weighted_count += 1 - if group.get("host_redirect", {}): + if http_group.get("host_redirect", {}): group_host_redirect_count += 1 - if group.get("host_rewrite", None): + if http_group.get("host_rewrite", None): group_host_rewrite_count += 1 - res_name = group.get( + res_name = http_group.get( + "resolver", self.ambassador_module.get("resolver", "kubernetes-service") + ) + resolver = self.get_resolver(res_name) + + if resolver: + if resolver.kind == "KubernetesServiceResolver": + group_resolver_kube_service += 1 + elif resolver.kind == "KubernetesEndpoinhResolver": + group_resolver_kube_endpoint += 1 + elif resolver.kind == "ConsulResolver": + group_resolver_consul += 1 + + for tcp_group in self.ordered_tcp_mapping_groups(): + group_count += 1 + group_tcp_count += 1 + + if len(tcp_group.mappings) > 1: + group_canary_count += 1 + + mapping_count += len(tcp_group.mappings) + + res_name = tcp_group.get( "resolver", self.ambassador_module.get("resolver", "kubernetes-service") ) resolver = self.get_resolver(res_name) diff --git a/python/ambassador/ir/irambassador.py b/python/ambassador/ir/irambassador.py index fd10cd4a7f1..5fdd53d2be6 100644 --- a/python/ambassador/ir/irambassador.py +++ b/python/ambassador/ir/irambassador.py @@ -459,7 +459,7 @@ def add_mappings(self, ir: "IR", aconf: Config): if mapping is not None: # Cache hit. We know a priori that anything in the cache under a Mapping # key must be an IRBaseMapping, but let's assert that rather than casting. - assert isinstance(mapping, IRBaseMapping) + assert isinstance(mapping, IRHTTPMapping) else: mapping = IRHTTPMapping( ir, @@ -474,39 +474,7 @@ def add_mappings(self, ir: "IR", aconf: Config): ) mapping.referenced_by(self) - ir.add_mapping(aconf, mapping) - - # if ir.edge_stack_allowed: - # if self.diagnostics and self.diagnostics.get("enabled", False): - # ir.logger.debug("adding mappings for Edge Policy Console") - # edge_stack_response_header = {"x-content-type-options": "nosniff"} - # mapping = IRHTTPMapping(ir, aconf, rkey=self.rkey, location=self.location, - # name="edgestack-direct-mapping", - # metadata_labels={"ambassador_diag_class": "private"}, - # prefix="/edge_stack/", - # rewrite="/edge_stack_ui/edge_stack/", - # service="127.0.0.1:8500", - # precedence=1000000, - # timeout_ms=60000, - # hostname="*", - # add_response_headers=edge_stack_response_header) - # mapping.referenced_by(self) - # ir.add_mapping(aconf, mapping) - - # mapping = IRHTTPMapping(ir, aconf, rkey=self.rkey, location=self.location, - # name="edgestack-fallback-mapping", - # metadata_labels={"ambassador_diag_class": "private"}, - # prefix="^/$", prefix_regex=True, - # rewrite="/edge_stack_ui/", - # service="127.0.0.1:8500", - # precedence=-1000000, - # timeout_ms=60000, - # hostname="*", - # add_response_headers=edge_stack_response_header) - # mapping.referenced_by(self) - # ir.add_mapping(aconf, mapping) - # else: - # ir.logger.debug("diagnostics disabled, skipping mapping for Edge Policy Console") + ir.add_http_mapping(aconf, mapping) def get_default_label_domain(self) -> str: return self.default_label_domain diff --git a/python/ambassador/ir/irbasemappinggroup.py b/python/ambassador/ir/irbasemappinggroup.py index 01efd12fbc2..16dd0004e38 100644 --- a/python/ambassador/ir/irbasemappinggroup.py +++ b/python/ambassador/ir/irbasemappinggroup.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union from ..config import Config from .irbasemapping import IRBaseMapping @@ -55,15 +55,17 @@ def cache_key(self) -> str: return self._cache_key - def normalize_weights_in_mappings(self) -> bool: + def normalize_weights_in_mappings( + self, mappings: List[IRBaseMapping] + ) -> Tuple[List[IRBaseMapping], bool]: # If there's only one mapping in the group, it's automatically weighted # at 100%. - if len(self.mappings) == 1: + if len(mappings) == 1: self.logger.debug( - "Assigning weight 100 to single mapping %s in group", self.mappings[0].name + "Assigning weight 100 to single mapping %s in group", mappings[0].name ) - self.mappings[0]._weight = 100 - return True + mappings[0]._weight = 100 + return mappings, True # For multiple mappings, we need to normalize the weights. weightless_mappings = [] @@ -72,11 +74,11 @@ def normalize_weights_in_mappings(self) -> bool: normalized_mappings = [] current_weight = 0 - for mapping in self.mappings: + for mapping in mappings: if "weight" in mapping: if mapping.weight > 100: self.post_error(f"Mapping {mapping.name} has invalid weight {mapping.weight}") - return False + return mappings, False # increment current weight by mapping's weight current_weight += round(mapping.weight) @@ -98,7 +100,7 @@ def normalize_weights_in_mappings(self) -> bool: self.post_error( f"Total weight of mappings exceeds 100, please reconfigure for correct behavior..." ) - return False + return mappings, False if num_weightless_mappings > 0: # You might expect that we'd want to generate errors for the case where we hit 100% @@ -137,5 +139,4 @@ def normalize_weights_in_mappings(self) -> bool: weightless_mapping._weight = current_weight normalized_mappings.append(weightless_mapping) - self.mappings = normalized_mappings - return True + return normalized_mappings, True diff --git a/python/ambassador/ir/ircors.py b/python/ambassador/ir/ircors.py index ee160d41aed..a655589834d 100644 --- a/python/ambassador/ir/ircors.py +++ b/python/ambassador/ir/ircors.py @@ -52,10 +52,10 @@ def setup(self, ir: "IR", aconf: Config) -> bool: return True - def set_id(self, group_id: str): + def set_id(self, mapping_key: str): self["filter_enabled"] = { "default_value": {"denominator": "HUNDRED", "numerator": 100}, - "runtime_key": f"routing.cors_enabled.{group_id}", + "runtime_key": f"routing.cors_enabled.{mapping_key}", } def dup(self) -> "IRCORS": diff --git a/python/ambassador/ir/irhost.py b/python/ambassador/ir/irhost.py index c35ee0ffed6..0673f816211 100644 --- a/python/ambassador/ir/irhost.py +++ b/python/ambassador/ir/irhost.py @@ -449,9 +449,9 @@ def matches_httpgroup(self, group: "IRHTTPMappingGroup") -> bool: # The synthetic Mappings for diagnostics, readiness, and liveness probes always match all Hosts. # They can all still be disabled if desired via the Ambassador Module resource if groupName in [ - "GROUP: internal_readiness_probe_mapping", - "GROUP: internal_liveness_probe_mapping", - "GROUP: internal_diagnostics_probe_mapping", + "HTTP_MAPPING_GROUP: internal_readiness_probe_mapping", + "HTTP_MAPPING_GROUP: internal_liveness_probe_mapping", + "HTTP_MAPPING_GROUP: internal_diagnostics_probe_mapping", ]: return True diff --git a/python/ambassador/ir/irhttpmapping.py b/python/ambassador/ir/irhttpmapping.py index 700fe071aeb..678183797f9 100644 --- a/python/ambassador/ir/irhttpmapping.py +++ b/python/ambassador/ir/irhttpmapping.py @@ -6,15 +6,14 @@ from ..config import Config from .irbasemapping import IRBaseMapping, normalize_service_name -from .irbasemappinggroup import IRBaseMappingGroup from .ircors import IRCORS from .irerrorresponse import IRErrorResponse -from .irhttpmappinggroup import IRHTTPMappingGroup from .irretrypolicy import IRRetryPolicy from .irutils import selector_matches if TYPE_CHECKING: from .ir import IR # pragma: no cover + from .irhttpmappinggroup import IRHTTPMappingGroup # Kind of cheating here so that it's easy to json-serialize key-value pairs (including with regex) @@ -403,7 +402,9 @@ def __init__( self.post_error(RichStatus.fromError("outlier_detection is not supported")) @staticmethod - def group_class() -> Type[IRBaseMappingGroup]: + def group_class() -> Type["IRHTTPMappingGroup"]: + from .irhttpmappinggroup import IRHTTPMappingGroup + return IRHTTPMappingGroup def _enforce_mutual_exclusion(self, preferred, other): diff --git a/python/ambassador/ir/irhttpmappinggroup.py b/python/ambassador/ir/irhttpmappinggroup.py index 421ad15c4c3..e7ccdcadb0a 100644 --- a/python/ambassador/ir/irhttpmappinggroup.py +++ b/python/ambassador/ir/irhttpmappinggroup.py @@ -1,4 +1,6 @@ -from typing import TYPE_CHECKING, Any, ClassVar, Dict, List, Optional, Tuple +import re +from typing import TYPE_CHECKING, Any, ClassVar, Dict, List, Optional, Tuple, Union +from typing import cast from typing import cast as typecast from ambassador.utils import RichStatus @@ -7,23 +9,45 @@ from .irbasemapping import IRBaseMapping from .irbasemappinggroup import IRBaseMappingGroup from .ircluster import IRCluster +from .irhttpmapping import IRHTTPMapping from .irresource import IRResource +from .irutils import are_mapping_group_fixes_disabled if TYPE_CHECKING: from .ir import IR # pragma: no cover ######## -## IRHTTPMappingGroup is a collection of Mappings. We'll use it to build Envoy routes later, -## so the group itself ends up with some of the group-wide attributes of its Mappings. +## IRHTTPMappingGroup is a collection of Mappings. We'll use it to build Envoy routes later. +## The group itself shares a common group of settings for matching traffic. All mappings added to the group +## should have the same traffic matching settings. Settings for traffic modifications and where traffic is routed +## are set on a per-mapping basis. class IRHTTPMappingGroup(IRBaseMappingGroup): - host_redirect: Optional[IRBaseMapping] - shadow: List[IRBaseMapping] - rewrite: str - add_request_headers: Dict[str, str] - add_response_headers: Dict[str, str] + shadow_mappings: List[IRHTTPMapping] + + # This is the initial mapping used to create the group. We keep it in right now to support the case when + # ENABLE_MAPPING_GROUP_FIXES is not true and we want to have a single set of settings (adding headers, etc.) + # be shared across all mappings in a group. This will be removed in a future release. + seed_mapping: IRHTTPMapping + + # List of the fields within Mappings that control what requests to match on. + # we are not adding these as class fields since we do not want these keys to be set at all unless + # the mappings that are added to this group use those settings + TrafficMatchSettings: ClassVar[Dict[str, bool]] = { + "host": True, + "host_regex": True, + "prefix": True, + "prefix_exact": True, + "prefix_regex": True, + "case_sensitive": True, + "headers": True, + "method": True, + "method_regex": True, + "query_parameters": True, + "precedence": True, + } CoreMappingKeys: ClassVar[Dict[str, bool]] = { "bypass_auth": True, @@ -82,101 +106,67 @@ def helper_mappings(res: IRResource, k: str) -> Tuple[str, List[dict]]: reversed(sorted([x.as_dict() for x in res.mappings], key=lambda x: x["route_weight"])) ) - @staticmethod - def helper_shadows(res: IRResource, k: str) -> Tuple[str, List[dict]]: - return k, list([x.as_dict() for x in res[k]]) - def __init__( self, ir: "IR", aconf: Config, location: str, - mapping: IRBaseMapping, + mapping: IRHTTPMapping, rkey: str = "ir.mappinggroup", kind: str = "IRHTTPMappingGroup", name: str = "ir.mappinggroup", **kwargs, ) -> None: - # print("IRHTTPMappingGroup __init__ (%s %s %s)" % (kind, name, kwargs)) del rkey # silence unused-variable warning - if "host_redirect" in kwargs: - raise Exception( - "IRHTTPMappingGroup cannot accept a host_redirect as a keyword argument" - ) - - if "path_redirect" in kwargs: - raise Exception( - "IRHTTPMappingGroup cannot accept a path_redirect as a keyword argument" - ) - - if "prefix_redirect" in kwargs: - raise Exception( - "IRHTTPMappingGroup cannot accept a prefix_redirect as a keyword argument" - ) - - if "regex_redirect" in kwargs: - raise Exception( - "IRHTTPMappingGroup cannot accept a regex_redirect as a keyword argument" - ) - - if ("shadow" in kwargs) or ("shadows" in kwargs): - raise Exception( - "IRHTTPMappingGroup cannot accept shadow or shadows as a keyword argument" - ) + self.shadow_mappings: List[IRHTTPMapping] = [] super().__init__( ir=ir, aconf=aconf, rkey=mapping.rkey, location=location, kind=kind, name=name, **kwargs ) - - self.host_redirect = None - self.shadows: List[IRBaseMapping] = [] # XXX This should really be IRHTTPMapping, no? - self.add_dict_helper("mappings", IRHTTPMappingGroup.helper_mappings) - self.add_dict_helper("shadows", IRHTTPMappingGroup.helper_shadows) - - # Time to lift a bunch of core stuff from the first mapping up into the - # group. if ("group_weight" not in self) and ("route_weight" in mapping): self.group_weight = mapping.route_weight - for k in IRHTTPMappingGroup.CoreMappingKeys: - if (k not in self) and (k in mapping): + # Time to lift the traffic matching settings from the first mapping up into the group + for k in IRHTTPMappingGroup.TrafficMatchSettings: + if k in mapping: self[k] = mapping[k] + if "group_id" in mapping: + self["group_id"] = mapping["group_id"] + self.seed_mapping = mapping self.add_mapping(aconf, mapping) - # self.add_request_headers = {} - # self.add_response_headers = {} - # self.labels = {} - - def add_mapping(self, aconf: Config, mapping: IRBaseMapping) -> None: + def add_mapping(self, aconf: Config, mapping: IRHTTPMapping) -> None: mismatches = [] - for k in IRHTTPMappingGroup.CoreMappingKeys: - if (k in mapping) and ((k not in self) or (mapping[k] != self[k])): - mismatches.append((k, mapping[k], self.get(k, "-unset-"))) - - if mismatches: - self.post_error( - "cannot accept new mapping %s with mismatched %s." - "Please verify field is set with the same value in all related mappings." - "Example: When canary is configured, related mappings should have same fields and values" - % (mapping.name, ", ".join(["%s: %s != %s" % (x, y, z) for x, y, z in mismatches])) - ) - return - - # self.ir.logger.debug("%s: add mapping %s" % (self, mapping.as_json())) - - # Per the schema, host_redirect and shadow are Booleans. They won't be _saved_ as - # Booleans, though: instead we just save the Mapping that they're a part of. - host_redirect = mapping.get("host_redirect", False) - shadow = mapping.get("shadow", False) + if are_mapping_group_fixes_disabled(): + for k in IRHTTPMappingGroup.CoreMappingKeys: + if (k in mapping) and ( + (k not in self.seed_mapping) or (mapping[k] != self.seed_mapping[k]) + ): + mismatches.append((k, mapping[k], self.get(k, "-unset-"))) + else: + for k in mapping.keys(): + if ( + k.startswith("_") + or mapping.skip_key(k) + or (k in IRHTTPMappingGroup.DoNotFlattenKeys) + ): + continue + self.seed_mapping[k] = mapping[k] + else: + for k in IRHTTPMappingGroup.TrafficMatchSettings: + if (k in mapping) and ((k not in self) or (mapping[k] != self[k])): + mismatches.append((k, mapping[k], self.get(k, "-unset-"))) # First things first: if both shadow and host_redirect are set in this Mapping, # we're going to let shadow win. Kill the host_redirect part. + host_redirect = mapping.get("host_redirect", False) + shadow = mapping.get("shadow", False) if shadow and host_redirect: errstr = "At most one of host_redirect and shadow may be set; ignoring host_redirect" aconf.post_error(RichStatus.fromError(errstr), resource=mapping) @@ -186,61 +176,27 @@ def add_mapping(self, aconf: Config, mapping: IRBaseMapping) -> None: mapping.pop("prefix_redirect", None) mapping.pop("regex_redirect", None) - # OK. Is this a shadow Mapping? - if shadow: - # Yup. Make sure that we don't have multiple shadows. - if self.shadows: - errstr = "cannot accept %s as second shadow after %s" % ( - mapping.name, - self.shadows[0].name, - ) - aconf.post_error(RichStatus.fromError(errstr), resource=self) - else: - # All good. Save it. - self.shadows.append(mapping) - elif host_redirect: - # Not a shadow, but a host_redirect. Make sure we don't have multiples of - # those either. - - if self.host_redirect: - errstr = "cannot accept %s as second host_redirect after %s" % ( - mapping.name, - typecast(IRBaseMapping, self.host_redirect).name, - ) - aconf.post_error(RichStatus.fromError(errstr), resource=self) - elif len(self.mappings) > 0: - errstr = ( - "cannot accept %s with host_redirect after mappings without host_redirect (eg %s)" - % (mapping.name, self.mappings[0].name) - ) - aconf.post_error(RichStatus.fromError(errstr), resource=self) - else: - # All good. Save it. - self.host_redirect = mapping + if mismatches: + self.post_error( + "http mapping group cannot accept new mapping %s with mismatched %s." + "Please verify field is set with the same value in all related mappings." + "Example: When canary is configured, related mappings should have same request matching fields and values (ex: prefix/hostname)" + % (mapping.name, ", ".join(["%s: %s != %s" % (x, y, z) for x, y, z in mismatches])) + ) + return else: - # Neither shadow nor host_redirect are set in the Mapping. - # - # XXX At the moment, we do not do the right thing with the case where some Mappings - # in a group have host_redirect and some do not, so make sure that that can't happen. - - if self.host_redirect: - aconf.post_error( - "cannot accept %s without host_redirect after %s with host_redirect" - % (mapping.name, typecast(IRBaseMapping, self.host_redirect).name) - ) + # All good. Save this mapping. + if shadow: + self.shadow_mappings.append(mapping) else: - # All good. Save this mapping. self.mappings.append(mapping) - if mapping.route_weight > self.group_weight: - self.group_weight = mapping.route_weight + self.group_weight = mapping.group_weight self.referenced_by(mapping) - # self.ir.logger.debug("%s: group now %s" % (self, self.as_json())) - def add_cluster_for_mapping( - self, mapping: IRBaseMapping, marker: Optional[str] = None + self, mapping: IRHTTPMapping, marker: Optional[str] = None ) -> IRCluster: # Find or create the cluster for this Mapping... @@ -330,145 +286,98 @@ def add_cluster_for_mapping( def finalize(self, ir: "IR", aconf: Config) -> List[IRCluster]: """ - Finalize a MappingGroup based on the attributes of its Mappings. Core elements get lifted into - the Group so we can more easily build Envoy routes; host-redirect and shadow get handled, etc. - + Finalize a MappingGroup based on the attributes of its Mappings. :param ir: the IR we're working from :param aconf: the Config we're working from :return: a list of the IRClusters this Group uses """ - - add_request_headers: Dict[str, Any] = {} - add_response_headers: Dict[str, Any] = {} metadata_labels: Dict[str, str] = {} self.ir.logger.debug(f"IRHTTPMappingGroup: finalize %s", self.group_id) - for mapping in sorted(self.mappings, key=lambda m: m.route_weight): - # if verbose: - # self.ir.logger.debug("%s mapping %s" % (self, mapping.as_json())) - - for k in mapping.keys(): - if ( - k.startswith("_") - or mapping.skip_key(k) - or (k in IRHTTPMappingGroup.DoNotFlattenKeys) - ): - # if verbose: - # self.ir.logger.debug("%s: don't flatten %s" % (self, k)) - continue + assert isinstance(mapping, IRHTTPMapping) + # If no rewrite was given at all, default the rewrite to "/", so /, so e.g., if we map + # /prefix1/ to the service service1, then http://ambassador.example.com/prefix1/foo/bar + # would effectively be written to http://service1/foo/bar + # + # If they did give a rewrite, leave it alone so that the Envoy config can correctly + # handle an empty rewrite as no rewriting at all. + if "rewrite" not in mapping: + mapping.rewrite = "/" - # if verbose: - # self.ir.logger.debug("%s: flatten %s" % (self, k)) + if mapping.get("load_balancer", None) is None: + mapping["load_balancer"] = ir.ambassador_module.load_balancer - self[k] = mapping[k] + if mapping.get("keepalive", None) is None: + keepalive_default = ir.ambassador_module.get("keepalive", None) + if keepalive_default: + mapping["keepalive"] = keepalive_default - add_request_headers.update(mapping.get("add_request_headers", {})) - add_response_headers.update(mapping.get("add_response_headers", {})) + labels: Dict[str, Any] = mapping.get("labels", None) + if not labels: + # No labels. Use the default label domain to see if we have some valid defaults. + defaults = ir.ambassador_module.get_default_labels() + if defaults: + domain = ir.ambassador_module.get_default_label_domain() + mapping.labels = {domain: [{"defaults": defaults}]} + else: + # Walk all the domains in our labels, and prepend the defaults, if any. + for domain in labels.keys(): + defaults = ir.ambassador_module.get_default_labels(domain) + ir.logger.debug("%s: defaults %s" % (domain, defaults)) + if defaults: + ir.logger.debug("%s: labels %s" % (domain, labels[domain])) + for label in labels[domain]: + ir.logger.debug("%s: label %s" % (domain, label)) + lkeys = label.keys() + if len(lkeys) > 1: + err = RichStatus.fromError( + "label has multiple entries (%s) instead of just one" % lkeys + ) + aconf.post_error(err, self) + lkey = list(lkeys)[0] + if lkey.startswith("v0_ratelimit_"): + # Don't prepend defaults, as this was imported from a V0 rate_limit. + continue + label[lkey] = defaults + label[lkey] metadata_labels.update(mapping.get("metadata_labels") or {}) - if add_request_headers: - self.add_request_headers = add_request_headers - if add_response_headers: - self.add_response_headers = add_response_headers - if metadata_labels: self.metadata_labels = metadata_labels - if self.get("load_balancer", None) is None: - self["load_balancer"] = ir.ambassador_module.load_balancer - - # if verbose: - # self.ir.logger.debug("%s after flattening %s" % (self, self.as_json())) - - total_weight = 0.0 - unspecified_mappings = 0 - - # If no rewrite was given at all, default the rewrite to "/", so /, so e.g., if we map - # /prefix1/ to the service service1, then http://ambassador.example.com/prefix1/foo/bar - # would effectively be written to http://service1/foo/bar - # - # If they did give a rewrite, leave it alone so that the Envoy config can correctly - # handle an empty rewrite as no rewriting at all. - - if "rewrite" not in self: - self.rewrite = "/" - - # OK. Save some typing with local variables for default labels and our labels... - labels: Dict[str, Any] = self.get("labels", None) - - if self.get("keepalive", None) is None: - keepalive_default = ir.ambassador_module.get("keepalive", None) - if keepalive_default: - self["keepalive"] = keepalive_default - - if not labels: - # No labels. Use the default label domain to see if we have some valid defaults. - defaults = ir.ambassador_module.get_default_labels() - - if defaults: - domain = ir.ambassador_module.get_default_label_domain() - - self.labels = {domain: [{"defaults": defaults}]} - else: - # Walk all the domains in our labels, and prepend the defaults, if any. - # ir.logger.info("%s: labels %s" % (self.as_json(), labels)) - - for domain in labels.keys(): - defaults = ir.ambassador_module.get_default_labels(domain) - ir.logger.debug("%s: defaults %s" % (domain, defaults)) - - if defaults: - ir.logger.debug("%s: labels %s" % (domain, labels[domain])) - - for label in labels[domain]: - ir.logger.debug("%s: label %s" % (domain, label)) - - lkeys = label.keys() - if len(lkeys) > 1: - err = RichStatus.fromError( - "label has multiple entries (%s) instead of just one" % lkeys - ) - aconf.post_error(err, self) - - lkey = list(lkeys)[0] - - if lkey.startswith("v0_ratelimit_"): - # Don't prepend defaults, as this was imported from a V0 rate_limit. - continue - - label[lkey] = defaults + label[lkey] - - if self.shadows: - # Only one shadow is supported right now. - shadow = self.shadows[0] + for mapping in self.mappings: + assert isinstance(mapping, IRHTTPMapping) + # Mappings that do hostname redirects don't need a cluster + redir = mapping.get("host_redirect", None) + if not redir: + mapping.cluster = self.add_cluster_for_mapping(mapping, mapping.cluster_tag) - # The shadow is an IRMapping. Save the cluster for it. - shadow.cluster = self.add_cluster_for_mapping(shadow, marker="shadow") + for shadow_mapping in self.shadow_mappings: + # Add a special marker for mappings that do traffic mirroring/shadowing + shadow_mapping.cluster = self.add_cluster_for_mapping(shadow_mapping, marker="shadow") - # We don't need a cluster for host_redirect: it's just a name to redirect to. + self.ir.logger.debug(f"IRHTTPMappingGroup: normalizing weights for %s", self.group_id) - redir = self.get("host_redirect", None) + normalized_mappings, ok = self.normalize_weights_in_mappings(self.mappings) + if not ok: + self.post_error(f"Could not normalize mapping weights, ignoring...") + return [] + self.mappings = normalized_mappings - if not redir: + if len(self.shadow_mappings) > 0: self.ir.logger.debug( - f"IRHTTPMappingGroup: checking mapping clusters for %s", self.group_id + f"IRHTTPMappingGroup: normalizing shadow weights for %s", self.group_id ) - for mapping in self.mappings: - mapping.cluster = self.add_cluster_for_mapping(mapping, mapping.cluster_tag) - - self.ir.logger.debug(f"IRHTTPMappingGroup: normalizing weights for %s", self.group_id) - - if not self.normalize_weights_in_mappings(): - self.post_error(f"Could not normalize mapping weights, ignoring...") + normalized_mappings, ok = self.normalize_weights_in_mappings( + cast(List[IRBaseMapping], self.shadow_mappings) + ) + if not ok: + self.post_error(f"Could not normalize shadow weights, ignoring...") return [] - return list([mapping.cluster for mapping in self.mappings]) - else: - # Flatten the case_sensitive field for host_redirect if it exists - if "case_sensitive" in redir: - self["case_sensitive"] = redir["case_sensitive"] + self.shadow_mappings = cast(List[IRHTTPMapping], normalized_mappings) - return [] + # return all the clusters from our mappings (note that redirect mappings won't have a cluster) + return [mapping.cluster for mapping in self.mappings if "cluster" in mapping] diff --git a/python/ambassador/ir/irlistener.py b/python/ambassador/ir/irlistener.py index 3b7aca1e6ad..56166e38fde 100644 --- a/python/ambassador/ir/irlistener.py +++ b/python/ambassador/ir/irlistener.py @@ -318,10 +318,7 @@ def load_all(cls, ir: "IR", aconf: Config) -> None: def finalize(cls, ir: "IR", aconf: Config) -> None: # Finally, cycle over our TCPMappingGroups and make sure we have # Listeners for all of them, too. - for group in ir.ordered_groups(): - if not isinstance(group, IRTCPMappingGroup): - continue - + for group in ir.ordered_tcp_mapping_groups(): # OK. If we have a Listener binding here already, use it -- that lets the user override # any choices we might make if they want to. If there's no Listener here, though, we'll # need to create one. diff --git a/python/ambassador/ir/irmappingfactory.py b/python/ambassador/ir/irmappingfactory.py index 9a6f605ae5d..4a9ff5bc7e9 100644 --- a/python/ambassador/ir/irmappingfactory.py +++ b/python/ambassador/ir/irmappingfactory.py @@ -83,8 +83,12 @@ def load_config( ir.logger.debug("IR: MappingFactory adding live mappings") for mapping in live_mappings: - ir.logger.debug("IR: MappingFactory adding %s" % mapping.name) - ir.add_mapping(aconf, mapping) + if isinstance(mapping, IRHTTPMapping): + ir.logger.debug("IR: MappingFactory adding http Mapping %s" % mapping.name) + ir.add_http_mapping(aconf, mapping) + if isinstance(mapping, IRTCPMapping): + ir.logger.debug("IR: MappingFactory adding tcp Mapping %s" % mapping.name) + ir.add_tcp_mapping(aconf, mapping) ir.cache.dump("MappingFactory") @@ -95,9 +99,20 @@ def finalize(cls, ir: "IR", aconf: Config) -> None: ir.logger.debug("IR: MappingFactory finalizing") - for group in ir.groups.values(): - ir.logger.debug("IR: MappingFactory finalizing group %s", group.group_id) - group.finalize(ir, aconf) - ir.logger.debug("IR: MappingFactory finalized group %s", group.group_id) + for http_group in ir.http_mapping_groups.values(): + ir.logger.debug( + "IR: MappingFactory finalizing http mapping group %s", http_group.group_id + ) + http_group.finalize(ir, aconf) + ir.logger.debug( + "IR: MappingFactory finalized http mapping group %s", http_group.group_id + ) + + for tcp_group in ir.tcp_mapping_groups.values(): + ir.logger.debug( + "IR: MappingFactory finalizing tcp mapping group %s", tcp_group.group_id + ) + tcp_group.finalize(ir, aconf) + ir.logger.debug("IR: MappingFactory finalized tcp mapping group %s", tcp_group.group_id) ir.logger.debug("IR: MappingFactory finalized") diff --git a/python/ambassador/ir/irtcpmapping.py b/python/ambassador/ir/irtcpmapping.py index 15c32c8f0e5..9f83d1ed543 100644 --- a/python/ambassador/ir/irtcpmapping.py +++ b/python/ambassador/ir/irtcpmapping.py @@ -3,11 +3,10 @@ from ..config import Config from .irbasemapping import IRBaseMapping, normalize_service_name -from .irbasemappinggroup import IRBaseMappingGroup -from .irtcpmappinggroup import IRTCPMappingGroup if TYPE_CHECKING: from .ir import IR # pragma: no cover + from .irtcpmappinggroup import IRTCPMappingGroup class IRTCPMapping(IRBaseMapping): @@ -95,7 +94,9 @@ def __init__( ir.logger.debug("IRTCPMapping %s: self.host = %s", name, self.get("host") or "i'*'") @staticmethod - def group_class() -> Type[IRBaseMappingGroup]: + def group_class() -> Type["IRTCPMappingGroup"]: + from .irtcpmappinggroup import IRTCPMappingGroup + return IRTCPMappingGroup def bind_to(self) -> str: diff --git a/python/ambassador/ir/irtcpmappinggroup.py b/python/ambassador/ir/irtcpmappinggroup.py index 06e18b64e48..8d5585dc809 100644 --- a/python/ambassador/ir/irtcpmappinggroup.py +++ b/python/ambassador/ir/irtcpmappinggroup.py @@ -1,10 +1,11 @@ -from typing import TYPE_CHECKING, ClassVar, Dict, List, Optional, Tuple +from typing import TYPE_CHECKING, Any, ClassVar, Dict, List, Optional, Tuple, Union from ..config import Config from .irbasemapping import IRBaseMapping from .irbasemappinggroup import IRBaseMappingGroup from .ircluster import IRCluster from .irresource import IRResource +from .irtcpmapping import IRTCPMapping if TYPE_CHECKING: from .ir import IR # pragma: no cover @@ -57,13 +58,12 @@ def __init__( ir: "IR", aconf: Config, location: str, - mapping: IRBaseMapping, + mapping: IRTCPMapping, rkey: str = "ir.mappinggroup", kind: str = "IRTCPMappingGroup", name: str = "ir.mappinggroup", **kwargs, ) -> None: - # print("IRTCPMappingGroup __init__ (%s %s %s)" % (kind, name, kwargs)) del rkey # silence unused-variable warning super().__init__( @@ -84,7 +84,7 @@ def __init__( self.add_mapping(aconf, mapping) - def add_mapping(self, aconf: Config, mapping: IRBaseMapping) -> None: + def add_mapping(self, aconf: Config, mapping: IRTCPMapping) -> None: mismatches = [] for k in IRTCPMappingGroup.CoreMappingKeys: @@ -196,11 +196,8 @@ def finalize(self, ir: "IR", aconf: Config) -> List[IRCluster]: or mapping.skip_key(k) or (k in IRTCPMappingGroup.DoNotFlattenKeys) ): - # self.ir.logger.debug("%s: don't flatten %s" % (self, k)) continue - # self.ir.logger.debug("%s: flatten %s" % (self, k)) - self[k] = mapping[k] # Should we have higher weights win over lower if there are conflicts? @@ -210,62 +207,17 @@ def finalize(self, ir: "IR", aconf: Config) -> List[IRCluster]: if metadata_labels: self.metadata_labels = metadata_labels - # self.ir.logger.debug("%s after flattening %s" % (self, self.as_json())) - total_weight = 0.0 unspecified_mappings = 0 - # # OK. Save some typing with local variables for default labels and our labels... - # labels: Dict[str, Any] = self.get('labels', None) - # - # if not labels: - # # No labels. Use the default label domain to see if we have some valid defaults. - # defaults = ir.ambassador_module.get_default_labels() - # - # if defaults: - # domain = ir.ambassador_module.get_default_label_domain() - # - # self.labels = { - # domain: [ - # { - # 'defaults': defaults - # } - # ] - # } - # else: - # # Walk all the domains in our labels, and prepend the defaults, if any. - # # ir.logger.info("%s: labels %s" % (self.as_json(), labels)) - # - # for domain in labels.keys(): - # defaults = ir.ambassador_module.get_default_labels(domain) - # ir.logger.debug("%s: defaults %s" % (domain, defaults)) - # - # if defaults: - # ir.logger.debug("%s: labels %s" % (domain, labels[domain])) - # - # for label in labels[domain]: - # ir.logger.debug("%s: label %s" % (domain, label)) - # - # lkeys = label.keys() - # if len(lkeys) > 1: - # err = RichStatus.fromError("label has multiple entries (%s) instead of just one" % - # lkeys) - # aconf.post_error(err, self) - # - # lkey = list(lkeys)[0] - # - # if lkey.startswith('v0_ratelimit_'): - # # Don't prepend defaults, as this was imported from a V0 rate_limit. - # continue - # - # label[lkey] = defaults + label[lkey] - for mapping in self.mappings: mapping.cluster = self.add_cluster_for_mapping(mapping, mapping.cluster_tag) self.logger.debug(f"Normalizing weights in mappings now...") - if not self.normalize_weights_in_mappings(): + normalized_mappings, ok = self.normalize_weights_in_mappings(self.mappings) + if not ok: self.post_error(f"Could not normalize mapping weights, ignoring...") return [] + self.mappings = normalized_mappings return list([mapping.cluster for mapping in self.mappings]) diff --git a/python/ambassador/ir/irutils.py b/python/ambassador/ir/irutils.py index de6fb36cba8..c35f9e5e3ce 100644 --- a/python/ambassador/ir/irutils.py +++ b/python/ambassador/ir/irutils.py @@ -178,6 +178,10 @@ def disable_strict_selectors() -> bool: return parse_bool(os.environ.get("DISABLE_STRICT_LABEL_SELECTORS", "false")) +def are_mapping_group_fixes_disabled() -> bool: + return parse_bool(os.environ.get("DISABLE_MAPPING_GROUP_FIXES", "false")) + + ################ ## selector_matches is a utility for doing K8s label selector matching. diff --git a/python/ambassador_diag/diagd.py b/python/ambassador_diag/diagd.py index 3d9f8611a48..1c708447e5e 100644 --- a/python/ambassador_diag/diagd.py +++ b/python/ambassador_diag/diagd.py @@ -1831,7 +1831,7 @@ def _load_ir( app.kubestatus.post(kind, resource_name, namespace, text) - group_count = len(app.ir.groups) + group_count = len(app.ir.http_mapping_groups) + len(app.ir.tcp_mapping_groups) cluster_count = len(app.ir.clusters) listener_count = len(app.ir.listeners) service_count = len(app.ir.services) @@ -1944,15 +1944,18 @@ def check_environment(self, ir: Optional[IR] = None) -> None: tls_count += 1 break - for group in ir.groups.values(): - for mapping in group.mappings: + for http_group in ir.http_mapping_groups.values(): + for mapping in http_group.mappings: pfx = mapping.get("prefix", None) name = mapping.get("name", None) - if pfx: if not pfx.startswith("/ambassador/v0") or not name.startswith("internal_"): mapping_count += 1 + for group in ir.tcp_mapping_groups.values(): + for mapping in group.mappings: + mapping_count += 1 + if error_count: env_status.failure( "Error check", diff --git a/python/tests/unit/test_cache.py b/python/tests/unit/test_cache.py index a686bd6ed2d..4e5fe12e4f4 100644 --- a/python/tests/unit/test_cache.py +++ b/python/tests/unit/test_cache.py @@ -684,7 +684,7 @@ def check_group( ir, econf = b match = False - group = ir.groups.get("3644d75eb336f323bec43e48d4cfd8a950157607", None) + group = ir.http_mapping_groups.get("3644d75eb336f323bec43e48d4cfd8a950157607", None) if current_mappings: # There are some active mappings. Make sure that the group exists, that it has the diff --git a/python/tests/unit/test_irmapping.py b/python/tests/unit/test_irmapping.py index 42b8e3abdb8..899a9de3354 100644 --- a/python/tests/unit/test_irmapping.py +++ b/python/tests/unit/test_irmapping.py @@ -13,7 +13,7 @@ from ambassador import IR, Config from ambassador.fetch import ResourceFetcher -from ambassador.ir.irbasemappinggroup import IRBaseMappingGroup +from ambassador.ir.irhttpmappinggroup import IRHTTPMappingGroup from ambassador.utils import NullSecretHandler @@ -47,7 +47,7 @@ def test_ir_mapping(): """ conf = _get_ir_config(yaml) - all_mappings: List[IRBaseMappingGroup] = [] + all_mappings: List[IRHTTPMappingGroup] = [] for i in conf.groups.values(): all_mappings = all_mappings + i.mappings diff --git a/python/tests/unit/test_mapping.py b/python/tests/unit/test_mapping.py index 46978f5aa0a..ae53fda0a0c 100644 --- a/python/tests/unit/test_mapping.py +++ b/python/tests/unit/test_mapping.py @@ -32,7 +32,7 @@ def test_mapping_host_star_error(): assert errors[0]["ok"] == False assert errors[0]["error"] == "host exact-match * contains *, which cannot match anything." - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): assert g.prefix != "/star/" # print(json.dumps(ir.as_dict(), sort_keys=True, indent=4)) @@ -68,7 +68,7 @@ def test_mapping_host_authority_star_error(): errors[0]["error"] == ":authority exact-match '*' contains *, which cannot match anything." ) - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): assert g.prefix != "/star/" # print(json.dumps(ir.as_dict(), sort_keys=True, indent=4)) @@ -100,7 +100,7 @@ def test_mapping_host_ok(): found = 0 - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): if g.prefix == "/wanted_group/": assert g.host == "foo.example.com" found += 1 @@ -137,7 +137,7 @@ def test_mapping_host_authority_ok(): found = 0 - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): if g.prefix == "/wanted_group/": assert g.host == "foo.example.com" found += 1 @@ -175,7 +175,7 @@ def test_mapping_host_authority_and_host(): found = 0 - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): if g.prefix == "/wanted_group/": assert g.host == "foo.example.com" found += 1 @@ -211,7 +211,7 @@ def test_mapping_hostname_ok(): found = 0 - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): if g.prefix == "/wanted_group/": assert g.host == "*.example.com" found += 1 @@ -248,7 +248,7 @@ def test_mapping_hostname_and_host(): found = 0 - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): if g.prefix == "/wanted_group/": assert g.host == "*.example.com" found += 1 @@ -286,7 +286,7 @@ def test_mapping_hostname_and_authority(): found = 0 - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): if g.prefix == "/wanted_group/": assert g.host == "*.example.com" found += 1 @@ -325,7 +325,7 @@ def test_mapping_hostname_and_host_and_authority(): found = 0 - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): if g.prefix == "/wanted_group/": assert g.host == "*.example.com" found += 1 diff --git a/python/tests/unit/test_mapping_canary_group.py b/python/tests/unit/test_mapping_canary_group.py index 396d247bdc5..00c8b5f13ed 100644 --- a/python/tests/unit/test_mapping_canary_group.py +++ b/python/tests/unit/test_mapping_canary_group.py @@ -47,7 +47,7 @@ def test_mapping_canary_group_selectors(test_case): ) mapping_groups = [] - for g in ir.groups.values(): + for g in ir.http_mapping_groups.values(): if g.prefix.startswith("/ambassador") or g.prefix.startswith("/.ambassador"): continue