-
Notifications
You must be signed in to change notification settings - Fork 17
support enabling webvalve in non-local, production-like envs #34
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
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
37b0e1c
default services to enabled with webvalve_enabled=1 in production
samandmoore fc49f1e
move the to_s so it's more obvious. update changelog and version
samandmoore 36a6381
fix typo
samandmoore 3f10dc6
clarify test language a bit
samandmoore 1468bbd
okay, here's a better approach
samandmoore 1183e2c
in on+allowing mode, only install webmock if at least one service is …
samandmoore 63b5309
update manager specs
samandmoore e778e8e
update fake service config specs
samandmoore 20a6091
add exhaustive specs for the envs
samandmoore 19eb0d4
okay, test coverage, complete?
samandmoore 0752a61
fix disabled reference
samandmoore 4e8fd68
slight readme tweak
samandmoore 04d2aaf
add ADR for new features
samandmoore a2235d7
implement PR feedback with WEBVALVE_ENABLED and WEBVALVE_SERVICE_ENAB…
samandmoore 2bd0c8b
update the ADR
samandmoore db4947c
update changelog
samandmoore 820e982
update readme
samandmoore c51648b
delete notes
samandmoore File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| # ADR 001: Use ENV variable to control behavior in production-like envs | ||
|
|
||
| ## Status | ||
|
|
||
| Accepted | ||
|
|
||
| ## Context | ||
|
|
||
| We would like to be able to use WebValve to fake external service | ||
| integrations in production-like environments. | ||
|
|
||
| When using WebValve in a production-like environment, we'd like to be | ||
| able to support three different use-cases: | ||
| * enable all services by default, and don't load WebValve at all. This | ||
| is equivalent to WebValve's current production behavior: | ||
| zero-overhead, no risk of accidentally loading a fake service | ||
| instead of a real one. | ||
| * enable all services by default (as if WebValve is disabled) but allow | ||
| each service to be flipped into faking mode based on an env | ||
| variable. This allows us to support faking a service in a staging | ||
| environment where we cannot actually integrate with a real or | ||
| sandbox version of the external service while still connecting to | ||
| real versions of all other services. | ||
| * disable all services by default (as if WebValve is enabled) but allow | ||
| each service to be flipped into real mode based on an env variable. | ||
| This allows us to spin up cheap one-off environments for | ||
| user-testing, proof-of-concepting, etc. | ||
|
|
||
| In summary, we need ways to control: | ||
| * the activation of WebValve: loading the library, loading | ||
| the fakes, configuring WebMock | ||
| * the default mode of WebValve: intercepting vs. allowing | ||
| (pass-through) | ||
| * the explicit enabling / disabling of individual services | ||
|
|
||
| We came up with a few approaches to support these use-cases: | ||
|
|
||
| *Activate if required* | ||
| This will load and activate WebValve when it's required. If you don't | ||
| want to activate it, don't require it. The downside to this approach is | ||
| it's easy to accidentally load WebValve in the wrong env. Additionally, | ||
| in an env where we want to enable all services by default and only | ||
| disable select ones, we'd have to define the ${SERVICE}_ENABLED env | ||
| variable for all services and update each time we add a new services, | ||
| which can be quite annoying. | ||
|
|
||
| *Activate based on env variables* | ||
| Introduce a new WEBVALVE_SERVICE_ENABLED_DEFAULT env variable that | ||
| controls the default service enabled behavior, or the "mode" webvalve | ||
| runs in: on and allowing traffic by default, or on and intercepting | ||
| traffic by default. If the WEBVALVE_ENABLED variable is unset, don't | ||
| activate the lib. If WEBVALVE_ENABLED is set to truthy (1/t/true) and | ||
| WEBVALVE_SERVICE_ENABLED_DEFAULT is unset then load in passthru mode, | ||
| allowing fakes to be toggled on explicitly via their ${SERVICE}_ENABLED | ||
| env variable. If WEBVALVE_ENABLED is explicitly set to to truthy and | ||
| WEBVALVE_SERVICE_ENABLED_DEFAULT is set to falsey (0/f/false) then load | ||
| in intercepting mode, allowing fakes to be toggled off explicitly via | ||
| their ${SERVICE}_ENABLED env variable. | ||
|
|
||
| *Don't support it at all* | ||
| Lastly, a sort of non-option option: don't support faking in | ||
| production-like envs. Don't support any envs other than dev and test. | ||
| Advise that the gem should only go in the dev/test group. This is safest | ||
| for production use, but means that production-like envs that want to | ||
| swap out real versions of services would have to do it by actually | ||
| deploying a version of their fake service and connecting to it out of | ||
| process as if it were a real service. This is nice from the "make it | ||
| real" angle, but introduces quite a bit of overhead and it's | ||
| well-aligned with the WebValve philosophy of making things convenient | ||
| and as isolated as possible. | ||
|
|
||
| ## Decision | ||
|
|
||
| We chose the "Activate based on env variables" approach. It introduces | ||
| more complexity to this library, but support for these use-cases feels | ||
| worth it. We're not the happiest with having | ||
| `WEBVALVE_SERVICE_ENABLED_DEFAULT` as an env variable name, but I think | ||
| that we can document it clearly to head off confusion for the power | ||
| users that actually want to utilize WebValve in production-like | ||
| environments. For the standard user, nothing about how they're currently | ||
| using WebValve will change. | ||
|
|
||
| Here's a summary of the behavior based on environment. | ||
|
|
||
| When the env is test/development | ||
| * webvalve is enabled and always runs in intercepting mode where | ||
| services are intercepted by default | ||
|
|
||
| When the env is NOT test/development, e.g. production | ||
| * webvalve is disabled unless WEBVALVE_ENABLED=1/t/true | ||
| * when WEBVALVE_ENABLED is truthy, webvalve is enabled in allowing mode | ||
| where all traffic is allowed by default | ||
| * $SERVICE_ENABLED=0/f/false can be used to switch a service into intercepting | ||
| mode, i.e. enable the fake service | ||
| * when WEBVALVE_ENABLED is truthy and | ||
| WEBVALVE_SERVICE_ENABLED_DEFAULT=0/f/false then webvalve is enabled in | ||
| intercepting mode where all traffic is intercepted by default | ||
| * $SERVICE_ENABLED=1/t/true can be used to switch a service into | ||
| allowing mode, i.e. allow the traffic to that service to go through to | ||
| the internet | ||
|
|
||
| ## Consequences | ||
|
|
||
| More complexity to manage in this library. | ||
|
|
||
| Controlling WebValve activation via an ENV variable, makes it slightly | ||
| easier to unintentionally enable WebValve in production. | ||
|
|
||
| The current behavior of WEBVALVE_ENABLED in production is slightly | ||
| altered: by default we will allow all traffic in production. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,10 @@ | |
| require 'set' | ||
|
|
||
| module WebValve | ||
| ALWAYS_ENABLED_ENVS = %w(development test).freeze | ||
| ENABLED_VALUES = %w(1 t true).freeze | ||
| DISABLED_VALUES = %w(0 f false).freeze | ||
|
|
||
| # @api private | ||
| class Manager | ||
| include Singleton | ||
|
|
@@ -19,16 +23,46 @@ def allow_url(url) | |
| end | ||
|
|
||
| def setup | ||
| fake_service_configs.each do |config| | ||
| if config.should_intercept? | ||
| webmock_service config | ||
| else | ||
| allowlist_service config | ||
| return unless enabled? | ||
|
|
||
| if intercepting? | ||
| fake_service_configs.each do |config| | ||
| if !WebValve.env.test? && config.explicitly_enabled? | ||
| allowlist_service config | ||
| else | ||
| webmock_service config | ||
| end | ||
| end | ||
| WebMock.disable_net_connect! webmock_disable_options | ||
| WebMock.enable! | ||
| end | ||
|
|
||
| if allowing? | ||
| fake_service_configs.each do |config| | ||
| if config.explicitly_disabled? | ||
| webmock_service config | ||
| end | ||
| end | ||
| if fake_service_configs.any?(&:explicitly_disabled?) | ||
| WebMock.allow_net_connect! | ||
| WebMock.enable! | ||
| end | ||
| end | ||
| end | ||
|
|
||
| # @api private | ||
| def enabled? | ||
| in_always_intercepting_env? || explicitly_enabled? | ||
| end | ||
|
|
||
| # @api private | ||
| def intercepting? | ||
| in_always_intercepting_env? || (explicitly_enabled? && !services_enabled_by_default?) | ||
| end | ||
|
|
||
| WebMock.enable! | ||
| WebMock.disable_net_connect! webmock_disable_options | ||
| # @api private | ||
| def allowing? | ||
| !in_always_intercepting_env? && explicitly_enabled? && services_enabled_by_default? | ||
| end | ||
|
|
||
| # @api private | ||
|
|
@@ -49,6 +83,38 @@ def allowlisted_urls | |
|
|
||
| private | ||
|
|
||
| def explicitly_enabled? | ||
| ENABLED_VALUES.include?(ENV['WEBVALVE_ENABLED']) | ||
| end | ||
|
|
||
| def services_enabled_by_default? | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if in some earlier routine it would make sense to do something like this: def set_defaults!
if Webvalve.env.in?(ALWAYS_ENABLED_ENVS)
if ENV.key? 'WEBVALVE_SERVICE_ENABLED_DEFAULT'
WebValve.logger.warn SERVICE_ENABLED_DEFAULT_WARNING
end
if ENV.key? 'WEBVALVE_ENABLED'
WebValve.logger.warn WEBVALVE_ENABLED_WARNING
end
ENV['WEBVALVE_ENABLED'] = '1'
ENV['WEBVALVE_SERVICE_ENABLED_DEFAULT'] = '0'
else
ENV['WEBVALVE_ENABLED'] ||= '0'
ENV['WEBVALVE_SERVICE_ENABLED_DEFAULT'] ||= '1'
end
endAnd then inside of def enabled?
ENABLED_VALUES.include?(ENV.fetch('WEBVALVE_ENABLED'))
end
def services_enabled_by_default?
ENABLED_VALUES.include?(ENV.fetch('WEBVALVE_SERVICE_ENABLED_DEFAULT'))
end
def allowing?
enabled? && services_enabled_by_default?
end
def intercepting?
enabled? && !services_enabled_by_default?
end |
||
| if WebValve.env.in?(ALWAYS_ENABLED_ENVS) | ||
| if ENV.key? 'WEBVALVE_SERVICE_ENABLED_DEFAULT' | ||
| WebValve.logger.warn(<<~MESSAGE) | ||
| WARNING: Ignoring WEBVALVE_SERVICE_ENABLED_DEFAULT environment variable setting (#{ENV['WEBVALVE_SERVICE_ENABLED_DEFAULT']}) | ||
| WebValve is always enabled in intercepting mode in development and test environments. | ||
| MESSAGE | ||
| end | ||
| false | ||
| else | ||
| ENABLED_VALUES.include?(ENV.fetch('WEBVALVE_SERVICE_ENABLED_DEFAULT', '1')) | ||
| end | ||
| end | ||
|
|
||
| def in_always_intercepting_env? | ||
| if WebValve.env.in?(ALWAYS_ENABLED_ENVS) | ||
| if ENV.key? 'WEBVALVE_ENABLED' | ||
| WebValve.logger.warn(<<~MESSAGE) | ||
| WARNING: Ignoring WEBVALVE_ENABLED environment variable setting (#{ENV['WEBVALVE_ENABLED']}) | ||
| WebValve is always enabled in development and test environments. | ||
| MESSAGE | ||
| end | ||
| true | ||
| else | ||
| false | ||
| end | ||
| end | ||
|
|
||
| def webmock_disable_options | ||
| { allow_localhost: true }.tap do |opts| | ||
| opts[:allow] = allowlisted_url_regexps unless WebValve.env.test? | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| module WebValve | ||
| VERSION = "0.10.0" | ||
| VERSION = "0.11.0" | ||
| end |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If I pass
SERVICE_ENABLED=TRUE, the way this is written, bothexplicitly_enabled?andexplicitly_disabled?will returnfalse, right?This might be why ActiveModel only includes FALSE_VALUES -- everything else (aside from
niland "") means true. https://github.com/rails/rails/blob/master/activemodel/lib/active_model/type/boolean.rb#L17There 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.
i have some explicit tests somewhere in here that show that a value like
"this"is not considered truthy and i think the same for falsey for these methods. i'm happy to change that behavior, but what i was going for here is that the env variable being set doesn't do anything unless it's properly set to a true-like value or a false-like value. thoughts?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.
Got it -- so we say that
SERVICE_ENABLED=something_invalidis neither explicitly enabled, nor explicitly disabled -- I buy it, but it doesn't feel like a super necessary distinction (like, as an end-developer I wouldn't be surprised ifSERVICE_ENABLED=bananagot cast totrue) and my more general inclination would be to bool-cast ENV vars the same way everywhere and reduce the local complexity in basically any gem I write. (Like, normally I'd rely on ActiveModel's Boolean type, but that's not available in every lib. I wish Ruby had a built-in shorthand for boolean ENV vars.)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.
Yea. I'm down to change course in this PR. So what casts to false? Only "", nil, "f", "false", false, "0", and 0?
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.
Yup, seems reasonable. Since this is only stringy I don't think you need
falseand0, unless you are writing a more generic caster.