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

AppProtect gRPC automation tests #1603

Merged
merged 10 commits into from Jun 3, 2021
Merged

AppProtect gRPC automation tests #1603

merged 10 commits into from Jun 3, 2021

Conversation

vepatel
Copy link
Contributor

@vepatel vepatel commented May 18, 2021

Proposed changes

  • Add automation tests for AppProtect gRPC support

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

Hi @vepatel

please see my questions and comments

Client sends request to /sayhello
"""
syslog_pod = kube_apis.v1.list_namespaced_pod(test_namespace).items[-1].metadata.name
block_response = subprocess.run(["binaries/./grpc_client", "-address", f"{backend_setup.ingress_host}:{backend_setup.ssl_port}"], capture_output=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

would it make sense to use https://github.com/grpc/grpc/blob/v1.37.1/examples/python/helloworld/greeter_client.py ? (providing we can make it work). this way there will be no need to add any binaries to the repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm checking https://pypi.org/project/grpc-requests/ which is based on the same, will update if it works!

tests/suite/test_app_protect_grpc.py Show resolved Hide resolved
spec:
containers:
- name: greeter
image: nginxkic/test-grpc-server:0.1
Copy link
Contributor

Choose a reason for hiding this comment

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

should we publish the sources similarly to how we published the sources for the TCP and UDP servers? so we don't loose it

Copy link
Contributor Author

@vepatel vepatel May 19, 2021

Choose a reason for hiding this comment

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

I'm not too sure about publishing sources for this one as the code isn't written by our team members unlike TCP/UDP server one which @soneillf5 wrote. Also If we're to add source in the tests, i'd prefer adding them to examples/ first.

Copy link
Contributor

Choose a reason for hiding this comment

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

in the case of the backend, it's just a Dockerfile, right? we don't really modify the backend code?

Copy link
Contributor

Choose a reason for hiding this comment

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

where does the test-grpc-server come from then ?

tests/suite/test_app_protect_grpc.py Show resolved Hide resolved
print("------------------------- Deploy Syslog -----------------------------")
src_syslog_yaml = f"{TEST_DATA}/appprotect/syslog.yaml"
create_items_from_yaml(kube_apis, src_syslog_yaml, test_namespace)
wait_before_test(40)
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if this wait is necessary here. is it for syslog pods? if that is the case, can we wait until the pod is ready?

Copy link
Contributor Author

@vepatel vepatel May 19, 2021

Choose a reason for hiding this comment

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

great suggestion but there is an issue at times when the pod is ready but endpoint isn't which is why the wait is for (see error below), I'll reduce the wait and use it in conjunction with waiting for pod.

line 85, in backend_setup
    .subsets[0]
TypeError: 'NoneType' object is not subscriptable

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. maybe we can wait for that condition (we have 1 endpoint), instead of for the pod being ready?

tests/suite/test_app_protect_grpc.py Outdated Show resolved Hide resolved
@vepatel vepatel removed the request for review from ciarams87 May 31, 2021 15:40
@vepatel vepatel merged commit 65adfd1 into master Jun 3, 2021
@vepatel vepatel deleted the tests/ap-grpc-automation branch June 3, 2021 10:12
@lucacome lucacome added the chore Pull requests for routine tasks label Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Pull requests for routine tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants