(POOLER-71) Add dummy authentication provider #180
Merged
Conversation
This file contains 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
|
👍 |
|
I can rebase onto master if it's easier. |
mckern
suggested changes
Feb 8, 2017
Gemfile
Outdated
|
|
||
| # Evaluate Gemfile.local if it exists | ||
| if File.exists? "#{__FILE__}.local" | ||
| eval(File.read("#{__FILE__}.local"), binding) |
There was a problem hiding this comment.
I'm not generally a fan of the Gemfile.local pattern, but it's not something I feel strongly enough about to push back on.
BUT! You can eschew the passing of binding if you use instance_eval instead:
instance_eval(File.read("#{__FILE__}.local")
There was a problem hiding this comment.
I'll piggyback these comments over to #177
|
@mckern Actually I can do one of a few things. As part of #177 I added a VMPOOLER_DEBUG environment variable.
I'm leaning towards option 3. Fail hard, fail fast. |
glennsarti
force-pushed
the
ticket/maint/add-dummy-auth
branch
from
e847137 to
3c441ac
Compare
|
Rebased off of master now. The PR ordering just served to confuse things. |
Previously it was difficult to do local development as VMPooler requires an LDAP service for authentication. This commit adds a dummy authentication provider. The provider has passes authentication if the username and password are different, and fails if the username and password are the same. This commit also updates the documentation in the config YML file.
mckern
force-pushed
the
ticket/maint/add-dummy-auth
branch
from
a82b243 to
2c74f4f
Compare
mckern
reviewed
Feb 10, 2017
lib/vmpooler.rb
Outdated
| @@ -32,6 +32,17 @@ def self.config(filepath='vmpooler.yaml') | |||
| parsed_config = YAML.load_file(config_file) | |||
| end | |||
|
|
|||
| # Bail out if someone attempts to start vmpooler with dummy authentication | |||
| # without enbaling debug mode. | |||
| if parsed_config[:auth]['provider'] == 'dummy' && ENV['VMPOOLER_DEBUG'] | |||
There was a problem hiding this comment.
No, this needs to be fixed. Good job, me!
mckern
force-pushed
the
ticket/maint/add-dummy-auth
branch
from
c8e591d to
c1216d4
Compare
If a user attempts to start vmpooler using dummy authentication without setting the environment variable VMPOOLER_DEBUG, the vmpooler will now refuse to start.
mckern
force-pushed
the
ticket/maint/add-dummy-auth
branch
from
c1216d4 to
eb67cca
Compare
|
Changes are fine by me. LGTM |
mckern
approved these changes
Feb 10, 2017
underscorgan
approved these changes
Feb 10, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously it was difficult to do local development as VMPooler requires an LDAP
service for authentication. This commit adds a dummy authentication provider.
The provider has passes authentication if the username and password are
different, and fails if the username and password are the same. This commit
also updates the documentation in the config YML file.