Skip to content
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

Modify GitHubBuilder to resolve user credentials from the system environ... #131

Merged
merged 2 commits into from
Sep 30, 2014

Conversation

mocleiri
Copy link
Collaborator

...ment

Using the Jenkins EnvInject or Credentials Binding Plugins its possible to
pass credentials as Environment Variables.

Its useful for Github.connect() to be able to directly read the values of the
login, password and oauth properties from the environment.

This commit modifies the base Github.connect() method to resolve credentials
in two steps:

  1. ~/.github credentials file if it exists.
  2. login, password or oauth variables from the environment

A further fromEnvironment() method is provided to support
loading from non-standard variable names.

The old Github.connect() method would throw an IOException if the ~/.github file
did not exist. Now it will fail silently instead dropping back to the anonymous
users access level.

@buildhive
Copy link

Kohsuke Kawaguchi » github-api #237 SUCCESS
This pull request looks good
(what's this?)

@mocleiri mocleiri force-pushed the resolve-credentials-from-environment branch from 25d60e0 to 7ad908b Compare September 29, 2014 17:39
@buildhive
Copy link

Kohsuke Kawaguchi » github-api #238 SUCCESS
This pull request looks good
(what's this?)

…ronment

Using the Jenkins EnvInject or Credentials Binding Plugins its possible to
pass credentials as Environment Variables.

Its useful for Github.connect() to be able to directly read the values of the
'login', 'password' and 'oauth' properties directly from the environment.

This commit modifies the base Github.connect() method to resolve credentials
in two steps:

1. ~/.github credentials file if it exists.
2. login, password or oauth variables from the environment

A further fromEnvironment() method is provided to support
loading from non-standard variable names.

The old Github.connect() method would throw an IOException if the ~/.github file
did not exist.  Now it will fail silently instead dropping back to the anonymous
users access level.

Added new unit tests into GitHubTest.
@mocleiri mocleiri force-pushed the resolve-credentials-from-environment branch from 7ad908b to 4d6c5c1 Compare September 29, 2014 17:42
@buildhive
Copy link

Kohsuke Kawaguchi » github-api #239 SUCCESS
This pull request looks good
(what's this?)

@mocleiri
Copy link
Collaborator Author

The first version of this pull request tried to resolve the system environment variables through System.getProperties() instead of System.getenv().

I've rewritten the commit in the pull request to read from the environment.

I copied some code from stack overflow that is used to modify the in memory environment to test that the GitHubBuilder does resolve the values from the environment properly.

@mocleiri
Copy link
Collaborator Author

I hesitate to merge this incase it would break people depending on GitHub.connect() throwing a FileNotFoundException if ~/.github does not exist.

I will plan on adding another commit that will check if the connection could be configured using the environment and if not then still throw the file not found exception.

With the previous change if no credentials were defined Github.connect() would
fall back on an anonymous connection.

This commit changes the behaviour back to what it was before so that if there
are no credentials defined in the ~/.github file and no credentials defined
in the environment an IOException is thrown to alert the method caller.

The caller can call Github.connectAnonymously() if that scenario is allowed.

This should handle most cases unless callers are depending on the
FileNotFoundException being specifically thrown instead of an IOException.
@buildhive
Copy link

Kohsuke Kawaguchi » github-api #242 SUCCESS
This pull request looks good
(what's this?)

mocleiri added a commit that referenced this pull request Sep 30, 2014
…nment

Modify GitHubBuilder to resolve user credentials from the system environ...
@mocleiri mocleiri merged commit 556786f into hub4j:master Sep 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants