-
Notifications
You must be signed in to change notification settings - Fork 176
Added changes for IPv6 structure #1606
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
base: develop
Are you sure you want to change the base?
Added changes for IPv6 structure #1606
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor suggestion to reduce the amount of code duplication / files for the PHP script. Looks good otherwise.
I have not tested all those apps yet locally.
assets/php/htdocs/.htaccess
Outdated
@@ -0,0 +1,2 @@ | |||
RewriteEngine On | |||
RewriteRule ^(ipv4-test|ipv6-test|dual-stack-test)$ $1.php [L] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could technically rewrite to:
RewriteRule ^(ipv4|ipv6|dual-stack)-test$ ip-test.php?target=$1 [L]
This will then translate /ipv6-test
to a call to ip-test.php?target=ipv6
, etc.
Then you can have a single file, and the same type of map you have like in the other tests. Instead of the request path you'd base it on the value of the $_GET["target"]
parameter in PHP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the provided solution. I've added the changes it and it worked.
assets/php/htdocs/ip_functions.php
Outdated
@@ -0,0 +1,12 @@ | |||
<?php | |||
|
|||
function fetchIpAddress($url) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could now fold in this function into ip-test.php
and remove the include there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better. Done. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I'm not sure what the best practice is for Java based payloads, and if it should be as pre-bundled .jar
or built on the fly. But that' for the cf-deployment
experts to decide.
Key Updates:
Direct Output from Public Sites:
The main difference in the approach is that the apps now directly return the output from public Ips without additional processing for IP determination or success validation.
Go test Changes:
The Go app tests each buildpack listed in the script using the three main scenarios: IPv4, IPv6, and Dual-stack. I have created new function CurlAppWithStatusCode since our previously used function did not obtain the HTTP status code.
The updated function in the
app_curler.go
now modifies the curl command to retrieve both the response body and the HTTP status code. We utilize this output in the ginko tests by separating the IP address from the status code. Subsequently, we verify that the IP aligns with the scenario being validated and that the status code indicates success (HTTP 200).