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.

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

b/149050012: Remove backend protocol flag #31

Merged
merged 11 commits into from
Feb 27, 2020
146 changes: 66 additions & 80 deletions docker/generic/start_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,12 @@
# Default Listener port
DEFAULT_LISTENER_PORT = 8080

# Default Backend HTTP/1.x port
DEFAULT_BACKEND_HTTP1_PORT = 80

# Default backend
DEFAULT_BACKEND = "127.0.0.1:8082"
DEFAULT_BACKEND = "http://127.0.0.1:8082"

# Default rollout_strategy
DEFAULT_ROLLOUT_STRATEGY = "fixed"

# Protocol prefixes
GRPC_PREFIX = "grpc://"
HTTP_PREFIX = "http://"
HTTPS_PREFIX = "https://"

# Google default application credentials environment variable
GOOGLE_CREDS_KEY = "GOOGLE_APPLICATION_CREDENTIALS"

Expand Down Expand Up @@ -96,15 +88,6 @@ def gen_bootstrap_conf(args):
return cmd


def start_proxy(proxy_conf):
try:
os.execv(PROXY_STARTER, proxy_conf)
except OSError as err:
logging.error("Failed to launch ESPv2")
logging.error(err.strerror)
sys.exit(1)


class ArgumentParser(argparse.ArgumentParser):
def error(self, message):
self.print_help(sys.stderr)
Expand Down Expand Up @@ -155,24 +138,40 @@ def make_argparser():
--service, --version, and --rollout_strategy.
''')

# TODO(b/149050012): Update this help text when `--backend_http_protocol`
# is introduced.
parser.add_argument(
'-a',
'--backend',
default=DEFAULT_BACKEND,
help=''' Change the application server address to which ESPv2
proxies requests. Default value: {backend}. For HTTPS backends,
please use "https://" prefix, e.g. https://127.0.0.1:8082.
For HTTP/1.x backends, prefix "http://" is optional.
For GRPC backends, please use "grpc://" prefix,
e.g. grpc://127.0.0.1:8082.'''.format(backend=DEFAULT_BACKEND))

parser.add_argument(
'--backend_protocol',
default=None,
help='''Backend Protocol. Overrides the protocol in --backend.
Choices: [http|https|grpc|grpcs].
Default value: http.''',
choices=['http', 'https', 'grpc', 'grpcs'])
help='''
nareddyt marked this conversation as resolved.
Show resolved Hide resolved
Specify the backend application server address.
Default value: {backend}.
nareddyt marked this conversation as resolved.
Show resolved Hide resolved

Please use the following schemes as a prefix to specify the protocol
your backend uses. This must be formatted as a full address, with
scheme, hostname, and port.

SCHEME PROTOCOL SECURITY EXAMPLE
http:// HTTP None http://127.0.0.1:80
https:// HTTP TLS https://127.0.0.1:443
grpc:// gRPC None grpc://127.0.0.1:80
grpcs:// gRPC TLS grpcs://127.0.0.1:443

If the scheme is not specified in the URI, ESPv2 automatically uses https.

If the port is not specified in the URI, ESPv2 automatically uses:
- 80 for schemes without TLS
- 443 for schemes with TLS

If your backend requires TLS, you must use a secure scheme.

If you need to use a remote backend, we recommend specifying the URI
nareddyt marked this conversation as resolved.
Show resolved Hide resolved
in the service configuration instead of this flag. The following references
are available on the public Google Cloud Endpoints documentation:
- The OpenAPI `x-google-backend` extension
- The gRPC service configuration reference
'''.format(backend=DEFAULT_BACKEND))

parser.add_argument('--listener_port', default=None, type=int, help='''
The port to accept downstream connections.
Expand Down Expand Up @@ -286,19 +285,6 @@ def make_argparser():
help='''Enable fetching service name, service config ID and rollout
strategy from the metadata service.''')

parser.add_argument(
'--enable_backend_routing',
action='store_true',
default=False,
help='''
===
DEPRECATED: This flag will automatically be enabled if needed, so it
does NOT need to be set manually.
===
Enable ESPv2 to route requests according to the
"x-google-backend" or "backend" configuration
''')

parser.add_argument(
'--envoy_use_remote_address',
action='store_true',
Expand Down Expand Up @@ -474,10 +460,40 @@ def make_argparser():
- Debug level service configuration logs in Config Manager
- Admin interface in Envoy
''')

# Start Deprecated Flags Section

parser.add_argument(
'--enable_backend_routing',
action='store_true',
default=False,
help='''
===
DEPRECATED: This flag will automatically be enabled if needed, so it
does NOT need to be set manually.
===
Enable ESPv2 to route requests according to the
"x-google-backend" or "backend" configuration
''')
parser.add_argument(
'--backend_protocol',
default=None,
help='''
===
DEPRECATED: This flag will automatically be set based on the scheme
specified in the --backend flag. Overrides are no longer needed.
===
Backend Protocol. Overrides the protocol in --backend.
Choices: [http1|http2|grpc].
Default value: http1.''',
choices=['http1', 'http2', 'grpc'])

# End Deprecated Flags Section

return parser

# Check whether there are conflict flags. If so, return the error string. Otherwise returns None.
# This function also changes some default flag value.
# Check whether there are conflict flags. If so, return the error string.
# Otherwise returns None. This function also changes some default flag value.
def enforce_conflict_args(args):
if args.rollout_strategy:
if args.rollout_strategy not in {"fixed", "managed"}:
Expand Down Expand Up @@ -526,40 +542,10 @@ def gen_proxy_config(args):
logging.error(check_conflict_result)
sys.exit(1)

if args.backend_protocol is None:
if args.backend.startswith(GRPC_PREFIX):
backend_protocol = "grpc"
backends = args.backend[len(GRPC_PREFIX):]
elif args.backend.startswith(HTTP_PREFIX):
backend_protocol = "http"
backends = args.backend[len(HTTP_PREFIX):]
elif args.backend.startswith(HTTPS_PREFIX):
backend_protocol = "https"
backend = args.backend[len(HTTPS_PREFIX):]
if not re.search(r':[0-9]+$', backend):
backend = backend + ':443'
backends = backend
else:
backend_protocol = "http"
backends = args.backend
else:
backend_protocol = args.backend_protocol
backends = args.backend

cluster_args = backends.split(':')
if len(cluster_args) == 2:
cluster_address = cluster_args[0]
cluster_port = cluster_args[1]
elif len(cluster_args) == 1:
cluster_address = cluster_args[0]
cluster_port = str(DEFAULT_BACKEND_HTTP1_PORT)
else:
print("incorrect backend")
sys.exit(1)

proxy_conf = [
CONFIGMANAGER_BIN, "--logtostderr", "--backend_protocol", backend_protocol,
"--cluster_address", cluster_address, "--cluster_port", cluster_port,
CONFIGMANAGER_BIN,
"--logtostderr",
"--backend_address", args.backend,
"--rollout_strategy", args.rollout_strategy,
]

