Skip to content

Commit

Permalink
Update use of deprecated fields
Browse files Browse the repository at this point in the history
This is driven by "which of these cause test failures".  It is NOT an
audit of the codebase cross-referenced with
<https://www.envoyproxy.io/docs/envoy/latest/intro/deprecated>.  It would
be a good idea to do that audit at some point.
  • Loading branch information
LukeShu committed Feb 19, 2020
1 parent 5b28532 commit 310bb7b
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 28 deletions.
7 changes: 6 additions & 1 deletion cmd/ambex/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,15 @@ import (
"github.com/datawire/ambassador/pkg/envoy-control-plane/cache"
"github.com/datawire/ambassador/pkg/envoy-control-plane/server"

// envoy protobuf
// envoy protobuf -- Be sure to import the package of any types that the Python
// emits a "@type" of in the generated config, even if that package is otherwise
// not used by ambex.
v2 "github.com/datawire/ambassador/pkg/api/envoy/api/v2"
_ "github.com/datawire/ambassador/pkg/api/envoy/api/v2/auth"
core "github.com/datawire/ambassador/pkg/api/envoy/api/v2/core"
_ "github.com/datawire/ambassador/pkg/api/envoy/config/accesslog/v2"
bootstrap "github.com/datawire/ambassador/pkg/api/envoy/config/bootstrap/v2"
_ "github.com/datawire/ambassador/pkg/api/envoy/config/filter/network/http_connection_manager/v2"
discovery "github.com/datawire/ambassador/pkg/api/envoy/service/discovery/v2"
)

Expand Down
22 changes: 21 additions & 1 deletion python/ambassador/envoy/v2/v2bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,27 @@ def __init__(self, config: 'V2Config') -> None:
"cds_config": { "ads": {} },
"lds_config": { "ads": {} }
},
"admin": dict(config.admin)
"admin": dict(config.admin),
'layered_runtime': {
'layers': [
{
'name': 'static_layer',
'static_layer': {
# For now, we enable the deprecated & disallowed_by_default "HTTP_JSON_V1" Zipkin
# collector_endpoint_version because it repesents the Zipkin v1 API, while the
# non-deprecated options HTTP_JSON and HTTP_PROTO are the Zipkin v2 API; switching
# top one of them would change how Envoy talks to the outside world.
'envoy.deprecated_features:envoy.config.trace.v2.ZipkinConfig.HTTP_JSON_V1': True,
# Give our users more time to migrate to v2; we've said that we'll continue
# supporting both for a while even after we change the default.
'envoy.deprecated_features:envoy.config.filter.http.ext_authz.v2.ExtAuthz.use_alpha': True,
# We haven't yet told users that we'll be deprecating `regex_type: unsafe`.
'envoy.deprecated_features:envoy.api.v2.route.RouteMatch.regex': True, # HTTP path
'envoy.deprecated_features:envoy.api.v2.route.HeaderMatcher.regex_match': True # HTTP header
}
}
]
}
})

