Skip to content

Commit

Permalink
Code review feedback and fix test
Browse files Browse the repository at this point in the history
  • Loading branch information
@ciarams87
ciarams87 committed Nov 4, 2021
1 parent 5edecba commit 8547018
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 95 deletions.
12 changes: 5 additions & 7 deletions examples-of-custom-resources/grpc-upstreams/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,8 @@ $ kubectl create -f grpc-app.yaml
---- ------ ---- ---- -------
Normal AddedOrUpdated 7s nginx-ingress-controller Configuration for default/virtual-server was added or updated
```
2. Access the application using grpcurl
```
$ grpcurl -insecure -d '{\"name\": \"world!\"}' -protoset helloworld.protoset $IC_IP:$IC_HTTPS_PORT helloWorld.greeter/SayHello
Greeting: Hello world!
...
```
2. Access the application using a version of the helloworld gRPC client; see the (documentation here)[https://grpc.io/docs/languages/go/quickstart/] - e.g.:
$(go env GOPATH)/bin/greeter_client
2021/11/01 14:22:25 Received: world
2021/11/01 14:22:25 Greeting: Hello world
86 changes: 56 additions & 30 deletions internal/configs/version2/nginx-plus.virtualserver.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,23 +294,7 @@ server {
auth_jwt_key_file {{ .Secret }};
{{ end }}

{{ range $e := $l.ErrorPages }}
error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}";
{{ end }}

{{ if $l.ProxyInterceptErrors }}
proxy_intercept_errors on;
{{ end }}

{{ if $l.InternalProxyPass }}
proxy_pass {{ $l.InternalProxyPass }};
{{ end }}

{{ if or $l.ProxyPass $l.GRPCPass}}
{{ $proxyOrGRPC := "proxy" }}
{{ if $l.GRPCPass }}
{{ $proxyOrGRPC = "grpc" }}
{{ end }}
{{ $proxyOrGRPC := "proxy" }}{{ if $l.GRPCPass }}{{ $proxyOrGRPC = "grpc" }}{{ end }}

{{ with $l.EgressMTLS }}
{{ if .Certificate }}
Expand Down Expand Up @@ -349,6 +333,19 @@ server {
{{ end }}
{{ end }}

{{ range $e := $l.ErrorPages }}
error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}";
{{ end }}

{{ if $l.ProxyInterceptErrors }}
{{ $proxyOrGRPC }}_intercept_errors on;
{{ end }}

{{ if $l.InternalProxyPass }}
proxy_pass {{ $l.InternalProxyPass }};
{{ end }}

{{ if or $l.ProxyPass $l.GRPCPass}}
{{ range $r := $l.Rewrites }}
rewrite {{ $r }};
{{ end }}
Expand Down Expand Up @@ -435,19 +432,48 @@ server {

{{ with $ssl := $s.SSL }}
{{ if $ssl.HTTP2 }}
location @grpcerror400 { default_type application/grpc; return 400 "\n"; }
location @grpcerror401 { default_type application/grpc; return 401 "\n"; }
location @grpcerror403 { default_type application/grpc; return 403 "\n"; }
location @grpcerror404 { default_type application/grpc; return 404 "\n"; }
location @grpcerror405 { default_type application/grpc; return 405 "\n"; }
location @grpcerror408 { default_type application/grpc; return 408 "\n"; }
location @grpcerror414 { default_type application/grpc; return 414 "\n"; }
location @grpcerror426 { default_type application/grpc; return 426 "\n"; }
location @grpcerror500 { default_type application/grpc; return 500 "\n"; }
location @grpcerror501 { default_type application/grpc; return 501 "\n"; }
location @grpcerror502 { default_type application/grpc; return 502 "\n"; }
location @grpcerror503 { default_type application/grpc; return 503 "\n"; }
location @grpcerror504 { default_type application/grpc; return 504 "\n"; }
location @grpc_deadline_exceeded {
default_type application/grpc;
add_header grpc-status 4;
add_header grpc-message 'deadline exceeded';
return 204;
}
location @grpc_permission_denied {
default_type application/grpc;
add_header grpc-status 7;
add_header grpc-message 'permission denied';
return 204;
}
location @grpc_resource_exhausted {
default_type application/grpc;
add_header grpc-status 8;
add_header grpc-message 'resource exhausted';
return 204;
}
location @grpc_unimplemented {
default_type application/grpc;
add_header grpc-status 12;
add_header grpc-message unimplemented;
return 204;
}
location @grpc_internal {
default_type application/grpc;
add_header grpc-status 13;
add_header grpc-message 'internal error';
return 204;
}
location @grpc_unavailable {
default_type application/grpc;
add_header grpc-status 14;
add_header grpc-message unavailable;
return 204;
}
location @grpc_unauthenticated {
default_type application/grpc;
add_header grpc-status 16;
add_header grpc-message unauthenticated;
return 204;
}
{{ end }}
{{ end }}
}
86 changes: 56 additions & 30 deletions internal/configs/version2/nginx.virtualserver.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,23 +224,7 @@ server {
{{ if $rl.Delay }} delay={{ $rl.Delay }}{{ end }}{{ if $rl.NoDelay }} nodelay{{ end }};
{{ end }}

{{ range $e := $l.ErrorPages }}
error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}";
{{ end }}

{{ if $l.ProxyInterceptErrors }}
proxy_intercept_errors on;
{{ end }}

{{ if $l.InternalProxyPass }}
proxy_pass {{ $l.InternalProxyPass }};
{{ end }}

{{ if or $l.ProxyPass $l.GRPCPass}}
{{ $proxyOrGRPC := "proxy" }}
{{ if $l.GRPCPass }}
{{ $proxyOrGRPC = "grpc" }}
{{ end }}
{{ $proxyOrGRPC := "proxy" }}{{ if $l.GRPCPass }}{{ $proxyOrGRPC = "grpc" }}{{ end }}

{{ with $l.EgressMTLS }}
{{ if .Certificate }}
Expand All @@ -260,6 +244,19 @@ server {
{{ $proxyOrGRPC }}_ssl_name {{ .SSLName }};
{{ end }}

{{ range $e := $l.ErrorPages }}
error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}";
{{ end }}

{{ if $l.ProxyInterceptErrors }}
{{ $proxyOrGRPC }}_intercept_errors on;
{{ end }}

{{ if $l.InternalProxyPass }}
proxy_pass {{ $l.InternalProxyPass }};
{{ end }}

{{ if or $l.ProxyPass $l.GRPCPass}}
{{ range $r := $l.Rewrites }}
rewrite {{ $r }};
{{ end }}
Expand Down Expand Up @@ -337,19 +334,48 @@ server {

{{ with $ssl := $s.SSL }}
{{ if $ssl.HTTP2 }}
location @grpcerror400 { default_type application/grpc; return 400 "\n"; }
location @grpcerror401 { default_type application/grpc; return 401 "\n"; }
location @grpcerror403 { default_type application/grpc; return 403 "\n"; }
location @grpcerror404 { default_type application/grpc; return 404 "\n"; }
location @grpcerror405 { default_type application/grpc; return 405 "\n"; }
location @grpcerror408 { default_type application/grpc; return 408 "\n"; }
location @grpcerror414 { default_type application/grpc; return 414 "\n"; }
location @grpcerror426 { default_type application/grpc; return 426 "\n"; }
location @grpcerror500 { default_type application/grpc; return 500 "\n"; }
location @grpcerror501 { default_type application/grpc; return 501 "\n"; }
location @grpcerror502 { default_type application/grpc; return 502 "\n"; }
location @grpcerror503 { default_type application/grpc; return 503 "\n"; }
location @grpcerror504 { default_type application/grpc; return 504 "\n"; }
location @grpc_deadline_exceeded {
default_type application/grpc;
add_header grpc-status 4;
add_header grpc-message 'deadline exceeded';
return 204;
}
location @grpc_permission_denied {
default_type application/grpc;
add_header grpc-status 7;
add_header grpc-message 'permission denied';
return 204;
}
location @grpc_resource_exhausted {
default_type application/grpc;
add_header grpc-status 8;
add_header grpc-message 'resource exhausted';
return 204;
}
location @grpc_unimplemented {
default_type application/grpc;
add_header grpc-status 12;
add_header grpc-message unimplemented;
return 204;
}
location @grpc_internal {
default_type application/grpc;
add_header grpc-status 13;
add_header grpc-message 'internal error';
return 204;
}
location @grpc_unavailable {
default_type application/grpc;
add_header grpc-status 14;
add_header grpc-message unavailable;
return 204;
}
location @grpc_unauthenticated {
default_type application/grpc;
add_header grpc-status 16;
add_header grpc-message unauthenticated;
return 204;
}
{{ end }}
{{ end }}
}
15 changes: 0 additions & 15 deletions tests/suite/test_v_s_route_grpc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ def fin():

request.addfinalizer(fin)

@pytest.mark.ciara
@pytest.mark.vsr
@pytest.mark.smoke
@pytest.mark.parametrize('crd_ingress_controller, v_s_route_setup',
Expand Down Expand Up @@ -160,20 +159,6 @@ def test_validation_flow(self, kube_apis, ingress_controller_prerequisites,
self.patch_valid_vs_route(kube_apis, v_s_route_setup)
wait_before_test()

print("\nTest 2: No HTTP2")
replace_configmap_from_yaml(kube_apis.v1,
ingress_controller_prerequisites.config_map['metadata']['name'],
ingress_controller_prerequisites.namespace,
f"{DEPLOYMENTS}/common/nginx-config.yaml")
wait_before_test()
text = "gRPC requires enabled HTTP/2 and TLS termination"
response_m = read_custom_resource(kube_apis.custom_objects, v_s_route_setup.route_m.namespace,
"virtualserverroutes",v_s_route_setup.route_m.name)
assert (response_m["status"]
and response_m["status"]["reason"] == "AddedOrUpdatedWithWarning"
and response_m["status"]["state"] == "Warning"
and text in response_m["status"]["message"])

@pytest.mark.parametrize("backend_setup", [{"app_type": "grpc-vs-mixed"}], indirect=True)
def test_mixed_config(self, kube_apis, ingress_controller_prerequisites, crd_ingress_controller,
backend_setup, v_s_route_setup):
Expand Down
13 changes: 0 additions & 13 deletions tests/suite/test_virtual_server_grpc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,16 +176,3 @@ def test_validation_flow(self, kube_apis, ingress_controller_prerequisites,

self.patch_valid_vs(kube_apis, virtual_server_setup)
wait_before_test()

print("\nTest 2: No HTTP2")
replace_configmap_from_yaml(kube_apis.v1,
ingress_controller_prerequisites.config_map['metadata']['name'],
ingress_controller_prerequisites.namespace,
f"{DEPLOYMENTS}/common/nginx-config.yaml")
wait_before_test()
text = "gRPC requires enabled HTTP/2 and TLS termination"
response = read_custom_resource(kube_apis.custom_objects, virtual_server_setup.namespace,
"virtualservers", virtual_server_setup.vs_name)
assert (response["status"] and response["status"]["reason"] == "AddedOrUpdatedWithWarning"
and response["status"]["state"] == "Warning")
assert text in response["status"]["message"]

0 comments on commit 8547018

Please sign in to comment.