-
Notifications
You must be signed in to change notification settings - Fork 361
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
feat(TPG >= 4.50)!: Adding edge_security_policy #311
feat(TPG >= 4.50)!: Adding edge_security_policy #311
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Check the code in template dir. Can you update the code in template directory and execute make build? |
Can you also bump the provider version to 4.50 release which added this feature? |
192e18f
to
bad6258
Compare
@imrannayer @bharathkkb - It appears the LINT actually failed for #305 "Error: The following tests have failed: check_generate_modules", however it reported back to GH as pass and so missed that an updated |
Sure thing. I can fix the template. I suspected something was missed in a previous PR, but thanks for confirming. I'm on it. |
Thanks @zsiegel! |
/gcbrun |
@zsiegel integration test is failing with following error
|
Thanks for this! I will take a look this week and see if I can get it all cleared up. |
Ok I think I missed a ref for the backends in the autogen config. Let's give this a try! I was having some issues getting my local setup and I'm traveling but if this does not work I will need to get my local testing setup properly. Thanks |
I actually suspect I need to update the |
I updated this :) |
/gcbrun |
@zsiegel Lint check failed and also integration test is throwing following errors:
|
@imrannayer I fixed the lint issues. I am set up to run the tests via my local machine now, but I am running into quota issues. I submitted a request to see if I can get those increased! |
@imrannayer I ran the tests locally and everything passed except for one thing.
This looks like a random/transient error to me? |
Do t worry about that error. Update ur PR. |
16226bd
to
cf04d44
Compare
@zsiegel can you plz execute |
I did this already but let me give it another try. |
I ran this again and am not seeing any changes. Are you referring to the certificate manager changes for the certificate_map? I can investigate further. |
@zsiegel leme know when you can make the suggested change. |
/gcbrun |
LGTM |
@zsiegel Thanks for the PR |
Adds the edge security policy option.
Note that when I ran
make build
, there were additional changes made that seem unrelated to mine, and I do not understand why. Happy to fix it if there are any pointers on what to adjust or run.was changed back to