clusters = [{
Expand Down
13 changes: 10 additions & 3 deletions python/ambassador/envoy/v2/v2cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,20 @@ def __init__(self, config: 'V2Config', cluster: IRCluster) -> None:
# minimal `tls_context` to enable HTTPS origination.

if ctx.get('_ambassador_enabled', False):
fields['tls_context'] = {
envoy_ctx = {
'common_tls_context': {}
}
else:
envoy_ctx = V2TLSContext(ctx=ctx, host_rewrite=cluster.get('host_rewrite', None))
if envoy_ctx:
fields['tls_context'] = envoy_ctx

if envoy_ctx:
fields['transport_socket'] = {
'name': 'envoy.transport_sockets.tls',
'typed_config': {
'@type': 'type.googleapis.com/envoy.api.v2.auth.UpstreamTlsContext',
**envoy_ctx
}
}


keepalive = cluster.get('keepalive', None)
Expand Down
28 changes: 25 additions & 3 deletions python/ambassador/envoy/v2/v2config.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,39 @@ def __init__(self, ir: IR) -> None:
V2Bootstrap.generate(self)

def as_dict(self) -> Dict[str, Any]:
bootstrap_config, ads_config = self.split_config()

d = {
'bootstrap': self.bootstrap,
'static_resources': self.static_resources
'bootstrap': bootstrap_config,
**ads_config
}

return d

def split_config(self) -> Tuple[Dict[str, Any], Dict[str, Any]]:
ads_config = {
'@type': '/envoy.config.bootstrap.v2.Bootstrap',
'static_resources': self.static_resources
'static_resources': self.static_resources,
'layered_runtime': {
'layers': [
{
'name': 'static_layer',
'static_layer': {
# For now, we enable the deprecated & disallowed_by_default "HTTP_JSON_V1" Zipkin
# collector_endpoint_version because it repesents the Zipkin v1 API, while the
# non-deprecated options HTTP_JSON and HTTP_PROTO are the Zipkin v2 API; switching
# top one of them would change how Envoy talks to the outside world.
'envoy.deprecated_features:envoy.config.trace.v2.ZipkinConfig.HTTP_JSON_V1': True,
# Give our users more time to migrate to v2; we've said that we'll continue
# supporting both for a while even after we change the default.
'envoy.deprecated_features:envoy.config.filter.http.ext_authz.v2.ExtAuthz.use_alpha': True,
# We haven't yet told users that we'll be deprecating `regex_type: unsafe`.
'envoy.deprecated_features:envoy.api.v2.route.RouteMatch.regex': True, # HTTP path
'envoy.deprecated_features:envoy.api.v2.route.HeaderMatcher.regex_match': True # HTTP header
}
}
]
}
}

bootstrap_config = dict(self.bootstrap)
Expand Down
20 changes: 13 additions & 7 deletions python/ambassador/envoy/v2/v2listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,7 @@ def __init__(self, config: 'V2Config', service_port: int) -> None:
self.first_vhost: Optional[V2VirtualHost] = None
self.http_filters: List[dict] = []
self.listener_filters: List[dict] = []
self.traffic_direction: str = "UNSPECIFIED"

self.config.ir.logger.debug(f"V2Listener {self.name} created")

Expand Down Expand Up @@ -731,7 +732,8 @@ def __init__(self, config: 'V2Config', service_port: int) -> None:

self.access_log.append({
'name': 'envoy.file_access_log',
'config': {
'typed_config': {
'@type': 'type.googleapis.com/envoy.config.accesslog.v2.FileAccessLog',
'path': self.config.ir.ambassador_module.envoy_log_path,
'json_format': json_format
}
Expand All @@ -746,7 +748,8 @@ def __init__(self, config: 'V2Config', service_port: int) -> None:
self.config.ir.logger.debug("V2Listener: Using log_format '%s'" % log_format)
self.access_log.append({
'name': 'envoy.file_access_log',
'config': {
'typed_config': {
'@type': 'type.googleapis.com/envoy.config.accesslog.v2.FileAccessLog',
'path': self.config.ir.ambassador_module.envoy_log_path,
'format': log_format + '\n'
}
Expand Down Expand Up @@ -788,9 +791,8 @@ def __init__(self, config: 'V2Config', service_port: int) -> None:
if self.config.ir.tracing:
self.base_http_config["generate_request_id"] = True

self.base_http_config["tracing"] = {
"operation_name": "egress"
}
self.base_http_config["tracing"] = {}
self.traffic_direction = "OUTBOUND"

req_hdrs = self.config.ir.tracing.get('tag_headers', [])

Expand Down Expand Up @@ -886,7 +888,10 @@ def finalize(self) -> None:
filter_chain["filters"] = [
{
"name": "envoy.http_connection_manager",
"config": http_config
"typed_config": {
"@type": "type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager",
**http_config
}
}
]

Expand All @@ -903,7 +908,8 @@ def as_dict(self) -> dict:
"name": self.name,
"address": self.address,
"filter_chains": self.filter_chains,
"listener_filters": self.listener_filters
"listener_filters": self.listener_filters,
"traffic_direction": self.traffic_direction
}

def pretty(self) -> dict:
Expand Down
4 changes: 4 additions & 0 deletions python/ambassador/envoy/v2/v2route.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,14 @@ def __init__(self, config: 'V2Config', group: IRHTTPMappingGroup, mapping: IRBas

request_headers_to_remove = group.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)
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)
Expand Down
15 changes: 6 additions & 9 deletions python/ambassador/ir/ircors.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,15 @@ def __init__(self, ir: 'IR', aconf: Config,
def setup(self, ir: 'IR', aconf: Config) -> bool:
# 'origins' cannot be treated like other keys, because if it's a
# list, then it remains as is, but if it's a string, then it's
# converted to a list
# converted to a list. It has already been validated by the
# JSON schema to either be a string or a list of strings.
origins = self.pop('origins', None)

if origins is not None:
if type(origins) is list:
self.allow_origin = origins
elif type(origins) is str:
self.allow_origin = origins.split(',')
else:
self.post_error(RichStatus.fromError("invalid CORS origin - {}".format(origins),
module=self))
return False
if type(origins) is not list:
origins = origins.split(',')

self.allow_origin_string_match = [{'exact': origin} for origin in origins]

for from_key, to_key in [ ( 'max_age', 'max_age' ),
( 'credentials', 'allow_credentials' ),
Expand Down
6 changes: 3 additions & 3 deletions python/tests/t_listeneridletimeout.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def queries(self):
yield Query(self.url("config_dump"), phase=2)

def check(self):
expected_val = '30.000s'
expected_val = '30s'
actual_val = ''
body = json.loads(self.results[0].body)
for config_obj in body.get('configs'):
Expand All @@ -45,7 +45,7 @@ def check(self):
for filters in filter_chains:
for filter in filters.get('filters'):
if filter.get('name') == 'envoy.http_connection_manager':
filter_config = filter.get('config')
filter_config = filter.get('typed_config')
common_http_protocol_options = filter_config.get('common_http_protocol_options')
if common_http_protocol_options:
actual_val = common_http_protocol_options.get('idle_timeout', '')
Expand All @@ -56,4 +56,4 @@ def check(self):
assert False, "Expected to find common_http_protocol_options.idle_timeout property on listener"
else:
assert False, "Expected to find common_http_protocol_options property on listener"
assert found_idle_timeout, "Expected common_http_protocol_options.idle_timeout = {}, Got common_http_protocol_options.idle_timeout = {}".format(expected_val, actual_val)
assert found_idle_timeout, "Expected common_http_protocol_options.idle_timeout = {}, Got common_http_protocol_options.idle_timeout = {}".format(expected_val, actual_val)
2 changes: 1 addition & 1 deletion python/tests/t_logservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def check(self):
for dal in config_obj.get('dynamic_active_listeners'):
for filter_chain in dal.get('listener').get('filter_chains'):
for filter_obj in filter_chain.get('filters'):
access_logs = filter_obj.get('config').get('access_log')
access_logs = filter_obj.get('typed_config').get('access_log')
found_configured_access_log = False
for access_log in access_logs:
if access_log.get('name') == 'envoy.http_grpc_access_log' and access_log.get('config').get('common_config').get('grpc_service').get('envoy_grpc').get('cluster_name') == 'cluster_logging_stenography_25565_default':
Expand Down

0 comments on commit 310bb7b

Please sign in to comment.