Expand Down
32 changes: 20 additions & 12 deletions src/go/bootstrap/static/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestServiceToBootstrapConfig(t *testing.T) {
{
desc: "envoy config with service control, no tracing",
opt_mod: func(opt *options.ConfigGeneratorOptions) {
opt.BackendProtocol = "http"
opt.BackendAddress = "http://127.0.0.1:8082"
opt.DisableTracing = true
},
serviceConfigPath: platform.GetFilePath(platform.ScServiceConfig),
Expand All @@ -54,7 +54,7 @@ func TestServiceToBootstrapConfig(t *testing.T) {
{
desc: "envoy config for auth",
opt_mod: func(opt *options.ConfigGeneratorOptions) {
opt.BackendProtocol = "http"
opt.BackendAddress = "http://127.0.0.1:8082"
opt.DisableTracing = true
opt.SkipServiceControlFilter = true
},
Expand All @@ -64,7 +64,7 @@ func TestServiceToBootstrapConfig(t *testing.T) {
{
desc: "envoy config with dynamic routing",
opt_mod: func(opt *options.ConfigGeneratorOptions) {
opt.BackendProtocol = "http"
opt.BackendAddress = "http://127.0.0.1:8082"
opt.DisableTracing = true
opt.SkipServiceControlFilter = true
},
Expand All @@ -74,7 +74,7 @@ func TestServiceToBootstrapConfig(t *testing.T) {
{
desc: "envoy config for path matcher",
opt_mod: func(opt *options.ConfigGeneratorOptions) {
opt.BackendProtocol = "http"
opt.BackendAddress = "http://127.0.0.1:8082"
opt.DisableTracing = true
opt.SkipServiceControlFilter = true
},
Expand All @@ -84,7 +84,7 @@ func TestServiceToBootstrapConfig(t *testing.T) {
{
desc: "grpc dynamic routing",
opt_mod: func(opt *options.ConfigGeneratorOptions) {
opt.BackendProtocol = "grpc"
opt.BackendAddress = "grpc://127.0.0.1:8082"
opt.DisableTracing = true
},
serviceConfigPath: platform.GetFilePath(platform.GrpcEchoServiceConfig),
Expand All @@ -95,7 +95,8 @@ func TestServiceToBootstrapConfig(t *testing.T) {
for testIdx, tc := range testData {
configBytes, err := ioutil.ReadFile(tc.serviceConfigPath)
if err != nil {
t.Fatalf("ReadFile failed, got %v", err)
t.Errorf("ReadFile failed, got %v", err)
continue
}
unmarshaler := &jsonpb.Unmarshaler{
AnyResolver: util.Resolver,
Expand All @@ -104,7 +105,8 @@ func TestServiceToBootstrapConfig(t *testing.T) {

var s confpb.Service
if err := unmarshaler.Unmarshal(bytes.NewBuffer(configBytes), &s); err != nil {
t.Fatalf("Unmarshal() returned error %v, want nil", err)
t.Errorf("Unmarshal() returned error %v, want nil", err)
continue
}

opts := flags.EnvoyConfigOptionsFromFlags()
Expand All @@ -113,29 +115,35 @@ func TestServiceToBootstrapConfig(t *testing.T) {
// Function under test
gotBootstrap, err := ServiceToBootstrapConfig(&s, FakeConfigID, opts)
if err != nil {
t.Fatal(err)
t.Error(err)
continue
}

envoyConfig, err := ioutil.ReadFile(tc.envoyConfigPath)
if err != nil {
t.Fatalf("ReadFile failed, got %v", err)
t.Errorf("ReadFile failed, got %v", err)
continue
}

var expectedBootstrap bootstrappb.Bootstrap
if err := unmarshaler.Unmarshal(bytes.NewBuffer(envoyConfig), &expectedBootstrap); err != nil {
t.Fatalf("Unmarshal() returned error %v, want nil", err)
t.Errorf("Unmarshal() returned error %v, want nil", err)
continue
}

gotString, err := bootstrapToJson(gotBootstrap)
if err != nil {
t.Fatal(err)
t.Error(err)
continue
}
wantString, err := bootstrapToJson(&expectedBootstrap)
if err != nil {
t.Fatal(err)
t.Error(err)
continue
}
if gotString != wantString {
t.Errorf("test(%d): %s\ngot : %v, \nwant: %v", testIdx, tc.desc, gotString, wantString)
continue
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/go/configgenerator/cluster_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func makeBackendCluster(opt *options.ConfigGeneratorOptions, brc *sc.BackendRout
ClusterDiscoveryType: &v2pb.Cluster_Type{v2pb.Cluster_LOGICAL_DNS},
LoadAssignment: util.CreateLoadAssignment(brc.Hostname, brc.Port),
}
isHttp2 := brc.HttpProtocol == util.HTTP2 || brc.Protocol == util.GRPC
isHttp2 := brc.Protocol == util.GRPC || brc.HttpProtocol == util.HTTP2

if brc.UseTLS {
var alpnProtocols []string
Expand Down
Loading