Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prepare for Envoy 1.13 upgrade; migrate from now-disabled deprecated fields #2328

Merged
merged 5 commits into from
Feb 20, 2020
Merged

Prepare for Envoy 1.13 upgrade; migrate from now-disabled deprecated fields #2328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7b0420b
envoy.mk: Tidy whitespace
LukeShu Feb 14, 2020
87ed625
envoy.mk: Fix missing semicolon
LukeShu Feb 14, 2020
519290c
ambex/main.go: Tidy up imports
LukeShu Feb 18, 2020
5b28532
envoy.mk: Clean up variables a touch
LukeShu Feb 19, 2020
b0b7bd8
Update use of deprecated fields
LukeShu Feb 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion builder/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ARG base="frolvlad/alpine-glibc:alpine-3.10"
ARG artifacts="builder"

# This controls where we copy envoy from.
ARG envoy="quay.io/datawire/ambassador-base:envoy-7.d17d947caef13f1bdd235c3fccff77814883bb46.opt"
ARG envoy="i-forgot-to-set-build-arg-envoy"

########################################
# The builder image
Expand Down
8 changes: 4 additions & 4 deletions builder/builder.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ bootstrap() {

if [ -z "$(builder)" ] ; then
printf "${CYN}==> ${GRN}Bootstrapping build image${END}\n"
${DBUILD} --target builder ${DIR} -t builder
${DBUILD} --build-arg envoy="${ENVOY_DOCKER_TAG}" --target builder ${DIR} -t builder
if [ "$(uname -s)" == Darwin ]; then
DOCKER_GID=$(stat -f "%g" /var/run/docker.sock)
else
Expand Down Expand Up @@ -356,11 +356,11 @@ case "${cmd}" in
docker rmi -f "${name}" &> /dev/null
docker commit -c 'ENTRYPOINT [ "/bin/bash" ]' $(builder) "${name}"
printf "${CYN}==> ${GRN}Building ${BLU}${BUILDER_NAME}${END}\n"
${DBUILD} ${DIR} --build-arg artifacts=${name} --target ambassador -t ${BUILDER_NAME}
${DBUILD} ${DIR} --build-arg artifacts=${name} --build-arg envoy="${ENVOY_DOCKER_TAG}" --target ambassador -t ${BUILDER_NAME}
printf "${CYN}==> ${GRN}Building ${BLU}kat-client${END}\n"
${DBUILD} ${DIR} --build-arg artifacts=${name} --target kat-client -t kat-client
${DBUILD} ${DIR} --build-arg artifacts=${name} --build-arg envoy="${ENVOY_DOCKER_TAG}" --target kat-client -t kat-client
printf "${CYN}==> ${GRN}Building ${BLU}kat-server${END}\n"
${DBUILD} ${DIR} --build-arg artifacts=${name} --target kat-server -t kat-server
${DBUILD} ${DIR} --build-arg artifacts=${name} --build-arg envoy="${ENVOY_DOCKER_TAG}" --target kat-server -t kat-server
fi
dexec rm -f /buildroot/image.dirty
;;
Expand Down
24 changes: 15 additions & 9 deletions cmd/ambex/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,29 @@ import (
"strings"
"syscall"

"github.com/fsnotify/fsnotify"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc"

log "github.com/sirupsen/logrus"
// protobuf library
"github.com/gogo/protobuf/jsonpb"
"github.com/gogo/protobuf/proto"
"github.com/gogo/protobuf/types"

v2 "github.com/datawire/ambassador/pkg/api/envoy/api/v2"
core "github.com/datawire/ambassador/pkg/api/envoy/api/v2/core"
// envoy control plane
"github.com/datawire/ambassador/pkg/envoy-control-plane/cache"
"github.com/datawire/ambassador/pkg/envoy-control-plane/server"

// 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"

"github.com/fsnotify/fsnotify"

"github.com/gogo/protobuf/jsonpb"
"github.com/gogo/protobuf/proto"
"github.com/gogo/protobuf/types"
)

const (
Expand Down
26 changes: 16 additions & 10 deletions cxx/envoy.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,26 @@ srcdir := $(OSS_HOME)/cxx
include $(OSS_HOME)/build-aux/prelude.mk

YES_I_AM_OK_WITH_COMPILING_ENVOY ?=
YES_I_AM_UPDATING_THE_BASE_IMAGES ?=

_git_remote_urls := $(shell git remote | xargs -n1 git remote get-url --all)
IS_PRIVATE ?= $(findstring private,$(_git_remote_urls))

# IF YOU MESS WITH ANY OF THESE VALUES, YOU MUST RUN `make update-base`.
_git_remote_urls := $(shell git remote | xargs -n1 git remote get-url --all)
IS_PRIVATE ?= $(findstring private,$(_git_remote_urls))
ENVOY_REPO ?= $(if $(IS_PRIVATE),git@github.com:datawire/envoy-private.git,git://github.com/datawire/envoy.git)
ENVOY_COMMIT ?= d17d947caef13f1bdd235c3fccff77814883bb46
ENVOY_COMPILATION_MODE ?= opt
# Increment BASE_ENVOY_RELVER on changes to `docker/base-envoy/Dockerfile`, or Envoy recipes
# Increment BASE_ENVOY_RELVER on changes to `docker/base-envoy/Dockerfile`, or Envoy recipes
BASE_ENVOY_RELVER ?= 7
ENVOY_DOCKER_TAG ?= $(if $(IS_PRIVATE),quay.io/datawire/ambassador-base-private:envoy-$(BASE_ENVOY_RELVER).$(ENVOY_COMMIT).$(ENVOY_COMPILATION_MODE),quay.io/datawire/ambassador-base:envoy-$(BASE_ENVOY_RELVER).$(ENVOY_COMMIT).$(ENVOY_COMPILATION_MODE))

BASE_VERSION.envoy ?= $(BASE_ENVOY_RELVER).$(ENVOY_COMMIT).$(ENVOY_COMPILATION_MODE)
ENVOY_DOCKER_REPO ?= quay.io/datawire/ambassador-base$(if $(IS_PRIVATE),-private)
ENVOY_DOCKER_VERSION ?= $(BASE_ENVOY_RELVER).$(ENVOY_COMMIT).$(ENVOY_COMPILATION_MODE)
ENVOY_DOCKER_TAG ?= $(ENVOY_DOCKER_REPO):envoy-$(ENVOY_DOCKER_VERSION)
# END LIST OF VARIABLES REQUIRING `make update-base`.

# for builder.sh...
export ENVOY_DOCKER_TAG

#
# Envoy build

Expand Down Expand Up @@ -59,7 +65,7 @@ $(srcdir)/envoy: FORCE
$(srcdir)/envoy-build-image.txt: $(srcdir)/envoy $(WRITE_IFCHANGED) FORCE
@PS4=; set -ex -o pipefail; { \
pushd $</ci; \
echo "$$(pwd)"; \
echo "$$(pwd)"; \
. envoy_build_sha.sh; \
popd; \
echo docker.io/envoyproxy/envoy-build-ubuntu@sha256:$$ENVOY_BUILD_SHA | $(WRITE_IFCHANGED) $@; \
Expand Down Expand Up @@ -100,12 +106,12 @@ ENVOY_BASH.deps = $(srcdir)/envoy-build-container.txt
$(OSS_HOME)/bin_linux_amd64/envoy-static: $(ENVOY_BASH.deps) FORCE
mkdir -p $(@D)
@PS4=; set -ex; { \
if docker run --rm --entrypoint=true $(BASE_IMAGE.envoy); then \
rsync -Pav --blocking-io -e 'docker run --rm -i' $$(docker image inspect $(BASE_IMAGE.envoy) --format='{{.Id}}' | sed 's/^sha256://'):/usr/local/bin/envoy $@; \
if docker run --rm --entrypoint=true $(ENVOY_DOCKER_TAG); then \
rsync -Pav --blocking-io -e 'docker run --rm -i' $$(docker image inspect $(ENVOY_DOCKER_TAG) --format='{{.Id}}' | sed 's/^sha256://'):/usr/local/bin/envoy $@; \
else \
if [ -z '$(YES_I_AM_UPDATING_THE_BASE_IMAGES)' ]; then \
{ set +x; } &>/dev/null; \
echo 'error: failed to pull $(BASE_IMAGE.envoy), but $$YES_I_AM_UPDATING_THE_BASE_IMAGES is not set'; \
echo 'error: failed to pull $(ENVOY_DOCKER_TAG), but $$YES_I_AM_UPDATING_THE_BASE_IMAGES is not set'; \
echo ' If you are trying to update the base images, then set that variable to a non-empty value.'; \
echo ' If you are not trying to update the base images, then check your network connection and Docker credentials.'; \
exit 1; \
Expand All @@ -116,7 +122,7 @@ $(OSS_HOME)/bin_linux_amd64/envoy-static: $(ENVOY_BASH.deps) FORCE
exit 1; \
fi; \
$(call ENVOY_BASH.cmd, \
docker exec --workdir=/root/envoy $$(cat $(srcdir)/envoy-build-container.txt) /bin/bash -c "export CC=/opt/llvm/bin/clang && export CXX=/opt/llvm/bin/clang++ && bazel build --verbose_failures -c $(ENVOY_COMPILATION_MODE) --config=clang //source/exe:envoy-static;" \
docker exec --workdir=/root/envoy $$(cat $(srcdir)/envoy-build-container.txt) /bin/bash -c "export CC=/opt/llvm/bin/clang && export CXX=/opt/llvm/bin/clang++ && bazel build --verbose_failures -c $(ENVOY_COMPILATION_MODE) --config=clang //source/exe:envoy-static;"; \
rsync -Pav --blocking-io -e 'docker exec -i' $$(cat $(srcdir)/envoy-build-container.txt):/root/envoy/bazel-bin/source/exe/envoy-static $@; \
); \
fi; \
Expand Down
22 changes: 21 additions & 1 deletion python/ambassador/envoy/v2/v2bootstrap.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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"
Copy link
Contributor Author

@LukeShu LukeShu Feb 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong-ish to me, but I think it's what preserves existing behavior.

See envoyproxy/envoy#7723 and envoyproxy/envoy#7999

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're correct. @alexgervais might have some thoughts as to whether we should support configuring this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
At some point, "operation_name": "egress" was required for tracing to work on upstream calls. Having a quick look at the docs, and it would seem traffic_direction could be omitted entirely. Anyways, I'd be happy to test tracing manually before shipping.


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
View file
Open in desktop
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
View file
Open in desktop
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
13 changes: 13 additions & 0 deletions python/tests/gold/acceptancegrpcbridgetest/bootstrap-ads.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@
"ads": {}
}
},
"layered_runtime": {
"layers": [
{
"name": "static_layer",
"static_layer": {
"envoy.deprecated_features:envoy.api.v2.route.HeaderMatcher.regex_match": true,
"envoy.deprecated_features:envoy.api.v2.route.RouteMatch.regex": true,
"envoy.deprecated_features:envoy.config.filter.http.ext_authz.v2.ExtAuthz.use_alpha": true,
"envoy.deprecated_features:envoy.config.trace.v2.ZipkinConfig.HTTP_JSON_V1": true
}
}
]
},
"node": {
"cluster": "acceptancegrpcbridgetest-default",
"id": "test-id"
Expand Down
Loading