IPv6 validation for Ruby apps - #1587
Conversation
|
Here are some suggestions (from Copilot) for improvement in this PR based on the diff provided: 1. Error Message Handling in Sinatra RouteIn the new Ruby code, the response always includes Suggestion: # Before:
"#{validation_name} validation resulted in #{result[:success] ? 'success' : 'failure'}. Detected IP type is #{result[:ip_type]}. Error message: #{result[:error]}"
# After:
message = "#{validation_name} validation resulted in #{result[:success] ? 'success' : 'failure'}. Detected IP type is #{result[:ip_type]}."
message += " Error message: #{result[:error]}" if result[:error]
message2. Potential Hardcoding & DRY PrincipleThe Suggestion: 3. Error Logging GranularityCurrently, in Suggestion: logger.error("Failed to reach #{endpoint}: #{e.class} - #{e.message}\n#{e.backtrace.join("\n")}")4. IP Type DetectionThe Suggestion: def determine_ip_type(ip_string)
ip = IPAddr.new(ip_string)
return 'IPv4' if ip.ipv4?
return 'IPv6' if ip.ipv6?
'Unknown'
rescue IPAddr::InvalidAddressError
'Invalid IP'
end5. Test CoverageWhile the Go test adds a Ruby test case, it only checks for a Suggestion: 6. Code FormattingThere are some double blank lines in the Ruby file that could be reduced to keep the code tidy. |
…ce-tests into CFDEPLOY-221-ipv6-validation-ruby
Applied all change, except 5.Test Coverage: We use endpoint mapper in the app to go through all of the endpoints. The Go tests are checking each of them ( /ipv4-test, /ipv6-test and /dual-stack-test) for the correct output. In ruby's case we are checking explicitly for "Healthy" when the default path is hit. For most of the buildpacks - the default is "Hello". For ruby it needed special handling. |
Are you submitting this PR against the develop branch?
Yes.
What is this change about?
The change is part of the IPv6 egress validation cycle. We progressively extend all of the exciting buildpacks relevant to our ecosystem. In this PR we provide additional endpoints in the ruby application for testing IPv6 support. In case ipv6 validation is enabled, we verify that egress IPv6 calls are possible with ruby applications.
Please provide contextual information.
https://github.com/cloudfoundry/community/blob/main/toc/rfc/rfc-0038-ipv6-dual-stack-for-cf.md
What version of cf-deployment have you run this cf-acceptance-test change against?
v48.9.0
Please check all that apply for this PR:
Did you update the README as appropriate for this change?
If you are introducing a new acceptance test, what is your rationale for including it CATs rather than your own acceptance test suite?
_CATs should validate IPv6 egress calls with Ruby application. We add changes regarding to this buildpack only. The test group for ipv6 was already created.
How many more (or fewer) seconds of runtime will this change introduce to CATs?
Around 90 seconds per test.
What is the level of urgency for publishing this change?
Tag your pair, your PM, and/or team!
@oliver-heinrich @iaftab-alam